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

@@ -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
+The seed is in the swarm secret store, and so are the seeds of the issuer key
-connects to the queue: a responder without it doesn't start. The
+the responder signs with and the user key it connects as. The responder reads
-`ensure-xkey` verb creates the pair when the store holds none. It runs from
+all three before it connects to the queue: a responder missing one doesn't
-its own unit before the server, because the server needs the public half to
+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.
 

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<Output = Result<Option<String>>> + Send;
+    /// Store `value` at `path`, failing when anything is already there.
+    fn create(&self, path: &str, value: &str) -> impl Future<Output = Result<()>> + 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<Option<String>> {
+        let stored: Option<Stored> = 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<HashMap<String, String>>,
+        rival: Mutex<HashMap<String, String>>,
+        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<String> {
+            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<Option<String>> {
+            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(())
+        }
+    }
+}

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
+//! reply: the server this answers always seals. The `ensure-callout-keys`
-//! creates that key in the secret store, and runs before the server does.
+//! 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/<pid>/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
+//! them as flags is fine. The callout's three seeds are not files: they are
-//! from the secret store, at the path `xkey` names.
+//! 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
+    /// Create the callout xkey, issuer and user keys in the secret store when
-    /// check that its public half is stored beside it. Needs no NATS server.
+    /// it holds none, and check that each public half is stored beside its
-    EnsureXkey {
+    /// 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<PathBuf>,
+
+        /// User seed to store when the store holds none, handled as
+        /// `--import-issuer-seed-file` is.
+        #[arg(long)]
+        import_user_seed_file: Option<PathBuf>,
     },
 }
 
@@ -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<String>,
 
     /// 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
+/// The trim matters: an `echo`-created file ends in a newline, which is not
-/// nkey seed with a trailing byte is not a seed — it fails at parse with a
+/// part of the secret. The value is never logged, and the error deliberately
-/// 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
 /// names only the path.
 fn read_secret(path: &std::path::Path) -> anyhow::Result<String> {
     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
+    // Before the connect: a responder that cannot open a sealed request, sign
-    // not start answering. Read, never created here; see `xkey`.
+    // a grant or authenticate to the queue must not start answering.
-    let callout_xkey = xkey::load(
+    let (callout_xkey, issuer, user_seed) =
-        &swarm_secret_client::SecretStore::connect(
+        load_callout_keys(&settings, &args.store_cert_role).await?;
-            &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");
     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`].
+/// The callout xkey, the issuer key, and the user key's seed, read from the
-async fn ensure_xkey(store_cert_role: &str) -> anyhow::Result<()> {
+/// 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!(
+        let Some(Command::EnsureCalloutKeys {
-            cli.command,
+            store_cert_role,
-            Some(Command::EnsureXkey { ref store_cert_role }) if store_cert_role == "swarm-nats-auth"
+            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",
+            "--client-secret-file",
-            "/run/credentials/callout-user.seed",
+            "/run/credentials/oidc-client.secret",
-            "ensure-xkey",
+            "ensure-callout-keys",
         ])
         .expect_err("a verb and the responder's flags conflict");
     }

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> {
+        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<KeyPair> {
+    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<KeyPair> {
+    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<Option<String>> {
+    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<KeyPair> {
+    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());
+    }
+}

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<Output = Result<Option<String>>> + Send;
-    /// Store `value` at `path`, failing when anything is already there.
-    fn create(&self, path: &str, value: &str) -> impl Future<Output = Result<()>> + 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<Option<String>> {
-        let stored: Option<Stored> = 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<Vec<u8>> {
 
 #[cfg(test)]
 pub(crate) mod tests {
-    use std::collections::HashMap;
-    use std::sync::Mutex;
-    use std::sync::atomic::{AtomicUsize, Ordering};
-
     use super::*;
-
+    use crate::keystore::tests::Fake;
-    /// 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<HashMap<String, String>>,
-        rival: Mutex<HashMap<String, String>>,
-        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<String> {
-            self.values.lock().expect("unpoisoned").get(path).cloned()
-        }
-    }
-
-    impl KeyStore for Fake {
-        async fn read(&self, path: &str) -> Result<Option<String>> {
-            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(())
-        }
-    }
 
     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
+        store.rival(SEED_PATH, &seed_of(&winner));
-            .rival
+        store.rival(PUBLIC_PATH, &winner.public_key());
-            .lock()
-            .expect("unpoisoned")
-            .insert(SEED_PATH.to_owned(), seed_of(&winner));
-        store
-            .rival
-            .lock()
-            .expect("unpoisoned")
-            .insert(PUBLIC_PATH.to_owned(), 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();