vegaprotocol / vega

A Go implementation of the Vega Protocol, a protocol for creating and trading derivatives on a fully decentralised network.
https://vega.xyz
GNU Affero General Public License v3.0
37 stars 19 forks source link

Understand the work required for HSM support with Ethereum and break into tickets #4046

Closed gordsport closed 2 years ago

gordsport commented 2 years ago

As part of the Sweeterwater 💦 planning cycle the validators have asked for Hardware Security Module (HSM) support for Ethereum keys

This ticket is for the core team to understand and find a technical solution for this that can be broken into tickets and implemented via the usual core team process

The tasks to complete this refinement are likely to look something like this (but feel free to amend to suit):

To include:

jeremyletang commented 2 years ago

Couple of things I can think of that we may want to look at / know answer for at the end of tis spike:

Also leett's keep in mind that we want to support HSM only for ethereum at first, and that most likely we will want to use a tool like clef which allow management of accounts, with different backends (keystore, hsm, ledger, etc...)

karlem commented 2 years ago

Update on questions from above

karlem commented 2 years ago

Update no. 2

As of today, there is currently multiple options for accounts management backends on Ethereum

There is also a fork of the project that allows direct usage of HSM devices with PKCS 11 interface. Please see here: https://github.com/ThalesGroup/go-ethereum/tree/hsm_wallet_integration_v1.8.13/accounts/hsmwallet

All these options above can be easily plugged into the general Ethereum account manager being used currently by Vega.

jeremyletang commented 2 years ago

Action following the recent meeting:

To be discussed later

wwestgarth commented 2 years ago

Just from a security point of view adding support for clef will create a small attack surface to the vega wallet. Clef has a fairly well-defined Security Model about how a sign request coming in is untrusted, until manual invention (or a rule) says it is fine. Its very clear that on the untrusted channel there is no mutual authentication. So this also applies to the Vega-wallets -- it will have no idea what it is sending sign-requests to.

Assuming we are talking to a malicious clef it can really only affect the vega-wallet by:

So it's not too bad and any attack will likely only at most cause a DoS, but it is worth being aware of. I think any attempts at mitigation e.g PIDs/user checks for pipe/port squatting when clef is local, early signature verification if we have the public-key to hand (which is in essence half of a TLS handshake) will be more work for little gain.

As an aside: clef's use use of "rules" to provide automatic signing invalidates their entire security model. I can't see a sufficient way to constrain a rule enough to provide adequate security, but it seems to be a known issues with a mention in the README to add a handshake and make "the addition of rules less dangerous."

Ultimately how the rule is set is a decision for the validator. As I understand the ones asking for HSM support are already using clef so should be aware nuances and will hopefully have their own defence-in-depth strategy.

jeremyletang commented 2 years ago

@wwestgarth the vega wallet will not use clef. Clef will be used only for the purpose of storing ethereum keys.

You assumption are all rights, but we would expect the validators to be first non malicious (2+1 / 3 non malicious at least, so the network keeps creating blocks) but also have secured appropriately their infrastructures.

As you mention yea, the validators are asking for clef, and are used to it, also yea it is for them to make sure they set it up appropriately.

gordsport commented 2 years ago

@vega-paul the broken out tickets for you to review on this are: Add support for Clef to Ethereum node wallet Refactor node wallet to support Clef

Once done and no other tickets are required this can be moved to done so work can be tracked in the child tickets