Skip to content

add webauthn public key and signature type#97

Merged
spoonincode merged 5 commits intomasterfrom
webauthn
Jun 28, 2019
Merged

add webauthn public key and signature type#97
spoonincode merged 5 commits intomasterfrom
webauthn

Conversation

@spoonincode
Copy link
Copy Markdown
Contributor

@spoonincode spoonincode commented May 24, 2019

Add webauthn key and signature support to fc. Public keys are comprised of three components: the compressed r1 public key, the webauthn user presence requirement, and the relying party ID (origin). Signatures are comprised of three components: the compressed signature, authenticator data blob from the webauthn authenticator, and client json string which is from the browser.

JSON parsing is required to extract the webauthn challenge, origin, and signature type from the client json. RapidJSON is brought in as a submodule strictly for use with webauthn json parsing.

A fair number of changes are added not strictly for webauthn support in fc but also to facilitate usage of the webauthn key type in eosio where it will be gated via a protocol feature and special handling for chainbase storage of webauthn keys due to the variable length nature of them.

It's important to remember that this is consensus changing so it should be merged to fc's master only on the cusp of the corresponding PR in eos that gates webauthn support via a protocol feature.

Copy link
Copy Markdown
Contributor

@b1bart b1bart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot remember why we went for the "shim" pattern for R1 and K1, this does not use the same pattern and perhaps thats ok. At the very least, it maintains our external interfaces (serialization and variant) expectations so, we should be able to move to the "shim" pattern if we remember why we did it in the first place.

${CMAKE_CURRENT_SOURCE_DIR}/vendor/websocketpp
)

# try and make this very clear that this json parser is intended only for webauthn parsing..
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems overly punitive. Is there a substantive reason that we would have to whitelist use of this json parser?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since JSON parsing is consensus I am concerned about the version of rapidjson being accidentally upgraded in the future and breaking that consensus. So, I tried to make it very clear this version of rapidjson is strictly for webauthn parsing: it's in a directory called webauthn_json and it can only be included from elliptic_webauthn.cpp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants