From 48ad0cdcf67b7874391624aa33bbba6c3736b83b Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Wed, 14 Oct 2026 18:11:21 +0200 Subject: [PATCH] fix: Distinguish invalid upload tokens and warn about ephemeral encryption --- objectstore-server/src/config.rs | 1 + objectstore-server/src/endpoints/common.rs | 1 + objectstore-server/src/endpoints/mod.rs | 2 +- objectstore-server/src/state.rs | 8 +++++++- objectstore-server/tests/resumable.rs | 6 +++--- objectstore-service/src/error.rs | 6 ++++++ objectstore-service/src/service.rs | 21 +++++++++++++++++---- 7 files changed, 36 insertions(+), 9 deletions(-) diff --git a/objectstore-server/src/config.rs b/objectstore-server/src/config.rs index a87541e51..8e456f4bf 100644 --- a/objectstore-server/src/config.rs +++ b/objectstore-server/src/config.rs @@ -688,6 +688,7 @@ pub struct Service { /// generated at startup, which means that resumable upload sessions won't work in a /// multi-instance deployment or survive a restart. /// Configure a persistent keyring in production. + /// Using the ephemeral fallback emits a startup warning. /// /// Keys must contain exactly 32 raw bytes. /// diff --git a/objectstore-server/src/endpoints/common.rs b/objectstore-server/src/endpoints/common.rs index 453299090..0ff0348d6 100644 --- a/objectstore-server/src/endpoints/common.rs +++ b/objectstore-server/src/endpoints/common.rs @@ -143,6 +143,7 @@ impl ApiError { ApiError::Service(error) => match error.kind() { ServiceErrorKind::InvalidMetadata | ServiceErrorKind::InvalidUploadId + | ServiceErrorKind::InvalidUploadSessionToken | ServiceErrorKind::ClientStream | ServiceErrorKind::ChunkExceedsUploadLength { .. } | ServiceErrorKind::ChunkTooSmall { .. } => StatusCode::BAD_REQUEST, diff --git a/objectstore-server/src/endpoints/mod.rs b/objectstore-server/src/endpoints/mod.rs index 0b1a25d04..a501bb467 100644 --- a/objectstore-server/src/endpoints/mod.rs +++ b/objectstore-server/src/endpoints/mod.rs @@ -76,7 +76,7 @@ //! //! | Status | Meaning | Client action | //! |--------|---------|---------------| -//! | `400` | Malformed session token, missing `Upload-Length`, nonempty offset query, chunk exceeding the declared length, or non-final chunk shorter than the granularity | Correct the request | +//! | `400` | Malformed session token or one that cannot be decrypted or authenticated, missing `Upload-Length`, nonempty offset query, chunk exceeding the declared length, or non-final chunk shorter than the granularity | Correct the request | //! | `404` | The upload session is unknown or does not belong to this object | Start a new session or correct the request | //! | `409` | A chunk's offset does not match the authoritative offset | Query the offset and continue from there | //! | `410` | The session expired or was canceled | Start a new session | diff --git a/objectstore-server/src/state.rs b/objectstore-server/src/state.rs index f342cad70..5a09fdec6 100644 --- a/objectstore-server/src/state.rs +++ b/objectstore-server/src/state.rs @@ -71,7 +71,13 @@ impl Services { .unwrap_or_default(); let cipher = match config.service.cipher()? { Some(cipher) => cipher, - None => Cipher::ephemeral().context("failed to initialize encryption")?, + None => { + let cipher = Cipher::ephemeral().context("failed to initialize encryption")?; + objectstore_log::warn!( + "No persistent encryption keyring configured; using a process-local ephemeral key. Resumable uploads cannot reliably cross replicas or survive restarts. Configure service.encryption in production." + ); + cipher + } }; let backend = backend::from_config(config.storage.clone(), &streams).await?; let concurrency = ConcurrencyLimiter::new(config.service.max_concurrency) diff --git a/objectstore-server/tests/resumable.rs b/objectstore-server/tests/resumable.rs index d4fc6eccc..3cc706a07 100644 --- a/objectstore-server/tests/resumable.rs +++ b/objectstore-server/tests/resumable.rs @@ -16,7 +16,7 @@ use reqwest::StatusCode; const SESSION: &str = "c29tZS10b2tlbg"; /// Protected `some-token`, bound to `test/org.1/objects/my-key` with the test key below. -const PROTECTED_SESSION: &str = "AAR0ZXN0XSsQ-Z1kEHOi7Np_EvIdRGcZVhFjaV20NWpuzZNyrxcbQD6sDvNFCqXBJF_z8DcrYZoGhzYxYnmX2KOEB4AjPDlkorw3DrUD4AwxlhqSmHMERGPtexVbxt_DS9b-jpDPwJA"; +const PROTECTED_SESSION: &str = "AAR0ZXN0AwMDAwMDAwMDAwMDXtzMYTBNPTYlKSd-0XWLaZ5Iz4DG8rpBALHl24aGNMueygQP8J7cYuKqPnXhoYYheEh2qu72VVUWHwKBXE3rA5KNXyfbyR90J8SNpaf4qVbQ3qYebw1sUApe1dqI4oMSD4zQeuucFkE"; const OBJECT_PATH: &str = "/v1/objects/test/org=1/my-key"; @@ -392,7 +392,7 @@ async fn session_token_requires_base64url() -> Result<()> { } #[tokio::test] -async fn unknown_session_returns_not_found() -> Result<()> { +async fn plaintext_session_returns_bad_request() -> Result<()> { let server = test_server().await; let response = reqwest::Client::new() @@ -401,7 +401,7 @@ async fn unknown_session_returns_not_found() -> Result<()> { .send() .await?; - assert_eq!(response.status(), StatusCode::NOT_FOUND); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); Ok(()) } diff --git a/objectstore-service/src/error.rs b/objectstore-service/src/error.rs index 8faf29040..657d55b05 100644 --- a/objectstore-service/src/error.rs +++ b/objectstore-service/src/error.rs @@ -62,6 +62,8 @@ pub enum ErrorKind { }, /// A resumable upload session expired or was canceled. UploadSessionGone, + /// A resumable upload session token could not be decrypted or authenticated. + InvalidUploadSessionToken, /// The backend does not recognize a resumable upload session. UnknownUploadSession, /// A resumable chunk would exceed the total upload length. @@ -115,6 +117,9 @@ impl fmt::Display for ErrorKind { write!(f, "upload offset mismatch (server holds {offset} bytes)") } Self::UploadSessionGone => f.write_str("upload session gone"), + Self::InvalidUploadSessionToken => { + f.write_str("invalid upload session token: could not decrypt or authenticate it") + } Self::UnknownUploadSession => f.write_str("unknown upload session"), Self::ChunkExceedsUploadLength { offset, @@ -207,6 +212,7 @@ impl Error { ErrorKind::RangeNotSatisfiable { .. } => Level::DEBUG, ErrorKind::UploadOffsetMismatch { .. } => Level::DEBUG, ErrorKind::UploadSessionGone => Level::DEBUG, + ErrorKind::InvalidUploadSessionToken => Level::DEBUG, ErrorKind::UnknownUploadSession => Level::DEBUG, ErrorKind::ChunkExceedsUploadLength { .. } => Level::DEBUG, ErrorKind::ChunkTooSmall { .. } => Level::DEBUG, diff --git a/objectstore-service/src/service.rs b/objectstore-service/src/service.rs index adb029639..9ab2bd298 100644 --- a/objectstore-service/src/service.rs +++ b/objectstore-service/src/service.rs @@ -517,7 +517,7 @@ impl StorageService { let session: Session = self .cipher .decrypt(token.as_bytes()) - .map_err(|_| ErrorKind::UnknownUploadSession)?; + .map_err(|_| ErrorKind::InvalidUploadSessionToken)?; if session.object_id != *expected_id { return Err(ErrorKind::UnknownUploadSession.into()); } @@ -1504,8 +1504,18 @@ mod tests { service .upload_offset(id.clone(), EncryptedSessionToken::new(b"backend token")) .await, - Err(error) if error.kind() == ErrorKind::UnknownUploadSession + Err(error) if error.kind() == ErrorKind::InvalidUploadSessionToken )); + let other_service = StorageService::new( + Box::new(TestBackend::new(hooks.clone())), + Cipher::ephemeral().unwrap(), + ); + let error = other_service + .upload_offset(id.clone(), token.clone()) + .await + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::InvalidUploadSessionToken); + assert!(error.source().is_none()); let other_id = ObjectId::new(make_context(), "other".into()); assert!(matches!( service.upload_offset(other_id, token.clone()).await, @@ -1560,8 +1570,11 @@ mod tests { .upload_offset(id, EncryptedSessionToken::new(b"backend token")) .await; let error = result.unwrap_err(); - assert_eq!(error.kind(), ErrorKind::UnknownUploadSession); - assert_eq!(error.to_string(), "unknown upload session"); + assert_eq!(error.kind(), ErrorKind::InvalidUploadSessionToken); + assert_eq!( + error.to_string(), + "invalid upload session token: could not decrypt or authenticate it" + ); assert!(error.source().is_none()); assert!(hooks.seen_tokens.lock().unwrap().is_empty()); }