-
Notifications
You must be signed in to change notification settings - Fork 136
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 multi-range check to assertAlmostReduced
#1703
Conversation
7ba0cee
to
5e07eb9
Compare
9a672dc
to
e79912e
Compare
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.
@MartinMinkov the change looks good to me now, but it's breaking and we should ship it in a backwards-compatible way. check()
is called internally just by using the overall class as method input. So it's the class that we have to create an alternative version for.
How about this:
- Leave
ForeignCurve
class untouched - New
ForeignCurveV2
which extendsForeignCurve
and just overrides thecheck()
method - new
createForeignCurveV2()
which is exactly likecreateForeignCurve()
but usesForeignCurveV2
- deprecate
createForeignCurve()
, move doccomments over tocreateForeignCurveV2()
And same for EcdsaSignature
!
a7d29b2
to
60f70cb
Compare
…d EcdsaSignatureV2 classes
1562250
to
dd7d8a2
Compare
…e, ForeignCurve and EcdsaSignature due to security vulnerability
dd7d8a2
to
c37bf6a
Compare
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.
Perfect!
Summary
Addressing an issue with
EcsdaSignature
andForeignCurve
overridingcheck()
to useassertAlmostReduced
. This skips the option to multi-range check all the input variables, which can lead to passing assertions if a malicious prover wishes to do so.Changes
check
functions ofEcdsaSignature
andForeignCurve