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

Request identity v1 implementation #1349

Merged
merged 6 commits into from
Apr 5, 2024

Conversation

jackkleeman
Copy link
Contributor

@jackkleeman jackkleeman commented Apr 2, 2024

Implements a simple request signing approach using ed25519 jwts. Signed parameters:

  • path as aud (which will include service method)
  • issue time, expiry time, and not before time - currently +- 60s

The header looks like:

{
  "typ": "JWT",
  "alg": "EdDSA",
  "kid": "publickeyv1_3bQPC1Lsw1PoXFswYssFGUfoDYughmDy5sdY27FobREn"
}

The payload looks like:

{
  "aud": "/discover",
  "exp": 1712249043,
  "iat": 1712248143,
  "nbf": 1712247243
}

The goal of this signing protocol is to prove recent ownership of the private key without requiring mutual TLS. It is not intended to provide strong interception protection, but only client identification; request bodies and headers remain mutable by a MITM attacker. Server-authenticated TLS must still be used to provide interception protection.

A single private key can be given as a local pem file. Public keys in a format like publickeyv1_CVmG1AvSyedeZpwwd3MRGbRu5yFt3QXXEpQJKyigB9A5 are emitted into logs and can be safely included in service code to aid in verification.

Headers:

  1. x-restate-jwt-v1 -> the jwt
  2. x-restate-signature-scheme -> v1

@jackkleeman jackkleeman force-pushed the request-signing branch 3 times, most recently from 3584180 to 421a38c Compare April 4, 2024 15:58
@jackkleeman jackkleeman changed the title Request signing v1 implementation Request identity v1 implementation Apr 4, 2024
Copy link
Contributor

@igalshilman igalshilman left a comment

Choose a reason for hiding this comment

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

I’ve checked the JWT part, and the tests looks good!

One question about the expiration, is the unit is seconds or milliseconds?

And also, this probably should be a configuration or at least a constant?

I didn’t look into the details of integrating this into the invoker, if you need some feedback there, maybe ping Francesco/Till?

@jackkleeman
Copy link
Contributor Author

the unit is seconds, i will clarify and make it a constant - dont see any need to config it yet

will ping runtime folks for a wider review. thx

Copy link
Contributor

@AhmedSoliman AhmedSoliman left a comment

Choose a reason for hiding this comment

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

Really nice. I left a couple of tiny optional nits but this is good to go.

Great job 👏

crates/service-client/Cargo.toml Outdated Show resolved Hide resolved
crates/service-client/src/http.rs Outdated Show resolved Hide resolved
crates/service-client/src/http.rs Outdated Show resolved Hide resolved
#[derive(Debug, thiserror::Error)]
pub enum HttpError {
#[error(transparent)]
Hyper(#[from] hyper::Error),
#[error(transparent)]
Http(#[from] hyper::http::Error),
#[error(transparent)]
IdentityV1(#[from] <super::request_identity::v1::Signer<'static> as SignRequest>::Error),
Copy link
Contributor

Choose a reason for hiding this comment

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

neat!

crates/service-client/src/request_identity/mod.rs Outdated Show resolved Hide resolved
crates/service-client/src/request_identity/v1.rs Outdated Show resolved Hide resolved
@jackkleeman jackkleeman merged commit 34290a7 into restatedev:main Apr 5, 2024
3 of 4 checks passed
@jackkleeman jackkleeman deleted the request-signing branch April 5, 2024 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants