fix(auth): reject invalid challenge signatures instead of transitioning to AuthOk
This commit is contained in:
@@ -210,13 +210,16 @@ where
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
if valid {
|
if !valid {
|
||||||
self.transport
|
error!("Invalid challenge solution signature");
|
||||||
.send(Ok(Outbound::AuthSuccess))
|
return Err(Error::InvalidChallengeSolution);
|
||||||
.await
|
|
||||||
.map_err(|_| Error::Transport)?;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
self.transport
|
||||||
|
.send(Ok(Outbound::AuthSuccess))
|
||||||
|
.await
|
||||||
|
.map_err(|_| Error::Transport)?;
|
||||||
|
|
||||||
Ok(key.clone())
|
Ok(key.clone())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -165,3 +165,69 @@ pub async fn test_challenge_auth() {
|
|||||||
|
|
||||||
task.await.unwrap().unwrap();
|
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)
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user