Public key improvements #136
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
infinisil
force-pushed
the
public-key-refactor
branch
2 times, most recently
from
471923e
to
37124f6
Compare
infinisil
added a commit
that referenced
this pull request
Feb 9, 2022
|
Best reviewed by just looking at the first commit |
ErinvanderVeen
approved these changes
Feb 10, 2022
- Change `CosePublicKey` to just be a combination of `PublicKey` and `CoseSignAlg`, but using a smart constructor to ensure that the signature schemes of the two match. This allows removing the public key parameter duplication. Also introduces `PublicKeyWithSignAlg`, which `CosePublicKey` is actually aliased to. - Use `PublicKeyWithSignAlg` as an input to the `verify` function, and use `PublicKeyWithSignAlg` in attestation format statements that need to call this function. - Introduce `UncheckedPublicKey` and a smart "constructor" `checkPublicKey` to turn it into a `PublicKey`, which makes sure that only valid public keys can be used. For example this implements a check that for ECDSA keys, the point is on the curve. - Use `Integer`s to represent ECDSA key coordinates. Previously this was not done because the spec said to persist leading zeroes. But we can just calculate the leading zeroes ourselves by looking at the number of bytes that the curve uses
Renames Crypto.WebAuthn.Cose.{Key -> PublicKeyWithSignAlg}
Renames Crypto.WebAuthn.Cose.{Algorithm -> SignAlg}
This better reflects the types these modules define
infinisil
force-pushed
the
public-key-refactor
branch
from
6ae82f0
to
1183083
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While trying to fix #133, I noticed some other things that could be improved on the public key handling. For now only a draft, it will change a bunch still I think. The main reasons for doing this:
CosePublicKeywas an inlined combination ofCoseSignAlgandPublicKey, therefore it duplicated all the fields, which is kind of badverifyonly return an error when the error is caused by the signature or the message. Previously it would also return an error if the public key doesn't match the passed signature algorithm. This error therefore needs to be handled earlier.