Hacker News Reader

Comment by Chris911 | Hacker News Reader

The ACL implementation: https://github.com/antirez/redis/blob/unstable/src/acl.c

Love the code and how it is explained! Beautiful.

Not a security expert, but in time_independent_strcmp(), first comment about strlen()s: couldn't the attacker use his own accounts with known passwords to determine the length of some other user's password? Also, given the name of this function I would expect the comparison to be time independent, even if attacker can change both strings' lengths... Or am I missing something? Haven't touched C in a loooong time... :)

Hello, the attacker here can control only one string: we want the time taken by the function to be independent from the POV of the string provided by the user. That is, we don't want that the user-controlled string can affect the time the function takes. The other string is the string set inside the database. About information leaks about the length, that would be completely acceptable: anyway the user is very likely to just it the user password length by extracting a random number, there is no real protection there. The problem of this kind of timing attacks is that it can leak the actual user string content. Such function should hopefully prevent this problem.

The comments in the code make it seem like absolutely no information about the secret is leaked:

> /* Again the time of the following two copies is proportional to > * len(a) + len(b) so no info is leaked. */ > memcpy(bufa,a,alen); > memcpy(bufb,b,blen);

If the attacker controls one of the inputs, the execution time reveals something about the length of the other input, right?

Or maybe you just meant that the length is leaked by the contents are not leaked? (I agree that it's generally considered ok for "timing-safe equals" functions to leak the length of the secret. But if you ARE allowed to leak the length, you can simplify the code by just checking the length in the beginning and exiting if they're not equal.)

And if you don't want to leak the length, it's easy: pre-SHA-512 the secret and then only compare hashes instead of comparing the full strings.

Thank you for taking the time to explain. Also the fact that a total stranger to this code (like me) can even reason about it, is incredible.

My point was not so much that there is a bug, but more that the assumptions that this function makes are not obvious (to me at least) from its signature or the header comment. I could easily imagine someone using this function assuming that it does, you know, time insensitive string comparison, but this is only true in a very limited context. Even renaming vars to `a_unchangeable` and `b_user_controlled` (or a comment) would help. But again, this is just a very minor point from a passing stranger, so feel free to ignore it. :)

And thanks for providing Redis!

Reply on news.ycombinator.com