feat(relay): implement nonces for authentication (#1654)

To complete the authentication scheme for the relay, we need to prompt
the client with a nonce when they send an unauthenticated request. The
semantic meaning of a nonce is opaque to the client. As a starting
point, we implement a count-based scheme. Each nonce is valid for 10
requests. After that, a request will be rejected with a 401 and the
client has to authenticate with a new nonce.

This scheme provides a basic form of replay-protection.

.github/workflows/rust.yml

@@ -30,7 +30,7 @@ jobs:
       - run: cargo doc --no-deps --document-private-items
         env:
           RUSTDOCFLAGS: "-D warnings"
-      - run: cargo clippy -- -D warnings
+      - run: cargo clippy --all-targets --all-features -- -D warnings
       - run: cargo test
 
   cross: # cross is separate from test because cross-compiling yields different artifacts and we cannot reuse the cache.

rust/Cargo.lock

@@ -1657,6 +1657,7 @@ dependencies = [
  "tokio",
  "tracing",
  "tracing-subscriber",
+ "uuid",
  "webrtc",
 ]
 

rust/relay/Cargo.toml

@@ -22,6 +22,7 @@ once_cell = "1.17.1"
 proptest = { version = "1.2.0", optional = true }
 test-strategy = "0.3.0"
 derive_more = { version = "0.99.17", features = ["from"] }
+uuid = { version = "1.3.3", features = ["v4"] }
 
 [dev-dependencies]
 webrtc = "0.7.2"

rust/relay/src/auth.rs

@@ -4,8 +4,11 @@ use once_cell::sync::Lazy;
 use sha2::digest::FixedOutput;
 use sha2::Sha256;
 use std::borrow::ToOwned;
+use std::collections::hash_map::Entry;
+use std::collections::HashMap;
 use std::time::{Duration, SystemTime};
 use stun_codec::rfc5389::attributes::{MessageIntegrity, Realm, Username};
+use uuid::Uuid;
 
 // TODO: Upstream a const constructor to `stun-codec`.
 pub static FIREZONE: Lazy<Realm> = Lazy::new(|| Realm::new("firezone".to_owned()).unwrap());
@@ -37,11 +40,53 @@ impl MessageIntegrityExt for MessageIntegrity {
     }
 }
 
+/// Tracks valid nonces for the TURN relay.
+///
+/// The semantic nature of nonces is an implementation detail of the relay in TURN.
+/// This could just as easily also be a time-based logic (i.e. nonces are valid for 10min).
+///
+/// For simplicity reasons, we use a count-based strategy.
+/// Each nonce can be used for a certain number of requests before it is invalid.
+#[derive(Default)]
+pub struct Nonces {
+    inner: HashMap<Uuid, u64>,
+}
+
+impl Nonces {
+    /// How many requests a client can perform with the same nonce.
+    const NUM_REQUESTS: u64 = 10;
+
+    pub fn add_new(&mut self, nonce: Uuid) {
+        self.inner.insert(nonce, Self::NUM_REQUESTS);
+    }
+
+    /// Record the usage of a nonce in a request.
+    pub fn handle_nonce_used(&mut self, nonce: Uuid) -> Result<(), Error> {
+        let mut entry = match self.inner.entry(nonce) {
+            Entry::Vacant(_) => return Err(Error::InvalidNonce),
+            Entry::Occupied(entry) => entry,
+        };
+
+        let remaining_requests = entry.get_mut();
+
+        if *remaining_requests == 0 {
+            entry.remove();
+
+            return Err(Error::InvalidNonce);
+        }
+
+        *remaining_requests -= 1;
+
+        Ok(())
+    }
+}
+
 #[derive(Debug, PartialEq)]
 pub enum Error {
     Expired,
     InvalidPassword,
     InvalidUsername,
+    InvalidNonce,
 }
 
 pub(crate) fn split_username(username: &str) -> Result<(u64, &str), Error> {
@@ -98,7 +143,7 @@ mod tests {
         hex!("4c98bf59c99b3e467ecd7cf9d6b3e5279645fca59be67bc5bb4af3cf653761ab");
     const RELAY_SECRET_2: [u8; 32] =
         hex!("7e35e34801e766a6a29ecb9e22810ea4e3476c2b37bf75882edf94a68b1d9607");
-    const SAMPLE_USERNAME: &'static str = "n23JJ2wKKtt30oXi";
+    const SAMPLE_USERNAME: &str = "n23JJ2wKKtt30oXi";
 
     #[test]
     fn generate_password_test_vector() {
@@ -128,7 +173,7 @@ mod tests {
 
         let result = message_integrity.verify(
             &RELAY_SECRET_1,
-            &format!("1685200000:n23JJ2wKKtt30oXi"),
+            "1685200000:n23JJ2wKKtt30oXi",
             systemtime_from_unix(1685200000 - 1000),
         );
 
@@ -142,7 +187,7 @@ mod tests {
 
         let result = message_integrity.verify(
             &RELAY_SECRET_1,
-            &format!("1685199000:n23JJ2wKKtt30oXi"),
+            "1685199000:n23JJ2wKKtt30oXi",
             systemtime_from_unix(1685200000),
         );
 
@@ -155,7 +200,7 @@ mod tests {
 
         let result = message_integrity.verify(
             &RELAY_SECRET_1,
-            &format!("1685200000:n23JJ2wKKtt30oXi"),
+            "1685200000:n23JJ2wKKtt30oXi",
             systemtime_from_unix(168520000 + 1000),
         );
 
@@ -168,13 +213,41 @@ mod tests {
 
         let result = message_integrity.verify(
             &RELAY_SECRET_1,
-            &format!("foobar"),
+            "foobar",
             systemtime_from_unix(168520000 + 1000),
         );
 
         assert_eq!(result.unwrap_err(), Error::InvalidUsername)
     }
 
+    #[test]
+    fn nonces_are_valid_for_10_requests() {
+        let mut nonces = Nonces::default();
+        let nonce = Uuid::new_v4();
+
+        nonces.add_new(nonce);
+
+        for _ in 0..10 {
+            nonces.handle_nonce_used(nonce).unwrap();
+        }
+
+        assert_eq!(
+            nonces.handle_nonce_used(nonce).unwrap_err(),
+            Error::InvalidNonce
+        );
+    }
+
+    #[test]
+    fn unknown_nonces_are_invalid() {
+        let mut nonces = Nonces::default();
+        let nonce = Uuid::new_v4();
+
+        assert_eq!(
+            nonces.handle_nonce_used(nonce).unwrap_err(),
+            Error::InvalidNonce
+        );
+    }
+
     fn message_integrity(
         relay_secret: &[u8],
         username_expiry: u64,

rust/relay/src/proptest.rs

@@ -7,13 +7,14 @@ use std::ops::Add;
 use std::time::{Duration, SystemTime};
 use stun_codec::rfc5766::attributes::{ChannelNumber, Lifetime, RequestedTransport};
 use stun_codec::TransactionId;
+use uuid::Uuid;
 
 pub fn transaction_id() -> impl Strategy<Value = TransactionId> {
-    any::<[u8; 12]>().prop_map(|bytes| TransactionId::new(bytes))
+    any::<[u8; 12]>().prop_map(TransactionId::new)
 }
 
 pub fn binding() -> impl Strategy<Value = Binding> {
-    transaction_id().prop_map(|id| Binding::new(id))
+    transaction_id().prop_map(Binding::new)
 }
 
 pub fn udp_requested_transport() -> impl Strategy<Value = RequestedTransport> {
@@ -32,6 +33,10 @@ pub fn username_salt() -> impl Strategy<Value = String> {
     string_regex("[a-zA-Z0-9]{10}").unwrap()
 }
 
+pub fn nonce() -> impl Strategy<Value = Uuid> {
+    any::<u128>().prop_map(Uuid::from_u128)
+}
+
 /// We let "now" begin somewhere around 2000 up until 2100.
 pub fn now() -> impl Strategy<Value = SystemTime> {
     const YEAR: u64 = 60 * 60 * 24 * 365;

rust/relay/src/server.rs

@@ -6,7 +6,7 @@ pub use crate::server::client_message::{
     Allocate, Binding, ChannelBind, ClientMessage, CreatePermission, Refresh,
 };
 
-use crate::auth::{MessageIntegrityExt, FIREZONE};
+use crate::auth::{MessageIntegrityExt, Nonces, FIREZONE};
 use crate::rfc8656::PeerAddressFamilyMismatch;
 use crate::stun_codec_ext::{MessageClassExt, MethodExt};
 use crate::TimeEvents;
@@ -29,6 +29,8 @@ use stun_codec::rfc5766::attributes::{
 use stun_codec::rfc5766::errors::{AllocationMismatch, InsufficientCapacity};
 use stun_codec::rfc5766::methods::{ALLOCATE, CHANNEL_BIND, CREATE_PERMISSION, REFRESH};
 use stun_codec::{Message, MessageClass, MessageEncoder, Method, TransactionId};
+use tracing::log;
+use uuid::Uuid;
 
 /// A sans-IO STUN & TURN server.
 ///
@@ -59,6 +61,8 @@ pub struct Server<R> {
 
     auth_secret: [u8; 32],
 
+    nonces: Nonces,
+
     time_events: TimeEvents<TimedAction>,
 }
 
@@ -141,6 +145,7 @@ where
             auth_secret: rng.gen(),
             rng,
             time_events: TimeEvents::default(),
+            nonces: Default::default(),
         }
     }
 
@@ -148,6 +153,13 @@ where
         self.auth_secret
     }
 
+    /// Registers a new, valid nonce.
+    ///
+    /// Each nonce is valid for 10 requests.
+    pub fn add_nonce(&mut self, nonce: Uuid) {
+        self.nonces.add_new(nonce);
+    }
+
     /// Process the bytes received from a client.
     ///
     /// After calling this method, you should call [`Server::next_command`] until it returns `None`.
@@ -211,7 +223,11 @@ where
             .get_attribute::<ErrorCode>()
             .map_or(false, |error| error == &ErrorCode::from(Unauthorized))
         {
-            error_response.add_attribute(Nonce::new("foobar".to_owned()).unwrap().into()); // TODO: Implement proper nonce handling.
+            let new_nonce = Uuid::from_u128(self.rng.gen());
+
+            self.add_nonce(new_nonce);
+
+            error_response.add_attribute(Nonce::new(new_nonce.to_string()).unwrap().into());
             error_response.add_attribute((*FIREZONE).clone().into());
         }
 
@@ -603,12 +619,28 @@ where
         request: &(impl StunRequest + ProtectedRequest),
         now: SystemTime,
     ) -> Result<(), Message<Attribute>> {
-        request
+        let message_integrity = request
             .message_integrity()
-            .verify(&self.auth_secret, request.username().name(), now)
+            .map_err(|e| error_response(e, request))?;
+        let username = request.username().map_err(|e| error_response(e, request))?;
+        let nonce = request
+            .nonce()
+            .map_err(|e| error_response(e, request))?
+            .value()
+            .parse::<Uuid>()
+            .map_err(|e| {
+                log::debug!("failed to parse nonce: {e}");
+
+                error_response(Unauthorized, request)
+            })?;
+
+        self.nonces
+            .handle_nonce_used(nonce)
             .map_err(|_| error_response(Unauthorized, request))?;
 
-        // TODO: Check if nonce is valid.
+        message_integrity
+            .verify(&self.auth_secret, username.name(), now)
+            .map_err(|_| error_response(Unauthorized, request))?;
 
         Ok(())
     }
@@ -834,19 +866,24 @@ impl_stun_request_for!(Refresh, REFRESH);
 
 /// Private helper trait to make [`Server::verify_auth`] more ergonomic to use.
 trait ProtectedRequest {
-    fn message_integrity(&self) -> &MessageIntegrity;
-    fn username(&self) -> &Username;
+    fn message_integrity(&self) -> Result<&MessageIntegrity, Unauthorized>;
+    fn username(&self) -> Result<&Username, Unauthorized>;
+    fn nonce(&self) -> Result<&Nonce, Unauthorized>;
 }
 
 macro_rules! impl_protected_request_for {
     ($t:ty) => {
         impl ProtectedRequest for $t {
-            fn message_integrity(&self) -> &MessageIntegrity {
-                self.message_integrity()
+            fn message_integrity(&self) -> Result<&MessageIntegrity, Unauthorized> {
+                self.message_integrity().ok_or(Unauthorized)
             }
 
-            fn username(&self) -> &Username {
-                self.username()
+            fn username(&self) -> Result<&Username, Unauthorized> {
+                self.username().ok_or(Unauthorized)
+            }
+
+            fn nonce(&self) -> Result<&Nonce, Unauthorized> {
+                self.nonce().ok_or(Unauthorized)
             }
         }
     };

rust/relay/src/server/client_message.rs

@@ -5,14 +5,15 @@ use crate::Attribute;
 use bytecodec::DecodeExt;
 use std::io;
 use std::time::Duration;
-use stun_codec::rfc5389::attributes::{ErrorCode, MessageIntegrity, Username};
-use stun_codec::rfc5389::errors::{BadRequest, Unauthorized};
+use stun_codec::rfc5389::attributes::{ErrorCode, MessageIntegrity, Nonce, Username};
+use stun_codec::rfc5389::errors::BadRequest;
 use stun_codec::rfc5389::methods::BINDING;
 use stun_codec::rfc5766::attributes::{
     ChannelNumber, Lifetime, RequestedTransport, XorPeerAddress,
 };
 use stun_codec::rfc5766::methods::{ALLOCATE, CHANNEL_BIND, CREATE_PERMISSION, REFRESH};
 use stun_codec::{BrokenMessage, Message, MessageClass, TransactionId};
+use uuid::Uuid;
 
 /// The maximum lifetime of an allocation.
 const MAX_ALLOCATION_LIFETIME: Duration = Duration::from_secs(3600);
@@ -43,13 +44,13 @@ impl Decoder {
                     (ALLOCATE, Request) => {
                         Ok(Allocate::parse(&message).map(ClientMessage::Allocate))
                     }
-                    (REFRESH, Request) => Ok(Refresh::parse(&message).map(ClientMessage::Refresh)),
+                    (REFRESH, Request) => Ok(Ok(ClientMessage::Refresh(Refresh::parse(&message)))),
                     (CHANNEL_BIND, Request) => {
                         Ok(ChannelBind::parse(&message).map(ClientMessage::ChannelBind))
                     }
-                    (CREATE_PERMISSION, Request) => {
-                        Ok(CreatePermission::parse(&message).map(ClientMessage::CreatePermission))
-                    }
+                    (CREATE_PERMISSION, Request) => Ok(Ok(ClientMessage::CreatePermission(
+                        CreatePermission::parse(&message),
+                    ))),
                     (_, Request) => Ok(Err(bad_request(&message))),
                     (method, class) => {
                         Err(Error::DecodeStun(bytecodec::Error::from(io::Error::new(
@@ -102,25 +103,29 @@ impl Binding {
 
 pub struct Allocate {
     transaction_id: TransactionId,
-    message_integrity: MessageIntegrity,
+    message_integrity: Option<MessageIntegrity>,
     requested_transport: RequestedTransport,
     lifetime: Option<Lifetime>,
-    username: Username,
+    username: Option<Username>,
+    nonce: Option<Nonce>,
 }
 
 impl Allocate {
-    pub fn new_udp(
+    pub fn new_authenticated_udp(
         transaction_id: TransactionId,
         lifetime: Option<Lifetime>,
         username: Username,
         relay_secret: &[u8],
+        nonce: Uuid,
     ) -> Self {
         let requested_transport = RequestedTransport::new(UDP_TRANSPORT);
+        let nonce = Nonce::new(nonce.as_hyphenated().to_string()).expect("len(uuid) < 128");
 
         let mut message =
             Message::<Attribute>::new(MessageClass::Request, ALLOCATE, transaction_id);
         message.add_attribute(requested_transport.clone().into());
         message.add_attribute(username.clone().into());
+        message.add_attribute(nonce.clone().into());
 
         if let Some(lifetime) = &lifetime {
             message.add_attribute(lifetime.clone().into());
@@ -137,28 +142,48 @@ impl Allocate {
 
         Self {
             transaction_id,
-            message_integrity,
+            message_integrity: Some(message_integrity),
             requested_transport,
             lifetime,
-            username,
+            username: Some(username),
+            nonce: Some(nonce),
+        }
+    }
+
+    pub fn new_unauthenticated_udp(
+        transaction_id: TransactionId,
+        lifetime: Option<Lifetime>,
+    ) -> Self {
+        let requested_transport = RequestedTransport::new(UDP_TRANSPORT);
+
+        let mut message =
+            Message::<Attribute>::new(MessageClass::Request, ALLOCATE, transaction_id);
+        message.add_attribute(requested_transport.clone().into());
+
+        if let Some(lifetime) = &lifetime {
+            message.add_attribute(lifetime.clone().into());
+        }
+
+        Self {
+            transaction_id,
+            message_integrity: None,
+            requested_transport,
+            lifetime,
+            username: None,
+            nonce: None,
         }
     }
 
     pub fn parse(message: &Message<Attribute>) -> Result<Self, Message<Attribute>> {
         let transaction_id = message.transaction_id();
-        let message_integrity = message
-            .get_attribute::<MessageIntegrity>()
-            .ok_or(unauthorized(message))?
-            .clone();
+        let message_integrity = message.get_attribute::<MessageIntegrity>().cloned();
+        let nonce = message.get_attribute::<Nonce>().cloned();
         let requested_transport = message
             .get_attribute::<RequestedTransport>()
             .ok_or(bad_request(message))?
             .clone();
         let lifetime = message.get_attribute::<Lifetime>().cloned();
-        let username = message
-            .get_attribute::<Username>()
-            .ok_or(bad_request(message))?
-            .clone();
+        let username = message.get_attribute::<Username>().cloned();
 
         Ok(Allocate {
             transaction_id,
@@ -166,6 +191,7 @@ impl Allocate {
             requested_transport,
             lifetime,
             username,
+            nonce,
         })
     }
 
@@ -173,8 +199,8 @@ impl Allocate {
         self.transaction_id
     }
 
-    pub fn message_integrity(&self) -> &MessageIntegrity {
-        &self.message_integrity
+    pub fn message_integrity(&self) -> Option<&MessageIntegrity> {
+        self.message_integrity.as_ref()
     }
 
     pub fn requested_transport(&self) -> &RequestedTransport {
@@ -185,16 +211,21 @@ impl Allocate {
         compute_effective_lifetime(self.lifetime.as_ref())
     }
 
-    pub fn username(&self) -> &Username {
-        &self.username
+    pub fn username(&self) -> Option<&Username> {
+        self.username.as_ref()
+    }
+
+    pub fn nonce(&self) -> Option<&Nonce> {
+        self.nonce.as_ref()
     }
 }
 
 pub struct Refresh {
     transaction_id: TransactionId,
-    message_integrity: MessageIntegrity,
+    message_integrity: Option<MessageIntegrity>,
     lifetime: Option<Lifetime>,
-    username: Username,
+    username: Option<Username>,
+    nonce: Option<Nonce>,
 }
 
 impl Refresh {
@@ -203,9 +234,13 @@ impl Refresh {
         lifetime: Option<Lifetime>,
         username: Username,
         relay_secret: &[u8],
+        nonce: Uuid,
     ) -> Self {
+        let nonce = Nonce::new(nonce.as_hyphenated().to_string()).expect("len(uuid) < 128");
+
         let mut message = Message::<Attribute>::new(MessageClass::Request, REFRESH, transaction_id);
         message.add_attribute(username.clone().into());
+        message.add_attribute(nonce.clone().into());
 
         if let Some(lifetime) = &lifetime {
             message.add_attribute(lifetime.clone().into());
@@ -222,55 +257,57 @@ impl Refresh {
 
         Self {
             transaction_id,
-            message_integrity,
+            message_integrity: Some(message_integrity),
             lifetime,
-            username,
+            username: Some(username),
+            nonce: Some(nonce),
         }
     }
 
-    pub fn parse(message: &Message<Attribute>) -> Result<Self, Message<Attribute>> {
+    pub fn parse(message: &Message<Attribute>) -> Self {
         let transaction_id = message.transaction_id();
-        let message_integrity = message
-            .get_attribute::<MessageIntegrity>()
-            .ok_or(unauthorized(message))?
-            .clone();
+        let message_integrity = message.get_attribute::<MessageIntegrity>().cloned();
+        let nonce = message.get_attribute::<Nonce>().cloned();
         let lifetime = message.get_attribute::<Lifetime>().cloned();
-        let username = message
-            .get_attribute::<Username>()
-            .ok_or(bad_request(message))?
-            .clone();
+        let username = message.get_attribute::<Username>().cloned();
 
-        Ok(Refresh {
+        Refresh {
             transaction_id,
             message_integrity,
             lifetime,
             username,
-        })
+            nonce,
+        }
     }
 
     pub fn transaction_id(&self) -> TransactionId {
         self.transaction_id
     }
 
-    pub fn message_integrity(&self) -> &MessageIntegrity {
-        &self.message_integrity
+    pub fn message_integrity(&self) -> Option<&MessageIntegrity> {
+        self.message_integrity.as_ref()
     }
 
     pub fn effective_lifetime(&self) -> Lifetime {
         compute_effective_lifetime(self.lifetime.as_ref())
     }
 
-    pub fn username(&self) -> &Username {
-        &self.username
+    pub fn username(&self) -> Option<&Username> {
+        self.username.as_ref()
+    }
+
+    pub fn nonce(&self) -> Option<&Nonce> {
+        self.nonce.as_ref()
     }
 }
 
 pub struct ChannelBind {
     transaction_id: TransactionId,
     channel_number: ChannelNumber,
-    message_integrity: MessageIntegrity,
+    message_integrity: Option<MessageIntegrity>,
+    nonce: Option<Nonce>,
     xor_peer_address: XorPeerAddress,
-    username: Username,
+    username: Option<Username>,
 }
 
 impl ChannelBind {
@@ -280,12 +317,16 @@ impl ChannelBind {
         xor_peer_address: XorPeerAddress,
         username: Username,
         relay_secret: &[u8],
+        nonce: Uuid,
     ) -> Self {
+        let nonce = Nonce::new(nonce.as_hyphenated().to_string()).expect("len(uuid) < 128");
+
         let mut message =
             Message::<Attribute>::new(MessageClass::Request, CHANNEL_BIND, transaction_id);
         message.add_attribute(username.clone().into());
         message.add_attribute(channel_number.into());
         message.add_attribute(xor_peer_address.clone().into());
+        message.add_attribute(nonce.clone().into());
 
         let (expiry, salt) = split_username(username.name()).expect("a valid username");
         let expiry_systemtime = systemtime_from_unix(expiry);
@@ -299,9 +340,10 @@ impl ChannelBind {
         Self {
             transaction_id,
             channel_number,
-            message_integrity,
+            message_integrity: Some(message_integrity),
             xor_peer_address,
-            username,
+            username: Some(username),
+            nonce: Some(nonce),
         }
     }
 
@@ -311,14 +353,9 @@ impl ChannelBind {
             .get_attribute::<ChannelNumber>()
             .copied()
             .ok_or(bad_request(message))?;
-        let message_integrity = message
-            .get_attribute::<MessageIntegrity>()
-            .ok_or(unauthorized(message))?
-            .clone();
-        let username = message
-            .get_attribute::<Username>()
-            .ok_or(bad_request(message))?
-            .clone();
+        let message_integrity = message.get_attribute::<MessageIntegrity>().cloned();
+        let nonce = message.get_attribute::<Nonce>().cloned();
+        let username = message.get_attribute::<Username>().cloned();
         let xor_peer_address = message
             .get_attribute::<XorPeerAddress>()
             .ok_or(bad_request(message))?
@@ -328,6 +365,7 @@ impl ChannelBind {
             transaction_id,
             channel_number,
             message_integrity,
+            nonce,
             xor_peer_address,
             username,
         })
@@ -341,66 +379,59 @@ impl ChannelBind {
         self.channel_number
     }
 
-    pub fn message_integrity(&self) -> &MessageIntegrity {
-        &self.message_integrity
+    pub fn message_integrity(&self) -> Option<&MessageIntegrity> {
+        self.message_integrity.as_ref()
     }
 
     pub fn xor_peer_address(&self) -> &XorPeerAddress {
         &self.xor_peer_address
     }
 
-    pub fn username(&self) -> &Username {
-        &self.username
+    pub fn username(&self) -> Option<&Username> {
+        self.username.as_ref()
+    }
+
+    pub fn nonce(&self) -> Option<&Nonce> {
+        self.nonce.as_ref()
     }
 }
 
 pub struct CreatePermission {
     transaction_id: TransactionId,
-    message_integrity: MessageIntegrity,
-    username: Username,
+    message_integrity: Option<MessageIntegrity>,
+    username: Option<Username>,
+    nonce: Option<Nonce>,
 }
 
 impl CreatePermission {
-    pub fn new(
-        transaction_id: TransactionId,
-        message_integrity: MessageIntegrity,
-        username: Username,
-    ) -> Self {
-        Self {
-            transaction_id,
-            message_integrity,
-            username,
-        }
-    }
-
-    pub fn parse(message: &Message<Attribute>) -> Result<Self, Message<Attribute>> {
+    pub fn parse(message: &Message<Attribute>) -> Self {
         let transaction_id = message.transaction_id();
-        let message_integrity = message
-            .get_attribute::<MessageIntegrity>()
-            .ok_or(unauthorized(message))?
-            .clone();
-        let username = message
-            .get_attribute::<Username>()
-            .ok_or(bad_request(message))?
-            .clone();
+        let message_integrity = message.get_attribute::<MessageIntegrity>().cloned();
+        let username = message.get_attribute::<Username>().cloned();
+        let nonce = message.get_attribute::<Nonce>().cloned();
 
-        Ok(CreatePermission {
+        CreatePermission {
             transaction_id,
             message_integrity,
             username,
-        })
+            nonce,
+        }
     }
 
     pub fn transaction_id(&self) -> TransactionId {
         self.transaction_id
     }
 
-    pub fn message_integrity(&self) -> &MessageIntegrity {
-        &self.message_integrity
+    pub fn message_integrity(&self) -> Option<&MessageIntegrity> {
+        self.message_integrity.as_ref()
     }
 
-    pub fn username(&self) -> &Username {
-        &self.username
+    pub fn username(&self) -> Option<&Username> {
+        self.username.as_ref()
+    }
+
+    pub fn nonce(&self) -> Option<&Nonce> {
+        self.nonce.as_ref()
     }
 }
 
@@ -426,17 +457,6 @@ fn bad_request(message: &Message<Attribute>) -> Message<Attribute> {
     message
 }
 
-fn unauthorized(message: &Message<Attribute>) -> Message<Attribute> {
-    let mut message = Message::new(
-        MessageClass::ErrorResponse,
-        message.method(),
-        message.transaction_id(),
-    );
-    message.add_attribute(ErrorCode::from(Unauthorized).into());
-
-    message
-}
-
 #[derive(Debug)]
 pub enum Error {
     BadChannelData(io::Error),

rust/relay/tests/regression.rs

@@ -8,14 +8,14 @@ use std::collections::HashMap;
 use std::iter;
 use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4};
 use std::time::{Duration, SystemTime};
-use stun_codec::rfc5389::attributes::{Username, XorMappedAddress};
+use stun_codec::rfc5389::attributes::{ErrorCode, Nonce, Realm, Username, XorMappedAddress};
+use stun_codec::rfc5389::errors::Unauthorized;
 use stun_codec::rfc5389::methods::BINDING;
 use stun_codec::rfc5766::attributes::{ChannelNumber, Lifetime, XorPeerAddress, XorRelayAddress};
 use stun_codec::rfc5766::methods::{ALLOCATE, CHANNEL_BIND, REFRESH};
-use stun_codec::{
-    DecodedMessage, Message, MessageClass, MessageDecoder, MessageEncoder, TransactionId,
-};
+use stun_codec::{Message, MessageClass, MessageDecoder, MessageEncoder, TransactionId};
 use test_strategy::proptest;
+use uuid::Uuid;
 use Output::{CreateAllocation, FreeAllocation, Wake};
 
 #[proptest]
@@ -46,18 +46,20 @@ fn deallocate_once_time_expired(
     source: SocketAddrV4,
     public_relay_addr: Ipv4Addr,
     #[strategy(relay::proptest::now())] now: SystemTime,
+    #[strategy(relay::proptest::nonce())] nonce: Uuid,
 ) {
-    let mut server = TestServer::new(public_relay_addr);
+    let mut server = TestServer::new(public_relay_addr).with_nonce(nonce);
     let secret = server.auth_secret();
 
     server.assert_commands(
         from_client(
             source,
-            Allocate::new_udp(
+            Allocate::new_authenticated_udp(
                 transaction_id,
                 Some(lifetime.clone()),
                 valid_username(now, &username_salt),
                 &secret,
+                nonce,
             ),
             now,
         ),
@@ -77,6 +79,56 @@ fn deallocate_once_time_expired(
     );
 }
 
+#[proptest]
+fn unauthenticated_allocate_triggers_authentication(
+    #[strategy(relay::proptest::transaction_id())] transaction_id: TransactionId,
+    #[strategy(relay::proptest::allocation_lifetime())] lifetime: Lifetime,
+    #[strategy(relay::proptest::username_salt())] username_salt: String,
+    source: SocketAddrV4,
+    public_relay_addr: Ipv4Addr,
+    #[strategy(relay::proptest::now())] now: SystemTime,
+) {
+    // Nonces are generated randomly and we control the randomness in the test, thus this is deterministic.
+    let first_nonce = Uuid::from_u128(0x0);
+
+    let mut server = TestServer::new(public_relay_addr);
+    let secret = server.auth_secret();
+
+    server.assert_commands(
+        from_client(
+            source,
+            Allocate::new_unauthenticated_udp(transaction_id, Some(lifetime.clone())),
+            now,
+        ),
+        [send_message(
+            source,
+            unauthorized_allocate_response(transaction_id, first_nonce),
+        )],
+    );
+
+    server.assert_commands(
+        from_client(
+            source,
+            Allocate::new_authenticated_udp(
+                transaction_id,
+                Some(lifetime.clone()),
+                valid_username(now, &username_salt),
+                &secret,
+                first_nonce,
+            ),
+            now,
+        ),
+        [
+            Wake(now + lifetime.lifetime()),
+            CreateAllocation(49152),
+            send_message(
+                source,
+                allocate_response(transaction_id, public_relay_addr, 49152, source, &lifetime),
+            ),
+        ],
+    );
+}
+
 #[proptest]
 fn when_refreshed_in_time_allocation_does_not_expire(
     #[strategy(relay::proptest::transaction_id())] allocate_transaction_id: TransactionId,
@@ -87,19 +139,21 @@ fn when_refreshed_in_time_allocation_does_not_expire(
     source: SocketAddrV4,
     public_relay_addr: Ipv4Addr,
     #[strategy(relay::proptest::now())] now: SystemTime,
+    #[strategy(relay::proptest::nonce())] nonce: Uuid,
 ) {
-    let mut server = TestServer::new(public_relay_addr);
+    let mut server = TestServer::new(public_relay_addr).with_nonce(nonce);
     let secret = server.auth_secret();
     let first_wake = now + allocate_lifetime.lifetime();
 
     server.assert_commands(
         from_client(
             source,
-            Allocate::new_udp(
+            Allocate::new_authenticated_udp(
                 allocate_transaction_id,
                 Some(allocate_lifetime.clone()),
                 valid_username(now, &username_salt),
                 &secret,
+                nonce,
             ),
             now,
         ),
@@ -131,6 +185,7 @@ fn when_refreshed_in_time_allocation_does_not_expire(
                 Some(refresh_lifetime.clone()),
                 valid_username(now, &username_salt),
                 &secret,
+                nonce,
             ),
             now,
         ),
@@ -160,19 +215,21 @@ fn when_receiving_lifetime_0_for_existing_allocation_then_delete(
     source: SocketAddrV4,
     public_relay_addr: Ipv4Addr,
     #[strategy(relay::proptest::now())] now: SystemTime,
+    #[strategy(relay::proptest::nonce())] nonce: Uuid,
 ) {
-    let mut server = TestServer::new(public_relay_addr);
+    let mut server = TestServer::new(public_relay_addr).with_nonce(nonce);
     let secret = server.auth_secret();
     let first_wake = now + allocate_lifetime.lifetime();
 
     server.assert_commands(
         from_client(
             source,
-            Allocate::new_udp(
+            Allocate::new_authenticated_udp(
                 allocate_transaction_id,
                 Some(allocate_lifetime.clone()),
                 valid_username(now, &username_salt),
                 &secret,
+                nonce,
             ),
             now,
         ),
@@ -203,6 +260,7 @@ fn when_receiving_lifetime_0_for_existing_allocation_then_delete(
                 Some(Lifetime::new(Duration::ZERO).unwrap()),
                 valid_username(now, &username_salt),
                 &secret,
+                nonce,
             ),
             now,
         ),
@@ -240,20 +298,22 @@ fn ping_pong_relay(
     #[strategy(relay::proptest::now())] now: SystemTime,
     peer_to_client_ping: [u8; 32],
     client_to_peer_ping: [u8; 32],
+    #[strategy(relay::proptest::nonce())] nonce: Uuid,
 ) {
     let _ = env_logger::try_init();
 
-    let mut server = TestServer::new(public_relay_addr);
+    let mut server = TestServer::new(public_relay_addr).with_nonce(nonce);
     let secret = server.auth_secret();
 
     server.assert_commands(
         from_client(
             source,
-            Allocate::new_udp(
+            Allocate::new_authenticated_udp(
                 allocate_transaction_id,
                 Some(lifetime.clone()),
                 valid_username(now, &username_salt),
                 &secret,
+                nonce,
             ),
             now,
         ),
@@ -284,6 +344,7 @@ fn ping_pong_relay(
                 XorPeerAddress::new(peer.into()),
                 valid_username(now, &username_salt),
                 &secret,
+                nonce,
             ),
             now,
         ),
@@ -326,6 +387,12 @@ impl TestServer {
         }
     }
 
+    fn with_nonce(mut self, nonce: Uuid) -> Self {
+        self.server.add_nonce(nonce);
+
+        self
+    }
+
     fn auth_secret(&mut self) -> [u8; 32] {
         self.server.auth_secret()
     }
@@ -399,7 +466,7 @@ impl TestServer {
                         when.duration_since(SystemTime::UNIX_EPOCH)
                             .unwrap()
                             .as_secs(),
-                        parse_message(&payload).expect("self.server to produce valid message")
+                        parse_message(&payload)
                     )
                 }
                 (
@@ -474,6 +541,23 @@ fn allocate_response(
     message
 }
 
+fn unauthorized_allocate_response(
+    transaction_id: TransactionId,
+    nonce: Uuid,
+) -> Message<Attribute> {
+    let mut message =
+        Message::<Attribute>::new(MessageClass::ErrorResponse, ALLOCATE, transaction_id);
+    message.add_attribute(ErrorCode::from(Unauthorized).into());
+    message.add_attribute(
+        Nonce::new(nonce.as_hyphenated().to_string())
+            .unwrap()
+            .into(),
+    );
+    message.add_attribute(Realm::new("firezone".to_owned()).unwrap().into());
+
+    message
+}
+
 fn refresh_response(transaction_id: TransactionId, lifetime: Lifetime) -> Message<Attribute> {
     let mut message =
         Message::<Attribute>::new(MessageClass::SuccessResponse, REFRESH, transaction_id);
@@ -486,8 +570,11 @@ fn channel_bind_response(transaction_id: TransactionId) -> Message<Attribute> {
     Message::<Attribute>::new(MessageClass::SuccessResponse, CHANNEL_BIND, transaction_id)
 }
 
-fn parse_message(message: &[u8]) -> DecodedMessage<Attribute> {
-    MessageDecoder::new().decode_from_bytes(message).unwrap()
+fn parse_message(message: &[u8]) -> Message<Attribute> {
+    MessageDecoder::new()
+        .decode_from_bytes(message)
+        .unwrap()
+        .unwrap()
 }
 
 enum Input<'a> {
@@ -526,7 +613,7 @@ fn send_message<'a>(source: impl Into<SocketAddr>, message: Message<Attribute>)
     Output::SendMessage((source.into(), message))
 }
 
-fn send_channel_data<'a>(source: impl Into<SocketAddr>, message: ChannelData<'a>) -> Output<'a> {
+fn send_channel_data(source: impl Into<SocketAddr>, message: ChannelData) -> Output {
     Output::SendChannelData((source.into(), message))
 }