From 4bcdc79ed8366053947a25cd0f86ceba33e5495d Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Mon, 20 Nov 2023 14:54:45 +0100 Subject: [PATCH 1/9] stats: always use tcp/udp prefix Even when on detection-only mode. So that we always have enip_tcp and enip_udp in stats and never just `enip`. Ticket: 6304 --- src/app-layer.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/app-layer.c b/src/app-layer.c index 3625e87e9ed6..102319042bcc 100644 --- a/src/app-layer.c +++ b/src/app-layer.c @@ -1061,16 +1061,18 @@ void AppLayerSetupCounters(void) for (uint8_t p = 0; p < IPPROTOS_MAX; p++) { const uint8_t ipproto = ipprotos[p]; const uint8_t ipproto_map = FlowGetProtoMapping(ipproto); - const uint8_t other_ipproto = ipproto == IPPROTO_TCP ? IPPROTO_UDP : IPPROTO_TCP; const char *ipproto_suffix = (ipproto == IPPROTO_TCP) ? "_tcp" : "_udp"; + uint8_t ipprotos_all[256 / 8]; for (AppProto alproto = 0; alproto < ALPROTO_MAX; alproto++) { if (alprotos[alproto] == 1) { const char *tx_str = "app_layer.tx."; const char *alproto_str = AppLayerGetProtoName(alproto); - if (AppLayerParserProtoIsRegistered(ipproto, alproto) && - AppLayerParserProtoIsRegistered(other_ipproto, alproto)) { + memset(ipprotos_all, 0, sizeof(ipprotos_all)); + AppLayerProtoDetectSupportedIpprotos(alproto, ipprotos_all); + if ((ipprotos_all[IPPROTO_TCP / 8] & (1 << (IPPROTO_TCP % 8))) && + (ipprotos_all[IPPROTO_UDP / 8] & (1 << (IPPROTO_UDP % 8)))) { snprintf(applayer_counter_names[ipproto_map][alproto].name, sizeof(applayer_counter_names[ipproto_map][alproto].name), "%s%s%s", str, alproto_str, ipproto_suffix); From f714678d72146fac30e2bbfbb8a0df1a1689d13f Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Mon, 11 Sep 2023 09:51:24 +0200 Subject: [PATCH 2/9] schema: adds missing modbus field ./stats/app_layer/error/modbus --- etc/schema.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/etc/schema.json b/etc/schema.json index ed8adcf49fa0..76d88ec4c8fb 100644 --- a/etc/schema.json +++ b/etc/schema.json @@ -3783,6 +3783,9 @@ "krb5_udp": { "$ref": "#/$defs/stats_applayer_error" }, + "modbus": { + "$ref": "#/$defs/stats_applayer_error" + }, "mqtt": { "$ref": "#/$defs/stats_applayer_error" }, From 3103505cb0fa87f18b63434a94c1b3814f5b8003 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 14 Dec 2023 11:31:37 +0100 Subject: [PATCH 3/9] stats: incr app-proto flow counter for detection-only Ticket: 6633 --- src/app-layer.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/app-layer.c b/src/app-layer.c index 102319042bcc..e159b932afc3 100644 --- a/src/app-layer.c +++ b/src/app-layer.c @@ -510,6 +510,20 @@ static int TCPProtoDetect(ThreadVars *tv, TcpReassemblyThreadCtx *ra_ctx, if (r != 1) { StreamTcpUpdateAppLayerProgress(ssn, direction, data_len); } + if (r == 0) { + if (*alproto_otherdir == ALPROTO_UNKNOWN) { + TcpStream *opposing_stream; + if (*stream == &ssn->client) { + opposing_stream = &ssn->server; + } else { + opposing_stream = &ssn->client; + } + if (StreamTcpIsSetStreamFlagAppProtoDetectionCompleted(opposing_stream)) { + // can happen in detection-only + AppLayerIncFlowCounter(tv, f); + } + } + } if (r < 0) { goto parser_error; } From 1afb485dfa253f4b409fa1acf0b7790cf1d2f09b Mon Sep 17 00:00:00 2001 From: Juliana Fajardini Date: Fri, 15 Dec 2023 13:57:01 -0300 Subject: [PATCH 4/9] pgsql: remove unused msg field The `ConsolidatedDataRow` struct had a `length` field that wasn't truly used. Related to Bug #6389 --- rust/src/pgsql/logger.rs | 1 - rust/src/pgsql/parser.rs | 2 -- rust/src/pgsql/pgsql.rs | 1 - 3 files changed, 4 deletions(-) diff --git a/rust/src/pgsql/logger.rs b/rust/src/pgsql/logger.rs index 51f6cb60993b..d54b97b3e1a1 100644 --- a/rust/src/pgsql/logger.rs +++ b/rust/src/pgsql/logger.rs @@ -234,7 +234,6 @@ fn log_response(res: &PgsqlBEMessage, jb: &mut JsonBuilder) -> Result<(), JsonEr } PgsqlBEMessage::ConsolidatedDataRow(ConsolidatedDataRowPacket { identifier: _, - length: _, row_cnt, data_size, }) => { diff --git a/rust/src/pgsql/parser.rs b/rust/src/pgsql/parser.rs index 345f30a79872..3b8afcabf306 100644 --- a/rust/src/pgsql/parser.rs +++ b/rust/src/pgsql/parser.rs @@ -210,7 +210,6 @@ pub struct BackendKeyDataMessage { #[derive(Debug, PartialEq, Eq)] pub struct ConsolidatedDataRowPacket { pub identifier: u8, - pub length: u32, pub row_cnt: u16, pub data_size: u64, } @@ -924,7 +923,6 @@ pub fn parse_consolidated_data_row(i: &[u8]) -> IResult<&[u8], PgsqlBEMessage> { Ok((i, PgsqlBEMessage::ConsolidatedDataRow( ConsolidatedDataRowPacket { identifier, - length, row_cnt: 1, data_size: add_up_data_size(rows), } diff --git a/rust/src/pgsql/pgsql.rs b/rust/src/pgsql/pgsql.rs index 8ca7f4de6a61..d2d0a02f88da 100644 --- a/rust/src/pgsql/pgsql.rs +++ b/rust/src/pgsql/pgsql.rs @@ -487,7 +487,6 @@ impl PgsqlState { let dummy_resp = PgsqlBEMessage::ConsolidatedDataRow(ConsolidatedDataRowPacket { identifier: b'D', - length: tx.get_row_cnt() as u32, // TODO this is ugly. We can probably get rid of `length` field altogether... row_cnt: tx.get_row_cnt(), data_size: tx.data_size, // total byte count of all data_row messages combined }); From 15ed51f9b87011025615245d89152da9c567f49b Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Thu, 14 Dec 2023 18:19:41 -0600 Subject: [PATCH 5/9] feature: provide a Rust binding to the feature API As the feature module is not available for Rust unit tests, a mock version is also provided. --- rust/src/feature.rs | 60 +++++++++++++++++++++++++++++++++++++++++++++ rust/src/lib.rs | 1 + 2 files changed, 61 insertions(+) create mode 100644 rust/src/feature.rs diff --git a/rust/src/feature.rs b/rust/src/feature.rs new file mode 100644 index 000000000000..abd09669af11 --- /dev/null +++ b/rust/src/feature.rs @@ -0,0 +1,60 @@ +/* Copyright (C) 2023 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +//! Rust bindings to the "feature" API. +//! +//! As this feature module is a binding to a Suricata C module it is +//! not available to Rust unit tests. Instead when running Rust unit +//! tests and "mock" version is provided that will return true for any +//! feature starting with "true" and false for any other feature name. + +#[cfg(test)] +mod mock { + /// Check for a feature returning true if found. + /// + /// This a "mock" variant of `requires` that will return true for + /// any feature starting with string `true`, and false for + /// anything else. + pub fn requires(feature: &str) -> bool { + return feature.starts_with("true"); + } +} + +#[cfg(not(test))] +mod real { + use std::ffi::CString; + use std::os::raw::c_char; + + extern "C" { + fn RequiresFeature(feature: *const c_char) -> bool; + } + + /// Check for a feature returning true if found. + pub fn requires(feature: &str) -> bool { + if let Ok(feature) = CString::new(feature) { + unsafe { RequiresFeature(feature.as_ptr()) } + } else { + false + } + } +} + +#[cfg(not(test))] +pub use real::*; + +#[cfg(test)] +pub use mock::*; diff --git a/rust/src/lib.rs b/rust/src/lib.rs index da2859637783..84b82bde19f7 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -116,3 +116,4 @@ pub mod plugin; pub mod lzma; pub mod util; pub mod ffi; +pub mod feature; From 5d5b0509a543f2b6f09cc81acf0248a361b03aa1 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Tue, 28 Nov 2023 15:35:09 -0600 Subject: [PATCH 6/9] requires: add requires keyword Add a new rule keyword "requires" that allows a rule to require specific Suricata versions and/or Suricata features to be enabled. Example: requires: feature geoip, version >= 7.0.0, version < 8; requires: version >= 7.0.3 < 8 requires: version >= 7.0.3 < 8 | >= 8.0.3 Feature: #5972 Co-authored-by: Philippe Antoine --- doc/userguide/rules/meta.rst | 47 ++ rust/src/detect/mod.rs | 1 + rust/src/detect/requires.rs | 805 +++++++++++++++++++++++++++++++++++ rust/src/log.rs | 9 +- src/Makefile.am | 2 + src/detect-engine-loader.c | 50 ++- src/detect-engine-register.c | 2 + src/detect-engine-register.h | 2 + src/detect-engine.c | 4 + src/detect-parse.c | 11 +- src/detect-requires.c | 50 +++ src/detect-requires.h | 23 + src/detect.h | 10 + 13 files changed, 997 insertions(+), 19 deletions(-) create mode 100644 rust/src/detect/requires.rs create mode 100644 src/detect-requires.c create mode 100644 src/detect-requires.h diff --git a/doc/userguide/rules/meta.rst b/doc/userguide/rules/meta.rst index 06e5040e73a5..0e888add697a 100644 --- a/doc/userguide/rules/meta.rst +++ b/doc/userguide/rules/meta.rst @@ -211,3 +211,50 @@ The format is:: If the value is src_ip then the source IP in the generated event (src_ip field in JSON) is the target of the attack. If target is set to dest_ip then the target is the destination IP in the generated event. + +requires +-------- + +The ``requires`` keyword allows a rule to require specific Suricata +features to be enabled, or the Suricata version to match an +expression. Rules that do not meet the requirements will by ignored, +and Suricata will not treat them as errors. + +When parsing rules, the parser attempts to process the ``requires`` +keywords before others. This allows it to occur after keywords that +may only be present in specific versions of Suricata, as specified by +the ``requires`` statement. However, the keywords preceding it must +still adhere to the basic known formats of Suricata rules. + +The format is:: + + requires: feature geoip, version >= 7.0.0 + +To require multiple features, the feature sub-keyword must be +specified multiple times:: + + requires: feature geoip, feature lua + +Alternatively, *and* expressions may be expressed like:: + + requires: version >= 7.0.4 < 8 + +and *or* expressions may expressed with ``|`` like:: + + requires: version >= 7.0.4 < 8 | >= 8.0.3 + +to express that a rules requires version 7.0.4 or greater, but less +than 8, **OR** greater than or equal to 8.0.3. Which could be useful +if a keyword wasn't added until 7.0.4 and the 8.0.3 patch releases, as +it would not exist in 8.0.1. + +This can be extended to multiple release branches:: + + requires: version >= 7.0.10 < 8 | >= 8.0.5 < 9 | >= 9.0.3 + +If no *minor* or *patch* version component is provided, it will +default to 0. + +The ``version`` may only be specified once, if specified more than +once the rule will log an error and not be loaded. + diff --git a/rust/src/detect/mod.rs b/rust/src/detect/mod.rs index 41c7ff2455bd..d33c9ae7fabf 100644 --- a/rust/src/detect/mod.rs +++ b/rust/src/detect/mod.rs @@ -24,3 +24,4 @@ pub mod parser; pub mod stream_size; pub mod uint; pub mod uri; +pub mod requires; diff --git a/rust/src/detect/requires.rs b/rust/src/detect/requires.rs new file mode 100644 index 000000000000..e9e1acac5087 --- /dev/null +++ b/rust/src/detect/requires.rs @@ -0,0 +1,805 @@ +/* Copyright (C) 2023 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +use std::collections::{HashSet, VecDeque}; +use std::{cmp::Ordering, ffi::CStr}; + +// std::ffi::{c_char, c_int} is recommended these days, but requires +// Rust 1.64.0. +use std::os::raw::{c_char, c_int}; + +use nom7::bytes::complete::take_while; +use nom7::combinator::map; +use nom7::multi::{many1, separated_list1}; +use nom7::sequence::tuple; +use nom7::{ + branch::alt, + bytes::complete::{tag, take_till}, + character::complete::{char, multispace0}, + combinator::map_res, + sequence::preceded, + IResult, +}; + +#[derive(Debug, Eq, PartialEq)] +enum RequiresError { + /// Suricata is greater than the required version. + VersionGt, + + /// Suricata is less than the required version. + VersionLt(SuricataVersion), + + /// The running Suricata is missing a required feature. + MissingFeature(String), + + /// The Suricata version, of Suricata itself is bad and failed to parse. + BadSuricataVersion, + + /// The requires expression is bad and failed to parse. + BadRequires, + + /// MultipleVersions + MultipleVersions, + + /// Passed in requirements not a valid UTF-8 string. + Utf8Error, +} + +impl RequiresError { + /// Return a pointer to a C compatible constant error message. + const fn c_errmsg(&self) -> *const c_char { + let msg = match self { + Self::VersionGt => "Suricata version greater than required\0", + Self::VersionLt(_) => "Suricata version less than required\0", + Self::MissingFeature(_) => "Suricata missing a required feature\0", + Self::BadSuricataVersion => "Failed to parse running Suricata version\0", + Self::BadRequires => "Failed to parse requires expression\0", + Self::MultipleVersions => "Version may only be specified once\0", + Self::Utf8Error => "Requires expression is not valid UTF-8\0", + }; + msg.as_ptr() as *const c_char + } +} + +#[derive(Clone, Debug, Eq, PartialEq)] +enum VersionCompareOp { + Gt, + Gte, + Lt, + Lte, +} + +#[derive(Debug, Clone, Eq, PartialEq)] +struct SuricataVersion { + major: u8, + minor: u8, + patch: u8, +} + +impl PartialOrd for SuricataVersion { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for SuricataVersion { + fn cmp(&self, other: &Self) -> Ordering { + match self.major.cmp(&other.major) { + Ordering::Equal => match self.minor.cmp(&other.minor) { + Ordering::Equal => self.patch.cmp(&other.patch), + other => other, + }, + other => other, + } + } +} + +impl std::fmt::Display for SuricataVersion { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}.{}.{}", self.major, self.minor, self.patch) + } +} + +impl SuricataVersion { + fn new(major: u8, minor: u8, patch: u8) -> Self { + Self { + major, + minor, + patch, + } + } +} + +/// Parse a version expression. +/// +/// Parse into a version expression into a nested array, for example: +/// +/// version: >= 7.0.3 < 8 | >= 8.0.3 +/// +/// would result in something like: +/// +/// [ +/// [{op: gte, version: 7.0.3}, {op:lt, version: 8}], +/// [{op: gte, version: 8.0.3}], +/// ] +fn parse_version_expression(input: &str) -> IResult<&str, Vec>> { + let sep = preceded(multispace0, tag("|")); + let inner_parser = many1(tuple((parse_op, parse_version))); + let (input, versions) = separated_list1(sep, inner_parser)(input)?; + + let versions = versions + .into_iter() + .map(|versions| { + versions + .into_iter() + .map(|(op, version)| RuleRequireVersion { op, version }) + .collect() + }) + .collect(); + + Ok((input, versions)) +} + +#[derive(Debug, Eq, PartialEq)] +struct RuleRequireVersion { + pub op: VersionCompareOp, + pub version: SuricataVersion, +} + +#[derive(Debug, Default, Eq, PartialEq)] +struct Requires { + pub features: Vec, + + /// The version expression. + /// + /// - All of the inner most must evaluate to true. + /// - To pass, any of the outer must be true. + pub version: Vec>, +} + +fn parse_op(input: &str) -> IResult<&str, VersionCompareOp> { + preceded( + multispace0, + alt(( + map(tag(">="), |_| VersionCompareOp::Gte), + map(tag(">"), |_| VersionCompareOp::Gt), + map(tag("<="), |_| VersionCompareOp::Lte), + map(tag("<"), |_| VersionCompareOp::Lt), + )), + )(input) +} + +/// Parse the next part of the version. +/// +/// That is all chars up to eof, or the next '.' or '-'. +fn parse_next_version_part(input: &str) -> IResult<&str, u8> { + map_res( + take_till(|c| c == '.' || c == '-' || c == ' '), + |s: &str| s.parse::(), + )(input) +} + +/// Parse a version string into a SuricataVersion. +fn parse_version(input: &str) -> IResult<&str, SuricataVersion> { + let (input, major) = preceded(multispace0, parse_next_version_part)(input)?; + let (input, minor) = if input.is_empty() || input.starts_with(' ') { + (input, 0) + } else { + preceded(char('.'), parse_next_version_part)(input)? + }; + let (input, patch) = if input.is_empty() || input.starts_with(' ') { + (input, 0) + } else { + preceded(char('.'), parse_next_version_part)(input)? + }; + + Ok((input, SuricataVersion::new(major, minor, patch))) +} + +fn parse_key_value(input: &str) -> IResult<&str, (&str, &str)> { + // Parse the keyword, any sequence of characters, numbers or "-" or "_". + let (input, key) = preceded( + multispace0, + take_while(|c: char| c.is_alphanumeric() || c == '-' || c == '_'), + )(input)?; + let (input, value) = preceded(multispace0, take_till(|c: char| c == ','))(input)?; + Ok((input, (key, value))) +} + +fn parse_requires(mut input: &str) -> Result { + let mut requires = Requires::default(); + + while !input.is_empty() { + let (rest, (keyword, value)) = + parse_key_value(input).map_err(|_| RequiresError::BadRequires)?; + match keyword { + "feature" => { + requires.features.push(value.trim().to_string()); + } + "version" => { + if !requires.version.is_empty() { + return Err(RequiresError::MultipleVersions); + } + let (_, versions) = + parse_version_expression(value).map_err(|_| RequiresError::BadRequires)?; + requires.version = versions; + } + _ => { + // Unknown keyword, allow by warn in case we extend + // this in the future. + SCLogWarning!("Unknown requires keyword: {}", keyword); + } + } + + // No consume any remaining ',' or whitespace. + input = rest.trim_start_matches(|c: char| c == ',' || c.is_whitespace()); + } + Ok(requires) +} + +fn parse_suricata_version(version: &CStr) -> Result { + let version = version + .to_str() + .map_err(|_| RequiresError::BadSuricataVersion.c_errmsg())?; + let (_, version) = + parse_version(version).map_err(|_| RequiresError::BadSuricataVersion.c_errmsg())?; + Ok(version) +} + +fn check_version( + version: &RuleRequireVersion, suricata_version: &SuricataVersion, +) -> Result<(), RequiresError> { + match version.op { + VersionCompareOp::Gt => { + if suricata_version <= &version.version { + return Err(RequiresError::VersionLt(version.version.clone())); + } + } + VersionCompareOp::Gte => { + if suricata_version < &version.version { + return Err(RequiresError::VersionLt(version.version.clone())); + } + } + VersionCompareOp::Lt => { + if suricata_version >= &version.version { + return Err(RequiresError::VersionGt); + } + } + VersionCompareOp::Lte => { + if suricata_version > &version.version { + return Err(RequiresError::VersionGt); + } + } + } + Ok(()) +} + +fn check_requires( + requires: &Requires, suricata_version: &SuricataVersion, +) -> Result<(), RequiresError> { + if !requires.version.is_empty() { + let mut errs = VecDeque::new(); + let mut ok = 0; + for or_versions in &requires.version { + let mut err = None; + for version in or_versions { + if let Err(_err) = check_version(version, suricata_version) { + err = Some(_err); + break; + } + } + if let Some(err) = err { + errs.push_back(err); + } else { + ok += 1; + } + } + if ok == 0 { + return Err(errs.pop_front().unwrap()); + } + } + + for feature in &requires.features { + if !crate::feature::requires(feature) { + return Err(RequiresError::MissingFeature(feature.to_string())); + } + } + + Ok(()) +} + +/// Status object to hold required features and the latest version of +/// Suricata required. +/// +/// Full qualified name as it is exposed to C. +#[derive(Debug, Default)] +pub struct SCDetectRequiresStatus { + min_version: Option, + features: HashSet, + + /// Number of rules that didn't meet a feature. + feature_count: u64, + + /// Number of rules where the Suricata version wasn't new enough. + lt_count: u64, + + /// Number of rules where the Suricata version was too new. + gt_count: u64, +} + +#[no_mangle] +pub extern "C" fn SCDetectRequiresStatusNew() -> *mut SCDetectRequiresStatus { + Box::into_raw(Box::default()) +} + +#[no_mangle] +pub unsafe extern "C" fn SCDetectRequiresStatusFree(status: *mut SCDetectRequiresStatus) { + if !status.is_null() { + std::mem::drop(Box::from_raw(status)); + } +} + +#[no_mangle] +pub unsafe extern "C" fn SCDetectRequiresStatusLog( + status: &mut SCDetectRequiresStatus, suricata_version: *const c_char, tenant_id: u32, +) { + let suricata_version = CStr::from_ptr(suricata_version) + .to_str() + .unwrap_or(""); + + let mut parts = vec![]; + if status.lt_count > 0 { + let min_version = status + .min_version + .as_ref() + .map(|v| v.to_string()) + .unwrap_or_else(|| "".to_string()); + let msg = format!( + "{} {} skipped because the running Suricata version {} is less than {}", + status.lt_count, + if status.lt_count > 1 { + "rules were" + } else { + "rule was" + }, + suricata_version, + &min_version + ); + parts.push(msg); + } + if status.gt_count > 0 { + let msg = format!( + "{} {} for an older version Suricata", + status.gt_count, + if status.gt_count > 1 { + "rules were skipped as they are" + } else { + "rule was skipped as it is" + } + ); + parts.push(msg); + } + if status.feature_count > 0 { + let features = status + .features + .iter() + .map(|f| f.to_string()) + .collect::>() + .join(", "); + let msg = format!( + "{}{} {} skipped because the running Suricata version does not have feature{}: [{}]", + if tenant_id > 0 { + format!("tenant id: {} ", tenant_id) + } else { + String::new() + }, + status.feature_count, + if status.feature_count > 1 { + "rules were" + } else { + "rule was" + }, + if status.feature_count > 1 { "s" } else { "" }, + &features + ); + parts.push(msg); + } + + let msg = parts.join("; "); + + if status.lt_count > 0 { + SCLogNotice!("{}", &msg); + } else if status.gt_count > 0 || status.feature_count > 0 { + SCLogInfo!("{}", &msg); + } +} + +/// Parse a "requires" rule option. +/// +/// Return values: +/// * 0 - OK, rule should continue loading +/// * -1 - Error parsing the requires content +/// * -4 - Requirements not met, don't continue loading the rule, this +/// value is chosen so it can be passed back to the options parser +/// as its treated as a non-fatal silent error. +#[no_mangle] +pub unsafe extern "C" fn SCDetectCheckRequires( + requires: *const c_char, suricata_version_string: *const c_char, errstr: *mut *const c_char, + status: &mut SCDetectRequiresStatus, +) -> c_int { + // First parse the running Suricata version. + let suricata_version = match parse_suricata_version(CStr::from_ptr(suricata_version_string)) { + Ok(version) => version, + Err(err) => { + *errstr = err; + return -1; + } + }; + + let requires = match CStr::from_ptr(requires) + .to_str() + .map_err(|_| RequiresError::Utf8Error) + .and_then(parse_requires) + { + Ok(requires) => requires, + Err(err) => { + *errstr = err.c_errmsg(); + return -1; + } + }; + + match check_requires(&requires, &suricata_version) { + Ok(()) => 0, + Err(err) => { + match &err { + RequiresError::VersionLt(version) => { + if let Some(min_version) = &status.min_version { + if version > min_version { + status.min_version = Some(version.clone()); + } + } else { + status.min_version = Some(version.clone()); + } + status.lt_count += 1; + } + RequiresError::MissingFeature(feature) => { + status.features.insert(feature.to_string()); + status.feature_count += 1; + } + RequiresError::VersionGt => { + status.gt_count += 1; + } + _ => {} + } + *errstr = err.c_errmsg(); + return -4; + } + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_suricata_version() { + // 7.1.1 < 7.1.2 + assert!(SuricataVersion::new(7, 1, 1) < SuricataVersion::new(7, 1, 2)); + + // 7.1.1 <= 7.1.2 + assert!(SuricataVersion::new(7, 1, 1) <= SuricataVersion::new(7, 1, 2)); + + // 7.1.1 <= 7.1.1 + assert!(SuricataVersion::new(7, 1, 1) <= SuricataVersion::new(7, 1, 1)); + + // NOT 7.1.1 < 7.1.1 + assert!(SuricataVersion::new(7, 1, 1) >= SuricataVersion::new(7, 1, 1)); + + // 7.3.1 < 7.22.1 + assert!(SuricataVersion::new(7, 3, 1) < SuricataVersion::new(7, 22, 1)); + + // 7.22.1 >= 7.3.4 + assert!(SuricataVersion::new(7, 22, 1) >= SuricataVersion::new(7, 3, 4)); + } + + #[test] + fn test_parse_op() { + assert_eq!(parse_op(">").unwrap().1, VersionCompareOp::Gt); + assert_eq!(parse_op(">=").unwrap().1, VersionCompareOp::Gte); + assert_eq!(parse_op("<").unwrap().1, VersionCompareOp::Lt); + assert_eq!(parse_op("<=").unwrap().1, VersionCompareOp::Lte); + + assert!(parse_op("=").is_err()); + } + + #[test] + fn test_parse_version() { + assert_eq!( + parse_version("7").unwrap().1, + SuricataVersion { + major: 7, + minor: 0, + patch: 0, + } + ); + + assert_eq!( + parse_version("7.1").unwrap().1, + SuricataVersion { + major: 7, + minor: 1, + patch: 0, + } + ); + + assert_eq!( + parse_version("7.1.2").unwrap().1, + SuricataVersion { + major: 7, + minor: 1, + patch: 2, + } + ); + + // Suricata pre-releases will have a suffix starting with a + // '-', so make sure we accept those versions as well. + assert_eq!( + parse_version("8.0.0-dev").unwrap().1, + SuricataVersion { + major: 8, + minor: 0, + patch: 0, + } + ); + + assert!(parse_version("7.1.2a").is_err()); + assert!(parse_version("a").is_err()); + assert!(parse_version("777").is_err()); + assert!(parse_version("product-1").is_err()); + } + + #[test] + fn test_parse_requires() { + let requires = parse_requires(" feature geoip").unwrap(); + assert_eq!(&requires.features[0], "geoip"); + + let requires = parse_requires(" feature geoip, feature lua ").unwrap(); + assert_eq!(&requires.features[0], "geoip"); + assert_eq!(&requires.features[1], "lua"); + + let requires = parse_requires("version >=7").unwrap(); + assert_eq!( + requires, + Requires { + features: vec![], + version: vec![vec![RuleRequireVersion { + op: VersionCompareOp::Gte, + version: SuricataVersion { + major: 7, + minor: 0, + patch: 0, + } + }]], + } + ); + + let requires = parse_requires("version >= 7.1").unwrap(); + assert_eq!( + requires, + Requires { + features: vec![], + version: vec![vec![RuleRequireVersion { + op: VersionCompareOp::Gte, + version: SuricataVersion { + major: 7, + minor: 1, + patch: 0, + } + }]], + } + ); + + let requires = parse_requires("feature output::file-store, version >= 7.1.2").unwrap(); + assert_eq!( + requires, + Requires { + features: vec!["output::file-store".to_string()], + version: vec![vec![RuleRequireVersion { + op: VersionCompareOp::Gte, + version: SuricataVersion { + major: 7, + minor: 1, + patch: 2, + } + }]], + } + ); + + let requires = parse_requires("feature geoip, version >= 7.1.2 < 8").unwrap(); + assert_eq!( + requires, + Requires { + features: vec!["geoip".to_string()], + version: vec![vec![ + RuleRequireVersion { + op: VersionCompareOp::Gte, + version: SuricataVersion { + major: 7, + minor: 1, + patch: 2, + }, + }, + RuleRequireVersion { + op: VersionCompareOp::Lt, + version: SuricataVersion { + major: 8, + minor: 0, + patch: 0, + } + } + ]], + } + ); + } + + #[test] + fn test_check_requires() { + // Have 7.0.4, require >= 8. + let suricata_version = SuricataVersion::new(7, 0, 4); + let requires = parse_requires("version >= 8").unwrap(); + assert_eq!( + check_requires(&requires, &suricata_version), + Err(RequiresError::VersionLt(SuricataVersion { + major: 8, + minor: 0, + patch: 0, + })), + ); + + // Have 7.0.4, require 7.0.3. + let suricata_version = SuricataVersion::new(7, 0, 4); + let requires = parse_requires("version >= 7.0.3").unwrap(); + assert_eq!(check_requires(&requires, &suricata_version), Ok(())); + + // Have 8.0.0, require >= 7.0.0 and < 8.0 + let suricata_version = SuricataVersion::new(8, 0, 0); + let requires = parse_requires("version >= 7.0.0 < 8").unwrap(); + assert_eq!( + check_requires(&requires, &suricata_version), + Err(RequiresError::VersionGt) + ); + + // Have 8.0.0, require >= 7.0.0 and < 9.0 + let suricata_version = SuricataVersion::new(8, 0, 0); + let requires = parse_requires("version >= 7.0.0 < 9").unwrap(); + assert_eq!(check_requires(&requires, &suricata_version), Ok(())); + + // Require feature foobar. + let suricata_version = SuricataVersion::new(8, 0, 0); + let requires = parse_requires("feature foobar").unwrap(); + assert_eq!( + check_requires(&requires, &suricata_version), + Err(RequiresError::MissingFeature("foobar".to_string())) + ); + + // Require feature foobar, but this time we have the feature. + let suricata_version = SuricataVersion::new(8, 0, 0); + let requires = parse_requires("feature true_foobar").unwrap(); + assert_eq!(check_requires(&requires, &suricata_version), Ok(())); + + let suricata_version = SuricataVersion::new(8, 0, 1); + let requires = parse_requires("version >= 7.0.3 < 8").unwrap(); + assert!(check_requires(&requires, &suricata_version).is_err()); + + let suricata_version = SuricataVersion::new(7, 0, 1); + let requires = parse_requires("version >= 7.0.3 < 8").unwrap(); + assert!(check_requires(&requires, &suricata_version).is_err()); + + let suricata_version = SuricataVersion::new(7, 0, 3); + let requires = parse_requires("version >= 7.0.3 < 8").unwrap(); + assert!(check_requires(&requires, &suricata_version).is_ok()); + + let suricata_version = SuricataVersion::new(8, 0, 3); + let requires = parse_requires("version >= 7.0.3 < 8 | >= 8.0.3").unwrap(); + assert!(check_requires(&requires, &suricata_version).is_ok()); + + let suricata_version = SuricataVersion::new(8, 0, 2); + let requires = parse_requires("version >= 7.0.3 < 8 | >= 8.0.3").unwrap(); + assert!(check_requires(&requires, &suricata_version).is_err()); + + let suricata_version = SuricataVersion::new(7, 0, 2); + let requires = parse_requires("version >= 7.0.3 < 8 | >= 8.0.3").unwrap(); + assert!(check_requires(&requires, &suricata_version).is_err()); + + let suricata_version = SuricataVersion::new(7, 0, 3); + let requires = parse_requires("version >= 7.0.3 < 8 | >= 8.0.3").unwrap(); + assert!(check_requires(&requires, &suricata_version).is_ok()); + + // Example of something that requires a fix/feature that was + // implemented in 7.0.5, 8.0.4, 9.0.3. + let requires = parse_requires("version >= 7.0.5 < 8 | >= 8.0.4 < 9 | >= 9.0.3").unwrap(); + assert!(check_requires(&requires, &SuricataVersion::new(6, 0, 0)).is_err()); + assert!(check_requires(&requires, &SuricataVersion::new(7, 0, 4)).is_err()); + assert!(check_requires(&requires, &SuricataVersion::new(7, 0, 5)).is_ok()); + assert!(check_requires(&requires, &SuricataVersion::new(8, 0, 3)).is_err()); + assert!(check_requires(&requires, &SuricataVersion::new(8, 0, 4)).is_ok()); + assert!(check_requires(&requires, &SuricataVersion::new(9, 0, 2)).is_err()); + assert!(check_requires(&requires, &SuricataVersion::new(9, 0, 3)).is_ok()); + assert!(check_requires(&requires, &SuricataVersion::new(10, 0, 0)).is_ok()); + + let requires = parse_requires("version >= 8 < 9").unwrap(); + assert!(check_requires(&requires, &SuricataVersion::new(6, 0, 0)).is_err()); + assert!(check_requires(&requires, &SuricataVersion::new(7, 0, 0)).is_err()); + assert!(check_requires(&requires, &SuricataVersion::new(8, 0, 0)).is_ok()); + assert!(check_requires(&requires, &SuricataVersion::new(9, 0, 0)).is_err()); + + // Unknown keyword. + let requires = parse_requires("feature lua, foo bar, version >= 7.0.3").unwrap(); + assert_eq!( + requires, + Requires { + features: vec!["lua".to_string()], + version: vec![vec![RuleRequireVersion { + op: VersionCompareOp::Gte, + version: SuricataVersion { + major: 7, + minor: 0, + patch: 3, + } + }]], + } + ); + } + + #[test] + fn test_parse_version_expression() { + let version_str = ">= 7.0.3 < 8 | >= 8.0.3"; + let (rest, versions) = parse_version_expression(version_str).unwrap(); + assert!(rest.is_empty()); + assert_eq!( + versions, + vec![ + vec![ + RuleRequireVersion { + op: VersionCompareOp::Gte, + version: SuricataVersion { + major: 7, + minor: 0, + patch: 3, + } + }, + RuleRequireVersion { + op: VersionCompareOp::Lt, + version: SuricataVersion { + major: 8, + minor: 0, + patch: 0, + } + }, + ], + vec![RuleRequireVersion { + op: VersionCompareOp::Gte, + version: SuricataVersion { + major: 8, + minor: 0, + patch: 3, + } + },], + ] + ); + } +} diff --git a/rust/src/log.rs b/rust/src/log.rs index 744169a97039..7bf0be8a97c6 100644 --- a/rust/src/log.rs +++ b/rust/src/log.rs @@ -29,7 +29,7 @@ pub enum Level { NotSet = -1, _None = 0, Error, - _Warning, + Warning, Notice, Info, _Perf, @@ -115,6 +115,13 @@ macro_rules!SCLogError { }; } +#[macro_export] +macro_rules!SCLogWarning { + ($($arg:tt)*) => { + $crate::do_log!($crate::log::Level::Warning, $($arg)*); + }; +} + #[macro_export] macro_rules!SCLogNotice { ($($arg:tt)*) => { diff --git a/src/Makefile.am b/src/Makefile.am index 6d115ac48ae5..9cb4f815531c 100755 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -280,6 +280,7 @@ noinst_HEADERS = \ detect-rawbytes.h \ detect-reference.h \ detect-replace.h \ + detect-requires.h \ detect-rev.h \ detect-rfb-name.h \ detect-rfb-secresult.h \ @@ -895,6 +896,7 @@ libsuricata_c_a_SOURCES = \ detect-rawbytes.c \ detect-reference.c \ detect-replace.c \ + detect-requires.c \ detect-rev.c \ detect-rfb-name.c \ detect-rfb-secresult.c \ diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c index 40919568503a..9c3e3e8b533c 100644 --- a/src/detect-engine-loader.c +++ b/src/detect-engine-loader.c @@ -44,6 +44,8 @@ #include "util-threshold-config.h" #include "util-path.h" +#include "rust.h" + #ifdef HAVE_GLOB_H #include #endif @@ -109,11 +111,11 @@ char *DetectLoadCompleteSigPath(const DetectEngineCtx *de_ctx, const char *sig_f * \param badsigs_tot Will store number of invalid signatures in the file * \retval 0 on success, -1 on error */ -static int DetectLoadSigFile(DetectEngineCtx *de_ctx, char *sig_file, - int *goodsigs, int *badsigs) +static int DetectLoadSigFile( + DetectEngineCtx *de_ctx, char *sig_file, int *goodsigs, int *badsigs, int *skippedsigs) { Signature *sig = NULL; - int good = 0, bad = 0; + int good = 0, bad = 0, skipped = 0; char line[DETECT_MAX_RULE_SIZE] = ""; size_t offset = 0; int lineno = 0, multiline = 0; @@ -196,6 +198,12 @@ static int DetectLoadSigFile(DetectEngineCtx *de_ctx, char *sig_file, if (!de_ctx->sigerror_ok) { bad++; } + if (de_ctx->sigerror_requires) { + SCLogInfo("Skipping signature due to missing requirements: %s from file %s at line " + "%" PRId32, + line, sig_file, lineno - multiline); + skipped++; + } } multiline = 0; } @@ -203,6 +211,7 @@ static int DetectLoadSigFile(DetectEngineCtx *de_ctx, char *sig_file, *goodsigs = good; *badsigs = bad; + *skippedsigs = skipped; return 0; } @@ -212,8 +221,8 @@ static int DetectLoadSigFile(DetectEngineCtx *de_ctx, char *sig_file, * \param sig_file Filename (or pattern) holding signatures * \retval -1 on error */ -static int ProcessSigFiles(DetectEngineCtx *de_ctx, char *pattern, - SigFileLoaderStat *st, int *good_sigs, int *bad_sigs) +static int ProcessSigFiles(DetectEngineCtx *de_ctx, char *pattern, SigFileLoaderStat *st, + int *good_sigs, int *bad_sigs, int *skipped_sigs) { int r = 0; @@ -250,7 +259,7 @@ static int ProcessSigFiles(DetectEngineCtx *de_ctx, char *pattern, } else { SCLogConfig("Loading rule file: %s", fname); } - r = DetectLoadSigFile(de_ctx, fname, good_sigs, bad_sigs); + r = DetectLoadSigFile(de_ctx, fname, good_sigs, bad_sigs, skipped_sigs); if (r < 0) { ++(st->bad_files); } @@ -259,6 +268,7 @@ static int ProcessSigFiles(DetectEngineCtx *de_ctx, char *pattern, st->good_sigs_total += *good_sigs; st->bad_sigs_total += *bad_sigs; + st->skipped_sigs_total += *skipped_sigs; #ifdef HAVE_GLOB_H } @@ -286,6 +296,7 @@ int SigLoadSignatures(DetectEngineCtx *de_ctx, char *sig_file, bool sig_file_exc char varname[128] = "rule-files"; int good_sigs = 0; int bad_sigs = 0; + int skipped_sigs = 0; if (strlen(de_ctx->config_prefix) > 0) { snprintf(varname, sizeof(varname), "%s.rule-files", @@ -307,8 +318,9 @@ int SigLoadSignatures(DetectEngineCtx *de_ctx, char *sig_file, bool sig_file_exc else { TAILQ_FOREACH(file, &rule_files->head, next) { sfile = DetectLoadCompleteSigPath(de_ctx, file->val); - good_sigs = bad_sigs = 0; - ret = ProcessSigFiles(de_ctx, sfile, sig_stat, &good_sigs, &bad_sigs); + good_sigs = bad_sigs = skipped_sigs = 0; + ret = ProcessSigFiles( + de_ctx, sfile, sig_stat, &good_sigs, &bad_sigs, &skipped_sigs); SCFree(sfile); if (de_ctx->failure_fatal && ret != 0) { @@ -327,7 +339,7 @@ int SigLoadSignatures(DetectEngineCtx *de_ctx, char *sig_file, bool sig_file_exc /* If a Signature file is specified from command-line, parse it too */ if (sig_file != NULL) { - ret = ProcessSigFiles(de_ctx, sig_file, sig_stat, &good_sigs, &bad_sigs); + ret = ProcessSigFiles(de_ctx, sig_file, sig_stat, &good_sigs, &bad_sigs, &skipped_sigs); if (ret != 0) { if (de_ctx->failure_fatal) { @@ -351,15 +363,23 @@ int SigLoadSignatures(DetectEngineCtx *de_ctx, char *sig_file, bool sig_file_exc } } else { /* we report the total of files and rules successfully loaded and failed */ - if (strlen(de_ctx->config_prefix) > 0) + if (strlen(de_ctx->config_prefix) > 0) { SCLogInfo("tenant id %d: %" PRId32 " rule files processed. %" PRId32 - " rules successfully loaded, %" PRId32 " rules failed", + " rules successfully loaded, %" PRId32 " rules failed, %" PRId32 + " rules skipped", de_ctx->tenant_id, sig_stat->total_files, sig_stat->good_sigs_total, - sig_stat->bad_sigs_total); - else + sig_stat->bad_sigs_total, sig_stat->skipped_sigs_total); + } else { SCLogInfo("%" PRId32 " rule files processed. %" PRId32 - " rules successfully loaded, %" PRId32 " rules failed", - sig_stat->total_files, sig_stat->good_sigs_total, sig_stat->bad_sigs_total); + " rules successfully loaded, %" PRId32 " rules failed, %" PRId32 + " rules skipped", + sig_stat->total_files, sig_stat->good_sigs_total, sig_stat->bad_sigs_total, + sig_stat->skipped_sigs_total); + } + if (de_ctx->requirements != NULL && sig_stat->skipped_sigs_total > 0) { + SCDetectRequiresStatusLog(de_ctx->requirements, PROG_VER, + strlen(de_ctx->config_prefix) > 0 ? de_ctx->tenant_id : 0); + } } if ((sig_stat->bad_sigs_total || sig_stat->bad_files) && de_ctx->failure_fatal) { diff --git a/src/detect-engine-register.c b/src/detect-engine-register.c index a97da4617197..9f37e0945544 100644 --- a/src/detect-engine-register.c +++ b/src/detect-engine-register.c @@ -119,6 +119,7 @@ #include "detect-flow.h" #include "detect-flow-age.h" #include "detect-flow-pkts.h" +#include "detect-requires.h" #include "detect-tcp-window.h" #include "detect-ftpbounce.h" #include "detect-isdataat.h" @@ -575,6 +576,7 @@ void SigTableSetup(void) DetectFlowPktsToServerRegister(); DetectFlowBytesToClientRegister(); DetectFlowBytesToServerRegister(); + DetectRequiresRegister(); DetectWindowRegister(); DetectRpcRegister(); DetectFtpbounceRegister(); diff --git a/src/detect-engine-register.h b/src/detect-engine-register.h index 2e4a330788ed..9dd01f5fd487 100644 --- a/src/detect-engine-register.h +++ b/src/detect-engine-register.h @@ -115,6 +115,8 @@ enum DetectKeywordId { DETECT_FLOW_BYTES_TO_CLIENT, DETECT_FLOW_BYTES_TO_SERVER, + DETECT_REQUIRES, + DETECT_AL_TLS_VERSION, DETECT_AL_TLS_SUBJECT, DETECT_AL_TLS_ISSUERDN, diff --git a/src/detect-engine.c b/src/detect-engine.c index 3e1ce93cf671..13e09be71889 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -2652,6 +2652,10 @@ void DetectEngineCtxFree(DetectEngineCtx *de_ctx) SCFree(de_ctx->tenant_path); } + if (de_ctx->requirements) { + SCDetectRequiresStatusFree(de_ctx->requirements); + } + SCFree(de_ctx); //DetectAddressGroupPrintMemory(); //DetectSigGroupPrintMemory(); diff --git a/src/detect-parse.c b/src/detect-parse.c index 83c22aa8adac..b94d54dd6cd0 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -2146,12 +2146,17 @@ static Signature *SigInitHelper(DetectEngineCtx *de_ctx, const char *sigstr, sig->gid = 1; int ret = SigParse(de_ctx, sig, sigstr, dir, &parser); - if (ret == -3) { + if (ret == -4) { + /* Rule requirements not met. */ de_ctx->sigerror_silent = true; de_ctx->sigerror_ok = true; + de_ctx->sigerror_requires = true; goto error; - } - else if (ret == -2) { + } else if (ret == -3) { + de_ctx->sigerror_silent = true; + de_ctx->sigerror_ok = true; + goto error; + } else if (ret == -2) { de_ctx->sigerror_silent = true; goto error; } else if (ret < 0) { diff --git a/src/detect-requires.c b/src/detect-requires.c new file mode 100644 index 000000000000..4d7f916b3b82 --- /dev/null +++ b/src/detect-requires.c @@ -0,0 +1,50 @@ +/* Copyright (C) 2023 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +#include "detect-requires.h" +#include "suricata-common.h" +#include "detect-engine.h" +#include "rust.h" + +static int DetectRequiresSetup(DetectEngineCtx *de_ctx, Signature *s, const char *rawstr) +{ + if (de_ctx->requirements == NULL) { + de_ctx->requirements = (void *)SCDetectRequiresStatusNew(); + BUG_ON(de_ctx->requirements == NULL); + } + + const char *errmsg = NULL; + int res = SCDetectCheckRequires(rawstr, PROG_VER, &errmsg, de_ctx->requirements); + if (res == -1) { + // The requires expression is bad, log an error. + SCLogError("%s: %s", errmsg, rawstr); + de_ctx->sigerror = errmsg; + } else if (res < -1) { + // This Suricata instance didn't meet the requirements. + SCLogInfo("Suricata did not meet the rule requirements: %s: %s", errmsg, rawstr); + return -4; + } + return res; +} + +void DetectRequiresRegister(void) +{ + sigmatch_table[DETECT_REQUIRES].name = "requires"; + sigmatch_table[DETECT_REQUIRES].desc = "require Suricata version or features"; + sigmatch_table[DETECT_REQUIRES].url = "/rules/meta-keywords.html#requires"; + sigmatch_table[DETECT_REQUIRES].Setup = DetectRequiresSetup; +} diff --git a/src/detect-requires.h b/src/detect-requires.h new file mode 100644 index 000000000000..70f1dc43b814 --- /dev/null +++ b/src/detect-requires.h @@ -0,0 +1,23 @@ +/* Copyright (C) 2023 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +#ifndef __DETECT_REQUIRES_H__ +#define __DETECT_REQUIRES_H__ + +void DetectRequiresRegister(void); + +#endif /* __DETECT_REQUIRES_H__ */ diff --git a/src/detect.h b/src/detect.h index 8278290a2992..5d55720758ea 100644 --- a/src/detect.h +++ b/src/detect.h @@ -53,6 +53,9 @@ struct SCSigOrderFunc_; struct SCSigSignatureWrapper_; +/* Forward declarations for structures from Rust. */ +typedef struct SCDetectRequiresStatus SCDetectRequiresStatus; + enum SignatureType { SIG_TYPE_NOT_SET = 0, SIG_TYPE_IPONLY, // rule is handled by IPONLY engine @@ -797,6 +800,7 @@ typedef struct SigFileLoaderStat_ { int total_files; int good_sigs_total; int bad_sigs_total; + int skipped_sigs_total; } SigFileLoaderStat; typedef struct DetectEngineThreadKeywordCtxItem_ { @@ -925,6 +929,9 @@ typedef struct DetectEngineCtx_ { bool sigerror_silent; bool sigerror_ok; + /** The rule errored out due to missing requirements. */ + bool sigerror_requires; + bool filedata_config_initialized; /* specify the configuration for mpm context factory */ @@ -1032,6 +1039,9 @@ typedef struct DetectEngineCtx_ { /* path to the tenant yaml for this engine */ char *tenant_path; + + /* Track rule requirements for reporting after loading rules. */ + SCDetectRequiresStatus *requirements; } DetectEngineCtx; /* Engine groups profiles (low, medium, high, custom) */ From 435c03172ed7ebaa117765760e75bdfd38c7fca0 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Wed, 29 Nov 2023 10:54:54 -0600 Subject: [PATCH 7/9] requires: pre-scan rule for requires expressions Add a "pre-scan" rule parse that will check for requires statement. It will return a special error code (-4) if the requires fails due to missing requirements. Syntactic errors will also abort parsing here. Feature: #5972 --- src/detect-parse.c | 62 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 15 deletions(-) diff --git a/src/detect-parse.c b/src/detect-parse.c index b94d54dd6cd0..493bee10ea9f 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -845,7 +845,8 @@ int SigMatchListSMBelongsTo(const Signature *s, const SigMatch *key_sm) return -1; } -static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr, char *output, size_t output_size) +static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr, char *output, + size_t output_size, bool requires) { SigTableElmt *st = NULL; char *optname = NULL; @@ -899,6 +900,12 @@ static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr, } optname = optstr; + if (requires) { + if (strcmp(optname, "requires")) { + goto finish; + } + } + /* Call option parsing */ st = SigTableGet(optname); if (st == NULL || st->Setup == NULL) { @@ -1038,6 +1045,7 @@ static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr, } s->init_data->negated = false; +finish: if (strlen(optend) > 0) { strlcpy(output, optend, output_size); return 1; @@ -1323,9 +1331,11 @@ static inline int SigParseList(char **input, char *output, /** * \internal * \brief split a signature string into a few blocks for further parsing + * + * \param scan_only just scan, don't validate */ -static int SigParseBasics(DetectEngineCtx *de_ctx, - Signature *s, const char *sigstr, SignatureParser *parser, uint8_t addrs_direction) +static int SigParseBasics(DetectEngineCtx *de_ctx, Signature *s, const char *sigstr, + SignatureParser *parser, uint8_t addrs_direction, bool scan_only) { char *index, dup[DETECT_MAX_RULE_SIZE]; @@ -1370,6 +1380,10 @@ static int SigParseBasics(DetectEngineCtx *de_ctx, } strlcpy(parser->opts, index, sizeof(parser->opts)); + if (scan_only) { + return 0; + } + /* Parse Action */ if (SigParseAction(s, parser->action) < 0) goto error; @@ -1431,12 +1445,13 @@ static inline bool CheckAscii(const char *str) * \param s memory structure to store the signature in * \param sigstr the raw signature as a null terminated string * \param addrs_direction direction (for bi-directional sigs) + * \param require only scan rule for requires * * \param -1 parse error * \param 0 ok */ -static int SigParse(DetectEngineCtx *de_ctx, Signature *s, - const char *sigstr, uint8_t addrs_direction, SignatureParser *parser) +static int SigParse(DetectEngineCtx *de_ctx, Signature *s, const char *sigstr, + uint8_t addrs_direction, SignatureParser *parser, bool requires) { SCEnter(); @@ -1450,12 +1465,7 @@ static int SigParse(DetectEngineCtx *de_ctx, Signature *s, SCReturnInt(-1); } - s->sig_str = SCStrdup(sigstr); - if (unlikely(s->sig_str == NULL)) { - SCReturnInt(-1); - } - - int ret = SigParseBasics(de_ctx, s, sigstr, parser, addrs_direction); + int ret = SigParseBasics(de_ctx, s, sigstr, parser, addrs_direction, requires); if (ret < 0) { SCLogDebug("SigParseBasics failed"); SCReturnInt(-1); @@ -1467,21 +1477,27 @@ static int SigParse(DetectEngineCtx *de_ctx, Signature *s, char input[buffer_size]; char output[buffer_size]; memset(input, 0x00, buffer_size); - memcpy(input, parser->opts, strlen(parser->opts)+1); + memcpy(input, parser->opts, strlen(parser->opts) + 1); /* loop the option parsing. Each run processes one option * and returns the rest of the option string through the * output variable. */ do { memset(output, 0x00, buffer_size); - ret = SigParseOptions(de_ctx, s, input, output, buffer_size); + ret = SigParseOptions(de_ctx, s, input, output, buffer_size, requires); if (ret == 1) { memcpy(input, output, buffer_size); } } while (ret == 1); + + if (ret < 0) { + /* Suricata didn't meet the rule requirements, skip. */ + goto end; + } } +end: DetectIPProtoRemoveAllSMs(de_ctx, s); SCReturnInt(ret); @@ -2142,17 +2158,33 @@ static Signature *SigInitHelper(DetectEngineCtx *de_ctx, const char *sigstr, if (sig == NULL) goto error; + sig->sig_str = SCStrdup(sigstr); + if (unlikely(sig->sig_str == NULL)) { + goto error; + } + /* default gid to 1 */ sig->gid = 1; - int ret = SigParse(de_ctx, sig, sigstr, dir, &parser); + /* We do a first parse of the rule in a requires, or scan-only + * mode. Syntactic errors will be picked up here, but the only + * part of the rule that is validated completely is the "requires" + * keyword. */ + int ret = SigParse(de_ctx, sig, sigstr, dir, &parser, true); if (ret == -4) { /* Rule requirements not met. */ de_ctx->sigerror_silent = true; de_ctx->sigerror_ok = true; de_ctx->sigerror_requires = true; goto error; - } else if (ret == -3) { + } else if (ret < 0) { + goto error; + } + + /* Now completely parse the rule. */ + ret = SigParse(de_ctx, sig, sigstr, dir, &parser, false); + BUG_ON(ret == -4); + if (ret == -3) { de_ctx->sigerror_silent = true; de_ctx->sigerror_ok = true; goto error; From 71bbba9248e696f0fd2e912ad9631052b3788775 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Wed, 29 Nov 2023 12:57:23 -0600 Subject: [PATCH 8/9] detect-parse: parse sid in pre-scan During the pre-scan for "requires", also parse the SID if possible. If the rule fails high level parsing (syntax), the SID will not be parsed. But every keyword other than "sid" and "requires" should expect to be provided with a parsed sid. --- src/detect-parse.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/detect-parse.c b/src/detect-parse.c index 493bee10ea9f..45f188df1167 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -900,10 +900,11 @@ static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr, } optname = optstr; - if (requires) { - if (strcmp(optname, "requires")) { - goto finish; - } + /* Check for options that are only to be processed during the + * first "requires" pass. */ + bool requires_only = strcmp(optname, "requires") == 0 || strcmp(optname, "sid") == 0; + if ((requires && !requires_only) || (!requires && requires_only)) { + goto finish; } /* Call option parsing */ @@ -2137,10 +2138,7 @@ static int SigValidate(DetectEngineCtx *de_ctx, Signature *s) AppLayerHtpNeedFileInspection(); } } - if (s->id == 0) { - SCLogError("Signature missing required value \"sid\"."); - SCReturnInt(0); - } + SCReturnInt(1); } @@ -2181,6 +2179,12 @@ static Signature *SigInitHelper(DetectEngineCtx *de_ctx, const char *sigstr, goto error; } + /* Check for a SID before continuuing. */ + if (sig->id == 0) { + SCLogError("Signature missing required value \"sid\"."); + goto error; + } + /* Now completely parse the rule. */ ret = SigParse(de_ctx, sig, sigstr, dir, &parser, false); BUG_ON(ret == -4); From 5cc872fa1a328a38939f72f0f21b4e9a46900dee Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Thu, 14 Dec 2023 12:32:59 -0600 Subject: [PATCH 9/9] rust.h: don't include util-file.h, not needed --- src/rust.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/rust.h b/src/rust.h index 12c90e67f30d..8ab174f9ec78 100644 --- a/src/rust.h +++ b/src/rust.h @@ -18,8 +18,6 @@ #ifndef __RUST_H__ #define __RUST_H__ -#include "util-file.h" - // hack for include orders cf SCSha256 typedef struct HttpRangeContainerBlock HttpRangeContainerBlock; #include "rust-context.h"