-
Notifications
You must be signed in to change notification settings - Fork 106
WebAuthn key format #1318
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
WebAuthn key format #1318
Conversation
src/Pact/Types/Command.hs
Outdated
@@ -95,7 +102,7 @@ data Command a = Command | |||
, _cmdSigs :: ![UserSig] | |||
, _cmdHash :: !PactHash | |||
} deriving (Eq,Show,Ord,Generic,Functor,Foldable,Traversable) | |||
instance (Serialize a) => Serialize (Command a) | |||
-- instance (Serialize a) => Serialize (Command a) |
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 instance seemed unused, and it's very risky to provide this instance, but I don't want to break downstream if I don't have to.
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.
@jmcardon do you want this instance back? If so maybe we should write up a ticket for adding this back and using an explicit definition instead of using deriving.
eebc66a
to
bfd8ee1
Compare
bfd8ee1
to
b0456fd
Compare
affb653
to
ad875c6
Compare
Co-authored-by: Emily Pillmore <[email protected]>
src/Pact/Types/Command.hs
Outdated
@@ -95,7 +102,7 @@ data Command a = Command | |||
, _cmdSigs :: ![UserSig] | |||
, _cmdHash :: !PactHash | |||
} deriving (Eq,Show,Ord,Generic,Functor,Foldable,Traversable) | |||
instance (Serialize a) => Serialize (Command a) | |||
-- instance (Serialize a) => Serialize (Command a) |
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.
@jmcardon do you want this instance back? If so maybe we should write up a ticket for adding this back and using an explicit definition instead of using deriving.
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.
LGTM
This PR allows WebAuthn keys in keysets.
Public keys of different formats are distinguished by their prefix.
The new enum
DynKeyPair
ranges over Ed25519 keypairs and WebAuthn keypairs. Our client-side function for building commands (mkCommand
,mkCommand'
) have counterparts that acceptDynKeyPair
instead ofEd25519KeyPair
:mkCommandWithDynKeys
. The new command generators will perform different signing algorithms and produce different signatures, depending on the type of keypair used. This distinction is only meaningful in tests, because real users will never have a WebAuthn private key. Real users using WebAuthn-signed transactions would be using a web client and an authenticator device that hold the secret key on the user's behalf.The PR adds a lot of new functions for generating, parsing and printing WebAuthn keys. The functions applying to WebAuthn private keys are only used for testing.
The following repl session demonstrates that "WEBAUTHN-" prefixed keys pass format enforcement and are usable as keyset guards:
PR checklist:
cabal run tests
. If they pass locally, docs are generated.Additionally, please justify why you should or should not do the following: