Harden JWT key handling and redact secrets from command logs

src/auth.rs

@@ -1,3 +1,4 @@
+use crate::secrets;
 use anyhow::Context;
 use anyhow::Result;
 use hmac::{Hmac, Mac};
@@ -12,13 +13,50 @@ use std::sync::Mutex;
 
 type Claims = BTreeMap<String, u32>;
 
+/// Holds the admin JWT signing key. `None` until a key is configured
+/// (or until a random key is generated on first use when nothing is configured).
 pub static JWT_KEY: Lazy<Mutex<Option<String>>> = Lazy::new(|| Mutex::new(Default::default()));
-pub fn set_jwt_key(b: &str) {
+
+/// Install the HMAC secret used to sign and verify admin JWTs.
+///
+/// Empty / whitespace-only keys are rejected: with an empty key anyone could
+/// forge an admin token. Callers should treat the returned error as fatal.
+pub fn set_jwt_key(b: &str) -> Result<()> {
+    if b.trim().is_empty() {
+        anyhow::bail!("jwt key cannot be empty");
+    }
     *JWT_KEY.lock().unwrap() = Some(b.to_string());
+    Ok(())
+}
+
+/// Install the signing key from a stack config, where an empty string means
+/// "not configured" (local dev): [`get_jwt_key`] then generates a random key
+/// on first use instead. A configured-but-blank key is still a hard error.
+pub fn set_jwt_key_from_config(b: &str) -> Result<()> {
+    if b.is_empty() {
+        return Ok(());
+    }
+    set_jwt_key(b)
 }
-fn get_jwt_key() -> String {
-    let jk = &*JWT_KEY.lock().unwrap();
-    jk.clone().unwrap_or("some-secret".to_string()).to_owned()
+
+/// The admin JWT signing key:
+/// - a key installed via [`set_jwt_key`] is used as-is;
+/// - a key that is somehow empty is an error (never sign with an empty key);
+/// - when nothing is configured, a random key is generated once and kept for
+///   the process lifetime. There is no hardcoded fallback secret; note that a
+///   generated key rotates on restart, invalidating previously issued tokens
+///   (local dev: set `jwt_key` in the stack config to keep tokens stable).
+pub fn get_jwt_key() -> Result<String> {
+    let mut jk = JWT_KEY.lock().unwrap();
+    match jk.as_deref() {
+        Some(k) if k.trim().is_empty() => anyhow::bail!("jwt key is set but empty"),
+        Some(k) => Ok(k.to_string()),
+        None => {
+            let k = secrets::random_word(48);
+            *jk = Some(k.clone());
+            Ok(k)
+        }
+    }
 }
 
 #[derive(Clone)]
@@ -35,8 +73,9 @@ impl AdminJwtClaims {
         })
     }
     pub fn check(token: &str) -> std::result::Result<Self, JwtError> {
+        let key = jwt_key().map_err(|_| JwtError::Invalid)?;
         let claims: Claims = token
-            .verify_with_key(&jwt_key())
+            .verify_with_key(&key)
             .map_err(|_| JwtError::Invalid)?;
         let jwtc = AdminJwtClaims::from_claims(claims).map_err(|_| JwtError::Missing)?;
         if jwtc.clone().exp < now() {
@@ -47,17 +86,18 @@ impl AdminJwtClaims {
     }
 }
 
-fn jwt_key() -> Hmac<Sha256> {
-    let jk = get_jwt_key();
-    let key: Hmac<Sha256> = Hmac::new_from_slice(jk.as_bytes()).expect("failed");
-    key
+fn jwt_key() -> Result<Hmac<Sha256>> {
+    let jk = get_jwt_key()?;
+    let key: Hmac<Sha256> = Hmac::new_from_slice(jk.as_bytes())
+        .context("failed to build hmac key from jwt key")?;
+    Ok(key)
 }
 
 pub fn make_jwt(user: u32) -> Result<String> {
     let mut claims = BTreeMap::new();
     claims.insert("exp", now() + days(7));
     claims.insert("user", user);
-    let token = claims.sign_with_key(&jwt_key())?;
+    let token = claims.sign_with_key(&jwt_key()?)?;
     Ok(token)
 }
 
@@ -105,3 +145,108 @@ pub fn hash_pass(pwd: &str) -> Result<bool> {
     let valid = bcrypt::verify(pwd, &hashed)?;
     Ok(valid)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// JWT_KEY is process-global state: serialize every test that touches it.
+    static KEY_TEST_LOCK: Mutex<()> = Mutex::new(());
+
+    fn reset_key() {
+        *JWT_KEY.lock().unwrap() = None;
+    }
+
+    #[test]
+    fn get_jwt_key_generates_random_key_when_unset() {
+        let _g = KEY_TEST_LOCK.lock().unwrap();
+        reset_key();
+        let k1 = get_jwt_key().expect("should generate a random key");
+        assert!(!k1.is_empty());
+        assert_ne!(k1, "some-secret");
+        assert_eq!(k1.len(), 48);
+        assert!(k1.chars().all(|c| c.is_ascii_alphanumeric()));
+        // the generated key is stable for the process lifetime
+        let k2 = get_jwt_key().expect("should reuse the generated key");
+        assert_eq!(k1, k2);
+        reset_key();
+    }
+
+    #[test]
+    fn set_jwt_key_rejects_empty_and_blank() {
+        let _g = KEY_TEST_LOCK.lock().unwrap();
+        reset_key();
+        assert!(set_jwt_key("").is_err());
+        assert!(set_jwt_key("   ").is_err());
+        // failed installs leave no key behind
+        assert!(JWT_KEY.lock().unwrap().is_none());
+        reset_key();
+    }
+
+    #[test]
+    fn set_jwt_key_accepts_real_key() {
+        let _g = KEY_TEST_LOCK.lock().unwrap();
+        reset_key();
+        set_jwt_key("a-real-key").expect("valid key should install");
+        assert_eq!(get_jwt_key().unwrap(), "a-real-key");
+        reset_key();
+    }
+
+    #[test]
+    fn get_jwt_key_errors_when_key_set_but_empty() {
+        let _g = KEY_TEST_LOCK.lock().unwrap();
+        reset_key();
+        // install directly, bypassing set_jwt_key, to simulate a legacy empty key
+        *JWT_KEY.lock().unwrap() = Some("".to_string());
+        assert!(get_jwt_key().is_err());
+        *JWT_KEY.lock().unwrap() = Some("  ".to_string());
+        assert!(get_jwt_key().is_err());
+        reset_key();
+    }
+
+    #[test]
+    fn set_jwt_key_from_config_empty_means_not_configured() {
+        let _g = KEY_TEST_LOCK.lock().unwrap();
+        reset_key();
+        // an empty config value means "not configured": a random key is
+        // generated on first use instead of a hardcoded fallback
+        set_jwt_key_from_config("").expect("empty = not configured");
+        let k = get_jwt_key().unwrap();
+        assert!(!k.is_empty());
+        assert_ne!(k, "some-secret");
+        reset_key();
+    }
+
+    #[test]
+    fn set_jwt_key_from_config_blank_is_fatal() {
+        let _g = KEY_TEST_LOCK.lock().unwrap();
+        reset_key();
+        assert!(set_jwt_key_from_config("  ").is_err());
+        reset_key();
+    }
+
+    #[test]
+    fn make_jwt_roundtrips_through_check() {
+        let _g = KEY_TEST_LOCK.lock().unwrap();
+        reset_key();
+        let token = make_jwt(7).expect("sign with generated key");
+        let claims = AdminJwtClaims::check(&token).expect("token verifies with same key");
+        assert_eq!(claims.user, 7);
+        assert!(claims.exp > 0);
+        reset_key();
+    }
+
+    #[test]
+    fn check_rejects_token_signed_with_different_key() {
+        let _g = KEY_TEST_LOCK.lock().unwrap();
+        reset_key();
+        let token = make_jwt(7).unwrap();
+        set_jwt_key("another-key").unwrap();
+        match AdminJwtClaims::check(&token) {
+            Err(JwtError::Invalid) => {}
+            Err(e) => panic!("expected Invalid, got JwtError::{:?}", e),
+            Ok(_) => panic!("expected Invalid, got a valid token"),
+        }
+        reset_key();
+    }
+}

src/bin/cln/mod.rs

@@ -30,7 +30,7 @@ pub async fn main() -> Result<()> {
     let stack = make_stack();
     log::info!("STACK {:?}", stack);
 
-    sphinx_swarm::auth::set_jwt_key(&stack.jwt_key);
+    sphinx_swarm::auth::set_jwt_key_from_config(&stack.jwt_key)?;
     handler::hydrate_stack(stack.clone()).await;
 
     let (tx, rx) = mpsc::channel::<CmdRequest>(1000);

src/bin/cln_mainnet_test.rs

@@ -21,7 +21,7 @@ pub async fn main() -> Result<()> {
     let stack = make_stack();
     let clients = builder::build_stack(proj, &docker, &stack).await?;
 
-    sphinx_swarm::auth::set_jwt_key(&stack.jwt_key);
+    sphinx_swarm::auth::set_jwt_key_from_config(&stack.jwt_key)?;
 
     handler::hydrate(stack, clients).await;
 

src/bin/stack/mod.rs

@@ -37,7 +37,9 @@ async fn main() -> Result<()> {
     }
 
     // put the jwt key into a var
-    sphinx_swarm::auth::set_jwt_key(&stack.jwt_key);
+    // (an empty config value means "not configured": a random key is generated
+    // at boot; a configured-but-blank key fails startup)
+    sphinx_swarm::auth::set_jwt_key_from_config(&stack.jwt_key)?;
     // hydrate the "stack" without clients
     handler::hydrate_stack(stack.clone()).await;
 

src/bin/super/mod.rs

@@ -69,7 +69,7 @@ async fn main() -> Result<()> {
     let s: state::Super = load_config_file(project).await.expect("YAML CONFIG FAIL");
     log::info!("SUPER!!! {:?}", s);
 
-    sphinx_swarm::auth::set_jwt_key(&s.jwt_key);
+    sphinx_swarm::auth::set_jwt_key_from_config(&s.jwt_key)?;
 
     state::hydrate(s).await;
 
@@ -170,7 +170,14 @@ fn access(cmd: &Cmd, state: &Super, user_id: &Option<u32>) -> bool {
     if user.is_none() {
         return false;
     }
-
+    // self-service commands: the payload's user_id must be the authenticated
+    // caller — never let it target another user's record (IDOR guard)
+    if let Cmd::Swarm(c) = cmd {
+        match c {
+            SwarmCmd::ChangePassword(cp) => return cp.user_id == user_id,
+            _ => {}
+        }
+    }
     return match user.unwrap().role {
         Role::Super => true,
         Role::Admin => false,

src/bin/tome/mod.rs

@@ -23,7 +23,7 @@ pub async fn main() -> Result<()> {
     let stack = make_stack();
     log::info!("STACK {:?}", stack);
 
-    sphinx_swarm::auth::set_jwt_key(&stack.jwt_key);
+    sphinx_swarm::auth::set_jwt_key_from_config(&stack.jwt_key)?;
     handler::hydrate_stack(stack.clone()).await;
 
     let (tx, rx) = mpsc::channel::<CmdRequest>(1000);

src/bin/v1/mod.rs

@@ -57,7 +57,7 @@ pub async fn main() -> Result<()> {
     // println!("{}", st);
     // return Ok(());
 
-    sphinx_swarm::auth::set_jwt_key(&stack.jwt_key);
+    sphinx_swarm::auth::set_jwt_key_from_config(&stack.jwt_key)?;
     handler::hydrate_stack(stack.clone()).await;
 
     let (tx, rx) = mpsc::channel::<CmdRequest>(1000);