7.0.x: backport treating unknown requirements as unsatisfied with opt-out flag - v1

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 ``<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

doc/userguide/upgrade.rst

@@ -34,6 +34,27 @@ 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 unsatisfied requirements, causing the rule to not be loaded. See
+  :ref:`keyword_requires`. To opt out of this change and to ignore
+  uknown requirements, effectively treating them as satified the
+  ``ignore-unknown-requirements`` configuration option can be used.
+
+  Command line example::
+
+    --set ignore-unknown-requirements=true
+
+  Or as a top-level configuration option in ``suricata.yaml``:
+
+  .. code-block:: yaml
+
+    default-rule-path: /var/lib/suricata/rules
+    rule-files:
+      - suricata.rules
+    ignore-unknown-requirements: true
+
 Upgrading 6.0 to 7.0
 --------------------
 

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<Vec<RuleRequireVersion>>,
+
+    /// Unknown parameters to requires.
+    pub unknown: Vec<String>,
 }
 
 fn parse_op(input: &str) -> IResult<&str, VersionCompareOp> {
@@ -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));
             }
         }
 
@@ -289,8 +297,14 @@ fn check_version(
 }
 
 fn check_requires(
-    requires: &Requires, suricata_version: &SuricataVersion,
+    requires: &Requires, suricata_version: &SuricataVersion, ignore_unknown: bool,
 ) -> Result<(), RequiresError> {
+    if !ignore_unknown && !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;
@@ -439,7 +453,7 @@ pub unsafe extern "C" fn SCDetectRequiresStatusLog(
 #[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,
+    status: &mut SCDetectRequiresStatus, ignore_unknown: c_int,
 ) -> c_int {
     // First parse the running Suricata version.
     let suricata_version = match parse_suricata_version(CStr::from_ptr(suricata_version_string)) {
@@ -462,7 +476,9 @@ pub unsafe extern "C" fn SCDetectCheckRequires(
         }
     };
 
-    match check_requires(&requires, &suricata_version) {
+    let ignore_unknown = ignore_unknown != 0;
+
+    match check_requires(&requires, &suricata_version, ignore_unknown) {
         Ok(()) => 0,
         Err(err) => {
             match &err {
@@ -483,6 +499,7 @@ pub unsafe extern "C" fn SCDetectCheckRequires(
                 RequiresError::VersionGt => {
                     status.gt_count += 1;
                 }
+                RequiresError::UnknownRequirement(_) => {}
                 _ => {}
             }
             *errstr = err.c_errmsg();
@@ -594,6 +611,7 @@ mod test {
                         patch: 0,
                     }
                 }]],
+                unknown: vec![],
             }
         );
 
@@ -610,6 +628,7 @@ mod test {
                         patch: 0,
                     }
                 }]],
+                unknown: vec![],
             }
         );
 
@@ -626,6 +645,7 @@ mod test {
                         patch: 2,
                     }
                 }]],
+                unknown: vec![],
             }
         );
 
@@ -652,6 +672,7 @@ mod test {
                         }
                     }
                 ]],
+                unknown: vec![],
             }
         );
     }
@@ -662,7 +683,7 @@ mod test {
         let suricata_version = SuricataVersion::new(7, 0, 4);
         let requires = parse_requires("version >= 8").unwrap();
         assert_eq!(
-            check_requires(&requires, &suricata_version),
+            check_requires(&requires, &suricata_version, false),
             Err(RequiresError::VersionLt(SuricataVersion {
                 major: 8,
                 minor: 0,
@@ -673,86 +694,86 @@ mod test {
         // 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(()));
+        assert_eq!(check_requires(&requires, &suricata_version, false), 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),
+            check_requires(&requires, &suricata_version, false),
             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(()));
+        assert_eq!(check_requires(&requires, &suricata_version, false), 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),
+            check_requires(&requires, &suricata_version, false),
             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(()));
+        assert_eq!(check_requires(&requires, &suricata_version, false), 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());
+        assert!(check_requires(&requires, &suricata_version, false).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());
+        assert!(check_requires(&requires, &suricata_version, false).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());
+        assert!(check_requires(&requires, &suricata_version, false).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());
+        assert!(check_requires(&requires, &suricata_version, false).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());
+        assert!(check_requires(&requires, &suricata_version, false).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());
+        assert!(check_requires(&requires, &suricata_version, false).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());
+        assert!(check_requires(&requires, &suricata_version, false).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());
+        assert!(check_requires(&requires, &SuricataVersion::new(6, 0, 0), false).is_err());
+        assert!(check_requires(&requires, &SuricataVersion::new(7, 0, 4), false).is_err());
+        assert!(check_requires(&requires, &SuricataVersion::new(7, 0, 5), false).is_ok());
+        assert!(check_requires(&requires, &SuricataVersion::new(8, 0, 3), false).is_err());
+        assert!(check_requires(&requires, &SuricataVersion::new(8, 0, 4), false).is_ok());
+        assert!(check_requires(&requires, &SuricataVersion::new(9, 0, 2), false).is_err());
+        assert!(check_requires(&requires, &SuricataVersion::new(9, 0, 3), false).is_ok());
+        assert!(check_requires(&requires, &SuricataVersion::new(10, 0, 0), false).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());
+        assert!(check_requires(&requires, &SuricataVersion::new(6, 0, 0), false).is_err());
+        assert!(check_requires(&requires, &SuricataVersion::new(7, 0, 0), false).is_err());
+        assert!(check_requires(&requires, &SuricataVersion::new(8, 0, 0), false).is_ok());
+        assert!(check_requires(&requires, &SuricataVersion::new(9, 0, 0), false).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 +782,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), false).is_err());
     }
 
     #[test]
@@ -802,4 +829,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), false).is_err());
+    }
 }

src/detect-requires.c

@@ -20,6 +20,10 @@
 #include "detect-engine.h"
 #include "rust.h"
 
+/* Set to true if unknown requirements should be ingored. In Suricata
+ * 8, unknown requirements are treated as unsatisfied requirements. */
+static int g_ignore_unknown_requirements = 0;
+
 static int DetectRequiresSetup(DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
 {
     if (de_ctx->requirements == NULL) {
@@ -28,7 +32,8 @@ static int DetectRequiresSetup(DetectEngineCtx *de_ctx, Signature *s, const char
     }
 
     const char *errmsg = NULL;
-    int res = SCDetectCheckRequires(rawstr, PROG_VER, &errmsg, de_ctx->requirements);
+    int res = SCDetectCheckRequires(
+            rawstr, PROG_VER, &errmsg, de_ctx->requirements, g_ignore_unknown_requirements);
     if (res == -1) {
         // The requires expression is bad, log an error.
         SCLogError("%s: %s", errmsg, rawstr);
@@ -43,6 +48,8 @@ static int DetectRequiresSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 
 void DetectRequiresRegister(void)
 {
+    ConfGetBool("ignore-unknown-requirements", &g_ignore_unknown_requirements);
+
     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";