From abe0546ab08139984b1ed4e82b1b580b38f7c54d Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 9 May 2025 15:10:26 +0200 Subject: [PATCH 1/9] model: store x-amz-checksum-type (full_object | composite) --- src/api/s3/copy.rs | 26 ++++- src/api/s3/multipart.rs | 5 + src/api/s3/post_object.rs | 1 + src/api/s3/put.rs | 2 + src/model/s3/object_table.rs | 220 ++++++++++++++++++++++++++++++++++- 5 files changed, 249 insertions(+), 5 deletions(-) diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs index 8892d4ff..0c6877b3 100644 --- a/src/api/s3/copy.rs +++ b/src/api/s3/copy.rs @@ -78,8 +78,23 @@ pub async fn handle_copy( }, )?; + let was_multipart = source_version_meta.etag.contains('-'); // HACK + // Extract source checksum info before source_object_meta_inner is consumed let source_checksum = source_object_meta_inner.checksum; + let source_checksum_type = match (source_object_meta_inner.checksum_type, source_checksum) { + (Some(ct), _) => Some(ct), + (None, Some(_)) => { + // Migrated object from garage v1.x or older + // determine checksum type depending if this is a multipart upload or not + if was_multipart { + Some(ChecksumType::Composite) + } else { + Some(ChecksumType::FullObject) + } + } + (None, None) => None, + }; let source_checksum_algorithm = source_checksum.map(|x| x.algorithm()); // If source object has a checksum, the destination object must as well. @@ -88,7 +103,6 @@ pub async fn handle_copy( let checksum_algorithm = checksum_algorithm.or(source_checksum_algorithm); // Determine metadata of destination object - let was_multipart = source_version_meta.etag.contains('-'); let dest_object_meta = ObjectVersionMetaInner { headers: match req.headers().get("x-amz-metadata-directive") { Some(v) if v == hyper::header::HeaderValue::from_static("REPLACE") => { @@ -97,6 +111,7 @@ pub async fn handle_copy( _ => source_object_meta_inner.into_owned().headers, }, checksum: source_checksum, + checksum_type: source_checksum_type, }; // Do actual object copying @@ -144,8 +159,12 @@ pub async fn handle_copy( } else { ChecksumMode::Verify(&expected_checksum) }; - // If source and dest encryption use different keys, - // we must decrypt content and re-encrypt, so rewrite all data blocks. + // For multipart uploads that had a composite checksum, set checksum type + // to full object as it will be recalculated. + let dest_object_meta = ObjectVersionMetaInner { + checksum_type: checksum_algorithm.map(|_| ChecksumType::FullObject), + ..dest_object_meta + }; handle_copy_reencrypt( ctx, dest_key, @@ -247,6 +266,7 @@ async fn handle_copy_metaonly( state: ObjectVersionState::Uploading { encryption: new_meta.encryption.clone(), checksum_algorithm: None, + checksum_type: None, multipart: false, }, }; diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index 2758c273..dea87b98 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -54,6 +54,7 @@ pub async fn handle_create_multipart_upload( let meta = ObjectVersionMetaInner { headers, checksum: None, + checksum_type: None, }; // Determine whether object should be encrypted, and if so the key @@ -78,6 +79,8 @@ pub async fn handle_create_multipart_upload( multipart: true, encryption: object_encryption, checksum_algorithm, + // TODO: add support for full-object checksums + checksum_type: checksum_algorithm.map(|_| ChecksumType::Composite), }, }; let object = Object::new(*bucket_id, key.to_string(), vec![object_version]); @@ -440,6 +443,8 @@ pub async fn handle_complete_multipart_upload( let new_meta = ObjectVersionMetaInner { headers: meta.into_owned().headers, checksum: checksum_extra, + // TODO: add support for full-object checksums + checksum_type: checksum_extra.map(|_| ChecksumType::Composite), }; encryption.encrypt_meta(new_meta)? } diff --git a/src/api/s3/post_object.rs b/src/api/s3/post_object.rs index d1899138..a0016e15 100644 --- a/src/api/s3/post_object.rs +++ b/src/api/s3/post_object.rs @@ -233,6 +233,7 @@ pub async fn handle_post_object( let meta = ObjectVersionMetaInner { headers, checksum: expected_checksums.extra, + checksum_type: expected_checksums.extra.map(|_| ChecksumType::FullObject), }; let encryption = EncryptionParams::new_from_headers( diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 425636b6..f5bd515b 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -83,6 +83,7 @@ pub async fn handle_put( let meta = ObjectVersionMetaInner { headers, checksum: expected_checksums.extra, + checksum_type: expected_checksums.extra.map(|_| ChecksumType::FullObject), }; // Determine whether object should be encrypted, and if so the key @@ -243,6 +244,7 @@ pub(crate) async fn save_stream> + Unpin>( state: ObjectVersionState::Uploading { encryption: encryption.encrypt_meta(meta.clone())?, checksum_algorithm: None, // don't care; overwritten later + checksum_type: None, multipart: false, }, }; diff --git a/src/model/s3/object_table.rs b/src/model/s3/object_table.rs index 3d805d1a..974d1f1b 100644 --- a/src/model/s3/object_table.rs +++ b/src/model/s3/object_table.rs @@ -259,7 +259,9 @@ mod v010 { compressed: bool, /// Whether the encryption uses an Object Encryption Key derived /// from the master SSE-C key, instead of the master SSE-C key itself. - /// This is the case of objects created in Garage v2+ + /// This is the case of objects created in Garage v2+. + /// This field is kept for compatibility with Garage v2.0.0-beta1, + /// which did not yet implement the v2 module below. #[serde(default)] use_oek: bool, }, @@ -378,7 +380,221 @@ mod v010 { } } -pub use v010::*; +mod v2 { + use garage_util::data::{Hash, Uuid}; + use garage_util::migrate::Migrate; + use serde::{Deserialize, Serialize}; + + use super::v010; + pub use v010::{ChecksumAlgorithm, ChecksumValue}; + + /// An object + #[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)] + pub struct Object { + /// The bucket in which the object is stored, used as partition key + pub bucket_id: Uuid, + + /// The key at which the object is stored in its bucket, used as sorting key + pub key: String, + + /// The list of currently stored versions of the object + pub(super) versions: Vec, + } + + /// Information about a version of an object + #[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)] + pub struct ObjectVersion { + /// Id of the version + pub uuid: Uuid, + /// Timestamp of when the object was created + pub timestamp: u64, + /// State of the version + pub state: ObjectVersionState, + } + + /// State of an object version + #[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize)] + pub enum ObjectVersionState { + /// The version is being received + Uploading { + /// Indicates whether this is a multipart upload + multipart: bool, + /// Checksum algorithm to use + checksum_algorithm: Option, + /// Checksum algorithm type (full object or composite) + checksum_type: Option, + /// Encryption params + headers to be included in the final object + encryption: ObjectVersionEncryption, + }, + /// The version is fully received + Complete(ObjectVersionData), + /// The version uploaded containded errors or the upload was explicitly aborted + Aborted, + } + + /// Data stored in object version + #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug, Serialize, Deserialize)] + pub enum ObjectVersionData { + /// The object was deleted, this Version is a tombstone to mark it as such + DeleteMarker, + /// The object is short, it's stored inlined. + /// It is never compressed. For encrypted objects, it is encrypted using + /// AES256-GCM, like the encrypted headers. + Inline(ObjectVersionMeta, #[serde(with = "serde_bytes")] Vec), + /// The object is not short, Hash of first block is stored here, next segments hashes are + /// stored in the version table + FirstBlock(ObjectVersionMeta, Hash), + } + + /// Metadata about the object version + #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug, Serialize, Deserialize)] + pub struct ObjectVersionMeta { + /// Size of the object. If object is encrypted/compressed, + /// this is always the size of the unencrypted/uncompressed data + pub size: u64, + /// etag of the object + pub etag: String, + /// Encryption params + headers (encrypted or plaintext) + pub encryption: ObjectVersionEncryption, + } + + /// Encryption information + metadata + #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug, Serialize, Deserialize)] + pub enum ObjectVersionEncryption { + SseC { + /// Encrypted serialized ObjectVersionInner struct. + /// This is never compressed, just encrypted using AES256-GCM. + #[serde(with = "serde_bytes")] + inner: Vec, + /// Whether data blocks are compressed in addition to being encrypted + /// (compression happens before encryption, whereas for non-encrypted + /// objects, compression is handled at the level of the block manager) + compressed: bool, + /// Whether the encryption uses an Object Encryption Key derived + /// from the master SSE-C key, instead of the master SSE-C key itself. + /// This is the case of objects created in Garage v2+ + use_oek: bool, + }, + Plaintext { + /// Plain-text headers + inner: ObjectVersionMetaInner, + }, + } + + /// Vector of headers, as tuples of the format (header name, header value) + /// Note: checksum can be Some(_) with checksum_type = None for objects that + /// have been migrated from Garage version before v2.0, as the distinction between + /// full-object and composite checksums was not implemented yet. + #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Debug, Serialize, Deserialize)] + pub struct ObjectVersionMetaInner { + pub headers: HeaderList, + pub checksum: Option, + pub checksum_type: Option, + } + + pub type HeaderList = Vec<(String, String)>; + + #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Debug, Serialize, Deserialize)] + pub enum ChecksumType { + FullObject, + Composite, + } + + impl garage_util::migrate::Migrate for Object { + const VERSION_MARKER: &'static [u8] = b"G2s3ob"; + + type Previous = v010::Object; + + fn migrate(old: v010::Object) -> Object { + Object { + bucket_id: old.bucket_id, + key: old.key, + versions: old.versions.into_iter().map(migrate_version).collect(), + } + } + } + + fn migrate_version(old: v010::ObjectVersion) -> ObjectVersion { + ObjectVersion { + uuid: old.uuid, + timestamp: old.timestamp, + state: match old.state { + v010::ObjectVersionState::Uploading { + multipart, + checksum_algorithm, + encryption, + } => ObjectVersionState::Uploading { + multipart, + checksum_algorithm, + checksum_type: Some(match multipart { + false => ChecksumType::FullObject, + true => ChecksumType::Composite, + }), + encryption: migrate_encryption(encryption), + }, + v010::ObjectVersionState::Complete(d) => { + ObjectVersionState::Complete(migrate_data(d)) + } + v010::ObjectVersionState::Aborted => ObjectVersionState::Aborted, + }, + } + } + + fn migrate_data(old: v010::ObjectVersionData) -> ObjectVersionData { + match old { + v010::ObjectVersionData::DeleteMarker => ObjectVersionData::DeleteMarker, + v010::ObjectVersionData::Inline(meta, data) => { + ObjectVersionData::Inline(migrate_meta(meta), data) + } + v010::ObjectVersionData::FirstBlock(meta, fb) => { + ObjectVersionData::FirstBlock(migrate_meta(meta), fb) + } + } + } + + fn migrate_meta(old: v010::ObjectVersionMeta) -> ObjectVersionMeta { + ObjectVersionMeta { + size: old.size, + etag: old.etag, + encryption: migrate_encryption(old.encryption), + } + } + + fn migrate_encryption(old: v010::ObjectVersionEncryption) -> ObjectVersionEncryption { + match old { + v010::ObjectVersionEncryption::SseC { + inner, + compressed, + use_oek, + } => ObjectVersionEncryption::SseC { + inner, + compressed, + use_oek, + }, + v010::ObjectVersionEncryption::Plaintext { inner } => { + ObjectVersionEncryption::Plaintext { + inner: ObjectVersionMetaInner::migrate(inner), + } + } + } + } + + impl Migrate for ObjectVersionMetaInner { + const VERSION_MARKER: &'static [u8] = b"G2s3om"; + + type Previous = v010::ObjectVersionMetaInner; + + fn migrate(old: v010::ObjectVersionMetaInner) -> ObjectVersionMetaInner { + ObjectVersionMetaInner { + headers: old.headers, + checksum: old.checksum, + checksum_type: None, + } + } + } +} + +pub use v2::*; impl Object { /// Initialize an Object struct from parts From 768794daae8a47098043a4a420416bdb1ebf5fe8 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 9 May 2025 15:14:30 +0200 Subject: [PATCH 2/9] api: change ExpectedChecksums to not be a reference --- src/api/s3/copy.rs | 13 ++++++------- src/api/s3/post_object.rs | 2 +- src/api/s3/put.rs | 6 +++--- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs index 0c6877b3..7c5de1a4 100644 --- a/src/api/s3/copy.rs +++ b/src/api/s3/copy.rs @@ -149,15 +149,14 @@ pub async fn handle_copy( ) .await? } else { - let expected_checksum = ExpectedChecksums { - md5: None, - sha256: None, - extra: source_checksum, - }; let checksum_mode = if was_multipart || source_checksum_algorithm != checksum_algorithm { ChecksumMode::Calculate(checksum_algorithm) } else { - ChecksumMode::Verify(&expected_checksum) + ChecksumMode::Verify(ExpectedChecksums { + md5: None, + sha256: None, + extra: source_checksum, + }) }; // For multipart uploads that had a composite checksum, set checksum type // to full object as it will be recalculated. @@ -345,7 +344,7 @@ async fn handle_copy_reencrypt( source_version: &ObjectVersion, source_version_data: &ObjectVersionData, source_encryption: EncryptionParams, - checksum_mode: ChecksumMode<'_>, + checksum_mode: ChecksumMode, ) -> Result { // basically we will read the source data (decrypt if necessary) // and save that in a new object (encrypt if necessary), diff --git a/src/api/s3/post_object.rs b/src/api/s3/post_object.rs index a0016e15..1d5fb1c8 100644 --- a/src/api/s3/post_object.rs +++ b/src/api/s3/post_object.rs @@ -262,7 +262,7 @@ pub async fn handle_post_object( encryption, StreamLimiter::new(stream, conditions.content_length), &key, - ChecksumMode::Verify(&expected_checksums), + ChecksumMode::Verify(expected_checksums), ) .await?; diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index f5bd515b..81bd259e 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -48,8 +48,8 @@ pub(crate) struct SaveStreamResult { pub(crate) etag: String, } -pub(crate) enum ChecksumMode<'a> { - Verify(&'a ExpectedChecksums), +pub(crate) enum ChecksumMode { + Verify(ExpectedChecksums), VerifyFrom { checksummer: StreamingChecksumReceiver, trailer_algo: Option, @@ -140,7 +140,7 @@ pub(crate) async fn save_stream> + Unpin>( encryption: EncryptionParams, body: S, key: &String, - checksum_mode: ChecksumMode<'_>, + checksum_mode: ChecksumMode, ) -> Result { let ReqCtx { garage, bucket_id, .. From 589a992af8dd88717c1e83e198c56f11bf399d3d Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 9 May 2025 15:36:38 +0200 Subject: [PATCH 3/9] api: parse x-amz-checksum-type and validate combination with algo --- src/api/common/signature/checksum.rs | 5 +++ src/api/s3/multipart.rs | 50 ++++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs index 0fb66ce5..2d7a1389 100644 --- a/src/api/common/signature/checksum.rs +++ b/src/api/common/signature/checksum.rs @@ -22,6 +22,7 @@ pub const CONTENT_MD5: HeaderName = HeaderName::from_static("content-md5"); pub const X_AMZ_CHECKSUM_ALGORITHM: HeaderName = HeaderName::from_static("x-amz-checksum-algorithm"); pub const X_AMZ_CHECKSUM_MODE: HeaderName = HeaderName::from_static("x-amz-checksum-mode"); +pub const X_AMZ_CHECKSUM_TYPE: HeaderName = HeaderName::from_static("x-amz-checksum-type"); pub const X_AMZ_CHECKSUM_CRC32: HeaderName = HeaderName::from_static("x-amz-checksum-crc32"); pub const X_AMZ_CHECKSUM_CRC32C: HeaderName = HeaderName::from_static("x-amz-checksum-crc32c"); pub const X_AMZ_CHECKSUM_CRC64NVME: HeaderName = @@ -29,6 +30,10 @@ pub const X_AMZ_CHECKSUM_CRC64NVME: HeaderName = pub const X_AMZ_CHECKSUM_SHA1: HeaderName = HeaderName::from_static("x-amz-checksum-sha1"); pub const X_AMZ_CHECKSUM_SHA256: HeaderName = HeaderName::from_static("x-amz-checksum-sha256"); +// Values for x-amz-checksum-type +pub const COMPOSITE: &[u8] = b"COMPOSITE"; +pub const FULL_OBJECT: &[u8] = b"FULL_OBJECT"; + pub type Crc32Checksum = [u8; 4]; pub type Crc32cChecksum = [u8; 4]; pub type Crc64NvmeChecksum = [u8; 8]; diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index dea87b98..b946fd3d 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -8,7 +8,7 @@ use crc32c::Crc32cHasher as Crc32c; use crc32fast::Hasher as Crc32; use crc64fast_nvme::Digest as Crc64Nvme; use futures::prelude::*; -use hyper::{Request, Response}; +use hyper::{header::HeaderValue, HeaderMap, Request, Response}; use md5::{Digest, Md5}; use sha1::Sha1; use sha2::Sha256; @@ -70,6 +70,7 @@ pub async fn handle_create_multipart_upload( let object_encryption = encryption.encrypt_meta(meta)?; let checksum_algorithm = request_checksum_algorithm(req.headers())?; + let checksum_type = request_checksum_type(req.headers(), &checksum_algorithm)?; // Create object in object table let object_version = ObjectVersion { @@ -79,8 +80,7 @@ pub async fn handle_create_multipart_upload( multipart: true, encryption: object_encryption, checksum_algorithm, - // TODO: add support for full-object checksums - checksum_type: checksum_algorithm.map(|_| ChecksumType::Composite), + checksum_type, }, }; let object = Object::new(*bucket_id, key.to_string(), vec![object_version]); @@ -292,6 +292,8 @@ pub async fn handle_complete_multipart_upload( let (req_head, req_body) = req.into_parts(); let expected_checksum = request_checksum_value(&req_head.headers)?; + let checksum_algorithm = expected_checksum.map(|x| x.algorithm()); + let checksum_type = request_checksum_type(&req_head.headers, &checksum_algorithm)?; let body = req_body.collect().await?; @@ -347,8 +349,9 @@ pub async fn handle_complete_multipart_upload( for req_part in body_list_of_parts.iter() { match have_parts.get(&req_part.part_number) { Some(part) if part.etag.as_ref() == Some(&req_part.etag) && part.size.is_some() => { - // alternative version: if req_part.checksum.is_some() && part.checksum != req_part.checksum { - if part.checksum != req_part.checksum { + if checksum_type == Some(ChecksumType::Composite) + && part.checksum != req_part.checksum + { return Err(Error::InvalidDigest(format!( "Invalid checksum for part {}: in request = {:?}, uploaded part = {:?}", req_part.part_number, req_part.checksum, part.checksum @@ -405,7 +408,7 @@ pub async fn handle_complete_multipart_upload( // To understand how etags are calculated, read more here: // https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html // https://teppen.io/2018/06/23/aws_s3_etags/ - let mut checksummer = MultipartChecksummer::init(checksum_algorithm); + let mut checksummer = MultipartChecksummer::init(checksum_algorithm, checksum_type); for part in parts.iter() { checksummer.update(part.etag.as_ref().unwrap(), part.checksum)?; } @@ -654,6 +657,36 @@ fn parse_complete_multipart_upload_body( // ====== checksummer ==== +pub fn request_checksum_type( + headers: &HeaderMap, + algo: &Option, +) -> Result, Error> { + match (headers.get(X_AMZ_CHECKSUM_TYPE), algo) { + (None, None) => Ok(None), + (None, Some(ChecksumAlgorithm::Crc64Nvme)) => Ok(Some(ChecksumType::FullObject)), + (None, Some(_)) => Ok(Some(ChecksumType::Composite)), + (Some(_), None) => Err(Error::bad_request( + "Cannot specify x-amz-checksum-type when no checksum algorithm is in use.", + )), + (Some(x), Some(algo)) => { + let checksum_type = match x.as_bytes() { + COMPOSITE => ChecksumType::Composite, + FULL_OBJECT => ChecksumType::FullObject, + _ => return Err(Error::bad_request("Invalid x-amz-checksum-type value")), + }; + match (checksum_type, algo) { + (ChecksumType::Composite, ChecksumAlgorithm::Crc64Nvme) + | (ChecksumType::FullObject, ChecksumAlgorithm::Sha1) + | (ChecksumType::FullObject, ChecksumAlgorithm::Sha256) => Err(Error::bad_request(format!( + "checksum type {:?} is not supported for algorithm {:?}", + checksum_type, algo + ))), + _ => Ok(Some(checksum_type)), + } + } + } +} + #[derive(Default)] pub(crate) struct MultipartChecksummer { pub md5: Md5, @@ -669,7 +702,10 @@ pub(crate) enum MultipartExtraChecksummer { } impl MultipartChecksummer { - pub(crate) fn init(algo: Option) -> Self { + pub(crate) fn init(algo: Option, cktype: Option) -> Self { + if cktype == Some(ChecksumType::FullObject) { + todo!() + } Self { md5: Md5::new(), extra: match algo { From e475c7f802d84ccc71ba9f9fd2a68ff7cde5bc6b Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 9 May 2025 16:32:33 +0200 Subject: [PATCH 4/9] api: switch to crc_fast for checksumming and implement full-object mpu checksums --- Cargo.lock | 83 +++++--- Cargo.toml | 4 +- src/api/common/Cargo.toml | 4 +- src/api/common/signature/checksum.rs | 59 +++--- src/api/s3/Cargo.toml | 4 +- src/api/s3/list.rs | 2 + src/api/s3/multipart.rs | 211 ++++++++++++--------- src/garage/Cargo.toml | 2 +- src/garage/tests/s3/streaming_signature.rs | 15 +- 9 files changed, 235 insertions(+), 149 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2b9adcc0..a4b94cb6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -556,7 +556,7 @@ checksum = "b62ddb9cb1ec0a098ad4bbf9344d0713fa193ae1a80af55febcff2627b6a00c1" dependencies = [ "getrandom 0.2.15", "instant", - "rand", + "rand 0.8.5", ] [[package]] @@ -838,6 +838,20 @@ version = "2.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "19d374276b40fb8bbdee95aef7c7fa6b5316ec764510eb64b8dd0e2ed0d7e7f5" +[[package]] +name = "crc-fast" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf0ab672ed7761a44410115d2e0e5ccd86b987ba186a78a4148fb02bb302a774" +dependencies = [ + "cc", + "crc", + "digest", + "libc", + "rand 0.9.1", + "regex", +] + [[package]] name = "crc32c" version = "0.6.8" @@ -896,7 +910,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3" dependencies = [ "generic-array", - "rand_core", + "rand_core 0.6.4", "typenum", ] @@ -1211,7 +1225,7 @@ dependencies = [ "bytes", "bytesize", "chrono", - "crc32fast", + "crc-fast", "format_table", "futures", "garage_api_admin", @@ -1292,9 +1306,7 @@ dependencies = [ "base64 0.21.7", "bytes", "chrono", - "crc32c", - "crc32fast", - "crc64fast-nvme", + "crc-fast", "crypto-common", "err-derive", "futures", @@ -1353,9 +1365,7 @@ dependencies = [ "base64 0.21.7", "bytes", "chrono", - "crc32c", - "crc32fast", - "crc64fast-nvme", + "crc-fast", "err-derive", "form_urlencoded", "futures", @@ -1407,7 +1417,7 @@ dependencies = [ "garage_util", "hex", "opentelemetry", - "rand", + "rand 0.8.5", "serde", "tokio", "tokio-util 0.7.14", @@ -1448,7 +1458,7 @@ dependencies = [ "hex", "http 1.3.1", "parse_duration", - "rand", + "rand 0.8.5", "serde", "serde_bytes", "tokio", @@ -1473,7 +1483,7 @@ dependencies = [ "opentelemetry-contrib", "pin-project", "pretty_env_logger", - "rand", + "rand 0.8.5", "rmp-serde", "serde", "tokio", @@ -1503,7 +1513,7 @@ dependencies = [ "nix", "opentelemetry", "pnet_datalink", - "rand", + "rand 0.8.5", "reqwest", "schemars", "serde", @@ -1527,7 +1537,7 @@ dependencies = [ "hex", "hexdump", "opentelemetry", - "rand", + "rand 0.8.5", "serde", "serde_bytes", "tokio", @@ -1554,7 +1564,7 @@ dependencies = [ "lazy_static", "mktemp", "opentelemetry", - "rand", + "rand 0.8.5", "rmp-serde", "rustc_version", "serde", @@ -2868,7 +2878,7 @@ dependencies = [ "lazy_static", "percent-encoding", "pin-project", - "rand", + "rand 0.8.5", "thiserror 1.0.69", "tokio", "tokio-stream", @@ -3010,7 +3020,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166" dependencies = [ "base64ct", - "rand_core", + "rand_core 0.6.4", "subtle", ] @@ -3359,8 +3369,18 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" dependencies = [ "libc", - "rand_chacha", - "rand_core", + "rand_chacha 0.3.1", + "rand_core 0.6.4", +] + +[[package]] +name = "rand" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9fbfd9d094a40bf3ae768db9361049ace4c0e04a4fd6b359518bd7b73a73dd97" +dependencies = [ + "rand_chacha 0.9.0", + "rand_core 0.9.3", ] [[package]] @@ -3370,7 +3390,17 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" dependencies = [ "ppv-lite86", - "rand_core", + "rand_core 0.6.4", +] + +[[package]] +name = "rand_chacha" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3022b5f1df60f26e1ffddd6c66e8aa15de382ae63b3a0c1bfc0e4d3e3f325cb" +dependencies = [ + "ppv-lite86", + "rand_core 0.9.3", ] [[package]] @@ -3382,6 +3412,15 @@ dependencies = [ "getrandom 0.2.15", ] +[[package]] +name = "rand_core" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "99d9a13982dcf210057a8a78572b2217b667c3beacbf3a0d8b454f6f82837d38" +dependencies = [ + "getrandom 0.3.2", +] + [[package]] name = "redox_syscall" version = "0.2.16" @@ -4465,7 +4504,7 @@ dependencies = [ "indexmap 1.9.3", "pin-project", "pin-project-lite", - "rand", + "rand 0.8.5", "slab", "tokio", "tokio-util 0.7.14", @@ -4717,7 +4756,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "79daa5ed5740825c40b389c5e50312b9c86df53fccd33f281df655642b43869d" dependencies = [ "getrandom 0.2.15", - "rand", + "rand 0.8.5", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index a7e3e225..004472f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,9 +49,7 @@ bytes = "1.0" bytesize = "1.1" cfg-if = "1.0" chrono = { version = "0.4", features = ["serde"] } -crc32fast = "1.4" -crc32c = "0.6" -crc64fast-nvme = "1.2" +crc-fast = "1.1" crypto-common = "0.1" err-derive = "0.3" gethostname = "0.4" diff --git a/src/api/common/Cargo.toml b/src/api/common/Cargo.toml index 672df030..75838148 100644 --- a/src/api/common/Cargo.toml +++ b/src/api/common/Cargo.toml @@ -21,9 +21,7 @@ garage_util.workspace = true base64.workspace = true bytes.workspace = true chrono.workspace = true -crc32fast.workspace = true -crc32c.workspace = true -crc64fast-nvme.workspace = true +crc-fast.workspace = true crypto-common.workspace = true err-derive.workspace = true hex.workspace = true diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs index 2d7a1389..464fdc75 100644 --- a/src/api/common/signature/checksum.rs +++ b/src/api/common/signature/checksum.rs @@ -1,10 +1,7 @@ -use std::convert::{TryFrom, TryInto}; -use std::hash::Hasher; +use std::convert::TryInto; use base64::prelude::*; -use crc32c::Crc32cHasher as Crc32c; -use crc32fast::Hasher as Crc32; -use crc64fast_nvme::Digest as Crc64Nvme; +use crc_fast::{CrcAlgorithm, Digest as CrcDigest}; use md5::{Digest, Md5}; use sha1::Sha1; use sha2::Sha256; @@ -41,6 +38,21 @@ pub type Md5Checksum = [u8; 16]; pub type Sha1Checksum = [u8; 20]; pub type Sha256Checksum = [u8; 32]; +// -- MAP OF CRC ALGORITHMS : +// CRC32 -> CrcAlgorithm::Crc32IsoHdlc +// CRC32C -> CrcAlgorithm::Crc32Iscsi +// CRC64NVME -> CrcAlgorithm::Crc64Nvme + +pub fn new_crc32() -> CrcDigest { + CrcDigest::new(CrcAlgorithm::Crc32IsoHdlc) +} +pub fn new_crc32c() -> CrcDigest { + CrcDigest::new(CrcAlgorithm::Crc32Iscsi) +} +pub fn new_crc64nvme() -> CrcDigest { + CrcDigest::new(CrcAlgorithm::Crc64Nvme) +} + #[derive(Debug, Default, Clone)] pub struct ExpectedChecksums { // base64-encoded md5 (content-md5 header) @@ -52,9 +64,9 @@ pub struct ExpectedChecksums { } pub struct Checksummer { - pub crc32: Option, - pub crc32c: Option, - pub crc64nvme: Option, + pub crc32: Option, + pub crc32c: Option, + pub crc64nvme: Option, pub md5: Option, pub sha1: Option, pub sha256: Option, @@ -103,13 +115,13 @@ impl Checksummer { self.sha256 = Some(Sha256::new()); } if matches!(&expected.extra, Some(ChecksumValue::Crc32(_))) { - self.crc32 = Some(Crc32::new()); + self.crc32 = Some(new_crc32()); } if matches!(&expected.extra, Some(ChecksumValue::Crc32c(_))) { - self.crc32c = Some(Crc32c::default()); + self.crc32c = Some(new_crc32c()); } if matches!(&expected.extra, Some(ChecksumValue::Crc64Nvme(_))) { - self.crc64nvme = Some(Crc64Nvme::default()); + self.crc64nvme = Some(new_crc64nvme()); } if matches!(&expected.extra, Some(ChecksumValue::Sha1(_))) { self.sha1 = Some(Sha1::new()); @@ -119,13 +131,13 @@ impl Checksummer { pub fn add(mut self, algo: Option) -> Self { match algo { Some(ChecksumAlgorithm::Crc32) => { - self.crc32 = Some(Crc32::new()); + self.crc32 = Some(new_crc32()); } Some(ChecksumAlgorithm::Crc32c) => { - self.crc32c = Some(Crc32c::default()); + self.crc32c = Some(new_crc32c()); } Some(ChecksumAlgorithm::Crc64Nvme) => { - self.crc64nvme = Some(Crc64Nvme::default()); + self.crc64nvme = Some(new_crc64nvme()); } Some(ChecksumAlgorithm::Sha1) => { self.sha1 = Some(Sha1::new()); @@ -143,10 +155,10 @@ impl Checksummer { crc32.update(bytes); } if let Some(crc32c) = &mut self.crc32c { - crc32c.write(bytes); + crc32c.update(bytes); } if let Some(crc64nvme) = &mut self.crc64nvme { - crc64nvme.write(bytes); + crc64nvme.update(bytes); } if let Some(md5) = &mut self.md5 { md5.update(bytes); @@ -161,11 +173,9 @@ impl Checksummer { pub fn finalize(self) -> Checksums { Checksums { - crc32: self.crc32.map(|x| u32::to_be_bytes(x.finalize())), - crc32c: self - .crc32c - .map(|x| u32::to_be_bytes(u32::try_from(x.finish()).unwrap())), - crc64nvme: self.crc64nvme.map(|x| u64::to_be_bytes(x.sum64())), + crc32: self.crc32.map(|x| u32::to_be_bytes(x.finalize() as u32)), + crc32c: self.crc32c.map(|x| u32::to_be_bytes(x.finalize() as u32)), + crc64nvme: self.crc64nvme.map(|x| u64::to_be_bytes(x.finalize())), md5: self.md5.map(|x| x.finalize()[..].try_into().unwrap()), sha1: self.sha1.map(|x| x.finalize()[..].try_into().unwrap()), sha256: self.sha256.map(|x| x.finalize()[..].try_into().unwrap()), @@ -197,10 +207,11 @@ impl Checksums { } if let Some(extra) = expected.extra { let algo = extra.algorithm(); - if self.extract(Some(algo)) != Some(extra) { + let calculated = self.extract(Some(algo)); + if calculated != Some(extra) { return Err(Error::InvalidDigest(format!( - "Failed to validate checksum for algorithm {:?}", - algo + "Failed to validate checksum for algorithm {:?}: calculated {:?}, expected {:?}", + algo, calculated, extra ))); } } diff --git a/src/api/s3/Cargo.toml b/src/api/s3/Cargo.toml index 02866c35..2d11761f 100644 --- a/src/api/s3/Cargo.toml +++ b/src/api/s3/Cargo.toml @@ -27,9 +27,7 @@ async-compression.workspace = true base64.workspace = true bytes.workspace = true chrono.workspace = true -crc32fast.workspace = true -crc32c.workspace = true -crc64fast-nvme.workspace = true +crc-fast.workspace = true err-derive.workspace = true hex.workspace = true hmac.workspace = true diff --git a/src/api/s3/list.rs b/src/api/s3/list.rs index 797fdec0..f86dd7c1 100644 --- a/src/api/s3/list.rs +++ b/src/api/s3/list.rs @@ -1002,9 +1002,11 @@ mod tests { inner: ObjectVersionMetaInner { headers: vec![], checksum: None, + checksum_type: None, }, }, checksum_algorithm: None, + checksum_type: None, }, } } diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index b946fd3d..336872cd 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -1,12 +1,9 @@ use std::collections::HashMap; -use std::convert::{TryFrom, TryInto}; -use std::hash::Hasher; +use std::convert::TryInto; use std::sync::Arc; use base64::prelude::*; -use crc32c::Crc32cHasher as Crc32c; -use crc32fast::Hasher as Crc32; -use crc64fast_nvme::Digest as Crc64Nvme; +use crc_fast::{CrcAlgorithm, Digest as CrcDigest}; use futures::prelude::*; use hyper::{header::HeaderValue, HeaderMap, Request, Response}; use md5::{Digest, Md5}; @@ -410,7 +407,11 @@ pub async fn handle_complete_multipart_upload( // https://teppen.io/2018/06/23/aws_s3_etags/ let mut checksummer = MultipartChecksummer::init(checksum_algorithm, checksum_type); for part in parts.iter() { - checksummer.update(part.etag.as_ref().unwrap(), part.checksum)?; + checksummer.update( + part.etag.as_ref().unwrap(), + part.checksum, + part.size.unwrap(), + )?; } let (checksum_md5, checksum_extra) = checksummer.finalize(); @@ -693,36 +694,14 @@ pub(crate) struct MultipartChecksummer { pub extra: Option, } -pub(crate) enum MultipartExtraChecksummer { - Crc32(Crc32), - Crc32c(Crc32c), - Crc64Nvme(Crc64Nvme), - Sha1(Sha1), - Sha256(Sha256), -} - impl MultipartChecksummer { pub(crate) fn init(algo: Option, cktype: Option) -> Self { - if cktype == Some(ChecksumType::FullObject) { - todo!() - } Self { md5: Md5::new(), - extra: match algo { - None => None, - Some(ChecksumAlgorithm::Crc32) => { - Some(MultipartExtraChecksummer::Crc32(Crc32::new())) - } - Some(ChecksumAlgorithm::Crc32c) => { - Some(MultipartExtraChecksummer::Crc32c(Crc32c::default())) - } - Some(ChecksumAlgorithm::Crc64Nvme) => { - Some(MultipartExtraChecksummer::Crc64Nvme(Crc64Nvme::default())) - } - Some(ChecksumAlgorithm::Sha1) => Some(MultipartExtraChecksummer::Sha1(Sha1::new())), - Some(ChecksumAlgorithm::Sha256) => { - Some(MultipartExtraChecksummer::Sha256(Sha256::new())) - } + extra: match (algo, cktype) { + (None, None) => None, + (Some(algo), Some(cktype)) => Some(MultipartExtraChecksummer::init(algo, cktype)), + _ => unreachable!(), }, } } @@ -731,68 +710,130 @@ impl MultipartChecksummer { &mut self, etag: &str, checksum: Option, + part_len: u64, ) -> Result<(), Error> { self.md5 .update(&hex::decode(&etag).ok_or_message("invalid etag hex")?); - match (&mut self.extra, checksum) { - (None, _) => (), - ( - Some(MultipartExtraChecksummer::Crc32(ref mut crc32)), - Some(ChecksumValue::Crc32(x)), - ) => { - crc32.update(&x); - } - ( - Some(MultipartExtraChecksummer::Crc32c(ref mut crc32c)), - Some(ChecksumValue::Crc32c(x)), - ) => { - crc32c.write(&x); - } - ( - Some(MultipartExtraChecksummer::Crc64Nvme(ref mut crc64nvme)), - Some(ChecksumValue::Crc64Nvme(x)), - ) => { - crc64nvme.write(&x); - } - (Some(MultipartExtraChecksummer::Sha1(ref mut sha1)), Some(ChecksumValue::Sha1(x))) => { - sha1.update(&x); - } - ( - Some(MultipartExtraChecksummer::Sha256(ref mut sha256)), - Some(ChecksumValue::Sha256(x)), - ) => { - sha256.update(&x); - } - (Some(_), b) => { - return Err(Error::internal_error(format!( - "part checksum was not computed correctly, got: {:?}", - b - ))) - } + if let Some(extra) = &mut self.extra { + extra.update(checksum, part_len)?; } Ok(()) } pub(crate) fn finalize(self) -> (Md5Checksum, Option) { let md5 = self.md5.finalize()[..].try_into().unwrap(); - let extra = match self.extra { - None => None, - Some(MultipartExtraChecksummer::Crc32(crc32)) => { - Some(ChecksumValue::Crc32(u32::to_be_bytes(crc32.finalize()))) - } - Some(MultipartExtraChecksummer::Crc32c(crc32c)) => Some(ChecksumValue::Crc32c( - u32::to_be_bytes(u32::try_from(crc32c.finish()).unwrap()), - )), - Some(MultipartExtraChecksummer::Crc64Nvme(crc64nvme)) => Some( - ChecksumValue::Crc64Nvme(u64::to_be_bytes(crc64nvme.sum64())), - ), - Some(MultipartExtraChecksummer::Sha1(sha1)) => { - Some(ChecksumValue::Sha1(sha1.finalize()[..].try_into().unwrap())) - } - Some(MultipartExtraChecksummer::Sha256(sha256)) => Some(ChecksumValue::Sha256( - sha256.finalize()[..].try_into().unwrap(), - )), - }; + let extra = self.extra.map(|c| c.finalize()); (md5, extra) } } + +pub(crate) enum MultipartExtraChecksummer { + FullObjectCrc(CrcAlgorithm, Option), + CompositeCrc(ChecksumAlgorithm, CrcDigest), + CompositeSha1(Sha1), + CompositeSha256(Sha256), +} + +impl MultipartExtraChecksummer { + fn init(algo: ChecksumAlgorithm, cktype: ChecksumType) -> Self { + match (algo, cktype) { + (algo, ChecksumType::FullObject) => { + let crc_type = match algo { + ChecksumAlgorithm::Crc32 => CrcAlgorithm::Crc32IsoHdlc, + ChecksumAlgorithm::Crc32c => CrcAlgorithm::Crc32Iscsi, + ChecksumAlgorithm::Crc64Nvme => CrcAlgorithm::Crc64Nvme, + _ => unreachable!(), + }; + Self::FullObjectCrc(crc_type, None) + } + (ChecksumAlgorithm::Crc32, ChecksumType::Composite) => { + Self::CompositeCrc(ChecksumAlgorithm::Crc32, new_crc32()) + } + (ChecksumAlgorithm::Crc32c, ChecksumType::Composite) => { + Self::CompositeCrc(ChecksumAlgorithm::Crc32c, new_crc32c()) + } + (ChecksumAlgorithm::Sha1, ChecksumType::Composite) => Self::CompositeSha1(Sha1::new()), + (ChecksumAlgorithm::Sha256, ChecksumType::Composite) => { + Self::CompositeSha256(Sha256::new()) + } + _ => unreachable!(), + } + } + + fn update(&mut self, checksum: Option, part_len: u64) -> Result<(), Error> { + match (self, checksum) { + (Self::FullObjectCrc(crc_algo, crc_value), Some(ck)) => { + let ck_u64 = match ck { + ChecksumValue::Crc32(x) => u32::from_be_bytes(x) as u64, + ChecksumValue::Crc32c(x) => u32::from_be_bytes(x) as u64, + ChecksumValue::Crc64Nvme(x) => u64::from_be_bytes(x), + _ => { + return Err(Error::internal_error(format!( + "part checksum was not computed correctly, got: {:?}", + ck + ))) + } + }; + *crc_value = match *crc_value { + None => Some(ck_u64), + Some(prev) => Some(crc_fast::checksum_combine( + *crc_algo, prev, ck_u64, part_len, + )), + }; + } + (Self::CompositeCrc(_, digest), Some(ck)) => match ck { + ChecksumValue::Crc32(x) => digest.update(&x), + ChecksumValue::Crc32c(x) => digest.update(&x), + ChecksumValue::Crc64Nvme(x) => digest.update(&x), + _ => { + return Err(Error::internal_error(format!( + "part checksum was not computed correctly, got: {:?}", + ck + ))) + } + }, + (Self::CompositeSha1(sha1), Some(ChecksumValue::Sha1(x))) => { + sha1.update(&x); + } + (Self::CompositeSha256(sha256), Some(ChecksumValue::Sha256(x))) => { + sha256.update(&x); + } + _ => { + return Err(Error::internal_error(format!( + "part checksum was not computed correctly, got: {:?}", + checksum + ))) + } + } + Ok(()) + } + fn finalize(self) -> ChecksumValue { + match self { + Self::FullObjectCrc(algo, value) => match (algo, value) { + (CrcAlgorithm::Crc32IsoHdlc, Some(v)) => { + ChecksumValue::Crc32(u32::to_be_bytes(v as u32)) + } + (CrcAlgorithm::Crc32Iscsi, Some(v)) => { + ChecksumValue::Crc32c(u32::to_be_bytes(v as u32)) + } + (CrcAlgorithm::Crc64Nvme, Some(v)) => ChecksumValue::Crc64Nvme(u64::to_be_bytes(v)), + _ => unreachable!(), + }, + Self::CompositeCrc(algo, crc) => match algo { + ChecksumAlgorithm::Crc32 => { + ChecksumValue::Crc32(u32::to_be_bytes(crc.finalize() as u32)) + } + ChecksumAlgorithm::Crc32c => { + ChecksumValue::Crc32c(u32::to_be_bytes(crc.finalize() as u32)) + } + _ => unreachable!(), + }, + Self::CompositeSha1(sha1) => { + ChecksumValue::Sha1(sha1.finalize()[..].try_into().unwrap()) + } + Self::CompositeSha256(sha256) => { + ChecksumValue::Sha256(sha256.finalize()[..].try_into().unwrap()) + } + } + } +} diff --git a/src/garage/Cargo.toml b/src/garage/Cargo.toml index 3d0734b4..5bcb2cd2 100644 --- a/src/garage/Cargo.toml +++ b/src/garage/Cargo.toml @@ -79,7 +79,7 @@ static_init.workspace = true assert-json-diff.workspace = true serde_json.workspace = true base64.workspace = true -crc32fast.workspace = true +crc-fast.workspace = true k2v-client.workspace = true diff --git a/src/garage/tests/s3/streaming_signature.rs b/src/garage/tests/s3/streaming_signature.rs index a86feefc..abd4f1ec 100644 --- a/src/garage/tests/s3/streaming_signature.rs +++ b/src/garage/tests/s3/streaming_signature.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use base64::prelude::*; -use crc32fast::Hasher as Crc32; +use crc_fast::{checksum as crc_checksum, CrcAlgorithm}; use crate::common; use crate::common::ext::CommandExt; @@ -69,9 +69,8 @@ async fn test_putobject_streaming() { { let etag = "\"46cf18a9b447991b450cad3facf5937e\""; - let mut crc32 = Crc32::new(); - crc32.update(&BODY[..]); - let crc32 = BASE64_STANDARD.encode(&u32::to_be_bytes(crc32.finalize())[..]); + let crc32 = crc_checksum(CrcAlgorithm::Crc32IsoHdlc, &BODY[..]) as u32; + let crc32 = BASE64_STANDARD.encode(&u32::to_be_bytes(crc32)[..]); let mut headers = HashMap::new(); headers.insert("x-amz-checksum-crc32".to_owned(), crc32.clone()); @@ -129,7 +128,8 @@ async fn test_putobject_streaming_unsigned_trailer() { let mut headers = HashMap::new(); headers.insert("content-type".to_owned(), content_type.to_owned()); - let empty_crc32 = BASE64_STANDARD.encode(&u32::to_be_bytes(Crc32::new().finalize())[..]); + let empty_crc32 = crc_checksum(CrcAlgorithm::Crc32IsoHdlc, &[]) as u32; + let empty_crc32 = BASE64_STANDARD.encode(&u32::to_be_bytes(empty_crc32)[..]); let res = ctx .custom_request @@ -180,9 +180,8 @@ async fn test_putobject_streaming_unsigned_trailer() { { let etag = "\"46cf18a9b447991b450cad3facf5937e\""; - let mut crc32 = Crc32::new(); - crc32.update(&BODY[..]); - let crc32 = BASE64_STANDARD.encode(&u32::to_be_bytes(crc32.finalize())[..]); + let crc32 = crc_checksum(CrcAlgorithm::Crc32IsoHdlc, &BODY[..]) as u32; + let crc32 = BASE64_STANDARD.encode(&u32::to_be_bytes(crc32)[..]); // try sending with wrong crc32, check that it fails let err_res = ctx From fbb40c4ea0060a3b4e147eee63a8febb53e9ecb0 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Thu, 22 May 2025 16:20:58 +0200 Subject: [PATCH 5/9] object_table: merge checksum_algorithm and checksum_type for Uploading state --- src/api/s3/copy.rs | 5 ++-- src/api/s3/list.rs | 1 - src/api/s3/multipart.rs | 50 ++++++++++++++++++++---------------- src/api/s3/put.rs | 1 - src/model/s3/object_table.rs | 17 ++++++------ 5 files changed, 39 insertions(+), 35 deletions(-) diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs index 7c5de1a4..dcc85d82 100644 --- a/src/api/s3/copy.rs +++ b/src/api/s3/copy.rs @@ -265,7 +265,6 @@ async fn handle_copy_metaonly( state: ObjectVersionState::Uploading { encryption: new_meta.encryption.clone(), checksum_algorithm: None, - checksum_type: None, multipart: false, }, }; @@ -531,7 +530,7 @@ pub async fn handle_upload_part_copy( // Now, actually copy the blocks let mut checksummer = Checksummer::init(&Default::default(), !dest_encryption.is_encrypted()) - .add(dest_object_checksum_algorithm); + .add(dest_object_checksum_algorithm.map(|(algo, _)| algo)); // First, create a stream that is able to read the source blocks // and extract the subrange if necessary. @@ -662,7 +661,7 @@ pub async fn handle_upload_part_copy( let checksums = checksummer.finalize(); let etag = dest_encryption.etag_from_md5(&checksums.md5); - let checksum = checksums.extract(dest_object_checksum_algorithm); + let checksum = checksums.extract(dest_object_checksum_algorithm.map(|(algo, _)| algo)); // Put the part's ETag in the Versiontable dest_mpu.parts.put( diff --git a/src/api/s3/list.rs b/src/api/s3/list.rs index f86dd7c1..c62cf118 100644 --- a/src/api/s3/list.rs +++ b/src/api/s3/list.rs @@ -1006,7 +1006,6 @@ mod tests { }, }, checksum_algorithm: None, - checksum_type: None, }, } } diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index 336872cd..87fd2f98 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -66,8 +66,10 @@ pub async fn handle_create_multipart_upload( )?; let object_encryption = encryption.encrypt_meta(meta)?; - let checksum_algorithm = request_checksum_algorithm(req.headers())?; - let checksum_type = request_checksum_type(req.headers(), &checksum_algorithm)?; + let checksum_algorithm = request_checksum_algorithm_and_type( + req.headers(), + request_checksum_algorithm(req.headers())?, + )?; // Create object in object table let object_version = ObjectVersion { @@ -77,7 +79,6 @@ pub async fn handle_create_multipart_upload( multipart: true, encryption: object_encryption, checksum_algorithm, - checksum_type, }, }; let object = Object::new(*bucket_id, key.to_string(), vec![object_version]); @@ -227,7 +228,7 @@ pub async fn handle_put_part( MpuPart { version: version_uuid, etag: Some(etag.clone()), - checksum: checksums.extract(checksum_algorithm), + checksum: checksums.extract(checksum_algorithm.map(|(algo, _)| algo)), size: Some(total_size), }, ); @@ -289,8 +290,10 @@ pub async fn handle_complete_multipart_upload( let (req_head, req_body) = req.into_parts(); let expected_checksum = request_checksum_value(&req_head.headers)?; - let checksum_algorithm = expected_checksum.map(|x| x.algorithm()); - let checksum_type = request_checksum_type(&req_head.headers, &checksum_algorithm)?; + let req_checksum_algorithm = request_checksum_algorithm_and_type( + &req_head.headers, + expected_checksum.map(|x| x.algorithm()), + )?; let body = req_body.collect().await?; @@ -321,6 +324,9 @@ pub async fn handle_complete_multipart_upload( } => (encryption, checksum_algorithm), _ => unreachable!(), }; + if req_checksum_algorithm != checksum_algorithm { + return Err(Error::InvalidDigest("checksum algorithm does not correspond to algorithm specified in CreateMultipartUpload".into())); + } // Check that part numbers are an increasing sequence. // (it doesn't need to start at 1 nor to be a continuous sequence, @@ -346,7 +352,7 @@ pub async fn handle_complete_multipart_upload( for req_part in body_list_of_parts.iter() { match have_parts.get(&req_part.part_number) { Some(part) if part.etag.as_ref() == Some(&req_part.etag) && part.size.is_some() => { - if checksum_type == Some(ChecksumType::Composite) + if checksum_algorithm.map(|(_, ty)| ty) == Some(ChecksumType::Composite) && part.checksum != req_part.checksum { return Err(Error::InvalidDigest(format!( @@ -405,7 +411,7 @@ pub async fn handle_complete_multipart_upload( // To understand how etags are calculated, read more here: // https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html // https://teppen.io/2018/06/23/aws_s3_etags/ - let mut checksummer = MultipartChecksummer::init(checksum_algorithm, checksum_type); + let mut checksummer = MultipartChecksummer::init(checksum_algorithm); for part in parts.iter() { checksummer.update( part.etag.as_ref().unwrap(), @@ -447,8 +453,7 @@ pub async fn handle_complete_multipart_upload( let new_meta = ObjectVersionMetaInner { headers: meta.into_owned().headers, checksum: checksum_extra, - // TODO: add support for full-object checksums - checksum_type: checksum_extra.map(|_| ChecksumType::Composite), + checksum_type: checksum_algorithm.map(|(_, ty)| ty), }; encryption.encrypt_meta(new_meta)? } @@ -658,14 +663,19 @@ fn parse_complete_multipart_upload_body( // ====== checksummer ==== -pub fn request_checksum_type( +pub fn request_checksum_algorithm_and_type( headers: &HeaderMap, - algo: &Option, -) -> Result, Error> { + algo: Option, +) -> Result, Error> { match (headers.get(X_AMZ_CHECKSUM_TYPE), algo) { (None, None) => Ok(None), - (None, Some(ChecksumAlgorithm::Crc64Nvme)) => Ok(Some(ChecksumType::FullObject)), - (None, Some(_)) => Ok(Some(ChecksumType::Composite)), + (None, Some(algo)) => { + let ty = match algo { + ChecksumAlgorithm::Crc64Nvme => ChecksumType::FullObject, + _ => ChecksumType::Composite, + }; + Ok(Some((algo, ty))) + } (Some(_), None) => Err(Error::bad_request( "Cannot specify x-amz-checksum-type when no checksum algorithm is in use.", )), @@ -682,7 +692,7 @@ pub fn request_checksum_type( "checksum type {:?} is not supported for algorithm {:?}", checksum_type, algo ))), - _ => Ok(Some(checksum_type)), + (ty, algo) => Ok(Some((algo, ty))), } } } @@ -695,14 +705,10 @@ pub(crate) struct MultipartChecksummer { } impl MultipartChecksummer { - pub(crate) fn init(algo: Option, cktype: Option) -> Self { + pub(crate) fn init(algo: Option<(ChecksumAlgorithm, ChecksumType)>) -> Self { Self { md5: Md5::new(), - extra: match (algo, cktype) { - (None, None) => None, - (Some(algo), Some(cktype)) => Some(MultipartExtraChecksummer::init(algo, cktype)), - _ => unreachable!(), - }, + extra: algo.map(|(algo, cktype)| MultipartExtraChecksummer::init(algo, cktype)), } } diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 81bd259e..a2ea1039 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -244,7 +244,6 @@ pub(crate) async fn save_stream> + Unpin>( state: ObjectVersionState::Uploading { encryption: encryption.encrypt_meta(meta.clone())?, checksum_algorithm: None, // don't care; overwritten later - checksum_type: None, multipart: false, }, }; diff --git a/src/model/s3/object_table.rs b/src/model/s3/object_table.rs index 974d1f1b..ebee67f8 100644 --- a/src/model/s3/object_table.rs +++ b/src/model/s3/object_table.rs @@ -419,10 +419,8 @@ mod v2 { Uploading { /// Indicates whether this is a multipart upload multipart: bool, - /// Checksum algorithm to use - checksum_algorithm: Option, - /// Checksum algorithm type (full object or composite) - checksum_type: Option, + /// Checksum algorithm and algorithm type to use + checksum_algorithm: Option<(ChecksumAlgorithm, ChecksumType)>, /// Encryption params + headers to be included in the final object encryption: ObjectVersionEncryption, }, @@ -489,6 +487,10 @@ mod v2 { pub struct ObjectVersionMetaInner { pub headers: HeaderList, pub checksum: Option, + // checksum_type has to be stored separately, because when migrating + // from older versions of Garage, we can't know the correct value in + // ObjectVersionMetaInner::migrate (because it cannot take an argument + // that says whether the object was multipart or not) pub checksum_type: Option, } @@ -525,10 +527,9 @@ mod v2 { encryption, } => ObjectVersionState::Uploading { multipart, - checksum_algorithm, - checksum_type: Some(match multipart { - false => ChecksumType::FullObject, - true => ChecksumType::Composite, + checksum_algorithm: checksum_algorithm.map(|algo| match multipart { + false => (algo, ChecksumType::FullObject), + true => (algo, ChecksumType::Composite), }), encryption: migrate_encryption(encryption), }, From cfd10480ee476865163d6f380332f9aae996751a Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Thu, 22 May 2025 16:36:33 +0200 Subject: [PATCH 6/9] CopyObject: fix checksumming choice logic --- src/api/s3/copy.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs index dcc85d82..e1bef40e 100644 --- a/src/api/s3/copy.rs +++ b/src/api/s3/copy.rs @@ -78,7 +78,8 @@ pub async fn handle_copy( }, )?; - let was_multipart = source_version_meta.etag.contains('-'); // HACK + let was_multipart = source_version_meta.etag.contains('-') // HACK + || source_object_meta_inner.checksum_type == Some(ChecksumType::Composite); // Extract source checksum info before source_object_meta_inner is consumed let source_checksum = source_object_meta_inner.checksum; @@ -131,8 +132,8 @@ pub async fn handle_copy( // See: https://docs.aws.amazon.com/AmazonS3/latest/userguide/checking-object-integrity.html let must_recopy = !EncryptionParams::is_same(&source_encryption, &dest_encryption) - || source_checksum_algorithm != checksum_algorithm - || (was_multipart && checksum_algorithm.is_some()); + || (checksum_algorithm.is_some() + && (was_multipart || checksum_algorithm != source_checksum_algorithm)); let res = if !must_recopy { // In most cases, we can just copy the metadata and link blocks of the From 77125e94645b72e7dee4dc26efe777ff43bbf3db Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 18 Feb 2025 12:16:44 +0100 Subject: [PATCH 7/9] add boto3 test for STREAMING-UNSIGNED-PAYLOAD-TRAILER --- flake.lock | 8 ++++---- flake.nix | 4 ++-- script/test-smoke.sh | 17 +++++++++++++++++ shell.nix | 2 ++ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/flake.lock b/flake.lock index 2cfbfda4..f1b77c6c 100644 --- a/flake.lock +++ b/flake.lock @@ -50,17 +50,17 @@ }, "nixpkgs": { "locked": { - "lastModified": 1736692550, - "narHash": "sha256-7tk8xH+g0sJkKLTJFOxphJxxOjMDFMWv24nXslaU2ro=", + "lastModified": 1747825515, + "narHash": "sha256-BWpMQymVI73QoKZdcVCxUCCK3GNvr/xa2Dc4DM1o2BE=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "7c4869c47090dd7f9f1bdfb49a22aea026996815", + "rev": "cd2812de55cf87df88a9e09bf3be1ce63d50c1a6", "type": "github" }, "original": { "owner": "NixOS", "repo": "nixpkgs", - "rev": "7c4869c47090dd7f9f1bdfb49a22aea026996815", + "rev": "cd2812de55cf87df88a9e09bf3be1ce63d50c1a6", "type": "github" } }, diff --git a/flake.nix b/flake.nix index fc599e0b..82429a33 100644 --- a/flake.nix +++ b/flake.nix @@ -2,9 +2,9 @@ description = "Garage, an S3-compatible distributed object store for self-hosted deployments"; - # Nixpkgs 24.11 as of 2025-01-12 + # Nixpkgs 25.05 as of 2025-05-22 inputs.nixpkgs.url = - "github:NixOS/nixpkgs/7c4869c47090dd7f9f1bdfb49a22aea026996815"; + "github:NixOS/nixpkgs/cd2812de55cf87df88a9e09bf3be1ce63d50c1a6"; # Rust overlay as of 2025-02-03 inputs.rust-overlay.url = diff --git a/script/test-smoke.sh b/script/test-smoke.sh index acf56a90..eee206ba 100755 --- a/script/test-smoke.sh +++ b/script/test-smoke.sh @@ -112,6 +112,23 @@ if [ -z "$SKIP_S3CMD" ]; then done fi +# BOTO3 +if [ -z "$SKIP_BOTO3" ]; then + echo "🛠️ Testing with boto3 for STREAMING-UNSIGNED-PAYLOAD-TRAILER" + source ${SCRIPT_FOLDER}/dev-env-aws.sh + AWS_ENDPOINT_URL=https://localhost:4443 python < Date: Thu, 22 May 2025 18:14:53 +0200 Subject: [PATCH 8/9] fix test-smoke for CompleteMultipartUpload --- src/api/s3/multipart.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index 87fd2f98..4fea1c44 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -294,6 +294,10 @@ pub async fn handle_complete_multipart_upload( &req_head.headers, expected_checksum.map(|x| x.algorithm()), )?; + debug!( + "CompleteMultipartUpload expected checksum: {:?}, request checksum type: {:?}", + expected_checksum, req_checksum_algorithm + ); let body = req_body.collect().await?; @@ -324,8 +328,16 @@ pub async fn handle_complete_multipart_upload( } => (encryption, checksum_algorithm), _ => unreachable!(), }; - if req_checksum_algorithm != checksum_algorithm { - return Err(Error::InvalidDigest("checksum algorithm does not correspond to algorithm specified in CreateMultipartUpload".into())); + debug!( + "CompleteMultipartUpload object checksum_algorithm: {:?}", + checksum_algorithm + ); + if req_checksum_algorithm.is_some() && req_checksum_algorithm != checksum_algorithm { + return Err(Error::InvalidDigest(format!( + "checksum algorithm {:?} does not correspond to algorithm specified in CreateMultipartUpload {:?}", + req_checksum_algorithm, + checksum_algorithm + ))); } // Check that part numbers are an increasing sequence. @@ -352,9 +364,7 @@ pub async fn handle_complete_multipart_upload( for req_part in body_list_of_parts.iter() { match have_parts.get(&req_part.part_number) { Some(part) if part.etag.as_ref() == Some(&req_part.etag) && part.size.is_some() => { - if checksum_algorithm.map(|(_, ty)| ty) == Some(ChecksumType::Composite) - && part.checksum != req_part.checksum - { + if req_part.checksum.is_some() && part.checksum != req_part.checksum { return Err(Error::InvalidDigest(format!( "Invalid checksum for part {}: in request = {:?}, uploaded part = {:?}", req_part.part_number, req_part.checksum, part.checksum From c13af97b81f164a3c11b4b3d5abb2b9cc70a8c57 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Thu, 22 May 2025 18:17:56 +0200 Subject: [PATCH 9/9] return ChecksumType in CompleteMultipartUpload result --- src/api/common/signature/checksum.rs | 4 ++-- src/api/s3/multipart.rs | 9 +++++++-- src/api/s3/xml.rs | 5 +++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs index 464fdc75..d223175f 100644 --- a/src/api/common/signature/checksum.rs +++ b/src/api/common/signature/checksum.rs @@ -28,8 +28,8 @@ pub const X_AMZ_CHECKSUM_SHA1: HeaderName = HeaderName::from_static("x-amz-check pub const X_AMZ_CHECKSUM_SHA256: HeaderName = HeaderName::from_static("x-amz-checksum-sha256"); // Values for x-amz-checksum-type -pub const COMPOSITE: &[u8] = b"COMPOSITE"; -pub const FULL_OBJECT: &[u8] = b"FULL_OBJECT"; +pub const COMPOSITE: &str = "COMPOSITE"; +pub const FULL_OBJECT: &str = "FULL_OBJECT"; pub type Crc32Checksum = [u8; 4]; pub type Crc32cChecksum = [u8; 4]; diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index 4fea1c44..bb00f066 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -518,6 +518,11 @@ pub async fn handle_complete_multipart_upload( Some(ChecksumValue::Sha256(x)) => Some(s3_xml::Value(BASE64_STANDARD.encode(&x))), _ => None, }, + checksum_type: match checksum_algorithm { + Some((_, ChecksumType::Composite)) => Some(s3_xml::Value(COMPOSITE.into())), + Some((_, ChecksumType::FullObject)) => Some(s3_xml::Value(FULL_OBJECT.into())), + None => None, + }, }; let xml = s3_xml::to_xml_with_header(&result)?; @@ -691,8 +696,8 @@ pub fn request_checksum_algorithm_and_type( )), (Some(x), Some(algo)) => { let checksum_type = match x.as_bytes() { - COMPOSITE => ChecksumType::Composite, - FULL_OBJECT => ChecksumType::FullObject, + x if x == COMPOSITE.as_bytes() => ChecksumType::Composite, + x if x == FULL_OBJECT.as_bytes() => ChecksumType::FullObject, _ => return Err(Error::bad_request("Invalid x-amz-checksum-type value")), }; match (checksum_type, algo) { diff --git a/src/api/s3/xml.rs b/src/api/s3/xml.rs index 7dea3d1c..6c1d8f88 100644 --- a/src/api/s3/xml.rs +++ b/src/api/s3/xml.rs @@ -141,6 +141,8 @@ pub struct CompleteMultipartUploadResult { pub checksum_sha1: Option, #[serde(rename = "ChecksumSHA256")] pub checksum_sha256: Option, + #[serde(rename = "ChecksumType")] + pub checksum_type: Option, } #[derive(Debug, Serialize, PartialEq, Eq)] @@ -514,6 +516,7 @@ mod tests { #[test] fn complete_multipart_upload_result() -> Result<(), ApiError> { + use garage_api_common::signature::checksum::COMPOSITE; let result = CompleteMultipartUploadResult { xmlns: (), location: Some(Value("https://garage.tld/mybucket/a/plop".to_string())), @@ -525,6 +528,7 @@ mod tests { checksum_crc64nvme: None, checksum_sha1: Some(Value("ZJAnHyG8PeKz9tI8UTcHrJos39A=".into())), checksum_sha256: None, + checksum_type: Some(Value(COMPOSITE.into())), }; assert_eq!( to_xml_with_header(&result)?, @@ -535,6 +539,7 @@ mod tests { a/plop\ "3858f62230ac3c915f300c664312c11f-9"\ ZJAnHyG8PeKz9tI8UTcHrJos39A=\ + COMPOSITE\ " ); Ok(())