From 8feda7990c90c501cefa7044c0a040189ba4c2f6 Mon Sep 17 00:00:00 2001 From: CleverWild Date: Sun, 29 Mar 2026 23:05:38 +0200 Subject: [PATCH] fix(auth): reject invalid challenge signatures instead of transitioning to AuthOk --- .../src/actors/user_agent/auth/state.rs | 13 ++-- .../arbiter-server/tests/user_agent/auth.rs | 66 +++++++++++++++++++ 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/server/crates/arbiter-server/src/actors/user_agent/auth/state.rs b/server/crates/arbiter-server/src/actors/user_agent/auth/state.rs index c422589..eea0661 100644 --- a/server/crates/arbiter-server/src/actors/user_agent/auth/state.rs +++ b/server/crates/arbiter-server/src/actors/user_agent/auth/state.rs @@ -210,13 +210,16 @@ where } }; - if valid { - self.transport - .send(Ok(Outbound::AuthSuccess)) - .await - .map_err(|_| Error::Transport)?; + if !valid { + error!("Invalid challenge solution signature"); + return Err(Error::InvalidChallengeSolution); } + self.transport + .send(Ok(Outbound::AuthSuccess)) + .await + .map_err(|_| Error::Transport)?; + Ok(key.clone()) } } diff --git a/server/crates/arbiter-server/tests/user_agent/auth.rs b/server/crates/arbiter-server/tests/user_agent/auth.rs index 285ddcf..3fd8c92 100644 --- a/server/crates/arbiter-server/tests/user_agent/auth.rs +++ b/server/crates/arbiter-server/tests/user_agent/auth.rs @@ -165,3 +165,69 @@ pub async fn test_challenge_auth() { task.await.unwrap().unwrap(); } + +#[tokio::test] +#[test_log::test] +pub async fn test_challenge_auth_rejects_invalid_signature() { + let db = db::create_test_pool().await; + let actors = GlobalActors::spawn(db.clone()).await.unwrap(); + + let new_key = ed25519_dalek::SigningKey::generate(&mut rand::rng()); + let pubkey_bytes = new_key.verifying_key().to_bytes().to_vec(); + + // Pre-register key with key_type + { + let mut conn = db.get().await.unwrap(); + insert_into(schema::useragent_client::table) + .values(( + schema::useragent_client::public_key.eq(pubkey_bytes.clone()), + schema::useragent_client::key_type.eq(1i32), + )) + .execute(&mut conn) + .await + .unwrap(); + } + + let (server_transport, mut test_transport) = ChannelTransport::new(); + let db_for_task = db.clone(); + let task = tokio::spawn(async move { + let mut props = UserAgentConnection::new(db_for_task, actors); + auth::authenticate(&mut props, server_transport).await + }); + + test_transport + .send(auth::Inbound::AuthChallengeRequest { + pubkey: AuthPublicKey::Ed25519(new_key.verifying_key()), + bootstrap_token: None, + }) + .await + .unwrap(); + + let response = test_transport + .recv() + .await + .expect("should receive challenge"); + let challenge = match response { + Ok(resp) => match resp { + auth::Outbound::AuthChallenge { nonce } => nonce, + other => panic!("Expected AuthChallenge, got {other:?}"), + }, + Err(err) => panic!("Expected Ok response, got Err({err:?})"), + }; + + // Sign a different challenge value so signature format is valid but verification must fail. + let wrong_challenge = arbiter_proto::format_challenge(challenge + 1, &pubkey_bytes); + let signature = new_key.sign(&wrong_challenge); + + test_transport + .send(auth::Inbound::AuthChallengeSolution { + signature: signature.to_bytes().to_vec(), + }) + .await + .unwrap(); + + assert!(matches!( + task.await.unwrap(), + Err(auth::Error::InvalidChallengeSolution) + )); +}