app-layer: add direction to transaction creation where needed

Build on Eric's but set the direction on transaction creation when
needed. I think this makes it a little more clear, and easier to
document when creating single direction transactions.

This also somewhat abstracts the inner-workings of a directional
transaction from the implementation.

Ticket: #4759

rust/src/applayer.rs

@@ -148,6 +148,8 @@ impl Drop for AppLayerTxData {
 }
 
 impl AppLayerTxData {
+    /// Create new AppLayerTxData for a transaction that covers both
+    /// directions.
     pub fn new() -> Self {
         Self {
             config: AppLayerTxConfig::new(),
@@ -163,9 +165,33 @@ impl AppLayerTxData {
             events: std::ptr::null_mut(),
         }
     }
+
+    /// Create new AppLayerTxData for a transaction in a single
+    /// direction.
+    pub fn for_direction(direction: Direction) -> Self {
+        let (detect_flags_ts, detect_flags_tc) = match direction {
+            Direction::ToServer => (0, APP_LAYER_TX_SKIP_INSPECT_FLAG),
+            Direction::ToClient => (APP_LAYER_TX_SKIP_INSPECT_FLAG, 0),
+        };
+        Self {
+            config: AppLayerTxConfig::new(),
+            logged: LoggerFlags::new(),
+            files_opened: 0,
+            files_logged: 0,
+            files_stored: 0,
+            file_flags: 0,
+            file_tx: 0,
+            detect_flags_ts,
+            detect_flags_tc,
+            de_state: std::ptr::null_mut(),
+            events: std::ptr::null_mut(),
+        }
+    }
+
     pub fn init_files_opened(&mut self) {
         self.files_opened = 1;
     }
+
     pub fn incr_files_opened(&mut self) {
         self.files_opened += 1;
     }
@@ -180,14 +206,6 @@ impl AppLayerTxData {
             self.file_flags |= state_flags;
         }
     }
-
-    pub fn set_inspect_direction(&mut self, direction: Direction) {
-        if direction == Direction::ToClient {
-            self.detect_flags_ts |= APP_LAYER_TX_SKIP_INSPECT_FLAG;
-        } else {
-            self.detect_flags_tc |= APP_LAYER_TX_SKIP_INSPECT_FLAG;
-        }
-    }
 }
 
 #[macro_export]

rust/src/bittorrent_dht/bittorrent_dht.rs

@@ -19,7 +19,7 @@ use crate::applayer::{self, *};
 use crate::bittorrent_dht::parser::{
     parse_bittorrent_dht_packet, BitTorrentDHTError, BitTorrentDHTRequest, BitTorrentDHTResponse,
 };
-use crate::core::{AppProto, Flow, ALPROTO_UNKNOWN, IPPROTO_UDP};
+use crate::core::{AppProto, Flow, ALPROTO_UNKNOWN, IPPROTO_UDP, Direction};
 use std::ffi::CString;
 use std::os::raw::c_char;
 
@@ -46,8 +46,11 @@ pub struct BitTorrentDHTTransaction {
 }
 
 impl BitTorrentDHTTransaction {
-    pub fn new() -> Self {
-        Self::default()
+    pub fn new(direction: Direction) -> Self {
+	Self {
+	    tx_data: AppLayerTxData::for_direction(direction),
+	    ..Default::default()
+	}
     }
 
     /// Set an event on the transaction
@@ -77,11 +80,10 @@ impl BitTorrentDHTState {
         self.transactions.iter().find(|&tx| tx.tx_id == tx_id + 1)
     }
 
-    fn new_tx(&mut self, _direction: crate::core::Direction) -> BitTorrentDHTTransaction {
-        let mut tx = BitTorrentDHTTransaction::default();
+    fn new_tx(&mut self, direction: Direction) -> BitTorrentDHTTransaction {
+        let mut tx = BitTorrentDHTTransaction::new(direction);
         self.tx_id += 1;
         tx.tx_id = self.tx_id;
-        tx.tx_data.set_inspect_direction(_direction);
         return tx;
     }
 

rust/src/bittorrent_dht/parser.rs

@@ -446,6 +446,7 @@ pub fn parse_bittorrent_dht_packet(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::core::Direction;
     use test_case::test_case;
 
     #[test_case(
@@ -576,7 +577,7 @@ mod tests {
         expected_error: Option<BitTorrentDHTError>, expected_transaction_id: Vec<u8>,
         expected_client_version: Option<Vec<u8>>,
     ) {
-        let mut tx = BitTorrentDHTTransaction::new();
+        let mut tx = BitTorrentDHTTransaction::new(Direction::ToServer);
         parse_bittorrent_dht_packet(encoded, &mut tx).unwrap();
         assert_eq!(request_type, tx.request_type);
         assert_eq!(expected_request, tx.request);
@@ -637,7 +638,7 @@ mod tests {
         "test parse bittorrent dht packet err 10"
     )]
     fn test_parse_bittorrent_dht_packet_err(encoded: &[u8], expected_error: &str) {
-        let mut tx = BitTorrentDHTTransaction::new();
+        let mut tx = BitTorrentDHTTransaction::new(Direction::ToServer);
         let err = parse_bittorrent_dht_packet(encoded, &mut tx).unwrap_err();
         assert_eq!(expected_error, err.to_string());
     }

rust/src/core.rs

@@ -49,6 +49,18 @@ pub enum Direction {
     ToClient = 0x08,
 }
 
+impl Direction {
+    /// Return true if the direction is to server.
+    pub fn is_to_server(&self) -> bool {
+	matches!(self, Self::ToServer)
+    }
+
+    /// Return true if the direction is to client.
+    pub fn is_to_client(&self) -> bool {
+	matches!(self, Self::ToClient)
+    }
+}
+
 impl Default for Direction {
     fn default() -> Self { Direction::ToServer }
 }
@@ -315,3 +327,17 @@ impl Flow {
         unsafe { (FlowGetSourcePort(self), FlowGetDestinationPort(self)) }
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_direction() {
+	assert!(Direction::ToServer.is_to_server());
+	assert!(!Direction::ToServer.is_to_client());
+
+	assert!(Direction::ToClient.is_to_client());
+	assert!(!Direction::ToClient.is_to_server());
+    }
+}

rust/src/dns/dns.rs

@@ -249,8 +249,11 @@ impl Transaction for DNSTransaction {
 }
 
 impl DNSTransaction {
-    pub fn new() -> Self {
-        Default::default()
+    pub fn new(direction: Direction) -> Self {
+	Self {
+	    tx_data: AppLayerTxData::for_direction(direction),
+            ..Default::default()
+	}
     }
 
     /// Get the DNS transactions ID (not the internal tracking ID).
@@ -336,8 +339,8 @@ impl DNSState {
         Default::default()
     }
 
-    pub fn new_tx(&mut self) -> DNSTransaction {
-        let mut tx = DNSTransaction::new();
+    pub fn new_tx(&mut self, direction: Direction) -> DNSTransaction {
+        let mut tx = DNSTransaction::new(direction);
         self.tx_id += 1;
         tx.id = self.tx_id;
         return tx;
@@ -410,9 +413,8 @@ impl DNSState {
                 let z_flag = request.header.flags & 0x0040 != 0;
                 let opcode = ((request.header.flags >> 11) & 0xf) as u8;
 
-                let mut tx = self.new_tx();
+                let mut tx = self.new_tx(Direction::ToServer);
                 tx.request = Some(request);
-                tx.tx_data.set_inspect_direction(Direction::ToServer);
                 self.transactions.push_back(tx);
 
                 if z_flag {
@@ -484,14 +486,13 @@ impl DNSState {
                 let z_flag = response.header.flags & 0x0040 != 0;
                 let opcode = ((response.header.flags >> 11) & 0xf) as u8;
 
-                let mut tx = self.new_tx();
+                let mut tx = self.new_tx(Direction::ToClient);
                 if let Some(ref mut config) = &mut self.config {
                     if let Some(config) = config.remove(&response.header.tx_id) {
                         tx.tx_data.config = config;
                     }
                 }
                 tx.response = Some(response);
-                tx.tx_data.set_inspect_direction(Direction::ToClient);
                 self.transactions.push_back(tx);
 
                 if z_flag {

rust/src/ike/ike.rs

@@ -123,8 +123,12 @@ impl Transaction for IKETransaction {
 }
 
 impl IKETransaction {
-    pub fn new() -> Self {
-        Default::default()
+    pub fn new(direction: Direction) -> Self {
+	Self {
+	    direction,
+	    tx_data: applayer::AppLayerTxData::for_direction(direction),
+	    ..Default::default()
+	}
     }
 
     /// Set an event.
@@ -170,8 +174,8 @@ impl IKEState {
         self.transactions.iter_mut().find(|tx| tx.tx_id == tx_id + 1)
     }
 
-    pub fn new_tx(&mut self) -> IKETransaction {
-        let mut tx = IKETransaction::new();
+    pub fn new_tx(&mut self, direction: Direction) -> IKETransaction {
+        let mut tx = IKETransaction::new(direction);
         self.tx_id += 1;
         tx.tx_id = self.tx_id;
         return tx;

rust/src/ike/ikev1.rs

@@ -74,11 +74,9 @@ pub struct Ikev1Container {
 pub fn handle_ikev1(
     state: &mut IKEState, current: &[u8], isakmp_header: IsakmpHeader, direction: Direction,
 ) -> AppLayerResult {
-    let mut tx = state.new_tx();
+    let mut tx = state.new_tx(direction);
 
     tx.ike_version = 1;
-    tx.direction = direction;
-    tx.tx_data.set_inspect_direction(direction);
     tx.hdr.spi_initiator = format!("{:016x}", isakmp_header.init_spi);
     tx.hdr.spi_responder = format!("{:016x}", isakmp_header.resp_spi);
     tx.hdr.maj_ver = isakmp_header.maj_ver;

rust/src/ike/ikev2.rs

@@ -113,11 +113,10 @@ pub fn handle_ikev2(
         length: isakmp_header.length,
     };
 
-    let mut tx = state.new_tx();
+    let mut tx = state.new_tx(direction);
     tx.ike_version = 2;
     // use init_spi as transaction identifier
     // tx.xid = hdr.init_spi; todo is this used somewhere?
-    tx.tx_data.set_inspect_direction(direction);
     tx.hdr.ikev2_header = hdr.clone();
     tx.hdr.spi_initiator = format!("{:016x}", isakmp_header.init_spi);
     tx.hdr.spi_responder = format!("{:016x}", isakmp_header.resp_spi);

rust/src/krb/krb5.rs

@@ -125,7 +125,7 @@ impl KRB5State {
     /// Parse a Kerberos request message
     ///
     /// Returns 0 in case of success, or -1 on error
-    fn parse(&mut self, i: &[u8], _direction: Direction) -> i32 {
+    fn parse(&mut self, i: &[u8], direction: Direction) -> i32 {
         match der_read_element_header(i) {
             Ok((_rem,hdr)) => {
                 // Kerberos messages start with an APPLICATION header
@@ -137,7 +137,7 @@ impl KRB5State {
                     11 => {
                         let res = krb5_parser::parse_as_rep(i);
                         if let Ok((_,kdc_rep)) = res {
-                            let mut tx = self.new_tx();
+                            let mut tx = self.new_tx(direction);
                             tx.msg_type = MessageType::KRB_AS_REP;
                             tx.cname = Some(kdc_rep.cname);
                             tx.realm = Some(kdc_rep.crealm);
@@ -157,7 +157,7 @@ impl KRB5State {
                     13 => {
                         let res = krb5_parser::parse_tgs_rep(i);
                         if let Ok((_,kdc_rep)) = res {
-                            let mut tx = self.new_tx();
+                            let mut tx = self.new_tx(direction);
                             tx.msg_type = MessageType::KRB_TGS_REP;
                             tx.cname = Some(kdc_rep.cname);
                             tx.realm = Some(kdc_rep.crealm);
@@ -180,7 +180,7 @@ impl KRB5State {
                     30 => {
                         let res = krb5_parser::parse_krb_error(i);
                         if let Ok((_,error)) = res {
-                            let mut tx = self.new_tx();
+                            let mut tx = self.new_tx(direction);
                             tx.msg_type = MessageType(self.req_id as u32);
                             tx.cname = error.cname;
                             tx.realm = error.crealm;
@@ -213,9 +213,9 @@ impl KRB5State {
         self.transactions.clear();
     }
 
-    fn new_tx(&mut self) -> KRB5Transaction {
+    fn new_tx(&mut self, direction: Direction) -> KRB5Transaction {
         self.tx_id += 1;
-        KRB5Transaction::new(self.tx_id)
+        KRB5Transaction::new(direction, self.tx_id)
     }
 
     fn get_tx_by_id(&mut self, tx_id: u64) -> Option<&KRB5Transaction> {
@@ -239,8 +239,8 @@ impl KRB5State {
 }
 
 impl KRB5Transaction {
-    pub fn new(id: u64) -> KRB5Transaction {
-        let mut krbtx = KRB5Transaction{
+    pub fn new(direction: Direction, id: u64) -> KRB5Transaction {
+        let krbtx = KRB5Transaction{
             msg_type: MessageType(0),
             cname: None,
             realm: None,
@@ -249,9 +249,8 @@ impl KRB5Transaction {
             ticket_etype: None,
             error_code: None,
             id,
-            tx_data: applayer::AppLayerTxData::new(),
+            tx_data: applayer::AppLayerTxData::for_direction(direction),
         };
-        krbtx.tx_data.set_inspect_direction(Direction::ToClient);
         return krbtx;
     }
 }

rust/src/mqtt/detect.rs

@@ -393,6 +393,7 @@ mod test {
     use crate::mqtt::mqtt::MQTTTransaction;
     use crate::mqtt::mqtt_message::*;
     use crate::mqtt::parser::FixedHeader;
+    use crate::core::Direction;
     use std;
 
     #[test]
@@ -410,7 +411,7 @@ mod test {
                 topics: vec!["foo".to_string(), "baar".to_string()],
                 properties: None,
             }),
-        });
+        }, Direction::ToServer);
         t.msg.push(MQTTMessage {
             header: FixedHeader {
                 message_type: MQTTTypeCode::UNSUBSCRIBE,
@@ -471,7 +472,7 @@ mod test {
                 ],
                 properties: None,
             }),
-        });
+        }, Direction::ToServer);
         t.msg.push(MQTTMessage {
             header: FixedHeader {
                 message_type: MQTTTypeCode::SUBSCRIBE,

rust/src/mqtt/mqtt.rs

@@ -70,22 +70,22 @@ pub struct MQTTTransaction {
 }
 
 impl MQTTTransaction {
-    pub fn new(msg: MQTTMessage) -> MQTTTransaction {
-        let mut m = MQTTTransaction::new_empty();
+    pub fn new(msg: MQTTMessage, direction: Direction) -> MQTTTransaction {
+        let mut m = MQTTTransaction::new_empty(direction);
         m.msg.push(msg);
         return m;
     }
 
-    pub fn new_empty() -> MQTTTransaction {
+    pub fn new_empty(direction: Direction) -> MQTTTransaction {
         return MQTTTransaction {
             tx_id: 0,
             pkt_id: None,
             complete: false,
             logged: LoggerFlags::new(),
             msg: Vec::new(),
-            toclient: false,
-            toserver: false,
-            tx_data: applayer::AppLayerTxData::new(),
+            toclient: direction.is_to_client(),
+            toserver: direction.is_to_server(),
+            tx_data: applayer::AppLayerTxData::for_direction(direction),
         };
     }
 }
@@ -175,16 +175,14 @@ impl MQTTState {
     }
 
     fn new_tx(&mut self, msg: MQTTMessage, toclient: bool) -> MQTTTransaction {
-        let mut tx = MQTTTransaction::new(msg);
+	let direction = if toclient {
+	    Direction::ToClient
+	} else {
+	    Direction::ToServer
+	};
+        let mut tx = MQTTTransaction::new(msg, direction);
         self.tx_id += 1;
         tx.tx_id = self.tx_id;
-        if toclient {
-            tx.toclient = true;
-            tx.tx_data.set_inspect_direction(Direction::ToClient);
-        } else {
-            tx.toserver = true;
-            tx.tx_data.set_inspect_direction(Direction::ToServer);
-        }
         if self.transactions.len() > unsafe { MQTT_MAX_TX } {
             let mut index = self.tx_index_completed;
             for tx_old in &mut self.transactions.range_mut(self.tx_index_completed..) {
@@ -579,7 +577,7 @@ impl MQTTState {
     }
 
     fn set_event_notx(&mut self, event: MQTTEvent, toclient: bool) {
-        let mut tx = MQTTTransaction::new_empty();
+        let mut tx = MQTTTransaction::new_empty(if toclient { Direction::ToClient } else { Direction::ToServer });
         self.tx_id += 1;
         tx.tx_id = self.tx_id;
         if toclient {

rust/src/ntp/ntp.rs

@@ -86,7 +86,7 @@ impl NTPState {
     /// Parse an NTP request message
     ///
     /// Returns 0 if successful, or -1 on error
-    fn parse(&mut self, i: &[u8], _direction: u8) -> i32 {
+    fn parse(&mut self, i: &[u8], direction: Direction) -> i32 {
         match parse_ntp(i) {
             Ok((_,ref msg)) => {
                 // SCLogDebug!("parse_ntp: {:?}",msg);
@@ -95,7 +95,7 @@ impl NTPState {
                     NtpPacket::V4(pkt) => (pkt.mode, pkt.ref_id),
                 };
                 if mode == NtpMode::SymmetricActive || mode == NtpMode::Client {
-                    let mut tx = self.new_tx(_direction);
+                    let mut tx = self.new_tx(direction);
                     // use the reference id as identifier
                     tx.xid = ref_id;
                     self.transactions.push(tx);
@@ -121,15 +121,9 @@ impl NTPState {
         self.transactions.clear();
     }
 
-    fn new_tx(&mut self, _direction: u8) -> NTPTransaction {
+    fn new_tx(&mut self, direction: Direction) -> NTPTransaction {
         self.tx_id += 1;
-        let mut tx = NTPTransaction::new(self.tx_id);
-        if _direction == 0 {
-            tx.tx_data.set_inspect_direction(Direction::ToServer);
-        } else {
-            tx.tx_data.set_inspect_direction(Direction::ToClient);
-        }
-        tx
+        NTPTransaction::new(direction, self.tx_id)
     }
 
     pub fn get_tx_by_id(&mut self, tx_id: u64) -> Option<&NTPTransaction> {
@@ -154,11 +148,11 @@ impl NTPState {
 }
 
 impl NTPTransaction {
-    pub fn new(id: u64) -> NTPTransaction {
+    pub fn new(direction: Direction, id: u64) -> NTPTransaction {
         NTPTransaction {
             xid: 0,
             id,
-            tx_data: applayer::AppLayerTxData::new(),
+            tx_data: applayer::AppLayerTxData::for_direction(direction),
         }
     }
 }
@@ -187,7 +181,7 @@ pub unsafe extern "C" fn rs_ntp_parse_request(_flow: *const core::Flow,
                                        _data: *const std::os::raw::c_void,
                                        ) -> AppLayerResult {
     let state = cast_pointer!(state,NTPState);
-    if state.parse(stream_slice.as_slice(), 0) < 0 {
+    if state.parse(stream_slice.as_slice(), Direction::ToServer) < 0 {
         return AppLayerResult::err();
     }
     AppLayerResult::ok()
@@ -201,7 +195,7 @@ pub unsafe extern "C" fn rs_ntp_parse_response(_flow: *const core::Flow,
                                        _data: *const std::os::raw::c_void,
                                        ) -> AppLayerResult {
     let state = cast_pointer!(state,NTPState);
-    if state.parse(stream_slice.as_slice(), 1) < 0 {
+    if state.parse(stream_slice.as_slice(), Direction::ToClient) < 0 {
         return AppLayerResult::err();
     }
     AppLayerResult::ok()
@@ -324,7 +318,7 @@ pub unsafe extern "C" fn rs_register_ntp_parser() {
 
 #[cfg(test)]
 mod tests {
-    use super::NTPState;
+    use super::*;
 
     #[test]
     fn test_ntp_parse_request_valid() {
@@ -339,6 +333,6 @@ mod tests {
         ];
 
         let mut state = NTPState::new();
-        assert_eq!(0, state.parse(REQ, 0));
+        assert_eq!(0, state.parse(REQ, Direction::ToServer));
     }
 }

rust/src/quic/quic.rs

@@ -57,8 +57,9 @@ impl QuicTransaction {
         header: QuicHeader, data: QuicData, sni: Option<Vec<u8>>, ua: Option<Vec<u8>>,
         extv: Vec<QuicTlsExtension>, ja3: Option<String>, client: bool,
     ) -> Self {
+	let direction = if client { Direction::ToServer } else { Direction::ToClient };
         let cyu = Cyu::generate(&header, &data.frames);
-        let mut ntx = QuicTransaction {
+        QuicTransaction {
             tx_id: 0,
             header,
             cyu,
@@ -67,17 +68,12 @@ impl QuicTransaction {
             extv,
             ja3,
             client,
-            tx_data: AppLayerTxData::new(),
-        };
-        if client {
-            ntx.tx_data.set_inspect_direction(Direction::ToServer);
-        } else {
-            ntx.tx_data.set_inspect_direction(Direction::ToClient);
+            tx_data: AppLayerTxData::for_direction(direction),
         }
-        return ntx;
     }
 
     fn new_empty(client: bool, header: QuicHeader) -> Self {
+	let direction = if client { Direction::ToServer } else { Direction::ToClient };
         QuicTransaction {
             tx_id: 0,
             header,
@@ -87,7 +83,7 @@ impl QuicTransaction {
             extv: Vec::new(),
             ja3: None,
             client,
-            tx_data: AppLayerTxData::new(),
+            tx_data: AppLayerTxData::for_direction(direction),
         }
     }
 }
@@ -141,11 +137,6 @@ impl QuicState {
         let mut tx = QuicTransaction::new(header, data, sni, ua, extb, ja3, client);
         self.max_tx_id += 1;
         tx.tx_id = self.max_tx_id;
-        if client {
-            tx.tx_data.set_inspect_direction(Direction::ToServer);
-        } else {
-            tx.tx_data.set_inspect_direction(Direction::ToClient);
-        }
         self.transactions.push_back(tx);
     }
 

rust/src/snmp/snmp.rs

@@ -204,11 +204,9 @@ impl<'a> SNMPState<'a> {
         self.transactions.clear();
     }
 
-    fn new_tx(&mut self, _direction: Direction) -> SNMPTransaction<'a> {
+    fn new_tx(&mut self, direction: Direction) -> SNMPTransaction<'a> {
         self.tx_id += 1;
-        let mut tx = SNMPTransaction::new(self.version, self.tx_id);
-        tx.tx_data.set_inspect_direction(_direction);
-        tx
+        SNMPTransaction::new(direction, self.version, self.tx_id)
     }
 
     fn get_tx_by_id(&mut self, tx_id: u64) -> Option<&SNMPTransaction> {
@@ -237,7 +235,7 @@ impl<'a> SNMPState<'a> {
 }
 
 impl<'a> SNMPTransaction<'a> {
-    pub fn new(version: u32, id: u64) -> SNMPTransaction<'a> {
+    pub fn new(direction: Direction, version: u32, id: u64) -> SNMPTransaction<'a> {
         SNMPTransaction {
             version,
             info: None,
@@ -245,7 +243,7 @@ impl<'a> SNMPTransaction<'a> {
             usm: None,
             encrypted: false,
             id,
-            tx_data: applayer::AppLayerTxData::new(),
+            tx_data: applayer::AppLayerTxData::for_direction(direction),
         }
     }
 }