Order rule traversal in RuleMatcher::Match() operations by Rule index

This ordering fixes a test failure we're seeing on Alpine for the
signatures/tcp-end-of-match btest, since discrepancies in rule match traversal
could lead to discrepancies in corresponding event ordering.

It looks safe to rely on across platforms since the index is driven by signature
load order, which shouldn't deviate. If this somehow doesn't hold in the future,
we'll only wind up with a test failure, not incorrect match behavior.

(Correction to 2e03fbb8b0, which I pushed
accidentally.)
This commit is contained in:
Christian Kreibich 2024-11-14 16:30:11 -08:00
parent 5e0e2a8bd8
commit b24c5c0e46
3 changed files with 10 additions and 20 deletions

View file

@ -59,9 +59,6 @@ public:
void PrintDebug(); void PrintDebug();
bool operator==(const Rule& other) { return strcmp(ID(), other.ID()) == 0; }
bool operator<(const Rule& other) { return strcmp(ID(), other.ID()) < 0; }
static const char* TypeToString(Rule::PatternType type); static const char* TypeToString(Rule::PatternType type);
private: private:

View file

@ -657,8 +657,7 @@ RuleMatcher::MIME_Matches* RuleMatcher::Match(RuleFileMagicState* state, const u
} }
// Find rules for which patterns have matched. // Find rules for which patterns have matched.
auto cmp = [](Rule* a, Rule* b) { return *a < *b; }; map<decltype(Rule::idx), Rule*> rule_matches;
set<Rule*, decltype(cmp)> rule_matches(cmp);
for ( AcceptingMatchSet::const_iterator it = accepted_matches.begin(); it != accepted_matches.end(); ++it ) { for ( AcceptingMatchSet::const_iterator it = accepted_matches.begin(); it != accepted_matches.end(); ++it ) {
auto [aidx, mpos] = *it; auto [aidx, mpos] = *it;
@ -666,11 +665,11 @@ RuleMatcher::MIME_Matches* RuleMatcher::Match(RuleFileMagicState* state, const u
Rule* r = Rule::rule_table[aidx - 1]; Rule* r = Rule::rule_table[aidx - 1];
if ( AllRulePatternsMatched(r, mpos, accepted_matches) ) if ( AllRulePatternsMatched(r, mpos, accepted_matches) )
rule_matches.insert(r); rule_matches[r->Index()] = r;
} }
for ( set<Rule*>::const_iterator it = rule_matches.begin(); it != rule_matches.end(); ++it ) { for ( const auto& entry : rule_matches ) {
Rule* r = *it; Rule* r = entry.second;
for ( const auto& action : r->actions ) { for ( const auto& action : r->actions ) {
const RuleActionMIME* ram = dynamic_cast<const RuleActionMIME*>(action); const RuleActionMIME* ram = dynamic_cast<const RuleActionMIME*>(action);
@ -842,12 +841,7 @@ void RuleMatcher::Match(RuleEndpointState* state, Rule::PatternType type, const
// matched patterns per connection (which is a plausible assumption). // matched patterns per connection (which is a plausible assumption).
// Find rules for which patterns have matched. // Find rules for which patterns have matched.
auto cmp = [](pair<Rule*, MatchPos> a, pair<Rule*, MatchPos> b) { map<decltype(Rule::idx), std::pair<Rule*, MatchPos>> rule_matches;
if ( *a.first == *b.first )
return a.second < b.second;
return *a.first < *b.first;
};
set<pair<Rule*, MatchPos>, decltype(cmp)> rule_matches(cmp);
for ( AcceptingMatchSet::const_iterator it = accepted_matches.begin(); it != accepted_matches.end(); ++it ) { for ( AcceptingMatchSet::const_iterator it = accepted_matches.begin(); it != accepted_matches.end(); ++it ) {
AcceptIdx aidx = it->first; AcceptIdx aidx = it->first;
@ -856,13 +850,12 @@ void RuleMatcher::Match(RuleEndpointState* state, Rule::PatternType type, const
Rule* r = Rule::rule_table[aidx - 1]; Rule* r = Rule::rule_table[aidx - 1];
if ( AllRulePatternsMatched(r, mpos, accepted_matches) ) if ( AllRulePatternsMatched(r, mpos, accepted_matches) )
rule_matches.insert(make_pair(r, mpos)); rule_matches[r->Index()] = make_pair(r, mpos);
} }
// Check which of the matching rules really belong to any of our nodes. // Check which of the matching rules really belong to any of our nodes.
for ( const auto& entry : rule_matches ) {
for ( set<pair<Rule*, MatchPos>>::const_iterator it = rule_matches.begin(); it != rule_matches.end(); ++it ) { auto [r, match_end_pos] = entry.second;
auto [r, match_end_pos] = *it;
DBG_LOG(DBG_RULES, "Accepted rule: %s", r->id); DBG_LOG(DBG_RULES, "Accepted rule: %s", r->id);

View file

@ -1,7 +1,7 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
signature_match, message, 1448
signature_match with end_of_match, message, 1448, rather than all. (Robin Sommer)\x0a\x0a * Fix parallel make portability
portability_match with end_of_match, 1448, rather than all. (Robin Sommer)\x0a\x0a * Fix parallel make portability portability_match with end_of_match, 1448, rather than all. (Robin Sommer)\x0a\x0a * Fix parallel make portability
portability_match, 1448 portability_match, 1448
portability_match_with_msg with end_of_match, custom message, 1448, 69, rather than all. (Robin Sommer)\x0a\x0a * Fix parallel make portability portability_match_with_msg with end_of_match, custom message, 1448, 69, rather than all. (Robin Sommer)\x0a\x0a * Fix parallel make portability
portability_match_with_msg, custom message, 1448 portability_match_with_msg, custom message, 1448
signature_match, message, 1448
signature_match with end_of_match, message, 1448, rather than all. (Robin Sommer)\x0a\x0a * Fix parallel make portability