From 8a4bb97377d401a517038f649ed7005c4805d4f8 Mon Sep 17 00:00:00 2001 From: atlas Date: Tue, 6 Oct 2026 10:13:34 +0200 Subject: [PATCH] swarm-nats-auth: read the callout issuer and user seeds from the store The responder loads the issuer and callout-user seeds from the secret store under its own role, beside the xkey, and no longer takes --issuer-seed-file / --user-seed-file. `ensure-xkey` becomes `ensure-callout-keys`: it ensures the xkey as before, and for each nkey creates the seed create-only when the store holds none, from --import-{issuer,user}-seed-file when that file is present, so a key kept on disk keeps its public half. Once a read of the store returns a seed, the import file is removed; when that seed differs from the file the store's is kept and a warning names the file. The public halves are published create-only at swarm/public/swarm-nats/callout-{issuer,user}. The KeyStore trait and its test double move to `keystore` so both key modules share them. --- swarm-nats-auth/README.md | 11 +- swarm-nats-auth/src/keystore.rs | 114 ++++++++++ swarm-nats-auth/src/main.rs | 175 +++++++++------ swarm-nats-auth/src/nkey.rs | 365 ++++++++++++++++++++++++++++++++ swarm-nats-auth/src/xkey.rs | 119 +---------- 5 files changed, 610 insertions(+), 174 deletions(-) create mode 100644 swarm-nats-auth/src/keystore.rs create mode 100644 swarm-nats-auth/src/nkey.rs diff --git a/swarm-nats-auth/README.md b/swarm-nats-auth/README.md index b4132638..5c40ee83 100644 --- a/swarm-nats-auth/README.md +++ b/swarm-nats-auth/README.md @@ -41,10 +41,13 @@ with the xkey's seed and seals its reply to that header's key. The responder logs a warning for a request with no header, or one that doesn't open, and sends no reply: the server it serves always seals, so no server waits on one. -The seed is in the swarm secret store, and the responder reads it before it -connects to the queue: a responder without it doesn't start. The -`ensure-xkey` verb creates the pair when the store holds none. It runs from -its own unit before the server, because the server needs the public half to +The seed is in the swarm secret store, and so are the seeds of the issuer key +the responder signs with and the user key it connects as. The responder reads +all three before it connects to the queue: a responder missing one doesn't +start. The `ensure-callout-keys` verb creates each pair when the store holds +none, taking an issuer or user seed from the import file it is given when that +file exists, and removes the file once the store holds a seed. It runs from +its own unit before the server, because the server needs the public halves to start and the responder needs the server. `docs/swarm/secrets.md` has the paths and the boot order. diff --git a/swarm-nats-auth/src/keystore.rs b/swarm-nats-auth/src/keystore.rs new file mode 100644 index 00000000..9e621ccb --- /dev/null +++ b/swarm-nats-auth/src/keystore.rs @@ -0,0 +1,114 @@ +//! The two store operations the callout's keys need, so a test can stand in +//! for the secret store. Every key is kept create-only: a stored seed is read, +//! never replaced. + +use std::future::Future; + +use anyhow::{Context, Result}; +use swarm_secret_client::SecretStore; + +/// Read and create-only write of one value per path. +pub trait KeyStore { + /// The value at `path`, or `None` when nothing is stored there. + fn read(&self, path: &str) -> impl Future>> + Send; + /// Store `value` at `path`, failing when anything is already there. + fn create(&self, path: &str, value: &str) -> impl Future> + Send; +} + +/// One field, `value`, as every single-value secret in the store is kept. +/// No `Debug`: the values held here include seeds. +#[derive(serde::Serialize, serde::Deserialize)] +struct Stored { + value: String, +} + +impl KeyStore for SecretStore { + async fn read(&self, path: &str) -> Result> { + let stored: Option = self + .read_optional(path) + .await + .with_context(|| format!("reading {path}"))?; + Ok(stored.map(|s| s.value)) + } + + async fn create(&self, path: &str, value: &str) -> Result<()> { + SecretStore::create( + self, + path, + &Stored { + value: value.to_owned(), + }, + ) + .await + .with_context(|| format!("creating {path}")) + } +} + +#[cfg(test)] +pub(crate) mod tests { + use std::collections::HashMap; + use std::sync::Mutex; + use std::sync::atomic::{AtomicUsize, Ordering}; + + use super::*; + + /// A store that keeps values in memory and counts writes. `rival` is a + /// value another start lands at a path between this one's read and its + /// create, which the create then loses to. + #[derive(Default)] + pub(crate) struct Fake { + values: Mutex>, + rival: Mutex>, + writes: AtomicUsize, + } + + impl Fake { + pub(crate) fn holding(entries: &[(&str, &str)]) -> Self { + let fake = Self::default(); + for (path, value) in entries { + fake.put(path, value); + } + fake + } + + pub(crate) fn put(&self, path: &str, value: &str) { + self.values + .lock() + .expect("unpoisoned") + .insert(path.to_owned(), value.to_owned()); + } + + pub(crate) fn get(&self, path: &str) -> Option { + self.values.lock().expect("unpoisoned").get(path).cloned() + } + + pub(crate) fn writes(&self) -> usize { + self.writes.load(Ordering::SeqCst) + } + + pub(crate) fn rival(&self, path: &str, value: &str) { + self.rival + .lock() + .expect("unpoisoned") + .insert(path.to_owned(), value.to_owned()); + } + } + + impl KeyStore for Fake { + async fn read(&self, path: &str) -> Result> { + Ok(self.get(path)) + } + + async fn create(&self, path: &str, value: &str) -> Result<()> { + if let Some(theirs) = self.rival.lock().expect("unpoisoned").remove(path) { + self.put(path, &theirs); + } + if self.get(path).is_some() { + anyhow::bail!("403 permission denied"); + } + self.writes.fetch_add(1, Ordering::SeqCst); + self.put(path, value); + Ok(()) + } + } +} diff --git a/swarm-nats-auth/src/main.rs b/swarm-nats-auth/src/main.rs index e84d4b21..cf6ab0b4 100644 --- a/swarm-nats-auth/src/main.rs +++ b/swarm-nats-auth/src/main.rs @@ -16,8 +16,9 @@ //! //! Every request is opened with the callout xkey, and its reply is sealed //! back to the server; see `xkey`. A request that does not open gets no -//! reply: the server this answers always seals. The `ensure-xkey` verb -//! creates that key in the secret store, and runs before the server does. +//! reply: the server this answers always seals. The `ensure-callout-keys` +//! verb creates that key and the two nkeys (see `nkey`) in the secret store, +//! and runs before the server does. //! //! # Secrets //! @@ -25,8 +26,8 @@ //! previously learned the hard way here: a value in nix config is rendered into //! the world-readable store, and a value in `argv` is readable by anyone via //! `/proc//cmdline`, which is `0444`. Paths are not secrets, so passing -//! them as flags is fine. The xkey seed is the exception to files: it is read -//! from the secret store, at the path `xkey` names. +//! them as flags is fine. The callout's three seeds are not files: they are +//! read from the secret store, at the paths `xkey` and `nkey` name. use std::path::PathBuf; @@ -36,6 +37,8 @@ use futures_util::StreamExt; mod agent_token; mod introspect; +mod keystore; +mod nkey; mod policy; mod request; mod respond; @@ -62,12 +65,23 @@ struct Cli { #[derive(Debug, Subcommand)] enum Command { - /// Create the callout xkey in the secret store when it holds none, and - /// check that its public half is stored beside it. Needs no NATS server. - EnsureXkey { + /// Create the callout xkey, issuer and user keys in the secret store when + /// it holds none, and check that each public half is stored beside its + /// seed. Needs no NATS server. + EnsureCalloutKeys { /// Role on the store's `cert` auth mount to log in as. #[arg(long, default_value = "swarm-nats-auth")] store_cert_role: String, + + /// Issuer seed to store when the store holds none. Removed once the + /// store holds an issuer seed. Absent or empty, a new key is made. + #[arg(long)] + import_issuer_seed_file: Option, + + /// User seed to store when the store holds none, handled as + /// `--import-issuer-seed-file` is. + #[arg(long)] + import_user_seed_file: Option, }, } @@ -77,16 +91,6 @@ struct Args { #[arg(long, default_value = "nats://127.0.0.1:4222")] nats_url: String, - /// Path to the seed of the callout-exempt user this responder connects as. - /// Its public half is `services.hyperhive.deploy.nats.calloutUserPublicKey`. - #[arg(long)] - user_seed_file: PathBuf, - - /// Path to the account signing seed used to sign issued user JWTs. Its - /// public half is `services.hyperhive.deploy.nats.calloutIssuerPublicKey`. - #[arg(long)] - issuer_seed_file: PathBuf, - /// Authelia's OIDC introspection endpoint. #[arg(long)] introspection_url: String, @@ -156,7 +160,7 @@ struct Args { agent_token_publish_subjects: Vec, /// Role on the secret store's `cert` auth mount this responder logs in - /// as to read the callout xkey and agent credentials. The store is found + /// as to read the callout keys and agent credentials. The store is found /// through the `BAO_*` environment, which the responder does not start /// without. #[arg(long, default_value = "swarm-nats-auth")] @@ -165,10 +169,8 @@ struct Args { /// Read a secret file and strip surrounding whitespace. /// -/// The trim matters: an `echo`-created seed file ends in a newline, and an -/// nkey seed with a trailing byte is not a seed — it fails at parse with a -/// message about encoding rather than about the file, which sends you looking -/// in the wrong place. The value is never logged, and the error deliberately +/// The trim matters: an `echo`-created file ends in a newline, which is not +/// part of the secret. The value is never logged, and the error deliberately /// names only the path. fn read_secret(path: &std::path::Path) -> anyhow::Result { let raw = std::fs::read_to_string(path).with_context(|| format!("read {}", path.display()))?; @@ -194,7 +196,21 @@ async fn main() -> anyhow::Result<()> { .init(); let cli = Cli::parse(); match (cli.command, cli.responder) { - (Some(Command::EnsureXkey { store_cert_role }), _) => ensure_xkey(&store_cert_role).await, + ( + Some(Command::EnsureCalloutKeys { + store_cert_role, + import_issuer_seed_file, + import_user_seed_file, + }), + _, + ) => { + ensure_callout_keys( + &store_cert_role, + import_issuer_seed_file.as_deref(), + import_user_seed_file.as_deref(), + ) + .await + } (None, Some(args)) => serve(&args).await, (None, None) => unreachable!("clap requires the responder's flags when no verb is given"), } @@ -222,25 +238,13 @@ async fn serve(args: &Args) -> anyhow::Result<()> { )? .with_agent_token_subjects(args.agent_token_publish_subjects.clone())?; let http = reqwest::Client::new(); - let issuer = nkeys::KeyPair::from_seed(&read_secret(&args.issuer_seed_file)?) - .context("parse the account signing seed")?; let settings = swarm_secret_client::client::Settings::from_env() .context("reading the secret store's address and this responder's identity")?; - // Before the connect: a responder that cannot open a sealed request must - // not start answering. Read, never created here; see `xkey`. - let callout_xkey = xkey::load( - &swarm_secret_client::SecretStore::connect( - &settings, - &args.store_cert_role, - swarm_secret_client::client::DEFAULT_CERT_MOUNT, - ) - .await - .context("logging in to the secret store for the callout xkey")?, - ) - .await?; - tracing::info!(public_key = %callout_xkey.public_key(), "loaded the callout xkey"); + // Before the connect: a responder that cannot open a sealed request, sign + // a grant or authenticate to the queue must not start answering. + let (callout_xkey, issuer, user_seed) = + load_callout_keys(&settings, &args.store_cert_role).await?; let store = agent_token::Store::new(settings, args.store_cert_role.clone()); - let user_seed = read_secret(&args.user_seed_file)?; let client = async_nats::ConnectOptions::with_nkey(user_seed) .name("swarm-nats-auth") .connect(&args.nats_url) @@ -361,8 +365,37 @@ fn open_request( } } -/// The `ensure-xkey` verb: see [`xkey::ensure`]. -async fn ensure_xkey(store_cert_role: &str) -> anyhow::Result<()> { +/// The callout xkey, the issuer key, and the user key's seed, read from the +/// store and never created here; see `xkey` and `nkey`. +async fn load_callout_keys( + settings: &swarm_secret_client::client::Settings, + store_cert_role: &str, +) -> anyhow::Result<(nkeys::XKey, nkeys::KeyPair, String)> { + let keys = swarm_secret_client::SecretStore::connect( + settings, + store_cert_role, + swarm_secret_client::client::DEFAULT_CERT_MOUNT, + ) + .await + .context("logging in to the secret store for the callout keys")?; + let callout_xkey = xkey::load(&keys).await?; + tracing::info!(public_key = %callout_xkey.public_key(), "loaded the callout xkey"); + let issuer = nkey::load(&keys, &nkey::ISSUER).await?; + tracing::info!(public_key = %issuer.public_key(), "loaded the callout issuer key"); + let user = nkey::load(&keys, &nkey::USER).await?; + tracing::info!(public_key = %user.public_key(), "loaded the callout user key"); + let user_seed = user + .seed() + .map_err(|_| anyhow::anyhow!("the callout user key has no seed"))?; + Ok((callout_xkey, issuer, user_seed)) +} + +/// The `ensure-callout-keys` verb: see [`xkey::ensure`] and [`nkey::ensure`]. +async fn ensure_callout_keys( + store_cert_role: &str, + import_issuer_seed_file: Option<&std::path::Path>, + import_user_seed_file: Option<&std::path::Path>, +) -> anyhow::Result<()> { let store = swarm_secret_client::SecretStore::from_env(store_cert_role) .await .context("logging in to the secret store")?; @@ -372,6 +405,17 @@ async fn ensure_xkey(store_cert_role: &str) -> anyhow::Result<()> { path = xkey::PUBLIC_PATH, "the callout xkey is in the secret store" ); + for (pair, import) in [ + (&nkey::ISSUER, import_issuer_seed_file), + (&nkey::USER, import_user_seed_file), + ] { + let key = nkey::ensure(&store, pair, import).await?; + tracing::info!( + public_key = %key.public_key(), + path = pair.public_path, + "the {} key is in the secret store", pair.name + ); + } Ok(()) } @@ -440,10 +484,6 @@ mod tests { fn the_units_own_argument_vector_parses() { let args = responder([ "swarm-nats-auth", - "--user-seed-file", - "/run/credentials/callout-user.seed", - "--issuer-seed-file", - "/run/credentials/issuer.seed", "--client-secret-file", "/run/credentials/oidc-client.secret", "--introspection-url", @@ -503,10 +543,6 @@ mod tests { fn an_agent_suffix_without_a_leading_dash_parses_too() { let args = responder([ "swarm-nats-auth", - "--user-seed-file", - "/run/credentials/callout-user.seed", - "--issuer-seed-file", - "/run/credentials/issuer.seed", "--client-secret-file", "/run/credentials/oidc-client.secret", "--introspection-url", @@ -524,10 +560,6 @@ mod tests { fn the_default_suffix_is_what_the_module_passes() { let args = responder([ "swarm-nats-auth", - "--user-seed-file", - "/run/credentials/callout-user.seed", - "--issuer-seed-file", - "/run/credentials/issuer.seed", "--client-secret-file", "/run/credentials/oidc-client.secret", "--introspection-url", @@ -543,16 +575,37 @@ mod tests { fn the_ensure_verb_parses_as_the_unit_runs_it() { let cli = Cli::try_parse_from([ "swarm-nats-auth", - "ensure-xkey", + "ensure-callout-keys", "--store-cert-role", "swarm-nats-auth", + "--import-issuer-seed-file", + "/var/lib/swarm-nats-callout/issuer.seed", + "--import-user-seed-file", + "/var/lib/swarm-nats-callout/callout-user.seed", ]) .expect("the verb needs no responder flags"); assert!(cli.responder.is_none()); - assert!(matches!( - cli.command, - Some(Command::EnsureXkey { ref store_cert_role }) if store_cert_role == "swarm-nats-auth" - )); + let Some(Command::EnsureCalloutKeys { + store_cert_role, + import_issuer_seed_file, + import_user_seed_file, + }) = cli.command + else { + panic!("the verb parses as itself"); + }; + assert_eq!(store_cert_role, "swarm-nats-auth"); + assert_eq!( + import_issuer_seed_file.as_deref(), + Some(std::path::Path::new( + "/var/lib/swarm-nats-callout/issuer.seed" + )) + ); + assert_eq!( + import_user_seed_file.as_deref(), + Some(std::path::Path::new( + "/var/lib/swarm-nats-callout/callout-user.seed" + )) + ); } /// A bare invocation is neither: the responder's flags stay required @@ -566,9 +619,9 @@ mod tests { fn the_verb_does_not_take_the_responders_flags() { Cli::try_parse_from([ "swarm-nats-auth", - "--user-seed-file", - "/run/credentials/callout-user.seed", - "ensure-xkey", + "--client-secret-file", + "/run/credentials/oidc-client.secret", + "ensure-callout-keys", ]) .expect_err("a verb and the responder's flags conflict"); } diff --git a/swarm-nats-auth/src/nkey.rs b/swarm-nats-auth/src/nkey.rs new file mode 100644 index 00000000..7227a148 --- /dev/null +++ b/swarm-nats-auth/src/nkey.rs @@ -0,0 +1,365 @@ +//! The callout's two nkeys: the account key the responder signs every user +//! JWT with ([`ISSUER`]), and the user key it connects to the queue as +//! ([`USER`]). +//! +//! Each seed and its public half live in the secret store beside the xkey's, +//! and only the `swarm-nats-auth` role reads them. The queue's host renders +//! both public halves into `auth_callout` from there. +//! +//! [`ensure`] creates a pair when the store holds no seed, from the seed in an +//! import file when one is given and present, and removes that file once the +//! store holds a seed. When the store already holds a different seed, the +//! store's is kept and the file is removed after a warning. [`load`] only ever +//! reads, for the same reason as `xkey::load`. + +use std::io::ErrorKind; +use std::path::Path; + +use anyhow::{Context, Result, anyhow}; +use nkeys::{KeyPair, KeyPairType}; + +use crate::keystore::KeyStore; + +/// One of the callout's nkeys: where its two halves are stored, and which +/// kind of key it is. +pub struct Pair { + /// What logs and errors call it. + pub name: &'static str, + /// The seed's path under the store's KV mount. + pub seed_path: &'static str, + /// The public half's path under the store's KV mount. + pub public_path: &'static str, + kind: KeyPairType, +} + +/// The account key. The nix halves are `issuerSeedLeaf` and +/// `issuerPublicLeaf` in `nix/host-modules/swarm-bao.nix`. +pub const ISSUER: Pair = Pair { + name: "callout issuer", + seed_path: "swarm/services/swarm-nats/callout/issuer-seed", + public_path: "swarm/public/swarm-nats/callout-issuer", + kind: KeyPairType::Account, +}; + +/// The callout-exempt user. The nix halves are `userSeedLeaf` and +/// `userPublicLeaf` in `nix/host-modules/swarm-bao.nix`. +pub const USER: Pair = Pair { + name: "callout user", + seed_path: "swarm/services/swarm-nats/callout/user-seed", + public_path: "swarm/public/swarm-nats/callout-user", + kind: KeyPairType::User, +}; + +impl Pair { + /// Never carries the parse error: it is about the bytes of the seed. + fn parse_seed(&self, seed: &str, source: &str) -> Result { + KeyPair::from_seed(seed.trim()) + .ok() + .filter(|key| key.key_pair_type() == self.kind) + .ok_or_else(|| anyhow!("{source} does not hold a {} seed", self.name)) + } +} + +/// The stored pair, created first when no seed is stored. +/// +/// The created seed is the one in `import` when that file exists and is not +/// empty, so a key that was kept on disk keeps its public half. Once the store +/// holds a seed, `import` is removed: after a warning when it holds a +/// different seed, since the store's is the one in use. +/// +/// Safe to run from two places at once, as `xkey::ensure` is. +/// +/// # Errors +/// When the store cannot be read or written, when `import` cannot be read or +/// removed, when the stored seed is not of this pair's kind, when the store +/// holds none and `import` is not a seed of that kind, and when the stored +/// public half is not the seed's. +pub async fn ensure(store: &impl KeyStore, pair: &Pair, import: Option<&Path>) -> Result { + let key = match store.read(pair.seed_path).await? { + Some(seed) => pair.parse_seed(&seed, pair.seed_path)?, + None => create_seed(store, pair, import).await?, + }; + publish(store, pair, &key.public_key()).await?; + if let Some(import) = import { + retire(store, pair, import).await?; + } + Ok(key) +} + +/// The stored pair. Never writes. +/// +/// # Errors +/// When the store cannot be read, holds no seed, or holds one that is not of +/// this pair's kind. +pub async fn load(store: &impl KeyStore, pair: &Pair) -> Result { + let seed = store.read(pair.seed_path).await?.with_context(|| { + format!( + "nothing is stored at {}; swarm-bao-nats-callout-xkey creates it", + pair.seed_path + ) + })?; + pair.parse_seed(&seed, pair.seed_path) +} + +/// The seed in `path`, or `None` when there is no such file or it is empty. +fn read_import(path: &Path) -> Result> { + match std::fs::read_to_string(path) { + Ok(raw) if raw.trim().is_empty() => Ok(None), + Ok(raw) => Ok(Some(raw.trim().to_owned())), + Err(e) if e.kind() == ErrorKind::NotFound => Ok(None), + Err(e) => Err(e).with_context(|| format!("reading {}", path.display())), + } +} + +async fn create_seed(store: &impl KeyStore, pair: &Pair, import: Option<&Path>) -> Result { + let imported = match import { + Some(path) => read_import(path)?.map(|seed| (path, seed)), + None => None, + }; + let key = match &imported { + Some((path, seed)) => pair.parse_seed(seed, &path.display().to_string())?, + None => KeyPair::new(pair.kind.clone()), + }; + let seed = key + .seed() + .map_err(|_| anyhow!("the {} key has no seed", pair.name))?; + match store.create(pair.seed_path, &seed).await { + Ok(()) => { + if let Some((path, _)) = &imported { + tracing::info!( + public_key = %key.public_key(), + from = %path.display(), + "imported the {} seed into the store", pair.name + ); + } else { + tracing::info!(public_key = %key.public_key(), "created the {} seed", pair.name); + } + Ok(key) + } + Err(e) => match store.read(pair.seed_path).await? { + Some(theirs) => pair.parse_seed(&theirs, pair.seed_path), + None => Err(e), + }, + } +} + +async fn publish(store: &impl KeyStore, pair: &Pair, public: &str) -> Result<()> { + let stored = match store.read(pair.public_path).await? { + Some(stored) => stored, + None => match store.create(pair.public_path, public).await { + Ok(()) => return Ok(()), + Err(e) => store.read(pair.public_path).await?.ok_or(e)?, + }, + }; + anyhow::ensure!( + stored == public, + "{} holds a public key that is not the one for the seed at {}", + pair.public_path, + pair.seed_path + ); + Ok(()) +} + +/// Removes `import` once a read of the store, made after it, returns a seed +/// of this pair's kind. Logs a warning first when that seed is not the one in +/// the file. +async fn retire(store: &impl KeyStore, pair: &Pair, import: &Path) -> Result<()> { + let on_disk = match std::fs::read_to_string(import) { + Ok(raw) => raw.trim().to_owned(), + Err(e) if e.kind() == ErrorKind::NotFound => return Ok(()), + Err(e) => return Err(e).with_context(|| format!("reading {}", import.display())), + }; + let stored = store.read(pair.seed_path).await?.with_context(|| { + format!( + "{} is empty after it was ensured; keeping {}", + pair.seed_path, + import.display() + ) + })?; + pair.parse_seed(&stored, pair.seed_path)?; + if !on_disk.is_empty() && stored.trim() != on_disk { + tracing::warn!( + file = %import.display(), + path = pair.seed_path, + "the store holds a different {} seed than this file; keeping the store's and removing the file", + pair.name + ); + } + std::fs::remove_file(import).with_context(|| format!("removing {}", import.display()))?; + tracing::info!(file = %import.display(), "removed the {} seed file", pair.name); + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::*; + use crate::keystore::tests::Fake; + + fn seed_of(key: &KeyPair) -> String { + key.seed().expect("a generated key has a seed") + } + + /// A file under a directory of its own, removed with the guard. + struct Import { + dir: PathBuf, + path: PathBuf, + } + + impl Import { + fn new(test: &str) -> Self { + let dir = std::env::temp_dir().join(format!( + "swarm-nats-auth-nkey-{}-{test}", + std::process::id() + )); + std::fs::create_dir_all(&dir).expect("a temp dir"); + let path = dir.join("seed"); + Self { dir, path } + } + + fn holding(test: &str, contents: &str) -> Self { + let import = Self::new(test); + std::fs::write(&import.path, contents).expect("written"); + import + } + } + + impl Drop for Import { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.dir); + } + } + + #[tokio::test] + async fn an_empty_store_with_no_import_gets_a_new_pair_of_the_right_kind() { + let store = Fake::default(); + let issuer = ensure(&store, &ISSUER, None).await.expect("ensured"); + let user = ensure(&store, &USER, None).await.expect("ensured"); + assert!(issuer.public_key().starts_with('A'), "an account key"); + assert!(user.public_key().starts_with('U'), "a user key"); + assert_eq!(store.get(ISSUER.seed_path), Some(seed_of(&issuer))); + assert_eq!(store.get(ISSUER.public_path), Some(issuer.public_key())); + assert_eq!(store.get(USER.seed_path), Some(seed_of(&user))); + assert_eq!(store.get(USER.public_path), Some(user.public_key())); + assert_eq!(store.writes(), 4); + } + + /// The key kept on disk keeps its public half, and the file is gone once + /// the store holds the same seed. + #[tokio::test] + async fn an_import_file_is_stored_as_is_and_then_removed() { + let existing = KeyPair::new_account(); + let import = Import::holding("imported", &format!("{}\n", seed_of(&existing))); + let store = Fake::default(); + let key = ensure(&store, &ISSUER, Some(&import.path)) + .await + .expect("ensured"); + assert_eq!(key.public_key(), existing.public_key()); + assert_eq!(store.get(ISSUER.seed_path), Some(seed_of(&existing))); + assert_eq!(store.get(ISSUER.public_path), Some(existing.public_key())); + assert!(!import.path.exists(), "the import file is removed"); + } + + #[tokio::test] + async fn an_absent_or_empty_import_file_mints_a_new_key() { + let absent = Import::new("absent"); + let store = Fake::default(); + ensure(&store, &USER, Some(&absent.path)) + .await + .expect("ensured"); + assert!(store.get(USER.seed_path).is_some()); + + let empty = Import::holding("empty", "\n"); + let store = Fake::default(); + ensure(&store, &USER, Some(&empty.path)) + .await + .expect("ensured"); + assert!(store.get(USER.seed_path).is_some()); + assert!(!empty.path.exists(), "the empty file is removed"); + } + + /// The store's seed is the one in use, so it wins over a file that + /// differs, and the file goes. + #[tokio::test] + async fn a_stored_seed_wins_over_a_different_import_file() { + let stored = KeyPair::new_account(); + let store = Fake::holding(&[ + (ISSUER.seed_path, &seed_of(&stored)), + (ISSUER.public_path, &stored.public_key()), + ]); + let import = Import::holding("differs", &seed_of(&KeyPair::new_account())); + let key = ensure(&store, &ISSUER, Some(&import.path)) + .await + .expect("ensured"); + assert_eq!(key.public_key(), stored.public_key()); + assert_eq!(store.get(ISSUER.seed_path), Some(seed_of(&stored))); + assert_eq!(store.writes(), 0); + assert!(!import.path.exists(), "the differing file is removed"); + } + + /// Lost the create to a start that stored another seed: that seed is + /// kept, and the file still goes. + #[tokio::test] + async fn an_import_that_loses_the_create_race_takes_the_winners_seed() { + let winner = KeyPair::new_user(); + let store = Fake::default(); + store.rival(USER.seed_path, &seed_of(&winner)); + let import = Import::holding("race", &seed_of(&KeyPair::new_user())); + let key = ensure(&store, &USER, Some(&import.path)) + .await + .expect("ensured"); + assert_eq!(key.public_key(), winner.public_key()); + assert_eq!(store.get(USER.public_path), Some(winner.public_key())); + assert!(!import.path.exists()); + } + + /// Nothing is stored and the file is not a seed of this kind: the file + /// is the only copy there may be, so it is kept and nothing is stored. + #[tokio::test] + async fn an_import_of_the_wrong_kind_is_refused_and_kept_without_quoting_it() { + let user_seed = seed_of(&KeyPair::new_user()); + let import = Import::holding("wrong-kind", &user_seed); + let store = Fake::default(); + let e = ensure(&store, &ISSUER, Some(&import.path)) + .await + .expect_err("a user seed is no issuer"); + assert!(!format!("{e:#}").contains(&user_seed), "{e:#}"); + assert!(import.path.exists(), "the file is kept"); + assert_eq!(store.writes(), 0); + } + + #[tokio::test] + async fn a_stored_seed_of_the_wrong_kind_is_refused_without_quoting_it() { + let account_seed = seed_of(&KeyPair::new_account()); + let store = Fake::holding(&[(USER.seed_path, &account_seed)]); + let e = ensure(&store, &USER, None) + .await + .expect_err("an account seed"); + assert!(!format!("{e:#}").contains(&account_seed), "{e:#}"); + let e = load(&store, &USER).await.expect_err("an account seed"); + assert!(!format!("{e:#}").contains(&account_seed), "{e:#}"); + } + + #[tokio::test] + async fn a_public_half_that_is_not_the_seeds_is_refused() { + let existing = KeyPair::new_user(); + let store = Fake::holding(&[ + (USER.seed_path, &seed_of(&existing)), + (USER.public_path, &KeyPair::new_user().public_key()), + ]); + assert!(ensure(&store, &USER, None).await.is_err()); + } + + #[tokio::test] + async fn loading_reads_the_stored_seed_and_never_creates_one() { + let store = Fake::default(); + assert!(load(&store, &ISSUER).await.is_err()); + assert_eq!(store.writes(), 0); + + let existing = KeyPair::new_account(); + store.put(ISSUER.seed_path, &seed_of(&existing)); + let key = load(&store, &ISSUER).await.expect("loaded"); + assert_eq!(key.public_key(), existing.public_key()); + } +} diff --git a/swarm-nats-auth/src/xkey.rs b/swarm-nats-auth/src/xkey.rs index ab79a252..d7052136 100644 --- a/swarm-nats-auth/src/xkey.rs +++ b/swarm-nats-auth/src/xkey.rs @@ -11,11 +11,10 @@ //! responder that minted a new pair on its own start would hold a key the //! running server does not know, and every request would fail to open. -use std::future::Future; - use anyhow::{Context, Result, anyhow}; use nkeys::XKey; -use swarm_secret_client::SecretStore; + +use crate::keystore::KeyStore; /// The seed's path under the store's KV mount. The nix half is /// `xkeySeedLeaf` in `nix/host-modules/swarm-bao.nix`. @@ -30,44 +29,6 @@ pub const PUBLIC_PATH: &str = "swarm/public/swarm-nats/callout-xkey"; /// server's own public xkey, which the reply is sealed to. pub const SERVER_XKEY_HEADER: &str = "Nats-Server-Xkey"; -/// The two operations [`ensure`] and [`load`] need from the store, so a test -/// can stand in for it. -pub trait KeyStore { - /// The value at `path`, or `None` when nothing is stored there. - fn read(&self, path: &str) -> impl Future>> + Send; - /// Store `value` at `path`, failing when anything is already there. - fn create(&self, path: &str, value: &str) -> impl Future> + Send; -} - -/// One field, `value`, as every single-value secret in the store is kept. -/// No `Debug`: one of the values held here is the seed. -#[derive(serde::Serialize, serde::Deserialize)] -struct Stored { - value: String, -} - -impl KeyStore for SecretStore { - async fn read(&self, path: &str) -> Result> { - let stored: Option = self - .read_optional(path) - .await - .with_context(|| format!("reading {path}"))?; - Ok(stored.map(|s| s.value)) - } - - async fn create(&self, path: &str, value: &str) -> Result<()> { - SecretStore::create( - self, - path, - &Stored { - value: value.to_owned(), - }, - ) - .await - .with_context(|| format!("creating {path}")) - } -} - /// The stored pair, created first when no seed is stored. /// /// Safe to run from two places at once. Each writes with a create that fails @@ -165,60 +126,8 @@ pub fn seal(own: &XKey, server: &XKey, reply: &[u8]) -> Result> { #[cfg(test)] pub(crate) mod tests { - use std::collections::HashMap; - use std::sync::Mutex; - use std::sync::atomic::{AtomicUsize, Ordering}; - use super::*; - - /// A store that keeps values in memory and counts writes. `rival` is a - /// value another start lands at a path between this one's read and its - /// create, which the create then loses to. - #[derive(Default)] - struct Fake { - values: Mutex>, - rival: Mutex>, - writes: AtomicUsize, - } - - impl Fake { - fn holding(entries: &[(&str, &str)]) -> Self { - let fake = Self::default(); - for (path, value) in entries { - fake.put(path, value); - } - fake - } - - fn put(&self, path: &str, value: &str) { - self.values - .lock() - .expect("unpoisoned") - .insert(path.to_owned(), value.to_owned()); - } - - fn get(&self, path: &str) -> Option { - self.values.lock().expect("unpoisoned").get(path).cloned() - } - } - - impl KeyStore for Fake { - async fn read(&self, path: &str) -> Result> { - Ok(self.get(path)) - } - - async fn create(&self, path: &str, value: &str) -> Result<()> { - if let Some(theirs) = self.rival.lock().expect("unpoisoned").remove(path) { - self.put(path, &theirs); - } - if self.get(path).is_some() { - anyhow::bail!("403 permission denied"); - } - self.writes.fetch_add(1, Ordering::SeqCst); - self.put(path, value); - Ok(()) - } - } + use crate::keystore::tests::Fake; fn seed_of(key: &XKey) -> String { key.seed().expect("a generated key has a seed") @@ -231,7 +140,7 @@ pub(crate) mod tests { assert_eq!(store.get(SEED_PATH), Some(seed_of(&key))); assert_eq!(store.get(PUBLIC_PATH), Some(key.public_key())); assert!(key.public_key().starts_with('X'), "a curve public key"); - assert_eq!(store.writes.load(Ordering::SeqCst), 2); + assert_eq!(store.writes(), 2); } #[tokio::test] @@ -243,7 +152,7 @@ pub(crate) mod tests { ]); let key = ensure(&store).await.expect("ensured"); assert_eq!(key.public_key(), existing.public_key()); - assert_eq!(store.writes.load(Ordering::SeqCst), 0); + assert_eq!(store.writes(), 0); } /// A run that stopped between its two writes left a seed and no public @@ -255,7 +164,7 @@ pub(crate) mod tests { let key = ensure(&store).await.expect("ensured"); assert_eq!(key.public_key(), existing.public_key()); assert_eq!(store.get(PUBLIC_PATH), Some(existing.public_key())); - assert_eq!(store.writes.load(Ordering::SeqCst), 1); + assert_eq!(store.writes(), 1); } /// Two starts read an empty store at once and both generate; the one @@ -264,21 +173,13 @@ pub(crate) mod tests { async fn a_start_that_loses_the_create_race_takes_the_winners_seed() { let winner = XKey::new(); let store = Fake::default(); - store - .rival - .lock() - .expect("unpoisoned") - .insert(SEED_PATH.to_owned(), seed_of(&winner)); - store - .rival - .lock() - .expect("unpoisoned") - .insert(PUBLIC_PATH.to_owned(), winner.public_key()); + store.rival(SEED_PATH, &seed_of(&winner)); + store.rival(PUBLIC_PATH, &winner.public_key()); let key = ensure(&store).await.expect("ensured"); assert_eq!(key.public_key(), winner.public_key()); assert_eq!(store.get(SEED_PATH), Some(seed_of(&winner))); assert_eq!(store.get(PUBLIC_PATH), Some(winner.public_key())); - assert_eq!(store.writes.load(Ordering::SeqCst), 0); + assert_eq!(store.writes(), 0); } #[tokio::test] @@ -306,7 +207,7 @@ pub(crate) mod tests { async fn loading_reads_the_stored_seed_and_never_creates_one() { let store = Fake::default(); assert!(load(&store).await.is_err()); - assert_eq!(store.writes.load(Ordering::SeqCst), 0); + assert_eq!(store.writes(), 0); assert_eq!(store.get(SEED_PATH), None); let existing = XKey::new();