fix(app): fail closed on an unreadable bucket encryption config (#7183)

The object write path read the bucket default encryption configuration
with `.ok()`, which made "this bucket has no default encryption" and "the
encryption configuration cannot be read" the same value. A bucket whose
encryption blob is damaged therefore stored plaintext objects the
operator had mandated be encrypted, with nothing returned to the client
and nothing in the object to tell those writes apart afterwards.

PUT, COPY and the snowball extract path now share one resolver: an
absent configuration still writes plaintext exactly as before, and every
other outcome refuses the write, carrying the accessor's typed error so
a damaged blob surfaces as a deterministic InternalError while a
transient metadata read failure surfaces as the retryable
ServiceUnavailable. A missing bucket and a cold metadata cache both
still resolve to "no configuration", so neither becomes a refusal. This
matches `prepare_sse_configuration` in `storage::sse`, the resolver the
multipart writer has always used, which fails closed on this lookup.
This commit is contained in:
Zhengchao An
2026-09-05 14:13:59 +08:00
committed by GitHub
parent a3b8183be9
commit d8c3b1bb26
5 changed files with 321 additions and 3 deletions
+45 -1
View File
@@ -394,7 +394,7 @@ impl DefaultObjectUsecase {
// Bucket metadata uses the bucket name as its namespace-lock key. Load
// every copy-time bucket snapshot before a same-object key can collide
// with that key (for example, copying `bucket/bucket` onto itself).
let bucket_sse_config = metadata_sys::get_sse_config(&bucket).await.ok();
let bucket_sse_config = load_bucket_default_sse_config(&bucket).await?;
let object_lock_config_state = load_bucket_object_lock_config_state(&bucket).await?;
if cp_src_dst_same && key == bucket {
dst_opts.object_lock_config_snapshot =
@@ -1388,4 +1388,48 @@ mod tests {
.unwrap_err();
assert_eq!(err.code(), &S3ErrorCode::InvalidRequest);
}
#[tokio::test]
#[serial_test::serial]
async fn execute_copy_object_refuses_a_bucket_whose_encryption_config_is_unreadable() {
use crate::app::storage_api::test::contract::bucket::{BucketOperations as _, MakeBucketOptions};
let (store, context) = real_store_test_context().await;
let bucket = format!("copy-sse-unreadable-{}", Uuid::new_v4());
let source = "source.bin";
let destination = "destination.bin";
store
.make_bucket(&bucket, &MakeBucketOptions::default())
.await
.expect("unreadable-encryption copy bucket must be created");
let mut reader = PutObjReader::from_vec(b"copied while the bucket still had a readable configuration".to_vec());
store
.put_object(&bucket, source, &mut reader, &ObjectOptions::default())
.await
.expect("copy source object must be written");
install_unreadable_bucket_sse_config(&bucket).await;
let input = CopyObjectInput::builder()
.copy_source(CopySource::Bucket {
bucket: bucket.clone().into(),
key: source.into(),
version_id: None,
})
.bucket(bucket.clone())
.key(destination.to_string())
.build()
.expect("copy input must build");
let usecase = DefaultObjectUsecase::with_context(Some(Arc::clone(&context)));
let err = Box::pin(usecase.execute_copy_object(build_request(input, Method::PUT)))
.await
.expect_err("an unreadable bucket encryption configuration must refuse the copy");
assert_eq!(err.code(), &S3ErrorCode::InternalError);
let lookup_err = store
.get_object_info(&bucket, destination, &ObjectOptions::default())
.await
.expect_err("a refused copy must not leave a destination object behind");
assert!(is_err_object_not_found(&lookup_err), "{lookup_err}");
}
}
+1 -1
View File
@@ -2037,7 +2037,7 @@ impl DefaultObjectUsecase {
let sse_customer_key_md5 = sse_customer_key_md5.or(h_md5);
let original_sse = server_side_encryption.or(extract_server_side_encryption_from_headers(&req.headers)?);
let bucket_sse_config = metadata_sys::get_sse_config(&bucket).await.ok();
let bucket_sse_config = load_bucket_default_sse_config(&bucket).await?;
let (mut effective_sse, mut effective_kms_key_id) = resolve_bucket_default_sse(
bucket_sse_config.as_ref().map(|(config, _timestamp)| config),
original_sse,
+119 -1
View File
@@ -1485,8 +1485,9 @@ impl DefaultObjectUsecase {
};
let sse_config_stage_start = put_stage_metrics_enabled.then(Instant::now);
let bucket_sse_config = metadata_sys::get_sse_config(&bucket).await.ok();
let bucket_sse_config = load_bucket_default_sse_config(&bucket).await;
rustfs_io_metrics::record_put_object_stage_duration_from("app_sse_config_lookup", sse_config_stage_start);
let bucket_sse_config = bucket_sse_config?;
debug!(
target: "rustfs::app::object_usecase",
component = "app",
@@ -3912,4 +3913,121 @@ mod tests {
.expect_err("writes after the zero-byte quota update must be denied");
assert!(matches!(err, StorageError::QuotaExceeded { current: 4096, limit: 0 }));
}
#[tokio::test]
#[serial_test::serial]
async fn execute_put_object_refuses_a_bucket_whose_encryption_config_is_unreadable() {
use crate::app::storage_api::test::contract::bucket::{BucketOperations as _, MakeBucketOptions};
let (store, context) = real_store_test_context().await;
let bucket = format!("put-sse-unreadable-{}", Uuid::new_v4());
let object = "object.bin";
store
.make_bucket(&bucket, &MakeBucketOptions::default())
.await
.expect("unreadable-encryption PUT bucket must be created");
install_unreadable_bucket_sse_config(&bucket).await;
let payload = Bytes::from_static(b"an operator mandated encryption for this bucket");
let input = PutObjectInput::builder()
.bucket(bucket.clone())
.key(object.to_string())
.body(Some(StreamingBlob::from(s3s::Body::from(payload.clone()))))
.content_length(Some(i64::try_from(payload.len()).expect("test payload length must fit i64")))
.build()
.expect("PUT input must build");
let usecase = DefaultObjectUsecase::with_context(Some(Arc::clone(&context)));
let err = Box::pin(usecase.execute_put_object(&FS::new(), build_request(input, Method::PUT)))
.await
.expect_err("an unreadable bucket encryption configuration must refuse the write");
assert_eq!(err.code(), &S3ErrorCode::InternalError);
let lookup_err = store
.get_object_info(&bucket, object, &ObjectOptions::default())
.await
.expect_err("a refused PUT must not leave an object behind");
assert!(is_err_object_not_found(&lookup_err), "{lookup_err}");
}
#[tokio::test]
#[serial_test::serial]
async fn execute_put_object_still_writes_plaintext_without_bucket_encryption() {
use crate::app::storage_api::test::contract::bucket::{BucketOperations as _, MakeBucketOptions};
let (store, context) = real_store_test_context().await;
let bucket = format!("put-sse-absent-{}", Uuid::new_v4());
let object = "object.bin";
store
.make_bucket(&bucket, &MakeBucketOptions::default())
.await
.expect("plaintext PUT bucket must be created");
let payload = Bytes::from_static(b"no default encryption is configured for this bucket");
let input = PutObjectInput::builder()
.bucket(bucket.clone())
.key(object.to_string())
.body(Some(StreamingBlob::from(s3s::Body::from(payload.clone()))))
.content_length(Some(i64::try_from(payload.len()).expect("test payload length must fit i64")))
.build()
.expect("PUT input must build");
let usecase = DefaultObjectUsecase::with_context(Some(Arc::clone(&context)));
Box::pin(usecase.execute_put_object(&FS::new(), build_request(input, Method::PUT)))
.await
.expect("a bucket without default encryption must still accept a plaintext write");
let stored = store
.get_object_info(&bucket, object, &ObjectOptions::default())
.await
.expect("the plaintext object must be readable");
assert_eq!(stored.size, i64::try_from(payload.len()).expect("test payload length must fit i64"));
assert!(
!stored
.user_defined
.keys()
.any(|key| key.eq_ignore_ascii_case(AMZ_SERVER_SIDE_ENCRYPTION)
|| key.starts_with("x-rustfs-encryption-")
|| key.starts_with("x-minio-encryption-")),
"the object must carry no encryption metadata: {:?}",
stored.user_defined
);
}
#[tokio::test]
#[serial_test::serial]
async fn execute_put_object_extract_refuses_a_bucket_whose_encryption_config_is_unreadable() {
use crate::app::storage_api::test::contract::bucket::{BucketOperations as _, MakeBucketOptions};
let (store, context) = real_store_test_context().await;
let bucket = format!("extract-sse-unreadable-{}", Uuid::new_v4());
store
.make_bucket(&bucket, &MakeBucketOptions::default())
.await
.expect("unreadable-encryption extract bucket must be created");
install_unreadable_bucket_sse_config(&bucket).await;
let payload = Bytes::from_static(b"archive bytes that must never be unpacked in plaintext");
let input = PutObjectInput::builder()
.bucket(bucket.clone())
.key("archive.tar".to_string())
.body(Some(StreamingBlob::from(s3s::Body::from(payload.clone()))))
.content_length(Some(i64::try_from(payload.len()).expect("test payload length must fit i64")))
.build()
.expect("extract PUT input must build");
let mut req = build_request(input, Method::PUT);
req.headers.insert(AMZ_SNOWBALL_EXTRACT, HeaderValue::from_static("true"));
let usecase = DefaultObjectUsecase::with_context(Some(Arc::clone(&context)));
let err = Box::pin(usecase.execute_put_object(&FS::new(), req))
.await
.expect_err("an unreadable bucket encryption configuration must refuse the extract upload");
assert_eq!(err.code(), &S3ErrorCode::InternalError);
let lookup_err = store
.get_object_info(&bucket, "archive.tar", &ObjectOptions::default())
.await
.expect_err("a refused extract upload must not leave an object behind");
assert!(is_err_object_not_found(&lookup_err), "{lookup_err}");
}
}
+123
View File
@@ -269,6 +269,129 @@ pub(super) fn resolve_bucket_default_sse(
(effective_sse, effective_kms_key_id)
}
/// The bucket's default encryption configuration for a write path.
///
/// `Ok(None)` carries one meaning only — this bucket has no default encryption
/// — and the write proceeds in plaintext exactly as before. Every other
/// outcome refuses the write rather than collapsing onto that same value: an
/// encryption blob that exists but cannot be read fails closed in
/// `get_sse_config` since rustfs/rustfs#7172, and swallowing that error here
/// stores plaintext into a bucket whose operator mandated encryption, with
/// nothing returned to the client and nothing in the object to tell it apart
/// afterwards (rustfs/backlog#2287).
///
/// The states the lookup can report, and what each one does:
///
/// * configured and readable — apply the bucket default;
/// * no encryption blob at all, including a bucket that does not exist and a
/// bucket whose metadata document is absent — `ConfigNotFound`, so a cold
/// cache and a missing bucket are never turned into a refusal, and the write
/// still fails later with its own `NoSuchBucket`;
/// * blob present but unparseable — deterministic, so retrying cannot help;
/// surfaces as `InternalError` until an operator repairs or removes it;
/// * the metadata read itself failed (namespace lock, quorum, disk, an
/// uninitialized metadata system) — transient, and the typed error maps to
/// the retryable `ServiceUnavailable`.
///
/// The last two are distinguished by the typed error the accessor returns, not
/// re-derived here: [`ApiError`] already separates them. This mirrors
/// `prepare_sse_configuration` in `storage::sse`, the resolver the multipart
/// writer uses, which has always failed closed on the same lookup.
pub(super) async fn load_bucket_default_sse_config(
bucket: &str,
) -> S3Result<Option<(ServerSideEncryptionConfiguration, OffsetDateTime)>> {
classify_bucket_default_sse_lookup(bucket, metadata_sys::get_sse_config(bucket).await)
}
fn classify_bucket_default_sse_lookup(
bucket: &str,
lookup: Result<(ServerSideEncryptionConfiguration, OffsetDateTime), StorageError>,
) -> S3Result<Option<(ServerSideEncryptionConfiguration, OffsetDateTime)>> {
match lookup {
Ok(config) => Ok(Some(config)),
Err(err) if err == StorageError::ConfigNotFound => Ok(None),
Err(err) => {
let api_error = ApiError::from(err);
error!(
event = "bucket_sse_config_lookup_failed",
component = LOG_COMPONENT_APP,
subsystem = LOG_SUBSYSTEM_OBJECT,
result = "write_refused",
bucket = %bucket,
code = %api_error.code.as_str(),
error = %api_error,
"Bucket default encryption is unreadable; refusing the write instead of storing plaintext"
);
Err(api_error.into())
}
}
}
#[cfg(test)]
mod bucket_default_sse_lookup_tests {
use super::*;
use s3s::dto::{ServerSideEncryptionByDefault, ServerSideEncryptionRule};
use time::OffsetDateTime;
fn sse_config() -> ServerSideEncryptionConfiguration {
ServerSideEncryptionConfiguration {
rules: vec![ServerSideEncryptionRule {
apply_server_side_encryption_by_default: Some(ServerSideEncryptionByDefault {
sse_algorithm: ServerSideEncryption::from_static(ServerSideEncryption::AES256),
kms_master_key_id: None,
}),
blocked_encryption_types: None,
bucket_key_enabled: None,
}],
}
}
#[test]
fn an_absent_configuration_still_writes_plaintext() {
let resolved = classify_bucket_default_sse_lookup("bucket", Err(StorageError::ConfigNotFound))
.expect("a bucket without default encryption must keep writing plaintext");
assert!(resolved.is_none());
assert_eq!(resolve_bucket_default_sse(None, None, None, false), (None, None));
}
#[test]
fn a_readable_configuration_is_returned() {
let resolved = classify_bucket_default_sse_lookup("bucket", Ok((sse_config(), OffsetDateTime::UNIX_EPOCH)))
.expect("a readable configuration must not refuse the write")
.expect("a readable configuration must be applied");
assert_eq!(resolved.0.rules.len(), 1);
}
#[test]
fn an_unreadable_configuration_refuses_the_write() {
let err = classify_bucket_default_sse_lookup(
"bucket",
Err(StorageError::other("persisted bucket encryption configuration is invalid")),
)
.expect_err("a corrupt encryption blob must never degrade to plaintext");
assert_eq!(err.code(), &S3ErrorCode::InternalError);
}
#[test]
fn an_unavailable_metadata_read_refuses_the_write_as_retryable() {
let err = classify_bucket_default_sse_lookup("bucket", Err(StorageError::ErasureReadQuorum))
.expect_err("an unreadable metadata subsystem must never degrade to plaintext");
assert_eq!(err.code(), &S3ErrorCode::ServiceUnavailable);
}
#[test]
fn a_missing_bucket_keeps_its_own_error() {
let err = classify_bucket_default_sse_lookup("bucket", Err(StorageError::BucketNotFound("bucket".to_string())))
.expect_err("a bucket-not-found lookup must not be reported as an encryption failure");
assert_eq!(err.code(), &S3ErrorCode::NoSuchBucket);
}
}
#[cfg(test)]
mod deadlock_request_guard_tests {
use super::DeadlockRequestGuard;
+33
View File
@@ -96,3 +96,36 @@ pub(super) fn real_cold_fill_plan(
};
plan
}
/// A store with an ambient `AppContext`, for tests that drive a handler end to
/// end without the object-data-cache overrides of
/// [`real_cold_fill_test_context`].
pub(super) async fn real_store_test_context() -> (Arc<ECStore>, Arc<AppContext>) {
let store = crate::app::gating_test_env::shared_gating_ecstore().await;
if current_app_context().is_none() {
crate::app::runtime_sources::install_test_app_context(Arc::clone(&store)).await;
}
let ambient = current_app_context().expect("real-store tests require an ambient AppContext");
let context = Arc::new(AppContext::new(Arc::clone(&store), ambient.iam(), ambient.kms()));
(store, context)
}
/// Leave the bucket in the state a damaged encryption blob produces: the raw
/// document is retained and the typed configuration stays `None`, which is the
/// durable "exists but cannot be read" signal `get_sse_config` fails closed on
/// (rustfs/rustfs#7172).
pub(super) async fn install_unreadable_bucket_sse_config(bucket: &str) {
use crate::app::storage_api::test::{get_global_bucket_metadata_sys, set_bucket_metadata};
let sys = get_global_bucket_metadata_sys().expect("bucket metadata system must be initialized");
let metadata = {
let sys = sys.read().await;
sys.get(bucket).await.expect("bucket metadata must be cached")
};
let mut metadata = (*metadata).clone();
metadata.encryption_config_xml = b"<ServerSideEncryptionConfiguration>truncated".to_vec();
metadata.sse_config = None;
set_bucket_metadata(bucket.to_string(), metadata)
.await
.expect("unreadable bucket encryption configuration must be installed");
}