From b8897ce809b72e226b3bcaecab52962c455ee2d7 Mon Sep 17 00:00:00 2001 From: Lukas Sismis Date: Thu, 19 Sep 2024 09:24:01 +0200 Subject: [PATCH 1/4] ssh: add option to select behavior of encrypted parts Ticket: 6788 --- rust/src/ssh/ssh.rs | 56 +++++++++++++++++++++++++++++++++++++++------ src/app-layer-ssh.c | 21 +++++++++++++++++ suricata.yaml.in | 8 +++++++ 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/rust/src/ssh/ssh.rs b/rust/src/ssh/ssh.rs index 99c88c9d7cf5..88d347c9d474 100644 --- a/rust/src/ssh/ssh.rs +++ b/rust/src/ssh/ssh.rs @@ -22,14 +22,34 @@ use nom7::Err; use std::ffi::CString; use std::sync::atomic::{AtomicBool, Ordering}; use crate::frames::Frame; +use std::sync::Mutex; + +#[repr(C)] +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[allow(non_camel_case_types)] +pub enum SshEncryptionHandling { + SSH_HANDLE_ENCRYPTION_DEFAULT = 0, // Disable raw content, continue tracking + SSH_HANDLE_ENCRYPTION_BYPASS = 1, // Skip processing of flow, bypass if possible + SSH_HANDLE_ENCRYPTION_FULL = 2, // Handle fully like any other protocol +} static mut ALPROTO_SSH: AppProto = ALPROTO_UNKNOWN; static HASSH_ENABLED: AtomicBool = AtomicBool::new(false); +lazy_static::lazy_static! { + static ref ENCRYPTION_BYPASS_ENABLED: Mutex = + Mutex::new(SshEncryptionHandling::SSH_HANDLE_ENCRYPTION_DEFAULT); +} + fn hassh_is_enabled() -> bool { HASSH_ENABLED.load(Ordering::Relaxed) } +fn encryption_bypass_mode() -> SshEncryptionHandling { + let mode = ENCRYPTION_BYPASS_ENABLED.lock().unwrap(); + *mode +} + #[derive(AppLayerFrameType)] pub enum SshFrameType { RecordHdr, @@ -194,13 +214,24 @@ impl SSHState { parser::MessageCode::NewKeys => { hdr.flags = SSHConnectionState::SshStateFinished; if ohdr.flags >= SSHConnectionState::SshStateFinished { - unsafe { - AppLayerParserStateSetFlag( - pstate, - APP_LAYER_PARSER_NO_INSPECTION - | APP_LAYER_PARSER_NO_REASSEMBLY - | APP_LAYER_PARSER_BYPASS_READY, - ); + let mut flags = 0; + + match encryption_bypass_mode() { + SshEncryptionHandling::SSH_HANDLE_ENCRYPTION_BYPASS => { + flags |= APP_LAYER_PARSER_NO_INSPECTION + | APP_LAYER_PARSER_NO_REASSEMBLY + | APP_LAYER_PARSER_BYPASS_READY; + } + SshEncryptionHandling::SSH_HANDLE_ENCRYPTION_DEFAULT => { + flags |= APP_LAYER_PARSER_NO_INSPECTION; + } + _ => {} + } + + if flags != 0 { + unsafe { + AppLayerParserStateSetFlag(pstate, flags); + } } } } @@ -551,6 +582,17 @@ pub extern "C" fn rs_ssh_hassh_is_enabled() -> bool { hassh_is_enabled() } +#[no_mangle] +pub extern "C" fn SCSshEnableBypass(mode: SshEncryptionHandling) { + let mut bypass_mode = ENCRYPTION_BYPASS_ENABLED.lock().unwrap(); + *bypass_mode = mode; +} + +#[no_mangle] +pub extern "C" fn SCSshEncryptionBypassMode() -> SshEncryptionHandling { + encryption_bypass_mode() +} + #[no_mangle] pub unsafe extern "C" fn rs_ssh_tx_get_log_condition( tx: *mut std::os::raw::c_void) -> bool { let tx = cast_pointer!(tx, SSHTransaction); diff --git a/src/app-layer-ssh.c b/src/app-layer-ssh.c index 71bc786ad6b4..595e067af17b 100644 --- a/src/app-layer-ssh.c +++ b/src/app-layer-ssh.c @@ -55,6 +55,8 @@ /* HASSH fingerprints are disabled by default */ #define SSH_CONFIG_DEFAULT_HASSH false +/* Bypassing the encrypted part of the connections */ +#define SSH_CONFIG_DEFAULT_ENCRYPTION_BYPASS SSH_HANDLE_ENCRYPTION_DEFAULT static int SSHRegisterPatternsForProtocolDetection(void) { @@ -103,6 +105,25 @@ void RegisterSSHParsers(void) if (RunmodeIsUnittests() || enable_hassh) { rs_ssh_enable_hassh(); } + + SshEncryptionHandling encryption_bypass = SSH_CONFIG_DEFAULT_ENCRYPTION_BYPASS; + ConfNode *encryption_node = ConfGetNode("app-layer.protocols.ssh.encryption-handling"); + if (encryption_node != NULL && encryption_node->val != NULL) { + if (strcmp(encryption_node->val, "full") == 0) { + encryption_bypass = SSH_HANDLE_ENCRYPTION_FULL; + } else if (strcmp(encryption_node->val, "default") == 0) { + encryption_bypass = SSH_HANDLE_ENCRYPTION_DEFAULT; + } else if (strcmp(encryption_node->val, "bypass") == 0) { + encryption_bypass = SSH_HANDLE_ENCRYPTION_BYPASS; + } else { + encryption_bypass = SSH_CONFIG_DEFAULT_ENCRYPTION_BYPASS; + } + } + + if (encryption_bypass) { + SCLogConfig("ssh: bypass on the start of encryption enabled"); + SCSshEnableBypass(encryption_bypass); + } } SCLogDebug("Registering Rust SSH parser."); diff --git a/suricata.yaml.in b/suricata.yaml.in index 4bc9e87aa2af..8fc902f7630c 100644 --- a/suricata.yaml.in +++ b/suricata.yaml.in @@ -944,6 +944,14 @@ app-layer: ssh: enabled: yes #hassh: yes + + # What to do when the encrypted communications start: + # - default: keep tracking but stop inspection + # - full: keep tracking and inspect as normal + # - bypass: stop processing this flow as much as possible. + # Offload flow bypass to kernel or hardware if possible. + # + # encryption-handling: default doh2: enabled: yes http2: From 65504b36c15e4772f01b2c76013ecafbb9f2257e Mon Sep 17 00:00:00 2001 From: Dean Balandin Date: Tue, 27 Jun 2023 12:40:37 +0000 Subject: [PATCH 2/4] stream: decouple stream.bypass dependency from tls bypass Decouple app.protocols.tls.encryption-handling and stream.bypass. There's no apparent reason why encrypted TLS bypass traffic should depend on stream bypass, as these are unrelated features. Ticket: 6788 --- doc/userguide/configuration/suricata-yaml.rst | 28 ++++++++++--------- .../performance/ignoring-traffic.rst | 12 +++++--- doc/userguide/upgrade.rst | 11 ++++++++ src/stream-tcp.c | 8 ++---- 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/doc/userguide/configuration/suricata-yaml.rst b/doc/userguide/configuration/suricata-yaml.rst index 8004ecce9167..b4ba1ecb44dd 100644 --- a/doc/userguide/configuration/suricata-yaml.rst +++ b/doc/userguide/configuration/suricata-yaml.rst @@ -1827,21 +1827,23 @@ Encrypted traffic There is no decryption of encrypted traffic, so once the handshake is complete continued tracking of the session is of limited use. The ``encryption-handling`` -option controls the behavior after the handshake. +option in ``app-layer.protocols.tls`` and ``app-layer.protocols.ssh`` controls +the behavior after the handshake. -If ``encryption-handling`` is set to ``default`` (or if the option is not set), -Suricata will continue to track the SSL/TLS session. Inspection will be limited, -as raw ``content`` inspection will still be disabled. There is no point in doing -pattern matching on traffic known to be encrypted. Inspection for (encrypted) -Heartbleed and other protocol anomalies still happens. +If ``encryption-handling`` in TLS protocol is set to ``default`` +(or if the option is not set), Suricata will continue to track the SSL/TLS +session. Inspection will be limited, as raw ``content`` inspection will still +be disabled. There is no point in doing pattern matching on traffic known to +be encrypted. Inspection for (encrypted) Heartbleed and other protocol +anomalies still happens. -When ``encryption-handling`` is set to ``bypass``, all processing of this session is -stopped. No further parsing and inspection happens. If ``stream.bypass`` is enabled -this will lead to the flow being bypassed, either inside Suricata or by the -capture method if it supports it and is configured for it. +When ``encryption-handling`` is set to ``bypass``, all processing of this +session is stopped. No further parsing and inspection happens. This will also +lead to the flow being bypassed, either inside Suricata or by the capture method +if it supports it and is configured for it. -Finally, if ``encryption-handling`` is set to ``full``, Suricata will process the -flow as normal, without inspection limitations or bypass. +Finally, if ``encryption-handling`` is set to ``full``, Suricata will process +the flow as normal, without inspection limitations or bypass. The option has replaced the ``no-reassemble`` option. If ``no-reassemble`` is present, and ``encryption-handling`` is not, ``false`` is interpreted as @@ -2868,7 +2870,7 @@ The Teredo decoder can be disabled. It is enabled by default. ports: $TEREDO_PORTS # syntax: '[3544, 1234]' Using this default configuration, Teredo detection will run on UDP port -3544. If the `ports` parameter is missing, or set to `any`, all ports will be +1. If the `ports` parameter is missing, or set to `any`, all ports will be inspected for possible presence of Teredo. Advanced Options diff --git a/doc/userguide/performance/ignoring-traffic.rst b/doc/userguide/performance/ignoring-traffic.rst index a2c7a8825528..147849195070 100644 --- a/doc/userguide/performance/ignoring-traffic.rst +++ b/doc/userguide/performance/ignoring-traffic.rst @@ -73,10 +73,14 @@ Example:: encrypted traffic ----------------- -The TLS app layer parser has the ability to stop processing encrypted traffic -after the initial handshake. By setting the `app-layer.protocols.tls.encryption-handling` -option to `bypass` the rest of this flow is ignored. If flow bypass is enabled, -the bypass is done in the kernel or in hardware. +The TLS and SSH app layer parsers have the ability to stop processing +encrypted traffic after the initial handshake. By setting the +`app-layer.protocols.tls.encryption-handling` and +`app-layer.protocols.ssh.encryption-handling` options to `bypass` Suricata +bypasses flows once the handshake is completed and encrypted traffic is +detected. The rest of these flow is ignored. +The bypass is done in the kernel or in hardware, similar to how flow bypass +is done. .. _bypass: diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index 4bf74b65284d..4481c6226802 100644 --- a/doc/userguide/upgrade.rst +++ b/doc/userguide/upgrade.rst @@ -82,6 +82,17 @@ Major changes - Unknown requirements in the ``requires`` keyword will now be treated as unmet requirements, causing the rule to not be loaded. See :ref:`keyword_requires`. +- Encrypted traffic bypass has been decoupled from stream.bypass setting. + This means that encrypted traffic can be bypassed while tracking/fully + inspecting other traffic as well. +- Encrypted SSH traffic bypass is now independently controlled through + `app-layer.protocols.ssh.encryption-handling` setting. The setting can either + be `bypass`, `default` or `full`. + To retain the previous behavior of encrypted traffic bypass + combined with stream depth bypass, set + `app-layer.protocols.ssh.encryption-handling` to `bypass` (while also + setting `app-layer.protocols.tls.encryption-handling` to `bypass` and + `stream.bypass` to `true`). Removals ~~~~~~~~ diff --git a/src/stream-tcp.c b/src/stream-tcp.c index 7352f2bdc74f..03efb618d515 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -5692,17 +5692,13 @@ int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt, } if (ssn->flags & STREAMTCP_FLAG_BYPASS) { - /* we can call bypass callback, if enabled */ - if (StreamTcpBypassEnabled()) { - PacketBypassCallback(p); - } - - /* if stream is dead and we have no detect engine at all, bypass. */ + PacketBypassCallback(p); } else if (g_detect_disabled && (ssn->client.flags & STREAMTCP_STREAM_FLAG_NOREASSEMBLY) && (ssn->server.flags & STREAMTCP_STREAM_FLAG_NOREASSEMBLY) && StreamTcpBypassEnabled()) { + /* if stream is dead and we have no detect engine at all, bypass. */ SCLogDebug("bypass as stream is dead and we have no rules"); PacketBypassCallback(p); } From 0aa01ddc3af43138e4a503be8d76f7c26631646e Mon Sep 17 00:00:00 2001 From: Lukas Sismis Date: Thu, 19 Sep 2024 09:49:39 +0200 Subject: [PATCH 3/4] doc: remove trailing spaces in DPDK section --- doc/userguide/configuration/suricata-yaml.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/userguide/configuration/suricata-yaml.rst b/doc/userguide/configuration/suricata-yaml.rst index b4ba1ecb44dd..25b420e67f8e 100644 --- a/doc/userguide/configuration/suricata-yaml.rst +++ b/doc/userguide/configuration/suricata-yaml.rst @@ -2171,12 +2171,12 @@ are typically provided through the command line, are contained in the node parameters. There are two ways to specify arguments: lengthy and short. Dashes are omitted when describing the arguments. This setup node can be used to set up the memory configuration, accessible NICs, and other EAL-related -parameters, among other things. The node `dpdk.eal-params` also supports -multiple arguments of the same type. This can be useful for EAL arguments -such as `--vdev`, `--allow`, or `--block`. Values for these EAL arguments -are specified as a comma-separated list. -An example of such usage can be found in the example above where the `allow` -argument only makes `0000:3b:00.0` and `0000:3b:00.1` accessible to Suricata. +parameters, among other things. The node `dpdk.eal-params` also supports +multiple arguments of the same type. This can be useful for EAL arguments +such as `--vdev`, `--allow`, or `--block`. Values for these EAL arguments +are specified as a comma-separated list. +An example of such usage can be found in the example above where the `allow` +argument only makes `0000:3b:00.0` and `0000:3b:00.1` accessible to Suricata. arguments with list node. such as --vdev, --allow, --block eal options. The definition of lcore affinity as an EAL parameter is a standard practice. However, lcore parameters like `-l`, `-c`, From 541bd34a6d27bd073cee5129fb6c66036496370e Mon Sep 17 00:00:00 2001 From: Lukas Sismis Date: Thu, 19 Sep 2024 09:25:22 +0200 Subject: [PATCH 4/4] ssh: sync the hassh setting with the defaults --- suricata.yaml.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/suricata.yaml.in b/suricata.yaml.in index 8fc902f7630c..97f2518a1728 100644 --- a/suricata.yaml.in +++ b/suricata.yaml.in @@ -943,7 +943,7 @@ app-layer: #enabled: yes ssh: enabled: yes - #hassh: yes + # hassh: no # What to do when the encrypted communications start: # - default: keep tracking but stop inspection