Skip to content
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

JWT to authenticate external CL against Reth #1098

Closed
Tracked by #37
0xMelkor opened this issue Jan 31, 2023 · 5 comments · Fixed by #1127
Closed
Tracked by #37

JWT to authenticate external CL against Reth #1098

0xMelkor opened this issue Jan 31, 2023 · 5 comments · Fixed by #1127
Assignees
Labels
A-consensus Related to the consensus engine A-rpc Related to the RPC implementation C-enhancement New feature or request

Comments

@0xMelkor
Copy link
Contributor

0xMelkor commented Jan 31, 2023

Describe the feature

I propose Reth to adhere the JWT standard to setup a trust relationship with an external CL.

Given the current code base and available libraries I'd suggest to create a tower::Layer to cope with JWT authentication.
At a first glance the code could look like this:

   // Read these args from CLI
   // --authrpc.addr=0.0.0.0
   // --authrpc.port=8551
   // --authrpc.jwtsecret=/etc/jwt/jwtsecret

    // Read these from CLI
    const AUTH_JWT: &str = "/etc/jwt/jwtsecret"; 
    const AUTH_PORT: u32 = 8551; 
    const AUTH_ADDR: &str = "0.0.0.0";

    // RPC layer communicates with the implemented logic
    // through an unbounded channel
    let (tx, rx) = unbounded_channel();
   
    let chain_spec = MAINNET.clone();
    let client = Arc::new(MockEthProvider::default()); // Here we should provide a real Client implementation

    // EthConsensusEngine is already implemented as an endless future
    // that listens on the rx part of the channel. 
    // Let's spawn a task.
    tokio::spawn(EthConsensusEngine {
        client: client.clone(),
        chain_spec: chain_spec.clone(),
        local_store: Default::default(),
        rx: UnboundedReceiverStream::new(rx),
    });

    // We need to implement JwtLayer as a `tower::Layer`.
    // This layer checks a valid JWT is provided in CL HTTP requests
    let jwt_layer = JwtLayer::new(AUTH_JWT)?; // Invalid JWT make this fail
    let jwt_middleware = tower::ServiceBuilder::default().layer(jwt_layer);
    
    let server = ServerBuilder::default()
        .set_middleware(jwt_middleware)
        .build(format!("{AUTH_ADDR}:{AUTH_PORT}").parse::<SocketAddr>()?)
        .await?;

    let rpc = EngineApi { engine_tx: tx }.into_rpc();
    let handle = server.start(rpc)?;

What do you guys think?

Additional context

I suppose all this stuff applies if Reth is built with feature = "client"

@0xMelkor 0xMelkor added C-enhancement New feature or request S-needs-triage This issue needs to be labelled labels Jan 31, 2023
@mattsse mattsse added A-rpc Related to the RPC implementation A-consensus Related to the consensus engine labels Jan 31, 2023
@rkrasiuk rkrasiuk removed the S-needs-triage This issue needs to be labelled label Jan 31, 2023
@0xMelkor
Copy link
Contributor Author

0xMelkor commented Jan 31, 2023

Hi guys, are you ok if I take this up?
I'd love to jump in

@mattsse
Copy link
Collaborator

mattsse commented Jan 31, 2023

this is great,

We definitely need an auth middleware that verifies the jwt.

please go for it.

@onbjerg
Copy link
Member

onbjerg commented Feb 1, 2023

@0xMelkor Please note that this should only apply to the engine API, which is the eth_ and engine_ namespaces. These are required to run on a different port since these APIs have different security assumptions.

@onbjerg onbjerg mentioned this issue Feb 1, 2023
8 tasks
@0xMelkor
Copy link
Contributor Author

0xMelkor commented Feb 1, 2023

Please note that this should only apply to the engine API, which is the eth_ and engine_ namespaces.

Hi @onbjerg,
afaik the JWT check applies only to inbound CL calls, at the "Engine API" boundary (engine_ namespace). The eth_ namespace is not reserved for CL, and belongs to the "JSON-RPC API".

Maybe I'm missing something, can you clarify this?

@onbjerg
Copy link
Member

onbjerg commented Feb 1, 2023

@0xMelkor Yes, the transport exposing the engine_ namespace must also expose a subset of eth_. This transport must be separate to the regular RPC transport. The spec states:

Client software MUST expose Engine API at a port independent from JSON-RPC API. The default port for the Engine API is 8551. The Engine API is exposed under the engine namespace.

Further:

To facilitate an Engine API consumer to access state and logs (e.g. proof-of-stake deposits) through the same connection, the client MUST also expose the following subset of eth methods:

And finally, on JWT auth:

Engine API uses JWT authentication enabled by default. JWT authentication is specified in Authentication document.

Reference

In other words, if we enable RPC for reth, and we are running using beacon consensus, the following must be true:

  • The engine API runs on some port (default 8551). This port exposes the engine_ namespace and a subset of eth_ as described in the document above. This endpoint requires JWT authentication.
  • A seperate RPC instance runs on some other port (default 8545). This port exposes whatever namespaces the user wants and does not require JWT authentication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Related to the consensus engine A-rpc Related to the RPC implementation C-enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants