Skip to content

Conversation

@g11tech
Copy link
Contributor

@g11tech g11tech commented Feb 26, 2022

Kiln spec v2 has jwt based auth for the engine json rpc requests through both http/websocket protocols. Following are some salient features of implementing same in this PR:

  • jwt token auth for http and websocket upgrade requests
  • iat (issued at) +- 5 seconds from current time
  • --jwt-secret param to specify the file for jwt secret, in its absence a random 256 bit key is generated (and dumped in a file)
  • --rpcEngineAuth param addition to enable the above authentication mechanism, so that the requests remain backward compatible till all clients move to jwt auth

This PR is a fresh pull in continuation to this previous PR #1723
cc: @ryanio

@codecov
Copy link

codecov bot commented Feb 26, 2022

Codecov Report

Merging #1751 (baa69f8) into master (6f3da02) will decrease coverage by 0.02%.
The diff coverage is 84.78%.

Impacted file tree graph

Flag Coverage Δ
block 85.57% <ø> (ø)
blockchain 83.28% <ø> (ø)
client 72.09% <84.78%> (+0.18%) ⬆️
common 93.90% <ø> (ø)
devp2p 82.36% <ø> (-0.27%) ⬇️
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.

@ryanio
Copy link
Contributor

ryanio commented Feb 27, 2022

looks great, thanks so much for doing and sticking through! can't believe you also got the ws auth working, that is very cool. we're just missing some coverage on the ws auth now, would you be able to add a few cases to the rpc.spec tests to get coverage on the lines? then I'll approve and merge in :)

Copy link
Contributor

@ryanio ryanio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks fantastic, thank you! will merge in after ci completes

@ryanio ryanio merged commit c842a97 into ethereumjs:master Feb 28, 2022
@g11tech
Copy link
Contributor Author

g11tech commented Feb 28, 2022

looks fantastic, thank you! will merge in after ci completes

thank you @ryanio ❤️, and thanks a ton for your edits/improvs/feedback 🙏 happy to contribute to ethereumjs! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants