mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-15 09:33:13 +00:00
Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 77b712dcb6 |
@@ -3101,6 +3101,26 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn list_keys_fails_closed_on_a_corrupt_record() {
|
||||||
|
let (client, _temp_dir) = create_test_client().await;
|
||||||
|
client.create_key("corrupt", "AES_256", None).await.expect("create key");
|
||||||
|
|
||||||
|
let key_path = client.master_key_path("corrupt").expect("valid key id");
|
||||||
|
fs::write(&key_path, b"{\"key_id\":\"corrupt\"")
|
||||||
|
.await
|
||||||
|
.expect("write corrupt record");
|
||||||
|
|
||||||
|
let error = client
|
||||||
|
.list_keys(&ListKeysRequest::default(), None)
|
||||||
|
.await
|
||||||
|
.expect_err("a corrupt record must not disappear from a listing");
|
||||||
|
assert!(
|
||||||
|
matches!(error, KmsError::MaterialCorrupt { key_id, .. } if key_id == "corrupt"),
|
||||||
|
"got {error:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn missing_salt_with_pre_marker_key_records_still_initializes() {
|
async fn missing_salt_with_pre_marker_key_records_still_initializes() {
|
||||||
let (dev_client, temp_dir) = create_dev_mode_client().await;
|
let (dev_client, temp_dir) = create_dev_mode_client().await;
|
||||||
|
|||||||
@@ -160,7 +160,7 @@ pub(crate) fn ensure_rewrap_context_matches(
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Page size used when a [`ListKeysRequest`] does not ask for one.
|
/// Page size used when a [`ListKeysRequest`] does not ask for one.
|
||||||
pub(crate) const DEFAULT_LIST_KEYS_PAGE_SIZE: u32 = 100;
|
pub(crate) const DEFAULT_LIST_KEYS_PAGE_SIZE: u32 = DEFAULT_LIST_KEYS_LIMIT;
|
||||||
|
|
||||||
/// One page of a key set the backend has to slice itself.
|
/// One page of a key set the backend has to slice itself.
|
||||||
pub(crate) struct KeyPage<'a, T> {
|
pub(crate) struct KeyPage<'a, T> {
|
||||||
@@ -231,7 +231,7 @@ pub(crate) fn paginate_keys<'a, T>(sorted: &'a [T], request: &ListKeysRequest, k
|
|||||||
pub(crate) fn list_keys_page_size(limit: Option<u32>) -> Option<usize> {
|
pub(crate) fn list_keys_page_size(limit: Option<u32>) -> Option<usize> {
|
||||||
match limit.unwrap_or(DEFAULT_LIST_KEYS_PAGE_SIZE) {
|
match limit.unwrap_or(DEFAULT_LIST_KEYS_PAGE_SIZE) {
|
||||||
0 => None,
|
0 => None,
|
||||||
size => Some(size as usize),
|
size => Some(size.min(MAX_LIST_KEYS_LIMIT) as usize),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -860,6 +860,21 @@ mod tests {
|
|||||||
assert_eq!(page_of(&keys, Some(u32::MAX), Some("key-01")), (vec![keys[2].clone()], None, false));
|
assert_eq!(page_of(&keys, Some(u32::MAX), Some("key-01")), (vec![keys[2].clone()], None, false));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn list_limit_is_bounded_at_the_contract_maximum() {
|
||||||
|
assert_eq!(list_keys_page_size(Some(MAX_LIST_KEYS_LIMIT)), Some(MAX_LIST_KEYS_LIMIT as usize));
|
||||||
|
assert_eq!(
|
||||||
|
list_keys_page_size(Some(MAX_LIST_KEYS_LIMIT + 1)),
|
||||||
|
Some(MAX_LIST_KEYS_LIMIT as usize),
|
||||||
|
"one past the maximum must not expand a backend page"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
list_keys_page_size(Some(u32::MAX)),
|
||||||
|
Some(MAX_LIST_KEYS_LIMIT as usize),
|
||||||
|
"the largest wire value must use the same bounded page"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// The cursor is an identifier, so a marker naming a key that no longer
|
/// The cursor is an identifier, so a marker naming a key that no longer
|
||||||
/// exists resumes after where it would have been instead of restarting.
|
/// exists resumes after where it would have been instead of restarting.
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -59,6 +59,12 @@ pub struct StaticKmsBackend {
|
|||||||
key_id: String,
|
key_id: String,
|
||||||
/// The raw 32-byte AES-256 key material (zeroed on drop).
|
/// The raw 32-byte AES-256 key material (zeroed on drop).
|
||||||
key: Zeroizing<[u8; KEY_SIZE]>,
|
key: Zeroizing<[u8; KEY_SIZE]>,
|
||||||
|
/// The time this configured key became visible to the backend.
|
||||||
|
///
|
||||||
|
/// Static KMS has no persisted key record, so this startup timestamp is
|
||||||
|
/// the only stable creation date it can report across list and describe
|
||||||
|
/// calls. It must not be regenerated for every response.
|
||||||
|
created_at: Zoned,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl StaticKmsBackend {
|
impl StaticKmsBackend {
|
||||||
@@ -82,6 +88,7 @@ impl StaticKmsBackend {
|
|||||||
Ok(Self {
|
Ok(Self {
|
||||||
key_id: static_config.key_id.clone(),
|
key_id: static_config.key_id.clone(),
|
||||||
key: key.into(),
|
key: key.into(),
|
||||||
|
created_at: Zoned::now(),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -92,7 +99,7 @@ impl StaticKmsBackend {
|
|||||||
key_state: KeyState::Enabled,
|
key_state: KeyState::Enabled,
|
||||||
key_usage: KeyUsage::EncryptDecrypt,
|
key_usage: KeyUsage::EncryptDecrypt,
|
||||||
description: Some("Static single-key KMS backend".to_string()),
|
description: Some("Static single-key KMS backend".to_string()),
|
||||||
creation_date: Zoned::now(),
|
creation_date: self.created_at.clone(),
|
||||||
deletion_date: None,
|
deletion_date: None,
|
||||||
origin: "EXTERNAL".to_string(),
|
origin: "EXTERNAL".to_string(),
|
||||||
key_manager: "STATIC".to_string(),
|
key_manager: "STATIC".to_string(),
|
||||||
@@ -281,7 +288,7 @@ impl StaticKmsBackend {
|
|||||||
version: 1,
|
version: 1,
|
||||||
metadata: HashMap::new(),
|
metadata: HashMap::new(),
|
||||||
tags: HashMap::new(),
|
tags: HashMap::new(),
|
||||||
created_at: Zoned::now(),
|
created_at: self.created_at.clone(),
|
||||||
rotated_at: None,
|
rotated_at: None,
|
||||||
created_by: None,
|
created_by: None,
|
||||||
};
|
};
|
||||||
@@ -677,6 +684,23 @@ mod tests {
|
|||||||
assert!(!response.truncated);
|
assert!(!response.truncated);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn list_key_creation_date_is_stable_for_the_backend_lifetime() {
|
||||||
|
let (backend, key_id, _key) = create_test_backend().await;
|
||||||
|
|
||||||
|
let first = backend
|
||||||
|
.list_configured_key(&ListKeysRequest::default())
|
||||||
|
.expect("first list should succeed");
|
||||||
|
tokio::task::yield_now().await;
|
||||||
|
let second = backend
|
||||||
|
.list_configured_key(&ListKeysRequest::default())
|
||||||
|
.expect("second list should succeed");
|
||||||
|
let described = backend.configured_key_info(&key_id).expect("describe should succeed");
|
||||||
|
|
||||||
|
assert_eq!(first.keys[0].created_at, second.keys[0].created_at);
|
||||||
|
assert_eq!(first.keys[0].created_at, described.created_at);
|
||||||
|
}
|
||||||
|
|
||||||
/// A zero limit means zero keys, even for a backend whose whole key set is
|
/// A zero limit means zero keys, even for a backend whose whole key set is
|
||||||
/// a single configured key.
|
/// a single configured key.
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
|
|||||||
@@ -1213,8 +1213,13 @@ impl VaultKmsClient {
|
|||||||
// A key that disappeared between the listing and the read is
|
// A key that disappeared between the listing and the read is
|
||||||
// dropped from the page rather than failing it; the cursor comes
|
// dropped from the page rather than failing it; the cursor comes
|
||||||
// from the identifier list, so the listing still advances past it.
|
// from the identifier list, so the listing still advances past it.
|
||||||
let Ok(key_info) = self.describe_key(key_id, None).await else {
|
// Any other read error means the record is present but this build
|
||||||
continue;
|
// cannot interpret it. Hiding that entry would make the listing
|
||||||
|
// claim a complete key set while silently omitting corrupt data.
|
||||||
|
let key_info = match self.describe_key(key_id, None).await {
|
||||||
|
Ok(key_info) => key_info,
|
||||||
|
Err(KmsError::KeyNotFound { .. }) => continue,
|
||||||
|
Err(error) => return Err(error),
|
||||||
};
|
};
|
||||||
if request
|
if request
|
||||||
.status_filter
|
.status_filter
|
||||||
@@ -1955,6 +1960,32 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn list_keys_fails_closed_on_an_unreadable_kv2_record() {
|
||||||
|
let malformed_record = serde_json::json!({
|
||||||
|
"data": {"algorithm": "AES_256"},
|
||||||
|
"metadata": {
|
||||||
|
"created_time": "2026-01-01T00:00:00Z",
|
||||||
|
"deletion_time": "",
|
||||||
|
"custom_metadata": null,
|
||||||
|
"destroyed": false,
|
||||||
|
"version": 1
|
||||||
|
}
|
||||||
|
});
|
||||||
|
let (vault, client) = scripted_client(vec![
|
||||||
|
ScriptedResponse::ok(serde_json::json!({ "keys": ["poisoned"] })),
|
||||||
|
ScriptedResponse::ok(malformed_record),
|
||||||
|
])
|
||||||
|
.await;
|
||||||
|
|
||||||
|
let error = client
|
||||||
|
.list_keys(&ListKeysRequest::default(), None)
|
||||||
|
.await
|
||||||
|
.expect_err("a malformed KV2 record must not disappear from a listing");
|
||||||
|
assert!(matches!(error, KmsError::BackendError { .. }), "got {error:?}");
|
||||||
|
assert_eq!(vault.requests().len(), 2, "listing must read the listed record before failing");
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn wired_read_retries_transient_status_then_succeeds() {
|
async fn wired_read_retries_transient_status_then_succeeds() {
|
||||||
let (vault, client) = scripted_client(vec![
|
let (vault, client) = scripted_client(vec![
|
||||||
|
|||||||
+10
-2
@@ -415,7 +415,15 @@ impl DecryptRequest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Request to list keys
|
/// Default page size used when a request omits `limit`.
|
||||||
|
pub const DEFAULT_LIST_KEYS_LIMIT: u32 = 100;
|
||||||
|
/// Maximum page size accepted by KMS list implementations.
|
||||||
|
pub const MAX_LIST_KEYS_LIMIT: u32 = 1000;
|
||||||
|
|
||||||
|
/// Request to list keys.
|
||||||
|
///
|
||||||
|
/// List pages default to [`DEFAULT_LIST_KEYS_LIMIT`] entries and are capped at
|
||||||
|
/// [`MAX_LIST_KEYS_LIMIT`]. A zero limit is a valid request for an empty page.
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||||
pub struct ListKeysRequest {
|
pub struct ListKeysRequest {
|
||||||
/// Maximum number of keys to return
|
/// Maximum number of keys to return
|
||||||
@@ -431,7 +439,7 @@ pub struct ListKeysRequest {
|
|||||||
impl Default for ListKeysRequest {
|
impl Default for ListKeysRequest {
|
||||||
fn default() -> Self {
|
fn default() -> Self {
|
||||||
Self {
|
Self {
|
||||||
limit: Some(100),
|
limit: Some(DEFAULT_LIST_KEYS_LIMIT),
|
||||||
marker: None,
|
marker: None,
|
||||||
usage_filter: None,
|
usage_filter: None,
|
||||||
status_filter: None,
|
status_filter: None,
|
||||||
|
|||||||
@@ -180,6 +180,21 @@ fn key_list_filters(query_params: &HashMap<String, String>) -> Result<KeyListFil
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Parse the optional page size without turning malformed input into a
|
||||||
|
/// different request. Values above the backend contract maximum are bounded
|
||||||
|
/// so a client cannot make one list request describe an unbounded number of
|
||||||
|
/// keys.
|
||||||
|
fn key_list_limit(query_params: &HashMap<String, String>) -> Result<u32, String> {
|
||||||
|
let Some(value) = query_params.get("limit") else {
|
||||||
|
return Ok(DEFAULT_LIST_KEYS_LIMIT);
|
||||||
|
};
|
||||||
|
|
||||||
|
value
|
||||||
|
.parse::<u32>()
|
||||||
|
.map(|limit| limit.min(MAX_LIST_KEYS_LIMIT))
|
||||||
|
.map_err(|_| format!("invalid value for 'limit': expected an unsigned integer, got '{value}'"))
|
||||||
|
}
|
||||||
|
|
||||||
fn extract_key_id(uri: &hyper::Uri) -> Option<String> {
|
fn extract_key_id(uri: &hyper::Uri) -> Option<String> {
|
||||||
let query_params = extract_query_params(uri);
|
let query_params = extract_query_params(uri);
|
||||||
["keyId", "key-id", "key"]
|
["keyId", "key-id", "key"]
|
||||||
@@ -443,12 +458,16 @@ mod tests {
|
|||||||
use super::{
|
use super::{
|
||||||
CancelKmsKeyDeletionRequest, CreateKeyApiRequest, CreateKmsKeyRequest, DeleteKmsKeyRequest, DeleteKmsKeyResponse,
|
CancelKmsKeyDeletionRequest, CreateKeyApiRequest, CreateKmsKeyRequest, DeleteKmsKeyRequest, DeleteKmsKeyResponse,
|
||||||
DescribeKmsKeyResponse, GenerateDataKeyApiRequest, delete_key_error_status, delete_request_from_query, extract_key_id,
|
DescribeKmsKeyResponse, GenerateDataKeyApiRequest, delete_key_error_status, delete_request_from_query, extract_key_id,
|
||||||
extract_query_params, key_impact_if_requested, key_list_filters, kms_create_key_actions, kms_delete_key_actions,
|
extract_query_params, key_impact_if_requested, key_list_filters, key_list_limit, kms_create_key_actions,
|
||||||
kms_describe_key_actions, kms_generate_data_key_actions, kms_list_keys_actions, scoped_key_id, wants_key_impact,
|
kms_delete_key_actions, kms_describe_key_actions, kms_generate_data_key_actions, kms_list_keys_actions, scoped_key_id,
|
||||||
|
wants_key_impact,
|
||||||
};
|
};
|
||||||
use http::Uri;
|
use http::Uri;
|
||||||
use hyper::StatusCode;
|
use hyper::StatusCode;
|
||||||
use rustfs_kms::{KeyImpactReport, KeyReference, KeyReferenceKind, KeyStatus, KeyUsage, KmsError, ReferenceScope};
|
use rustfs_kms::{
|
||||||
|
DEFAULT_LIST_KEYS_LIMIT, KeyImpactReport, KeyReference, KeyReferenceKind, KeyStatus, KeyUsage, KmsError,
|
||||||
|
MAX_LIST_KEYS_LIMIT, ReferenceScope,
|
||||||
|
};
|
||||||
use rustfs_policy::policy::action::{Action, AdminAction, KmsAction};
|
use rustfs_policy::policy::action::{Action, AdminAction, KmsAction};
|
||||||
use rustfs_policy::policy::{Args, Policy};
|
use rustfs_policy::policy::{Args, Policy};
|
||||||
use std::collections::HashMap;
|
use std::collections::HashMap;
|
||||||
@@ -996,6 +1015,42 @@ mod tests {
|
|||||||
key_list_filters(&extract_query_params(&uri)).map(|filters| (filters.status, filters.usage))
|
key_list_filters(&extract_query_params(&uri)).map(|filters| (filters.status, filters.usage))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn list_limit(query: &str) -> Result<u32, String> {
|
||||||
|
let uri: Uri = format!("/rustfs/admin/v3/kms/keys{query}").parse().expect("uri should parse");
|
||||||
|
key_list_limit(&extract_query_params(&uri))
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_key_listing_limit_is_bounded_and_malformed_values_are_refused() {
|
||||||
|
assert_eq!(list_limit(""), Ok(DEFAULT_LIST_KEYS_LIMIT));
|
||||||
|
assert_eq!(list_limit("?limit=0"), Ok(0));
|
||||||
|
assert_eq!(list_limit(&format!("?limit={MAX_LIST_KEYS_LIMIT}")), Ok(MAX_LIST_KEYS_LIMIT));
|
||||||
|
assert_eq!(
|
||||||
|
list_limit(&format!("?limit={}", MAX_LIST_KEYS_LIMIT + 1)),
|
||||||
|
Ok(MAX_LIST_KEYS_LIMIT),
|
||||||
|
"one past the maximum must be clamped"
|
||||||
|
);
|
||||||
|
assert_eq!(list_limit("?limit=4294967295"), Ok(MAX_LIST_KEYS_LIMIT));
|
||||||
|
|
||||||
|
for query in ["?limit=abc", "?limit=-1", "?limit="] {
|
||||||
|
let error = list_limit(query).expect_err("malformed limits must not fall back to the default");
|
||||||
|
assert!(error.contains("invalid value for 'limit'"), "unhelpful error for {query}: {error}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn both_list_handlers_validate_the_requested_limit() {
|
||||||
|
let src = include_str!("kms_keys.rs");
|
||||||
|
|
||||||
|
for handler in ["ListKeysHandler", "ListKmsKeysHandler"] {
|
||||||
|
let block = operation_block(src, handler);
|
||||||
|
assert!(
|
||||||
|
block.contains("key_list_limit(&query_params)"),
|
||||||
|
"{handler} must reject malformed limits before reaching the KMS"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// A filter is either applied or refused. Answering a narrowed listing with
|
/// A filter is either applied or refused. Answering a narrowed listing with
|
||||||
/// the unfiltered key set looks, from the response alone, exactly like a key
|
/// the unfiltered key set looks, from the response alone, exactly like a key
|
||||||
/// set in which everything matches — the caller cannot tell that its
|
/// set in which everything matches — the caller cannot tell that its
|
||||||
@@ -1138,7 +1193,7 @@ impl Operation for ListKeysHandler {
|
|||||||
)?;
|
)?;
|
||||||
|
|
||||||
let query_params = extract_query_params(&req.uri);
|
let query_params = extract_query_params(&req.uri);
|
||||||
let limit = query_params.get("limit").and_then(|s| s.parse::<u32>().ok()).unwrap_or(100);
|
let limit = key_list_limit(&query_params).map_err(|message| s3_error!(InvalidArgument, "{}", message))?;
|
||||||
let marker = query_params.get("marker").cloned();
|
let marker = query_params.get("marker").cloned();
|
||||||
// Validated before the listing runs, so a filter the service cannot
|
// Validated before the listing runs, so a filter the service cannot
|
||||||
// apply fails as the input error it is instead of returning the whole
|
// apply fails as the input error it is instead of returning the whole
|
||||||
@@ -1878,7 +1933,23 @@ impl Operation for ListKmsKeysHandler {
|
|||||||
)?;
|
)?;
|
||||||
|
|
||||||
let query_params = extract_query_params(&req.uri);
|
let query_params = extract_query_params(&req.uri);
|
||||||
let limit = query_params.get("limit").and_then(|s| s.parse::<u32>().ok()).unwrap_or(100);
|
let limit = match key_list_limit(&query_params) {
|
||||||
|
Ok(limit) => limit,
|
||||||
|
Err(message) => {
|
||||||
|
let response = ListKmsKeysResponse {
|
||||||
|
success: false,
|
||||||
|
message,
|
||||||
|
keys: vec![],
|
||||||
|
truncated: false,
|
||||||
|
next_marker: None,
|
||||||
|
};
|
||||||
|
let data =
|
||||||
|
serde_json::to_vec(&response).map_err(|e| s3_error!(InternalError, "failed to serialize response: {}", e))?;
|
||||||
|
let mut headers = HeaderMap::new();
|
||||||
|
headers.insert(CONTENT_TYPE, "application/json".parse().expect("operation should succeed"));
|
||||||
|
return Ok(S3Response::with_headers((StatusCode::BAD_REQUEST, Body::from(data)), headers));
|
||||||
|
}
|
||||||
|
};
|
||||||
let marker = query_params.get("marker").cloned();
|
let marker = query_params.get("marker").cloned();
|
||||||
// Validated before the listing runs, so a filter the service cannot
|
// Validated before the listing runs, so a filter the service cannot
|
||||||
// apply fails as the input error it is instead of returning the whole
|
// apply fails as the input error it is instead of returning the whole
|
||||||
|
|||||||
Reference in New Issue
Block a user