From: Philippe Antoine Date: Mon, 21 Jun 2021 08:50:12 +0000 (+0200) Subject: ike: do not keep server transforms in state X-Git-Tag: suricata-7.0.0-beta1~1293 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=accdad7881e43a1d2895c22ad0e85fddbd37a429;p=thirdparty%2Fsuricata.git ike: do not keep server transforms in state Fixes #4534 Now, only the tx with the transforms will match with ike.chosen_sa_attribute --- diff --git a/rust/src/ike/detect.rs b/rust/src/ike/detect.rs index fc5bef396e..719a1730eb 100644 --- a/rust/src/ike/detect.rs +++ b/rust/src/ike/detect.rs @@ -166,39 +166,37 @@ pub extern "C" fn rs_ike_state_get_sa_attribute( } } } else if tx.ike_version == 2 { - 'outer2: for server_transform in tx.hdr.ikev2_transforms.iter() { - for attr in server_transform { - match attr { - IkeV2Transform::Encryption(e) => { - if sa == "alg_enc" { - ret_val = e.0 as u32; - ret_code = 1; - break 'outer2; - } + for attr in tx.hdr.ikev2_transforms.iter() { + match attr { + IkeV2Transform::Encryption(e) => { + if sa == "alg_enc" { + ret_val = e.0 as u32; + ret_code = 1; + break; } - IkeV2Transform::Auth(e) => { - if sa == "alg_auth" { - ret_val = e.0 as u32; - ret_code = 1; - break 'outer2; - } + } + IkeV2Transform::Auth(e) => { + if sa == "alg_auth" { + ret_val = e.0 as u32; + ret_code = 1; + break; } - IkeV2Transform::PRF(ref e) => { - if sa == "alg_prf" { - ret_val = e.0 as u32; - ret_code = 1; - break 'outer2; - } + } + IkeV2Transform::PRF(ref e) => { + if sa == "alg_prf" { + ret_val = e.0 as u32; + ret_code = 1; + break; } - IkeV2Transform::DH(ref e) => { - if sa == "alg_dh" { - ret_val = e.0 as u32; - ret_code = 1; - break 'outer2; - } + } + IkeV2Transform::DH(ref e) => { + if sa == "alg_dh" { + ret_val = e.0 as u32; + ret_code = 1; + break; } - _ => (), } + _ => (), } } } diff --git a/rust/src/ike/ike.rs b/rust/src/ike/ike.rs index cc30b42813..6787c22a28 100644 --- a/rust/src/ike/ike.rs +++ b/rust/src/ike/ike.rs @@ -57,7 +57,7 @@ pub struct IkeHeaderWrapper { pub msg_id: u32, pub flags: u8, pub ikev1_transforms: Vec>, - pub ikev2_transforms: Vec>, + pub ikev2_transforms: Vec, pub ikev1_header: IkeV1Header, pub ikev2_header: IkeV2Header, } @@ -136,6 +136,12 @@ impl IKETransaction { core::sc_detect_engine_state_free(state); } } + + /// Set an event. + pub fn set_event(&mut self, event: IkeEvent) { + let ev = event as u8; + core::sc_app_layer_decoder_events_set_event_raw(&mut self.events, ev); + } } impl Drop for IKETransaction { diff --git a/rust/src/ike/ikev2.rs b/rust/src/ike/ikev2.rs index 6082a5beea..e731637522 100644 --- a/rust/src/ike/ikev2.rs +++ b/rust/src/ike/ikev2.rs @@ -22,7 +22,7 @@ use crate::core::STREAM_TOCLIENT; use crate::ike::ipsec_parser::*; use super::ipsec_parser::IkeV2Transform; -use crate::ike::ike::{IKEState, IkeEvent}; +use crate::ike::ike::{IKEState, IKETransaction, IkeEvent}; use crate::ike::parser::IsakmpHeader; use ipsec_parser::{IkeExchangeType, IkePayloadType, IkeV2Header}; @@ -69,9 +69,6 @@ pub struct Ikev2Container { /// The transforms proposed by the initiator pub client_transforms: Vec>, - /// The transforms selected by the responder - pub server_transforms: Vec>, - /// The encryption algorithm selected by the responder pub alg_enc: IkeTransformEncType, /// The authentication algorithm selected by the responder @@ -92,7 +89,6 @@ impl Default for Ikev2Container { connection_state: IKEV2ConnectionState::Init, dh_group: IkeTransformDHType::None, client_transforms: Vec::new(), - server_transforms: Vec::new(), alg_enc: IkeTransformEncType::ENCR_NULL, alg_auth: IkeTransformAuthType::NONE, alg_prf: IkeTransformPRFType::PRF_NULL, @@ -128,7 +124,6 @@ pub fn handle_ikev2( tx.hdr.min_ver = isakmp_header.min_ver; tx.hdr.msg_id = isakmp_header.msg_id; tx.hdr.flags = isakmp_header.flags; - state.transactions.push(tx); let mut payload_types = Vec::new(); let mut errors = 0; let mut notify_types = Vec::new(); @@ -140,7 +135,7 @@ pub fn handle_ikev2( IkeV2PayloadContent::Dummy => (), IkeV2PayloadContent::SA(ref prop) => { // if hdr.flags & IKEV2_FLAG_INITIATOR != 0 { - add_proposals(state, prop, direction); + add_proposals(state, &mut tx, prop, direction); // } } IkeV2PayloadContent::KE(ref kex) => { @@ -171,50 +166,22 @@ pub fn handle_ikev2( } state.ikev2_container.connection_state = state.ikev2_container.connection_state.advance(payload); - if let Some(tx) = state.transactions.last_mut() { - // borrow back tx to update it - tx.payload_types - .ikev2_payload_types - .append(&mut payload_types); - tx.errors = errors; - tx.notify_types.append(&mut notify_types); - - if direction == STREAM_TOCLIENT - && state.ikev2_container.server_transforms.len() > 0 - { - tx.hdr.ikev2_transforms.clear(); - for transforms in &state.ikev2_container.server_transforms { - let mut proposal = Vec::new(); - transforms.iter().for_each(|t| match *t { - IkeV2Transform::Encryption(ref e) => { - proposal.push(IkeV2Transform::Encryption(*e)) - } - IkeV2Transform::Auth(ref e) => { - proposal.push(IkeV2Transform::Auth(*e)) - } - IkeV2Transform::PRF(ref e) => { - proposal.push(IkeV2Transform::PRF(*e)) - } - IkeV2Transform::DH(ref e) => proposal.push(IkeV2Transform::DH(*e)), - IkeV2Transform::ESN(ref e) => { - proposal.push(IkeV2Transform::ESN(*e)) - } - _ => (), - }); - tx.hdr.ikev2_transforms.push(proposal); - } - } - } + tx.payload_types + .ikev2_payload_types + .append(&mut payload_types); + tx.errors = errors; + tx.notify_types.append(&mut notify_types); } } _e => { SCLogDebug!("parse_ikev2_payload_with_type: {:?}", _e); } } + state.transactions.push(tx); return AppLayerResult::ok(); } -fn add_proposals(state: &mut IKEState, prop: &Vec, direction: u8) { +fn add_proposals(state: &mut IKEState, tx: &mut IKETransaction, prop: &Vec, direction: u8) { for p in prop { let transforms: Vec = p.transforms.iter().map(|x| x.into()).collect(); // Rule 1: warn on weak or unknown transforms @@ -234,7 +201,7 @@ fn add_proposals(state: &mut IKEState, prop: &Vec, direction: u8) | IkeTransformEncType::ENCR_NULL => { SCLogDebug!("Weak Encryption: {:?}", enc); // XXX send event only if direction == STREAM_TOCLIENT ? - state.set_event(IkeEvent::WeakCryptoEnc); + tx.set_event(IkeEvent::WeakCryptoEnc); } _ => (), } @@ -242,11 +209,11 @@ fn add_proposals(state: &mut IKEState, prop: &Vec, direction: u8) IkeV2Transform::PRF(ref prf) => match *prf { IkeTransformPRFType::PRF_NULL => { SCLogDebug!("'Null' PRF transform proposed"); - state.set_event(IkeEvent::InvalidProposal); + tx.set_event(IkeEvent::InvalidProposal); } IkeTransformPRFType::PRF_HMAC_MD5 | IkeTransformPRFType::PRF_HMAC_SHA1 => { SCLogDebug!("Weak PRF: {:?}", prf); - state.set_event(IkeEvent::WeakCryptoPrf); + tx.set_event(IkeEvent::WeakCryptoPrf); } _ => (), }, @@ -264,7 +231,7 @@ fn add_proposals(state: &mut IKEState, prop: &Vec, direction: u8) | IkeTransformAuthType::AUTH_HMAC_MD5_128 | IkeTransformAuthType::AUTH_HMAC_SHA1_160 => { SCLogDebug!("Weak auth: {:?}", auth); - state.set_event(IkeEvent::WeakCryptoAuth); + tx.set_event(IkeEvent::WeakCryptoAuth); } _ => (), } @@ -272,20 +239,20 @@ fn add_proposals(state: &mut IKEState, prop: &Vec, direction: u8) IkeV2Transform::DH(ref dh) => match *dh { IkeTransformDHType::None => { SCLogDebug!("'None' DH transform proposed"); - state.set_event(IkeEvent::InvalidProposal); + tx.set_event(IkeEvent::InvalidProposal); } IkeTransformDHType::Modp768 | IkeTransformDHType::Modp1024 | IkeTransformDHType::Modp1024s160 | IkeTransformDHType::Modp1536 => { SCLogDebug!("Weak DH: {:?}", dh); - state.set_event(IkeEvent::WeakCryptoDh); + tx.set_event(IkeEvent::WeakCryptoDh); } _ => (), }, IkeV2Transform::Unknown(_tx_type, _tx_id) => { SCLogDebug!("Unknown proposal: type={:?}, id={}", _tx_type, _tx_id); - state.set_event(IkeEvent::UnknownProposal); + tx.set_event(IkeEvent::UnknownProposal); } _ => (), } @@ -296,12 +263,12 @@ fn add_proposals(state: &mut IKEState, prop: &Vec, direction: u8) _ => false, }) { SCLogDebug!("No DH transform found"); - state.set_event(IkeEvent::WeakCryptoNoDh); + tx.set_event(IkeEvent::WeakCryptoNoDh); } // Rule 3: check if proposing AH ([RFC7296] section 3.3.1) if p.protocol_id == ProtocolID::AH { SCLogDebug!("Proposal uses protocol AH - no confidentiality"); - state.set_event(IkeEvent::NoEncryption); + tx.set_event(IkeEvent::NoEncryption); } // Rule 4: lack of integrity is accepted only if using an AEAD proposal // Look if no auth was proposed, including if proposal is Auth::None @@ -315,21 +282,35 @@ fn add_proposals(state: &mut IKEState, prop: &Vec, direction: u8) _ => false, }) { SCLogDebug!("No integrity transform found"); - state.set_event(IkeEvent::WeakCryptoNoAuth); + tx.set_event(IkeEvent::WeakCryptoNoAuth); } } // Finally if direction == STREAM_TOCLIENT { transforms.iter().for_each(|t| match *t { - IkeV2Transform::Encryption(ref e) => state.ikev2_container.alg_enc = *e, - IkeV2Transform::Auth(ref a) => state.ikev2_container.alg_auth = *a, - IkeV2Transform::PRF(ref p) => state.ikev2_container.alg_prf = *p, - IkeV2Transform::DH(ref dh) => state.ikev2_container.alg_dh = *dh, - IkeV2Transform::ESN(ref e) => state.ikev2_container.alg_esn = *e, - _ => (), + IkeV2Transform::Encryption(ref e) => { + state.ikev2_container.alg_enc = *e; + tx.hdr.ikev2_transforms.push(IkeV2Transform::Encryption(*e)); + } + IkeV2Transform::Auth(ref a) => { + state.ikev2_container.alg_auth = *a; + tx.hdr.ikev2_transforms.push(IkeV2Transform::Auth(*a)); + } + IkeV2Transform::PRF(ref p) => { + state.ikev2_container.alg_prf = *p; + tx.hdr.ikev2_transforms.push(IkeV2Transform::PRF(*p)); + } + IkeV2Transform::DH(ref dh) => { + state.ikev2_container.alg_dh = *dh; + tx.hdr.ikev2_transforms.push(IkeV2Transform::DH(*dh)); + } + IkeV2Transform::ESN(ref e) => { + state.ikev2_container.alg_esn = *e; + tx.hdr.ikev2_transforms.push(IkeV2Transform::ESN(*e)); + } + _ => {}, }); SCLogDebug!("Selected transforms: {:?}", transforms); - state.ikev2_container.server_transforms.push(transforms); } else { SCLogDebug!("Proposed transforms: {:?}", transforms); state.ikev2_container.client_transforms.push(transforms);