Skip to content

Commit

Permalink
requires: treat unknown requires keywords as unmet requirements
Browse files Browse the repository at this point in the history
For example, "requires: foo bar" is an unknown requirement, however
its not tracked, nor an error as it follows the syntax. Instead,
record these unknown keywords, and fail the requirements check if any
are present.

A future version of Suricata may have new requires keywords, for
example a check for keywords.

Ticket: #7418
(cherry picked from commit 820a3e5)
  • Loading branch information
jasonish committed Dec 4, 2024
1 parent 093397f commit 26fd130
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 2 deletions.
7 changes: 7 additions & 0 deletions doc/userguide/rules/meta.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
--------

Expand All @@ -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 ``<keyword>
<expression>`` 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
Expand Down
6 changes: 6 additions & 0 deletions doc/userguide/upgrade.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ also check all the new features that have been added but are not covered by
this guide. Those features are either not enabled by default or require
dedicated new configuration.

Upgrading to 7.0.7
------------------
- Unknown requirements in the ``requires`` keyword will now be treated
as unmet requirements, causing the rule to not be loaded. See
:ref:`keyword_requires`.

Upgrading 6.0 to 7.0
--------------------

Expand Down
34 changes: 32 additions & 2 deletions rust/src/detect/requires.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down Expand Up @@ -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<Vec<RuleRequireVersion>>,

/// Unknown parameters to requires.
pub unknown: Vec<String>,
}

fn parse_op(input: &str) -> IResult<&str, VersionCompareOp> {
Expand Down Expand Up @@ -242,6 +249,7 @@ fn parse_requires(mut input: &str) -> Result<Requires, RequiresError> {
// Unknown keyword, allow by warn in case we extend
// this in the future.
SCLogWarning!("Unknown requires keyword: {}", keyword);
requires.unknown.push(format!("{} {}", keyword, value));
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -594,6 +608,7 @@ mod test {
patch: 0,
}
}]],
unknown: vec![],
}
);

Expand All @@ -610,6 +625,7 @@ mod test {
patch: 0,
}
}]],
unknown: vec![],
}
);

Expand All @@ -626,6 +642,7 @@ mod test {
patch: 2,
}
}]],
unknown: vec![],
}
);

Expand All @@ -652,6 +669,7 @@ mod test {
}
}
]],
unknown: vec![],
}
);
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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]
Expand Down Expand Up @@ -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());
}
}

0 comments on commit 26fd130

Please sign in to comment.