From ea7f1609e5cb58320bd878ad9791ff3d35a79af2 Mon Sep 17 00:00:00 2001 From: James Sadler Date: Fri, 6 Nov 2026 22:17:18 +1100 Subject: [PATCH] refactor(guest,auth): name each key vendor backend in one place The five-way vendor switch was written again in each place that used it: the guest's GuestVendor Debug, VendorSettings::vendor, a dispatch macro with one arm per vendor and method, and run_vendor in the auth binding. A new vendor meant an edit to each one. In the guest, one macro call lists the vendor clients once. The macro makes the GuestVendor enum and forwards every KeyVendorBackend method to the configured client. Debug reads the name from vendor(), and VendorSettings::vendor is gone. The config keys and the constructor of each vendor stay where they are, because they are different data for each vendor, not the same switch. The dispatch did not forward same_key, so the guest used the default text compare for Google Cloud KMS and Azure, which ignore the key version. The guest does not rewrap a record, so no call path changes, but the macro now forwards it and a test shows it. In the auth binding, each row of BACKENDS now also holds how to build the vendor client from the environment. run_vendor reads the row, so the table is the one list of backends in the module. --- .changeset/keyset-vendor-table.md | 5 + languages/golang/encrypt/guest/src/config.rs | 37 +-- languages/golang/encrypt/guest/src/vendor.rs | 211 ++++++++++-------- .../typescript/packages/auth/src/keyset.rs | 154 +++++++++---- 4 files changed, 257 insertions(+), 150 deletions(-) create mode 100644 .changeset/keyset-vendor-table.md diff --git a/.changeset/keyset-vendor-table.md b/.changeset/keyset-vendor-table.md new file mode 100644 index 000000000..5d3a29334 --- /dev/null +++ b/.changeset/keyset-vendor-table.md @@ -0,0 +1,5 @@ +--- +'@cipherstash/auth': patch +--- + +Internal change to `keysetCreate` and `keysetRewrap`. One table now holds each key vendor backend: its short name, its vendor and how to build its client from the environment. There is no change to what they do. diff --git a/languages/golang/encrypt/guest/src/config.rs b/languages/golang/encrypt/guest/src/config.rs index e73e3a82e..d910e7e3f 100644 --- a/languages/golang/encrypt/guest/src/config.rs +++ b/languages/golang/encrypt/guest/src/config.rs @@ -662,16 +662,23 @@ mod tests { } )); - for (name, expected) in [ - ("gcp-kms", KeyVendor::Gcp), - ("azure-key-vault", KeyVendor::Azure), - ] { - let config = - vendor_config(vec![("key_vendor", s(name)), ("keyset_records", records())]) - .unwrap() - .unwrap(); - assert_eq!(config.settings.vendor(), expected); - } + let gcp = vendor_config(vec![ + ("key_vendor", s("gcp-kms")), + ("keyset_records", records()), + ]) + .unwrap() + .unwrap(); + assert!(matches!( + gcp.settings, + VendorSettings::Gcp { endpoint: None } + )); + let azure = vendor_config(vec![ + ("key_vendor", s("azure-key-vault")), + ("keyset_records", records()), + ]) + .unwrap() + .unwrap(); + assert!(matches!(azure.settings, VendorSettings::Azure)); let static_key = vendor_config(vec![ ("key_vendor", s("static")), @@ -684,7 +691,7 @@ mod tests { ]) .unwrap() .unwrap(); - assert_eq!(static_key.settings.vendor(), KeyVendor::Static); + assert!(matches!(static_key.settings, VendorSettings::Static { .. })); // The static key never prints. assert!(!format!("{static_key:?}").contains('7')); } @@ -774,10 +781,10 @@ mod tests { entries.push(("key_vendor".into(), s("gcp-kms"))); entries.push(("keyset_records".into(), records())); let cfg = parse_config(FfiValue::Object(entries)).expect("config parses"); - assert_eq!( - cfg.vendor.map(|v| v.settings.vendor()), - Some(KeyVendor::Gcp) - ); + assert!(matches!( + cfg.vendor.map(|v| v.settings), + Some(VendorSettings::Gcp { .. }) + )); } #[test] diff --git a/languages/golang/encrypt/guest/src/vendor.rs b/languages/golang/encrypt/guest/src/vendor.rs index a61ff8b9c..f60ba694b 100644 --- a/languages/golang/encrypt/guest/src/vendor.rs +++ b/languages/golang/encrypt/guest/src/vendor.rs @@ -76,25 +76,11 @@ pub enum VendorSettings { Static { key_ref: String, key: KeyBytes }, } -impl VendorSettings { - /// The backend these settings build. - pub fn vendor(&self) -> KeyVendor { - match self { - Self::Aws { .. } => KeyVendor::Aws, - Self::Vault { .. } => KeyVendor::Vault, - Self::Gcp { .. } => KeyVendor::Gcp, - Self::Azure => KeyVendor::Azure, - Self::Static { .. } => KeyVendor::Static, - } - } -} - impl std::fmt::Debug for VendorSettings { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // The static key never prints. - f.debug_tuple("VendorSettings") - .field(&self.vendor().name()) - .finish() + // No setting prints: the static key is one of them. The vendor + // prints once it is built ([`GuestVendor`]'s `Debug`). + f.debug_struct("VendorSettings").finish_non_exhaustive() } } @@ -362,95 +348,109 @@ impl stack_kms_azure::TokenProvider for HostCredentials } } -/// The guest's one key vendor backend: whichever the host configured. -/// -/// One enum, not a type parameter, so that the instance's cipher has one -/// type whatever the host chose at run time. -pub enum GuestVendor { - Aws(stack_kms_aws::AwsKms>), - Vault(stack_kms_vault::VaultTransit>), - Gcp(stack_kms_gcp::GcpKms>), - Azure(stack_kms_azure::AzureKeyVault>), - Static(StaticKeyVendor), -} +/// Declare [`GuestVendor`] from one list of its vendor clients, and forward +/// every [`KeyVendorBackend`] method to the configured one. The list is the +/// only place the variants are named, so a new vendor client is one line +/// here (plus its config keys in [`crate::config`] and its constructor in +/// [`build`]). +macro_rules! guest_vendors { + ($($variant:ident($client:ty),)+) => { + /// The guest's one key vendor backend: whichever the host configured. + /// + /// One enum, not a type parameter, so that the instance's cipher has + /// one type whatever the host chose at run time. + pub enum GuestVendor { + $($variant($client),)+ + } -impl std::fmt::Debug for GuestVendor { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let name = match self { - Self::Aws(_) => KeyVendor::Aws.name(), - Self::Vault(_) => KeyVendor::Vault.name(), - Self::Gcp(_) => KeyVendor::Gcp.name(), - Self::Azure(_) => KeyVendor::Azure.name(), - Self::Static(_) => KeyVendor::Static.name(), - }; - f.debug_tuple("GuestVendor").field(&name).finish() - } -} + impl KeyVendorBackend for GuestVendor + where + T: VendorTransport + Send + Sync, + C: CredentialImport, + { + fn vendor(&self) -> KeyVendor { + match self { $(Self::$variant(b) => b.vendor(),)+ } + } -/// Dispatch one `KeyVendorBackend` method to the configured client. -macro_rules! dispatch { - ($self:expr, $backend:ident => $call:expr) => { - match $self { - GuestVendor::Aws($backend) => $call, - GuestVendor::Vault($backend) => $call, - GuestVendor::Gcp($backend) => $call, - GuestVendor::Azure($backend) => $call, - GuestVendor::Static($backend) => $call, - } - }; -} + fn supports_mode(&self, mode: KeyMode) -> bool { + match self { $(Self::$variant(b) => b.supports_mode(mode),)+ } + } -impl KeyVendorBackend for GuestVendor -where - T: VendorTransport + Send + Sync, - C: CredentialImport, -{ - fn vendor(&self) -> KeyVendor { - dispatch!(self, b => b.vendor()) - } + fn same_key(&self, a: &str, b: &str) -> bool { + match self { $(Self::$variant(v) => v.same_key(a, b),)+ } + } - fn supports_mode(&self, mode: KeyMode) -> bool { - dispatch!(self, b => b.supports_mode(mode)) - } + async fn wrap( + &self, + key_ref: &str, + key: &KeyBytes, + aad: &[u8], + ) -> Result, Error> { + match self { $(Self::$variant(b) => b.wrap(key_ref, key, aad).await,)+ } + } - async fn wrap(&self, key_ref: &str, key: &KeyBytes, aad: &[u8]) -> Result, Error> { - dispatch!(self, b => b.wrap(key_ref, key, aad).await) - } + async fn unwrap( + &self, + key_ref: &str, + wrapped: &[u8], + aad: &[u8], + ) -> Result { + match self { $(Self::$variant(b) => b.unwrap(key_ref, wrapped, aad).await,)+ } + } - async fn unwrap(&self, key_ref: &str, wrapped: &[u8], aad: &[u8]) -> Result { - dispatch!(self, b => b.unwrap(key_ref, wrapped, aad).await) - } + async fn generate_wrapped( + &self, + key_ref: &str, + aad: &[u8], + ) -> Result<(KeyBytes, Vec), Error> { + match self { $(Self::$variant(b) => b.generate_wrapped(key_ref, aad).await,)+ } + } - async fn generate_wrapped( - &self, - key_ref: &str, - aad: &[u8], - ) -> Result<(KeyBytes, Vec), Error> { - dispatch!(self, b => b.generate_wrapped(key_ref, aad).await) - } + fn batch_limit(&self) -> Option { + match self { $(Self::$variant(b) => b.batch_limit(),)+ } + } - fn batch_limit(&self) -> Option { - dispatch!(self, b => b.batch_limit()) - } + async fn wrap_batch( + &self, + key_ref: &str, + items: &[WrapItem<'_>], + ) -> Result>, Error> { + match self { $(Self::$variant(b) => b.wrap_batch(key_ref, items).await,)+ } + } - async fn wrap_batch( - &self, - key_ref: &str, - items: &[WrapItem<'_>], - ) -> Result>, Error> { - dispatch!(self, b => b.wrap_batch(key_ref, items).await) - } + async fn unwrap_batch( + &self, + key_ref: &str, + items: &[UnwrapItem<'_>], + ) -> Result, Error> { + match self { $(Self::$variant(b) => b.unwrap_batch(key_ref, items).await,)+ } + } - async fn unwrap_batch( - &self, - key_ref: &str, - items: &[UnwrapItem<'_>], - ) -> Result, Error> { - dispatch!(self, b => b.unwrap_batch(key_ref, items).await) - } + async fn sleep(&self, duration: Duration) { + match self { $(Self::$variant(b) => b.sleep(duration).await,)+ } + } + } + }; +} + +guest_vendors! { + Aws(stack_kms_aws::AwsKms>), + Vault(stack_kms_vault::VaultTransit>), + Gcp(stack_kms_gcp::GcpKms>), + Azure(stack_kms_azure::AzureKeyVault>), + Static(StaticKeyVendor), +} - async fn sleep(&self, duration: Duration) { - dispatch!(self, b => b.sleep(duration).await) +/// Prints the vendor's name only: a client holds credentials or a key. +impl std::fmt::Debug for GuestVendor +where + T: VendorTransport + Send + Sync, + C: CredentialImport, +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_tuple("GuestVendor") + .field(&self.vendor().name()) + .finish() } } @@ -846,6 +846,27 @@ mod tests { ); } + /// Every method reaches the vendor client, `same_key` included: Google + /// Cloud KMS ignores the key version, where the trait's default would + /// compare the text. + #[test] + fn the_guest_vendor_forwards_to_its_client() { + let config = VendorConfig { + settings: VendorSettings::Gcp { endpoint: None }, + records: Vec::new(), + default_keyset: None, + root_key_ttl: None, + }; + let transport = Recording::default(); + let source = build(config, &transport, FakeImport::unused()).unwrap(); + let backend = source.backend(); + assert_eq!(backend.vendor(), KeyVendor::Gcp); + assert_eq!(format!("{backend:?}"), "GuestVendor(\"gcp-kms\")"); + let key = "projects/p/locations/l/keyRings/r/cryptoKeys/k"; + assert!(backend.same_key(&format!("{key}/cryptoKeyVersions/1"), key)); + assert!(!backend.same_key(key, "projects/p/locations/l/keyRings/r/cryptoKeys/other")); + } + #[test] fn a_bad_setting_names_its_config_key() { let config = VendorConfig { diff --git a/languages/typescript/packages/auth/src/keyset.rs b/languages/typescript/packages/auth/src/keyset.rs index bfc8983e3..8991c8ce1 100644 --- a/languages/typescript/packages/auth/src/keyset.rs +++ b/languages/typescript/packages/auth/src/keyset.rs @@ -22,6 +22,8 @@ use stack_kms_vendor::{ HttpVendorTransport, HttpVendorTransportOptions, KeyBytes, KeyVendorBackend, KeysetRecord, RecordError, StaticKeyVendor, }; +use std::future::Future; +use std::pin::Pin; use vitaminc::protected::{Controlled, Protected}; use vitaminc::random::{Generatable, SafeRand}; use vitaminc_protected::OpaqueDebug; @@ -236,22 +238,63 @@ fn vendor_config(vendor: KeyVendor, error: impl std::error::Error + 'static) -> /// them, each with its short name (the `--backend` value). The output /// names a backend with [`KeyVendor::name`] (`aws-kms`, …), and the input /// takes that name too, so a value the output gives is a valid input. -/// This table is the only list of the short names. -pub(crate) const BACKENDS: [(&str, KeyVendor); 5] = [ - ("aws", KeyVendor::Aws), - ("gcp", KeyVendor::Gcp), - ("azure", KeyVendor::Azure), - ("vault", KeyVendor::Vault), - ("static", KeyVendor::Static), +/// This table is the only list of the backends in this module: adding a +/// vendor is one row here. +pub(crate) const BACKENDS: [Backend; 5] = [ + Backend { + short: "aws", + vendor: KeyVendor::Aws, + run: Some(run_aws), + }, + Backend { + short: "gcp", + vendor: KeyVendor::Gcp, + run: Some(run_gcp), + }, + Backend { + short: "azure", + vendor: KeyVendor::Azure, + run: Some(run_azure), + }, + Backend { + short: "vault", + vendor: KeyVendor::Vault, + run: Some(run_vault), + }, + Backend { + short: "static", + vendor: KeyVendor::Static, + // The static key is not in the environment: `create` and `rewrap` + // read or generate it, and build the backend themselves. + run: None, + }, ]; +/// One row of [`BACKENDS`]. +pub(crate) struct Backend { + /// The short `--backend` name. + pub(crate) short: &'static str, + /// The vendor. + pub(crate) vendor: KeyVendor, + /// Build the vendor's client from the environment and run an operation + /// on it. `None` for the static key backend. + run: Option, +} + +/// A [`Backend::run`]: one vendor call (or two, for a rewrap) through a +/// client built from the environment. +type RunVendor = fn(Operation) -> Pin + Send>>; + +/// What a [`RunVendor`] returns. +type VendorResult = Result; + /// The backend that `text` names: a short name of [`BACKENDS`] or the /// [`KeyVendor::name`] of one of its vendors. fn parse_backend(text: &str) -> Result { BACKENDS .iter() - .find(|(short, vendor)| *short == text || vendor.name() == text) - .map(|(_, vendor)| *vendor) + .find(|backend| backend.short == text || backend.vendor.name() == text) + .map(|backend| backend.vendor) .ok_or_else(|| { KeysetError::Options(format!( "Unknown key backend {text:?}. {}", @@ -262,10 +305,10 @@ fn parse_backend(text: &str) -> Result { /// The accepted backend names, for an error message, from [`BACKENDS`]. fn backend_choices() -> String { - let short: Vec<&str> = BACKENDS.iter().map(|(short, _)| *short).collect(); + let short: Vec<&str> = BACKENDS.iter().map(|backend| backend.short).collect(); let full: Vec<&str> = BACKENDS .iter() - .map(|(_, vendor)| vendor.name()) + .map(|backend| backend.vendor.name()) .filter(|name| !short.contains(name)) .collect(); format!( @@ -493,30 +536,48 @@ fn result(record: &KeysetRecord, generated_static_key: Option) -> Keyset } /// Build the vendor's client from the environment and run `operation` on it. -async fn run_vendor(vendor: KeyVendor, operation: Operation) -> Result { - match vendor { - KeyVendor::Aws => { - let kms = stack_kms_aws::DefaultAwsKms::from_env() - .map_err(|error| vendor_config(vendor, error))?; - operation.run(&kms).await - } - KeyVendor::Vault => { - let vault = stack_kms_vault::DefaultVaultTransit::from_env() - .map_err(|error| vendor_config(vendor, error))?; - operation.run(&vault).await - } - KeyVendor::Azure => { - let azure = stack_kms_azure::DefaultAzureKeyVault::from_env() - .map_err(|error| vendor_config(vendor, error))?; - operation.run(&azure).await - } - KeyVendor::Gcp => run_gcp(operation).await, - // The callers handle the static key backend before this point, and - // `KeyVendor` is non-exhaustive. - _ => Err(KeysetError::Options(format!( - "stash keyset cannot reach the {vendor} key backend." - ))), - } +/// The vendor's row of [`BACKENDS`] says how. +async fn run_vendor(vendor: KeyVendor, operation: Operation) -> VendorResult { + // The callers handle the static key backend before this point, and + // `KeyVendor` is non-exhaustive. + let run = BACKENDS + .iter() + .find(|backend| backend.vendor == vendor) + .and_then(|backend| backend.run) + .ok_or_else(|| { + KeysetError::Options(format!( + "stash keyset cannot reach the {vendor} key backend." + )) + })?; + run(operation).await +} + +fn run_aws(operation: Operation) -> Pin + Send>> { + Box::pin(async move { + let kms = stack_kms_aws::DefaultAwsKms::from_env() + .map_err(|error| vendor_config(KeyVendor::Aws, error))?; + operation.run(&kms).await + }) +} + +fn run_vault(operation: Operation) -> Pin + Send>> { + Box::pin(async move { + let vault = stack_kms_vault::DefaultVaultTransit::from_env() + .map_err(|error| vendor_config(KeyVendor::Vault, error))?; + operation.run(&vault).await + }) +} + +fn run_azure(operation: Operation) -> Pin + Send>> { + Box::pin(async move { + let azure = stack_kms_azure::DefaultAzureKeyVault::from_env() + .map_err(|error| vendor_config(KeyVendor::Azure, error))?; + operation.run(&azure).await + }) +} + +fn run_gcp(operation: Operation) -> Pin + Send>> { + Box::pin(gcp(operation)) } /// Google Cloud KMS, with the credentials this binding supports. @@ -527,7 +588,7 @@ async fn run_vendor(vendor: KeyVendor, operation: Operation) -> Result Result { +async fn gcp(operation: Operation) -> VendorResult { use stack_kms_gcp::{ AccessToken, CachedToken, GcpKms, GcpKmsConfig, MetadataServerLogin, ACCESS_TOKEN_VARIABLE, }; @@ -888,10 +949,23 @@ mod tests { #[test] fn the_backend_names_are_the_cli_names() { - let names: Vec<_> = BACKENDS.iter().map(|(name, _)| *name).collect(); + let names: Vec<_> = BACKENDS.iter().map(|backend| backend.short).collect(); assert_eq!(names, ["aws", "gcp", "azure", "vault", "static"]); - for (name, vendor) in BACKENDS { - assert_eq!(parse_backend(name).unwrap(), vendor); + for backend in BACKENDS { + assert_eq!(parse_backend(backend.short).unwrap(), backend.vendor); + } + } + + /// Every vendor but the static key backend runs from the environment. + #[test] + fn every_vendor_client_backend_has_a_runner() { + for backend in BACKENDS { + assert_eq!( + backend.run.is_some(), + backend.vendor != KeyVendor::Static, + "{}", + backend.vendor + ); } } @@ -901,7 +975,7 @@ mod tests { fn the_backend_input_takes_the_output_names() { for vendor in KeyVendor::ALL { assert_eq!(parse_backend(vendor.name()).unwrap(), vendor); - assert!(BACKENDS.iter().any(|(_, v)| *v == vendor), "{vendor}"); + assert!(BACKENDS.iter().any(|b| b.vendor == vendor), "{vendor}"); } let Err(KeysetError::Options(message)) = parse_backend("zerokms") else { panic!("zerokms is not a key vendor backend");