Cover missing/mismatched/rotated DKIM key safety (0.7 matrix MAILAUTH-06)
Adds three cases exercising GenerateDKIMSignature()/ReadPrivateKey()
(src/libs/mailauth/dkim.c) key-failure handling on the outbound path:
- missing: the key file is unlinked before submission.
- mismatched: the key file is overwritten with content that isn't a
valid PEM private key at all.
- rotated: the key file is replaced with a genuinely different valid
keypair; delivery must succeed immediately (no manager restart)
and the signature must independently verify against the *new*
key, proving GenerateDKIMSignature() reads the key fresh from disk
per message rather than caching it.
For missing/mismatched, PrepareExternalMessage() (smtpc.c) requires
either a real signature or SRS rewriting before it will even open the
outbound connection; this authenticated submission qualifies for
neither, so the message must retain in Queue rather than leak out
unsigned, then recover and deliver normally once the key is restored.
Investigated but deliberately left out the matrix row's fourth case,
"unreadable" (a chmod(0o000) key file): confirmed via strace that
openat() on the key path still succeeds, because this whole instance
runs as netns-fixture.py's "fake root" (`unshare --user
--map-root-user`) -- a real Linux capability (CAP_DAC_OVERRIDE)
granted for any file the same real UID owns, not a Bongo behavior this
suite can turn off. The "missing" case already exercises the identical
fopen()-failure code path in ReadPrivateKey() regardless of whether
the underlying errno would be ENOENT or EACCES in a real deployment,
so C-level coverage of that failure mode isn't actually lost.
Verified 2x pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -83,7 +83,7 @@ from email.policy import SMTP as SMTP_POLICY
|
||||
from pathlib import Path
|
||||
|
||||
from cryptography.hazmat.primitives import hashes, serialization
|
||||
from cryptography.hazmat.primitives.asymmetric import padding
|
||||
from cryptography.hazmat.primitives.asymmetric import padding, rsa
|
||||
|
||||
|
||||
HOST = os.environ.get("BONGO_TEST_HOST", "127.0.0.1")
|
||||
@@ -558,6 +558,80 @@ def verify_dkim_signature_independently(mode: str, eml_path: Path) -> None:
|
||||
) from error
|
||||
|
||||
|
||||
def check_dkim_key_safety(mode: str, marker_suffix: str, break_fn) -> None:
|
||||
"""MAILAUTH-06: a missing/mismatched DKIM key must never cause
|
||||
outbound mail that requires signing to leak out unsigned --
|
||||
PrepareExternalMessage() (smtpc.c) requires either a real signature
|
||||
or SRS rewriting before it will even open the outbound connection
|
||||
(see this file's own module docstring, and dane-mta-sts-check.py's
|
||||
precedent for the same anti-spoofing gate); this authenticated
|
||||
submission qualifies for neither, so a broken key must retain the
|
||||
message in Queue, never deliver it unsigned. Once the key is
|
||||
restored, the same retained message must still recover and deliver
|
||||
normally -- broken safely, not broken permanently.
|
||||
|
||||
Deliberately does not cover the matrix row's "unreadable" case: a
|
||||
chmod(0o000) key file, verified by strace (openat() on the key path
|
||||
still succeeded, fd returned) still reads fine here, because this
|
||||
whole instance runs as netns-fixture.py's "fake root" (`unshare
|
||||
--user --map-root-user`, see that file's own docstring) -- a real
|
||||
Linux capability, CAP_DAC_OVERRIDE, granted for any file the same
|
||||
real UID owns, not a Bongo behavior this suite can turn off. The
|
||||
"missing" case below exercises the identical fopen()-failure code
|
||||
path in ReadPrivateKey() (src/libs/mailauth/dkim.c) regardless of
|
||||
whether the underlying errno was ENOENT or EACCES, so C-level
|
||||
coverage of that failure mode is not actually lost.
|
||||
"""
|
||||
original_bytes = DKIM_KEY_PATH.read_bytes()
|
||||
original_mode = DKIM_KEY_PATH.stat().st_mode & 0o777
|
||||
marker = f"{TOKEN}-keysafety-{marker_suffix}"
|
||||
queue_id: str | None = None
|
||||
try:
|
||||
break_fn()
|
||||
submit(marker)
|
||||
queue_id = wait_for_queued(marker)
|
||||
time.sleep(1)
|
||||
assert_not_captured(mode, marker)
|
||||
finally:
|
||||
if not DKIM_KEY_PATH.exists():
|
||||
DKIM_KEY_PATH.write_bytes(original_bytes)
|
||||
DKIM_KEY_PATH.chmod(original_mode)
|
||||
elif DKIM_KEY_PATH.read_bytes() != original_bytes:
|
||||
DKIM_KEY_PATH.write_bytes(original_bytes)
|
||||
DKIM_KEY_PATH.chmod(original_mode)
|
||||
retry_when_idle(queue_id)
|
||||
captured = wait_for_capture(mode, marker)
|
||||
assert_dkim_signature(mode, captured)
|
||||
verify_dkim_signature_independently(mode, captured)
|
||||
wait_for_queue_removal(queue_id)
|
||||
|
||||
|
||||
def check_dkim_key_rotation(mode: str) -> None:
|
||||
"""MAILAUTH-06: a rotated key must be picked up immediately, with no
|
||||
manager restart -- GenerateDKIMSignature() (smtpc.c) reads the key
|
||||
file fresh from disk for every message, never caching it. Delivers
|
||||
successfully (unlike the broken-key cases above) and the signature
|
||||
must independently verify against the *new* key, proving Bongo
|
||||
actually used it rather than some cached copy of the old one.
|
||||
"""
|
||||
marker = f"{TOKEN}-keysafety-rotated"
|
||||
original_bytes = DKIM_KEY_PATH.read_bytes()
|
||||
new_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
|
||||
new_pem = new_key.private_bytes(
|
||||
encoding=serialization.Encoding.PEM,
|
||||
format=serialization.PrivateFormat.TraditionalOpenSSL,
|
||||
encryption_algorithm=serialization.NoEncryption(),
|
||||
)
|
||||
try:
|
||||
DKIM_KEY_PATH.write_bytes(new_pem)
|
||||
submit(marker)
|
||||
captured = wait_for_capture(mode, marker)
|
||||
assert_dkim_signature(mode, captured)
|
||||
verify_dkim_signature_independently(mode, captured)
|
||||
finally:
|
||||
DKIM_KEY_PATH.write_bytes(original_bytes)
|
||||
|
||||
|
||||
def require_safe_environment() -> None:
|
||||
if os.environ.get("BONGO_ALLOW_LIVE_USER_TEST") != "1":
|
||||
raise SMTP09Error(
|
||||
@@ -657,6 +731,16 @@ def main() -> int:
|
||||
captured = wait_for_capture("smtps", recovery_marker)
|
||||
assert_dkim_signature("smtps", captured)
|
||||
wait_for_queue_removal(queue_id)
|
||||
|
||||
# MAILAUTH-06: missing/unreadable/mismatched/rotated DKIM key
|
||||
# safety ("unreadable" not independently coverable here -- see
|
||||
# check_dkim_key_safety()'s own docstring for why).
|
||||
check_dkim_key_safety("smtps", "missing", DKIM_KEY_PATH.unlink)
|
||||
check_dkim_key_safety(
|
||||
"smtps", "mismatched",
|
||||
lambda: DKIM_KEY_PATH.write_bytes(b"not a valid PEM private key\n"),
|
||||
)
|
||||
check_dkim_key_rotation("smtps")
|
||||
finally:
|
||||
if unavailable_socket is not None:
|
||||
unavailable_socket.close()
|
||||
@@ -679,7 +763,10 @@ def main() -> int:
|
||||
"unreachable=retained/automatic-backoff/recovered "
|
||||
"starttls=no-auth implicit-tls=auth "
|
||||
"auth-failure=retained/explicit-retry/recovered "
|
||||
"dkim=verified+independently-verified queue-clean=yes config-restored=yes"
|
||||
"dkim=verified+independently-verified "
|
||||
"dkim-key-safety=missing/mismatched=retained,"
|
||||
"rotated=immediate-no-restart "
|
||||
"queue-clean=yes config-restored=yes"
|
||||
)
|
||||
return 0
|
||||
|
||||
|
||||
Reference in New Issue
Block a user