From 8a65017f36daaabc823498bc33312baab80e1420 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Mon, 3 Aug 2026 01:53:01 +0800 Subject: [PATCH] fix(kms): bound persisted format parsing (#5652) fix(kms): harden persisted format compatibility --- crates/kms/src/backends/local.rs | 467 +++++++++++++++++----- crates/kms/src/backup/local_export.rs | 41 +- crates/kms/src/backup/local_restore.rs | 19 +- crates/kms/src/encryption/dek.rs | 238 ++++++++--- crates/kms/src/lib.rs | 122 ++++++ crates/kms/src/persisted_observability.rs | 62 +++ docs/operations/kms-backend-security.md | 4 +- 7 files changed, 790 insertions(+), 163 deletions(-) create mode 100644 crates/kms/src/persisted_observability.rs diff --git a/crates/kms/src/backends/local.rs b/crates/kms/src/backends/local.rs index 9245fe2b9..d7e550b5d 100644 --- a/crates/kms/src/backends/local.rs +++ b/crates/kms/src/backends/local.rs @@ -22,6 +22,7 @@ use crate::config::KmsConfig; use crate::config::LocalConfig; use crate::encryption::{AesDekCrypto, DataKeyEnvelope, DekCrypto, generate_key_material}; use crate::error::{KmsError, Result}; +use crate::persisted_observability::{BoundedUnknownFieldName, UnknownFieldSummary}; use crate::types::*; use aes_gcm::{ Aes256Gcm, Key, Nonce, @@ -32,12 +33,16 @@ use async_trait::async_trait; use base64::{Engine as _, engine::general_purpose::STANDARD as BASE64}; use jiff::Zoned; use rand::RngExt; -use serde::de::IgnoredAny; +use serde::de::{self, IgnoredAny, MapAccess, Visitor}; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use std::collections::HashMap; +use std::fmt; use std::path::{Component, Path, PathBuf}; -use std::sync::{Arc, Mutex}; +use std::sync::{ + Arc, Mutex, + atomic::{AtomicU64, Ordering}, +}; use std::time::Duration; use tokio::fs; use tracing::{debug, warn}; @@ -481,9 +486,32 @@ pub(crate) enum StoredKeyProtection { PlaintextDevOnly, } -/// The record's `at_rest_protection` value when this build cannot interpret -/// it, rendered for diagnostics. `Ok(None)` means the marker is absent -/// (pre-beta.9 records) or names a protection mode this build implements. +pub(crate) const UNKNOWN_STORED_KEY_PROTECTION: &str = "unknown-at-rest-protection"; +const MAX_PROTECTION_MARKER_RAW_BYTES: usize = 128; + +impl UnknownFieldSummary { + fn record_for_local_key(&self) { + let Some((field, field_name_truncated, field_count)) = self.record("local-key-record") else { + return; + }; + + static RECORDS_WITH_UNKNOWN_FIELDS: AtomicU64 = AtomicU64::new(0); + let observed_records = RECORDS_WITH_UNKNOWN_FIELDS.fetch_add(1, Ordering::Relaxed).saturating_add(1); + if observed_records.is_power_of_two() { + tracing::warn!( + field = ?field, + field_name_truncated, + field_count, + observed_records, + "Local KMS key record contains unknown fields" + ); + } + } +} + +/// Reports whether the record's `at_rest_protection` value is unknown to this +/// build. `false` means the marker is absent (pre-beta.9 records), null, or +/// names a protection mode this build implements. /// /// Every reader of a stored key record must consult this before its own /// schema parse. Letting a strict [`StoredKeyProtection`] field fail inside a @@ -491,24 +519,29 @@ pub(crate) enum StoredKeyProtection { /// operator who reads corruption starts a disaster recovery instead of a /// version rollback. The probe deliberately ignores every other field, so the /// verdict is available even for records whose schema this build cannot -/// satisfy, and no key material is copied out of the caller's buffer. +/// satisfy. The raw marker is borrowed and length-bounded before enum parsing; +/// it is never propagated into a caller-visible error or diagnostic. /// /// `Err` carries the JSON error so callers can keep their own classification /// for bytes that are not a record at all. -pub(crate) fn unknown_protection_marker(record: &[u8]) -> serde_json::Result> { +pub(crate) fn has_unknown_protection_marker(record: &[u8]) -> serde_json::Result { #[derive(Deserialize)] - struct MarkerProbe { + struct MarkerProbe<'a> { #[serde(default)] - at_rest_protection: Option, + #[serde(borrow)] + at_rest_protection: Option<&'a serde_json::value::RawValue>, } let Some(marker) = serde_json::from_slice::(record)?.at_rest_protection else { - return Ok(None); + return Ok(false); }; - if serde_json::from_value::(marker.clone()).is_ok() { - return Ok(None); + if marker.get().len() > MAX_PROTECTION_MARKER_RAW_BYTES { + return Ok(true); } - Ok(Some(marker.as_str().map(str::to_owned).unwrap_or_else(|| marker.to_string()))) + if serde_json::from_str::(marker.get()).is_ok() { + return Ok(false); + } + Ok(true) } /// Serializable representation of a master key stored on disk @@ -566,62 +599,178 @@ impl<'de> Deserialize<'de> for StoredMasterKey { where D: serde::Deserializer<'de>, { + enum Field { + FormatVersion, + KeyId, + Version, + Algorithm, + Usage, + Status, + Description, + Metadata, + CreatedAt, + RotatedAt, + CreatedBy, + DeletionDate, + EncryptedKeyMaterial, + Nonce, + AtRestProtection, + Unknown(BoundedUnknownFieldName), + } + + impl<'de> Deserialize<'de> for Field { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::Deserializer<'de>, + { + struct FieldVisitor; + + impl Visitor<'_> for FieldVisitor { + type Value = Field; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a Local KMS key record field name") + } + + fn visit_str(self, value: &str) -> std::result::Result + where + E: de::Error, + { + Ok(match value { + "format_version" => Field::FormatVersion, + "key_id" => Field::KeyId, + "version" => Field::Version, + "algorithm" => Field::Algorithm, + "usage" => Field::Usage, + "status" => Field::Status, + "description" => Field::Description, + "metadata" => Field::Metadata, + "created_at" => Field::CreatedAt, + "rotated_at" => Field::RotatedAt, + "created_by" => Field::CreatedBy, + "deletion_date" => Field::DeletionDate, + "encrypted_key_material" => Field::EncryptedKeyMaterial, + "nonce" => Field::Nonce, + "at_rest_protection" => Field::AtRestProtection, + _ => Field::Unknown(BoundedUnknownFieldName::new(value)), + }) + } + } + + deserializer.deserialize_identifier(FieldVisitor) + } + } + #[derive(Deserialize)] - struct Wire { - #[serde(default = "default_stored_master_key_format_version")] - format_version: u32, - key_id: String, - version: u32, - algorithm: String, - usage: KeyUsage, - status: KeyStatus, - description: Option, - metadata: HashMap, - #[serde(with = "crate::time_serde::zoned")] - created_at: Zoned, - #[serde(with = "crate::time_serde::option_zoned")] - rotated_at: Option, - created_by: Option, - #[serde(default, with = "crate::time_serde::option_zoned")] - deletion_date: Option, - encrypted_key_material: String, - nonce: Vec, - #[serde(default)] - at_rest_protection: StoredKeyProtection, - #[serde(flatten)] - unknown_fields: HashMap, + struct ZonedValue(#[serde(with = "crate::time_serde::zoned")] Zoned); + + #[derive(Deserialize)] + struct OptionalZonedValue(#[serde(with = "crate::time_serde::option_zoned")] Option); + + struct StoredMasterKeyVisitor; + + impl<'de> Visitor<'de> for StoredMasterKeyVisitor { + type Value = StoredMasterKey; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a Local KMS key record") + } + + fn visit_map(self, mut map: A) -> std::result::Result + where + A: MapAccess<'de>, + { + macro_rules! read_field { + ($slot:ident, $name:literal) => {{ + if $slot.is_some() { + return Err(de::Error::duplicate_field($name)); + } + $slot = Some(map.next_value()?); + }}; + } + + let mut format_version = None; + let mut key_id = None; + let mut version = None; + let mut algorithm = None; + let mut usage = None; + let mut status = None; + let mut description = None; + let mut metadata = None; + let mut created_at: Option = None; + let mut rotated_at: Option = None; + let mut created_by = None; + let mut deletion_date: Option = None; + let mut encrypted_key_material = None; + let mut nonce = None; + let mut at_rest_protection = None; + let mut unknown_fields = UnknownFieldSummary::default(); + + while let Some(field) = map.next_key()? { + match field { + Field::FormatVersion => read_field!(format_version, "format_version"), + Field::KeyId => read_field!(key_id, "key_id"), + Field::Version => read_field!(version, "version"), + Field::Algorithm => read_field!(algorithm, "algorithm"), + Field::Usage => read_field!(usage, "usage"), + Field::Status => read_field!(status, "status"), + Field::Description => read_field!(description, "description"), + Field::Metadata => read_field!(metadata, "metadata"), + Field::CreatedAt => read_field!(created_at, "created_at"), + Field::RotatedAt => read_field!(rotated_at, "rotated_at"), + Field::CreatedBy => read_field!(created_by, "created_by"), + Field::DeletionDate => read_field!(deletion_date, "deletion_date"), + Field::EncryptedKeyMaterial => read_field!(encrypted_key_material, "encrypted_key_material"), + Field::Nonce => read_field!(nonce, "nonce"), + Field::AtRestProtection => read_field!(at_rest_protection, "at_rest_protection"), + Field::Unknown(field) => { + let _: IgnoredAny = map.next_value()?; + unknown_fields.observe(field); + } + } + } + + let key = StoredMasterKey { + format_version: format_version.unwrap_or_else(default_stored_master_key_format_version), + key_id: key_id.ok_or_else(|| de::Error::missing_field("key_id"))?, + version: version.ok_or_else(|| de::Error::missing_field("version"))?, + algorithm: algorithm.ok_or_else(|| de::Error::missing_field("algorithm"))?, + usage: usage.ok_or_else(|| de::Error::missing_field("usage"))?, + status: status.ok_or_else(|| de::Error::missing_field("status"))?, + description: description.unwrap_or(None), + metadata: metadata.ok_or_else(|| de::Error::missing_field("metadata"))?, + created_at: created_at.ok_or_else(|| de::Error::missing_field("created_at"))?.0, + rotated_at: rotated_at.map(|value: OptionalZonedValue| value.0).unwrap_or(None), + created_by: created_by.unwrap_or(None), + deletion_date: deletion_date.map(|value: OptionalZonedValue| value.0).unwrap_or(None), + encrypted_key_material: encrypted_key_material + .ok_or_else(|| de::Error::missing_field("encrypted_key_material"))?, + nonce: nonce.ok_or_else(|| de::Error::missing_field("nonce"))?, + at_rest_protection: at_rest_protection.unwrap_or_default(), + }; + unknown_fields.record_for_local_key(); + Ok(key) + } } - let wire = Wire::deserialize(deserializer)?; - if let Some(field) = wire.unknown_fields.keys().min() { - static WARN_UNKNOWN_FIELDS: std::sync::Once = std::sync::Once::new(); - WARN_UNKNOWN_FIELDS.call_once(|| { - tracing::warn!( - key_id = %wire.key_id, - field = %field, - field_count = wire.unknown_fields.len(), - "Local KMS key record contains unknown fields" - ); - }); - } - - Ok(Self { - format_version: wire.format_version, - key_id: wire.key_id, - version: wire.version, - algorithm: wire.algorithm, - usage: wire.usage, - status: wire.status, - description: wire.description, - metadata: wire.metadata, - created_at: wire.created_at, - rotated_at: wire.rotated_at, - created_by: wire.created_by, - deletion_date: wire.deletion_date, - encrypted_key_material: wire.encrypted_key_material, - nonce: wire.nonce, - at_rest_protection: wire.at_rest_protection, - }) + const FIELDS: &[&str] = &[ + "format_version", + "key_id", + "version", + "algorithm", + "usage", + "status", + "description", + "metadata", + "created_at", + "rotated_at", + "created_by", + "deletion_date", + "encrypted_key_material", + "nonce", + "at_rest_protection", + ]; + deserializer.deserialize_struct("StoredMasterKey", FIELDS, StoredMasterKeyVisitor) } } @@ -909,6 +1058,22 @@ impl LocalKmsClient { path.display() ))); } + let has_unknown_marker = has_unknown_protection_marker(&content).map_err(|error| { + KmsError::configuration_error(format!( + "Local KMS master key salt at {} is missing and key record {} is not a readable JSON object ({error}); \ + refusing to generate a replacement salt", + Self::master_key_salt_path(config).display(), + path.display() + )) + })?; + if has_unknown_marker { + return Err(KmsError::configuration_error(format!( + "Local KMS master key salt at {} is missing and key record {} uses {UNKNOWN_STORED_KEY_PROTECTION}; \ + refusing to generate a replacement salt", + Self::master_key_salt_path(config).display(), + path.display() + ))); + } let probe = serde_json::from_slice::(&content).map_err(|error| { KmsError::configuration_error(format!( "Local KMS master key salt at {} is missing and key record {} is not interpretable by this build ({error}); \ @@ -965,10 +1130,10 @@ impl LocalKmsClient { // Two-stage parse so an unrecognised protection marker is reported as an // unsupported format (a newer build may still read the key) instead of being // folded into generic corruption with every other malformed record. - let unknown_marker = unknown_protection_marker(&content) + let has_unknown_marker = has_unknown_protection_marker(&content) .map_err(|e| KmsError::material_corrupt(key_id, format!("stored key record is not a readable JSON object: {e}")))?; - if let Some(version) = unknown_marker { - return Err(KmsError::unsupported_format_version(key_id, version)); + if has_unknown_marker { + return Err(KmsError::unsupported_format_version(key_id, UNKNOWN_STORED_KEY_PROTECTION)); } let stored_key: StoredMasterKey = serde_json::from_slice(&content) .map_err(|e| KmsError::material_corrupt(key_id, format!("stored key record does not deserialize: {e}")))?; @@ -2015,6 +2180,8 @@ impl KmsBackend for LocalKmsBackend { #[cfg(test)] mod tests { use super::*; + use crate::test_support::{deserialize_with_ignored_only_unknown, unknown_field_metric}; + use metrics_util::debugging::DebuggingRecorder; use std::collections::HashMap; use tempfile::TempDir; @@ -2227,8 +2394,8 @@ mod tests { ), ( "unknown protection marker", - with_field("at_rest_protection", serde_json::json!("post-quantum-v2")), - |e| matches!(e, KmsError::UnsupportedFormatVersion { version, .. } if version == "post-quantum-v2"), + with_field("at_rest_protection", serde_json::json!("secret-marker-value-must-not-leak")), + |e| matches!(e, KmsError::UnsupportedFormatVersion { version, .. } if version == UNKNOWN_STORED_KEY_PROTECTION), ), ]; @@ -2546,10 +2713,56 @@ mod tests { client.create_key("format-key", "AES_256", None).await.expect("create key"); let key_path = client.master_key_path("format-key").expect("valid key id"); - let mut record: serde_json::Value = - serde_json::from_slice(&fs::read(&key_path).await.expect("read key record")).expect("decode key record"); + let current_record = fs::read(&key_path).await.expect("read key record"); + let mut record: serde_json::Value = serde_json::from_slice(¤t_record).expect("decode key record"); assert_eq!(record.get("format_version"), Some(&serde_json::json!(STORED_MASTER_KEY_FORMAT_VERSION))); + #[derive(Deserialize)] + struct LegacyStoredMasterKeyProbe { + key_id: String, + version: u32, + algorithm: String, + usage: KeyUsage, + status: KeyStatus, + description: Option, + metadata: HashMap, + #[serde(with = "crate::time_serde::zoned")] + created_at: Zoned, + #[serde(with = "crate::time_serde::option_zoned")] + rotated_at: Option, + created_by: Option, + #[serde(default, with = "crate::time_serde::option_zoned")] + deletion_date: Option, + encrypted_key_material: String, + nonce: Vec, + #[serde(default)] + at_rest_protection: StoredKeyProtection, + } + let legacy: LegacyStoredMasterKeyProbe = + serde_json::from_slice(¤t_record).expect("the pre-format-version reader must accept a v1 record"); + let LegacyStoredMasterKeyProbe { + key_id, + version, + algorithm, + usage: _usage, + status: _status, + description: _description, + metadata: _metadata, + created_at: _created_at, + rotated_at: _rotated_at, + created_by: _created_by, + deletion_date: _deletion_date, + encrypted_key_material, + nonce, + at_rest_protection, + } = legacy; + assert_eq!(key_id, "format-key"); + assert_eq!(version, 1); + assert_eq!(algorithm, "AES_256"); + assert!(!encrypted_key_material.is_empty()); + assert!(nonce.is_empty()); + assert_eq!(at_rest_protection, StoredKeyProtection::PlaintextDevOnly); + // A record from before the explicit field was added remains readable. record .as_object_mut() @@ -2617,6 +2830,35 @@ mod tests { )); } + #[test] + fn unknown_protection_marker_is_static_for_every_unknown_shape() { + for marker in [ + serde_json::json!("secret-string-must-not-leak"), + serde_json::json!({"future_mode": "secret-object-must-not-leak"}), + serde_json::json!(["secret-array-must-not-leak"]), + serde_json::json!(99), + ] { + let record = + serde_json::to_vec(&serde_json::json!({"at_rest_protection": marker})).expect("encode protection marker"); + assert!(has_unknown_protection_marker(&record).expect("probe protection marker")); + } + let long_marker = "secret-marker-must-not-be-copied".repeat(1024); + let record = + serde_json::to_vec(&serde_json::json!({"at_rest_protection": long_marker})).expect("encode long protection marker"); + assert!(has_unknown_protection_marker(&record).expect("probe long protection marker")); + + for marker in [ + serde_json::Value::Null, + serde_json::json!("legacy-unspecified"), + serde_json::json!("encrypted-master-key"), + serde_json::json!("plaintext-dev-only"), + ] { + let record = + serde_json::to_vec(&serde_json::json!({"at_rest_protection": marker})).expect("encode protection marker"); + assert!(!has_unknown_protection_marker(&record).expect("probe protection marker")); + } + } + #[tokio::test] async fn stored_master_key_unknown_fields_remain_readable() { const UNKNOWN_FIELD_VALUE: &str = "field value must not be logged"; @@ -2627,41 +2869,66 @@ mod tests { .expect("create key"); let key_path = client.master_key_path("unknown-field-key").expect("valid key id"); - let mut record: serde_json::Value = + let record: serde_json::Value = serde_json::from_slice(&fs::read(&key_path).await.expect("read key record")).expect("decode key record"); - record["alpha_extension"] = serde_json::json!("field value must not be logged"); - record["zeta_extension"] = serde_json::json!("another value must not be logged"); - fs::write( - &key_path, - serde_json::to_vec_pretty(&record).expect("encode key record with unknown field"), - ) - .await - .expect("write key record with unknown field"); - + let long_field = format!("{}界", "a".repeat(126)); + let long_prefix = "a".repeat(126); + let injection_field = "b\n\u{1b}[31m"; + let record_with_unknown = |field: &str| { + let mut record = record.clone(); + let object = record.as_object_mut().expect("key record is an object"); + object.insert(field.to_owned(), serde_json::json!(UNKNOWN_FIELD_VALUE)); + object.insert("zeta_extension".to_owned(), serde_json::json!("another value must not be logged")); + serde_json::to_vec_pretty(&record).expect("encode key record with unknown fields") + }; + let long_record = record_with_unknown(&long_field); + let mut injection_record: serde_json::Value = + serde_json::from_slice(&record_with_unknown(injection_field)).expect("decode injection record"); + injection_record["key_id"] = serde_json::json!("tenant-secret-or-untrusted-record-id"); + let injection_record = serde_json::to_vec(&injection_record).expect("encode injection record"); let logs = crate::test_support::CapturedLogs::default(); let subscriber = tracing_subscriber::fmt() .with_ansi(false) .with_max_level(tracing::Level::WARN) .with_writer(logs.clone()) .finish(); - let record = fs::read(&key_path).await.expect("read key record"); - let stored: StoredMasterKey = tracing::subscriber::with_default(subscriber, || { - let stored = serde_json::from_slice(&record).expect("unknown fields must remain forward-compatible"); - let _: StoredMasterKey = - serde_json::from_slice(&record).expect("repeated unknown fields must remain forward-compatible"); + let dispatch = tracing::Dispatch::new(subscriber); + let parse = |record: &[u8]| { + let recorder = DebuggingRecorder::new(); + let stored = metrics::with_local_recorder(&recorder, || { + tracing::dispatcher::with_default(&dispatch, || { + serde_json::from_slice(record).expect("unknown fields must remain forward-compatible") + }) + }); + assert_eq!(unknown_field_metric(&recorder, "local-key-record"), 2); stored - }); + }; + let stored: StoredMasterKey = parse(&long_record); + let _: StoredMasterKey = parse(&long_record); + let _: StoredMasterKey = parse(&injection_record); + let _: StoredMasterKey = parse(&injection_record); assert_eq!(stored.key_id, "unknown-field-key"); let output = logs.output(); assert!(output.contains("WARN")); - assert_eq!(output.matches("Local KMS key record contains unknown fields").count(), 1); - assert!(output.contains("alpha_extension")); + assert_eq!(output.matches("Local KMS key record contains unknown fields").count(), 3); + assert!(output.contains(&long_prefix)); + assert!(!output.contains(&long_field)); + assert!(output.contains("field_name_truncated=true")); + assert!(output.contains(r#"\n\u{1b}[31m"#)); assert!(!output.contains("zeta_extension")); assert!(output.contains("field_count=2")); - assert!(output.contains("key_id=unknown-field-key")); + for observed_records in [1, 2, 4] { + assert!(output.contains(&format!("observed_records={observed_records}"))); + } + assert!(!output.contains("observed_records=3")); + assert!(!output.contains("tenant-secret-or-untrusted-record-id")); assert!(!output.contains(UNKNOWN_FIELD_VALUE)); assert!(!output.contains("another value must not be logged")); + + let streamed: StoredMasterKey = deserialize_with_ignored_only_unknown(record, "stream_only_extension") + .expect("unknown values must be consumed through deserialize_ignored_any"); + assert_eq!(streamed.key_id, "unknown-field-key"); } #[tokio::test] @@ -3269,8 +3536,9 @@ mod tests { serde_json::from_slice(&fs::read(&key_path).await.expect("read key file")).expect("decode record"); drop(client); + const UNKNOWN_MARKER_VALUE: &str = "secret-marker-value-must-not-leak"; let mut newer_build_record = pristine.clone(); - newer_build_record["at_rest_protection"] = serde_json::json!("post-quantum-v2"); + newer_build_record["at_rest_protection"] = serde_json::json!(UNKNOWN_MARKER_VALUE); let newer_build_record = serde_json::to_vec_pretty(&newer_build_record).expect("encode record"); let mut future_format_record = pristine.clone(); future_format_record["format_version"] = serde_json::json!(99); @@ -3281,7 +3549,7 @@ mod tests { }; for (name, content, expected_error) in [ - ("record from a newer build", newer_build_record, None), + ("record from a newer build", newer_build_record, Some(UNKNOWN_STORED_KEY_PROTECTION)), ( "record with a future format version", future_format_record, @@ -3304,6 +3572,10 @@ mod tests { "{name}: expected a salt-specific configuration error, got {error:?}" ); assert!(error.to_string().contains("salt"), "{name}: error must point at the salt: {error}"); + assert!( + !error.to_string().contains(UNKNOWN_MARKER_VALUE), + "{name}: raw marker values must stay redacted" + ); if let Some(expected_error) = expected_error { assert!( error.to_string().contains(expected_error), @@ -3335,7 +3607,9 @@ mod tests { let key_path = client.master_key_path("beta").expect("valid key id"); let mut record: serde_json::Value = serde_json::from_slice(&fs::read(&key_path).await.expect("read record")).expect("decode record"); - record["at_rest_protection"] = serde_json::json!("post-quantum-v2"); + record["at_rest_protection"] = serde_json::json!({ + "future_mode": ["secret-marker-value-must-not-leak"] + }); fs::write(&key_path, serde_json::to_vec_pretty(&record).expect("encode record")) .await .expect("write record"); @@ -3346,9 +3620,10 @@ mod tests { .expect_err("a listing must not quietly omit a key it cannot read"); assert!( matches!(&error, KmsError::UnsupportedFormatVersion { key_id, version } - if key_id == "beta" && version == "post-quantum-v2"), + if key_id == "beta" && version == UNKNOWN_STORED_KEY_PROTECTION), "got {error:?}" ); + assert!(!error.to_string().contains("secret-marker-value-must-not-leak")); } #[tokio::test] diff --git a/crates/kms/src/backup/local_export.rs b/crates/kms/src/backup/local_export.rs index ef95214f5..e26f7cd35 100644 --- a/crates/kms/src/backup/local_export.rs +++ b/crates/kms/src/backup/local_export.rs @@ -49,8 +49,8 @@ //! manifest, which decodes as an incomplete bundle and can never be restored. use crate::backends::local::{ - LocalKmsClient, STORED_MASTER_KEY_FORMAT_VERSION, StoredKeyProtection, stored_master_key_format_version, - unknown_protection_marker, + LocalKmsClient, STORED_MASTER_KEY_FORMAT_VERSION, StoredKeyProtection, UNKNOWN_STORED_KEY_PROTECTION, + has_unknown_protection_marker, stored_master_key_format_version, }; use crate::backup::capability::{AtRestProtection, BackupBackendKind, BackupResponsibility}; use crate::backup::error::BackupError; @@ -208,8 +208,7 @@ pub async fn export_local_backup( request: &LocalBackupExportRequest, ) -> Result { request.validate()?; - prepare_destination(&request.destination).await?; - + validate_destination(&request.destination).await?; let snapshot = collect_snapshot(client).await?; if snapshot.records.is_empty() { return Err(KmsError::invalid_operation( @@ -236,6 +235,7 @@ pub async fn export_local_backup( None => None, }; + prepare_destination(&request.destination).await?; let manifest = build_and_write_bundle(kek, request, &snapshot, master_key_verifier).await?; Ok(manifest) } @@ -389,11 +389,11 @@ async fn collect_snapshot(client: &LocalKmsClient) -> Result if format_version > STORED_MASTER_KEY_FORMAT_VERSION { return Err(KmsError::unsupported_format_version(&stem, format_version.to_string())); } - let unknown_marker = unknown_protection_marker(&raw).map_err(|error| { + let has_unknown_marker = has_unknown_protection_marker(&raw).map_err(|error| { KmsError::material_corrupt(&stem, format!("stored key record is not a readable JSON object: {error}")) })?; - if let Some(version) = unknown_marker { - return Err(KmsError::unsupported_format_version(&stem, version)); + if has_unknown_marker { + return Err(KmsError::unsupported_format_version(&stem, UNKNOWN_STORED_KEY_PROTECTION)); } let probe: StoredRecordProbe = serde_json::from_slice(&raw) .map_err(|error| KmsError::material_corrupt(&stem, format!("stored key record does not deserialize: {error}")))?; @@ -619,7 +619,7 @@ fn local_kdf_descriptor(snapshot: &CollectedSnapshot, master_key_verifier: Optio } } -async fn prepare_destination(destination: &Path) -> Result<()> { +async fn validate_destination(destination: &Path) -> Result<()> { if fs::try_exists(destination).await? { let mut entries = fs::read_dir(destination) .await @@ -630,6 +630,11 @@ async fn prepare_destination(destination: &Path) -> Result<()> { )); } } + Ok(()) +} + +async fn prepare_destination(destination: &Path) -> Result<()> { + validate_destination(destination).await?; fs::create_dir_all(destination.join(KEYS_DIR)).await?; Ok(()) } @@ -1146,7 +1151,7 @@ mod tests { let record_path = client.key_directory().join("alpha.key"); let mut record: serde_json::Value = serde_json::from_slice(&std::fs::read(&record_path).expect("read record")).expect("decode record"); - record["at_rest_protection"] = serde_json::json!("post-quantum-v2"); + record["at_rest_protection"] = serde_json::json!("secret-marker-value-must-not-leak"); std::fs::write(&record_path, serde_json::to_vec_pretty(&record).expect("encode record")).expect("write record"); let bundle = TempDir::new().expect("bundle dir"); @@ -1155,9 +1160,10 @@ mod tests { .expect_err("an uninterpretable record must abort the export"); assert!( matches!(&error, KmsError::UnsupportedFormatVersion { key_id, version } - if key_id == "alpha" && version == "post-quantum-v2"), + if key_id == "alpha" && version == UNKNOWN_STORED_KEY_PROTECTION), "got {error:?}" ); + assert!(!error.to_string().contains("secret-marker-value-must-not-leak")); } #[tokio::test] @@ -1172,7 +1178,8 @@ mod tests { std::fs::write(&record_path, serde_json::to_vec_pretty(&record).expect("encode record")).expect("write record"); let bundle = TempDir::new().expect("bundle dir"); - let error = export_local_backup(&client, &test_kek(), &export_request(bundle.path().join("bundle"))) + let destination = bundle.path().join("bundle"); + let error = export_local_backup(&client, &test_kek(), &export_request(destination.clone())) .await .expect_err("a newer record format must abort the export"); assert!( @@ -1180,5 +1187,17 @@ mod tests { if key_id == "alpha" && version == "99"), "got {error:?}" ); + assert!( + !destination.exists(), + "format validation must finish before the export creates its destination" + ); + + record["format_version"] = serde_json::json!(STORED_MASTER_KEY_FORMAT_VERSION); + std::fs::write(&record_path, serde_json::to_vec_pretty(&record).expect("encode supported record")) + .expect("write supported record"); + export_local_backup(&client, &test_kek(), &export_request(destination.clone())) + .await + .expect("the same destination must remain usable after validation fails"); + assert!(destination.join(LOCAL_BUNDLE_MANIFEST_FILE).exists()); } } diff --git a/crates/kms/src/backup/local_restore.rs b/crates/kms/src/backup/local_restore.rs index a1d69d944..3fe2612f2 100644 --- a/crates/kms/src/backup/local_restore.rs +++ b/crates/kms/src/backup/local_restore.rs @@ -54,8 +54,8 @@ use crate::backends::local::{ LOCAL_KMS_MASTER_KEY_SALT_FILE, LOCAL_KMS_MASTER_KEY_SALT_LEN, LOCAL_RESTORE_COMMIT_MARKER_FILE, LocalKmsClient, - STORED_MASTER_KEY_FORMAT_VERSION, StoredKeyProtection, durable_file, is_orphan_commit_temp_name, - stored_master_key_format_version, unknown_protection_marker, validate_key_id, + STORED_MASTER_KEY_FORMAT_VERSION, StoredKeyProtection, UNKNOWN_STORED_KEY_PROTECTION, durable_file, + has_unknown_protection_marker, is_orphan_commit_temp_name, stored_master_key_format_version, validate_key_id, }; use crate::backup::capability::AtRestProtection; use crate::backup::dry_run::{ @@ -618,10 +618,14 @@ fn decode_key_record( } .into()); } - let unknown_marker = unknown_protection_marker(&plaintext) + let has_unknown_marker = has_unknown_protection_marker(&plaintext) .map_err(|error| BackupError::corrupted(format!("bundled key record '{stem}' is not a readable JSON object: {error}")))?; - if let Some(version) = unknown_marker { - return Err(BackupError::UnsupportedRecordVersion { key_id: stem, version }.into()); + if has_unknown_marker { + return Err(BackupError::UnsupportedRecordVersion { + key_id: stem, + version: UNKNOWN_STORED_KEY_PROTECTION.to_owned(), + } + .into()); } let probe: RestoredRecordProbe = serde_json::from_slice(&plaintext) .map_err(|error| BackupError::corrupted(format!("bundled key record '{stem}' does not deserialize: {error}")))?; @@ -2084,7 +2088,7 @@ mod tests { fn bundled_record_from_a_newer_build_is_not_reported_as_corruption() { let record = serde_json::json!({ "key_id": "alpha", - "at_rest_protection": "post-quantum-v2", + "at_rest_protection": "secret-marker-value-must-not-leak", "encrypted_key_material": "AAAAAAAAAAAAAAAAAAAAAA==", "nonce": vec![0u8; 12], }); @@ -2102,9 +2106,10 @@ mod tests { }; assert!( matches!(inner, BackupError::UnsupportedRecordVersion { key_id, version } - if key_id == "alpha" && version == "post-quantum-v2"), + if key_id == "alpha" && version == UNKNOWN_STORED_KEY_PROTECTION), "got {inner:?}" ); + assert!(!error.to_string().contains("secret-marker-value-must-not-leak")); assert_eq!( RestoreBlocker::from(inner).code, RestoreBlockerCode::UnknownFormatVersion, diff --git a/crates/kms/src/encryption/dek.rs b/crates/kms/src/encryption/dek.rs index cfbf42c98..f1e98f755 100644 --- a/crates/kms/src/encryption/dek.rs +++ b/crates/kms/src/encryption/dek.rs @@ -21,12 +21,35 @@ #![allow(dead_code)] // Trait methods may be used by implementations use crate::error::{KmsError, Result}; +use crate::persisted_observability::{BoundedUnknownFieldName, UnknownFieldSummary}; use async_trait::async_trait; use jiff::Zoned; use rand::Rng; -use serde::de::IgnoredAny; +use serde::de::{self, IgnoredAny, MapAccess, Visitor}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; +use std::fmt; +use std::sync::atomic::{AtomicU64, Ordering}; + +impl UnknownFieldSummary { + fn record_for_data_key_envelope(&self) { + let Some((field, field_name_truncated, field_count)) = self.record("data-key-envelope") else { + return; + }; + + static RECORDS_WITH_UNKNOWN_FIELDS: AtomicU64 = AtomicU64::new(0); + let observed_records = RECORDS_WITH_UNKNOWN_FIELDS.fetch_add(1, Ordering::Relaxed).saturating_add(1); + if observed_records.is_power_of_two() { + tracing::warn!( + field = ?field, + field_name_truncated, + field_count, + observed_records, + "KMS data-key envelope contains unknown fields" + ); + } + } +} /// Data key envelope for encrypting/decrypting data keys /// @@ -59,44 +82,132 @@ impl<'de> Deserialize<'de> for DataKeyEnvelope { where D: serde::Deserializer<'de>, { + enum Field { + KeyId, + MasterKeyId, + KeySpec, + EncryptedKey, + Nonce, + EncryptionContext, + CreatedAt, + MasterKeyVersion, + Unknown(BoundedUnknownFieldName), + } + + impl<'de> Deserialize<'de> for Field { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::Deserializer<'de>, + { + struct FieldVisitor; + + impl Visitor<'_> for FieldVisitor { + type Value = Field; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a KMS data-key envelope field name") + } + + fn visit_str(self, value: &str) -> std::result::Result + where + E: de::Error, + { + Ok(match value { + "key_id" => Field::KeyId, + "master_key_id" => Field::MasterKeyId, + "key_spec" => Field::KeySpec, + "encrypted_key" => Field::EncryptedKey, + "nonce" => Field::Nonce, + "encryption_context" => Field::EncryptionContext, + "created_at" => Field::CreatedAt, + "master_key_version" => Field::MasterKeyVersion, + _ => Field::Unknown(BoundedUnknownFieldName::new(value)), + }) + } + } + + deserializer.deserialize_identifier(FieldVisitor) + } + } + #[derive(Deserialize)] - struct Wire { - key_id: String, - master_key_id: String, - key_spec: String, - encrypted_key: Vec, - nonce: Vec, - encryption_context: HashMap, - #[serde(with = "crate::time_serde::zoned")] - created_at: Zoned, - #[serde(default)] - master_key_version: Option, - #[serde(flatten)] - unknown_fields: HashMap, + struct ZonedValue(#[serde(with = "crate::time_serde::zoned")] Zoned); + + struct DataKeyEnvelopeVisitor; + + impl<'de> Visitor<'de> for DataKeyEnvelopeVisitor { + type Value = DataKeyEnvelope; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a KMS data-key envelope") + } + + fn visit_map(self, mut map: A) -> std::result::Result + where + A: MapAccess<'de>, + { + macro_rules! read_field { + ($slot:ident, $name:literal) => {{ + if $slot.is_some() { + return Err(de::Error::duplicate_field($name)); + } + $slot = Some(map.next_value()?); + }}; + } + + let mut key_id = None; + let mut master_key_id = None; + let mut key_spec = None; + let mut encrypted_key = None; + let mut nonce = None; + let mut encryption_context = None; + let mut created_at: Option = None; + let mut master_key_version = None; + let mut unknown_fields = UnknownFieldSummary::default(); + + while let Some(field) = map.next_key()? { + match field { + Field::KeyId => read_field!(key_id, "key_id"), + Field::MasterKeyId => read_field!(master_key_id, "master_key_id"), + Field::KeySpec => read_field!(key_spec, "key_spec"), + Field::EncryptedKey => read_field!(encrypted_key, "encrypted_key"), + Field::Nonce => read_field!(nonce, "nonce"), + Field::EncryptionContext => read_field!(encryption_context, "encryption_context"), + Field::CreatedAt => read_field!(created_at, "created_at"), + Field::MasterKeyVersion => read_field!(master_key_version, "master_key_version"), + Field::Unknown(field) => { + let _: IgnoredAny = map.next_value()?; + unknown_fields.observe(field); + } + } + } + + let envelope = DataKeyEnvelope { + key_id: key_id.ok_or_else(|| de::Error::missing_field("key_id"))?, + master_key_id: master_key_id.ok_or_else(|| de::Error::missing_field("master_key_id"))?, + key_spec: key_spec.ok_or_else(|| de::Error::missing_field("key_spec"))?, + encrypted_key: encrypted_key.ok_or_else(|| de::Error::missing_field("encrypted_key"))?, + nonce: nonce.ok_or_else(|| de::Error::missing_field("nonce"))?, + encryption_context: encryption_context.ok_or_else(|| de::Error::missing_field("encryption_context"))?, + created_at: created_at.ok_or_else(|| de::Error::missing_field("created_at"))?.0, + master_key_version: master_key_version.unwrap_or(None), + }; + unknown_fields.record_for_data_key_envelope(); + Ok(envelope) + } } - let wire = Wire::deserialize(deserializer)?; - if let Some(field) = wire.unknown_fields.keys().min() { - static WARN_UNKNOWN_FIELDS: std::sync::Once = std::sync::Once::new(); - WARN_UNKNOWN_FIELDS.call_once(|| { - tracing::warn!( - field = %field, - field_count = wire.unknown_fields.len(), - "KMS data-key envelope contains unknown fields" - ); - }); - } - - Ok(Self { - key_id: wire.key_id, - master_key_id: wire.master_key_id, - key_spec: wire.key_spec, - encrypted_key: wire.encrypted_key, - nonce: wire.nonce, - encryption_context: wire.encryption_context, - created_at: wire.created_at, - master_key_version: wire.master_key_version, - }) + const FIELDS: &[&str] = &[ + "key_id", + "master_key_id", + "key_spec", + "encrypted_key", + "nonce", + "encryption_context", + "created_at", + "master_key_version", + ]; + deserializer.deserialize_struct("DataKeyEnvelope", FIELDS, DataKeyEnvelopeVisitor) } } @@ -283,6 +394,8 @@ pub fn generate_key_material(algorithm: &str) -> Result> { #[cfg(test)] mod tests { use super::*; + use crate::test_support::{deserialize_with_ignored_only_unknown, unknown_field_metric}; + use metrics_util::debugging::DebuggingRecorder; #[tokio::test] async fn test_aes_dek_crypto_encrypt_decrypt() { @@ -417,40 +530,71 @@ mod tests { #[test] fn test_data_key_envelope_unknown_fields_remain_readable() { const UNKNOWN_FIELD_VALUE: &str = "field value must not be logged"; - let envelope_json = r#"{ + let envelope = serde_json::json!({ "key_id": "test-key-id", "master_key_id": "master-key-id", "key_spec": "AES_256", "encrypted_key": [1, 2, 3, 4], "nonce": [5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16], "encryption_context": {"bucket": "test-bucket"}, - "created_at": "2024-01-01T00:00:00+00:00[UTC]", - "alpha_extension": "field value must not be logged", - "zeta_extension": "another value must not be logged" - }"#; + "created_at": "2024-01-01T00:00:00+00:00[UTC]" + }); + let long_field = format!("{}界", "a".repeat(126)); + let long_prefix = "a".repeat(126); + let injection_field = "b\n\u{1b}[31m"; + let record_with_unknown = |field: &str| { + let mut record = envelope.clone(); + let object = record.as_object_mut().expect("envelope is an object"); + object.insert(field.to_owned(), serde_json::json!(UNKNOWN_FIELD_VALUE)); + object.insert("zeta_extension".to_owned(), serde_json::json!("another value must not be logged")); + serde_json::to_vec(&record).expect("encode envelope with unknown fields") + }; + let long_record = record_with_unknown(&long_field); + let injection_record = record_with_unknown(injection_field); let logs = crate::test_support::CapturedLogs::default(); let subscriber = tracing_subscriber::fmt() .with_ansi(false) .with_max_level(tracing::Level::WARN) .with_writer(logs.clone()) .finish(); - let deserialized: DataKeyEnvelope = tracing::subscriber::with_default(subscriber, || { - let envelope = serde_json::from_str(envelope_json).expect("unknown fields must remain readable"); - let _: DataKeyEnvelope = serde_json::from_str(envelope_json).expect("repeated unknown fields must remain readable"); + let dispatch = tracing::Dispatch::new(subscriber); + let parse = |record: &[u8]| { + let recorder = DebuggingRecorder::new(); + let envelope = metrics::with_local_recorder(&recorder, || { + tracing::dispatcher::with_default(&dispatch, || { + serde_json::from_slice(record).expect("unknown fields must remain readable") + }) + }); + assert_eq!(unknown_field_metric(&recorder, "data-key-envelope"), 2); envelope - }); + }; + let deserialized: DataKeyEnvelope = parse(&long_record); + let _: DataKeyEnvelope = parse(&long_record); + let _: DataKeyEnvelope = parse(&injection_record); + let _: DataKeyEnvelope = parse(&injection_record); assert_eq!(deserialized.key_id, "test-key-id"); assert_eq!(deserialized.master_key_version, None); let output = logs.output(); assert!(output.contains("WARN")); - assert_eq!(output.matches("KMS data-key envelope contains unknown fields").count(), 1); - assert!(output.contains("alpha_extension")); + assert_eq!(output.matches("KMS data-key envelope contains unknown fields").count(), 3); + assert!(output.contains(&long_prefix)); + assert!(!output.contains(&long_field)); + assert!(output.contains("field_name_truncated=true")); + assert!(output.contains(r#"\n\u{1b}[31m"#)); assert!(!output.contains("zeta_extension")); assert!(output.contains("field_count=2")); + for observed_records in [1, 2, 4] { + assert!(output.contains(&format!("observed_records={observed_records}"))); + } + assert!(!output.contains("observed_records=3")); assert!(!output.contains(UNKNOWN_FIELD_VALUE)); assert!(!output.contains("another value must not be logged")); + + let streamed: DataKeyEnvelope = deserialize_with_ignored_only_unknown(envelope, "stream_only_extension") + .expect("unknown values must be consumed through deserialize_ignored_any"); + assert_eq!(streamed.key_id, "test-key-id"); } #[test] diff --git a/crates/kms/src/lib.rs b/crates/kms/src/lib.rs index cfe237a4a..7b2a9812b 100644 --- a/crates/kms/src/lib.rs +++ b/crates/kms/src/lib.rs @@ -75,6 +75,7 @@ mod encryption; mod error; pub mod key_impact; pub mod manager; +mod persisted_observability; mod policy; pub mod probe; pub mod service; @@ -84,6 +85,12 @@ pub mod types; #[cfg(test)] pub(crate) mod test_support { + use crate::persisted_observability::UNKNOWN_FIELDS_METRIC; + use metrics_util::MetricKind; + use metrics_util::debugging::{DebugValue, DebuggingRecorder}; + use serde::Deserializer; + use serde::de::value::MapDeserializer; + use serde::de::{self, DeserializeOwned, IntoDeserializer, Visitor}; use std::io::{self, Write}; use std::sync::{Arc, Mutex}; @@ -119,6 +126,121 @@ pub(crate) mod test_support { .expect("captured logs should be UTF-8") } } + + pub(crate) fn unknown_field_metric(recorder: &DebuggingRecorder, record_kind: &str) -> u64 { + recorder + .snapshotter() + .snapshot() + .into_vec() + .into_iter() + .filter_map(|(composite, _unit, _description, value)| { + let matches = composite.kind() == MetricKind::Counter + && composite.key().name() == UNKNOWN_FIELDS_METRIC + && composite + .key() + .labels() + .any(|label| label.key() == "record_kind" && label.value() == record_kind); + match (matches, value) { + (true, DebugValue::Counter(count)) => Some(count), + _ => None, + } + }) + .sum() + } + + enum IgnoredOnlyValue { + Json(serde_json::Value), + Unknown, + } + + impl<'de> IntoDeserializer<'de, serde_json::Error> for IgnoredOnlyValue { + type Deserializer = Self; + + fn into_deserializer(self) -> Self::Deserializer { + self + } + } + + impl<'de> Deserializer<'de> for IgnoredOnlyValue { + type Error = serde_json::Error; + + fn deserialize_any(self, visitor: V) -> Result + where + V: Visitor<'de>, + { + match self { + Self::Json(value) => value.deserialize_any(visitor), + Self::Unknown => Err(de::Error::custom("unknown value was materialized")), + } + } + + fn deserialize_option(self, visitor: V) -> Result + where + V: Visitor<'de>, + { + match self { + Self::Json(value) => value.deserialize_option(visitor), + Self::Unknown => Err(de::Error::custom("unknown value was materialized")), + } + } + + fn deserialize_newtype_struct(self, name: &'static str, visitor: V) -> Result + where + V: Visitor<'de>, + { + match self { + Self::Json(value) => value.deserialize_newtype_struct(name, visitor), + Self::Unknown => Err(de::Error::custom("unknown value was materialized")), + } + } + + fn deserialize_enum( + self, + name: &'static str, + variants: &'static [&'static str], + visitor: V, + ) -> Result + where + V: Visitor<'de>, + { + match self { + Self::Json(value) => value.deserialize_enum(name, variants, visitor), + Self::Unknown => Err(de::Error::custom("unknown value was materialized")), + } + } + + fn deserialize_ignored_any(self, visitor: V) -> Result + where + V: Visitor<'de>, + { + match self { + Self::Json(value) => value.deserialize_ignored_any(visitor), + Self::Unknown => visitor.visit_unit(), + } + } + + serde::forward_to_deserialize_any! { + bool i8 i16 i32 i64 i128 u8 u16 u32 u64 u128 f32 f64 char str string + bytes byte_buf unit unit_struct seq tuple tuple_struct map struct identifier + } + } + + pub(crate) fn deserialize_with_ignored_only_unknown( + record: serde_json::Value, + unknown_field: &str, + ) -> Result + where + T: DeserializeOwned, + { + let object = record + .as_object() + .ok_or_else(|| de::Error::custom("test record must be an object"))?; + let entries = object + .iter() + .map(|(key, value)| (key.clone(), IgnoredOnlyValue::Json(value.clone()))) + .chain([(unknown_field.to_owned(), IgnoredOnlyValue::Unknown)]); + T::deserialize(MapDeserializer::<_, serde_json::Error>::new(entries)) + } } // Re-export public API diff --git a/crates/kms/src/persisted_observability.rs b/crates/kms/src/persisted_observability.rs new file mode 100644 index 000000000..0b6d746e8 --- /dev/null +++ b/crates/kms/src/persisted_observability.rs @@ -0,0 +1,62 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +const UNKNOWN_FIELD_NAME_MAX_BYTES: usize = 128; +pub(crate) const UNKNOWN_FIELDS_METRIC: &str = "rustfs_kms_persisted_unknown_fields_total"; + +pub(crate) struct BoundedUnknownFieldName { + value: String, + truncated: bool, +} + +impl BoundedUnknownFieldName { + pub(crate) fn new(value: &str) -> Self { + if value.len() <= UNKNOWN_FIELD_NAME_MAX_BYTES { + return Self { + value: value.to_owned(), + truncated: false, + }; + } + + let mut end = UNKNOWN_FIELD_NAME_MAX_BYTES; + while !value.is_char_boundary(end) { + end -= 1; + } + Self { + value: value[..end].to_owned(), + truncated: true, + } + } +} + +#[derive(Default)] +pub(crate) struct UnknownFieldSummary { + count: u64, + first: Option, +} + +impl UnknownFieldSummary { + pub(crate) fn observe(&mut self, field: BoundedUnknownFieldName) { + self.count = self.count.saturating_add(1); + if self.first.is_none() { + self.first = Some(field); + } + } + + pub(crate) fn record(&self, record_kind: &'static str) -> Option<(&str, bool, u64)> { + let field = self.first.as_ref()?; + metrics::counter!(UNKNOWN_FIELDS_METRIC, "record_kind" => record_kind).increment(self.count); + Some((&field.value, field.truncated, self.count)) + } +} diff --git a/docs/operations/kms-backend-security.md b/docs/operations/kms-backend-security.md index 75a565b40..17648a4c4 100644 --- a/docs/operations/kms-backend-security.md +++ b/docs/operations/kms-backend-security.md @@ -118,8 +118,8 @@ This section states only what is true of the current implementation. It is writt Nothing in this list requires a coordinated format cutover. The compatibility is deliberate and is covered by decode tests. -- **DEK envelopes.** `DataKeyEnvelope::master_key_version` is optional and omitted when absent, so envelopes written by non-rotating backends stay byte-identical to the historical seven-field JSON shape. An upgraded node reading a pre-versioning envelope resolves `None` to the key's recorded baseline version, or — for a key that was never rotated, and so has no baseline — to the current version, which is exactly the pre-versioning behavior. -- **Local key records.** Each `.key` record carries `format_version: 1`; records written before that field existed default to version 1 when read. A reader accepts a record whose version is at most the version it understands, and rejects a newer version with `UnsupportedFormatVersion` before it attempts to decrypt key material. Unknown fields remain accepted for rollback compatibility, but their names are emitted at `warn` level without values. +- **DEK envelopes.** `DataKeyEnvelope::master_key_version` is optional and omitted when absent, so envelopes written by non-rotating backends stay byte-identical to the historical seven-field JSON shape. An upgraded node reading a pre-versioning envelope resolves `None` to the key's recorded baseline version, or — for a key that was never rotated, and so has no baseline — to the current version, which is exactly the pre-versioning behavior. Unknown values are skipped while parsing; a bounded field-name sample is emitted at a progressively rate-limited `warn` level, and `rustfs_kms_persisted_unknown_fields_total{record_kind="data-key-envelope"}` counts every observed field. +- **Local key records.** Each `.key` record carries `format_version: 1`; records written before that field existed default to version 1 when read, and the pre-version reader ignores the added v1 marker. A reader accepts a record whose version is at most the version it understands, and rejects a newer version with `UnsupportedFormatVersion` before it attempts to decrypt key material. Unknown fields remain accepted for rollback compatibility. Their values are ignored while parsing; a bounded field-name sample is emitted at a progressively rate-limited `warn` level, and `rustfs_kms_persisted_unknown_fields_total{record_kind="local-key-record"}` counts every observed field. Once a future version greater than 1 has written a key record, do not roll back to a build that predates this marker: such a build cannot reject that future version before interpreting the rest of the record. - **KV2 key records.** `baseline_version` is read with a serde default, so records written by older builds deserialize unchanged, and `None` correctly means "never rotated". - **Transit metadata records.** Metadata persisted in KV v2 by either build decodes on the other.