next/653/20241202/v1

.github/workflows/builds.yml

@@ -747,7 +747,7 @@ jobs:
       - run: llvm-profdata merge -o default.profdata $(find suricata-verify/tests/ -name '*.profraw')
       - run: llvm-cov show ./src/suricata -instr-profile=default.profdata --show-instantiations --ignore-filename-regex="^/root/.*" > coverage.txt
       - name: Upload coverage to Codecov
-        uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238
+        uses: codecov/codecov-action@015f24e6818733317a2da2edd6290ab26238649a
         with:
           fail_ci_if_error: false
           flags: suricata-verify
@@ -1558,7 +1558,7 @@ jobs:
       - run: llvm-profdata-14 merge -o htp-test.profdata /tmp/htp-test.profraw
       - run: llvm-cov-14 show libhtp/test/test_all -instr-profile=htp-test.profdata --show-instantiations --ignore-filename-regex="^/root/.*" >> coverage.txt
       - name: Upload coverage to Codecov
-        uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238
+        uses: codecov/codecov-action@015f24e6818733317a2da2edd6290ab26238649a
         with:
           fail_ci_if_error: false
           flags: unittests
@@ -1664,7 +1664,7 @@ jobs:
       - run: llvm-profdata-14 merge -o default.profdata $(find /tmp/ -name '*.profraw')
       - run: llvm-cov-14 show ./src/suricata -instr-profile=default.profdata --show-instantiations --ignore-filename-regex="^/root/.*" > coverage.txt
       - name: Upload coverage to Codecov
-        uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238
+        uses: codecov/codecov-action@015f24e6818733317a2da2edd6290ab26238649a
         with:
           fail_ci_if_error: false
           flags: pcap
@@ -1801,7 +1801,7 @@ jobs:
       - run: llvm-profdata-14 merge -o default.profdata $(find /tmp/ -name '*.profraw')
       - run: llvm-cov-14 show ./src/suricata -instr-profile=default.profdata --show-instantiations --ignore-filename-regex="^/root/.*" > coverage.txt
       - name: Upload coverage to Codecov
-        uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238
+        uses: codecov/codecov-action@015f24e6818733317a2da2edd6290ab26238649a
         with:
           fail_ci_if_error: false
           flags: livemode
@@ -2093,7 +2093,7 @@ jobs:
       - run: llvm-profdata-14 merge -o default.profdata $(find /tmp/ -name '*.profraw')
       - run: llvm-cov-14 show ./src/suricata -instr-profile=default.profdata --show-instantiations --ignore-filename-regex="^/root/.*" > coverage.txt
       - name: Upload coverage to Codecov
-        uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238
+        uses: codecov/codecov-action@015f24e6818733317a2da2edd6290ab26238649a
         with:
           fail_ci_if_error: false
           flags: fuzzcorpus

.github/workflows/codeql.yml

@@ -41,7 +41,7 @@ jobs:
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
-      uses: github/codeql-action/[email protected].0
+      uses: github/codeql-action/[email protected].5
       with:
         languages: ${{ matrix.language }}
         queries: security-extended
@@ -62,4 +62,4 @@ jobs:
        ./configure --enable-warnings
        make
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/[email protected].0
+      uses: github/codeql-action/[email protected].5

.github/workflows/scorecards-analysis.yml

@@ -51,6 +51,6 @@ jobs:
 
       # Upload the results to GitHub's code scanning dashboard.
       - name: "Upload SARIF results"
-        uses: github/codeql-action/upload-sarif@cbe18979603527f12c7871a6eb04833ecf1548c7 # v1
+        uses: github/codeql-action/upload-sarif@3d3d628990a5f99229dd9fa1821cc5a4f31b613b # v1
         with:
           sarif_file: results.sarif

doc/userguide/rules/meta.rst

@@ -212,13 +212,21 @@ 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
 --------
 
 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.
+features to be enabled, specific keywords to be available, or the
+Suricata version to match an expression. Rules that do not meet the
+requirements will be 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
@@ -228,7 +236,7 @@ still adhere to the basic known formats of Suricata rules.
 
 The format is::
 
-   requires: feature geoip, version >= 7.0.0
+   requires: feature geoip, version >= 7.0.0, keyword foobar
 
 To require multiple features, the feature sub-keyword must be
 specified multiple times::
@@ -243,7 +251,7 @@ 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
+to express that a rule 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.

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
   <upgrade/8.0-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
 ~~~~~~~~

rust/src/detect/requires.rs

@@ -57,6 +57,12 @@ enum RequiresError {
 
     /// Passed in requirements not a valid UTF-8 string.
     Utf8Error,
+
+    /// An unknown requirement was provided.
+    UnknownRequirement(String),
+
+    /// Suricata does not have support for a required keyword.
+    MissingKeyword(String),
 }
 
 impl RequiresError {
@@ -70,6 +76,8 @@ 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",
+            Self::MissingKeyword(_) => "Suricata missing a required keyword\0",
         };
         msg.as_ptr() as *const c_char
     }
@@ -162,13 +170,20 @@ struct RuleRequireVersion {
 
 #[derive(Debug, Default, Eq, PartialEq)]
 struct Requires {
+    /// Features required to be enabled.
     pub features: Vec<String>,
 
+    /// Rule keywords required to exist.
+    pub keywords: Vec<String>,
+
     /// The version expression.
     ///
     /// - 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> {
@@ -238,10 +253,14 @@ fn parse_requires(mut input: &str) -> Result<Requires, RequiresError> {
                     parse_version_expression(value).map_err(|_| RequiresError::BadRequires)?;
                 requires.version = versions;
             }
+            "keyword" => {
+                requires.keywords.push(value.trim().to_string());
+            }
             _ => {
                 // 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 +310,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;
@@ -319,6 +344,12 @@ fn check_requires(
         }
     }
 
+    for keyword in &requires.keywords {
+        if !crate::feature::has_keyword(keyword) {
+            return Err(RequiresError::MissingKeyword(keyword.to_string()));
+        }
+    }
+
     Ok(())
 }
 
@@ -586,6 +617,7 @@ mod test {
             requires,
             Requires {
                 features: vec![],
+                keywords: vec![],
                 version: vec![vec![RuleRequireVersion {
                     op: VersionCompareOp::Gte,
                     version: SuricataVersion {
@@ -594,6 +626,7 @@ mod test {
                         patch: 0,
                     }
                 }]],
+                unknown: vec![],
             }
         );
 
@@ -602,6 +635,7 @@ mod test {
             requires,
             Requires {
                 features: vec![],
+                keywords: vec![],
                 version: vec![vec![RuleRequireVersion {
                     op: VersionCompareOp::Gte,
                     version: SuricataVersion {
@@ -610,6 +644,7 @@ mod test {
                         patch: 0,
                     }
                 }]],
+                unknown: vec![],
             }
         );
 
@@ -618,6 +653,7 @@ mod test {
             requires,
             Requires {
                 features: vec!["output::file-store".to_string()],
+                keywords: vec![],
                 version: vec![vec![RuleRequireVersion {
                     op: VersionCompareOp::Gte,
                     version: SuricataVersion {
@@ -626,6 +662,7 @@ mod test {
                         patch: 2,
                     }
                 }]],
+                unknown: vec![],
             }
         );
 
@@ -634,6 +671,7 @@ mod test {
             requires,
             Requires {
                 features: vec!["geoip".to_string()],
+                keywords: vec![],
                 version: vec![vec![
                     RuleRequireVersion {
                         op: VersionCompareOp::Gte,
@@ -652,6 +690,7 @@ mod test {
                         }
                     }
                 ]],
+                unknown: vec![],
             }
         );
     }
@@ -748,11 +787,12 @@ 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()],
+                keywords: vec![],
                 version: vec![vec![RuleRequireVersion {
                     op: VersionCompareOp::Gte,
                     version: SuricataVersion {
@@ -761,8 +801,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 +848,13 @@ 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_ok());
+
+        let requires = parse_requires("keyword bar").unwrap();
+        assert!(check_requires(&requires, &SuricataVersion::new(8, 0, 0)).is_err());
+    }
 }

rust/src/feature.rs

@@ -32,6 +32,15 @@ mod mock {
     pub fn requires(feature: &str) -> bool {
         return feature.starts_with("true");
     }
+
+    /// Check for a keyword returning true if found.
+    ///
+    /// This a "mock" variant of `has_keyword` that will return true
+    /// for any keyword starting with string `true`, and false for
+    /// anything else.
+    pub fn has_keyword(keyword: &str) -> bool {
+        return keyword.starts_with("true");
+    }
 }
 
 #[cfg(not(test))]
@@ -41,6 +50,7 @@ mod real {
 
     extern "C" {
         fn RequiresFeature(feature: *const c_char) -> bool;
+        fn SigTableHasKeyword(keyword: *const c_char) -> bool;
     }
 
     /// Check for a feature returning true if found.
@@ -51,6 +61,14 @@ mod real {
             false
         }
     }
+
+    pub fn has_keyword(keyword: &str) -> bool {
+        if let Ok(keyword) = CString::new(keyword) {
+            unsafe { SigTableHasKeyword(keyword.as_ptr()) }
+        } else {
+            false
+        }
+    }
 }
 
 #[cfg(not(test))]

src/detect-engine-register.c

@@ -347,6 +347,28 @@ static void SigMultilinePrint(int i, const char *prefix)
     printf("\n");
 }
 
+/** \brief Check if a keyword exists. */
+bool SigTableHasKeyword(const char *keyword)
+{
+    for (int i = 0; i < DETECT_TBLSIZE; i++) {
+        if (sigmatch_table[i].flags & SIGMATCH_NOT_BUILT) {
+            continue;
+        }
+
+        const char *name = sigmatch_table[i].name;
+
+        if (name == NULL || strlen(name) == 0) {
+            continue;
+        }
+
+        if (strcmp(keyword, name) == 0) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 int SigTableList(const char *keyword)
 {
     size_t size = DETECT_TBLSIZE;

src/detect-engine-register.h

@@ -24,6 +24,8 @@
 #ifndef SURICATA_DETECT_ENGINE_REGISTER_H
 #define SURICATA_DETECT_ENGINE_REGISTER_H
 
+#include "suricata-common.h"
+
 enum DetectKeywordId {
     DETECT_SID,
     DETECT_PRIORITY,
@@ -342,5 +344,6 @@ void SigTableCleanup(void);
 void SigTableInit(void);
 void SigTableSetup(void);
 void SigTableRegisterTests(void);
+bool SigTableHasKeyword(const char *keyword);
 
 #endif /* SURICATA_DETECT_ENGINE_REGISTER_H */