diff --git a/doc/userguide/rules/meta.rst b/doc/userguide/rules/meta.rst index 1ceb5fe834..89a98972e7 100644 --- a/doc/userguide/rules/meta.rst +++ b/doc/userguide/rules/meta.rst @@ -212,6 +212,8 @@ 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. +.. _keyword_requires: + requires -------- @@ -220,6 +222,11 @@ 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. +Requirements that follow the valid format of `` +`` but are not known to Suricata are allowed for future +compatiblity, however unknown requirement expressions will lead to the +requirement not being met, skipping the rule. + 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 diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index 63e2146280..7172c6b1c8 100644 --- a/doc/userguide/upgrade.rst +++ b/doc/userguide/upgrade.rst @@ -79,6 +79,9 @@ Major changes - sip.content_length - Napatech support has been moved to a capture plugin. See :doc:`Napatech plugin `. +- Unknown requirements in the ``requires`` keyword will now be treated + as unmet requirements, causing the rule to not be loaded. See + :ref:`keyword_requires`. Removals ~~~~~~~~ diff --git a/rust/src/detect/requires.rs b/rust/src/detect/requires.rs index e9e1acac50..2635605d26 100644 --- a/rust/src/detect/requires.rs +++ b/rust/src/detect/requires.rs @@ -57,6 +57,9 @@ enum RequiresError { /// Passed in requirements not a valid UTF-8 string. Utf8Error, + + /// An unknown requirement was provided. + UnknownRequirement(String), } impl RequiresError { @@ -70,6 +73,7 @@ impl RequiresError { 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", + Self::UnknownRequirement(_) => "Unknown requirements\0", }; msg.as_ptr() as *const c_char } @@ -169,6 +173,9 @@ struct Requires { /// - All of the inner most must evaluate to true. /// - To pass, any of the outer must be true. pub version: Vec>, + + /// Unknown parameters to requires. + pub unknown: Vec, } fn parse_op(input: &str) -> IResult<&str, VersionCompareOp> { @@ -242,6 +249,7 @@ fn parse_requires(mut input: &str) -> Result { // Unknown keyword, allow by warn in case we extend // this in the future. SCLogWarning!("Unknown requires keyword: {}", keyword); + requires.unknown.push(format!("{} {}", keyword, value)); } } @@ -291,6 +299,12 @@ fn check_version( fn check_requires( requires: &Requires, suricata_version: &SuricataVersion, ) -> Result<(), RequiresError> { + if !requires.unknown.is_empty() { + return Err(RequiresError::UnknownRequirement( + requires.unknown.join(","), + )); + } + if !requires.version.is_empty() { let mut errs = VecDeque::new(); let mut ok = 0; @@ -594,6 +608,7 @@ mod test { patch: 0, } }]], + unknown: vec![], } ); @@ -610,6 +625,7 @@ mod test { patch: 0, } }]], + unknown: vec![], } ); @@ -626,6 +642,7 @@ mod test { patch: 2, } }]], + unknown: vec![], } ); @@ -652,6 +669,7 @@ mod test { } } ]], + unknown: vec![], } ); } @@ -748,11 +766,11 @@ mod test { 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(); + let requires = parse_requires("feature true_lua, foo bar, version >= 7.0.3").unwrap(); assert_eq!( requires, Requires { - features: vec!["lua".to_string()], + features: vec!["true_lua".to_string()], version: vec![vec![RuleRequireVersion { op: VersionCompareOp::Gte, version: SuricataVersion { @@ -761,8 +779,14 @@ mod test { patch: 3, } }]], + unknown: vec!["foo bar".to_string()], } ); + + // This should not pass the requires check as it contains an + // unknown requires keyword. + //check_requires(&requires, &SuricataVersion::new(8, 0, 0)).unwrap(); + assert!(check_requires(&requires, &SuricataVersion::new(8, 0, 0)).is_err()); } #[test] @@ -802,4 +826,10 @@ mod test { ] ); } + + #[test] + fn test_requires_keyword() { + let requires = parse_requires("keyword true_bar").unwrap(); + assert!(check_requires(&requires, &SuricataVersion::new(8, 0, 0)).is_err()); + } }