Conversation
`RelaySigner` loads synapse's own ed25519 credentials.json and re-signs rewritten bodies with a fresh signature, so the upstream accepts them as synapse's own trusted traffic. Accepts both the 32-byte seed and 64-byte keypair secret-key encodings that relay's SecretKey supports. `RelayVerifier` authenticates inbound requests against a statically configured set of trusted downstream relays (new `relay_keys` config). This functionality is needed by the project-configs endpoint which rewrites the request body to fan requests across cells, invalidating the inbound relay signature. This change adds the new auth module helpers. Actually wiring it into the ingest-router, and loading the keys from config will be added as a follow up.
|
|
||
| // Relay's SecretKey accepts either a 64-byte keypair or a 32-byte seed so we support both too | ||
| // https://github.com/getsentry/relay/blame/9bfa40d9ea1d5a9225a7332e19d81f4a9b096a21/relay-auth/src/lib.rs#L298-L305 | ||
| let signing_key = if let Ok(keypair) = <[u8; 64]>::try_from(bytes.as_slice()) { |
There was a problem hiding this comment.
Is there any functional difference between this and the relay version? They look similar, but I'm guessing the relay version is inferring the type from the usage of the keypair variable, vs explicitly noting what we're converting into here?
let inner = if let Ok(keypair) = bytes.as_slice().try_into() {
ed25519_dalek::SigningKey::from_keypair_bytes(&keypair)
.map_err(|_| KeyParseError::BadKey)?
}...There was a problem hiding this comment.
The type is inferred in the Relay code since it's passed into SigningKey::from_keypair_bytes which takes a &[u8; 64].. Here we write the length explicitly. The functionality should be the same though.
| fn sign_body(&self, body: &[u8]) -> String { | ||
| let header = SignatureHeader { | ||
| timestamp: chrono::Utc::now(), | ||
| signature_algorithm: None, |
There was a problem hiding this comment.
Is the timestamp only used for signature expiration checks? I'm curious if extending the 5m timer has any other ramifications.
There was a problem hiding this comment.
Yep. Timestamp is used for the expiration checks exclusively and isn't load bearing for anything else in the codebase.
| // other algorithm up front: `key.verify` below only checks `Regular` signatures, so a | ||
| // prehashed (`v1`) or future signature would otherwise fail as an opaque mismatch. | ||
| if let Some(algo) = header.signature_algorithm.as_deref() | ||
| && algo != "v0" |
There was a problem hiding this comment.
Do we need to worry about case sensitivity on this?
There was a problem hiding this comment.
No - it's never been changed and relay is also strict about it, so we shouldn't loosen the case sensitivity here either.
RelaySignerloads synapse's own ed25519 credentials.json and re-signs rewritten bodies with a fresh signature, so the upstream accepts them as synapse's own trusted traffic. Accepts both the 32-byte seed and 64-byte keypair secret-key encodings that relay's SecretKey supports.RelayVerifierauthenticates inbound requests against a statically configured set of trusted downstream relays (newrelay_keysconfig).This functionality is needed by the project-configs endpoint which rewrites the request body to fan requests across cells, invalidating the inbound relay signature.
This change adds the new auth module helpers. Actually wiring it into the ingest-router, and loading the keys from config will be added as a follow up.