Skip to content
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

Remove VC response signing #5423

Closed
michaelsproul opened this issue Mar 18, 2024 · 9 comments
Closed

Remove VC response signing #5423

michaelsproul opened this issue Mar 18, 2024 · 9 comments
Assignees
Labels
RFC Request for comment UX-and-logs val-client Relates to the validator client binary

Comments

@michaelsproul
Copy link
Member

michaelsproul commented Mar 18, 2024

Description

Currently the validator client signs its responses using a secp256k1 key. Lighthouse is the only client to do this, and there is no cross-client standardisation on signed responses.

The reasons to keep response signing are:

  • It prevents a specific man-in-the-middle attack whereby an attacker sits between the VC API client and the VC. The attacker intercepts requests to the VC. They can use the token to make arbitrary (malicious) requests to the VC, but cannot spoof responses to the client because of the signature.

The reasons to remove response signing are:

  • Nobody (?) verifies response signatures, because the described attack is pretty obscure, and unlikely to occur in practice. Further, an attacker in this position (e.g. thanks to a DNS attack) has the power to wreck the VC pretty comprehensively. The signature just means that once they start doing this, the client is likely to detect the attack. A clever attacker could also just honour all of the client's requests, while using the user's token to make extra malicious requests alongside.
  • It makes it more difficult for users to provision VC API keys. The API key currently needs to contain a pubkey derived from a private key, and both files need to be set up by the user. Recently there has been discussion of standardising and simplifying key provision across clients, and our unique response signing scheme would be an impediment to this (Clarify authentication and token management ethereum/keymanager-APIs#74). In contrast, allowing the API key to be an arbitrary string of bytes is much better for UX.
  • Simpler implementation (delete all signing code).
  • Less overhead on sending responses (I haven't checked the perf impact of signing)

Version

Lighthouse v5.1.0

Steps to resolve

We'll leave this issue open for comments from users & developers. If no compelling argument is made for keeping response signing, then I think it should be removed.

Additional Info

Related:

@michaelsproul michaelsproul added RFC Request for comment val-client Relates to the validator client binary UX-and-logs labels Mar 18, 2024
@michaelsproul
Copy link
Member Author

cc @jmcruz1983 @mcdee @barnabasbusa

Do any of you use this feature?

@michaelsproul
Copy link
Member Author

cc @adaszko

@paulhauner
Copy link
Member

I'm happy for singing to be removed. You've made a good argument that it's unlikely to be valuable to users. Therefore it's not worth its weight and an unlikely candidate for standardisation.

@mcdee
Copy link
Contributor

mcdee commented Mar 18, 2024

Thanks for the ping; Vouch does not use this feature.

@dapplion
Copy link
Collaborator

dapplion commented Mar 18, 2024

in favour

- complexity
+ mantainable

@barnabasbusa
Copy link

Also in favor. I prefer more standard solutions, and not one offs by each clients.

@adaszko
Copy link
Contributor

adaszko commented Mar 18, 2024

We're OK with removing that as well. We do not use that feature.
My personal opinion: As other comments above have said, it's better to take care of this at the network level, which also protects from other attacks.

@ank-everstake
Copy link

We do not use / do not intend to use it. Securing network level seems like a much better place to do it than secp256k1 between the VC API client and the VC.

@jimmygchen
Copy link
Member

Completed in #5529 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for comment UX-and-logs val-client Relates to the validator client binary
Projects
None yet
Development

No branches or pull requests

8 participants