Merge pull request #922 from MISP/fix/modat-observation-cardinality

Fix MODAT parsing of multiple PTR and certificate RDN values

Author: adulau

documentation/expansion/modat.json

@@ -30,7 +30,8 @@
     "Preserve array nesting in JSON; repeat scalar leaves in the typed observation. Web files also get individual file objects.",
     "Link observations to input and related infrastructure; deduplicate shared objects and set discovered attributes to_ids=false.",
     "Bound search pagination; report rate limits and authentication errors without exposing response bodies or credentials.",
-    "Use fresh PyMISP UUIDs for generated objects and host attachments across enrichments; deduplicate identical evidence only within the current response."
+    "Use fresh PyMISP UUIDs for generated objects and host attachments across enrichments; deduplicate identical evidence only within the current response.",
+    "Preserve multiple reverse-DNS names in dns-record objects and repeated certificate RDNs in the full issuer/subject strings, respecting single-valued standard object relations."
   ],
   "input": {
     "format": "misp_standard",
@@ -210,7 +211,8 @@
       "misp_modules/data/modat/modat-dns-record/definition.json"
     ],
     "no_results": "Return original attribute and empty Object array.",
-    "partial_failure": "A failed API call returns an error for the enrichment; partial results are not silently returned."
+    "partial_failure": "A failed API call returns an error for the enrichment; partial results are not silently returned.",
+    "errors": "Unexpected input or response parsing failures report the processing stage and exception class. Exception messages, response contents and credentials are not logged or returned."
   },
   "config": {
     "apikey": {
@@ -267,7 +269,7 @@
     "Restart misp-modules, enable modat enrichment in MISP and set apikey. Configure optional result limits, timeout and DNS behavior as needed."
   ],
   "validation": {
-    "offline_tests": 80,
+    "offline_tests": 85,
     "command": "python -m pytest tests/test_modat.py -q",
     "live_authenticated": false
   }

misp_modules/modules/expansion/modat.py

@@ -3,6 +3,7 @@
 import base64
 import ipaddress
 import json
+import logging
 import math
 import re
 from pathlib import Path
@@ -13,6 +14,8 @@
 from pymisp import MISPAttribute, MISPEvent, MISPObject
 from pymisp.exceptions import PyMISPError
 
+LOGGER = logging.getLogger("misp-modules.modat")
+
 BASE_URL = "https://api.magnify.modat.io"
 DATA_PATH = Path(__file__).resolve().parents[2] / "data" / "modat"
 SERVICE_MAPPING = json.loads((DATA_PATH / "service_mapping.json").read_text())
@@ -383,11 +386,22 @@ def host(self, data):
         obj = MISPObject("ip-port", strict=True)
         self.add(obj, "ip", ip)
         self.add(obj, "domain", data.get("fqdns"))
-        self.add(obj, "ptr-record", data.get("rdns"))
+        rdns = data.get("rdns") or []
+        if not isinstance(rdns, list) or not all(isinstance(name, str) for name in rdns):
+            raise ModatError("MODAT returned invalid reverse DNS names.")
+        rdns = list(dict.fromkeys(name for name in rdns if name))
+        # ip-port permits one PTR value; dns-record can preserve the complete answer set.
+        if len(rdns) == 1:
+            self.add(obj, "ptr-record", rdns[0])
         if data.get("is_anycast") is not None:
             self.add(obj, "text", f"MODAT is_anycast={str(data['is_anycast']).lower()}")
         self.add(obj, "text", [f"MODAT classification: {tag}" for tag in data.get("tags", [])])
         host = self.put(obj)
+        if rdns:
+            reverse_dns = MISPObject("dns-record", strict=True)
+            self.add(reverse_dns, "queried-domain", ipaddress.ip_address(ip).reverse_pointer)
+            self.add(reverse_dns, "ptr-record", rdns)
+            self.put(reverse_dns, host.uuid)
         self.infrastructure(data, host.uuid)
         self.vulnerabilities(data.get("cves"), host.uuid)
         # Preserve extensions and all original nested data as evidence, without inventing attribute semantics.
@@ -455,7 +469,11 @@ def service(self, ip, data, parent):
             for side in ("issuer", "subject"):
                 dn = tls.get(side) or {}
                 for field, relation in dn_mapping.items():
-                    self.add(cert, f"{side}-{relation}", dn.get(field))
+                    values = list(dict.fromkeys(dn.get(field) or []))
+                    # Specialized x509 DN relations are single-valued. Repeated RDNs
+                    # are retained in the full DN below and the MODAT observation.
+                    if len(values) == 1:
+                        self.add(cert, f"{side}-{relation}", values[0])
                 if dn:
                     # Render an X.509 distinguished name, retaining repeated RDN values.
                     labels = {
@@ -575,6 +593,7 @@ def handler(q=False):
     if q is False:
         return False
     client = None
+    stage = "module request"
     try:
         request = json.loads(q)
         if not isinstance(request, dict):
@@ -584,6 +603,7 @@ def handler(q=False):
             raise ModatError("Provide an attribute with type, value and uuid.")
         if not isinstance(attribute["type"], str) or not isinstance(attribute["value"], (str, int)):
             raise ModatError("Attribute type and value must be strings (numeric values are also accepted).")
+        stage = "input attribute"
         UUID(str(attribute["uuid"]))
         config = request.get("config") or {}
         if not isinstance(config, dict) or not isinstance(config.get("apikey"), str) or not config["apikey"].strip():
@@ -597,37 +617,48 @@ def handler(q=False):
         if query and len(query) > 2000:
             raise ModatError("Query exceeds the API's 2000-character limit.")
         parser = ModatParser(attribute)
+        stage = "MODAT client configuration"
         client = ModatClient(config["apikey"].strip(), timeout)
         if ip:
+            stage = "MODAT host lookup"
             data = client.request(f"/host/{quote(ip, safe='')}/v1")
             if data is not None:
                 if not isinstance(data.get("data"), dict):
                     raise ModatError("MODAT returned invalid host details.")
+                stage = "MODAT host data"
                 parser.host(data["data"])
         else:
+            stage = "MODAT service search"
             for row in client.search("/service/search/v1", query, limit, page_size):
                 if not isinstance(row.get("service"), dict):
                     raise ModatError("MODAT returned an invalid service search record.")
+                stage = "MODAT service data"
                 parser.host(row)
         zones = [zone] if include_dns and zone else []
         if include_dns and reverse_dns and ip:
+            stage = "MODAT reverse DNS search"
             rrtype = "a" if ipaddress.ip_address(ip).version == 4 else "aaaa"
             for row in client.search("/dns/search/v1", _term((f"{rrtype}.ip",), ip), limit, min(page_size, 100)):
                 if not isinstance(row.get("fqdn"), str):
                     raise ModatError("MODAT returned an invalid DNS search record.")
                 zones.append(_domain(row["fqdn"]))
         for fqdn in dict.fromkeys(zones):
+            stage = "MODAT DNS zone lookup"
             data = client.request(f"/dns/zones/{quote(fqdn, safe='')}/v1")
             if data is not None:
+                stage = "MODAT DNS zone data"
                 parser.dns(data)
+        stage = "MISP enrichment output"
         return parser.result()
     except ModatError as exc:
         return {"error": str(exc)}
     except requests.RequestException:
         # Exceptions may contain request headers or API response bodies; do not echo them.
         return {"error": "Unable to reach MODAT (network error or timeout)."}
-    except (ValueError, TypeError, KeyError, AttributeError, PyMISPError):
-        return {"error": "Invalid request, attribute, or MODAT response data."}
+    except (ValueError, TypeError, KeyError, AttributeError, PyMISPError) as exc:
+        # Only identify the stage and exception class: exception messages may contain response data.
+        LOGGER.warning("MODAT enrichment failed during %s (%s).", stage, type(exc).__name__)
+        return {"error": f"Unable to process {stage} ({type(exc).__name__})."}
     finally:
         if client is not None:
             client.session.close()

tests/test_modat.py

@@ -461,3 +461,113 @@ def test_duplicate_host_evidence_is_reused_within_response():
     for obj in second["Object"]:
         refs = [(r["referenced_uuid"], r["relationship_type"]) for r in obj.get("ObjectReference", [])]
         assert len(refs) == len(set(refs))
+
+
+def test_multiple_reverse_dns_names_use_dns_record():
+    host = {"ip": "203.0.113.14", "rdns": ["one.example.com", "two.example.com"], "services": []}
+    with patch.object(modat.requests.Session, "request", return_value=response({"data": host})):
+        result = modat.handler(request("ip-src", host["ip"], include_dns=False))
+    assert "error" not in result
+    objects = result["results"]["Object"]
+    dns = next(o for o in objects if o["name"] == "dns-record")
+    assert {a["value"] for a in dns["Attribute"] if a["object_relation"] == "ptr-record"} == set(host["rdns"])
+    assert (
+        next(a["value"] for a in dns["Attribute"] if a["object_relation"] == "queried-domain")
+        == "14.113.0.203.in-addr.arpa"
+    )
+    endpoint = next(o for o in objects if o["name"] == "ip-port")
+    assert len([a for a in endpoint["Attribute"] if a["object_relation"] == "ptr-record"]) <= 1
+
+
+def test_certificate_multiple_distinguished_name_values_are_preserved():
+    observed = service()
+    observed["tls"]["issuer"] = {"common_name": ["Issuer One", "Issuer Two"], "organization": ["Org One", "Org Two"]}
+    observed["tls"]["subject"] = {"common_name": ["one.example.com", "two.example.com"], "country": ["LU", "BE"]}
+    host = {"ip": "203.0.113.14", "services": [observed]}
+    with patch.object(modat.requests.Session, "request", return_value=response({"data": host})):
+        result = modat.handler(request("ip-src", host["ip"], include_dns=False))
+    assert "error" not in result
+    cert = next(o for o in result["results"]["Object"] if o["name"] == "x509")
+    values = {a["object_relation"]: a["value"] for a in cert["Attribute"]}
+    assert "CN=Issuer One,CN=Issuer Two" in values["issuer"]
+    assert "O=Org One,O=Org Two" in values["issuer"]
+    assert "CN=one.example.com,CN=two.example.com" in values["subject"]
+    assert "C=LU,C=BE" in values["subject"]
+    for rel in ("issuer-common-name", "issuer-organization", "subject-common-name", "subject-country"):
+        assert len([a for a in cert["Attribute"] if a["object_relation"] == rel]) <= 1
+
+
+def test_full_misp_request_with_reverse_dns_and_multiple_rdn_values():
+    source = {
+        **attribute("ip-src"),
+        "id": "1",
+        "event_id": "2",
+        "category": "Network activity",
+        "value1": "203.0.113.14",
+        "value2": "",
+        "to_ids": False,
+        "timestamp": "1791885325",
+        "distribution": "0",
+        "comment": "",
+        "sharing_group_id": "0",
+        "deleted": False,
+        "disable_correlation": False,
+        "object_id": "0",
+        "object_relation": None,
+        "first_seen": None,
+        "last_seen": None,
+    }
+    observed = service()
+    observed["tls"]["subject"]["common_name"] = ["one.example.com", "two.example.com"]
+    host = {"ip": source["value"], "rdns": ["one.example.com", "two.example.com"], "services": [observed]}
+    zone = {
+        "fqdn": "example.com",
+        "records": {"a": [{"ip": source["value"], "ttl": 300, "created_at": "2026-01-01T00:00:00Z"}]},
+    }
+    q = json.dumps(
+        {
+            "module": "modat",
+            "attribute": source,
+            "event_id": "2",
+            "timeout": 10,
+            "config": {
+                "apikey": "test-secret",
+                "max_results": "1",
+                "page_size": "10",
+                "timeout": "10",
+                "include_dns": "1",
+                "reverse_dns": "1",
+            },
+        }
+    )
+    with patch.object(
+        modat.requests.Session,
+        "request",
+        side_effect=[
+            response({"data": host}),
+            response({"page": [{"fqdn": "example.com"}], "total_pages": 1}),
+            response(zone),
+        ],
+    ) as call:
+        result = modat.handler(q)
+    assert "error" not in result
+    assert call.call_count == 3
+    assert {"modat-service", "x509", "dns-record", "modat-dns-record", "domain-ip"} <= {
+        o["name"] for o in result["results"]["Object"]
+    }
+    assert result["results"]["Attribute"][0]["uuid"] == source["uuid"]
+
+
+@pytest.mark.parametrize("method,stage", [("host", "MODAT host data"), ("result", "MISP enrichment output")])
+def test_parser_errors_identify_stage_without_leaking_data(method, stage, caplog):
+    with (
+        patch.object(
+            modat.requests.Session, "request", return_value=response({"data": {"ip": "203.0.113.14", "services": []}})
+        ),
+        patch.object(modat.ModatParser, method, side_effect=TypeError("test-secret private response")),
+    ):
+        result = modat.handler(request(include_dns=False))
+    assert result["error"] == f"Unable to process {stage} (TypeError)."
+    assert stage in caplog.text
+    assert "test-secret" not in result["error"] + caplog.text
+    assert "private response" not in result["error"] + caplog.text