feat: add optional id and clv to JWT claims #6052

ensi321 · 2023-10-19T08:57:57Z

Motivation

When running multiple CL clients with different JWT secrets, it is difficult for EL clients to identify which JWT secrets to verify against which could be solved by having identifiers such as id and clv included in the tokens sent to the EL clients.

Description

Add id: string and clv: string to JwtClaim, which are used in JsonRpcHttpClient
Add jwtId and jwtVersion to eth1 and execution engine options
Introduce --jwtId cli flag to pass the value to jwtCliam.id.
Auto-populate jwtVersion in eth1 and execution engine opts during beacon handler init which are used in jwtClaim.clv
Rename --jwt-secret to --jwtSecret for the sake of being consistent with naming convention of other flags. Note this change is backward compatible since yargs handles hyphen case and camel case automatically

They are compliant with the execution-api specs.

Closes #6016

This reverts commit ef35d9f.

nflaig

Just some initial thoughts

packages/beacon-node/src/eth1/provider/eth1Provider.ts

packages/cli/src/options/beaconNodeOptions/execution.ts

nflaig · 2023-10-23T12:29:08Z

docs/usage/beacon-management.md

+### Set up and include identifiers in JWT tokens
+
+Lodestar auto-populates `clv` field in the claims of JWT authentication tokens with a non-configurable value `Lodestar/$CLIENT_VERSION` eg. `Lodestar/v1.3.0/2d0938e` to communicate the client's version. Lodestar also optionally includes `id` field in the claims with value `$JWT_ID` if the appropriate flag `--jwtId $JWT_ID` is added. 
+`id` and `clv` are particularly useful when running multiple consensus-layer clients with the different JWT secrets which makes the execution-layer client difficult to choose which JWT secret to verify against due to the inability to distinguish between the different consensus-layer clients. 


This might be confusing, as far as I am aware execution clients do not natively support multiplexing of consensus clients and I don't think this will ever be something that will be implemented. Without a multiplexer middleware like eleel it is not recommended to connect multiple CLs to a single EL.

This might be confusing, as far as I am aware execution clients do not natively support multiplexing of consensus clients and I don't think this will ever be something that will be implemented. Without a multiplexer middleware like eleel it is not recommended to connect multiple CLs to a single EL.

I thought about the possible use case of configuring --jwtId for normal users and I can't think of any. The multiplexer case is nice but users who read this doc probably won't ever need it.

I am inclined to just remove this entirely because the point of this doc is to set up a beacon node. Explaining things in jwt claim and the option to add id because of multiplexer seem really out of place in the doc.

Man writing user doc is hard

The multiplexer case is nice but users who read this doc probably won't ever need it.

Yeah, I rather see people using eleel come to the CLI reference to see if Lodestar has the option to set the jwt id

I am inclined to just remove this entirely because the point of this doc is to set up a beacon node.

+1, I think should keep it simple here

Man writing user doc is hard

Defintiely hard thing because it is quite subjective and user feedback is required in a lot of cases, @matthewkeil is working on a docs restructuring which should make it clearer on what details to include where.

In my opinion

step-by-step instructions should mostly omit unnecessary details

while sections explaining different concepts can go more into detail

packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts

packages/beacon-node/src/eth1/provider/jwt.ts

packages/beacon-node/src/execution/engine/http.ts

packages/cli/src/cmds/beacon/handler.ts

packages/cli/src/options/beaconNodeOptions/execution.ts

docs/usage/beacon-management.md

packages/beacon-node/test/unit/eth1/jwt.test.ts

Co-authored-by: Nico Flaig <nflaig@protonmail.com>

dapplion · 2023-10-24T10:45:24Z

docs/usage/beacon-management.md

@@ -33,7 +33,12 @@ You must generate a secret 32-byte (64 characters) hexadecimal string that will

 ### Configure Lodestar to locate the JWT secret

-When starting up a Lodestar beacon node in any configuration, ensure you add the `--jwt-secret $JWT_SECRET_PATH` flag to point to the saved secret key file.
+When starting up a Lodestar beacon node in any configuration, ensure you add the `--jwtSecret $JWT_SECRET_PATH` flag to point to the saved secret key file.


Why change this flag? I think yargs supports both notations and the --jwt-secret flag is kind of standarized across clients

kind of standarized across clients

Kinda, but not really

Teku: --ee-jwt-secret-file (ref)

Lighthouse: --execution-jwt (ref)

Prsym: --jwt-secret (ref)

Nimbus: --jwt-secret (ref)

For Lodestar, --jwt-secret is currently the only commonly used flag which uses different casing. I'd recommend we use the same standard (camelCase) for this flag as well.

packages/cli/src/cmds/beacon/handler.ts

packages/beacon-node/src/eth1/provider/jwt.ts

packages/cli/src/options/beaconNodeOptions/execution.ts

…uded in those

packages/beacon-node/src/eth1/provider/jwt.ts

nflaig

lgtm, thanks for all the updates

wemeetagain · 2023-10-24T14:21:00Z

@naviechan can you confirm that --jwt-secret still works and we don't need to set an alias?

ensi321 · 2023-10-24T14:23:51Z

@naviechan can you confirm that --jwt-secret still works and we don't need to set an alias?

Yes both --jwt-secret and --jwtSecret are correctly parsed

wemeetagain · 2023-11-09T07:41:41Z

🎉 This PR is included in v1.12.0 🎉

ensi321 and others added 9 commits October 19, 2023 15:21

Add id and clv to jwt claim and cli

15f1906

Update doc

156bd02

Add unit test

de7e876

Update comments and lint

70f2f72

Update unit test

ef35d9f

Update unit test

0d4dfff

Revert "Update unit test"

6233200

This reverts commit ef35d9f.

Add id and clv to eth1

a63cd22

Lint

445cd00

ensi321 marked this pull request as ready for review October 19, 2023 14:49

ensi321 requested a review from a team as a code owner October 19, 2023 14:49

nflaig reviewed Oct 19, 2023

View reviewed changes

packages/beacon-node/src/eth1/provider/eth1Provider.ts Outdated Show resolved Hide resolved

packages/cli/src/options/beaconNodeOptions/execution.ts Outdated Show resolved Hide resolved

packages/cli/src/options/beaconNodeOptions/execution.ts Outdated Show resolved Hide resolved

ensi321 added 6 commits October 19, 2023 23:51

Address comment

74dde4a

Update doc to remove jwt version

0d5f268

Remove jwt version from cli arg

b66e1b7

Populate jwtVersion from beaconHandlerinit

1377617

Rename jwt-id to jwtId

485da7a

Rename jwt-secret to jwtSecret

d49c063

nflaig reviewed Oct 23, 2023

View reviewed changes

ensi321 and others added 6 commits October 24, 2023 12:41

Use Date.now() for iat

baa0ae4

Co-authored-by: Nico Flaig <nflaig@protonmail.com>

Update packages/beacon-node/src/eth1/provider/jwt.ts

4b8098c

Co-authored-by: Nico Flaig <nflaig@protonmail.com>

Update packages/beacon-node/src/execution/engine/http.ts

2aa08fe

Co-authored-by: Nico Flaig <nflaig@protonmail.com>

Update packages/beacon-node/src/execution/engine/http.ts

40e61e1

Co-authored-by: Nico Flaig <nflaig@protonmail.com>

Update packages/cli/src/options/beaconNodeOptions/execution.ts

20dcfec

Co-authored-by: Nico Flaig <nflaig@protonmail.com>

Address comments

fef1859

dapplion reviewed Oct 24, 2023

View reviewed changes

nflaig reviewed Oct 24, 2023

View reviewed changes

packages/cli/src/cmds/beacon/handler.ts Show resolved Hide resolved

nflaig reviewed Oct 24, 2023

View reviewed changes

packages/beacon-node/src/eth1/provider/jwt.ts Outdated Show resolved Hide resolved

Fix jsdoc

262afb2

nflaig reviewed Oct 24, 2023

View reviewed changes

packages/cli/src/options/beaconNodeOptions/execution.ts Outdated Show resolved Hide resolved

nflaig and others added 3 commits October 24, 2023 14:05

id is not used for authentication but jwt tokens are, id is just incl…

36844c3

…uded in those

Remove jwt claim section from doc

10c6a05

Update private description and lint

cc5ffe4

nflaig reviewed Oct 24, 2023

View reviewed changes

packages/beacon-node/src/eth1/provider/jwt.ts Outdated Show resolved Hide resolved

Remove extra white space from jsdoc

8eb36c6

nflaig previously approved these changes Oct 24, 2023

View reviewed changes

wemeetagain previously approved these changes Oct 24, 2023

View reviewed changes

Fix lint issues

f4996d2

nflaig dismissed stale reviews from wemeetagain and themself via f4996d2 October 24, 2023 15:31

nflaig approved these changes Oct 24, 2023

View reviewed changes

nflaig enabled auto-merge (squash) October 24, 2023 16:04

wemeetagain approved these changes Oct 24, 2023

View reviewed changes

nflaig merged commit 499fd09 into ChainSafe:unstable Oct 24, 2023
15 checks passed

ensi321 deleted the jwt-id branch October 26, 2023 16:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add optional id and clv to JWT claims #6052

feat: add optional id and clv to JWT claims #6052

ensi321 commented Oct 19, 2023 •

edited

Loading

nflaig left a comment

nflaig Oct 23, 2023

ensi321 Oct 24, 2023 •

edited

Loading

nflaig Oct 24, 2023

dapplion Oct 24, 2023

nflaig Oct 24, 2023

nflaig left a comment

wemeetagain commented Oct 24, 2023

ensi321 commented Oct 24, 2023

wemeetagain commented Nov 9, 2023

feat: add optional id and clv to JWT claims #6052

feat: add optional id and clv to JWT claims #6052

Conversation

ensi321 commented Oct 19, 2023 • edited Loading

nflaig left a comment

Choose a reason for hiding this comment

nflaig Oct 23, 2023

Choose a reason for hiding this comment

ensi321 Oct 24, 2023 • edited Loading

Choose a reason for hiding this comment

nflaig Oct 24, 2023

Choose a reason for hiding this comment

dapplion Oct 24, 2023

Choose a reason for hiding this comment

nflaig Oct 24, 2023

Choose a reason for hiding this comment

nflaig left a comment

Choose a reason for hiding this comment

wemeetagain commented Oct 24, 2023

ensi321 commented Oct 24, 2023

wemeetagain commented Nov 9, 2023

ensi321 commented Oct 19, 2023 •

edited

Loading

ensi321 Oct 24, 2023 •

edited

Loading