-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
EIP-3030: BLS Remote Signer HTTP API Standard #3030
Conversation
Adding to the discussion: Hello! Please, do feel free to include more people in this conversation 🙂 |
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.
The frontmatter stuff must be changed, the rest is general feedback left here since there is no discussion link yet.
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
#### Pre-image validation | ||
|
||
A key feature for a remote signer, pre-image validation, implies that not only the `signingRoot`, but all the required elements needed to perform complete validation of the message, are sent through the wire to obtain a signature. |
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'd argue that pre-image validation is such a core feature that the post-image shouldn't even be sent to the signer. The signer should always be forced to calculate the hash-to-sign locally, given the input + info about signing request (maybe using mimetypes to signal what type of signing is requested)
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.
With this spec, as is, you cement the behaviour of a standard-compliant signer to sign blind checks. Which seems like a very bad idea, IMO
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.
We can do the following then:
- Add as required fields
domain
, thetype
of message to be produced (randao, block, or attestation), and the specificdata
per type of message. - It is responsibility of the Remote Signer Client to perform the adequate checks on the received payload in order to admit it and process the signature.
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.
You can also remove the signingRoot
parameter. Btw, can you point me towards info about what the signingRoot
consists of? Is it the ssz-hash of a full block? or a header? How large is the preimage?
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.
The signingRoot
is the string representation of a SHA256 Hash. The result of the Ethereum 2.0 function compute_signing_root
.
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 see. So, in practice, what's the size of the ssz_object
? Why not send it in it's entirety?
And depending on the size of that -- if it's extremely large, perhaps only portions + proofs/hashes for missing parts?
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.
Fyi this is Prysm's definition. We have a field for the beacon object for additional verifications such as slashing prevention.
https://github.com/prysmaticlabs/prysm/blob/master/proto/validator/accounts/v2/keymanager.proto#L53
It's not a mandatory field, the remote can ignore it
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.
there s one scenario in my head where having the signingRoot together with all the needed fields to recompute it might come in handy, tamper detection. if the sent root and the calculated root mismatch then you have a big problem
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.
In that case, someone is tampering with the traffic. So if they can modify one (e.g. the signingRoot, to sign something else), why wouldn't they tamper with both?
On the one hand, there's a vague argument "it might be handy". So the arguments for including it are:
- It may be handy, against a sloppy attack where the attacker modifies one but not both fields
The arguments against including it are:
- If it's included, a lazy implementation or insecurely configured signer can choose to blindly sign anything.
- If we do not include it, we force to sealer to at least verify that it's a block being sealed, and not something else.
- If we do not include it, since the selaer needs to parse the ssz-object, it becomes a very small step to also do at least some basic anti-slashing validations, such as "do not seal same height twice".
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.
well, you are right of course :D thanks ,
pushing for everything to get signeres to not sign anything blindly should be the the goal
|
||
#### Authentication | ||
|
||
Can be accomplished through the use of an HTTP Request Header. There are several ways to negotiate and issue a valid token to authenticate the communication between the validator client and the remote signer, each of them with potential drawbacks (e.g replay attacks, challenges in distributing the token to the validator client, etc.). In general, any method of authentication must be combined with transport encryption to be effective. |
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.
There is no need for authentication if the requestor is not privileged. And it wouldn't be privileged if you removed the need for the signer to trust the signingRoot
.
So in essence, you now force the validator to store some sealer-credential, instead of a sealing-key -- just swapping one set of secrets for another.
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 agree. This rationale should be more emphatic on the point about swapping of secrets, since it beats the purpose of this API. Please notice that the following paragraph advises the reader to explore the use of network Access Control Lists (ACLs).
#### Pre-image validation | ||
|
||
A key feature for a remote signer, pre-image validation, implies that not only the `signingRoot`, but all the required elements needed to perform complete validation of the message, are sent through the wire to obtain a signature. |
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.
Fyi this is Prysm's definition. We have a field for the beacon object for additional verifications such as slashing prevention.
https://github.com/prysmaticlabs/prysm/blob/master/proto/validator/accounts/v2/keymanager.proto#L53
It's not a mandatory field, the remote can ignore it
[BLS Remote Signer](https://github.com/sigp/rust-bls-remote-signer) | Rust | Sigma Prime | Supports proposed specification. | ||
[Web3signer](https://github.com/PegaSysEng/web3signer) | Java | PegaSys | Supports proposed specification, although with [slightly different methods](https://pegasyseng.github.io/web3signer/web3signer-eth2.html):<br>{`/sign` => `/api/v1/eth2/sign`, `/publicKeys` => `/api/v1/eth2/publicKeys`}. | ||
|
||
The Prysm client supports a [Remote Signing Wallet](https://docs.prylabs.network/docs/wallet/remote/), however its API requires using gRPC as transport. |
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.
Prysm supports both gRPC and JSON over HTTP. There's a grpc<->json/http proxy. We will update our documentations
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
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 looks good to merge as a draft. @hermanjunge I encourage you to heed the advice given by Martin and continue to iterate on this in its draft form before submitting a move to Review.
This EIP defines a HTTP API standard for a BLS remote signer, consumed by validator clients to sign block proposals and attestations in the context of Ethereum 2.0 (eth2).
Summary
This EIP defines a HTTP API standard for a BLS remote signer, consumed by validator clients to sign block proposals and attestations in the context of Ethereum 2.0 (eth2).
Discussion Thread
https://ethereum-magicians.org/t/eip-3030-bls-remote-signer-http-api-standard/4810