Fix triagebot matching (#1608)

* Match alerts based on their classname and allow supported alerts to be disabled in config

alerts/actions/triage_bot.json

@@ -1,4 +1,10 @@
 {
+  "enabled_alert_classnames": [
+    "AlertGenericLoader:ssh_open_crit",
+    "AlertAuthSignRelengSSH",
+    "AlertGenericLoader:duosecurity_bypass_generated",
+    "AlertGenericLoader:duosecurity_bypass_used"
+  ],
   "oauth_url": "https://auth.mozilla.auth0.com/oauth/token",
   "person_api_base": "https://person-api.sso.mozilla.com",
   "person_api_audience": "api.sso.mozilla.com",

alerts/actions/triage_bot.py

@@ -20,6 +20,7 @@
 
 CONFIG_FILE = os.path.join(os.path.dirname(__file__), "triage_bot.json")
 
+
 Alert = types.Dict[types.Any, types.Any]
 Email = str
 
@@ -53,6 +54,8 @@ class Config(types.NamedTuple):
     """Container type for the configuration parameters required by the
     alert action.
     """
+
+    enabled_alert_classnames: types.List[str]
     oauth_url: str
     person_api_base: str
     person_api_audience: str
@@ -188,6 +191,12 @@ def __init__(self, err_msg):
 DiscoveryInterface = types.Callable[[], types.List[LambdaFunction]]
 DispatchInterface = types.Callable[[AlertTriageRequest, str], DispatchResult]
 
+# Supported alerts have functions that can process those alerts along with a
+# configuration and OAuth token to produce an `AlertTriageRequest`.
+RequestBuilderInterface = types.Callable[
+    [dict, Config, str],
+    types.Optional[AlertTriageRequest]]
+
 
 class message(object):
     """The main interface to the alert action.
@@ -218,7 +227,11 @@ def __init__(self):
         logger.info("Performing initial Lambda function discovery")
         self._discover_lambda_fn()
 
-        self.registration = "*"
+        self.registration = [
+            classname.lower()
+            for classname in self._config.enabled_alert_classnames
+        ]
+
         self.priority = 1
 
     def onMessage(self, alert):
@@ -345,32 +358,16 @@ def try_make_outbound(
     """
 
     _source = alert.get("_source", {})
-    category = _source.get("category")
-    tags = _source.get("tags", [])
-
-    is_sensitive_host_access = "session" in tags and category == "session"
-
-#    is_duo_codes_generated = "duosecurity" in tags and\
-#        category == "duo" and\
-#        "codes generated" in _source.get("summary", "")
-#
-#    is_duo_bypass_codes_used = (
-#        "duo_bypass_codes_used" in tags and category == "bypassused"
-#    )
-#
-#    is_ssh_access_releng = "ssh" in tags and category == "access"
-
-    if is_sensitive_host_access:
-        return _make_sensitive_host_access(alert, cfg, oauth_tkn)
-
-#    if is_duo_codes_generated:
-#        return _make_duo_code_gen(alert, cfg, oauth_tkn)
-#
-#    if is_duo_bypass_codes_used:
-#        return _make_duo_code_used(alert, cfg, oauth_tkn)
-#
-#    if is_ssh_access_releng:
-#        return _make_ssh_access_releng(alert, cfg, oauth_tkn)
+
+    alert_class_name = _source.get('classname')
+
+    if alert_class_name is None:
+        return None
+
+    builder = _request_builder(alert_class_name)
+
+    if alert_class_name in cfg.enabled_alert_classnames:
+        return builder(alert, cfg, oauth_tkn)
 
     return None
 
@@ -765,3 +762,22 @@ def _update_duplicate_chain(
         raise APIError(error)
 
     return True
+
+
+def _request_builder(alert_classname: str) -> RequestBuilderInterface:
+    # Note that the alert action will convert classnames to lowercase, so
+    # classnames in a config's `enabled_alert_classnames` must be provided
+    # as they appear below.
+    SUPPORTED_ALERTS = {
+        'AlertGenericLoader:ssh_open_crit': _make_sensitive_host_access,
+        'AlertAuthSignRelengSSH': _make_ssh_access_releng,
+        'AlertGenericLoader:duosecurity_bypass_generated': _make_duo_code_gen,
+        'AlertGenericLoader:duosecurity_bypass_used': _make_duo_code_used,
+    }
+
+    # A `RequestBuilderInterface` can return `None` to indicate that it failed
+    # to process a request, so having this function return `None` directly
+    # actually creates unnecessary error handling.
+    return SUPPORTED_ALERTS.get(
+        alert_classname,
+        lambda _alert, _config, _token: None)

tests/alerts/actions/test_triage_bot.py

@@ -16,6 +16,7 @@ def _ssh_sensitive_host_alert():
             "utctimestamp": "2019-11-04T23:04:36.351726+00:00",
             "severity": "WARNING",
             "summary": "Session opened on sensitive host by (1): tester [test@website.com]",
+            "classname": "AlertGenericLoader:ssh_open_crit",
             "category": "session",
             "tags": ["session", "successful"],
             "events": [
@@ -78,6 +79,7 @@ def _duo_bypass_code_gen_alert():
             "utctimestamp": "2019-11-04T23:36:36.966791+00:00",
             "severity": "NOTICE",
             "summary": "DuoSecurity MFA Bypass codes generated (1): tester@website.com [a.website.com]",
+            "classname": "AlertGenericLoader:duosecurity_bypass_generated",
             "category": "duo",
             "tags": ["duosecurity"],
             "events": [
@@ -146,6 +148,7 @@ def _duo_bypass_code_used_alert():
             "utctimestamp": "2019-10-21T15:55:46.033838+00:00",
             "severity": "NOTICE",
             "summary": "DuoSecurity MFA Bypass codes used to log in (1): tester@website.com [website.com]",
+            "classname": "AlertGenericLoader:duosecurity_bypass_used",
             "category": "bypassused",
             "tags": ["duosecurity", "used", "duo_bypass_codes_used"],
             "events": [
@@ -219,6 +222,7 @@ def _ssh_access_releng_alert():
             "utctimestamp": "2019-11-05T01:14:57.912292+00:00",
             "severity": "NOTICE",
             "summary": "SSH login from 10.49.48.100 on releng.website.com as user tester",
+            "classname": "AlertAuthSignRelengSSH",
             "category": "access",
             "tags": ["ssh"],
             "events": [
@@ -305,14 +309,20 @@ class TestAlertRecognition(object):
     """
 
     mock_config = bot.Config(
+        [
+            "AlertGenericLoader:ssh_open_crit",
+            "AlertAuthSignRelengSSH",
+            "AlertGenericLoader:duosecurity_bypass_generated",
+            "AlertGenericLoader:duosecurity_bypass_used"
+        ],
         "", "", "", "", "", 0, "", "", "", 0, "", "", "", "", "", ""
     )
 
     def test_declines_unrecognized_alert(self):
         msg = _ssh_sensitive_host_alert()
 
         # Without the `session` tag, the alert should not fire.
-        msg["_source"]["tags"] = ["test"]
+        msg["_source"]["classname"] = "test"
 
         result = bot.try_make_outbound(msg, self.mock_config, "")
 
@@ -325,26 +335,26 @@ def test_recognizes_ssh_sensitive_host(self):
 
         assert result is not None
 
-#    def test_recognizes_duo_bypass_codes_generated(self):
-#        msg = _duo_bypass_code_gen_alert()
-#
-#        result = bot.try_make_outbound(msg, self.mock_config, "")
-#
-#        assert result is not None
-#
-#    def test_recognizes_duo_bypass_codes_used(self):
-#        msg = _duo_bypass_code_used_alert()
-#
-#        result = bot.try_make_outbound(msg, self.mock_config, "")
-#
-#        assert result is not None
-#
-#    def test_recognizes_ssh_access_releng(self):
-#        msg = _ssh_access_releng_alert()
-#
-#        result = bot.try_make_outbound(msg, self.mock_config, "")
-#
-#        assert result is not None
+    def test_recognizes_duo_bypass_codes_generated(self):
+        msg = _duo_bypass_code_gen_alert()
+
+        result = bot.try_make_outbound(msg, self.mock_config, "")
+
+        assert result is not None
+
+    def test_recognizes_duo_bypass_codes_used(self):
+        msg = _duo_bypass_code_used_alert()
+
+        result = bot.try_make_outbound(msg, self.mock_config, "")
+
+        assert result is not None
+
+    def test_recognizes_ssh_access_releng(self):
+        msg = _ssh_access_releng_alert()
+
+        result = bot.try_make_outbound(msg, self.mock_config, "")
+
+        assert result is not None
 
 
 class TestPersonAPI: