diff --git a/rust/src/ssh/detect.rs b/rust/src/ssh/detect.rs index 2b64ecbe17..322ec51580 100644 --- a/rust/src/ssh/detect.rs +++ b/rust/src/ssh/detect.rs @@ -298,7 +298,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOSERVER | STREAM_TOCLIENT, Some(SCSshTxGetSoftware), - SSHConnectionState::SshStateBannerDone as u8, + SSHConnectionState::SshStateKex as u8, ); SCDetectHelperKeywordAliasRegister( ssh_software_kw_id, @@ -340,7 +340,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOSERVER | STREAM_TOCLIENT, Some(SCSshTxGetProtocol), - SSHConnectionState::SshStateBannerDone as u8, + SSHConnectionState::SshStateKex as u8, ); SCDetectHelperKeywordAliasRegister( ssh_proto_kw_id, @@ -360,7 +360,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOSERVER, Some(SCSshTxGetHasshString), - SSHConnectionState::SshStateBannerDone as u8, + SSHConnectionState::SshStateKex as u8, ); SCDetectHelperKeywordAliasRegister( DETECT_SSH_HASSH_STRING, @@ -380,7 +380,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOCLIENT, Some(SCSshTxGetHasshString), - SSHConnectionState::SshStateBannerDone as u8, + SSHConnectionState::SshStateKex as u8, ); SCDetectHelperKeywordAliasRegister( DETECT_SSH_HASSH_SERVER_STRING, @@ -400,7 +400,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOSERVER, Some(SCSshTxGetHassh), - SSHConnectionState::SshStateBannerDone as u8, + SSHConnectionState::SshStateKex as u8, ); SCDetectHelperKeywordAliasRegister( DETECT_SSH_HASSH, @@ -421,7 +421,7 @@ pub unsafe extern "C" fn SCDetectSshRegister() { ALPROTO_SSH, STREAM_TOCLIENT, Some(SCSshTxGetHassh), - SSHConnectionState::SshStateBannerDone as u8, + SSHConnectionState::SshStateKex as u8, ); SCDetectHelperKeywordAliasRegister( DETECT_SSH_HASSH_SERVER, diff --git a/rust/src/ssh/ssh.rs b/rust/src/ssh/ssh.rs index c1d2668caa..b6baf5c6aa 100644 --- a/rust/src/ssh/ssh.rs +++ b/rust/src/ssh/ssh.rs @@ -61,14 +61,22 @@ pub enum SSHEvent { LongKexRecord, } +/// Per-direction phases; the value doubles as the detection progress +/// (progress changes when the processing of the next state begins). +/// SshStateDone is the registered completion state, never assigned +/// during stream parsing — the flow-EOF final inspection covers it — +/// so it is not hookable; it also stays out of the id-to-name +/// table, so the hook listing and the firewall policy key walk +/// see the same surface as the name-to-id table. #[repr(u8)] #[derive(AppLayerState, Copy, Clone, PartialOrd, PartialEq, Eq)] #[suricata(alstate_strip_prefix = "SshState")] pub enum SSHConnectionState { - SshStateInProgress = 0, + SshStateBanner = 0, SshStateBannerWaitEol = 1, - SshStateBannerDone = 2, - SshStateFinished = 3, + SshStateKex = 2, + SshStateSession = 3, + SshStateDone = 4, } pub const SSH_MAX_BANNER_LEN: usize = 256; @@ -79,7 +87,7 @@ pub struct SshHeader { record_left: u32, record_left_msg: parser::MessageCode, - flags: SSHConnectionState, + state: SSHConnectionState, pub protover: Vec, pub swver: Vec, @@ -99,7 +107,7 @@ impl SshHeader { record_left: 0, record_left_msg: parser::MessageCode::Undefined(0), - flags: SSHConnectionState::SshStateInProgress, + state: SSHConnectionState::SshStateBanner, protover: Vec::new(), swver: Vec::new(), @@ -215,8 +223,8 @@ impl SSHState { } } parser::MessageCode::NewKeys => { - hdr.flags = SSHConnectionState::SshStateFinished; - if ohdr.flags >= SSHConnectionState::SshStateFinished { + hdr.state = SSHConnectionState::SshStateSession; + if ohdr.state >= SSHConnectionState::SshStateSession { let mut flags = 0; match encryption_bypass_mode() { @@ -278,7 +286,7 @@ impl SSHState { //header with rem as incomplete data match head.msg_code { parser::MessageCode::NewKeys => { - hdr.flags = SSHConnectionState::SshStateFinished; + hdr.state = SSHConnectionState::SshStateSession; } parser::MessageCode::Kexinit if hassh_is_enabled() => { // check if buffer is bigger than maximum reassembled packet size @@ -340,7 +348,7 @@ impl SSHState { } else { &mut self.transaction.srv_hdr }; - if hdr.flags == SSHConnectionState::SshStateBannerWaitEol { + if hdr.state == SSHConnectionState::SshStateBannerWaitEol { match parser::ssh_parse_line(input) { Ok((rem, _)) => { let mut r = self.parse_record(rem, resp, pstate, flow, stream_slice); @@ -375,7 +383,7 @@ impl SSHState { if !banner.swver.is_empty() { hdr.swver.extend(banner.swver); } - hdr.flags = SSHConnectionState::SshStateBannerDone; + hdr.state = SSHConnectionState::SshStateKex; } else { SCLogDebug!("SSH invalid banner"); self.set_event(SSHEvent::InvalidBanner); @@ -419,7 +427,7 @@ impl SSHState { if !banner.swver.is_empty() { hdr.swver.extend(banner.swver); } - hdr.flags = SSHConnectionState::SshStateBannerWaitEol; + hdr.state = SSHConnectionState::SshStateBannerWaitEol; self.set_event(SSHEvent::LongBanner); return AppLayerResult::ok(); } else { @@ -466,7 +474,7 @@ unsafe extern "C" fn ssh_parse_request( let buf = stream_slice.as_slice(); let hdr = &mut state.transaction.cli_hdr; state.transaction.tx_data.0.updated_ts = true; - if hdr.flags < SSHConnectionState::SshStateBannerDone { + if hdr.state < SSHConnectionState::SshStateKex { return state.parse_banner(buf, false, pstate, flow, &stream_slice); } else { return state.parse_record(buf, false, pstate, flow, &stream_slice); @@ -481,7 +489,7 @@ unsafe extern "C" fn ssh_parse_response( let buf = stream_slice.as_slice(); let hdr = &mut state.transaction.srv_hdr; state.transaction.tx_data.0.updated_tc = true; - if hdr.flags < SSHConnectionState::SshStateBannerDone { + if hdr.state < SSHConnectionState::SshStateKex { return state.parse_banner(buf, true, pstate, flow, &stream_slice); } else { return state.parse_record(buf, true, pstate, flow, &stream_slice); @@ -506,9 +514,9 @@ pub unsafe extern "C" fn SCSshTxGetFlags( ) -> SSHConnectionState { let tx = cast_pointer!(tx, SSHTransaction); if direction == u8::from(Direction::ToServer) { - return tx.cli_hdr.flags; + return tx.cli_hdr.state; } else { - return tx.srv_hdr.flags; + return tx.srv_hdr.state; } } @@ -517,22 +525,82 @@ pub unsafe extern "C" fn SCSshTxGetAlStateProgress( tx: *mut std::os::raw::c_void, direction: u8, ) -> std::os::raw::c_int { let tx = cast_pointer!(tx, SSHTransaction); + // per-direction: each direction reports its own phase; the completion + // state is only reached at flow end, so the tx stays inspectable + // until then + let progress = if direction == u8::from(Direction::ToServer) { + tx.cli_hdr.state + } else { + tx.srv_hdr.state + }; + return progress as i32; +} - if tx.cli_hdr.flags >= SSHConnectionState::SshStateFinished - && tx.srv_hdr.flags >= SSHConnectionState::SshStateFinished - { - return SSHConnectionState::SshStateFinished as i32; +// State name tables for rule hooks. "done" is intentionally absent +// from the name-to-id table: it is unreachable during stream +// parsing, so a hook on it would be dead configuration. Legacy +// pre-redesign names do not resolve (pre-production - breaking +// changes allowed). +unsafe extern "C" fn ssh_state_id_by_name( + name: *const std::os::raw::c_char, dir: u8, +) -> std::os::raw::c_int { + if name.is_null() { + return -1; } - - if direction == u8::from(Direction::ToServer) { - if tx.cli_hdr.flags >= SSHConnectionState::SshStateBannerDone { - return SSHConnectionState::SshStateBannerDone as i32; + let Ok(s) = std::ffi::CStr::from_ptr(name).to_str() else { + return -1; + }; + let s2 = match Direction::from(dir) { + Direction::ToServer => { + if !s.starts_with("request_") { + return -1; + } + &s["request_".len()..] + } + Direction::ToClient => { + if !s.starts_with("response_") { + return -1; + } + &s["response_".len()..] } - } else if tx.srv_hdr.flags >= SSHConnectionState::SshStateBannerDone { - return SSHConnectionState::SshStateBannerDone as i32; + }; + match s2 { + "banner" => SSHConnectionState::SshStateBanner as i32, + "banner_wait_eol" => SSHConnectionState::SshStateBannerWaitEol as i32, + "kex" => SSHConnectionState::SshStateKex as i32, + "session" => SSHConnectionState::SshStateSession as i32, + _ => -1, } +} - return SSHConnectionState::SshStateInProgress as i32; +extern "C" fn ssh_state_name_by_id( + id: std::os::raw::c_int, dir: u8, +) -> *const std::os::raw::c_char { + // process-lifetime statics + static NAMES_TS: [&[u8]; 4] = [ + b"request_banner\0", + b"request_banner_wait_eol\0", + b"request_kex\0", + b"request_session\0", + ]; + static NAMES_TC: [&[u8]; 4] = [ + b"response_banner\0", + b"response_banner_wait_eol\0", + b"response_kex\0", + b"response_session\0", + ]; + let names = if dir == u8::from(Direction::ToServer) { + &NAMES_TS + } else { + &NAMES_TC + }; + // the completion state stays out of the id-to-name table as well, + // so the hook listing and the firewall policy key walk see the + // same surface as the name-to-id table + match id { + 0..=3 => names[id as usize].as_ptr() as *const std::os::raw::c_char, + _ => std::ptr::null(), + } } // Parser name as a C style string. @@ -556,8 +624,8 @@ pub unsafe extern "C" fn SCRegisterSshParser() { parse_tc: ssh_parse_response, get_tx_count: ssh_state_get_tx_count, get_tx: SCSshStateGetTx, - tx_comp_st_ts: SSHConnectionState::SshStateFinished as i32, - tx_comp_st_tc: SSHConnectionState::SshStateFinished as i32, + tx_comp_st_ts: SSHConnectionState::SshStateDone as i32, + tx_comp_st_tc: SSHConnectionState::SshStateDone as i32, tx_get_progress: SCSshTxGetAlStateProgress, get_eventinfo: Some(SSHEvent::get_event_info), get_eventinfo_byid: Some(SSHEvent::get_event_info_by_id), @@ -571,8 +639,8 @@ pub unsafe extern "C" fn SCRegisterSshParser() { flags: 0, get_frame_id_by_name: Some(SshFrameType::ffi_id_from_name), get_frame_name_by_id: Some(SshFrameType::ffi_name_from_id), - get_state_id_by_name: Some(SSHConnectionState::ffi_id_from_name), - get_state_name_by_id: Some(SSHConnectionState::ffi_name_from_id), + get_state_id_by_name: Some(ssh_state_id_by_name), + get_state_name_by_id: Some(ssh_state_name_by_id), }; let ip_proto_str = CString::new("tcp").unwrap(); @@ -618,14 +686,19 @@ pub extern "C" fn SCSshEnableBypass(mode: EncryptionHandling) { pub unsafe extern "C" fn SCSshTxGetLogCondition(tx: *mut std::os::raw::c_void) -> bool { let tx = cast_pointer!(tx, SSHTransaction); + // >= rather than ==: a direction at session has necessarily + // passed kex, and a single delivery can take a direction + // banner -> kex -> session, so a final {session, kex} pair must + // still admit the log (done is never assigned, so >= changes + // only the kex branch) if SCSshHasshIsEnabled() { - if tx.cli_hdr.flags == SSHConnectionState::SshStateFinished - && tx.srv_hdr.flags == SSHConnectionState::SshStateFinished + if tx.cli_hdr.state >= SSHConnectionState::SshStateSession + && tx.srv_hdr.state >= SSHConnectionState::SshStateSession { return true; } - } else if tx.cli_hdr.flags == SSHConnectionState::SshStateBannerDone - && tx.srv_hdr.flags == SSHConnectionState::SshStateBannerDone + } else if tx.cli_hdr.state >= SSHConnectionState::SshStateKex + && tx.srv_hdr.state >= SSHConnectionState::SshStateKex { return true; } diff --git a/src/app-layer-ssh.c b/src/app-layer-ssh.c index 9152863e0b..271ab5222f 100644 --- a/src/app-layer-ssh.c +++ b/src/app-layer-ssh.c @@ -200,7 +200,7 @@ static int SSHParserTest01(void) void *tx = SCSshStateGetTx(ssh_state, 0); FAIL_IF_NULL(tx); - FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) != SshStateBannerDone); + FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) != SshStateKex); FAIL_IF(SSHParserTestUtilCheck("2.0", "MySSHClient-0.5.1", tx, STREAM_TOSERVER)); FLOW_DESTROY(&f); @@ -244,7 +244,7 @@ static int SSHParserTest02(void) } void *tx = SCSshStateGetTx(ssh_state, 0); - if (SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) != SshStateBannerDone) { + if (SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) != SshStateKex) { printf("Client version string not parsed: "); goto end; } @@ -295,7 +295,7 @@ static int SSHParserTest03(void) } void *tx = SCSshStateGetTx(ssh_state, 0); - if (SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) == SshStateBannerDone) { + if (SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) == SshStateKex) { printf("Client version string parsed? It's not a valid string: "); goto end; } @@ -348,7 +348,7 @@ static int SSHParserTest04(void) } void *tx = SCSshStateGetTx(ssh_state, 0); - if (SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) != SshStateBannerDone) { + if (SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) != SshStateKex) { printf("Client version string not parsed: "); goto end; } @@ -399,7 +399,7 @@ static int SSHParserTest05(void) } void *tx = SCSshStateGetTx(ssh_state, 0); - if (SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) != SshStateBannerDone) { + if (SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) != SshStateKex) { printf("Client version string not parsed: "); goto end; } @@ -450,7 +450,7 @@ static int SSHParserTest06(void) } void *tx = SCSshStateGetTx(ssh_state, 0); - if (SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) == SshStateBannerDone) { + if (SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) == SshStateKex) { printf("Client version string parsed? It's not a valid string: "); goto end; } @@ -511,7 +511,7 @@ static int SSHParserTest07(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) != SshStateBannerDone); + FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) != SshStateKex); FAIL_IF(SSHParserTestUtilCheck("2.0", "MySSHClient-0.5.1", tx, STREAM_TOSERVER)); @@ -562,7 +562,7 @@ static int SSHParserTest08(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) != SshStateBannerDone); + FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) != SshStateKex); FAIL_IF(SSHParserTestUtilCheck("2.0", "MySSHClient-0.5.1", tx, STREAM_TOSERVER)); @@ -612,7 +612,7 @@ static int SSHParserTest09(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) != SshStateBannerDone); + FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) != SshStateKex); FAIL_IF(SSHParserTestUtilCheck("2.0", "MySSHClient-0.5.1", tx, STREAM_TOCLIENT)); @@ -663,7 +663,7 @@ static int SSHParserTest10(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) != SshStateBannerDone); + FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) != SshStateKex); FAIL_IF(SSHParserTestUtilCheck("2.0", "MySSHClient-0.5.1", tx, STREAM_TOCLIENT)); @@ -714,7 +714,7 @@ static int SSHParserTest11(void) goto end; } void *tx = SCSshStateGetTx(ssh_state, 0); - if (SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateFinished) { + if (SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateSession) { printf("Didn't detect the msg code of new keys (ciphered data starts): "); goto end; } @@ -778,7 +778,7 @@ static int SSHParserTest12(void) goto end; } void *tx = SCSshStateGetTx(ssh_state, 0); - if (SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateFinished) { + if (SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateSession) { printf("Didn't detect the msg code of new keys (ciphered data starts): "); goto end; } @@ -839,7 +839,7 @@ static int SSHParserTest13(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateFinished); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateSession); FAIL_IF(SSHParserTestUtilCheck("2.0", "MySSHClient-0.5.1", tx, STREAM_TOSERVER)); @@ -898,7 +898,7 @@ static int SSHParserTest14(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateFinished); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateSession); FAIL_IF(SSHParserTestUtilCheck("2.0", "MySSHClient-0.5.1", tx, STREAM_TOSERVER)); @@ -956,7 +956,7 @@ static int SSHParserTest15(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateFinished); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateSession); FAIL_IF(SSHParserTestUtilCheck("2.0", "MySSHClient-0.5.1", tx, STREAM_TOSERVER)); @@ -1012,7 +1012,7 @@ static int SSHParserTest16(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateFinished); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateSession); FAIL_IF(SSHParserTestUtilCheck("2.0", "MySSHClient-0.5.1", tx, STREAM_TOCLIENT)); @@ -1069,7 +1069,7 @@ static int SSHParserTest17(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateFinished); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateSession); FAIL_IF(SSHParserTestUtilCheck("2.0", "MySSHClient-0.5.1", tx, STREAM_TOCLIENT)); @@ -1136,7 +1136,7 @@ static int SSHParserTest18(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateFinished); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateSession); FAIL_IF(!(SCAppLayerParserStateIssetFlag(f->alparser, APP_LAYER_PARSER_NO_INSPECTION))); @@ -1202,7 +1202,7 @@ static int SSHParserTest19(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateFinished); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateSession); sshbuf3[sizeof(sshbuf3) - 2] = 0; FAIL_IF(SSHParserTestUtilCheck("2.0", (char *)sshbuf3, tx, STREAM_TOCLIENT)); @@ -1271,7 +1271,7 @@ static int SSHParserTest20(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateFinished); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateSession); FAIL_IF(SSHParserTestUtilCheck("2.0", NULL, tx, STREAM_TOCLIENT)); @@ -1338,7 +1338,7 @@ static int SSHParserTest21(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateFinished); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateSession); FAIL_IF(SSHParserTestUtilCheck("2.0", NULL, tx, STREAM_TOCLIENT)); @@ -1429,7 +1429,7 @@ static int SSHParserTest22(void) void *ssh_state = f->alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateFinished); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOCLIENT) != SshStateSession); FAIL_IF(SSHParserTestUtilCheck("2.0", "libssh", tx, STREAM_TOCLIENT)); @@ -1507,7 +1507,7 @@ static int SSHParserTest24(void) goto end; } void *tx = SCSshStateGetTx(ssh_state, 0); - if (SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateBannerDone) { + if (SCSshTxGetFlags(tx, STREAM_TOSERVER) != SshStateKex) { printf("Didn't detect the msg code of new keys (ciphered data starts): "); goto end; } @@ -1549,7 +1549,7 @@ static int SSHParserTest25(void) void *ssh_state = f.alstate; FAIL_IF_NULL(ssh_state); void *tx = SCSshStateGetTx(ssh_state, 0); - FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOSERVER) == SshStateBannerDone); + FAIL_IF(SCSshTxGetFlags(tx, STREAM_TOSERVER) == SshStateKex); const uint8_t *dummy = NULL; uint32_t dummy_len = 0; FAIL_IF(SCSshTxGetSoftware(tx, STREAM_TOCLIENT, &dummy, &dummy_len) != 0); @@ -1560,6 +1560,108 @@ static int SSHParserTest25(void) PASS; } +/** \test State name table: new names, legacy aliases, the unhookable + * completion state and the id-to-name mapping. */ +static int SSHParserTest26(void) +{ + /* new names */ + FAIL_IF(AppLayerParserGetStateIdByName( + IPPROTO_TCP, ALPROTO_SSH, "request_banner", STREAM_TOSERVER) != SshStateBanner); + FAIL_IF(AppLayerParserGetStateIdByName(IPPROTO_TCP, ALPROTO_SSH, "request_banner_wait_eol", + STREAM_TOSERVER) != SshStateBannerWaitEol); + FAIL_IF(AppLayerParserGetStateIdByName( + IPPROTO_TCP, ALPROTO_SSH, "request_kex", STREAM_TOSERVER) != SshStateKex); + FAIL_IF(AppLayerParserGetStateIdByName(IPPROTO_TCP, ALPROTO_SSH, "request_session", + STREAM_TOSERVER) != SshStateSession); + FAIL_IF(AppLayerParserGetStateIdByName(IPPROTO_TCP, ALPROTO_SSH, "response_banner", + STREAM_TOCLIENT) != SshStateBanner); + FAIL_IF(AppLayerParserGetStateIdByName(IPPROTO_TCP, ALPROTO_SSH, "response_session", + STREAM_TOCLIENT) != SshStateSession); + + /* legacy names are rejected: no backward-compat aliases + * (pre-production — breaking changes are allowed) */ + FAIL_IF(AppLayerParserGetStateIdByName( + IPPROTO_TCP, ALPROTO_SSH, "request_in_progress", STREAM_TOSERVER) != -1); + FAIL_IF(AppLayerParserGetStateIdByName( + IPPROTO_TCP, ALPROTO_SSH, "request_banner_done", STREAM_TOSERVER) != -1); + FAIL_IF(AppLayerParserGetStateIdByName( + IPPROTO_TCP, ALPROTO_SSH, "request_finished", STREAM_TOSERVER) != -1); + FAIL_IF(AppLayerParserGetStateIdByName( + IPPROTO_TCP, ALPROTO_SSH, "response_finished", STREAM_TOCLIENT) != -1); + + /* the completion state is registered but not hookable */ + FAIL_IF(AppLayerParserGetStateIdByName( + IPPROTO_TCP, ALPROTO_SSH, "request_done", STREAM_TOSERVER) != -1); + FAIL_IF(AppLayerParserGetStateIdByName( + IPPROTO_TCP, ALPROTO_SSH, "response_done", STREAM_TOCLIENT) != -1); + + /* unknown name, and wrong direction prefix */ + FAIL_IF(AppLayerParserGetStateIdByName( + IPPROTO_TCP, ALPROTO_SSH, "request_nosuchstate", STREAM_TOSERVER) != -1); + FAIL_IF(AppLayerParserGetStateIdByName( + IPPROTO_TCP, ALPROTO_SSH, "response_banner", STREAM_TOSERVER) != -1); + + /* id to name */ + FAIL_IF(strcmp(AppLayerParserGetStateNameById( + IPPROTO_TCP, ALPROTO_SSH, SshStateBanner, STREAM_TOSERVER), + "request_banner") != 0); + FAIL_IF(strcmp(AppLayerParserGetStateNameById( + IPPROTO_TCP, ALPROTO_SSH, SshStateKex, STREAM_TOSERVER), + "request_kex") != 0); + FAIL_IF(strcmp(AppLayerParserGetStateNameById( + IPPROTO_TCP, ALPROTO_SSH, SshStateSession, STREAM_TOSERVER), + "request_session") != 0); + /* the completion state is out of the id-to-name table: the hook + * listing and the policy key walk must not offer it */ + FAIL_IF(AppLayerParserGetStateNameById( + IPPROTO_TCP, ALPROTO_SSH, SshStateDone, STREAM_TOSERVER) != NULL); + FAIL_IF(AppLayerParserGetStateNameById( + IPPROTO_TCP, ALPROTO_SSH, SshStateDone, STREAM_TOCLIENT) != NULL); + FAIL_IF(AppLayerParserGetStateNameById(IPPROTO_TCP, ALPROTO_SSH, 9, STREAM_TOSERVER) != NULL); + + PASS; +} + +/** \test Per-direction session state: the toserver direction reports + * session after its own NewKeys while the toclient direction has not + * seen anything yet. */ +static int SSHParserTest27(void) +{ + Flow f; + uint8_t sshbuf1[] = "SSH-2.0-MySSHClient-0.5.1\r\n"; + uint32_t sshlen1 = sizeof(sshbuf1) - 1; + uint8_t sshbuf2[] = { 0x00, 0x00, 0x00, 0x03, 0x01, 21, 0x00 }; + uint32_t sshlen2 = sizeof(sshbuf2); + TcpSession ssn; + AppLayerParserThreadCtx *alp_tctx = AppLayerParserThreadCtxAlloc(); + FAIL_IF_NULL(alp_tctx); + + memset(&f, 0, sizeof(f)); + memset(&ssn, 0, sizeof(ssn)); + FLOW_INITIALIZE(&f); + f.protoctx = (void *)&ssn; + f.proto = IPPROTO_TCP; + f.alproto = ALPROTO_SSH; + + StreamTcpInitConfig(true); + + int r = AppLayerParserParse(NULL, alp_tctx, &f, ALPROTO_SSH, STREAM_TOSERVER, sshbuf1, sshlen1); + FAIL_IF(r != 0); + r = AppLayerParserParse(NULL, alp_tctx, &f, ALPROTO_SSH, STREAM_TOSERVER, sshbuf2, sshlen2); + FAIL_IF(r != 0); + + void *ssh_state = f.alstate; + FAIL_IF_NULL(ssh_state); + void *tx = SCSshStateGetTx(ssh_state, 0); + FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOSERVER) != SshStateSession); + FAIL_IF(SCSshTxGetAlStateProgress(tx, STREAM_TOCLIENT) != SshStateBanner); + + FLOW_DESTROY(&f); + AppLayerParserThreadCtxFree(alp_tctx); + StreamTcpFreeConfig(true); + PASS; +} + #endif /* UNITTESTS */ void SSHParserRegisterTests(void) @@ -1590,6 +1692,8 @@ void SSHParserRegisterTests(void) UtRegisterTest("SSHParserTest23", SSHParserTest23); UtRegisterTest("SSHParserTest24", SSHParserTest24); UtRegisterTest("SSHParserTest25", SSHParserTest25); + UtRegisterTest("SSHParserTest26 - State name table", SSHParserTest26); + UtRegisterTest("SSHParserTest27 - Per-direction session state", SSHParserTest27); #endif /* UNITTESTS */ } diff --git a/src/detect-engine-prefilter.c b/src/detect-engine-prefilter.c index 103e25ea3b..cdef64ceda 100644 --- a/src/detect-engine-prefilter.c +++ b/src/detect-engine-prefilter.c @@ -820,8 +820,11 @@ static int TxNonPFAddSig(DetectEngineCtx *de_ctx, HashListTable *tx_engines_hash add->sigs_cnt++; char engine_name[128]; - snprintf(engine_name, sizeof(engine_name), "%s:%s:non_pf:%s", AppProtoToString(alproto), name, - dir == 0 ? "toserver" : "toclient"); + /* the id-to-name lookup can return null for a state id outside + * the protocol's table (e.g. the ssh completion state); a null + * %s argument is UB and would register a "(null)" engine name */ + snprintf(engine_name, sizeof(engine_name), "%s:%s:non_pf:%s", AppProtoToString(alproto), + name ? name : "unknown", dir == 0 ? "toserver" : "toclient"); char *engine_name_heap = SCStrdup(engine_name); if (engine_name_heap == NULL) { SCFree(add->sigs); diff --git a/src/detect-parse.c b/src/detect-parse.c index 82b7d6606f..714a7de33f 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -4529,6 +4529,17 @@ struct FwPolicyStateRenameHint { * key an error under the section of the protocol that renamed it, so a * cross-protocol typo stays a plain warning */ static const struct FwPolicyStateRenameHint g_fw_policy_state_rename_hints[] = { + /* ssh: the state scale was rebuilt; the parked long-banner state + * (banner-wait-eol) was removed - the banner state plus the + * one-shot ssh.long_banner event cover it */ + { ALPROTO_SSH, "request-in-progress", "request-banner" }, + { ALPROTO_SSH, "request-banner-wait-eol", "request-banner" }, + { ALPROTO_SSH, "request-banner-done", "request-kex" }, + { ALPROTO_SSH, "request-finished", "request-session" }, + { ALPROTO_SSH, "response-in-progress", "response-banner" }, + { ALPROTO_SSH, "response-banner-wait-eol", "response-banner" }, + { ALPROTO_SSH, "response-banner-done", "response-kex" }, + { ALPROTO_SSH, "response-finished", "response-session" }, { ALPROTO_TLS, "client-in-progress", "client-started" }, { ALPROTO_TLS, "client-hello-done", "client-hello" }, { ALPROTO_TLS, "client-cert-done", "client-cert" },