diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index e96bcd9a0b95..08722f7c7e9f 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -13,7 +13,11 @@ Describe changes: ### Provide values to any of the below to override the defaults. -To use a pull request use a branch name like `pr/N` where `N` is the pull request number. +To use a pull request use a branch name like `pr/N` where `N` is the +pull request number. + +Alternatively, `SV_BRANCH` may also be a link to an +OISF/suricata-verify pull-request. ``` SV_REPO= diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 73bb9135d6f9..69db01dfceb6 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -117,7 +117,13 @@ jobs: - name: Fetching suricata-verify run: | - pr=$(echo "${SV_BRANCH}" | sed -n 's/^pr\/\([[:digit:]]\+\)$/\1/p') + # Looking for a pull request number. in the SV_BRANCH + # value. This could be "pr/NNN", "pull/NNN" or a link to an + # OISF/suricata-verify pull request. + pr=$(echo "${SV_BRANCH}" | sed -n \ + -e 's/^https:\/\/github.com\/OISF\/suricata-verify\/pull\/\([0-9]*\)$/\1/p' \ + -e 's/^pull\/\([0-9]*\)$/\1/p' \ + -e 's/^pr\/\([0-9]*\)$/\1/p') if [ "${pr}" ]; then SV_BRANCH="refs/pull/${pr}/head" echo "Using suricata-verify pull-request ${SV_BRANCH}" diff --git a/doc/userguide/configuration/multi-tenant.rst b/doc/userguide/configuration/multi-tenant.rst index d2b0496b8779..e727facf5d9e 100644 --- a/doc/userguide/configuration/multi-tenant.rst +++ b/doc/userguide/configuration/multi-tenant.rst @@ -4,31 +4,31 @@ Multi Tenancy Introduction ------------ -Multi tenancy support allows for different rule sets with different -rule vars. These tenants can then be assigned to VLANs or interfaces -(devices). +Multi tenancy support allows different tenants to use different +rule sets with different rule variables. + +Tenants are identified by their `selector`; a `selector` can be +a VLAN, interface/device, or from a pcap file ("direct"). YAML ---- -In the main ("master") YAML, the suricata.yaml, a new section called -"multi-detect" should be added. +Add a new section in the main ("master") Suricata configuration file -- ``suricata.yaml`` -- named ``multi-detect``. Settings: -* enabled: yes/no -> is multi-tenancy support enabled -* default: yes/no -> is the normal detect config a default 'fall back' tenant? -* selector: direct (for unix socket pcap processing, see below), vlan or device -* loaders: number of 'loader' threads, for parallel tenant loading at startup -* tenants: list of tenants +* `enabled`: yes/no -> is multi-tenancy support enabled +* `selector`: direct (for unix socket pcap processing, see below), VLAN or device +* `loaders`: number of `loader` threads, for parallel tenant loading at startup +* `tenants`: list of tenants * id: tenant id (numeric values only) * yaml: separate yaml file with the tenant specific settings -* mappings: +* `mappings`: - * vlan id or device - * tenant id: tenant to associate with the vlan id / device + * VLAN id or device: The outermost VLAN is used to match. + * tenant id: tenant to associate with the VLAN id or device :: @@ -93,12 +93,13 @@ configuration: ... -vlanid -~~~~~~ +vlan-id +~~~~~~~ -Assign tenants to vlan id's. +Assign tenants to VLAN ids. Suricata matches the outermost VLAN id with this value. +Multiple VLANs can have the same tenant id. VLAN id values must be between 1 and 4094. -Example of vlan mapping:: +Example of VLAN mapping:: mappings: - vlan-id: 1000 @@ -110,13 +111,13 @@ Example of vlan mapping:: The mappings can also be modified over the unix socket, see below. -Note: can only be used if 'vlan.use-for-tracking' is enabled. +Note: can only be used if ``vlan.use-for-tracking`` is enabled. device ~~~~~~ Assign tenants to devices. A single tenant can be assigned to a device. -Multiple devices can have the same tenant. +Multiple devices can have the same tenant id. Example of device mapping:: @@ -152,7 +153,7 @@ Unix Socket Registration ~~~~~~~~~~~~ -register-tenant +``register-tenant `` Examples: @@ -164,7 +165,7 @@ Examples: register-tenant 5 tenant-5.yaml register-tenant 7 tenant-7.yaml -unregister-tenant +``unregister-tenant `` :: @@ -174,8 +175,8 @@ unregister-tenant Unix socket runmode (pcap processing) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -The Unix Socket "pcap-file" command can be used to select the tenant -to inspect the pcap against: +The Unix Socket ``pcap-file`` command is used to associate the tenant with +the pcap: :: @@ -191,22 +192,22 @@ traffic2.pcap against tenant 2 and logs to /logs2/ and so on. Live traffic mode ~~~~~~~~~~~~~~~~~ -For live traffic currently only a vlan based multi-tenancy is supported. +Multi-tenancy supports both VLAN and devices with live traffic. -The master yaml needs to have the selector set to "vlan". +In the master configuration yaml file, specify ``device`` or ``vlan`` for the ``selector`` setting. Registration ~~~~~~~~~~~~ -Tenants can be mapped to vlan id's. +Tenants can be mapped to vlan ids. -register-tenant-handler vlan +``register-tenant-handler vlan `` :: register-tenant-handler 1 vlan 1000 -unregister-tenant-handler vlan +``unregister-tenant-handler vlan `` :: diff --git a/doc/userguide/security.rst b/doc/userguide/security.rst index 381e6f23be14..415a5b38eacf 100644 --- a/doc/userguide/security.rst +++ b/doc/userguide/security.rst @@ -7,13 +7,13 @@ data. This combination deserves extra security precautions that we discuss below. Additionally, supply chain attacks, particularly around rule -distribution could potentially target Suricata installations. +distribution, could potentially target Suricata installations. Running as a User Other Than Root --------------------------------- .. note:: If using the Suricata RPMs, either from the OISF COPR repo, - or the EPEL repo the following is already configured for + or the EPEL repo, the following is already configured for you. The only thing you might want to do is add your management user to the ``suricata`` group. @@ -21,7 +21,7 @@ Many Suricata examples and guides will show Suricata running as the *root* user, particularly when running on live traffic. As Suricata generally needs low level read (and in IPS write) access to network traffic, it is required that Suricata starts as root, however Suricata -does have the ability to drop down to a non-root user after startup +does have the ability to drop down to a non-root user after startup, which could limit the impact of a security vulnerability in Suricata itself. @@ -31,7 +31,7 @@ itself. Create User ~~~~~~~~~~~ -Before running as a non-root user you have to choose, and possibly +Before running as a non-root user, you need to choose and possibly create the user and group that will Suricata will run as. Typically this user would be a sytem user with the name ``suricata``. Such a user can be created with the following command:: @@ -124,8 +124,9 @@ Containers ---------- Containers such as Docker and Podman are other methods to provide -isolation between Suricata and host machine running Suricata, however -we still recommend running as a non-root user even in containers. +isolation between Suricata and the host machine running Suricata. +However, we still recommend running as a non-root user, even in +containers. Capabilities ~~~~~~~~~~~~ @@ -141,5 +142,5 @@ Podman Unfortunately Suricata will not work with *rootless* Podman, this is due to Suricata's requirement to start with root privileges to gain access to the network interfaces. However, if started with the above -capabilities, and configured to run as a non-root user it will drop +capabilities, and configured to run as a non-root user, it will drop root privileges before processing network data. diff --git a/doc/userguide/unix-socket.rst b/doc/userguide/unix-socket.rst index 479579a065f9..08ae1c301db8 100644 --- a/doc/userguide/unix-socket.rst +++ b/doc/userguide/unix-socket.rst @@ -42,11 +42,13 @@ example to write custom scripts: Commands in standard running mode --------------------------------- -You may need to install ``suricatasc`` if you have not done so, running the following command from python/suricatasc +Runnable script for suricatasc is available in `python/bin` directory of suricata. You can +run it with the following commands. :: - sudo python setup.py install + cd python + sudo ./bin/suricatasc The set of existing commands is the following: diff --git a/python/Makefile.am b/python/Makefile.am index bfa78381830d..b3547192bd14 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -26,7 +26,7 @@ install-exec-local: install -d -m 0755 "$(DESTDIR)$(prefix)/lib/suricata/python/suricatasc" install -d -m 0755 "$(DESTDIR)$(prefix)/bin" for src in $(LIBS); do \ - install $(srcdir)/$$src "$(DESTDIR)$(prefix)/lib/suricata/python/$$src"; \ + install -m 0644 $(srcdir)/$$src "$(DESTDIR)$(prefix)/lib/suricata/python/$$src"; \ done install suricata/config/defaults.py \ "$(DESTDIR)$(prefix)/lib/suricata/python/suricata/config/defaults.py" diff --git a/python/bin/suricatasc b/python/bin/suricatasc index dc108da4af58..d090f856f54d 100755 --- a/python/bin/suricatasc +++ b/python/bin/suricatasc @@ -1,6 +1,6 @@ #! /usr/bin/env python # -# Copyright(C) 2013-2022 Open Information Security Foundation +# Copyright(C) 2013-2023 Open Information Security Foundation # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -69,8 +69,16 @@ except SuricataReturnException as err: sys.exit(1) if args.command: - (command, arguments) = sc.parse_command(args.command) - res = sc.send_command(command, arguments) + try: + (command, arguments) = sc.parse_command(args.command) + except SuricataCommandException as err: + print(err.value) + sys.exit(1) + try: + res = sc.send_command(command, arguments) + except (SuricataCommandException, SuricataReturnException) as err: + print(err.value) + sys.exit(1) print(json.dumps(res)) sc.close() if res['return'] == 'OK': diff --git a/python/setup.py b/python/setup.py deleted file mode 100644 index 2ee483b83e5c..000000000000 --- a/python/setup.py +++ /dev/null @@ -1,74 +0,0 @@ -from __future__ import print_function - -import os -import re -import sys -import shutil - -from distutils.core import setup -from distutils.command.build_py import build_py - -# Get the Suricata version from configure.ac. -version = None -if os.path.exists("../configure.ac"): - with open("../configure.ac", "r") as conf: - for line in conf: - if line.find("AC_INIT") > 1: - m = re.search("AC_INIT\(\[suricata\],\[(\d.+)\]\)", line) - if m: - version = m.group(1) - break - else: - print("error: failed to parse Suricata version from: %s" % ( - line.strip()), file=sys.stderr) - sys.exit(1) -if version is None: - print("error: failed to find Suricata version", file=sys.stderr) - sys.exit(1) - -class do_build(build_py): - def run(self): - build_py.run(self) - defaults_py_out = os.path.join( - self.build_lib, "suricata", "config", "defaults.py") - if not os.path.exists(defaults_py_out): - # Must be an out of tree build, find defaults.py. - defaults_py_in = os.path.join( - self.build_lib, "..", "suricata", "config", "defaults.py") - if os.path.exists(defaults_py_in): - shutil.copy(defaults_py_in, defaults_py_out) - else: - print("error: failed to find defaults.py") - sys.exit(1) - -setup( - name="suricata", - description="Suricata control tools", - version=version, - author='OISF Developers, Eric Leblond', - author_email='oisf-devel@lists.openinfosecfoundation.org, eric@regit.org', - url='https://www.suricata.io/', - packages=[ - "suricata", - "suricata.config", - "suricata.ctl", - "suricata.sc", - "suricatasc", - ], - scripts=[ - "bin/suricatactl", - "bin/suricatasc", - ], - provides=['suricatactl', 'suricatasc'], - requires=['argparse','simplejson'], - classifiers=[ - 'Development Status :: 5 - Production/Stable', - 'Environment :: Console', - 'Intended Audience :: System Administrators', - 'License :: OSI Approved :: GNU General Public License (GPL)', - 'Operating System :: POSIX', - 'Programming Language :: Python', - 'Topic :: System :: Systems Administration', - ], - cmdclass={'build_py': do_build}, -) diff --git a/python/suricata/sc/suricatasc.py b/python/suricata/sc/suricatasc.py index d4f15fdd45a0..bc7de375360e 100644 --- a/python/suricata/sc/suricatasc.py +++ b/python/suricata/sc/suricatasc.py @@ -1,5 +1,4 @@ -#!/usr/bin/python -# Copyright(C) 2012-2020 Open Information Security Foundation +# Copyright(C) 2012-2023 Open Information Security Foundation # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -22,6 +21,7 @@ import select import sys from socket import AF_UNIX, error, socket +from inspect import currentframe from suricata.sc.specs import argsd @@ -30,6 +30,11 @@ INC_SIZE = 1024 +def get_linenumber(): + cf = currentframe() + return cf.f_back.f_lineno + + class SuricataException(Exception): """ Generic class for suricatasc exception @@ -136,7 +141,7 @@ def json_recv(self): def send_command(self, command, arguments=None): if command not in self.cmd_list and command != 'command-list': - raise SuricataCommandException("Command not found: {}".format(command)) + raise SuricataCommandException("L{}: Command not found: {}".format(get_linenumber(), command)) cmdmsg = {} cmdmsg['command'] = command @@ -156,7 +161,7 @@ def send_command(self, command, arguments=None): else: cmdret = None if not cmdret: - raise SuricataReturnException("Unable to get message from server") + raise SuricataReturnException("L{}: Unable to get message from server".format(get_linenumber)) if self.verbose: print("RCV: "+ json.dumps(cmdret)) @@ -169,7 +174,7 @@ def connect(self): self.socket = socket(AF_UNIX) self.socket.connect(self.sck_path) except error as err: - raise SuricataNetException(err) + raise SuricataNetException("L{}: {}".format(get_linenumber(), err)) self.socket.settimeout(10) #send version @@ -187,13 +192,13 @@ def connect(self): cmdret = None if not cmdret: - raise SuricataReturnException("Unable to get message from server") + raise SuricataReturnException("L{}: Unable to get message from server".format(get_linenumber())) if self.verbose: print("RCV: "+ json.dumps(cmdret)) if cmdret["return"] == "NOK": - raise SuricataReturnException("Error: %s" % (cmdret["message"])) + raise SuricataReturnException("L{}: Error: {}".format(get_linenumber(), cmdret["message"])) cmdret = self.send_command("command-list") @@ -223,9 +228,9 @@ def execute(self, command): except IndexError: phrase = " at least" if required_args_count != len(cmd_specs) else "" msg = "Missing arguments: expected{} {}".format(phrase, required_args_count) - raise SuricataCommandException(msg) + raise SuricataCommandException("L{}: {}".format(get_linenumber(), msg)) except ValueError as ve: - raise SuricataCommandException("Erroneous arguments: {}".format(ve)) + raise SuricataCommandException("L{}: Erroneous arguments: {}".format(get_linenumber(), ve)) elif c < len(full_cmd): arguments[spec["name"]] = spec_type(full_cmd[c]) return cmd, arguments @@ -237,7 +242,7 @@ def parse_command(self, command): if cmd in self.fn_commands: cmd, arguments = getattr(self, "execute")(command=command) else: - raise SuricataCommandException("Unknown command {}".format(command)) + raise SuricataCommandException("L{}: Unknown command: {}".format(get_linenumber(), command)) return cmd, arguments def interactive(self): @@ -253,6 +258,8 @@ def interactive(self): command = input(">>> ").strip() if command == "quit": break + if len(command.strip()) == 0: + continue try: cmd, arguments = self.parse_command(command) except SuricataCommandException as err: @@ -266,10 +273,13 @@ def interactive(self): try: self.close() self.connect() - except SuricataNetException as err: - print("Can't reconnect to suricata socket, discarding command") + except (SuricataNetException, SuricataReturnException) as err: + print(err.value) continue cmdret = self.send_command(cmd, arguments) + except (SuricataCommandException, SuricataReturnException) as err: + print("An exception occured: " + str(err.value)) + continue #decode json message if cmdret["return"] == "NOK": print("Error:") @@ -279,3 +289,4 @@ def interactive(self): print(json.dumps(cmdret["message"], sort_keys=True, indent=4, separators=(',', ': '))) except KeyboardInterrupt: print("[!] Interrupted") + sys.exit(0) diff --git a/rules/Makefile.am b/rules/Makefile.am index 4c0c744c9b8c..d0ea6eda622f 100644 --- a/rules/Makefile.am +++ b/rules/Makefile.am @@ -17,6 +17,7 @@ mqtt-events.rules \ nfs-events.rules \ ntp-events.rules \ quic-events.rules \ +rfb-events.rules \ smb-events.rules \ smtp-events.rules \ ssh-events.rules \ diff --git a/rules/http-events.rules b/rules/http-events.rules index 6376c807fcff..8c7763f1b661 100644 --- a/rules/http-events.rules +++ b/rules/http-events.rules @@ -89,4 +89,6 @@ alert http any any -> any any (msg:"SURICATA HTTP file name too long"; flow:esta alert http any any -> any any (msg:"SURICATA HTTP failed protocol change"; flow:established; app-layer-event:http.failed_protocol_change; flowint:http.anomaly.count,+,1; classtype:protocol-command-decode; sid:2221053; rev:1;) -# next sid 2221054 +#alert http any any -> any any (msg:"SURICATA HTTP request chunk extension"; flow:established; app-layer-event:http.request_chunk_extension; classtype:protocol-command-decode; sid:2221054; rev:1;) + +# next sid 2221055 diff --git a/rules/rfb-events.rules b/rules/rfb-events.rules new file mode 100644 index 000000000000..08bc493f4270 --- /dev/null +++ b/rules/rfb-events.rules @@ -0,0 +1,10 @@ +# RFB app-layer event rules. +# +# These SIDs fall in the 2233000+ range. See: +# http://doc.emergingthreats.net/bin/view/Main/SidAllocation and +# https://redmine.openinfosecfoundation.org/projects/suricata/wiki/AppLayer + +alert rfb any any -> any any (msg:"SURICATA RFB Malformed or unknown message"; app-layer-event:rfb.malformed_message; classtype:protocol-command-decode; sid:2233000; rev:1;) +alert rfb any any -> any any (msg:"SURICATA RFB Unimplemented security type"; app-layer-event:rfb.unimplemented_security_type; classtype:protocol-command-decode; sid:2233001; rev:1;) +alert rfb any any -> any any (msg:"SURICATA RFB Unknown security result"; app-layer-event:rfb.unknown_security_result; classtype:protocol-command-decode; sid:2233002; rev:1;) +alert rfb any any -> any any (msg:"SURICATA RFB Unexpected State in Parser"; app-layer-event:rfb.confused_state; classtype:protocol-command-decode; sid:2233003; rev:1;) diff --git a/rust/src/rfb/detect.rs b/rust/src/rfb/detect.rs index 7cfd7776b301..1cf0887c4fda 100644 --- a/rust/src/rfb/detect.rs +++ b/rust/src/rfb/detect.rs @@ -22,9 +22,7 @@ use std::ptr; #[no_mangle] pub unsafe extern "C" fn rs_rfb_tx_get_name( - tx: &mut RFBTransaction, - buffer: *mut *const u8, - buffer_len: *mut u32, + tx: &mut RFBTransaction, buffer: *mut *const u8, buffer_len: *mut u32, ) -> u8 { if let Some(ref r) = tx.tc_server_init { let p = &r.name; @@ -42,10 +40,7 @@ pub unsafe extern "C" fn rs_rfb_tx_get_name( } #[no_mangle] -pub unsafe extern "C" fn rs_rfb_tx_get_sectype( - tx: &mut RFBTransaction, - sectype: *mut u32, -) -> u8 { +pub unsafe extern "C" fn rs_rfb_tx_get_sectype(tx: &mut RFBTransaction, sectype: *mut u32) -> u8 { if let Some(ref r) = tx.chosen_security_type { *sectype = *r; return 1; @@ -58,8 +53,7 @@ pub unsafe extern "C" fn rs_rfb_tx_get_sectype( #[no_mangle] pub unsafe extern "C" fn rs_rfb_tx_get_secresult( - tx: &mut RFBTransaction, - secresult: *mut u32, + tx: &mut RFBTransaction, secresult: *mut u32, ) -> u8 { if let Some(ref r) = tx.tc_security_result { *secresult = r.status; @@ -67,4 +61,4 @@ pub unsafe extern "C" fn rs_rfb_tx_get_secresult( } return 0; -} \ No newline at end of file +} diff --git a/rust/src/rfb/logger.rs b/rust/src/rfb/logger.rs index 392b825b19d3..62bb20966fe7 100644 --- a/rust/src/rfb/logger.rs +++ b/rust/src/rfb/logger.rs @@ -17,10 +17,10 @@ // Author: Frank Honza -use std; -use std::fmt::Write; use super::rfb::RFBTransaction; use crate::jsonbuilder::{JsonBuilder, JsonError}; +use std; +use std::fmt::Write; fn log_rfb(tx: &RFBTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> { js.open_object("rfb")?; @@ -64,15 +64,17 @@ fn log_rfb(tx: &RFBTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> { } js.close()?; } - _ => () + _ => (), } if let Some(security_result) = &tx.tc_security_result { let _ = match security_result.status { 0 => js.set_string("security_result", "OK")?, 1 => js.set_string("security-result", "FAIL")?, 2 => js.set_string("security_result", "TOOMANY")?, - _ => js.set_string("security_result", - &format!("UNKNOWN ({})", security_result.status))?, + _ => js.set_string( + "security_result", + &format!("UNKNOWN ({})", security_result.status), + )?, }; } js.close()?; // Close authentication. @@ -92,15 +94,27 @@ fn log_rfb(tx: &RFBTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> { js.set_string_from_bytes("name", &tc_server_init.name)?; js.open_object("pixel_format")?; - js.set_uint("bits_per_pixel", tc_server_init.pixel_format.bits_per_pixel as u64)?; + js.set_uint( + "bits_per_pixel", + tc_server_init.pixel_format.bits_per_pixel as u64, + )?; js.set_uint("depth", tc_server_init.pixel_format.depth as u64)?; - js.set_bool("big_endian", tc_server_init.pixel_format.big_endian_flag != 0)?; - js.set_bool("true_color", tc_server_init.pixel_format.true_colour_flag != 0)?; + js.set_bool( + "big_endian", + tc_server_init.pixel_format.big_endian_flag != 0, + )?; + js.set_bool( + "true_color", + tc_server_init.pixel_format.true_colour_flag != 0, + )?; js.set_uint("red_max", tc_server_init.pixel_format.red_max as u64)?; js.set_uint("green_max", tc_server_init.pixel_format.green_max as u64)?; js.set_uint("blue_max", tc_server_init.pixel_format.blue_max as u64)?; js.set_uint("red_shift", tc_server_init.pixel_format.red_shift as u64)?; - js.set_uint("green_shift", tc_server_init.pixel_format.green_shift as u64)?; + js.set_uint( + "green_shift", + tc_server_init.pixel_format.green_shift as u64, + )?; js.set_uint("blue_shift", tc_server_init.pixel_format.blue_shift as u64)?; js.close()?; @@ -113,8 +127,9 @@ fn log_rfb(tx: &RFBTransaction, js: &mut JsonBuilder) -> Result<(), JsonError> { } #[no_mangle] -pub unsafe extern "C" fn rs_rfb_logger_log(tx: *mut std::os::raw::c_void, - js: &mut JsonBuilder) -> bool { +pub unsafe extern "C" fn rs_rfb_logger_log( + tx: *mut std::os::raw::c_void, js: &mut JsonBuilder, +) -> bool { let tx = cast_pointer!(tx, RFBTransaction); log_rfb(tx, js).is_ok() } diff --git a/rust/src/rfb/mod.rs b/rust/src/rfb/mod.rs index 4bde7935cb71..68d37ec8a0eb 100644 --- a/rust/src/rfb/mod.rs +++ b/rust/src/rfb/mod.rs @@ -20,4 +20,4 @@ pub mod detect; pub mod logger; pub mod parser; -pub mod rfb; \ No newline at end of file +pub mod rfb; diff --git a/rust/src/rfb/parser.rs b/rust/src/rfb/parser.rs index 0fba65333a36..73958062d2d6 100644 --- a/rust/src/rfb/parser.rs +++ b/rust/src/rfb/parser.rs @@ -17,6 +17,7 @@ // Author: Frank Honza +use nom7::bytes::streaming::tag; use nom7::bytes::streaming::take; use nom7::combinator::map_res; use nom7::number::streaming::*; @@ -24,7 +25,7 @@ use nom7::*; use std::fmt; use std::str; -#[derive(Debug,PartialEq)] +#[derive(Debug, PartialEq)] pub enum RFBGlobalState { TCServerProtocolVersion, TCSupportedSecurityTypes, @@ -37,7 +38,7 @@ pub enum RFBGlobalState { TSVncResponse, TCSecurityResult, TSClientInit, - Message, + Skip, } impl fmt::Display for RFBGlobalState { @@ -54,7 +55,7 @@ impl fmt::Display for RFBGlobalState { RFBGlobalState::TCSecurityResult => write!(f, "TCSecurityResult"), RFBGlobalState::TCServerSecurityType => write!(f, "TCServerSecurityType"), RFBGlobalState::TSClientInit => write!(f, "TSClientInit"), - RFBGlobalState::Message => write!(f, "Message"), + RFBGlobalState::Skip => write!(f, "Skip"), } } } @@ -115,12 +116,11 @@ pub struct ServerInit { } pub fn parse_protocol_version(i: &[u8]) -> IResult<&[u8], ProtocolVersion> { - let (i, _rfb_string) = map_res(take(3_usize), str::from_utf8)(i)?; - let (i, _) = be_u8(i)?; + let (i, _) = tag("RFB ")(i)?; let (i, major) = map_res(take(3_usize), str::from_utf8)(i)?; - let (i, _) = be_u8(i)?; + let (i, _) = tag(".")(i)?; let (i, minor) = map_res(take(3_usize), str::from_utf8)(i)?; - let (i, _) = be_u8(i)?; + let (i, _) = tag("\n")(i)?; Ok(( i, ProtocolVersion { diff --git a/rust/src/rfb/rfb.rs b/rust/src/rfb/rfb.rs index 1e568788f514..940417e8311b 100644 --- a/rust/src/rfb/rfb.rs +++ b/rust/src/rfb/rfb.rs @@ -16,6 +16,7 @@ */ // Author: Frank Honza +// Sascha Steinbiss use super::parser; use crate::applayer; @@ -28,6 +29,14 @@ use std::ffi::CString; static mut ALPROTO_RFB: AppProto = ALPROTO_UNKNOWN; +#[derive(FromPrimitive, Debug, AppLayerEvent)] +pub enum RFBEvent { + UnimplementedSecurityType, + UnknownSecurityResult, + MalformedMessage, + ConfusedState, +} + #[derive(AppLayerFrameType)] pub enum RFBFrameType { Pdu, @@ -86,6 +95,10 @@ impl RFBTransaction { tx_data: applayer::AppLayerTxData::new(), } } + + fn set_event(&mut self, event: RFBEvent) { + self.tx_data.set_event(event as u8); + } } pub struct RFBState { @@ -195,7 +208,9 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.ts_client_protocol_version = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!( + "no transaction set at protocol selection stage" + ); } } Err(Err::Incomplete(_)) => { @@ -205,6 +220,7 @@ impl RFBState { ); } Err(_) => { + // We even failed to parse the protocol version. return AppLayerResult::err(); } } @@ -227,7 +243,18 @@ impl RFBState { match chosen_security_type { 2 => self.state = parser::RFBGlobalState::TCVncChallenge, 1 => self.state = parser::RFBGlobalState::TSClientInit, - _ => return AppLayerResult::err(), + _ => { + if let Some(current_transaction) = self.get_current_tx() { + current_transaction + .set_event(RFBEvent::UnimplementedSecurityType); + } + // We have just have seen a security type we don't know about. + // This is not bad per se, it might just mean this is a + // proprietary one not in the spec. + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); + } } if let Some(current_transaction) = self.get_current_tx() { @@ -235,7 +262,7 @@ impl RFBState { current_transaction.chosen_security_type = Some(chosen_security_type as u32); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at security type stage"); } } Err(Err::Incomplete(_)) => { @@ -245,7 +272,13 @@ impl RFBState { ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // We failed to parse the security type. + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -267,7 +300,7 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.ts_vnc_response = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at security result stage"); } } Err(Err::Incomplete(_)) => { @@ -277,7 +310,12 @@ impl RFBState { ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } }, parser::RFBGlobalState::TSClientInit => match parser::parse_client_init(current) { @@ -298,7 +336,7 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.ts_client_init = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at client init stage"); } } Err(Err::Incomplete(_)) => { @@ -308,20 +346,34 @@ impl RFBState { ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // We failed to parse the client init. + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } }, - parser::RFBGlobalState::Message => { - //todo implement RFB messages, for now we stop here - return AppLayerResult::err(); - } - parser::RFBGlobalState::TCServerProtocolVersion => { - SCLogDebug!("Reversed traffic, expected response."); - return AppLayerResult::err(); + parser::RFBGlobalState::Skip => { + // End of parseable handshake reached, skip rest of traffic + return AppLayerResult::ok(); } _ => { - SCLogDebug!("Invalid state for request {}", self.state); - current = b""; + // We have gotten out of sync with the expected state flow. + // This could happen since we use a global state (i.e. that + // is used for both directions), but if traffic can not be + // parsed as expected elsewhere, we might not have advanced + // a state for one direction but received data in the + // "unexpected" direction, causing the parser to end up + // here. Let's stop trying to parse the traffic but still + // accept it. + SCLogDebug!("Invalid state for request: {}", self.state); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::ConfusedState); + } + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -367,7 +419,7 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_server_protocol_version = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set but we just set one"); } } Err(Err::Incomplete(_)) => { @@ -377,6 +429,7 @@ impl RFBState { ); } Err(_) => { + // We even failed to parse the protocol version. return AppLayerResult::err(); } } @@ -414,7 +467,7 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_supported_security_types = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at security type stage"); } } Err(Err::Incomplete(_)) => { @@ -424,7 +477,12 @@ impl RFBState { ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -450,8 +508,20 @@ impl RFBState { 1 => self.state = parser::RFBGlobalState::TSClientInit, 2 => self.state = parser::RFBGlobalState::TCVncChallenge, _ => { - // TODO Event unknown security type - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction + .set_event(RFBEvent::UnimplementedSecurityType); + } else { + debug_validate_fail!( + "no transaction set at security type stage" + ); + } + // We have just have seen a security type we don't know about. + // This is not bad per se, it might just mean this is a + // proprietary one not in the spec. + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } @@ -460,7 +530,7 @@ impl RFBState { current_transaction.chosen_security_type = Some(chosen_security_type); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at security type stage"); } } Err(Err::Incomplete(_)) => { @@ -470,7 +540,12 @@ impl RFBState { ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -492,7 +567,7 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_vnc_challenge = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at auth stage"); } } Err(Err::Incomplete(_)) => { @@ -502,7 +577,12 @@ impl RFBState { ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } }, parser::RFBGlobalState::TCSecurityResult => { @@ -525,12 +605,19 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_security_result = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!( + "no transaction set at security result stage" + ); } } else if request.status == 1 { self.state = parser::RFBGlobalState::TCFailureReason; } else { - // TODO: Event: unknown security result value + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::UnknownSecurityResult); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } Err(Err::Incomplete(_)) => { @@ -540,7 +627,12 @@ impl RFBState { ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -550,9 +642,9 @@ impl RFBState { if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_failure_reason = Some(request); } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at failure reason stage"); } - return AppLayerResult::err(); + return AppLayerResult::ok(); } Err(Err::Incomplete(_)) => { return AppLayerResult::incomplete( @@ -561,7 +653,12 @@ impl RFBState { ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -579,14 +676,14 @@ impl RFBState { current = rem; - self.state = parser::RFBGlobalState::Message; + self.state = parser::RFBGlobalState::Skip; if let Some(current_transaction) = self.get_current_tx() { current_transaction.tc_server_init = Some(request); // connection initialization is complete and parsed current_transaction.complete = true; } else { - return AppLayerResult::err(); + debug_validate_fail!("no transaction set at server init stage"); } } Err(Err::Incomplete(_)) => { @@ -596,17 +693,34 @@ impl RFBState { ); } Err(_) => { - return AppLayerResult::err(); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::MalformedMessage); + } + // Continue the flow but stop trying to map the protocol. + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } - parser::RFBGlobalState::Message => { + parser::RFBGlobalState::Skip => { //todo implement RFB messages, for now we stop here - return AppLayerResult::err(); + return AppLayerResult::ok(); } _ => { - SCLogDebug!("Invalid state for response"); - return AppLayerResult::err(); + // We have gotten out of sync with the expected state flow. + // This could happen since we use a global state (i.e. that + // is used for both directions), but if traffic can not be + // parsed as expected elsewhere, we might not have advanced + // a state for one direction but received data in the + // "unexpected" direction, causing the parser to end up + // here. Let's stop trying to parse the traffic but still + // accept it. + SCLogDebug!("Invalid state for response: {}", self.state); + if let Some(current_transaction) = self.get_current_tx() { + current_transaction.set_event(RFBEvent::ConfusedState); + } + self.state = parser::RFBGlobalState::Skip; + return AppLayerResult::ok(); } } } @@ -712,8 +826,8 @@ pub unsafe extern "C" fn rs_rfb_register_parser() { tx_comp_st_ts: 1, tx_comp_st_tc: 1, tx_get_progress: rs_rfb_tx_get_alstate_progress, - get_eventinfo: None, - get_eventinfo_byid: None, + get_eventinfo: Some(RFBEvent::get_event_info), + get_eventinfo_byid: Some(RFBEvent::get_event_info_by_id), localstorage_new: None, localstorage_free: None, get_tx_files: None, @@ -756,7 +870,7 @@ mod test { 0x67, 0x6c, 0x65, 0x73, 0x40, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x68, 0x6f, 0x73, 0x74, 0x2e, 0x6c, 0x6f, 0x63, 0x61, 0x6c, 0x64, 0x6f, 0x6d, 0x61, 0x69, 0x6e, ]; - let r = state.parse_request( + let r = state.parse_response( std::ptr::null(), StreamSlice::from_slice(buf, STREAM_START, 0), ); @@ -861,7 +975,7 @@ mod test { std::ptr::null(), StreamSlice::from_slice(&buf[36..90], STREAM_START, 0), ); - ok_state = parser::RFBGlobalState::Message; + ok_state = parser::RFBGlobalState::Skip; assert_eq!(init_state.state, ok_state); } } diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index b2c915d9342d..f3e0ad2e81ff 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -166,6 +166,7 @@ SCEnumCharMap http_decoder_event_table[] = { { "COMPRESSION_BOMB", HTTP_DECODER_EVENT_COMPRESSION_BOMB }, { "RANGE_INVALID", HTTP_DECODER_EVENT_RANGE_INVALID }, + { "REQUEST_CHUNK_EXTENSION", HTTP_DECODER_EVENT_REQUEST_CHUNK_EXTENSION }, /* suricata warnings/errors */ { "MULTIPART_GENERIC_ERROR", HTTP_DECODER_EVENT_MULTIPART_GENERIC_ERROR }, @@ -598,37 +599,47 @@ struct { const char *msg; uint8_t de; } htp_warnings[] = { - { "GZip decompressor:", HTTP_DECODER_EVENT_GZIP_DECOMPRESSION_FAILED}, - { "Request field invalid", HTTP_DECODER_EVENT_REQUEST_HEADER_INVALID}, - { "Response field invalid", HTTP_DECODER_EVENT_RESPONSE_HEADER_INVALID}, - { "Request header name is not a token", HTTP_DECODER_EVENT_REQUEST_HEADER_INVALID}, - { "Response header name is not a token", HTTP_DECODER_EVENT_RESPONSE_HEADER_INVALID}, -/* { "Host information in request headers required by HTTP/1.1", HTTP_DECODER_EVENT_MISSING_HOST_HEADER}, <- tx flag HTP_HOST_MISSING - { "Host information ambiguous", HTTP_DECODER_EVENT_HOST_HEADER_AMBIGUOUS}, <- tx flag HTP_HOST_AMBIGUOUS */ - { "Invalid request field folding", HTTP_DECODER_EVENT_INVALID_REQUEST_FIELD_FOLDING}, - { "Invalid response field folding", HTTP_DECODER_EVENT_INVALID_RESPONSE_FIELD_FOLDING}, - /* line is now: htp_log(connp, HTP_LOG_MARK, HTP_LOG_ERROR, 0, "Request server port=%d number differs from the actual TCP port=%d", port, connp->conn->server_port); - * luckily, "Request server port=" is unique */ -/* { "Request server port number differs from the actual TCP port", HTTP_DECODER_EVENT_REQUEST_SERVER_PORT_TCP_PORT_MISMATCH}, */ - { "Request server port=", HTTP_DECODER_EVENT_REQUEST_SERVER_PORT_TCP_PORT_MISMATCH}, - { "Request line: URI contains non-compliant delimiter", HTTP_DECODER_EVENT_URI_DELIM_NON_COMPLIANT}, - { "Request line: non-compliant delimiter between Method and URI", HTTP_DECODER_EVENT_METHOD_DELIM_NON_COMPLIANT}, - { "Request line: leading whitespace", HTTP_DECODER_EVENT_REQUEST_LINE_LEADING_WHITESPACE}, - { "Too many response content encoding layers", HTTP_DECODER_EVENT_TOO_MANY_ENCODING_LAYERS}, - { "C-E gzip has abnormal value", HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER}, - { "C-E deflate has abnormal value", HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER}, - { "C-E unknown setting", HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER}, - { "Excessive request header repetitions", HTTP_DECODER_EVENT_REQUEST_HEADER_REPETITION}, - { "Excessive response header repetitions", HTTP_DECODER_EVENT_RESPONSE_HEADER_REPETITION}, - { "Transfer-encoding has abnormal chunked value", HTTP_DECODER_EVENT_RESPONSE_ABNORMAL_TRANSFER_ENCODING}, - { "Chunked transfer-encoding on HTTP/0.9 or HTTP/1.0", HTTP_DECODER_EVENT_RESPONSE_CHUNKED_OLD_PROTO}, - { "Invalid response line: invalid protocol", HTTP_DECODER_EVENT_RESPONSE_INVALID_PROTOCOL}, - { "Invalid response line: invalid response status", HTTP_DECODER_EVENT_RESPONSE_INVALID_STATUS}, - { "Request line incomplete", HTTP_DECODER_EVENT_REQUEST_LINE_INCOMPLETE}, - { "Unexpected request body", HTTP_DECODER_EVENT_REQUEST_BODY_UNEXPECTED}, - { "LZMA decompressor: memory limit reached", HTTP_DECODER_EVENT_LZMA_MEMLIMIT_REACHED}, - { "Ambiguous request C-L value", HTTP_DECODER_EVENT_DUPLICATE_CONTENT_LENGTH_FIELD_IN_REQUEST}, - { "Ambiguous response C-L value", HTTP_DECODER_EVENT_DUPLICATE_CONTENT_LENGTH_FIELD_IN_RESPONSE}, + { "GZip decompressor:", HTTP_DECODER_EVENT_GZIP_DECOMPRESSION_FAILED }, + { "Request field invalid", HTTP_DECODER_EVENT_REQUEST_HEADER_INVALID }, + { "Response field invalid", HTTP_DECODER_EVENT_RESPONSE_HEADER_INVALID }, + { "Request header name is not a token", HTTP_DECODER_EVENT_REQUEST_HEADER_INVALID }, + { "Response header name is not a token", HTTP_DECODER_EVENT_RESPONSE_HEADER_INVALID }, + /* { "Host information in request headers required by HTTP/1.1", + HTTP_DECODER_EVENT_MISSING_HOST_HEADER}, <- tx flag HTP_HOST_MISSING { "Host information + ambiguous", HTTP_DECODER_EVENT_HOST_HEADER_AMBIGUOUS}, <- tx flag HTP_HOST_AMBIGUOUS */ + { "Invalid request field folding", HTTP_DECODER_EVENT_INVALID_REQUEST_FIELD_FOLDING }, + { "Invalid response field folding", HTTP_DECODER_EVENT_INVALID_RESPONSE_FIELD_FOLDING }, + /* line is now: htp_log(connp, HTP_LOG_MARK, HTP_LOG_ERROR, 0, "Request server port=%d number + * differs from the actual TCP port=%d", port, connp->conn->server_port); luckily, "Request + * server port=" is unique */ + /* { "Request server port number differs from the actual TCP port", + HTTP_DECODER_EVENT_REQUEST_SERVER_PORT_TCP_PORT_MISMATCH}, */ + { "Request server port=", HTTP_DECODER_EVENT_REQUEST_SERVER_PORT_TCP_PORT_MISMATCH }, + { "Request line: URI contains non-compliant delimiter", + HTTP_DECODER_EVENT_URI_DELIM_NON_COMPLIANT }, + { "Request line: non-compliant delimiter between Method and URI", + HTTP_DECODER_EVENT_METHOD_DELIM_NON_COMPLIANT }, + { "Request line: leading whitespace", HTTP_DECODER_EVENT_REQUEST_LINE_LEADING_WHITESPACE }, + { "Too many response content encoding layers", HTTP_DECODER_EVENT_TOO_MANY_ENCODING_LAYERS }, + { "C-E gzip has abnormal value", HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER }, + { "C-E deflate has abnormal value", HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER }, + { "C-E unknown setting", HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER }, + { "Excessive request header repetitions", HTTP_DECODER_EVENT_REQUEST_HEADER_REPETITION }, + { "Excessive response header repetitions", HTTP_DECODER_EVENT_RESPONSE_HEADER_REPETITION }, + { "Transfer-encoding has abnormal chunked value", + HTTP_DECODER_EVENT_RESPONSE_ABNORMAL_TRANSFER_ENCODING }, + { "Chunked transfer-encoding on HTTP/0.9 or HTTP/1.0", + HTTP_DECODER_EVENT_RESPONSE_CHUNKED_OLD_PROTO }, + { "Invalid response line: invalid protocol", HTTP_DECODER_EVENT_RESPONSE_INVALID_PROTOCOL }, + { "Invalid response line: invalid response status", + HTTP_DECODER_EVENT_RESPONSE_INVALID_STATUS }, + { "Request line incomplete", HTTP_DECODER_EVENT_REQUEST_LINE_INCOMPLETE }, + { "Unexpected request body", HTTP_DECODER_EVENT_REQUEST_BODY_UNEXPECTED }, + { "LZMA decompressor: memory limit reached", HTTP_DECODER_EVENT_LZMA_MEMLIMIT_REACHED }, + { "Ambiguous request C-L value", HTTP_DECODER_EVENT_DUPLICATE_CONTENT_LENGTH_FIELD_IN_REQUEST }, + { "Ambiguous response C-L value", + HTTP_DECODER_EVENT_DUPLICATE_CONTENT_LENGTH_FIELD_IN_RESPONSE }, + { "Request chunk extension", HTTP_DECODER_EVENT_REQUEST_CHUNK_EXTENSION }, }; #define HTP_ERROR_MAX (sizeof(htp_errors) / sizeof(htp_errors[0])) diff --git a/src/app-layer-htp.h b/src/app-layer-htp.h index 5adda7343ecc..c8c3a7f7b987 100644 --- a/src/app-layer-htp.h +++ b/src/app-layer-htp.h @@ -127,6 +127,7 @@ enum { HTTP_DECODER_EVENT_COMPRESSION_BOMB, HTTP_DECODER_EVENT_RANGE_INVALID, + HTTP_DECODER_EVENT_REQUEST_CHUNK_EXTENSION, /* suricata errors/warnings */ HTTP_DECODER_EVENT_MULTIPART_GENERIC_ERROR, diff --git a/src/detect-http-header.c b/src/detect-http-header.c index cc11cdf0c0da..e5101f9276b0 100644 --- a/src/detect-http-header.c +++ b/src/detect-http-header.c @@ -750,6 +750,7 @@ void DetectHttpRequestHeaderRegister(void) DetectBufferTypeSetDescriptionByName("http_request_header", "HTTP header name and value"); g_http_request_header_buffer_id = DetectBufferTypeGetByName("http_request_header"); + DetectBufferTypeSupportsMultiInstance("http_request_header"); } static int DetectHTTPResponseHeaderSetup(DetectEngineCtx *de_ctx, Signature *s, const char *arg) @@ -784,6 +785,7 @@ void DetectHttpResponseHeaderRegister(void) DetectBufferTypeSetDescriptionByName("http_response_header", "HTTP header name and value"); g_http_response_header_buffer_id = DetectBufferTypeGetByName("http_response_header"); + DetectBufferTypeSupportsMultiInstance("http_response_header"); } /************************************Unittests*********************************/ diff --git a/src/log-pcap.c b/src/log-pcap.c index 3238f2a076ab..1e1b6da1fb55 100644 --- a/src/log-pcap.c +++ b/src/log-pcap.c @@ -179,6 +179,8 @@ typedef struct PcapLogData_ { char *filename_parts[MAX_TOKS]; int filename_part_cnt; struct timeval last_pcap_dump; + int fopen_err; /**< set to the last fopen error */ + bool pcap_open_err; /**< true if the last pcap open errored */ PcapLogCompressionData compression; } PcapLogData; @@ -422,22 +424,41 @@ static int PcapLogOpenHandles(PcapLogData *pl, const Packet *p) if (pl->compression.format == PCAP_LOG_COMPRESSION_FORMAT_NONE) { if ((pl->pcap_dumper = pcap_dump_open(pl->pcap_dead_handle, pl->filename)) == NULL) { - SCLogInfo("Error opening dump file %s", pcap_geterr(pl->pcap_dead_handle)); + if (!pl->pcap_open_err) { + SCLogError("Error opening dump file %s", pcap_geterr(pl->pcap_dead_handle)); + pl->pcap_open_err = true; + } return TM_ECODE_FAILED; + } else { + pl->pcap_open_err = false; } } #ifdef HAVE_LIBLZ4 else if (pl->compression.format == PCAP_LOG_COMPRESSION_FORMAT_LZ4) { PcapLogCompressionData *comp = &pl->compression; - if ((pl->pcap_dumper = pcap_dump_fopen(pl->pcap_dead_handle, - comp->pcap_buf_wrapper)) == NULL) { - SCLogError("Error opening dump file %s", pcap_geterr(pl->pcap_dead_handle)); - return TM_ECODE_FAILED; - } + comp->file = fopen(pl->filename, "w"); if (comp->file == NULL) { - SCLogError("Error opening file for compressed output: %s", strerror(errno)); + if (errno != pl->fopen_err) { + SCLogError("Error opening file for compressed output: %s", strerror(errno)); + pl->fopen_err = errno; + } + return TM_ECODE_FAILED; + } else { + pl->fopen_err = 0; + } + + if ((pl->pcap_dumper = pcap_dump_fopen(pl->pcap_dead_handle, comp->pcap_buf_wrapper)) == + NULL) { + if (!pl->pcap_open_err) { + SCLogError("Error opening dump file %s", pcap_geterr(pl->pcap_dead_handle)); + pl->pcap_open_err = true; + } + fclose(comp->file); + comp->file = NULL; return TM_ECODE_FAILED; + } else { + pl->pcap_open_err = false; } uint64_t bytes_written = LZ4F_compressBegin(comp->lz4f_context, @@ -1166,6 +1187,10 @@ static void PcapLogDataFree(PcapLogData *pl) SCFree(pl->filename); SCFree(pl->prefix); + if (pl->pcap_dead_handle) { + pcap_close(pl->pcap_dead_handle); + } + #ifdef HAVE_LIBLZ4 if (pl->compression.format == PCAP_LOG_COMPRESSION_FORMAT_LZ4) { SCFree(pl->compression.buffer); diff --git a/src/output-json-alert.c b/src/output-json-alert.c index 472e6f98cded..e0ea7545ab97 100644 --- a/src/output-json-alert.c +++ b/src/output-json-alert.c @@ -1017,20 +1017,15 @@ static void JsonAlertLogSetupMetadata(AlertJsonOutputCtx *json_output_ctx, SetFlag(conf, "http-body-printable", LOG_JSON_HTTP_BODY, &flags); SetFlag(conf, "http-body", LOG_JSON_HTTP_BODY_BASE64, &flags); - /* Check for obsolete configuration flags to enable specific - * protocols. These are now just aliases for enabling - * app-layer logging. */ - SetFlag(conf, "http", LOG_JSON_APP_LAYER, &flags); - SetFlag(conf, "tls", LOG_JSON_APP_LAYER, &flags); - SetFlag(conf, "ssh", LOG_JSON_APP_LAYER, &flags); - SetFlag(conf, "smtp", LOG_JSON_APP_LAYER, &flags); - SetFlag(conf, "dnp3", LOG_JSON_APP_LAYER, &flags); - - /* And check for obsolete configuration flags for enabling - * app-layer and flow as these have been moved under the - * metadata key. */ - SetFlag(conf, "app-layer", LOG_JSON_APP_LAYER, &flags); - SetFlag(conf, "flow", LOG_JSON_FLOW, &flags); + /* Check for obsolete flags and warn that they have no effect. */ + static const char *deprecated_flags[] = { "http", "tls", "ssh", "smtp", "dnp3", "app-layer", + "flow", NULL }; + for (int i = 0; deprecated_flags[i] != NULL; i++) { + if (ConfNodeLookupChildValue(conf, deprecated_flags[i]) != NULL) { + SCLogWarning("Found deprecated eve-log.alert flag \"%s\", this flag has no effect", + deprecated_flags[i]); + } + } const char *payload_buffer_value = ConfNodeLookupChildValue(conf, "payload-buffer-size"); diff --git a/src/util-decode-mime.c b/src/util-decode-mime.c index c96f9afa8b69..141325b56ea3 100644 --- a/src/util-decode-mime.c +++ b/src/util-decode-mime.c @@ -1120,22 +1120,13 @@ static int ProcessDecodedDataChunk(const uint8_t *chunk, uint32_t len, tok = GetLine( remainPtr, len - (remainPtr - (uint8_t *)chunk), &remainPtr, &tokLen); if (tok != remainPtr) { - /* If last token found without CR/LF delimiter, then save - * and reconstruct with next chunk - */ - if (tok + tokLen - (uint8_t *)chunk == (int)len) { - PrintChars(SC_LOG_DEBUG, "LAST CHUNK LINE - CUTOFF", tok, tokLen); - SCLogDebug("\nCHUNK CUTOFF CHARS: %u delim %u\n", tokLen, - len - (uint32_t)(tok + tokLen - (uint8_t *)chunk)); - } else { - /* Search line for URL */ - ret = FindUrlStrings(tok, tokLen, state); - if (ret != MIME_DEC_OK) { - SCLogDebug("Error: FindUrlStrings() function" - " failed: %d", - ret); - break; - } + /* Search line for URL */ + ret = FindUrlStrings(tok, tokLen, state); + if (ret != MIME_DEC_OK) { + SCLogDebug("Error: FindUrlStrings() function" + " failed: %d", + ret); + break; } } } while (tok != remainPtr && remainPtr - (uint8_t *)chunk < (int)len); @@ -1577,7 +1568,7 @@ static int ProcessBodyLine(const uint8_t *buf, uint32_t len, } } else { /* Process non-decoded content */ - remaining = len + state->current_line_delimiter_len; + remaining = len; offset = 0; while (remaining > 0) { /* Plan to add CRLF to the end of each line */ @@ -1609,6 +1600,16 @@ static int ProcessBodyLine(const uint8_t *buf, uint32_t len, remaining -= tobuf; offset += tobuf; } + if (ret == MIME_DEC_OK) { + ret = ProcessDecodedDataChunk(state->data_chunk, state->data_chunk_len, state); + if (ret != MIME_DEC_OK) { + SCLogDebug("Error: ProcessDecodedDataChunk() function " + "failed"); + } + } + // keep end of line for next call (and skip it on completion) + memcpy(state->data_chunk, buf + offset, state->current_line_delimiter_len); + state->data_chunk_len = state->current_line_delimiter_len; } return ret; @@ -2040,6 +2041,11 @@ static int ProcessBodyComplete(MimeDecParseState *state) } } + MimeDecEntity *entity = (MimeDecEntity *)state->stack->top->data; + if ((entity->ctnt_flags & (CTNT_IS_BASE64 | CTNT_IS_QP)) == 0) { + // last eol of plaintext is the beginning of the boundary + state->data_chunk_len = 0; + } /* Invoke pre-processor and callback with remaining data */ ret = ProcessDecodedDataChunk(state->data_chunk, state->data_chunk_len, state); if (ret != MIME_DEC_OK) { @@ -2208,17 +2214,6 @@ static int ProcessMimeBody(const uint8_t *buf, uint32_t len, int body_found = 0; uint16_t tlen; - if (!g_disable_hashing) { - if (MimeDecGetConfig()->body_md5) { - if (state->body_begin == 1) { - if (state->md5_ctx == NULL) { - state->md5_ctx = SCMd5New(); - } - } - SCMd5Update(state->md5_ctx, buf, len + state->current_line_delimiter_len); - } - } - /* pass empty lines on if we're parsing the body, otherwise we have no use * for them, and in fact they would disrupt the state tracking */ if (len == 0) { @@ -2348,6 +2343,18 @@ static int ProcessMimeEntity(const uint8_t *buf, uint32_t len, MAX_LINE_LEN); } + if (!g_disable_hashing) { + if ((state->state_flag != HEADER_READY && state->state_flag != HEADER_STARTED) || + (state->stack->top->data->ctnt_flags & CTNT_IS_BODYPART)) { + if (MimeDecGetConfig()->body_md5) { + if (state->body_begin == 1 && state->md5_ctx == NULL) { + state->md5_ctx = SCMd5New(); + } + SCMd5Update(state->md5_ctx, buf, len + state->current_line_delimiter_len); + } + } + } + /* Looking for headers */ if (state->state_flag == HEADER_READY || state->state_flag == HEADER_STARTED) { @@ -2655,24 +2662,19 @@ static int TestDataChunkCallback(const uint8_t *chunk, uint32_t len, /* Add up the line counts */ if (len > 0) { - - uint8_t *remainPtr; - uint8_t *tok; - uint32_t tokLen; - + if ((*line_count) == 0) { + (*line_count)++; + } PrintChars(SC_LOG_DEBUG, "CHUNK", chunk, len); - - /* Parse each line one by one */ - remainPtr = (uint8_t *) chunk; - do { - tok = GetLine(remainPtr, len - (remainPtr - (uint8_t *) chunk), - &remainPtr, &tokLen); - if (tok != NULL && tok != remainPtr) { + for (uint32_t i = 0; i < len; i++) { + if (chunk[i] == CR || chunk[i] == LF) { + if (i + 1 < len && chunk[i] != chunk[i + 1] && + (chunk[i + 1] == CR || chunk[i + 1] == LF)) { + i++; + } (*line_count)++; } - - } while (tok != NULL && tok != remainPtr && - (uint32_t)(remainPtr - (uint8_t *) chunk) < len); + } SCLogDebug("line count (len=%u): %u", len, *line_count); }