From b2fe02c2d53b760649c67d58ee35d5d46c7bc4b5 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 16 Jun 2023 09:25:15 +0200 Subject: [PATCH] fix(relay): treat `stamp_secret` as string (#1660) Previously, the relay would treat the `stamp_secret` internally as bytes and share it with the outside world as hex-string. The portal however treats it as an opaque string and uses the UTF-8 bytes to create username and password. This patch aligns the relay's functionality with the portal and stores the `stamp_secret` internally as a string. --- rust/relay/src/auth.rs | 37 ++++++++++++------------- rust/relay/src/main.rs | 2 +- rust/relay/src/server.rs | 8 +++--- rust/relay/src/server/client_message.rs | 6 ++-- rust/relay/tests/regression.rs | 12 ++++---- 5 files changed, 31 insertions(+), 34 deletions(-) diff --git a/rust/relay/src/auth.rs b/rust/relay/src/auth.rs index af4230999..3733988fc 100644 --- a/rust/relay/src/auth.rs +++ b/rust/relay/src/auth.rs @@ -14,11 +14,11 @@ use uuid::Uuid; pub static FIREZONE: Lazy = Lazy::new(|| Realm::new("firezone".to_owned()).unwrap()); pub trait MessageIntegrityExt { - fn verify(&self, relay_secret: &[u8], username: &str, now: SystemTime) -> Result<(), Error>; + fn verify(&self, relay_secret: &str, username: &str, now: SystemTime) -> Result<(), Error>; } impl MessageIntegrityExt for MessageIntegrity { - fn verify(&self, relay_secret: &[u8], username: &str, now: SystemTime) -> Result<(), Error> { + fn verify(&self, relay_secret: &str, username: &str, now: SystemTime) -> Result<(), Error> { let (expiry_unix_timestamp, salt) = split_username(username)?; let expired = systemtime_from_unix(expiry_unix_timestamp); @@ -102,7 +102,7 @@ pub(crate) fn split_username(username: &str) -> Result<(u64, &str), Error> { } pub(crate) fn generate_password( - relay_secret: &[u8], + relay_secret: &str, expiry: SystemTime, username_salt: &str, ) -> String { @@ -134,24 +134,21 @@ pub(crate) fn systemtime_from_unix(seconds: u64) -> SystemTime { mod tests { use super::*; use crate::Attribute; - use hex_literal::hex; use stun_codec::rfc5389::attributes::Username; use stun_codec::rfc5389::methods::BINDING; use stun_codec::{Message, MessageClass, TransactionId}; - const RELAY_SECRET_1: [u8; 32] = - hex!("4c98bf59c99b3e467ecd7cf9d6b3e5279645fca59be67bc5bb4af3cf653761ab"); - const RELAY_SECRET_2: [u8; 32] = - hex!("7e35e34801e766a6a29ecb9e22810ea4e3476c2b37bf75882edf94a68b1d9607"); + const RELAY_SECRET_1: &str = "4c98bf59c99b3e467ecd7cf9d6b3e5279645fca59be67bc5bb4af3cf653761ab"; + const RELAY_SECRET_2: &str = "7e35e34801e766a6a29ecb9e22810ea4e3476c2b37bf75882edf94a68b1d9607"; const SAMPLE_USERNAME: &str = "n23JJ2wKKtt30oXi"; #[test] fn generate_password_test_vector() { let expiry = systemtime_from_unix(60 * 60 * 24 * 365 * 60); - let password = generate_password(&RELAY_SECRET_1, expiry, SAMPLE_USERNAME); + let password = generate_password(RELAY_SECRET_1, expiry, SAMPLE_USERNAME); - assert_eq!(password, "XnR4dOjSrxVx+3PR5/XIFKA80NckB04N7ndZMM6aoQg") + assert_eq!(password, "00hqldgk5xLeKKOB+xls9mHMVtgqzie9DulfgQwMv68") } #[test] @@ -159,7 +156,7 @@ mod tests { let expiry = systemtime_from_unix(1685984278); let password = generate_password( - "1cab293a-4032-46f4-862a-40e5d174b0d2".as_bytes(), + "1cab293a-4032-46f4-862a-40e5d174b0d2", expiry, "uvdgKvS9GXYZ_vmv", ); @@ -169,10 +166,10 @@ mod tests { #[test] fn smoke() { - let message_integrity = message_integrity(&RELAY_SECRET_1, 1685200000, "n23JJ2wKKtt30oXi"); + let message_integrity = message_integrity(RELAY_SECRET_1, 1685200000, "n23JJ2wKKtt30oXi"); let result = message_integrity.verify( - &RELAY_SECRET_1, + RELAY_SECRET_1, "1685200000:n23JJ2wKKtt30oXi", systemtime_from_unix(1685200000 - 1000), ); @@ -183,10 +180,10 @@ mod tests { #[test] fn expired_is_not_valid() { let message_integrity = - message_integrity(&RELAY_SECRET_1, 1685200000 - 1000, "n23JJ2wKKtt30oXi"); + message_integrity(RELAY_SECRET_1, 1685200000 - 1000, "n23JJ2wKKtt30oXi"); let result = message_integrity.verify( - &RELAY_SECRET_1, + RELAY_SECRET_1, "1685199000:n23JJ2wKKtt30oXi", systemtime_from_unix(1685200000), ); @@ -196,10 +193,10 @@ mod tests { #[test] fn different_relay_secret_makes_password_invalid() { - let message_integrity = message_integrity(&RELAY_SECRET_2, 1685200000, "n23JJ2wKKtt30oXi"); + let message_integrity = message_integrity(RELAY_SECRET_2, 1685200000, "n23JJ2wKKtt30oXi"); let result = message_integrity.verify( - &RELAY_SECRET_1, + RELAY_SECRET_1, "1685200000:n23JJ2wKKtt30oXi", systemtime_from_unix(168520000 + 1000), ); @@ -209,10 +206,10 @@ mod tests { #[test] fn invalid_username_format_fails() { - let message_integrity = message_integrity(&RELAY_SECRET_2, 1685200000, "n23JJ2wKKtt30oXi"); + let message_integrity = message_integrity(RELAY_SECRET_2, 1685200000, "n23JJ2wKKtt30oXi"); let result = message_integrity.verify( - &RELAY_SECRET_1, + RELAY_SECRET_1, "foobar", systemtime_from_unix(168520000 + 1000), ); @@ -249,7 +246,7 @@ mod tests { } fn message_integrity( - relay_secret: &[u8], + relay_secret: &str, username_expiry: u64, username_salt: &str, ) -> MessageIntegrity { diff --git a/rust/relay/src/main.rs b/rust/relay/src/main.rs index 57b9a3a26..4dbc3e2bd 100644 --- a/rust/relay/src/main.rs +++ b/rust/relay/src/main.rs @@ -46,7 +46,7 @@ async fn main() -> Result<()> { ) .init(); - let mut server = Server::new(args.public_ip4_addr, make_rng(args.rng_seed)); + let server = Server::new(args.public_ip4_addr, make_rng(args.rng_seed)); tracing::info!("Relay auth secret: {}", hex::encode(server.auth_secret())); diff --git a/rust/relay/src/server.rs b/rust/relay/src/server.rs index fdc5c02c2..d25d9e03c 100644 --- a/rust/relay/src/server.rs +++ b/rust/relay/src/server.rs @@ -59,7 +59,7 @@ pub struct Server { rng: R, - auth_secret: [u8; 32], + auth_secret: String, nonces: Nonces, @@ -142,15 +142,15 @@ where channel_numbers_by_peer: Default::default(), pending_commands: Default::default(), next_allocation_id: AllocationId(1), - auth_secret: rng.gen(), + auth_secret: hex::encode(rng.gen::<[u8; 32]>()), rng, time_events: TimeEvents::default(), nonces: Default::default(), } } - pub fn auth_secret(&mut self) -> [u8; 32] { - self.auth_secret + pub fn auth_secret(&self) -> &str { + &self.auth_secret } /// Registers a new, valid nonce. diff --git a/rust/relay/src/server/client_message.rs b/rust/relay/src/server/client_message.rs index 6c3397486..7a5e4568b 100644 --- a/rust/relay/src/server/client_message.rs +++ b/rust/relay/src/server/client_message.rs @@ -115,7 +115,7 @@ impl Allocate { transaction_id: TransactionId, lifetime: Option, username: Username, - relay_secret: &[u8], + relay_secret: &str, nonce: Uuid, ) -> Self { let requested_transport = RequestedTransport::new(UDP_TRANSPORT); @@ -233,7 +233,7 @@ impl Refresh { transaction_id: TransactionId, lifetime: Option, username: Username, - relay_secret: &[u8], + relay_secret: &str, nonce: Uuid, ) -> Self { let nonce = Nonce::new(nonce.as_hyphenated().to_string()).expect("len(uuid) < 128"); @@ -316,7 +316,7 @@ impl ChannelBind { channel_number: ChannelNumber, xor_peer_address: XorPeerAddress, username: Username, - relay_secret: &[u8], + relay_secret: &str, nonce: Uuid, ) -> Self { let nonce = Nonce::new(nonce.as_hyphenated().to_string()).expect("len(uuid) < 128"); diff --git a/rust/relay/tests/regression.rs b/rust/relay/tests/regression.rs index ca5044bfd..624840ed9 100644 --- a/rust/relay/tests/regression.rs +++ b/rust/relay/tests/regression.rs @@ -58,7 +58,7 @@ fn deallocate_once_time_expired( transaction_id, Some(lifetime.clone()), valid_username(now, &username_salt), - &secret, + secret, nonce, ), now, @@ -92,7 +92,7 @@ fn unauthenticated_allocate_triggers_authentication( let first_nonce = Uuid::from_u128(0x0); let mut server = TestServer::new(public_relay_addr); - let secret = server.auth_secret(); + let secret = server.auth_secret().to_owned(); server.assert_commands( from_client( @@ -142,7 +142,7 @@ fn when_refreshed_in_time_allocation_does_not_expire( #[strategy(relay::proptest::nonce())] nonce: Uuid, ) { let mut server = TestServer::new(public_relay_addr).with_nonce(nonce); - let secret = server.auth_secret(); + let secret = server.auth_secret().to_owned(); let first_wake = now + allocate_lifetime.lifetime(); server.assert_commands( @@ -218,7 +218,7 @@ fn when_receiving_lifetime_0_for_existing_allocation_then_delete( #[strategy(relay::proptest::nonce())] nonce: Uuid, ) { let mut server = TestServer::new(public_relay_addr).with_nonce(nonce); - let secret = server.auth_secret(); + let secret = server.auth_secret().to_owned(); let first_wake = now + allocate_lifetime.lifetime(); server.assert_commands( @@ -303,7 +303,7 @@ fn ping_pong_relay( let _ = env_logger::try_init(); let mut server = TestServer::new(public_relay_addr).with_nonce(nonce); - let secret = server.auth_secret(); + let secret = server.auth_secret().to_owned(); server.assert_commands( from_client( @@ -393,7 +393,7 @@ impl TestServer { self } - fn auth_secret(&mut self) -> [u8; 32] { + fn auth_secret(&self) -> &str { self.server.auth_secret() }