Page MenuHomePhabricator

Eliminate harmless non-constant time operations on secret data.
ClosedPublic

Authored by deadalnix on Fri, Mar 20, 00:37.

Details

Summary
  • Eliminate harmless non-constant time operations on secret data.

There were several places where the code was non-constant time
for invalid secret inputs. These are harmless under sane use
but get in the way of automatic const-time validation.

(Nonce overflow in signing is not addressed, nor is s==0 in
signing)

  • Adds a declassify operation to aid constant-time analysis.

ECDSA signing has a retry loop for the exceptionally unlikely case
that S==0. S is not a secret at this point and this case is so
rare that it will never be observed but branching on it will trip
up tools analysing if the code is constant time with respect to
secrets.

Derandomized ECDSA can also loop on k being zero or overflowing,
and while k is a secret these cases are too rare (1:2^255) to
ever observe and are also of no concern.

This adds a function for marking memory as no-longer-secret and
sets it up for use with the valgrind memcheck constant-time
test.

This is a backport of secp256k1 PR710

Test Plan
ninja check-secp256k1

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Fri, Mar 20, 00:37
Herald added a reviewer: Restricted Project. · View Herald TranscriptFri, Mar 20, 00:37
Fabien added a subscriber: Fabien.Fri, Mar 20, 18:02

Did you run the Travis build ?
Should we add a cmake option to enable valgrind as well ? That would at least make the CI cover the same tests between the 2 builds.

Fabien accepted this revision.Thu, Mar 26, 16:31

Comments can be addressed later on

This revision is now accepted and ready to land.Thu, Mar 26, 16:31