ethereum / execution-apis

Collection of APIs provided by Ethereum execution layer clients
Creative Commons Zero v1.0 Universal
961 stars 374 forks source link

UX Issue with JWT Secrets #208

Open prestonvanloon opened 2 years ago

prestonvanloon commented 2 years ago

Original discussion: https://github.com/ethereum/execution-apis/issues/162

The specification here states that the JWT secret must be a 256 bit secret written as a 0x prefixed hex string. As a long time JWT user, I found this arbitrary constraint to be very difficult to understand. Why must it be so rigid? I believe this is a serious UX concern and client teams will have to go through considerable efforts to communicate these requirements to users in documentation, cli help text, and well defined error messages. Users will have to adjust their previously well established practices with JWTs specifically for Ethereum client operation.

From a technical perspective, the signature part of HS256 does not require any length for a secret. Even an empty string is sufficient to construct a valid and verifiable JWT.

signature = HMACSHA256(
  base64UrlEncode(header) + "." +
  base64UrlEncode(payload),
  secret)

While it may be wise for a user to specify a secret of at least a given length to prevent brute for attacks, it seems like a poor trade off between UX and security to require the secret in a 0x prefixed hex string of at least 32 bytes. I'm also afraid that many at home stakers are not highly technical and these requirements add complexity and difficulty for end users that could lead to improper software operation.

Suggestion 1: Remove hard requirements on 0x prefixed hex string formats. Suggestion 2: Consider a reasonable secret key length or no secret key length requirement strongly enforced. Suggestion 3: Require clients to interpret 0x prefix as an indicator the rest of the file must be parsed as hex. Absence of 0x indicates the file is an ascii string.

mkalinin commented 2 years ago

My argument against allowing ascii strings as a secret is a family of attacks that can be employed in this case (like dictionary and other kinds of attack relevant to pass phrases). Not sure how strong this argument is in our context.

Also, the spec allows for using a secret that is generated by EL instead of specifying it manually, CL users will need to specify a path to jwt.hex in this case, tho the file location depends on EL client.

I would like to hear from @holiman @fredriksvantes on this

holiman commented 2 years ago

My reason for disallowing ascii text is that people will put secret or password as password, and that's that.

Now, I know it's a pita to "make up" and type down a 64-character hex-string, but the thing is, both CL and EL are fully capable of generating it, if it's not present. So in theory, as long as they both agree on the location, you could just boot them both and whichever is fastest will generate the secret.

prestonvanloon commented 2 years ago

Some operators may have lower security requirements and some may have higher security requirements. Perhaps we can agree to enforce some minimum security threshold, but requiring a fixed size secret in a specific format seems too opinionated for these specs.

holiman commented 2 years ago

I agree that it might be too opinionated beyond what's warranted by the security requirements. The opinionated:ness serves another purpose though, which is to standardize the behaviour, so that if you have a file which is accepted by geth, it should also be accepted by any other CL or EL node. And same for any file generated by any of them.

If we relax the spec, to allow for passwords, then we need to ensure that we keep a similar level of strict requirements so there's no ambiguity.

However, I don't see any point in allowing <256bit things. I mean, sure, I do -- but we already made the mistake in the original rpc design, that we chose UX over security. And people still shoot themselves in the foot by opening up admin and personal to the internet. I don't want to repeat that again.

I agree that there are legitimate situations when the security requirements aren't that high, but I don't we should cater to that usecase.

prestonvanloon commented 2 years ago

If we relax the spec, to allow for passwords, then we need to ensure that we keep a similar level of strict requirements so there's no ambiguity.

I agree, the spec must define an explicit minimum level of security. The root of this issue is around the format and fixed length. My request isn't about relaxing security, rather it is about making clients just a little bit easier to use. As is, this kind of requirement seem specific to Ethereum and it's another frustrating speed bump in the way of chain operators.