mgomes / api_auth

HMAC authentication for Rails and HTTP Clients
MIT License
480 stars 147 forks source link

add secure equals method to prevent timing attacks #56

Closed leishman closed 7 years ago

leishman commented 9 years ago

@mgomes @kjg

I updated the signatures_match? method to prevent timing attacks. The equality operator should not be used to compare two strings for authentication in Ruby (I believe). For example:

'abc' == 'abc' will take longer to evaluate than 'def' == 'abc' because the ruby interpreter will return false after analyzing the first bytes of each string in the second example, but will iterate through each byte of the strings in the first example. This information can be used by a malicious actor in what is known as a timing attack.

There are two solutions I know of to protect against this timing attack:

I learned about this attack from a Stanford cryptography professor in this lecture and he recommended the solution I propose here. His example is for Python but I assume that it applies to Ruby as well.

All tests pass.

kjg commented 9 years ago

I had never heard of a timing attack before, that's pretty interesting. Do you think it would be possible to perform a timing attack again api_auth considering the method used to generate the canonical string combined with the fact that the data here is being transmitted over the internet which introduces a lot of variability in time?

RLovelett commented 9 years ago

@kjg watch the lecture in the link. He addresses the Internet latency/variability thing. Short answer: yes it can still be done over the net.

dacamp commented 9 years ago

My understanding is that Benchmark isn't great for testing timing attacks, as it does a fair amount of rounding. I wanted to put the numbers in here anyways in hopes that some solution is implemented to limit risks associated with these types of attacks.

I tested master, @leishman's pull request, and Fast Secure Compare - (see [below](#Fast secure compare)) using the same two randomly generated secrets over 50,000 iterations of ApiAuth.send(:signatures_match?, signed_request, secret_key) where the signed_request remains static and secret_key is either a 'Match' (secret_1) or 'Differ' (secret_2).

Here's the script I used for testing: https://gist.github.com/dacamp/8d970d7065931045d343

Candidate: Local hack using fast_secure_compare

ApiAuth#signatures_match? (50000 iterations) timing attack against:
Secret 1: MVkg7f9VvH4eo1dD74jZX7egmTsLFoWbHeRxBWLpiiSr0UhuuCraEIAM/9k4wk9mcuijPvwTsWlfvBRqHoS78Q==
          Signatures match? true
Secret 2: ZNExKT/H/cHsLLQknAIrXslgFddBhp9T4cPpUCa4061/6Cbyv5mvXBE2ix0Qx3FO600R0oXx532FuhTpEh0jIg==
          Signatures match? false

Rehearsal -------------------------------------------
Match:    2.080000   0.010000   2.090000 (  2.096422)
Differ:   2.030000   0.010000   2.040000 (  2.044381)
---------------------------------- total: 4.130000sec

              user     system      total        real
Match:    1.920000   0.010000   1.930000 (  1.929009)
Differ:   1.970000   0.010000   1.980000 (  1.976619)
Candidate: leishman/api_auth (2fe4df2319)

ApiAuth#signatures_match? (50000 iterations) timing attack against:
Secret 1: MVkg7f9VvH4eo1dD74jZX7egmTsLFoWbHeRxBWLpiiSr0UhuuCraEIAM/9k4wk9mcuijPvwTsWlfvBRqHoS78Q==
          Signatures match? true
Secret 2: ZNExKT/H/cHsLLQknAIrXslgFddBhp9T4cPpUCa4061/6Cbyv5mvXBE2ix0Qx3FO600R0oXx532FuhTpEh0jIg==
          Signatures match? false

Rehearsal -------------------------------------------
Match:    2.250000   0.010000   2.260000 (  2.290130)
Differ:   2.170000   0.020000   2.190000 (  2.206009)
---------------------------------- total: 4.450000sec

              user     system      total        real
Match:    2.070000   0.010000   2.080000 (  2.083388)
Differ:   2.410000   0.010000   2.420000 (  2.441620)
Candidate: mgomes/api_auth (master)

ApiAuth#signatures_match? (50000 iterations) timing attack against:
Secret 1: MVkg7f9VvH4eo1dD74jZX7egmTsLFoWbHeRxBWLpiiSr0UhuuCraEIAM/9k4wk9mcuijPvwTsWlfvBRqHoS78Q==
          Signatures match? true
Secret 2: ZNExKT/H/cHsLLQknAIrXslgFddBhp9T4cPpUCa4061/6Cbyv5mvXBE2ix0Qx3FO600R0oXx532FuhTpEh0jIg==
          Signatures match? false

Rehearsal -------------------------------------------
Match:    2.130000   0.020000   2.150000 (  2.168062)
Differ:   2.230000   0.010000   2.240000 (  2.283192)
---------------------------------- total: 4.390000sec

              user     system      total        real
Match:    2.110000   0.010000   2.120000 (  2.121405)
Differ:   2.020000   0.010000   2.030000 (  2.032040)
Fast secure compare

Url: https://github.com/daxtens/fast_secure_compare

Method Hack:

diff --git a/lib/api_auth/base.rb b/lib/api_auth/base.rb
index 13142be..694fedb 100644
--- a/lib/api_auth/base.rb
+++ b/lib/api_auth/base.rb
@@ -8,6 +8,7 @@
 # Rails ActiveResource, it will integrate with that. It will even generate the
 # secret keys necessary for your clients to sign their requests.
 module ApiAuth
+  require 'fast_secure_compare'

   class << self

@@ -78,7 +79,7 @@ module ApiAuth
       if match_data = parse_auth_header(headers.authorization_header)
         hmac_from_user = match_data[2]
         hmac_of_request = hmac_signature(request, secret_key)
-        return secure_equals?(hmac_from_user, hmac_of_request, secret_key)
+        return FastSecureCompare.compare(hmac_from_user, hmac_of_request)
       end
       false
     end
johnmcconnell commented 9 years ago

@mgomes @leishman

I'm not seeing how this causes an exploit. Using the timing attack you can find calculated_hmac using a time of calculation, but that is unencrypted in the request headers anyway.

In other words, I'm not seeing how using the attack can cause someone to find the secret_key (or edit the message, or some other vulnerability).

For example, suppose you want to find the secret key. You start with 'a' * length and you find 'd' is the fastest first character. So your secret key becomes 'd' + 'a' * (length - 1).

However, iterating through the next character doesn't help you because changing the next character causes a significant change in the calculated hmac_signature.

More formally signature_hmac(message, 'd' + 'a' * (length - 1)) =~ signature_hmac(message, secret_key) does not guarantee or even correlate with the secret_key being 'd' + more_characters.

will0 commented 7 years ago

Howdy! I'm interested in getting this resolved, and I'd be happy to take on resolving the conflicts. There are a couple of other related issues (happy to say if you're comfortable with this channel) in HEAD I'm interested in helping to resolve that collectively make attack easier

johnmcconnell commented 7 years ago

@will0 I think there is a PR that is already up.

Also, I realized secure comp is needed to prevent forging a request

leishman commented 7 years ago

@johnmcconnell Just saw your comment from last year. The attack is not about finding the key, it's about brute-forcing the HMAC itself.

If the HMAC is unencrypted in a request header, then it's vulnerable to a MITM. Edit: but that's why we use TLS

will0 commented 7 years ago

@johnmcconnell do you mean this PR that is up?

johnmcconnell commented 7 years ago

@will0 On may 23rd a pull request was created to solve this issue look at: #110

will0 commented 7 years ago

@johnmcconnell thank you for pointing that out 👍

kjg commented 7 years ago

Fixed by #133 Thanks!