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
Add scalar blinding for ecmult_gen() #190
Conversation
This addresses issue #186. |
I'd like to see this rebased on top of #208. |
Please rebase (I can do so, if you want to). |
Updated. If this looks good I'll add another commit that adds an additional re-projection with H(gnb) in ecmult_gen(). |
To clarify, the point of difference here is that: And further, that the subsequent point scalar-multiplication And finally, that the point addition |
Multipliers are much more likely to have sidechannels in general owing to their greater complexity and internal architecture. They have have, on various real systems, even have had data dependent timing, fully visible from software (though I am not aware of this being the case case for any current common desktop hardware; I've experienced this on ppc440, arm7tdmi, and some other processors). More importantly, the blinding operation is tiny and very fast, compared to a huge number of multiply operations each of which could leak. In practice it would plausibly be harder to exploit any leak from the blinding because its so fast... but ultimately if additions leak, well, they leak and the library will thus leak regardless of what we do. b is (hopefully) unknown, so n has uniform probability if you only learn (n-b). If you can observe all behavior flawlessly then the protection may not help, but each randomization is cumulative and if you miss only one (e.g. the first one) you're out of luck if your only leak is the multipliers. It's not a guarantee, but we practice defense in depth, and this change does not produce a measurable slowdown (or at least it's below the timing noise on my laptop; the effect should be tiny). (The next commit will likely be a more measurable slowdown but still a small one). |
Thanks for the explanation @gmaxwell. |
But if you learn Granted, it is one more step, but. |
It doesn't compute bG every time (that would be slow and obviously have more leaks). It computes bG at start, and forward chains any time b is changed. |
Ah! Thanks for clearing that up :) |
@sipa API question-- instead of a explicit reset, which is a pretty obscure option which I doubt wouldn't ever be used except for testing infrastructure, what would you think of me using a null seed pointer to reset? |
Oh yes, using NULL seems fine.
|
@sipa updated for the api change. |
@sipa another behavior question: 'default seed`, zeros or the address of one of the libraries functions? (Advantage being that the state ends up somewhat unpredictable (esp on hosts with ASLR), even if the caller never call randomize; disadvantage is that behavior differs from host to host even if randomize isn't called which could complicate debugging) |
I don't think that using "attempted randomness" makes sense if no
guarantees can be made. It's not something to rely on, so let's just use
zeroes.
|
+1 to all zeros. Uninitialized memory or externally 'sourced' memory may lead to unexpected consequences. |
Perhaps you can add a test to see whether the Z coordinate of the result
differs after signing with seeded context?
|
@dcousens My suggestion was not uninitialized memory. Use of uninitialized memory would be undefined behavior and would permit the compiler to instead substitute formatting the user's disk. What I would have been completely safe but also of very little value (or I would have just done it, instead of asking). :) @sipa: sounds good. I think I still need to do scalar equals functions in the tests. |
@@ -1,5 +1,5 @@ | |||
/********************************************************************** | |||
* Copyright (c) 2013, 2014 Pieter Wuille * | |||
* Copyright (c) 2013, 2014, 2015 Pieter Wuille, Gregory Maxwell * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About time.
ACK apart from nits. I see no measurable slowdown (perhaps even a slight speedup, wtf?). |
Updated. |
/* The prior blinding value (if not reset) is chained forward by including it in the hash. */ | ||
secp256k1_scalar_get_b32(nonce32, &ctx->blind); | ||
/** Using a CSPRNG allows a failure free interface, avoids needing large amounts of random data, | ||
* and guards against weak or adversarial seeds. This is a simpler and safer interface then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/then/than/
This computes (n-b)G + bG with random value b, in place of nG in ecmult_gen() for signing. This is intended to reduce exposure to potential power/EMI sidechannels during signing and pubkey generation by blinding the secret value with another value which is hopefully unknown to the attacker. It may not be very helpful if the attacker is able to observe the setup or if even the scalar addition has an unacceptable leak, but it has low overhead in any case and the security should be purely additive on top of the existing defenses against sidechannels.
int retry; | ||
if (!seed32) { | ||
/* When seed is NULL, reset the initial point and blinding value. */ | ||
secp256k1_gej_set_ge(&ctx->initial, &secp256k1_ge_const_g); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This, and the next line are actually redundant. (I'm fine with keeping them for clarity, though).
d227579 Add scalar blinding and a secp256k1_context_randomize() call. (Gregory Maxwell)
ACK |
This computes (n-b)G + bG with random value b, in place of nG.
This is intended to reduce exposure to potential power/EMI sidechannels
during signing and pubkey generation by blinding the secret value with
another value which is hopefully unknown to the attacker.
It may not be very helpful if the attacker is able to observe the setup
or if even the scalar addition has an unacceptable leak, but it has low
overhead in any case and the security should be purely additive on top
of the existing defenses against sidechannels.
For comments right now, there is no interface presented for users to pass in a seed. We should probably change the API to use a context pointer created by start, and could change start to take a random uchar[32] at that time.