Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SRP: Use constant time comparisons of secrets #19

Open
jpgoldberg opened this issue May 10, 2019 · 5 comments
Open

SRP: Use constant time comparisons of secrets #19

jpgoldberg opened this issue May 10, 2019 · 5 comments

Comments

@jpgoldberg
Copy link

In srp/src/server.rs for example, we see

if user_proof == d.result().as_slice() {

where the types are byte slices, &[u8]. I suspect that the same kind of thing appears throughout the code (although I haven't checked).

That will result in a non-constant time comparison, and expose this to timing attacks.

I am new to Rust, so take my suggestion with a large grain of salt. It seems that if we create a trait for secrets and then implement comparison tests for that trait with constant time checks, we could use Rust's type system to enforce that we always have constant time comparisons.

@jpgoldberg
Copy link
Author

Ah. I didn't see that the readme explicitly says this doesn't do constant time comparisons.

@jpgoldberg
Copy link
Author

And so closing.

@newpavlov
Copy link
Member

I will reopen this issue, as it's quite easy to fix by using subtle as we do in other crates.

@newpavlov newpavlov reopened this May 10, 2019
@warner
Copy link
Member

warner commented Aug 7, 2019

There's no strong reason to not use a constant-time comparison, but:

  • the real threat is the time spent in the powm(b) function, where you're taking the secret server b value and taking a variable amount of time to convert it to the public b_pub value. Making that modular-exponentation take constant time is pretty difficult (the mitigation people usually try is to blind it with some random value, do the modexp, then remove the blinding)
    • but the higher-level protocol should never call SrpServer::new() more than once with a given secret b, so the attacker should never get more than one timing sample for a single value. The API might be safer to generate b internally rather than passing it in, to remove the opportunity for mistakes.
  • there's no reason to allow more than one verify() call per SrpServer instance, so again the attacker should only get one timing sample
  • the hash operation includes the secret key, so to do this right, you need a constant-time hash function, which is also non-trivial

I think PAKEs in general are safer against timing attacks because all the secrets tend to be single-use.

@warner warner changed the title Use constant time comparisons of secrets SRP: Use constant time comparisons of secrets Aug 8, 2019
@jbis9051
Copy link
Contributor

#79 adds timing safe comparisons for client and server proof verification. It does not fix the other timing safe issues described by @warner due to the complexity. Although I agree, I don't think a timing attack is actually feasible for the reasons @warner mentioned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants