From 2fe8dfdab775f2eaf5b791ffb5d9b74049167636 Mon Sep 17 00:00:00 2001 From: oxalica Date: Sun, 22 Sep 2024 10:54:34 -0400 Subject: [PATCH] test: add more tests for register verification --- blah-types/src/identity.rs | 163 ++++++++++++++++++++++++++++++++++--- blahd/src/lib.rs | 4 +- blahd/tests/webapi.rs | 138 +++++++++++++++++++++++-------- 3 files changed, 260 insertions(+), 45 deletions(-) diff --git a/blah-types/src/identity.rs b/blah-types/src/identity.rs index 4c3c334..741385e 100644 --- a/blah-types/src/identity.rs +++ b/blah-types/src/identity.rs @@ -46,6 +46,11 @@ impl UserIdentityDesc { /// Validate signatures of the identity description at given time. pub fn verify(&self, id_url: Option<&IdUrl>, now_timestamp: u64) -> Result<(), VerifyError> { + if let Some(id_url) = id_url { + if !self.profile.signee.payload.id_urls.contains(id_url) { + return Err(VerifyErrorImpl::MissingIdUrl.into()); + } + } if self.id_key != self.profile.signee.user.id_key { return Err(VerifyErrorImpl::ProfileIdKeyMismatch.into()); } @@ -75,11 +80,6 @@ impl UserIdentityDesc { self.profile .verify() .map_err(VerifyErrorImpl::ProfileSignature)?; - if let Some(id_url) = id_url { - if !self.profile.signee.payload.id_urls.contains(id_url) { - return Err(VerifyErrorImpl::MissingIdUrl.into()); - } - } Ok(()) } } @@ -138,7 +138,7 @@ impl ops::Deref for IdUrl { #[derive(Debug, Clone, PartialEq, Eq, Error)] #[non_exhaustive] pub enum IdUrlError { - #[error(transparent)] + #[error("invalid id-URL: {0}")] ParseUrl(#[from] url::ParseError), #[error("id-URL too long")] TooLong, @@ -198,16 +198,21 @@ impl FromStr for IdUrl { #[cfg(test)] mod tests { + use ed25519_dalek::SigningKey; + + use crate::SignExt; + use super::*; #[test] fn parse_id_url() { let parse = ::parse::; - assert!(matches!( - parse("not-a-url").unwrap_err(), - IdUrlError::ParseUrl(_) - )); + // Error message. + assert_eq!( + parse("not-a-url").unwrap_err().to_string(), + "invalid id-URL: relative URL without a base", + ); macro_rules! check_err { ($($s:expr, $err:expr;)*) => { @@ -241,4 +246,142 @@ mod tests { assert_eq!(parse("https://example.com").unwrap(), expect); assert_eq!(parse("https://:@example.com").unwrap(), expect); } + + #[test] + fn id_desc_verify() { + const TIMESTAMP: u64 = 42; + + let rng = &mut rand::rngs::mock::StepRng::new(42, 1); + let id_url = "https://example.com".parse::().unwrap(); + let id_priv = SigningKey::from_bytes(&[42; 32]); + let id_key = PubKey::from(id_priv.verifying_key()); + let mut id_desc = UserIdentityDesc { + id_key: id_key.clone(), + act_keys: Vec::new(), + profile: UserProfile { + preferred_chat_server_urls: Vec::new(), + id_urls: vec![id_url], + } + .sign_msg_with(&id_key, &id_priv, TIMESTAMP, rng) + .unwrap(), + }; + + macro_rules! assert_err { + ($ret:expr, $pat:pat $(,)?) => {{ + let err: VerifyError = $ret.unwrap_err(); + assert!( + matches!(err.0, $pat), + "unexpected result, got: {err:?}, expect: {}", + stringify!($pat), + ) + }}; + } + + // Mismatch. + assert_err!( + id_desc.verify(Some(&"https://not-example.com".parse().unwrap()), TIMESTAMP), + VerifyErrorImpl::MissingIdUrl, + ); + assert_err!( + id_desc.verify(None, TIMESTAMP), + VerifyErrorImpl::ProfileSigner, + ); + + // Ok. + id_desc.act_keys.push( + UserActKeyDesc { + act_key: id_key.clone(), + expire_time: TIMESTAMP + 1, + comment: String::new(), + } + .sign_msg_with(&id_key, &id_priv, TIMESTAMP, rng) + .unwrap(), + ); + id_desc.verify(None, TIMESTAMP).unwrap(); + + // Expired. + assert_err!( + id_desc.verify(None, TIMESTAMP + 2), + VerifyErrorImpl::ProfileSigner, + ); + + let act_priv = SigningKey::from_bytes(&[24; 32]); + let act_pub = PubKey::from(act_priv.verifying_key()); + id_desc.act_keys.push( + UserActKeyDesc { + act_key: act_pub.clone(), + expire_time: TIMESTAMP + 1, + comment: String::new(), + } + // Self-signed. + .sign_msg_with(&id_key, &act_priv, TIMESTAMP, rng) + .unwrap(), + ); + assert_err!( + id_desc.verify(None, TIMESTAMP), + VerifyErrorImpl::ActKeySigner(1), + ); + + id_desc.act_keys[1] = UserActKeyDesc { + act_key: act_pub.clone(), + expire_time: TIMESTAMP + 1, + comment: String::new(), + } + // Wrong id_key. + .sign_msg_with(&act_pub, &act_priv, TIMESTAMP, rng) + .unwrap(); + assert_err!( + id_desc.verify(None, TIMESTAMP), + VerifyErrorImpl::ActKeySigner(1), + ); + + // OK act key. + id_desc.act_keys[1] = UserActKeyDesc { + act_key: act_pub.clone(), + expire_time: TIMESTAMP + 1, + comment: String::new(), + } + .sign_msg_with(&id_key, &id_priv, TIMESTAMP, rng) + .unwrap(); + id_desc.verify(None, TIMESTAMP).unwrap(); + + // Profile id_key mismatch. + id_desc.profile = id_desc + .profile + .signee + .payload + .sign_msg(&act_pub, &act_priv) + .unwrap(); + assert_err!( + id_desc.verify(None, TIMESTAMP), + VerifyErrorImpl::ProfileIdKeyMismatch, + ); + + // OK, signed by act key. + id_desc.profile = id_desc + .profile + .signee + .payload + .sign_msg(&id_key, &act_priv) + .unwrap(); + id_desc.verify(None, TIMESTAMP).unwrap(); + + // Invalid signature. + id_desc.profile.sig[0] = 0; + assert_err!( + id_desc.verify(None, TIMESTAMP), + VerifyErrorImpl::ProfileSignature(_), + ); + id_desc.act_keys[1].sig[0] = 0; + assert_err!( + id_desc.verify(None, TIMESTAMP), + VerifyErrorImpl::ActKeySignature(1, _), + ); + + // Error message. + assert_eq!( + id_desc.verify(None, TIMESTAMP).unwrap_err().to_string(), + "invalid act_key[1] signature: signature error: Verification equation was not satisfied", + ); + } } diff --git a/blahd/src/lib.rs b/blahd/src/lib.rs index 9c926ca..c639517 100644 --- a/blahd/src/lib.rs +++ b/blahd/src/lib.rs @@ -36,7 +36,7 @@ mod id; mod register; mod utils; -pub use database::Database; +pub use database::{Config as DatabaseConfig, Database}; pub use middleware::ApiError; #[serde_inline_default] @@ -202,7 +202,7 @@ async fn user_get( async fn user_register( State(st): ArcState, SignedJson(msg): SignedJson, -) -> impl IntoResponse { +) -> Result { register::user_register(&st, msg).await } diff --git a/blahd/tests/webapi.rs b/blahd/tests/webapi.rs index 7c0b49e..0cd9f3d 100644 --- a/blahd/tests/webapi.rs +++ b/blahd/tests/webapi.rs @@ -7,7 +7,7 @@ use std::future::{Future, IntoFuture}; use std::sync::{Arc, LazyLock}; use std::time::{Duration, Instant}; -use anyhow::Result; +use anyhow::{Context, Result}; use axum::http::HeaderMap; use blah_types::identity::{IdUrl, UserActKeyDesc, UserIdentityDesc, UserProfile}; use blah_types::{ @@ -190,7 +190,8 @@ impl Server { struct Resp { error: ApiError, } - let Resp { mut error } = serde_json::from_str(&resp_str)?; + let Resp { mut error } = serde_json::from_str(&resp_str) + .with_context(|| format!("failed to parse response {resp_str:?}"))?; error.status = status; Err(ApiErrorWithHeaders { error, headers }.into()) } else if resp_str.is_empty() { @@ -320,6 +321,34 @@ impl Server { Ok(WithMsgId { cid, msg }) } } + + async fn get_me(&self, auth_user: Option<&User>) -> Result<(), Option<(u32, u8)>> { + let auth = auth_user.map(auth); + match self + .request::<(), NoContent>(Method::GET, "/user/me", auth.as_deref(), None) + .await + { + Ok(None) => Ok(()), + Err(err) => { + let err = err.downcast::().unwrap(); + assert_eq!(err.error.status, StatusCode::NOT_FOUND); + if !err.headers.contains_key(X_BLAH_NONCE) { + return Err(None); + } + let challenge_nonce = err.headers[X_BLAH_NONCE] + .to_str() + .unwrap() + .parse::() + .unwrap(); + let difficulty = err.headers[X_BLAH_DIFFICULTY] + .to_str() + .unwrap() + .parse::() + .unwrap(); + Err(Some((challenge_nonce, difficulty))) + } + } + } } #[fixture] @@ -359,15 +388,19 @@ fn server() -> Server { } } let db = Database::from_raw(conn).unwrap(); + server_with(db, &CONFIG) +} +// TODO: Testing config is hard to build because it does have a `Default` impl. +#[track_caller] +fn server_with(db: Database, config: &dyn Fn(u16) -> String) -> Server { // Use std's to avoid async, since we need no name resolution. let listener = std::net::TcpListener::bind(format!("{LOCALHOST}:0")).unwrap(); listener.set_nonblocking(true).unwrap(); let port = listener.local_addr().unwrap().port(); let listener = TcpListener::from_std(listener).unwrap(); - // TODO: Testing config is hard to build because it does have a `Default` impl. - let config = toml::from_str(&CONFIG(port)).unwrap(); + let config = toml::from_str(&config(port)).unwrap(); let st = AppState::new(db, config); let router = blahd::router(Arc::new(st)); @@ -909,7 +942,7 @@ async fn delete_room(server: Server, #[case] peer_chat: bool, #[case] alice_dele #[rstest] #[tokio::test] -async fn register(server: Server) { +async fn register_flow(server: Server) { let rid = server .create_room( &ALICE, @@ -919,44 +952,22 @@ async fn register(server: Server) { .await .unwrap(); - let get_me = |user: Option<&User>| { - let auth = user.map(auth); - server - .request::<(), ()>(Method::GET, "/user/me", auth.as_deref(), None) - .map_ok(|_| ()) - .map_err(|err| { - let err = err.downcast::().unwrap(); - assert_eq!(err.error.status, StatusCode::NOT_FOUND); - let challenge_nonce = err.headers[X_BLAH_NONCE] - .to_str() - .unwrap() - .parse::() - .unwrap(); - let difficulty = err.headers[X_BLAH_DIFFICULTY] - .to_str() - .unwrap() - .parse::() - .unwrap(); - (challenge_nonce, difficulty) - }) - }; - // Alice is registered. - get_me(Some(&ALICE)).await.unwrap(); + server.get_me(Some(&ALICE)).await.unwrap(); // Carol is not registered. - let (challenge_nonce, diff) = get_me(Some(&CAROL)).await.unwrap_err(); + let (challenge_nonce, diff) = server.get_me(Some(&CAROL)).await.unwrap_err().unwrap(); assert_eq!(diff, REGISTER_DIFFICULTY); // Without token. - let ret2 = get_me(None).await.unwrap_err(); + let ret2 = server.get_me(None).await.unwrap_err().unwrap(); assert_eq!(ret2, (challenge_nonce, diff)); let mut req = UserRegisterPayload { id_key: CAROL.pubkeys.id_key.clone(), // Invalid values. server_url: "http://localhost".parse().unwrap(), - id_url: "http://.".parse().unwrap(), + id_url: "http://com.".parse().unwrap(), challenge_nonce: challenge_nonce - 1, }; let register = |req: Signed| { @@ -1080,6 +1091,7 @@ async fn register(server: Server) { } .sign_msg(&CAROL.pubkeys.id_key, &CAROL.id_priv) .unwrap(); + // Incorrect URL, without port. let profile = sign_profile("https://localhost".parse().unwrap()); UserIdentityDesc { id_key: CAROL.pubkeys.id_key.clone(), @@ -1095,7 +1107,7 @@ async fn register(server: Server) { .expect_api_err(StatusCode::UNAUTHORIZED, "invalid_id_description"); // Still not registered. - get_me(Some(&CAROL)).await.unwrap_err(); + server.get_me(Some(&CAROL)).await.unwrap_err(); server .join_room(rid, &CAROL, MemberPermission::MAX_SELF_ADD) .await @@ -1107,13 +1119,73 @@ async fn register(server: Server) { register(sign_with_difficulty(&req, true)).await.unwrap(); // Registered now. - get_me(Some(&CAROL)).await.unwrap(); + server.get_me(Some(&CAROL)).await.unwrap(); server .join_room(rid, &CAROL, MemberPermission::MAX_SELF_ADD) .await .unwrap(); } +#[rstest] +#[case::disabled(false, true, true, true)] +#[case::no_http(true, false, true, true)] +#[case::no_port(true, true, false, true)] +#[case::no_single_label(true, true, true, false)] +#[tokio::test] +async fn register_config( + #[case] enabled: bool, + #[case] allow_http: bool, + #[case] allow_port: bool, + #[case] allow_single_label: bool, +) { + let config = |port| { + format!( + r#" +base_url="http://{LOCALHOST}:{port}" +[register] +enable_public = {enabled} +unsafe_allow_id_url_http = {allow_http} +unsafe_allow_id_url_custom_port = {allow_port} +unsafe_allow_id_url_single_label = {allow_single_label} + "# + ) + }; + + let db_config = blahd::DatabaseConfig { + in_memory: true, + ..Default::default() + }; + let server = server_with(Database::open(&db_config).unwrap(), &config); + + // Returns challenge headers only if registration is enabled. + let hdrs = server.get_me(Some(&CAROL)).await.unwrap_err(); + if enabled { + hdrs.unwrap(); + } else { + assert_eq!(hdrs, None); + } + + let server_url = format!("http://{}", server.domain()); + let req = server.sign( + &CAROL, + UserRegisterPayload { + id_key: CAROL.pubkeys.id_key.clone(), + // Unused values. + id_url: server_url.parse().unwrap(), + server_url: server_url.parse().unwrap(), + challenge_nonce: 0, + }, + ); + let ret = server + .request::<_, ()>(Method::POST, "/user/me", None, Some(req)) + .await; + if !enabled { + ret.expect_api_err(StatusCode::FORBIDDEN, "disabled"); + } else { + ret.expect_api_err(StatusCode::BAD_REQUEST, "invalid_id_url"); + } +} + #[rstest] #[tokio::test] async fn event(server: Server) {