From 3e4c15da5d7eb2d44f8bf0b1c8156baf0e4b7659 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Mon, 6 Jul 2026 14:05:19 +0800 Subject: [PATCH] fix(object-lock): prevent locked version deletes (#4297) --- crates/ecstore/src/object_api/types.rs | 1 + crates/ecstore/src/set_disk/mod.rs | 84 ++++++++++++++++++- crates/ecstore/src/set_disk/ops/object.rs | 27 +++++- .../ecstore/src/storage_api_contracts/mod.rs | 4 +- crates/storage-api/src/lib.rs | 1 + crates/storage-api/src/object.rs | 5 ++ rustfs/src/app/object_usecase.rs | 6 +- rustfs/src/app/storage_api.rs | 11 +-- rustfs/src/storage/storage_api.rs | 4 +- 9 files changed, 132 insertions(+), 11 deletions(-) diff --git a/crates/ecstore/src/object_api/types.rs b/crates/ecstore/src/object_api/types.rs index 53715d520..26e758544 100644 --- a/crates/ecstore/src/object_api/types.rs +++ b/crates/ecstore/src/object_api/types.rs @@ -61,6 +61,7 @@ pub struct ObjectOptions { pub eval_metadata: Option>, pub object_lock_retention: Option, + pub object_lock_delete: Option, pub want_checksum: Option, pub skip_verify_bitrot: bool, diff --git a/crates/ecstore/src/set_disk/mod.rs b/crates/ecstore/src/set_disk/mod.rs index 4411e96c9..650e1affb 100644 --- a/crates/ecstore/src/set_disk/mod.rs +++ b/crates/ecstore/src/set_disk/mod.rs @@ -45,7 +45,7 @@ use crate::bucket::lifecycle::lifecycle::TRANSITION_COMPLETE; use crate::bucket::metadata_sys; -use crate::bucket::object_lock::objectlock_sys::check_retention_for_modification; +use crate::bucket::object_lock::objectlock_sys::{check_object_lock_for_deletion, check_retention_for_modification}; use crate::bucket::replication::{ ReplicateDecision, ReplicationObjectBridge, ReplicationState, ReplicationStatusType, VersionPurgeStatusType, replication_state_to_filemeta, @@ -2528,6 +2528,29 @@ fn check_object_lock_retention_update(bucket: &str, object: &str, obj_info: &Obj Ok(()) } +async fn check_object_lock_delete(bucket: &str, object: &str, obj_info: &ObjectInfo, opts: &ObjectOptions) -> Result<()> { + if set_disk_delete_creates_delete_marker(opts) { + return Ok(()); + } + + let bypass_governance = opts + .object_lock_delete + .as_ref() + .is_some_and(|delete_opts| delete_opts.bypass_governance); + if check_object_lock_for_deletion(bucket, obj_info, bypass_governance) + .await + .is_some() + { + return Err(StorageError::PrefixAccessDenied(bucket.to_string(), object.to_string())); + } + + Ok(()) +} + +fn set_disk_delete_creates_delete_marker(opts: &ObjectOptions) -> bool { + opts.version_id.is_none() && opts.versioned && !opts.version_suspended +} + fn should_preserve_delete_replication_state(opts: &ObjectOptions) -> bool { opts.delete_replication.as_ref().is_some_and(|state| { state.replica_status == ReplicationStatusType::Replica @@ -5972,6 +5995,65 @@ mod tests { .expect("GOVERNANCE shortening with bypass should remain allowed"); } + #[tokio::test] + async fn test_check_object_lock_delete_blocks_compliance_version_delete() { + let retain_until = OffsetDateTime::now_utc() + Duration::from_secs(60 * 60 * 24 * 60); + let mut user_defined = HashMap::new(); + user_defined.insert( + X_AMZ_OBJECT_LOCK_MODE.as_str().to_string(), + s3s::dto::ObjectLockRetentionMode::COMPLIANCE.to_string(), + ); + user_defined.insert( + X_AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE.as_str().to_string(), + retain_until.format(&time::format_description::well_known::Rfc3339).unwrap(), + ); + + let obj_info = ObjectInfo { + user_defined: Arc::new(user_defined), + ..Default::default() + }; + let opts = ObjectOptions { + version_id: Some(Uuid::new_v4().to_string()), + versioned: true, + ..Default::default() + }; + + let err = check_object_lock_delete("bucket", "object", &obj_info, &opts) + .await + .expect_err("COMPLIANCE retention must block explicit version deletion"); + + assert!(matches!(err, StorageError::PrefixAccessDenied(_, _))); + } + + #[tokio::test] + async fn test_check_object_lock_delete_allows_versioned_delete_marker_creation() { + let retain_until = OffsetDateTime::now_utc() + Duration::from_secs(60 * 60 * 24 * 60); + let mut user_defined = HashMap::new(); + user_defined.insert( + X_AMZ_OBJECT_LOCK_MODE.as_str().to_string(), + s3s::dto::ObjectLockRetentionMode::COMPLIANCE.to_string(), + ); + user_defined.insert( + X_AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE.as_str().to_string(), + retain_until.format(&time::format_description::well_known::Rfc3339).unwrap(), + ); + + let obj_info = ObjectInfo { + user_defined: Arc::new(user_defined), + ..Default::default() + }; + let opts = ObjectOptions { + version_id: None, + versioned: true, + version_suspended: false, + ..Default::default() + }; + + check_object_lock_delete("bucket", "object", &obj_info, &opts) + .await + .expect("versioned delete marker creation should not delete the locked version"); + } + #[test] fn test_should_persist_encryption_original_size_rejects_plain_metadata() { let metadata = HashMap::from([("content-type".to_string(), "application/octet-stream".to_string())]); diff --git a/crates/ecstore/src/set_disk/ops/object.rs b/crates/ecstore/src/set_disk/ops/object.rs index f516d7833..ac4f7ce66 100644 --- a/crates/ecstore/src/set_disk/ops/object.rs +++ b/crates/ecstore/src/set_disk/ops/object.rs @@ -1327,10 +1327,31 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks { let mut vers_map: HashMap<&String, FileInfoVersions> = HashMap::new(); for (i, dobj) in objects.iter().enumerate() { + if del_errs[i].is_some() { + continue; + } + let explicit_null_version = is_explicit_null_version(dobj.version_id); + let version_id = delete_file_info_version_id(dobj.version_id); + let check_opts = ObjectOptions { + version_id: version_id.map(|version_id| version_id.to_string()), + versioned: ver_cfg.prefix_enabled(dobj.object_name.as_str()), + version_suspended: ver_cfg.suspended(), + object_lock_delete: opts.object_lock_delete.clone(), + no_lock: true, + ..Default::default() + }; + let (goi, _write_quorum, gerr) = self.get_object_info_and_quorum(bucket, &dobj.object_name, &check_opts).await; + if gerr.is_none() + && let Err(err) = check_object_lock_delete(bucket, &dobj.object_name, &goi, &check_opts).await + { + del_errs[i] = Some(err); + continue; + } + let mut vr = FileInfo { name: dobj.object_name.clone(), - version_id: delete_file_info_version_id(dobj.version_id), + version_id, idx: i, replication_state_internal: Some(dobj.replication_state()), ..Default::default() @@ -1531,6 +1552,10 @@ impl crate::storage_api_contracts::object::ObjectOperations for SetDisks { } } + if version_found { + check_object_lock_delete(bucket, object, &goi, &opts).await?; + } + let otd = ObjectToDelete { object_name: object.to_string(), version_id: opts diff --git a/crates/ecstore/src/storage_api_contracts/mod.rs b/crates/ecstore/src/storage_api_contracts/mod.rs index de8986cb4..c965fb923 100644 --- a/crates/ecstore/src/storage_api_contracts/mod.rs +++ b/crates/ecstore/src/storage_api_contracts/mod.rs @@ -45,8 +45,8 @@ pub(crate) mod object { use super::{Debug, Error, FileInfo, GetObjectReader, ObjectInfo, ObjectOptions, PutObjReader}; use crate::storage_api_contracts::range::HTTPRangeSpec; pub(crate) use rustfs_storage_api::{ - DeletedObject, HTTPPreconditions, ObjectIO, ObjectLockRetentionOptions, ObjectOperations, ObjectPreconditionError, - ObjectPreconditionPart, ObjectPreconditionState, ObjectToDelete, + DeletedObject, HTTPPreconditions, ObjectIO, ObjectLockDeleteOptions, ObjectLockRetentionOptions, ObjectOperations, + ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState, ObjectToDelete, }; pub(crate) trait EcstoreObjectIO: diff --git a/crates/storage-api/src/lib.rs b/crates/storage-api/src/lib.rs index e6bc3ea5d..deffaf1cf 100644 --- a/crates/storage-api/src/lib.rs +++ b/crates/storage-api/src/lib.rs @@ -30,6 +30,7 @@ pub use bucket::{BucketInfo, BucketOperations, BucketOptions, DeleteBucketOption pub use capability::{CapabilitySnapshotError, CapabilityState, CapabilityStatus}; pub use error::{StorageErrorCode, StorageResult}; pub use multipart::{CompletePart, ListMultipartsInfo, ListPartsInfo, MultipartInfo, MultipartUploadResult, PartInfo}; +pub use object::ObjectLockDeleteOptions; pub use object::{DeletedObject, ObjectToDelete}; pub use object::{ExpirationOptions, TransitionedObject}; pub use object::{HTTPPreconditions, HTTPRangeError, HTTPRangeSpec, ObjectLockRetentionOptions}; diff --git a/crates/storage-api/src/object.rs b/crates/storage-api/src/object.rs index 7853708bf..6c68e33ca 100644 --- a/crates/storage-api/src/object.rs +++ b/crates/storage-api/src/object.rs @@ -53,6 +53,11 @@ pub struct ExpirationOptions { pub expire: bool, } +#[derive(Debug, Default, Clone)] +pub struct ObjectLockDeleteOptions { + pub bypass_governance: bool, +} + #[derive(Debug, Default, Clone)] pub struct TransitionedObject { pub name: String, diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index eedab4af0..f3f30ac45 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -188,7 +188,7 @@ use tracing::{debug, error, instrument, warn}; use uuid::Uuid; use super::storage_api::object_usecase::{ - StorageDeletedObject, StorageObjectInfo as ObjectInfo, StorageObjectOptions as ObjectOptions, + StorageDeletedObject, StorageObjectInfo as ObjectInfo, StorageObjectLockDeleteOptions, StorageObjectOptions as ObjectOptions, StorageObjectToDelete as ObjectToDelete, StoragePutObjReader as PutObjReader, }; use crate::app::object_data_cache::{ @@ -4847,6 +4847,7 @@ impl DefaultObjectUsecase { ObjectOptions { versioned: version_cfg.enabled(), version_suspended: version_cfg.suspended(), + object_lock_delete: Some(StorageObjectLockDeleteOptions { bypass_governance }), ..Default::default() }, ) @@ -5047,6 +5048,9 @@ impl DefaultObjectUsecase { let mut opts: ObjectOptions = del_opts(&bucket, &key, version_id, &req.headers, metadata) .await .map_err(ApiError::from)?; + opts.object_lock_delete = Some(StorageObjectLockDeleteOptions { + bypass_governance: has_bypass_governance_header(&req.headers), + }); let force_delete = opts.delete_prefix; let lock_cfg = BucketObjectLockSys::get(&bucket).await; diff --git a/rustfs/src/app/storage_api.rs b/rustfs/src/app/storage_api.rs index 1b2555ae2..da00419e0 100644 --- a/rustfs/src/app/storage_api.rs +++ b/rustfs/src/app/storage_api.rs @@ -967,11 +967,12 @@ pub(crate) mod object_usecase { object_utils, options, request_context, s3_api, set_disk, sse, storage_class, timeout_wrapper, }; pub(crate) use crate::storage::storage_api::{ - ECStore, RFC1123, StorageDeletedObject, StorageObjectInfo, StorageObjectOptions, StorageObjectToDelete, - StoragePutObjReader, check_preconditions, get_validated_store, has_replication_rules, parse_object_lock_legal_hold, - parse_object_lock_retention, parse_part_number_i32_to_usize, remove_object_lock_metadata_for_copy, - strip_managed_encryption_metadata, validate_bucket_object_lock_enabled, validate_object_key, - validate_sse_headers_for_read, validate_sse_headers_for_write, validate_ssec_for_read, wrap_response_with_cors, + ECStore, RFC1123, StorageDeletedObject, StorageObjectInfo, StorageObjectLockDeleteOptions, StorageObjectOptions, + StorageObjectToDelete, StoragePutObjReader, check_preconditions, get_validated_store, has_replication_rules, + parse_object_lock_legal_hold, parse_object_lock_retention, parse_part_number_i32_to_usize, + remove_object_lock_metadata_for_copy, strip_managed_encryption_metadata, validate_bucket_object_lock_enabled, + validate_object_key, validate_sse_headers_for_read, validate_sse_headers_for_write, validate_ssec_for_read, + wrap_response_with_cors, }; } diff --git a/rustfs/src/storage/storage_api.rs b/rustfs/src/storage/storage_api.rs index 4a66cca39..57cccd7e5 100644 --- a/rustfs/src/storage/storage_api.rs +++ b/rustfs/src/storage/storage_api.rs @@ -43,7 +43,8 @@ pub(crate) mod contract { pub(crate) mod object { pub(crate) use super::super::storage_contracts::{ - DeletedObject, HTTPPreconditions, ObjectIO, ObjectLockRetentionOptions, ObjectOperations, ObjectToDelete, + DeletedObject, HTTPPreconditions, ObjectIO, ObjectLockDeleteOptions, ObjectLockRetentionOptions, ObjectOperations, + ObjectToDelete, }; } @@ -59,6 +60,7 @@ pub(crate) mod contract { pub(crate) type StorageDeletedObject = contract::object::DeletedObject; pub(crate) type StorageGetObjectReader = super::GetObjectReader; pub(crate) type StorageObjectInfo = super::ObjectInfo; +pub(crate) type StorageObjectLockDeleteOptions = contract::object::ObjectLockDeleteOptions; pub(crate) type StorageObjectOptions = super::ObjectOptions; pub(crate) type StorageObjectToDelete = contract::object::ObjectToDelete; pub(crate) type StoragePutObjReader = super::PutObjReader;