Split out stun types/proto to a separate crate

Now depends on stun-types/stun-proto for STUN.

.github/workflows/rust-fuzz.yml

@@ -26,12 +26,6 @@ jobs:
           command: 'install'
           args: 'cargo-fuzz'
 
-      - name: Run cargo-fuzz
-        uses: actions-rs/cargo@v1
-        with:
-          command: 'fuzz'
-          args: 'run stun_msg_from_bytes -- -max_total_time=20'
-
       - name: Run cargo-fuzz
         uses: actions-rs/cargo@v1
         with:

Cargo.toml

@@ -10,6 +10,7 @@ edition = "2021"
 rust-version = "1.68.2"
 
 [workspace.dependencies]
+stun-proto = "0.0.1"
 arbitrary = { version = "1", features = ["derive"] }
 byteorder = "1"
 get_if_addrs = "0.5"

fuzz/Cargo.toml

@@ -13,18 +13,12 @@ cargo-fuzz = true
 arbitrary.workspace = true
 libfuzzer-sys = "0.4"
 tracing.workspace = true
-tracing-subscriber.workspace = true
+tracing-subscriber = { workspace = true, features = ["env-filter"] }
 
 [dependencies.librice-proto]
 path = "../librice-proto"
 features = ["arbitrary"]
 
-[[bin]]
-name = "stun_msg_from_bytes"
-path = "fuzz_targets/stun_msg_from_bytes.rs"
-test = false
-doc = false
-
 [[bin]]
 name = "parse_candidate"
 path = "fuzz_targets/parse_candidate.rs"

librice-proto/Cargo.toml

@@ -18,17 +18,13 @@ capi = ["libc", "socket2", "tracing-subscriber/env-filter", "get_if_addrs"]
 [dependencies]
 arbitrary = { workspace = true, optional = true }
 byteorder.workspace = true
-crc = "3"
 get_if_addrs = { workspace = true, optional = true }
-hmac = "0.12"
-md-5 = "0.10"
 nom = "7"
 rand.workspace = true
-sha-1 = "0.10"
-sha2 = "0.10"
 tracing.workspace = true
 libc = { version = "0.2", optional = true }
 socket2 = { version = "0.5", optional = true }
+stun-proto.workspace = true
 tracing-subscriber = { workspace = true, optional = true }
 
 [dev-dependencies]

librice-proto/src/agent.rs

@@ -21,8 +21,8 @@ use crate::component::ComponentConnectionState;
 use crate::candidate::{ParseCandidateError, TransportType};
 use crate::conncheck::{CheckListSetPollRet, ConnCheckEvent, ConnCheckListSet, SelectedPair};
 use crate::stream::{Stream, StreamMut, StreamState};
-use crate::stun::agent::{StunError, Transmit};
-use crate::stun::attribute::StunParseError;
+use stun_proto::agent::{StunError, Transmit};
+use stun_proto::types::message::StunParseError;
 //use crate::turn::agent::TurnCredentials;
 
 /// Errors that can be returned as a result of agent operations.
@@ -49,10 +49,9 @@ pub enum AgentError {
     Aborted,
     TimedOut,
     StunParse,
+    StunWrite,
     /// Parsing the candidate failed.
     CandidateParse(ParseCandidateError),
-    /// An I/O error occurred.
-    IoError(std::io::Error),
     /// Data was received that does not match the protocol specifications.
     ProtocolViolation,
 }
@@ -65,12 +64,6 @@ impl Display for AgentError {
     }
 }
 
-impl From<std::io::Error> for AgentError {
-    fn from(e: std::io::Error) -> Self {
-        Self::IoError(e)
-    }
-}
-
 impl From<ParseCandidateError> for AgentError {
     fn from(e: ParseCandidateError) -> Self {
         Self::CandidateParse(e)
@@ -88,7 +81,7 @@ impl From<StunError> for AgentError {
             StunError::IntegrityCheckFailed => AgentError::IntegrityCheckFailed,
             StunError::ProtocolViolation => AgentError::ProtocolViolation,
             StunError::ParseError(_) => AgentError::StunParse,
-            StunError::IoError(e) => AgentError::IoError(e),
+            StunError::WriteError(_) => AgentError::StunWrite,
             StunError::Aborted => AgentError::Aborted,
             StunError::AlreadyInProgress => AgentError::AlreadyInProgress,
         }

librice-proto/src/candidate.rs

@@ -8,8 +8,8 @@
 
 //! ICE Candidates
 
-pub use crate::stun::TransportType;
 pub use parse::ParseCandidateError;
+pub use stun_proto::types::TransportType;
 
 use std::error::Error;
 use std::net::SocketAddr;
@@ -442,7 +442,7 @@ mod parse {
 
     use super::{Candidate, CandidateType, ParseCandidateTypeError};
     use super::{ParseTcpTypeError, TcpType};
-    use crate::stun::{ParseTransportTypeError, TransportType};
+    use stun_proto::types::{ParseTransportTypeError, TransportType};
 
     /// Errors produced when parsing a candidate
     #[derive(Debug)]

librice-proto/src/conncheck.rs

@@ -17,11 +17,11 @@ use std::time::{Duration, Instant};
 
 use crate::candidate::{Candidate, CandidatePair, CandidateType, TcpType, TransportType};
 use crate::component::ComponentConnectionState;
-use crate::stun::agent::{
+use stun_proto::agent::{
     HandleStunReply, StunAgent, StunError, StunRequest, StunRequestPollRet, Transmit,
 };
-use crate::stun::attribute::*;
-use crate::stun::message::*;
+use stun_proto::types::attribute::*;
+use stun_proto::types::message::*;
 
 /// ICE Credentials
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -70,7 +70,7 @@ impl SelectedPair {
     ///
     /// ```
     /// # use librice_proto::candidate::*;
-    /// # use librice_proto::stun::agent::StunAgent;
+    /// # use stun_proto::agent::StunAgent;
     /// # use librice_proto::component::SelectedPair;
     /// # use std::net::SocketAddr;
     /// let local_addr: SocketAddr = "127.0.0.1:2345".parse().unwrap();
@@ -111,8 +111,6 @@ impl SelectedPair {
 /// Return values when handling received data
 #[derive(Debug)]
 pub enum HandleRecvReply {
-    /// The data has been and should be ignored
-    Ignored,
     /// The data has been handled internally
     Handled,
     /// User data has been provided and should be handled further
@@ -341,7 +339,7 @@ impl ConnCheck {
             IntegrityAlgorithm::Sha1,
         )?;
         msg.add_fingerprint()?;
-        agent.stun_request_transaction(&msg, to).build()
+        agent.stun_request_transaction(&msg, to)
     }
 }
 
@@ -1347,7 +1345,7 @@ impl ConnCheckList {
             .component_ids
             .iter()
             .map(|&component_id| {
-                let nominated = self.pairs.iter().cloned().find(|check| check.nominate());
+                let nominated = self.pairs.iter().find(|check| check.nominate()).cloned();
                 nominated.or({
                     let mut valid: Vec<_> = self
                         .valid
@@ -1750,7 +1748,6 @@ impl ConnCheckListSet {
             });
         for reply in agent.handle_incoming_data(&transmit.data, transmit.from)? {
             match reply {
-                HandleStunReply::Ignore => ret.push(HandleRecvReply::Ignored),
                 HandleStunReply::Data(data, from) => ret.push(HandleRecvReply::Data(data, from)),
                 HandleStunReply::Stun(stun, from) => {
                     if stun.is_response() {
@@ -1804,40 +1801,46 @@ impl ConnCheckListSet {
         if let Some(error_msg) = Message::check_attribute_types(
             msg,
             &[
-                USERNAME,
-                FINGERPRINT,
-                MESSAGE_INTEGRITY,
-                ICE_CONTROLLED,
-                ICE_CONTROLLING,
-                PRIORITY,
-                USE_CANDIDATE,
+                Username::TYPE,
+                Fingerprint::TYPE,
+                MessageIntegrity::TYPE,
+                IceControlled::TYPE,
+                IceControlling::TYPE,
+                Priority::TYPE,
+                UseCandidate::TYPE,
+            ],
+            &[
+                Username::TYPE,
+                Fingerprint::TYPE,
+                MessageIntegrity::TYPE,
+                Priority::TYPE,
             ],
-            &[USERNAME, FINGERPRINT, MESSAGE_INTEGRITY, PRIORITY],
         ) {
             // failure -> send error response
             return Ok(Some(error_msg));
         }
-        let peer_nominating = if let Some(use_candidate_raw) = msg.raw_attribute(USE_CANDIDATE) {
+        let peer_nominating = if let Some(use_candidate_raw) = msg.raw_attribute(UseCandidate::TYPE)
+        {
             if UseCandidate::from_raw(use_candidate_raw).is_ok() {
                 true
             } else {
-                let response = Message::bad_request(msg)?;
+                let response = Message::bad_request(msg);
                 return Ok(Some(response));
             }
         } else {
             false
         };
 
-        let priority = match msg.attribute::<Priority>(PRIORITY) {
+        let priority = match msg.attribute::<Priority>() {
             Some(p) => p.priority(),
             None => {
-                let response = Message::bad_request(msg)?;
+                let response = Message::bad_request(msg);
                 return Ok(Some(response));
             }
         };
 
-        let ice_controlling = msg.attribute::<IceControlling>(ICE_CONTROLLING);
-        let ice_controlled = msg.attribute::<IceControlled>(ICE_CONTROLLED);
+        let ice_controlling = msg.attribute::<IceControlling>();
+        let ice_controlled = msg.attribute::<IceControlled>();
 
         /*
         if checklist.state == CheckListState::Completed && !peer_nominating {
@@ -1847,7 +1850,7 @@ impl ConnCheckListSet {
         }*/
 
         // validate username
-        if let Some(username) = msg.attribute::<Username>(USERNAME) {
+        if let Some(username) = msg.attribute::<Username>() {
             if !validate_username(username, &checklist.local_credentials) {
                 warn!("binding request failed username validation -> UNAUTHORIZED");
                 let mut response = Message::new_error(msg);
@@ -1856,7 +1859,7 @@ impl ConnCheckListSet {
             }
         } else {
             // existence is checked above so can only fail when the username is invalid
-            let response = Message::bad_request(msg)?;
+            let response = Message::bad_request(msg);
             return Ok(Some(response));
         }
 
@@ -2205,10 +2208,10 @@ impl ConnCheckListSet {
         // if response error -> fail TODO: might be a recoverable error!
         if response.has_class(MessageClass::Error) {
             warn!("error response {}", response);
-            if let Some(err) = response.attribute::<ErrorCode>(ERROR_CODE) {
+            if let Some(err) = response.attribute::<ErrorCode>() {
                 if err.code() == ErrorCode::ROLE_CONFLICT {
                     info!("Role conflict received {}", response);
-                    let new_role = stun_request.request().has_attribute(ICE_CONTROLLED);
+                    let new_role = stun_request.request().has_attribute(IceControlled::TYPE);
                     info!(
                         old_role = self.controlling,
                         new_role, "Role Conflict changing controlling from"
@@ -2234,7 +2237,7 @@ impl ConnCheckListSet {
             checklist.check_response_failure(conncheck.clone());
         }
 
-        if let Some(xor) = response.attribute::<XorMappedAddress>(XOR_MAPPED_ADDRESS) {
+        if let Some(xor) = response.attribute::<XorMappedAddress>() {
             let xor_addr = xor.addr(response.transaction_id());
             return self.check_success(checklist_i, conncheck, xor_addr, self.controlling);
         }
@@ -2571,7 +2574,6 @@ fn validate_username(username: Username, local_credentials: &Credentials) -> boo
 mod tests {
     use super::*;
     use crate::candidate::*;
-    use crate::stun::agent::*;
 
     fn init() {
         crate::tests::test_init_log();
@@ -2793,23 +2795,28 @@ mod tests {
         if let Some(error_msg) = Message::check_attribute_types(
             msg,
             &[
-                USERNAME,
-                FINGERPRINT,
-                MESSAGE_INTEGRITY,
-                ICE_CONTROLLED,
-                ICE_CONTROLLING,
-                PRIORITY,
-                USE_CANDIDATE,
+                Username::TYPE,
+                Fingerprint::TYPE,
+                MessageIntegrity::TYPE,
+                IceControlled::TYPE,
+                IceControlling::TYPE,
+                Priority::TYPE,
+                UseCandidate::TYPE,
+            ],
+            &[
+                Username::TYPE,
+                Fingerprint::TYPE,
+                MessageIntegrity::TYPE,
+                Priority::TYPE,
             ],
-            &[USERNAME, FINGERPRINT, MESSAGE_INTEGRITY, PRIORITY],
         ) {
             // failure -> send error response
             return Ok(error_msg);
         }
 
-        let ice_controlling = msg.attribute::<IceControlling>(ICE_CONTROLLING);
-        let ice_controlled = msg.attribute::<IceControlled>(ICE_CONTROLLED);
-        let username = msg.attribute::<Username>(USERNAME);
+        let ice_controlling = msg.attribute::<IceControlling>();
+        let ice_controlled = msg.attribute::<IceControlled>();
+        let username = msg.attribute::<Username>();
         let valid_username = username
             .map(|username| validate_username(username, local_credentials))
             .unwrap_or(false);
@@ -3202,10 +3209,9 @@ mod tests {
             for checklist_id in checklist_ids.into_iter() {
                 let reply = set.incoming_data(checklist_id, &reply).unwrap();
                 trace!("reply: {reply:?}");
-                assert!(matches!(
-                    reply[0],
-                    HandleRecvReply::Ignored | HandleRecvReply::Handled
-                ));
+                if !reply.is_empty() {
+                    assert!(matches!(reply[0], HandleRecvReply::Handled));
+                }
             }
         }
     }
@@ -3790,7 +3796,6 @@ mod tests {
         let local_addr = state.local.peer.stun_agent().local_addr();
         let stun_request = remote_agent
             .stun_request_transaction(&request, local_addr)
-            .build()
             .unwrap();
 
         info!("sending prflx request");
@@ -4165,7 +4170,6 @@ mod tests {
         let local_addr = state.local.peer.stun_agent().local_addr();
         let stun_request = remote_agent
             .stun_request_transaction(&request, local_addr)
-            .build()
             .unwrap();
 
         info!("sending request");