3 ms·
It seems like good practice is to check data retrieved from the cache is what is expected, e.g. the user id from the cache matches the logged-in user id. Unfor
by rrwo 4y ago
It seems like good practice is to check data retrieved from the cache is what is expected, e.g. the user id from the cache matches the logged-in user id.
Unfortunately, most devs don't think there is ever a need to check that until it fails.
- 3pt14159 4y agoI always put the user id in the cache name. Something like: user_profile_bio.[user_id] If that returns the wrong bio from Redis, then something is broken in Redis, no? Or are you suggesting that I create some sort of format that duplicates the user id in the data itself?
- cr3ative 4y agoWorks great until the Vary configuration on your CDN accidentally drops that.
- contravariant 4y agoShould you add a user_id in the data and check it's the one you want? Yes, I think that is exactly what they're saying, relying on the caching mechanism to handle your data protection sounds like it may be one of the steps involved in the bug Royal Mail is facing. Undoubtedly you are sure this won't ever go wrong, but all it takes is someone else to cache something at user_profile_bio.[customer_id] and you've got a problem (at best it won't work, at worst you'll get the wrong user profile).
- rrwo 4y agoYes. Adding a small amount of redundant data to your cache as a safety check check is important. Imagine instead of a user profile at Royal Mail this was patient data in a hospital. Getting this information wrong could endanger someone's life. And mistakes with private data that isn't safety critical can still cost a hefty fine from a government data protection office, or bad publicity and damage to your reputation.
- 3pt14159 4y agoI dunno, I guess the same could be said about your database then, no? If you're doing: SELECT * FROM serious_money_table WHERE user_id = :user_id; How is it all that different with Redis? Someone can mistakenly send in a :customer_id when writing records, but I don't really know anyone out there that also stores user_id into every table they query user_id from.
- spullara 4y agoIf you are querying for user_id by definition the table contains user_id. I think what they are suggesting is checking that the data returned from the query has the correct user_id. No one would do that because if you can't trust your database to answer queries correctly you have more problems than just that. My guess with the Royal Mail is that it is probably something dumb like using a field in (Java) servlet to hold the data which is then shared across all sessions. I have seen that multiple times in my career. People don't notice when testing as there is only one session at a time.
- contravariant 4y agoI'll be honest I don't have much experience with Redis specifically, I do however think that the caching layer is likely the wrong place to handle data security. It seems too easy for someone to accidentally circumvent the cache and thereby the security. I do think that organising the cache to catch problems early is a good idea though. The thing is that a function like def get_profile(user_id): profile = fetch_profile(user_id) assert profile.user_id == user_id return user_id makes it a lot harder to fuck up, doesn't matter if you swap some ids somewhere (i.e. fetch_profile actually uses the customer id, not the user_id), or if the cache fucks up, or if you accidentally wrote 'SELECT * FROM user_profile WHERE user_id = user_id'.
- madaxe_again 4y agoWorks fine until user_id is null for whatever reason, or you hit the length limit on your cache key and start losing the end of it, or until your cache server decides to just start serving up the same content for every key, because y’know, software (had this happen with memcached a long time back due to a tcp timing bug and a corrupted packet) and then you’re in the soup.