Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add local keymanager API #151
Add local keymanager API #151
Changes from 2 commits
730a0d5
d433e14
0ff09a3
51861f8
2fd107b
37490ab
db508c2
817a24e
b920615
44c772a
c0feb5f
6f56ae9
bd3ffe7
5e8dded
82ba2b9
5d27a29
e1f5232
3168f46
ec9f7d4
43581ac
aa3ff0a
84465af
c6e0daf
ef02cd2
08f627f
be49b64
3b14523
32f7d68
c048b4c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lighthouse has an auth scheme in its existing VC API:
Authorization
HTTP header: https://lighthouse-book.sigmaprime.io/api-vc-auth-header.htmlSignature
header in all responses (note, this is not safe from replay attacks): https://lighthouse-book.sigmaprime.io/api-vc-sig-header.htmlHere'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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a new tilt at an authentication scheme:
/eth/v1/keystores/auth
that 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.Authorization: Bearer $token
header 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:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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).