Add local keymanager API #151
Conversation
|
I'm a little concerned about this. Allowing remote access to this kind of functionality may make it very easy to get slashed... Having CLI access implies a level of admin access, and I guess with that a level of responsibility, and it'd be potentially less open to error with export/import processes... If a validator client with a password doesn't stop their validator from where ever its running and imports it to another client, that'll result in a slashing event, so we need to think really carefully whether this actually needs to be accessible from the standard rest api... If we were to allow it, I think you'd want the key/slashing data in a single payload, so that you can't load a key without having slashing protection data, or do something like without the slashing protection enforce a 2 epoch delay or something... We might be able to do something like verify it the specified validator hasn't attested or something currently, i'm not sure what validations could be worked in for altair - potentially there's a way to see the validator isn't active... We also would need to be very careful about stipulations on how this is secured. I'm not sure we want to stipulate that a single password is used, as it should be up to the person running the client and in line with security requirements that the company has. If they want to go and do one-time-passwords or 2FA, all power to them IMHO. We can definitely suggest that it's disabled by default without providing a flag etc, and that they need to be authenticated routes, but ultimately that's really the level we can stipulate to, and maybe that at least basic auth should be supported or something like that. The response of deleting keystores may need to contain more context about slashing summary of the keys removed... If we do go this way, it would probably be pertinent to not require a login/logout structure, but also to be able to just issue an authenticated api call that has no real 'session'. Partially I say this because sessions can be particularly painful to manage in balanced environments. Create/list accounts is something that needs more discussion. It's a definite attack vector to start creating millions of random accounts, and list accounts is something that non admin users shouldn't really be able to do (get half of the user/password problem, then brute force passwords) Potentially this should be a separate repo, given its dealing purely with keys etc, and won't be dealing with any of the structures in eth2.0-api? It's a whole bunch of infrastructure to confuse the existing apis if we add it all here, and fairly unrelated in real terms... Just a thought... |
|
I would like to echo the concerns of @rolfyone and add a few of my own. First, and most importantly, this locks in a specific way of thinking about a validator, specifically that there is an endpoint that is responsible for both validating and signing. This is becoming more commonly not the case, with multiple remote signer implementations available and being used. This is not only a conceptual issue but a practical one: which process runs the API? This repo builds an API that is run on the beacon node, and that's no use for these endpoints in many cases. Ditto the validator client, as it may not have ownership of the keys (but does need to know the pubkeys for which it is validating). The signer part is the most likely place to put this, or some of this, but they are generally built to be highly secure and allowing creation and deletion of keys seems to be an unlikely fit wit with their existing security model. Second, the general idea of accessing slashing protection data from an active endpoint is a Bad Thing. By definition, fetching this data from a running system means that it is out of date as soon as it has been retrieved and so cannot be trusted to be a complete picture when stored or imported elsewhere. More of an implementation "detail", but the lack of HTTPS is something that seems to be massively insecure. The idea that the keystore, plus the passphrase to access its private data, are both sent in the same request and in plaintext is a very concerning. Regarding the goals of this PR: I think that both are laudable, however I suspect that this would be very low impact. Client diversity, from what I have seen with those that install the software, is much more a question of ease of installation (and, long-term, maintenance) of the beacon node software. The operations to import validators, although not standard between implementations, is relatively simple and so would not benefit massively from having an API rather than its existing CLI. As for UI, the majority of any such project would be around the monitoring of an active beacon chain rather than its setup, in which I include the creation of validators. |
|
You are right, in that this proposal is initially motivated for a specific type of validator operator: the solo home DIY staker without good technical knowledge. This API is a simplification of the existing Prysm's validator API, which is also designed for that type of staker. This proposal is definitely not for professional staking pools or institutional stakers.
Moving this to a different repo is great idea since I agree this functionality doesn't have any resemblance with existing routes. The server for the validator manager API should be served by the a different process, the validator client. Happy to move this PR to a new repo once it exists.
I agree, that's a great point. The counter-argument there is to not allow to extract the keys in anyway with this API, to contain the damage if auth fails.
This simple auth scheme is meant to be served and used locally for solo stakers. In a company setting a more complex setup would have to be used.
That's a great point, I will add it.
This API should not be run by the beacon node, it should be run by the validator client unless using remote signing.
That's a great point. I noted this API is motivated for the solo home staker. However I will research how new approaches to validating can influence this.
@rolfyone idea of fetching slashing protection data only after deleting the keystores from the validator is good step in that direction.
If users are able to get SSL certificates they should, or just do self-signed certs for local networks. Should the openapi doc reflect that?
Given that all projects publish to docker registries it's quite easy to build systems around that. The goal of this is to allow staking without forcing CLI interaction.
We have the metrics standardization effort for that front. |
There was a problem hiding this comment.
Hey, this is just a quick fly-by of a few comments I raised during a discussion out of band.
Here is the link to the endpoints we currently have on the Lighthouse API now: https://lighthouse-book.sigmaprime.io/api-vc-endpoints.html This API needs some work before it can become GUI-ready. Among other things, we need to add some endpoints around importing/exporting slashing protection data.
I'll need to come back for a more detailed review, especially around the authentication method. Presently Lighthouse protects the API by requiring an authorization token.
For the record, I generally support the standardization of the VC API.
apis/accounts/auth/login.yaml
Outdated
| "200": | ||
| description: | | ||
| Successfully authenticated. The session ID is returned in a cookie named `SESSION`. You need to include this cookie in subsequent requests. | ||
| - HttpOnly: MUST be included |
There was a problem hiding this comment.
Is it possible to avoid cookies? Because as soon as you introduce cookies for authentication server become stateful. Also because Max-Age is not specified its possible to create cookies which will never be deleted, and so can be used if cookie become stolen.
What if you use token authentication instead?
There was a problem hiding this comment.
Could you describe in more detail what you refer to with "token authentication"?
Given that each node will likely implement different flavours of authentication, using HttpOnly cookies would simplify the UI implementation. The browser will just re-send whatever that specific node has sent in the Set-Cookie header.
- You can make the cookie stateless by signing it with a known key and adding data to it. Cookies fit 4096 bytes, so it should be enough for whatever session data a validator UI may need to know.
- You can also add a timestamp to the cookie's content and use that to invalidate it in the backend, irrespective of the Max-Age directive
There was a problem hiding this comment.
You can check for Json Web Token about token authentication which could be passed via headers or request string. The biggest difference is that tokens will not be managed by browsers.
This feature protects validators from loosing cookies through malicious browser extensions or malware.
As soon as communication finishes, tokens become not valid anymore.
There was a problem hiding this comment.
Lighthouse has an auth scheme in its existing VC API:
- A secret is passed by the
AuthorizationHTTP header: https://lighthouse-book.sigmaprime.io/api-vc-auth-header.html - Since the secret is a secp256k1 public key, the server also includes a
Signatureheader in all responses (note, this is not safe from replay attacks): https://lighthouse-book.sigmaprime.io/api-vc-sig-header.html
Here's an example of a curl request using this scheme:
curl -v localhost:5062/lighthouse/version -H "Authorization: Basic api-token-0x03eace4c98e8f77477bb99efb74f9af10d800bd3318f92c33b719a4644254d4123"This approach is simpler for the VC, but it doesn't allow for user specified passwords. Our design ethos is to keep the VC HTTP server as simple as possible and to handle all the user-facing authentication in the GUI. Here's a design doc we drew up a while ago: https://hackmd.io/zS3h-Kk_TTmYhvyoHap6DQ
Basically, we expect the VC and BN to be hiding behind a reverse-proxy provided by the GUI web app. This avoids needing to provide HTTPS certificates for the BN, VC and web application.
There was a problem hiding this comment.
@paulhauner This Signature Header is very cool idea! Is this like a roll-your-own TLS strategy to guarantee integrity of the responses? If so, could we take it a step further and make the client encrypt the requests with the pubkey so that the keystores + password are not sent in plain text? https://lighthouse-book.sigmaprime.io/api-vc-sig-header.html
There was a problem hiding this comment.
The beacon client should not provide extensive user authentication & authorization. A simple API key mechanism is enough for putting a UI-based user authenticatoin & authorization on top. In the case of implementing authentication & authorization on the beacon client, where do you draw the line with features? What about 2FA? Different storage techniques for sensible data? Etc.
Handling a JWT (Json Web Token) only makes sense when complex user management is needed.
When HTTPS is needed because access is provided on an unsecure network, a simple SSL termination service (e. g. nginx, haproxy, ...) would suffice and is more secure than handling SSL on the beacon client itself.
There was a problem hiding this comment.
Here's a new tilt at an authentication scheme:
- Add an endpoint
/eth/v1/keystores/auththat is not protected by any form of authentication. It returns a response with an absolute path to a file containing an auth token. The assumption here is that the user of the VC API is a privileged entity with access to the machine the VC is running on, and permission to read this file.
{
"token_path": "/home/michael/.lighthouse/mainnet/validators/api-token.txt"
}- All other endpoints require an
Authorization: Bearer $tokenheader with the value of the token loaded fromtoken_path. The intention is that the token remains the same across multiple VC restarts, unless the user specifically regenerates a new token (this can happen out of band, e.g. by deleting the file and restarting the VC, or using a non-standard API).
Open questions:
- Should the token be a JWT or just an arbitrary string from a URL-safe set of chars?
- Is it OK to do away with signed responses for now (a la https://lighthouse-book.sigmaprime.io/api-vc-sig-header.html), leaving open the possibility of adding them later?
On the JWT issue I tend to agree with @stefa2k that it's overkill for the VC. The main thing that JWT adds is the ability for the client to verify that the token was issued by the VC's public key... which does not seem useful. We could take a JWT-agnostic approach by permitting the VC to return extra metadata in the /auth response if it wishes (such as its JWT-signing public key). JWT-aware clients could use this to verify if they wanted to, while JWT-oblivious implementations could just use the JWT as an arbitrary token without parsing or verifying it.
I also think not requiring signed responses is fine.
Addendum:
The path that the VC stores the token in, and its permissions could be configurable via the CLI. This would enable things like running the VC and the UI as different users if desired, but I think we can get a long way even before those config options are added, and they needn't be standardised (the CLIs are already non-standard).
There was a problem hiding this comment.
hmm so on the Prysm side we are switching to something like this already though there's no API to expose the path...
this is what happens ( feature in develop prysm/ master prysm-web-ui)
you can generate the jwt either by running ./validator web generate-auth-token which will write to a file called auth-token in the prysm wallet directory and return a custom URL in the CLI (i.e. localhost:7500/initialize?token=) they can also change the path using --wallet-dir though it's for the whole wallet.
they should get the same thing when running ./validator --web in the console as well as that same file location but there is a cli command to regenerate
the frontend stores jwt this in local storage and injects it throughout the front end if it's designed with an initialize page 🤔
wondering if we can do something similar in this case
There was a problem hiding this comment.
@michaelsproul So providing the path through an API route would be for the front-end to tell the user to "Go to $path to grab your authenticating token" ? Maybe that's a very specific implementation of auth, not sure if it should be part of the standard. Maybe as a complimentary note or suggestion for same fs setups auth?
There was a problem hiding this comment.
@james-prysm Perfect! I think if Prysm can accept the JWT in an Authorization: Bearer <JWT> header on requests to the key manager API then that would be compatible. Not all programs (other clients, tools, etc) interacting with the JWT need to understand that it's a JWT, they can just treat it as an opaque API token if they wish, i.e. we are JWT-agnostic 😅
@dapplion I was thinking that the backend for the GUI could fetch the token automatically if it's running on the same machine, although you're right that it wouldn't work with the GUI and VC running on different hosts. It would be nice if we could provide a "no touch" set up that's client agnostic for the API token for the majority of use cases, and this was the only way I could think of achieving that. The alternative seems to be to leave the step where the user obtains the token client-specific, in which case the GUI has to hard-code a bunch of token-access heuristics, or up to the user, which is worse for UX (although should exist as a fallback option in case the fetch-from-fs fails).
|
I've updated and simplified the routes to do a simple atomic keymanager flow:
I've removed the login, logout routes and added notes about the need to protect this routes. I left the suggestion of using an auth token in all requests that should be distributed in some way. |
There was a problem hiding this comment.
I'm a big fan of the new simplified API @dapplion, thank you for putting it together!
Some observations, which I'm not sure are actionable:
- We're pushing the responsibility for keystore storage on to the user of this API. There's no way to get a keystore out of the API, which means if one wanted to migrate between VCs one would need an independent copy of the keystore files (and password). This is probably desirable from a security point of view, because it means in the worst case, unauthorized access to this API can only cause a liveness fault, and not a slashing. The current Lighthouse VC API also has this property.
- Deleting keystores increases the risk of data loss slightly compared to just disabling them. On the other hand, given (1) the user should have copies of their keystores handy anyway, and should definitely have a back up of their mnemonic to restore from. Deleting is also safer for preventing slashing (compared to disabling, where the key itself is still present and we rely on the absence of bugs to prevent it being used).
Aside from that I've raised two items which might be actionable in the code itself.
| $ref: "#/components/responses/InternalError" | ||
|
|
||
| delete: | ||
| operationId: DeleteKeys |
There was a problem hiding this comment.
question on intended behavior, if you send an empty request body is the intention to return the current state of slashing protection data? Also is the slashing protection data only on keys that are inactive or returning the data on the active ones as well 🤔
There was a problem hiding this comment.
probably never mind on the second part, it doesn't hurt to have more data right 🤔
There was a problem hiding this comment.
No, it's very risky to return slashing protection data for keys that haven't been deleted, as it represents stale information that could get you slashed.
Slashing protection data should only be returned for keys from the request that were successfully deleted (status == deleted). We should clarify that in the description
| slashing_protection: | ||
| type: object | ||
| required: false | ||
| description: Slashing protection of the imported keys data in EIP-3076 JSON format. |
There was a problem hiding this comment.
we're going to have to define this here if we want it listed in the schema...
There was a problem hiding this comment.
| securitySchemes: | ||
| cookieAuth: # arbitrary name for the security scheme; will be used in the "security" key later | ||
| type: apiKey | ||
| in: cookie | ||
| name: SESSION # cookie name |
There was a problem hiding this comment.
I think the rough consensus around cookies vs tokens fell in favour of tokens (see #151 (comment)). Prysm have implemented a JWT, and Lighthouse has a bearer token that's compatible.
We can specify token auth using the template here: https://swagger.io/docs/specification/authentication/bearer-authentication/
There was a problem hiding this comment.
yup in favor of a token for now as it keeps it more simple.
|
This code has been moved to this repo https://github.com/ethereum/keymanager-APIs/blob/master/keymanager-oapi.yaml Thank you so much everyone for engaging in this initiative ❤️ To continue the discussion please open an issue to new repo https://github.com/ethereum/keymanager-APIs.
|
## Issue Addressed Implements the standard key manager API from https://ethereum.github.io/keymanager-APIs/, formerly ethereum/beacon-APIs#151 Related to #2557 ## Proposed Changes - [x] Add all of the new endpoints from the standard API: GET, POST and DELETE. - [x] Add a `validators.enabled` column to the slashing protection database to support atomic disable + export. - [x] Add tests for all the common sequential accesses of the API - [x] Add tests for interactions with remote signer validators - [x] Add end-to-end tests for migration of validators from one VC to another - [x] Implement the authentication scheme from the standard (token bearer auth) ## Additional Info The `enabled` column in the validators SQL database is necessary to prevent a race condition when exporting slashing protection data. Without the slashing protection database having a way of knowing that a key has been disabled, a concurrent request to sign a message could insert a new record into the database. The `delete_concurrent_with_signing` test exercises this code path, and was indeed failing before the `enabled` column was added. The validator client authentication has been modified from basic auth to bearer auth, with basic auth preserved for backwards compatibility.
Why?
The main goal of this effort is to increase client diversity. UIs are very important to some users so this standard should ensure all clients have at least one.
The secondary goal is to offload UI work from core dev teams to other parties that can prioritize this work over Altair, the merge, etc.
EDIT: Note - this proposal is initially motivated for a specific type of validator operator: the solo home DIY staker without good technical knowledge. This API is a simplification of the existing Prysm's validator API, which is also designed for that type of staker
Minimal validator API to enable a basic validator manager workflow. Supports:
Does not support:
All routes SHOULD be disabled by default and only be enabled with a CLI flag such as
--enable-validator-api.All sensitive routes MUST be authenticated. There SHOULD exist a single password per validator client binary such that multiple users are not supported.
The validator API MUST allow to add and delete accounts but SHOULD not allow to retrieve them. Users are encouraged to use alternative locations to backup their keystores and use them to perform validator client migration.
See the hackmd document for more context https://hackmd.io/@dapplion/eth2-validator-api