Implementing a standard key manager API

[joaquim-verges]

I'm looking to help out on this proposal by @dapplion.

Working branch: https://github.com/joaquim-verges/web3signer/tree/keymanager

The branch will be broken down into individual PRs for each of the main tasks below.

Tasks breakdown

• Extract existing signing API yaml definitions into their own files - Openapi spec split #447
• Add new key manager API yaml definition - Introduce new Key Manager endpoints #455
• Add command line flag to enable API - Add command line flag to enable Key Manager API #448
• Conditionally register routes + handlers in the Eth2Runner
• Implement List handler + tests - Implement List Keystores endpoint #456
  • Json model parsing
  • Return the pubkeys from the artifact signer provider
  • (bonus) add plumbing required to pass derivation_data in the response
• Implement Import handler + tests - Implement Import Keystore endpoint #462
  • Json model parsing
  • Save keystores + passwords into a yaml config file with the expected format
  • That new yaml config file should be read and loaded in Eth2Runner#createArtifactSignerProvider as long as its saved in yaml format in the same keystore path passed through CLI
  • Invoke reload, which should just be a call to artifactSignerProvider.load()
  • Return expected response, handle errors
  • Write unit tests
• Implement Delete handler + tests - Implement Key Manager Delete Endpoint #489
  • Json model parsing
  • Find and delete the yaml files corresponding to the pubkeys coming from the request
  • export slashing data for removed/non_active keys
  • Return expected response, handle errors
  • Write unit tests
• Implement Authentication
  • Add Vertx JWT library dependency
  • Generate JWT token if doesn't exist or read from file and load in memory
  • Persist JWT token if doesn't exist on first generation
  • Add security handler to the OpenAPI router to verify the correct auth token is passed and fail otherwise

Context

The high level goal of this work is to increase client diversity by making it easy for individual stakers to spin up a minority client.

Working backwards from that high level goal, a possible solution would be to:

• Enable the management of minority clients via a platform like DAppNode. Right now Prysm is the only client supported.
• To do this, the other clients (Teku, Lodestar, Lighthouse, Nimbus) would ideally have a mini frontend for their client that can be accessed via the DappNode dashboard. At a minimum, the frontend needs to allow the user to manage keystores.
• Instead of each client building their own, that mini frontend could be standardized and built once for all clients, as long as they all talk to the same standardized API.

@dapplion has a proposal for such standard API

From there, a solution could be to implement that standard API in web3signer, and have the minority clients use web3signer as their remote signer following the EIP-3030 standard.

Any client doing this could get a simple, standard frontend "for free" to manage keys, and therefore would unlock the possibility to list them in a platform like DAppNode, greatly simplifying the workflow for individual stakers.

Open questions

Before I attempt an implementation of this API, I have some questions I'm hoping to get some answers to.

1. Is web3signer the right place to implement such API for key management?
2. web3signer already has an API for remote signing - would it make sense to expand this API to support key management? Or is it cleaner to separate those, despite the fact that some of the functionality overlaps (both APIs have an endpoint to list public keys for example)
3. Is there any other concerns implementing this API as an opt-in with a command line flag?

Let me know if I'm thinking about this the right way, and please correct any of my assumptions!

[jframe]

hi @joaquim-verges,

Thank you for your proposal. It would be great to have these improvements to web3signer.

1. Think it does make sense for web3signer to have a key management API as web3signer is responsible for signing when used as the remote signer. The client will still need to be made aware that the keys have changed. Perhaps they will call the list keys endpoint?
2. It would be better to have a seperate API as in under a different namespace so it is clear this is a different API. There is a little bit of overlap with the list public keys endpoint but that is the only one.
3. An opt-in feature flag is good idea

Let us know if you have any more questions either here or on our discord channel.

[dapplion]

hi @joaquim-verges,

Thank you for your proposal. It would be great to have these improvements to web3signer.

1. Think it does make sense for web3signer to have a key management API as web3signer is responsible for signing when used as the remote signer. The client will still need to be made aware that the keys have changed. Perhaps they will call the list keys endpoint?

2. It would be better to have a seperate API as in under a different namespace so it is clear this is a different API. There is a little bit of overlap with the list public keys endpoint but that is the only one.

3. An opt-in feature flag is good idea

Let us know if you have any more questions either here or on our discord channel.

Thanks for your input! Could you drop an invite to the Discord channel?

I agree this API routes should be served on a different namespace since the permissions will likely be very different

[joaquim-verges]

Thank you @jframe ! Started working on this and have follow up question on 2.

Right now there is an assumption that each runner has a single Open API spec resource (yaml file), and a single OpenAPIRouterFactory created in the base Runner. I see three options here:

a. Put the new API definitions in the existing web3signer-eth2.yaml. I don't feel like this is right, the existing spec is already big, some types might overlap too which would make it messy. Though there might be a way to make this cleaner by extracting/referencing different yaml files into one?

b. Have a separate yaml spec for the new API. Correct me if I'm wrong, but this would requiring creating a separate OpenAPI3RouterFactory in Eth2Runner, which also doesn't feel right to me given the current Runner hierarchy structure. But maybe I'm missing something, or there's a way to cleanly do this multi spec approach with OpenAPI?

c. Another option could be to register additional routes in code on the existing OpenAPI3RouterFactory, only if the opt-in feature flag is passed in. SImilar to how it's done for the swagger endpoints here. This approach is a bit more dynamic and easy to add only in the Eth2Runner, but I we loose the nice yaml definitions. Again might be missing some functionality of OpenAPI I'm not aware of here.

Let me know what you think about these approaches, or if you can think of a better one?

[jframe]

hi @joaquim-verges,

So for option b, looks like the vertx openapi module we are using doesn't support multiple openapi specs for a single OpenAPI3RouterFactory. Not sure what will happen if multiple OpenAPI3RouterFactory are used on the same vertx router. But like you said this doesn't fit the existing design pattern we have either. So I don't think this is the preferred option.

I think option a will probably be the way to go with separating out the yaml files. It fits the existing pattern with each Runner having their own set of endpoints provided by a single openapi spec so will be easier to get started with. If you are able to seperate the key manager api to a seperate openapi spec file using $ref or another approach that would still keep the open api files relatively clean.

[joaquim-verges]

Thanks @jframe - that's where I was leaning towards as well. Will re-organize the yaml file a bit to keep it clean.

[joaquim-verges]

@jframe @dapplion here's another question/concern:

Currently:

• Right now an Eth2Runner is created and started when executing web3signer from the command line
• The available signing keys are loaded from a keystore file defined by --key-store-path when the runner is ran
• Those keys are final for the duration of the runner, loading new ones requires re-running the command

With the new API allowing to import keys, how do you envision the user flow? Can I import keys while my web3signer is running and dynamically add a new signing key or should it require a restart?

The latter would be simpler of course, we could just save the imported keystores into their own files and let the user know they need to restart their signer for the new keys to take effect? (same for delete keys i guess?)

@dapplion maybe you have a better idea of what the user flow should look like in this case?

[jframe]

@joaquim-verges

It is possible to load new keys without restarting web3signer by using the reload endpoint.

From a user flow perspective I think the ideal would be that as soon as the key was imported it would be available to use i.e. sign and available on the list keys endpoint. @dapplion Is this the intent of the import success case that once it is decrypted and in the key manager storage it is available to sign with?

[ajsutton]

Yep, the imported keys should be live straight away (which is how we're implementing it in Teku). And for the delete case it's critically important that the keys are removed and can no longer sign before returning from the call. And that returned slashing protection data should be completely up to date. I wrote up a plan for handling this in memory in Teku at Consensys/teku#4559 which may be useful - we'd also have to delete the files on disk to prevent them loading again on the next restart (I think we'd do that before deleting from memory to ensure we can).

[dapplion]

With the new API allowing to import keys, how do you envision the user flow? Can I import keys while my web3signer is running and dynamically add a new signing key or should it require a restart?

The latter would be simpler of course, we could just save the imported keystores into their own files and let the user know they need to restart their signer for the new keys to take effect? (same for delete keys i guess?)

@joaquim-verges Yes keys must be importable and deletable while the application is running without requiring any restart.

[joaquim-verges]

@dapplion got it. One more question: should the keys imported via the API be persisted? Like if I restart web3signer, should it launch again with the previously imported keys?

Asking to know if it's enough to keep the keys imported via API in memory or if they need to be persisted on disk somewhere. I assume we want to persist the keys on disk, but want to make sure that's ok from a security standpoint?

[dapplion]

should the keys imported via the API be persisted? Like if I restart web3signer, should it launch again with the previously imported keys?

Yes, imported keys must be persisted in disk. If that's not clear in the API spec please feel free to drop a comment there.

[joaquim-verges]

Ok, here's in my mind the list of tasks needed to make this happen:

• Extract existing signing API yaml definitions into their own files
• Add new key manager API yaml definition
• Implement List handler + tests
  • Json model parsing
  • Return the pubkeys from the artifact signer provider
  • (bonus) add plumbing required to pass derivation_data in the response
• Implement Import handler + tests
  • Json model parsing
  • Save keystores + passwords into a yaml config file with the expected format (@jframe can't find a Bls example of such yaml config file in the repo, can you help me get one for reference?)
  • That new yaml config file should be read and loaded in Eth2Runner#createArtifactSignerProvider as long as its saved in yaml format in the same keystore path passed through CLI (so no changes requried there I believe)
  • Invoke reload, which should just be a call to artifactSignerProvider.load() (just needs to be exposed, probably via a function in the base Runner class)
  • Return expected response, handle errors
  • Write unit tests
• Implement Delete handler + tests
  • Json model parsing
  • Find and delete the yaml files corresponding to the pubkeys coming from the request
  • Invoke reload - I think that should be enough? Though I see reload doesn't actually clear the existing keys, that might be an issue. @jframe any suggestions here? Otherwise we'd have to add the plumbing to dynamically remove keys from memory as @ajsutton mentioned.
  • Return expected response, handle errors
  • Write unit tests
• Add command line flag to enable API
• Conditionally register routes + handlers in the Eth2Runner

Let me know if I covered everything, and if it sounds accurate.

[dapplion]

Task breakdown looks great! Feel free to add it to the main issue body with

• This format

so it easier to get for external readers

[jframe]

@joaquim-verges Removing keys dynamically at runtime isn't currently supported. So we will have to take the approach of removing it from memory first and make unable to be signed before removing the key file and returning the result. This also need to be done in such a way that other signers sharing the database can't sign. This is something we will probably want to add to the ArtifactSignerProvider.

[yorickdowne]

Adding auth would be great.

[jframe]

Definitely would be. We have a ticket to implement that with #545