From a780f9f06bc18b0e35dcbc0b16edd14b9d9b7522 Mon Sep 17 00:00:00 2001 From: overtrue Date: Sat, 1 Aug 2026 10:53:06 +0800 Subject: [PATCH] test(kms): pin the refused immediate deletion at the admin endpoint The KMS e2e lifecycles asserted that force_immediate destroys a key; a default server now refuses it, so they assert the refusal and that the key survives it, and finish through the window-bounded schedule path instead. Documents the server gate alongside the Vault policy that permanent deletion needs. --- .../src/kms/configured_roundtrip_test.rs | 86 +++++++++++-------- crates/e2e_test/src/kms/kms_vault_test.rs | 33 +++---- docs/operations/kms-backend-security.md | 4 +- 3 files changed, 71 insertions(+), 52 deletions(-) diff --git a/crates/e2e_test/src/kms/configured_roundtrip_test.rs b/crates/e2e_test/src/kms/configured_roundtrip_test.rs index 835ce4e9f..13e96ef0b 100644 --- a/crates/e2e_test/src/kms/configured_roundtrip_test.rs +++ b/crates/e2e_test/src/kms/configured_roundtrip_test.rs @@ -126,7 +126,10 @@ async fn assert_key_deletion_lifecycle(base_url: &str, access_key: &str, secret_ assert_eq!(cancelled["success"], true); assert_eq!(cancelled["key_metadata"]["key_state"], "Enabled"); - let removed = kms_admin_request( + // A default server refuses to skip the waiting window (rustfs/backlog#1585): + // immediate deletion is unrecoverable and takes every object encrypted under + // the key with it, so the endpoint must reject it rather than honour it. + let refused = kms_admin_request( base_url, http::Method::DELETE, "/rustfs/admin/v3/kms/keys/delete", @@ -140,37 +143,49 @@ async fn assert_key_deletion_lifecycle(base_url: &str, access_key: &str, secret_ access_key, secret_key, ) - .await?; - let removed: serde_json::Value = serde_json::from_str(&removed)?; - assert_eq!(removed["success"], true); + .await + .err() + .ok_or("immediate KMS key deletion must be refused on a default server")?; + assert!( + refused.to_string().contains("400 Bad Request"), + "refused immediate deletion must report a client error: {refused}" + ); - let listed = - kms_admin_request(base_url, http::Method::GET, "/rustfs/admin/v3/kms/keys", None, access_key, secret_key).await?; - let listed: serde_json::Value = serde_json::from_str(&listed)?; - assert_eq!(listed["success"], true); - let keys = listed["keys"] - .as_array() - .ok_or("list KMS keys response omitted keys after deletion")?; - if let Some(key) = keys.iter().find(|key| key["key_id"] == key_id) { - assert_eq!(key["status"], "PendingDeletion", "a retained force-deleted key must be pending deletion"); - let removed = kms_admin_request( - base_url, - http::Method::DELETE, - "/rustfs/admin/v3/kms/keys/delete", - Some( - &serde_json::json!({ - "key_id": key_id, - "force_immediate": true - }) - .to_string(), - ), - access_key, - secret_key, - ) - .await?; - let removed: serde_json::Value = serde_json::from_str(&removed)?; - assert_eq!(removed["success"], true); - } + // The refused request left the key alone, so the window-bounded path still + // has something to schedule. + let described = kms_admin_request( + base_url, + http::Method::GET, + &format!("/rustfs/admin/v3/kms/keys/{key_id}"), + None, + access_key, + secret_key, + ) + .await?; + let described: serde_json::Value = serde_json::from_str(&described)?; + assert_eq!( + described["key_metadata"]["key_state"], "Enabled", + "a refused immediate deletion must leave the key usable" + ); + + let rescheduled = kms_admin_request( + base_url, + http::Method::DELETE, + "/rustfs/admin/v3/kms/keys/delete", + Some( + &serde_json::json!({ + "key_id": key_id, + "pending_window_in_days": 7 + }) + .to_string(), + ), + access_key, + secret_key, + ) + .await?; + let rescheduled: serde_json::Value = serde_json::from_str(&rescheduled)?; + assert_eq!(rescheduled["success"], true); + assert!(rescheduled["deletion_date"].is_string()); let listed = kms_admin_request(base_url, http::Method::GET, "/rustfs/admin/v3/kms/keys", None, access_key, secret_key).await?; @@ -179,10 +194,11 @@ async fn assert_key_deletion_lifecycle(base_url: &str, access_key: &str, secret_ let keys = listed["keys"] .as_array() .ok_or("final list KMS keys response omitted keys after deletion")?; - assert!( - keys.iter().all(|key| key["key_id"] != key_id), - "force-deleted KMS key must no longer appear in list" - ); + let key = keys + .iter() + .find(|key| key["key_id"] == key_id) + .ok_or("a key awaiting its deletion window must still be listed")?; + assert_eq!(key["status"], "PendingDeletion", "a scheduled key must be pending deletion"); Ok(()) } diff --git a/crates/e2e_test/src/kms/kms_vault_test.rs b/crates/e2e_test/src/kms/kms_vault_test.rs index 0a1201bd8..1377241ab 100644 --- a/crates/e2e_test/src/kms/kms_vault_test.rs +++ b/crates/e2e_test/src/kms/kms_vault_test.rs @@ -449,29 +449,32 @@ async fn test_vault_kms_key_crud( info!("✅ Delete verification: Key state correctly changed to: {}", key_state); - // Force Delete - Force immediate deletion for PendingDeletion key - let force_delete_response = crate::common::execute_awscurl( + // Force Delete - a default server refuses to skip the waiting window + // (rustfs/backlog#1585): destroying the key material immediately would take + // every object encrypted under the key with it. + let force_delete_error = crate::common::execute_awscurl( &format!("{base_url}/rustfs/admin/v3/kms/keys/delete?keyId={key_id}&force_immediate=true"), "DELETE", None, access_key, secret_key, ) - .await?; + .await + .err() + .expect("Immediate KMS key deletion must be refused on a default server"); + info!("✅ Force Delete: correctly refused for key {}: {}", key_id, force_delete_error); - // Parse and validate the force delete response - let force_delete_result: serde_json::Value = serde_json::from_str(&force_delete_response)?; - assert_eq!(force_delete_result["success"], true, "Force delete operation must return success=true"); - info!("✅ Force Delete: Successfully force deleted key: {}", key_id); + // The refused request must leave the key exactly as it was: still present, + // still pending deletion, still recoverable through cancel-deletion. + let describe_after_refusal = + crate::common::awscurl_get(&format!("{base_url}/rustfs/admin/v3/kms/keys/{key_id}"), access_key, secret_key).await?; + let describe_after_refusal: serde_json::Value = serde_json::from_str(&describe_after_refusal)?; + assert_eq!( + describe_after_refusal["key_metadata"]["key_state"], "PendingDeletion", + "A refused immediate deletion must leave the key pending deletion" + ); - // Verify key no longer exists after force deletion (should return error) - let describe_force_deleted_result = - crate::common::awscurl_get(&format!("{base_url}/rustfs/admin/v3/kms/keys/{key_id}"), access_key, secret_key).await; - - // After force deletion, key should not be found (GET should fail) - assert!(describe_force_deleted_result.is_err(), "Force deleted key should not be found"); - - info!("✅ Force Delete verification: Key was permanently deleted and is no longer accessible"); + info!("✅ Force Delete verification: Key survived the refused immediate deletion"); info!("Vault KMS key CRUD operations completed successfully"); Ok(()) diff --git a/docs/operations/kms-backend-security.md b/docs/operations/kms-backend-security.md index c888bc497..d99c5dbac 100644 --- a/docs/operations/kms-backend-security.md +++ b/docs/operations/kms-backend-security.md @@ -44,7 +44,7 @@ path "secret/metadata/rustfs/kms/keys/*" { Notes: - The trailing wildcards also cover the per-version material records that rotation creates under `.../keys/{key_id}/versions/{N}`; no extra policy paths are needed. -- `delete` on the metadata path is required for permanent key deletion (`force_immediate`); drop it if you never hard-delete keys. +- `delete` on the metadata path is required for permanent key deletion (`force_immediate`); drop it if you never hard-delete keys. RustFS refuses `force_immediate` unless the server sets `RUSTFS_KMS_ALLOW_IMMEDIATE_DELETION=true`, so leaving that gate off keeps the capability unreachable no matter what the Vault policy allows. - Do not attach `sudo`, wildcard mounts, or Transit paths to this policy; the KV2 backend does not use them. - Auditing KV reads on the key prefix is strongly recommended: every read event is a potential master-key disclosure. @@ -62,7 +62,7 @@ Decryption loads exactly the version recorded in the envelope and fails closed w - Every version record that any stored DEK envelope references must remain readable. Until an object rewrap/migration capability exists, assume **every** version of a rotated key is referenced: destroying a version record permanently orphans all objects whose DEKs it wrapped. - Version records are ordinary KV v2 secrets under the key subtree. Never run `kv metadata delete` or `kv destroy` against `{prefix}/{key_id}/versions/*`, and do not apply `delete-version-after` or retention tooling to that subtree. RustFS-managed retention does not rely on KV2's own secret versioning (each version record has a single KV revision), so KV `max-versions` settings do not protect or endanger history — but metadata deletion always removes a record entirely. -- Permanent key deletion through RustFS (`force_immediate` after `PendingDeletion`) purges the key's version records together with the key record; that is the only supported way to remove them. +- Permanent key deletion through RustFS (`force_immediate` after `PendingDeletion`) purges the key's version records together with the key record; that is the only supported way to remove them. It is refused by default: the server must set `RUSTFS_KMS_ALLOW_IMMEDIATE_DELETION=true`, and the request must echo the key id back as `confirm_key_id`. Leave the gate off unless you are actively destroying keys, and turn it off again afterwards — the pending-deletion window plus `CancelKeyDeletion` is the only recovery path for objects encrypted under the key. - For Vault Transit, retention is governed by the Transit key's `min_decryption_version`: never raise it above the oldest version that may still protect live ciphertext. ### Upgrade before first rotation (hard constraint)