DEV Community

TiltedLunar123
TiltedLunar123

Posted on

How one line in my log scanner's allowlist was muting alerts I never told it to

threatlens is a log triage cli i've been building. you point it at windows event logs, or syslog, or cef, it runs a stack of detections, and it hands back alerts mapped to mitre att&ck. brute force, lateral movement, priv esc, the usual suspects. one of the features is an allowlist: a little yaml file where you say "these alerts are known good, stop showing them to me."

every detection tool needs this. a service account fails auth all day long and you don't want to see it every scan. so you add an entry and move on with your life.

i actually found this bug writing the docs, which is a bit embarrassing. here's roughly the example i was putting in the readme:

allowlist:
  - rule_name: "Brute-Force"
    username: "svc_monitor"
    reason: "service account, expected failed auths"
Enter fullscreen mode Exit fullscreen mode

read that and you'd assume it suppresses brute-force alerts for that one service account. that's what i assumed while typing it.

the bug

rule_name was matched with substring containment. the check was basically entry_name in alert_name. so "Brute" doesn't only match the brute-force detector. it matches any alert whose rule name contains the letters b-r-u-t-e. yes, the username was a second condition that narrowed it, but the shape of the problem stands on its own: the more generic your rule_name string is, the more you're muting without realising it.

the nasty case is short entries. someone tunes out a noisy privilege rule with rule_name: "Priv". now every rule with "priv" in its name is gone from the results. you meant to silence one alert and you took a handful of others down with it.

in a detection tool that is the exact wrong direction to fail. it's a false negative you caused on purpose, and it doesn't show up anywhere. no error, no count, no "hey this entry is greedy." the alerts just stop existing and your scan looks clean.

the fix

the v3.0.0 fix is boring, which is the point. exact, case-insensitive equality:

def _alert_allowed(alert, allowlist):
    for entry in allowlist:
        match = True
        # rule_name is matched on exact (case-insensitive) equality so that an
        # entry like "Brute" does not silently suppress "Brute-Force Detected".
        if "rule_name" in entry and entry["rule_name"].lower() != alert.rule_name.lower():
            match = False
        # ...username, computer, source_ip, severity checks...
        if match:
            return entry.get("reason", "allowlisted")
    return None
Enter fullscreen mode Exit fullscreen mode

that's the whole change. != instead of not in. now "Brute" suppresses the rule literally named "Brute" and nothing else. if you want to mute "Brute-Force Detected" you write the full name. a regression test pins the "Brute doesn't eat Brute-Force" case so it can't quietly come back.

it's technically a breaking change, which is annoying but honest. anyone who leaned on the old partial matching now has entries that match nothing, so their old noise reappears after they upgrade. that's why it shipped as a major version bump with a changelog line telling people to update allowlists that relied on partial matches. felt heavy for a two-character diff, but the behavior genuinely changed for existing configs, so semver says major.

what's still not great

two things i want to be straight about, because "i fixed the bug" is only half true.

first, i fixed rule_name. the same function still does substring matching for mitre_technique:

if "mitre_technique" in entry and entry["mitre_technique"].upper() not in alert.mitre_technique.upper():
    match = False
Enter fullscreen mode Exit fullscreen mode

so mitre_technique: "T1048" still suppresses "T1048.003" and anything else carrying T1048. i left it on purpose, because for att&ck a parent technique muting its sub-techniques is arguably what you want. but it is the exact pattern i just called a footgun a few lines up, and the inconsistency isn't documented anywhere except this post. if i were reviewing someone else's PR i'd flag it and ask them to at least write down why the two fields disagree.

second, and this one bugs me more: there's still no warning when an allowlist entry matches zero rule names. so if you upgrade to 3.0.0 and your old rule_name: "Brute" entry silently stops matching, nothing tells you. your tuning just quietly dies and your alert count creeps up with no explanation. the safe version of this fix would print "entry 'Brute' matched no alerts this scan" at the end. it's not in there yet. it should be.

if i started over

i'd make rule_name exact from day one and add an explicit rule_name_contains field for the people who actually want partial matching, instead of making greedy substring the silent default that everyone gets whether they wanted it or not. opt in to the footgun, don't opt out.

anyway, it works now, the suite is at 419 tests, and the whole thing runs offline with pyyaml as the only real dependency. it's defensive-only and mit licensed.

repo: https://github.com/TiltedLunar123/ThreatLens
pip install threatlens-cli

if you've got detection tooling with fuzzy matching baked into the tuning layer, go check whether "helpful" partial matching is quietly eating alerts you wanted to keep. it's an easy thing to ship and a hard thing to notice.

Top comments (0)