diff --git a/CHANGELOG.md b/CHANGELOG.md index af9b7a8..3bb47ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Bug fixes - **Failure-report timestamps are no longer skewed by the host's UTC offset in the Elasticsearch, OpenSearch, and Splunk HEC outputs** ([#811](https://github.com/domainaware/parsedmarc/issues/811), bug 1). `arrival_date_utc` is a UTC wall-clock string, but the three sinks parsed it into a naive `datetime` and called `.timestamp()`, which per the Python docs interprets naive values as *local* time — so on any non-UTC host, the epoch stored as the ES/OpenSearch `arrival_date` field, used in the failure-report dedup query, and sent as the Splunk HEC event `time` was off by the host's UTC offset (1–2 h for most of Europe). `human_timestamp_to_datetime()` / `human_timestamp_to_unix_timestamp()` gained an `assume_utc` keyword that attaches `timezone.utc` to naive parses, and the `arrival_date_utc` consumers now use it. +- **A system GeoIP file no longer silently overrides the bundled IPinfo database** ([#810](https://github.com/domainaware/parsedmarc/issues/810)). The IP database path resolution searched standard system paths (e.g. `/usr/share/GeoIP/GeoLite2-Country.mmdb`, plus CWD-relative names) *before* the database managed by parsedmarc, so on any host with a distro GeoIP package installed — often as a stale dependency of an unrelated package — every lookup used a country-only (and possibly years-old) database instead of the bundled IPinfo Lite database. That silently disabled ASN enrichment (`asn`, `as_name`, `as_domain` were `None` for every IP) and with it the ASN-fallback path into the reverse-DNS map. The precedence is now: explicit `ip_db_path` → the database selected by `load_ip_db()` (downloaded/cached/bundled) → the bundled copy → system paths as a true last resort. The selected database file is logged at debug level. **If you deliberately relied on the automatic system-path pickup to use MaxMind GeoLite2, set `ip_db_path` explicitly** — see the "Using MaxMind GeoLite2" section in the installation docs. ## 10.2.0 diff --git a/docs/source/installation.md b/docs/source/installation.md index b648ffd..7b5dc04 100644 --- a/docs/source/installation.md +++ b/docs/source/installation.md @@ -119,12 +119,28 @@ This installs the `msgconvert` script to `/usr/local/bin/msgconvert`. ## Using MaxMind GeoLite2 (optional) -`parsedmarc` will pick up the [MaxMind GeoLite2 Country database] if -it is installed at one of the standard system paths (e.g. -`/usr/share/GeoIP/GeoLite2-Country.mmdb`, -`/var/lib/GeoIP/GeoLite2-Country.mmdb`, or the equivalent location on -Windows). **Use this only if you specifically prefer MaxMind data over -the bundled IPinfo Lite database — most users do not need it.** +To use the [MaxMind GeoLite2 Country database] instead of the bundled +IPinfo Lite database, point the `ip_db_path` option at it explicitly: + +```ini +[general] +ip_db_path = /usr/share/GeoIP/GeoLite2-Country.mmdb +``` + +**Use this only if you specifically prefer MaxMind data over the +bundled IPinfo Lite database — most users do not need it.** Country +databases like GeoLite2 carry no ASN data, so source attribution for +IP addresses without reverse DNS is reduced when one is used. + +:::{note} +parsedmarc no longer picks up a GeoLite2/DBIP database from standard +system paths (e.g. `/usr/share/GeoIP/GeoLite2-Country.mmdb`) +automatically — a GeoIP file installed by an unrelated distro package +would silently override the bundled database and disable ASN +enrichment. System paths are now only consulted as a last resort when +the bundled database is missing. If you previously relied on the +automatic pickup, set `ip_db_path` as shown above. +::: Install [geoipupdate] for your platform: diff --git a/parsedmarc/utils.py b/parsedmarc/utils.py index 1fd2946..2255187 100644 --- a/parsedmarc/utils.py +++ b/parsedmarc/utils.py @@ -408,6 +408,10 @@ def human_timestamp_to_unix_timestamp( _IP_DB_PATH: str | None = None +# The last database path logged by _get_ip_database_path(), so the +# selection is logged when it changes rather than on every lookup. +_LAST_LOGGED_IP_DB_PATH: str | None = None + def load_ip_db( *, @@ -623,6 +627,12 @@ def _normalize_ip_record(record: dict) -> _IPDatabaseRecord: def _get_ip_database_path(db_path: str | None) -> str: + # Last-resort fallbacks for unusual installs where the bundled database + # is missing. Country-only databases (GeoLite2 / DBIP) lack the ASN + # fields source attribution depends on, so an incidental system GeoIP + # file must never shadow the parsedmarc-managed database + # (https://github.com/domainaware/parsedmarc/issues/810). To use + # MaxMind or DBIP data deliberately, set the ip_db_path option. db_paths = [ "ipinfo_lite.mmdb", "GeoLite2-Country.mmdb", @@ -646,19 +656,35 @@ def _get_ip_database_path(db_path: str | None) -> str: ) db_path = None - if db_path is None: - for system_path in db_paths: - if os.path.exists(system_path): - db_path = system_path - break + # The database parsedmarc manages takes precedence: the one selected by + # load_ip_db() (downloaded, cached, or bundled), or the bundled copy + # directly for library callers that never call load_ip_db(). + if db_path is None and _IP_DB_PATH is not None: + db_path = _IP_DB_PATH if db_path is None: - if _IP_DB_PATH is not None: - db_path = _IP_DB_PATH + bundled_path = str( + files(parsedmarc.resources.ipinfo).joinpath("ipinfo_lite.mmdb") + ) + if os.path.isfile(bundled_path): + db_path = bundled_path else: - db_path = str( - files(parsedmarc.resources.ipinfo).joinpath("ipinfo_lite.mmdb") - ) + for system_path in db_paths: + if os.path.exists(system_path): + db_path = system_path + break + else: + # Nothing found anywhere; use the bundled path so the + # os.stat() below raises a FileNotFoundError naming the + # expected install location. + db_path = bundled_path + + global _LAST_LOGGED_IP_DB_PATH + if db_path != _LAST_LOGGED_IP_DB_PATH: + # Log per selected path, not per lookup — this function runs on + # every uncached IP lookup and would flood --debug output. + logger.debug(f"Using IP database at {db_path}") + _LAST_LOGGED_IP_DB_PATH = db_path db_age = datetime.now() - datetime.fromtimestamp(os.stat(db_path).st_mtime) if db_age > timedelta(days=30): diff --git a/tests/test_utils.py b/tests/test_utils.py index 453f4bc..4bcc6c8 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,10 +1,12 @@ """Tests for parsedmarc.utils""" import os +import shutil import tempfile import time import unittest from datetime import datetime, timezone +from importlib.resources import files from tempfile import NamedTemporaryFile from unittest.mock import MagicMock, patch @@ -13,6 +15,7 @@ import requests from expiringdict import ExpiringDict import parsedmarc +import parsedmarc.resources.ipinfo import parsedmarc.utils from tests.tzutil import force_tz @@ -607,6 +610,23 @@ class TestTimestampAssumeUtc(unittest.TestCase): class TestUtilsIpDbPaths(unittest.TestCase): """Tests for IP database path validation""" + def setUp(self): + # These tests exercise the db-path fallback chain, which reads the + # module-level _IP_DB_PATH set by load_ip_db(); pin it to a known + # state and restore afterwards. The log-dedup marker is reset so + # the "Using IP database at ..." selection log fires regardless of + # which test (or prior lookup) ran first. + old_ip_db_path = parsedmarc.utils._IP_DB_PATH + old_logged_path = parsedmarc.utils._LAST_LOGGED_IP_DB_PATH + parsedmarc.utils._IP_DB_PATH = None + parsedmarc.utils._LAST_LOGGED_IP_DB_PATH = None + + def restore(): + parsedmarc.utils._IP_DB_PATH = old_ip_db_path + parsedmarc.utils._LAST_LOGGED_IP_DB_PATH = old_logged_path + + self.addCleanup(restore) + def testCustomPathFallsBack(self): """Non-existent custom db path falls back to default""" result = parsedmarc.utils.get_ip_address_country( @@ -619,6 +639,135 @@ class TestUtilsIpDbPaths(unittest.TestCase): result = parsedmarc.utils.get_ip_address_country("8.8.8.8") self.assertEqual(result, "US") + def testSystemGeoIpFileDoesNotShadowBundledDb(self): + """A country-only system GeoIP file must not shadow the bundled + IPinfo database — shadowing silently disables ASN enrichment + because GeoLite2/DBIP country databases carry no ASN fields. + Regression test for + https://github.com/domainaware/parsedmarc/issues/810. + + The fallback list includes CWD-relative names, so a decoy + ``GeoLite2-Country.mmdb`` in the working directory reproduces the + system-file shadowing on any machine, including CI runners with no + /usr/share/GeoIP. On the unfixed code the decoy won the path + search before the bundled database was ever considered.""" + old_cwd = os.getcwd() + tmp_dir = tempfile.mkdtemp() + self.addCleanup(lambda: (os.chdir(old_cwd), shutil.rmtree(tmp_dir))) + with open(os.path.join(tmp_dir, "GeoLite2-Country.mmdb"), "wb"): + pass + os.chdir(tmp_dir) + + record = parsedmarc.utils.get_ip_address_db_record("8.8.8.8") + self.assertEqual(record["asn"], 15169) + self.assertEqual(record["as_name"], "Google LLC") + self.assertEqual(record["as_domain"], "google.com") + + def testLoadedDbPathTakesPrecedenceOverSystemFiles(self): + """The database selected by load_ip_db() (_IP_DB_PATH) wins over + any system GeoIP file. Uses a copy of the bundled database at a + distinct path and verifies via the selection debug log that the + copy — not a CWD decoy — is the file actually opened.""" + tmp_dir = tempfile.mkdtemp() + old_cwd = os.getcwd() + self.addCleanup(lambda: (os.chdir(old_cwd), shutil.rmtree(tmp_dir))) + with open(os.path.join(tmp_dir, "GeoLite2-Country.mmdb"), "wb"): + pass + + bundled = str(files(parsedmarc.resources.ipinfo).joinpath("ipinfo_lite.mmdb")) + loaded_copy = os.path.join(tmp_dir, "loaded.mmdb") + shutil.copyfile(bundled, loaded_copy) + parsedmarc.utils._IP_DB_PATH = loaded_copy + os.chdir(tmp_dir) + + with self.assertLogs("parsedmarc.log", level="DEBUG") as cm: + record = parsedmarc.utils.get_ip_address_db_record("8.8.8.8") + self.assertEqual(record["asn"], 15169) + self.assertTrue( + any( + f"Using IP database at {loaded_copy}" in message + for message in cm.output + ) + ) + + def testDbSelectionIsLoggedOncePerPath(self): + """The "Using IP database at ..." debug log fires when the + selected path changes, not on every lookup, so --debug runs over + large report batches aren't flooded with one line per IP.""" + with self.assertLogs("parsedmarc.log", level="DEBUG") as cm: + parsedmarc.utils.get_ip_address_db_record("8.8.8.8") + parsedmarc.utils.get_ip_address_db_record("1.1.1.1") + selection_logs = [m for m in cm.output if "Using IP database at" in m] + self.assertEqual(len(selection_logs), 1) + + def testSystemPathUsedWhenBundledDbMissing(self): + """When the bundled database file is missing (tier 4 of the + precedence chain), a system/CWD GeoIP path is consulted as a last + resort. The bundled resource can't be deleted from an installed + package, so ``parsedmarc.utils.files`` is patched to point at a + nonexistent path; the assertion is on observable behavior — the + record comes from the decoy file, per the selection log.""" + tmp_dir = tempfile.mkdtemp() + old_cwd = os.getcwd() + self.addCleanup(lambda: (os.chdir(old_cwd), shutil.rmtree(tmp_dir))) + + bundled = str(files(parsedmarc.resources.ipinfo).joinpath("ipinfo_lite.mmdb")) + decoy = os.path.join(tmp_dir, "GeoLite2-Country.mmdb") + shutil.copyfile(bundled, decoy) + os.chdir(tmp_dir) + + missing = os.path.join(tmp_dir, "does-not-exist.mmdb") + with patch("parsedmarc.utils.files") as mock_files: + mock_files.return_value.joinpath.return_value = missing + with self.assertLogs("parsedmarc.log", level="DEBUG") as cm: + record = parsedmarc.utils.get_ip_address_db_record("8.8.8.8") + self.assertEqual(record["country"], "US") + self.assertTrue( + any( + "Using IP database at GeoLite2-Country.mmdb" in message + for message in cm.output + ) + ) + + def testMissingEverythingRaisesFileNotFoundError(self): + """When neither the bundled database nor any system path exists, + the error names the expected bundled install location.""" + tmp_dir = tempfile.mkdtemp() + old_cwd = os.getcwd() + self.addCleanup(lambda: (os.chdir(old_cwd), shutil.rmtree(tmp_dir))) + os.chdir(tmp_dir) + + missing = os.path.join(tmp_dir, "does-not-exist.mmdb") + with patch("parsedmarc.utils.files") as mock_files: + mock_files.return_value.joinpath.return_value = missing + with self.assertRaises(FileNotFoundError) as ctx: + parsedmarc.utils.get_ip_address_db_record("8.8.8.8") + self.assertIn(missing, str(ctx.exception)) + + def testOldDatabaseFileWarns(self): + """A database file older than 30 days triggers the staleness + warning.""" + tmp_dir = tempfile.mkdtemp() + self.addCleanup(lambda: shutil.rmtree(tmp_dir)) + + bundled = str(files(parsedmarc.resources.ipinfo).joinpath("ipinfo_lite.mmdb")) + old_copy = os.path.join(tmp_dir, "old.mmdb") + shutil.copyfile(bundled, old_copy) + forty_days_ago = time.time() - 40 * 24 * 3600 + os.utime(old_copy, (forty_days_ago, forty_days_ago)) + + with self.assertLogs("parsedmarc.log", level="WARNING") as cm: + record = parsedmarc.utils.get_ip_address_db_record( + "8.8.8.8", db_path=old_copy + ) + self.assertEqual(record["asn"], 15169) + self.assertTrue( + any( + "IP database is more than a month old" in message + for message in cm.output + ) + ) + class TestUtilsParseEmail(unittest.TestCase): """Tests for parse_email edge cases"""