client: Jwt token based auth for engine api

[g11tech]

Kiln spec v2 has jwt based auth for the CL requests for engine methods with following validations

• jwt token auth
• iat (issued at) +- 5 seconds from current time
• --jwt-secret param for file

This PR intends to implement the same.

TODOS:

• cleanup (console logs)
• read from file provided by --jwt-secret and validate its length as per spec requirement
• if no secret provided, dump a secret in a file which can be grabbed and supplied to CL
• Unit tests

[codecov]

Codecov Report

Merging #1723 (a5d9831) into merge-kiln (f4f34c8) will decrease coverage by 0.11%.
The diff coverage is 60.86%.

Flag	Coverage Δ
block	85.57% <ø> (ø)
blockchain	83.28% <ø> (ø)
client	71.75% <60.86%> (-0.32%)	⬇️
common	93.90% <ø> (+0.01%)	⬆️
devp2p	82.50% <ø> (+0.13%)	⬆️
ethash	90.76% <ø> (ø)
trie	86.18% <ø> (ø)
tx	89.94% <ø> (ø)
util	92.62% <ø> (ø)
vm	81.19% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[acolytec3]

I know this is probably not fair to ask but is there any way we can avoid adding adding express and its associated dependencies just to verify JWTs? Would it be possible to use jose instead and just import the JWT dependency, rather than adding a whole new server (i.e. express) and the associated dependencies needed to do this step? I'm sure its a bit more code up front and I haven't worked with JWTs in a while (if ever) so not 100% clear on how difficulty what I'm asking is but I'd feel better about this if we weren't adding express along with the already existing jayson and web socket servers we have in the client just for the sake of one middleware validation step. Thoughts?

[g11tech]

I know this is probably not fair to ask but is there any way we can avoid adding adding express and its associated dependencies just to verify JWTs? Would it be possible to use jose instead and just import the JWT dependency, rather than adding a whole new server (i.e. express) and the associated dependencies needed to do this step? I'm sure its a bit more code up front and I haven't worked with JWTs in a while (if ever) so not 100% clear on how difficulty what I'm asking is but I'd feel better about this if we weren't adding express along with the already existing jayson and web socket servers we have in the client just for the sake of one middleware validation step. Thoughts?

ummm... reason to add express was to primarily intercept the requests as jayson seems to not provide middleware-ing capability.
how about using connect to middleware the validation? then header can be inspected and validation done with jwt-simple or jose

[acolytec3]

Could work, or as simple as just using the built in router capability to check the jwt on receipt using your preferred (dependency minimized jwt library) and then calling the right message handler

[g11tech]

Could work, or as simple as just using the built in router capability to check the jwt on receipt using your preferred (dependency minimized jwt library) and then calling the right message handler

method routing doesn't seem to provide handle on req or headers for jwt token to be extracted and validated.

[acolytec3]

Does this useContext provide what you need? I can dig into it tomorrow but looks like this might pass the headers in where the JWT would live, right?

[g11tech]

Does this useContext provide what you need? I can dig into it tomorrow but looks like this might pass the headers in where the JWT would live, right?

unfortunately, the libraries' examples of building that context itself requires using express/connect to plug in a json body parser.
https://github.com/tedeh/jayson/blob/master/examples/context/server.js

If we totally want to avoid any extra dependency, we can catch inbuild's http req via http.createServer((req,res)=>{...}), collect data chunks, parse them (and validate the headers) and then call jayson's server.call to pass on the request for evaluation. Thats exactly what jayson is trying to do under the hood (minus the auth part): https://github.com/tedeh/jayson/blob/master/lib/utils.js#L138-L160

[acolytec3]

Hmm, okay. I still think we should consider minimizing the number of new dependencies we add in and lean on standard nodejs inbuilts where feasible, even if it is express and surrounds that are the new dependencies in question. @holgerd77 , @ryanio , any thoughts?

[ryanio]

@acolytec3 appreciate you taking a look, jayson already uses express under the hood so it was already a transitive dependency, we just need to use it in this way to get deeper in the stack as the jayson readme suggests. I'll be reviewing this PR today and see if we can simplify at all based on your suggestion, but it might be needed in this case.

(Edit: nvm I guess it doesn't use express by default as it's only in devDeps for jayson! My bad. I'll investigate if we can use @g11tech's suggestion to model after the internal util method.)

[acolytec3]

Fair. I didn't look down the dependency tree so not to worry if it's already there.

[ryanio]

ok this looks pretty awesome! just went over all the code and had just a few things i included in some commits.

i did try to connect to lodestar but i don't think i have the right command handy, i pulled and built this branch ChainSafe/lodestar#3777 - how should i run lodestar on kiln? our typical kintsugi run looked like this: ./lodestar beacon --rootDir kintsugi/temp --network=kintsugi --eth1.providerUrls=http://localhost:8545 --execution.urls=http://localhost:8545 i guess adding --jwt-secret=/Users/me/Library/Ethereum/ethereumjs/jwtsecret

i did have one question if it's possible to also do this auth with the websockets endpoint? i know some CL clients were communicating over ws

---

regarding not using express, I'm sympathetic as a way to reduce our dep count, so @g11tech if you want to see if it's possible without writing too much boilerplate code we should definitely consider it. perhaps give your suggested route a try and see if it's doable within a few hours and report back. if it gets needlessly complicated and we find express provides enough value in exchange for the added dep bloat, then we can take that route.

[g11tech]

ok this looks pretty awesome! just went over all the code and had just a few things i included in some commits.

Thank you ❤️

i did try to connect to lodestar but i don't think i have the right command handy, i pulled and built this branch ChainSafe/lodestar#3777 - how should i run lodestar on kiln? our typical kintsugi run looked like this: ./lodestar beacon --rootDir kintsugi/temp --network=kintsugi --eth1.providerUrls=http://localhost:8545 --execution.urls=http://localhost:8545 i guess adding --jwt-secret=/Users/me/Library/Ethereum/ethereumjs/jwtsecret

yes this command would work, but lodestar master (and the above branch) is now incompatible with kintsugi because of kiln updates. However your merge-kiln PR can be reabased on top of this for interop with lodestar

i did have one question if it's possible to also do this auth with the websockets endpoint? i know some CL clients were communicating over ws

let me dig into it

regarding not using express, I'm sympathetic as a way to reduce our dep count, so @g11tech if you want to see if it's possible without writing too much boilerplate code we should definitely consider it. perhaps give your suggested route a try and see if it's doable within a few hours and report back. if it gets needlessly complicated and we find express provides enough value in exchange for the added dep bloat, then we can take that route.

Yes I agree, swapped express with connect middleware to auth /patch requests to jayson 🙂

[ryanio]

looks great! let's wait a few days before merging until we get either a new spec release or when your lodestar PR is merged, so it doesn't break current engine connections for devnet-4

[ryanio]

@g11tech sorry i don't seem to be able to reopen this PR, but once i have merge-kiln-v2 branch up you can target that as base branch