-
Notifications
You must be signed in to change notification settings - Fork 311
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
Support explicit curve params #252
Conversation
This pull request introduces 3 alerts when merging b6755ff into b4c4203 - view on LGTM.com new alerts:
|
2b8db09
to
2f355cc
Compare
src/ecdsa/ellipticcurve.py
Outdated
:term:`uncompressed`, :term:`compressed`, and :term:`hybrid` encodings. | ||
|
||
Note: generally you will want to call the from_bytes method of | ||
either a child class, either PointJacobi or Point. |
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.
Too much "either", please rephrase.
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.
fixed
src/ecdsa/ellipticcurve.py
Outdated
:return: x and y coordinates of the encoded point | ||
:rtype: tuple(int, int) | ||
""" | ||
if valid_encodings is None: |
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.
Also make sure there is nothing unexpected in valid_encodings
?
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.
fixed
src/ecdsa/test_curves.py
Outdated
@classmethod | ||
def setUpClass(cls): | ||
# minimal, but with cofactor (excludes seed when compared to | ||
# OpenSSL output |
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.
missing )
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.
fixed
For decoding points it's not necessary to have all the data useful for decoding public keys. This will also make it possible to decode explicit EC parameters, as decoding of a public key requires knowledge of the curve's base point and the base point is in defined in the parameters, creating a chicken and an egg problem with using the VerifyingKey.from_string() to parse the base point.
as some standards, like PKIX in X.509 certificates, don't allow for explicit curve paramters, provide an API that limits the supported formats
725b3c8
to
77cabc0
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.
Re-reviewed the code around my previous comments, no objections.
Add support for reading and writing curve parameters.
TODO:
fixes #39