From 6c9a50a1adcd5acaef5580cd8dc2273c438db5b3 Mon Sep 17 00:00:00 2001 From: asonix Date: Wed, 4 Oct 2023 11:41:09 -0500 Subject: [PATCH] Implement constant-time equality for delete tokens, inline alias cleanup --- Cargo.lock | 18 +----------------- Cargo.toml | 2 +- dev.toml | 22 +++++++++++----------- src/error.rs | 3 --- src/lib.rs | 3 ++- src/queue.rs | 2 +- src/queue/cleanup.rs | 4 ++-- src/repo.rs | 4 ++++ 8 files changed, 22 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0a929d3..f937a9d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -937,12 +937,6 @@ dependencies = [ "tracing", ] -[[package]] -name = "half" -version = "1.8.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eabb4a44450da02c90444cf74558da904edde8fb4e9035a9a6a4e15445af0bd7" - [[package]] name = "hashbrown" version = "0.12.3" @@ -1649,12 +1643,12 @@ dependencies = [ "reqwest-tracing", "rusty-s3", "serde", - "serde_cbor", "serde_json", "serde_urlencoded", "sha2", "sled", "storage-path-generator", + "subtle", "thiserror", "time", "tokio", @@ -2128,16 +2122,6 @@ dependencies = [ "serde_derive", ] -[[package]] -name = "serde_cbor" -version = "0.11.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2bef2ebfde456fb76bbcf9f59315333decc4fda0b2b44b420243c11e0f5ec1f5" -dependencies = [ - "half", - "serde", -] - [[package]] name = "serde_derive" version = "1.0.188" diff --git a/Cargo.toml b/Cargo.toml index 6468987..415d08d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,12 +41,12 @@ reqwest-middleware = "0.2.2" reqwest-tracing = { version = "0.4.5" } rusty-s3 = "0.4.1" serde = { version = "1.0", features = ["derive"] } -serde_cbor = "0.11.2" serde_json = "1.0" serde_urlencoded = "0.7.1" sha2 = "0.10.0" sled = { version = "0.34.7" } storage-path-generator = "0.1.0" +subtle = { version = "2.5.0", default-features = false } thiserror = "1.0" time = { version = "0.3.0", features = ["serde", "serde-well-known"] } tokio = { version = "1", features = ["full", "tracing"] } diff --git a/dev.toml b/dev.toml index 43335e7..a7f209d 100644 --- a/dev.toml +++ b/dev.toml @@ -34,15 +34,15 @@ path = 'data/sled-repo-local' cache_capacity = 67108864 export_path = "data/exports-local" -# [store] -# type = 'filesystem' -# path = 'data/files-local' - [store] -type = 'object_storage' -endpoint = 'http://localhost:3900' -use_path_style = true -bucket_name = 'pict-rs' -region = 'garage' -access_key = 'GK2182acf19c2bdb8b9c20e16e' -secret_key = '0072105b8659adc02cce21d9135a88ebc279b3a35e170d23d31c63fb9307a168' +type = 'filesystem' +path = 'data/files-local' + +# [store] +# type = 'object_storage' +# endpoint = 'http://localhost:3900' +# use_path_style = true +# bucket_name = 'pict-rs' +# region = 'garage' +# access_key = 'GK2182acf19c2bdb8b9c20e16e' +# secret_key = '0072105b8659adc02cce21d9135a88ebc279b3a35e170d23d31c63fb9307a168' diff --git a/src/error.rs b/src/error.rs index f358d11..2f8d33b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -124,9 +124,6 @@ pub(crate) enum UploadError { #[error("Error in json")] Json(#[from] serde_json::Error), - #[error("Error in cbor")] - Cbor(#[from] serde_cbor::Error), - #[error("Range header not satisfiable")] Range, diff --git a/src/lib.rs b/src/lib.rs index f0cfbb2..4c9332b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -573,7 +573,8 @@ async fn delete( let token = DeleteToken::from_existing(&token); let alias = Alias::from_existing(&alias); - queue::cleanup_alias(&repo, alias, token).await?; + // cleanup alias inline + queue::cleanup::alias(&repo, alias, token).await?; Ok(HttpResponse::NoContent().finish()) } diff --git a/src/queue.rs b/src/queue.rs index 16970d2..aa4fdcf 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -12,7 +12,7 @@ use reqwest_middleware::ClientWithMiddleware; use std::{future::Future, path::PathBuf, pin::Pin}; use tracing::Instrument; -mod cleanup; +pub(crate) mod cleanup; mod process; #[derive(Debug)] diff --git a/src/queue/cleanup.rs b/src/queue/cleanup.rs index 039bccd..3bd8bf0 100644 --- a/src/queue/cleanup.rs +++ b/src/queue/cleanup.rs @@ -124,13 +124,13 @@ where } #[tracing::instrument(skip_all)] -async fn alias(repo: &R, alias: Alias, token: DeleteToken) -> Result<(), Error> +pub(crate) async fn alias(repo: &R, alias: Alias, token: DeleteToken) -> Result<(), Error> where R: FullRepo, { let saved_delete_token = repo.delete_token(&alias).await?; - if saved_delete_token.is_some() && saved_delete_token != Some(token) { + if !saved_delete_token.as_ref().is_some_and(|t| t.ct_eq(&token)) { return Err(UploadError::InvalidToken.into()); } diff --git a/src/repo.rs b/src/repo.rs index 430adfd..5fa8436 100644 --- a/src/repo.rs +++ b/src/repo.rs @@ -851,6 +851,10 @@ impl DeleteToken { None } } + + pub(crate) fn ct_eq(&self, rhs: &Self) -> bool { + subtle::ConstantTimeEq::ct_eq(self.id.as_bytes(), rhs.id.as_bytes()).unwrap_u8() == 1 + } } impl UploadId {