This repository has been archived by the owner on Aug 2, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
…py of all the structures or rely on custom comparators. It will now store the needed sorting information (where tiebreaking is explicit when constructing the map) and a lambda that evaluates the rest of the needs of `::satisfies(...)`. This required making some changes to allow for any "key weight" type in the `weight_tally_visitor`. The order of evaluation exactly matches the previous implementation.
spoonincode
added
the
CONSENSUS
Introduces a change that may modify consensus protocol rules on an existing blockchain.
label
May 27, 2019
…void implicit casting to uint. Also fix drift in tester that was passing compression_type as the subjective limit for signatures
…c_key_type to shared_key_weight & shared_public_key within shared_authority
b1bart
approved these changes
Jun 12, 2019
@@ -11,6 +11,78 @@ | |||
|
|||
namespace eosio { namespace chain { | |||
|
|||
using shared_public_key_data = fc::static_variant<fc::ecc::public_key_shim, fc::crypto::r1::public_key_shim, shared_string>; | |||
|
|||
template<class... Ts> struct overloaded : Ts... { using Ts::operator()...; }; |
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.
I think we should consider moving struct overloaded
to at least types.hpp
or, more ideally, fc/static_variant.hpp
it seems broadly useful and generalized
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
CONSENSUS
Introduces a change that may modify consensus protocol rules on an existing blockchain.
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.
Change Description
This PR adds a third key & signature type to eosio: WebAuthn. The attractive feature of WebAuthn is being able to use a hardware signing device (a WebAuthn authenticator -- like a YubiKey 5 for example) in a browser without browser extensions or other installed software. This is, of course, a consensus affecting change and it requires activation via a protocol feature.
See EOSIO/fc#97 for fc changes.
WebAuthn Public Keys
WebAuthn public keys start with a new prefix
PUB_WA
. They contain 3 fields:It is important to note that a WebAuthn key is scoped to a particular domain. The browser records the current origin in the WebAuthn signature and when eosio validates an authority the origin in the signature will only match keys with the same origin.
WebAuthn signatures can indicate three levels of user presence:
A key only strictly matches a single type of user presence: the highest level of presence indicated in the signature. Put another way: a key that matches "user present" will not match a signature that indicates "user verified". It is important to note that it is impossible to disable the possibility of user verification in the JavaScript WebAuthn API -- you can only discourage user verification. As such, when creating an authority that only cares about user presence, two keys will have to be added: one for user presence and one for user verification. Otherwise it's possible that even though the authority only requires user presence, a web client and authenticator may still generate a signature indicating user was verified. For weighted authorities with advanced thresholds, setting up the authority such that a single WebAuthn signature is only counted once will require some nesting.
WebAuthn authenticators will require a credential ID when signing with keys that are not resident on the authenticator. Storage of the credential ID is considered out of scope of these nodeos changes. It is not part of consensus.
WebAuthn Signatures
WebAuthn signatures start with a new prefix
SIG_WA
. They contain 3 fields:WebAuthn requires a "challenge" when creating a signature (an assertion in WebAuthn). This challenge must be the 32-byte transaction ID that would otherwise be signed had one been doing a signature with k1 or r1. Ultimately the 32-byte transaction id will be encoded in the client json and validated by eosio.
Experimental keosd wallet
An experimental keosd wallet is included in the
webauthn_keosd_wallet
branch. The keosd wallet talks directly to a CTAP2 authenticator (USB FIDO2 device, like a YubiKey 5). There is no plan to clean this up and merge it. It only works on macOS and can be kind of flaky. Still, it was good enough to develop and validate this feature.Review Notes
JSON
As part of signature validation the client json needs to be parsed. All of this happens in elliptic_webauthn.cpp in fc. Since this means the json parsing is effectively consensus, it is worth a review.
Protocol Feature Guards
WebAuthn usage is gated in 4 places:
There is no gating of webauthn signatures within transaction validation. The theory is if a WA key can't be in an authority then a transaction wouldn't be allowed either pre or post WA introduction. Certainly something to reflect on.
Subjective Length Limit
Large JSON in SIG_WAs could pose high computational overhead. Granted, even 500KB JSONs I experimented with less than a handful of milliseconds. Still, as a safety guard, there is a subjective limit configurable for the length of a SIG_WA (default is 16KB).
The subjective limit is only enforced:
Consensus Changes
API Changes
Documentation Additions