5 ms·

What is being complained about here? The obvious thing that stands out to me is potential timing attacks: 'and' doing short circuit eval, and '==' exiting earl

by codys 12y ago

What is being complained about here? The obvious thing that stands out to me is potential timing attacks: 'and' doing short circuit eval, and '==' exiting early when the first byte is non-equal (or the lengths are non-equal).

zaroth 12y ago
You nailed it. In other words, literally everything is wrong with that function.
- michaelmior 12y ago
  I would agree with the other comment that I suspect it would be impossible to get accurate data for a timing attack over Tor. Not only is the connection slow, but my experience has been that bandwidth and latency can be highly variable.
  - handsomeransoms 12y ago
    With enough sampling, it should be possible. Additionally, the values you are trying to infer are pretty small (8 bytes each) so that makes it easier.
    - michaelmior 12y ago
      Fair point. I'll admit I'm biased because I've messed around with trying time attacks on my vulnerable code in arguably ideal scenarios and haven't had much luck. So the feasibility of this is questionable to me, but I'm willing to accept that it's an issue. In any case, the above block of code should be easily fixable.
civilian 12y ago
The request is going over tor-- do timing attacks work with that? When I browse with tor my connection is so slow that I don't think I could get good data for timing attacks.
- sp332 12y ago
  Only mentioned in passing, but: Third, we have already accepted that our design is vulnerable to end-to-end timing attacks https://blog.torproject.org/blog/one-cell-enough https://blog.torproject.org/blog/one-cell-enough It doesn't matter if your connection is slow, all that matters is that latency is fairly consistent during the attack. You can use statistics to extract high-confidence data even from noisy signals, if you run the attack long enough to get lots of timing data.
  - bhitov 12y ago
    > Third, we have already accepted that our design is vulnerable to end-to-end timing attacks https://blog.torproject.org/blog/one-cell-enough https://blog.torproject.org/blog/one-cell-enough They are not talking about the same kind of timing attacks. Deanonymization attacks require vastly less latency consistency.
    - sp332 12y ago
      Oh, I see! Thanks for the context.
denom 12y ago
Would a good drop in replacement here be something like hashing the two strings to the same length and using `hmac.compare_digest` ? https://docs.python.org/2/library/hmac.html#hmac.compare_digest https://docs.python.org/2/library/hmac.html#hmac.compare_dig... Honestly, I don't know enough about timing attacks to be sure.
- mbreese 12y ago
  Here's some pseudo code... passed=true for (i=0; i< len(auth_username); i++) { if (i >= len(username) || auth_username[i] != username[i]) { passed = false } } for (i=0; i< len(auth_password); i++) { if (i >= len(password) || auth_password[i] != password[i]) { passed = false } } return passed This could potentially still be used to check the length of the username and password, but that would be more difficult. Hashing the username (like you mentioned) and comparing those character by character (like above) should yield the exact same timing for any inputs.
  - TheLoneWolfling 12y ago
    Note: There is no guarantee that a compiler won't optimize this to non-constant time. And even if it keeps it constant time, a processor may end up running a function like this in non-constant time, due to caching and branch prediction effects.
  - deleted 12y ago
    [deleted]
  - zaroth 12y ago
    Hashing the username (like you mentioned) and comparing those character by character (like above) should yield the exact same timing for any inputs. I like to use a "constant-time compare" against an HMAC of the inputs with a random key. You can initialize the key just once, it doesn't have to be a nonce, just an unknown value. The point is the attacker no longer knows or controls what value is on 'their side' of the comparison, so timing becomes irrelevant. For the comparison itself, I usually see |= and XOR being used and then the final value being checked to be non-zero, rather than a boolean. I think the theory is the compiler is a lot less likely the exit the loop early if you're accumulating with XOR versus a single branch flipping a boolean to be false. Here's a good write-up with some sample code: http://codahale.com/a-lesson-in-timing-attacks/ http://codahale.com/a-lesson-in-timing-attacks/. Note in that write-up they are talking about timing attacks comparing HMAC results, but that is when the attacker has control over the HMAC result not the HMAC input. If the attacker provided value is being fed into HMAC with a secret key, then you are absolutely safe from timing attacks. If the attacker has control over the HMAC result, and you are checking if that HMAC is valid, then you are back to defending against timing attacks by trying to achieve a constant-time compare. The only thing left to leak is the HMAC will clock differently based on the block-length of the input. But block size is rather coarse, and in this case you could limit inputs to be less than one block size in all cases.
micahflee 12y ago
I have doubts that a timing attack would even be exploitable here since it's a hidden service, but I just made the string comparison constant-time to be safe: https://github.com/micahflee/onionshare/issues/3 https://github.com/micahflee/onionshare/issues/3 Keep in mind that the username/password are just hex-encoded 128 bits from /dev/urandom, so they're not guessable at all without some sort of leakage attack, like a timing attack. And if anyone attempts to do a timing attack the person hosting the file will see all the requests scrolling down their terminal in real-time and can always hit ctrl-c. There's also the bit about knowing the hidden service .onion to attack in the first place, which wouldn't be trivial to discover, especially since I envision these to mostly be very short-lived. But all that said, this is great feedback. Keep it coming and feel free to open security issues on github.
- harshreality 12y ago
  What is the semantic difference between username and password? If they're both randomly generated for a particular resource, why not combine them into one access key field?

by codys 12y ago

zaroth 12y ago

You nailed it. In other words, literally everything is wrong with that function.

michaelmior 12y ago
I would agree with the other comment that I suspect it would be impossible to get accurate data for a timing attack over Tor. Not only is the connection slow, but my experience has been that bandwidth and latency can be highly variable.
- handsomeransoms 12y ago
  With enough sampling, it should be possible. Additionally, the values you are trying to infer are pretty small (8 bytes each) so that makes it easier.
  - michaelmior 12y ago
    Fair point. I'll admit I'm biased because I've messed around with trying time attacks on my vulnerable code in arguably ideal scenarios and haven't had much luck. So the feasibility of this is questionable to me, but I'm willing to accept that it's an issue. In any case, the above block of code should be easily fixable.

civilian 12y ago

The request is going over tor-- do timing attacks work with that? When I browse with tor my connection is so slow that I don't think I could get good data for timing attacks.

sp332 12y ago
Only mentioned in passing, but: Third, we have already accepted that our design is vulnerable to end-to-end timing attacks https://blog.torproject.org/blog/one-cell-enough https://blog.torproject.org/blog/one-cell-enough It doesn't matter if your connection is slow, all that matters is that latency is fairly consistent during the attack. You can use statistics to extract high-confidence data even from noisy signals, if you run the attack long enough to get lots of timing data.
- bhitov 12y ago
  > Third, we have already accepted that our design is vulnerable to end-to-end timing attacks https://blog.torproject.org/blog/one-cell-enough https://blog.torproject.org/blog/one-cell-enough They are not talking about the same kind of timing attacks. Deanonymization attacks require vastly less latency consistency.
  - sp332 12y ago
    Oh, I see! Thanks for the context.

denom 12y ago

Would a good drop in replacement here be something like hashing the two strings to the same length and using `hmac.compare_digest` ? https://docs.python.org/2/library/hmac.html#hmac.compare_digest https://docs.python.org/2/library/hmac.html#hmac.compare_dig... Honestly, I don't know enough about timing attacks to be sure.

mbreese 12y ago
Here's some pseudo code... passed=true for (i=0; i< len(auth_username); i++) { if (i >= len(username) || auth_username[i] != username[i]) { passed = false } } for (i=0; i< len(auth_password); i++) { if (i >= len(password) || auth_password[i] != password[i]) { passed = false } } return passed This could potentially still be used to check the length of the username and password, but that would be more difficult. Hashing the username (like you mentioned) and comparing those character by character (like above) should yield the exact same timing for any inputs.
- TheLoneWolfling 12y ago
  Note: There is no guarantee that a compiler won't optimize this to non-constant time. And even if it keeps it constant time, a processor may end up running a function like this in non-constant time, due to caching and branch prediction effects.
- deleted 12y ago
  [deleted]
- zaroth 12y ago
  Hashing the username (like you mentioned) and comparing those character by character (like above) should yield the exact same timing for any inputs. I like to use a "constant-time compare" against an HMAC of the inputs with a random key. You can initialize the key just once, it doesn't have to be a nonce, just an unknown value. The point is the attacker no longer knows or controls what value is on 'their side' of the comparison, so timing becomes irrelevant. For the comparison itself, I usually see |= and XOR being used and then the final value being checked to be non-zero, rather than a boolean. I think the theory is the compiler is a lot less likely the exit the loop early if you're accumulating with XOR versus a single branch flipping a boolean to be false. Here's a good write-up with some sample code: http://codahale.com/a-lesson-in-timing-attacks/ http://codahale.com/a-lesson-in-timing-attacks/. Note in that write-up they are talking about timing attacks comparing HMAC results, but that is when the attacker has control over the HMAC result not the HMAC input. If the attacker provided value is being fed into HMAC with a secret key, then you are absolutely safe from timing attacks. If the attacker has control over the HMAC result, and you are checking if that HMAC is valid, then you are back to defending against timing attacks by trying to achieve a constant-time compare. The only thing left to leak is the HMAC will clock differently based on the block-length of the input. But block size is rather coarse, and in this case you could limit inputs to be less than one block size in all cases.

micahflee 12y ago

I have doubts that a timing attack would even be exploitable here since it's a hidden service, but I just made the string comparison constant-time to be safe: https://github.com/micahflee/onionshare/issues/3 https://github.com/micahflee/onionshare/issues/3 Keep in mind that the username/password are just hex-encoded 128 bits from /dev/urandom, so they're not guessable at all without some sort of leakage attack, like a timing attack. And if anyone attempts to do a timing attack the person hosting the file will see all the requests scrolling down their terminal in real-time and can always hit ctrl-c. There's also the bit about knowing the hidden service .onion to attack in the first place, which wouldn't be trivial to discover, especially since I envision these to mostly be very short-lived. But all that said, this is great feedback. Keep it coming and feel free to open security issues on github.

harshreality 12y ago
What is the semantic difference between username and password? If they're both randomly generated for a particular resource, why not combine them into one access key field?