Check that OpenPGP has only PKESK and SEIPD packets (#323)

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## untagged
 
+- check that OpenPGP has only PKESK and SEIPD packets
+  ([#323](https://github.com/deltachat/chatmail/pull/323))
+
 - improve filtermail checks for encrypted messages and drop support for unencrypted MDNs
   ([#320](https://github.com/deltachat/chatmail/pull/320))
 

chatmaild/src/chatmaild/filtermail.py

@@ -1,7 +1,8 @@
 #!/usr/bin/env python3
 import asyncio
+import base64
+import binascii
 import logging
-import re
 import sys
 import time
 from email import policy
@@ -13,8 +14,89 @@ from aiosmtpd.controller import Controller
 
 from .config import read_config
 
-# Regular expression that matches multi-line base64.
-BASE64_REGEX = re.compile(r"([A-Za-z0-9+/=]+\r\n)+")
+
+def check_openpgp_payload(payload: bytes):
+    """Checks the OpenPGP payload.
+
+    OpenPGP payload must consist only of PKESK packets
+    terminated by a single SEIPD packet.
+
+    Returns True if OpenPGP payload is correct,
+    False otherwise.
+
+    May raise IndexError while trying to read OpenPGP packet header
+    if it is truncated.
+    """
+    i = 0
+    while i < len(payload):
+        # Only OpenPGP format is allowed.
+        if payload[i] & 0xC0 != 0xC0:
+            return False
+
+        packet_type_id = payload[i] & 0x3F
+        i += 1
+        if payload[i] < 192:
+            # One-octet length.
+            body_len = payload[i]
+            i += 1
+        elif payload[i] < 224:
+            # Two-octet length.
+            body_len = ((payload[i] - 192) << 8) + payload[i + 1] + 192
+            i += 2
+        elif payload[i] == 255:
+            # Five-octet length.
+            body_len = (
+                (payload[i + 1] << 24)
+                | (payload[i + 2] << 16)
+                | (payload[i + 3] << 8)
+                | payload[i + 4]
+            )
+            i += 5
+        else:
+            # Partial body length is not allowed.
+            return False
+
+        i += body_len
+
+        if i == len(payload):
+            if packet_type_id == 18:
+                # Last packet should be
+                # Symmetrically Encrypted and Integrity Protected Data Packet (SEIPD)
+                return True
+        elif packet_type_id != 1:
+            # All packets except the last one must be
+            # Public-Key Encrypted Session Key Packet (PKESK)
+            return False
+
+    if i > len(payload):
+        # Payload is truncated.
+        return False
+    return True
+
+
+def check_armored_payload(payload: str):
+    prefix = "-----BEGIN PGP MESSAGE-----\r\n\r\n"
+    if not payload.startswith(prefix):
+        return False
+    payload = payload.removeprefix(prefix)
+
+    suffix = "-----END PGP MESSAGE-----\r\n\r\n"
+    if not payload.endswith(suffix):
+        return False
+    payload = payload.removesuffix(suffix)
+
+    # Remove CRC24.
+    payload = payload.rpartition("=")[0]
+
+    try:
+        payload = base64.b64decode(payload)
+    except binascii.Error:
+        return False
+
+    try:
+        return check_openpgp_payload(payload)
+    except IndexError:
+        return False
 
 
 def check_encrypted(message):
@@ -46,19 +128,7 @@ def check_encrypted(message):
             if part.get_content_type() != "application/octet-stream":
                 return False
 
-            payload = part.get_payload()
-
-            prefix = "-----BEGIN PGP MESSAGE-----\r\n\r\n"
-            if not payload.startswith(prefix):
-                return False
-            payload = payload.removeprefix(prefix)
-
-            suffix = "-----END PGP MESSAGE-----\r\n\r\n"
-            if not payload.endswith(suffix):
-                return False
-            payload = payload.removesuffix(suffix)
-
-            if not BASE64_REGEX.fullmatch(payload):
+            if not check_armored_payload(part.get_payload()):
                 return False
         else:
             return False

chatmaild/src/chatmaild/tests/mail-data/literal.eml

@@ -0,0 +1,44 @@
+From: {from_addr}
+
+To: {to_addr}
+
+Subject: ...
+
+Date: Sun, 15 Oct 2023 16:43:21 +0000
+
+Message-ID: <Mr.UVyJWZmkCKM.hGzNc6glBE_@c2.testrun.org>
+
+In-Reply-To: <Mr.MvmCz-GQbi_.6FGRkhDf05c@c2.testrun.org>
+
+References: <Mr.3gckbNy5bch.uK3Hd2Ws6-w@c2.testrun.org>
+
+	<Mr.MvmCz-GQbi_.6FGRkhDf05c@c2.testrun.org>
+
+Chat-Version: 1.0
+
+Autocrypt: addr={from_addr}; prefer-encrypt=mutual;
+
+	keydata=xjMEZSwWjhYJKwYBBAHaRw8BAQdAQBEhqeJh0GueHB6kF/DUQqYCxARNBVokg/AzT+7LqH
+
+	rNFzxiYXJiYXpAYzIudGVzdHJ1bi5vcmc+wosEEBYIADMCGQEFAmUsFo4CGwMECwkIBwYVCAkKCwID
+
+	FgIBFiEEFTfUNvVnY3b9F7yHnmme1PfUhX8ACgkQnmme1PfUhX9A4AEAnHWHp49eBCMHK5t66gYPiW
+
+	XQuB1mwUjzGfYWB+0RXUoA/0xcQ3FbUNlGKW7Blp6eMFfViv6Mv2d3kNSXACB6nmcMzjgEZSwWjhIK
+
+	KwYBBAGXVQEFAQEHQBpY5L2M1XHo0uxf8SX1wNLBp/OVvidoWHQF2Jz+kJsUAwEIB8J4BBgWCAAgBQ
+
+	JlLBaOAhsMFiEEFTfUNvVnY3b9F7yHnmme1PfUhX8ACgkQnmme1PfUhX/INgEA37AJaNvruYsJVanP
+
+	IXnYw4CKd55UAwl8Zcy+M2diAbkA/0fHHcGV4r78hpbbL1Os52DPOdqYQRauIeJUeG+G6bQO
+
+MIME-Version: 1.0
+
+Content-Type: multipart/encrypted; protocol="application/pgp-encrypted";
+
+	boundary="YFrteb74qSXmggbOxZL9dRnhymywAi"
+
+
+
+
+

chatmaild/src/chatmaild/tests/test_filtermail.py

@@ -2,6 +2,7 @@ import pytest
 from chatmaild.filtermail import (
     BeforeQueueHandler,
     SendRateLimiter,
+    check_armored_payload,
     check_encrypted,
 )
 
@@ -61,6 +62,12 @@ def test_filtermail_encryption_detection(maildata):
     assert not check_encrypted(msg)
 
 
+def test_filtermail_no_literal_packets(maildata):
+    """Test that literal OpenPGP packet is not considered an encrypted mail."""
+    msg = maildata("literal.eml", from_addr="1@example.org", to_addr="2@example.org")
+    assert not check_encrypted(msg)
+
+
 def test_filtermail_unencrypted_mdn(maildata, gencreds):
     """Unencrypted MDNs should not pass."""
     from_addr = gencreds()[0]
@@ -120,3 +127,43 @@ def test_passthrough_senders(gencreds, handler, maildata):
 
     # assert that None/no error is returned
     assert not handler.check_DATA(envelope=env)
+
+
+def test_check_armored_payload():
+    payload = """-----BEGIN PGP MESSAGE-----\r
+\r
+wU4DSqFx0d1yqAoSAQdAYkX/ZN/Az4B0k7X47zKyWrXxlDEdS3WOy0Yf2+GJTFgg\r
+Zk5ql0mLG8Ze+ZifCS0XMO4otlemSyJ0K1ZPdFMGzUDBTgNqzkFabxXoXRIBB0AM\r
+755wlX41X6Ay3KhnwBq7yEqSykVH6F3x11iHPKraLCAGZoaS8bKKNy/zg5slda1X\r
+pt14b4aC1VwtSnYhcRRELNLD/wE2TFif+g7poMmFY50VyMPLYjVP96Z5QCT4+z4H\r
+Ikh/pRRN8S3JNMrRJHc6prooSJmLcx47Y5un7VFy390MsJ+LiUJuQMDdYWRAinfs\r
+Ebm89Ezjm7F03qbFPXE0X4ZNzVXS/eKO0uhJQdiov/vmbn41rNtHmNpqjaO0vi5+\r
+sS9tR7yDUrIXiCUCN78eBLVioxtktsPZm5cDORbQWzv+7nmCEz9/JowCUcBVdCGn\r
+1ofOaH82JCAX/cRx08pLaDNj6iolVBsi56Dd+2bGxJOZOG2AMcEyz0pXY0dOAJCD\r
+iUThcQeGIdRnU3j8UBcnIEsjLu2+C+rrwMZQESMWKnJ0rnqTk0pK5kXScr6F/L0L\r
+UE49ccIexNm3xZvYr5drszr6wz3Tv5fdue87P4etBt90gF/Vzknck+g1LLlkzZkp\r
+d8dI0k2tOSPjUbDPnSy1x+X73WGpPZmj0kWT+RGvq0nH6UkJj3AQTG2qf1T8jK+3\r
+rTp3LR9vDkMwDjX4R8SA9c0wdnUzzr79OYQC9lTnzcx+fM6BBmgQ2GrS33jaFLp7\r
+L6/DFpCl5zhnPjM/2dKvMkw/Kd6XS/vjwsO405FQdjSDiQEEAZA+ZvAfcjdccbbU\r
+yCO+x0QNdeBsufDVnh3xvzuWy4CICdTQT4s1AWRPCzjOj+SGmx5WqCLWfsd8Ma0+\r
+w/C7SfTYu1FDQILLM+llpq1M/9GPley4QZ8JQjo262AyPXsPF/OW48uuZz0Db1xT\r
+Yh4iHBztj4VSdy7l2+IyaIf7cnL4EEBFxv/MwmVDXvDlxyvfAfIsd3D9SvJESzKZ\r
+VWDYwaocgeCN+ojKu1p885lu1EfRbX3fr3YO02K5/c2JYDkc0Py0W3wUP/J1XUax\r
+pbKpzwlkxEgtmzsGqsOfMJqBV3TNDrOA2uBsa+uBqP5MGYLZ49S/4v/bW9I01Cr1\r
+D2ZkV510Y1Vgo66WlP8mRqOTyt/5WRhPD+MxXdk67BNN/PmO6tMlVoJDuk+XwWPR\r
+t2TvNaND/yabT9eYI55Og4fzKD6RIjouUX8DvKLkm+7aXxVs2uuLQ3Jco3O82z55\r
+dbShU1jYsrw9oouXUz06MHPbkdhNbF/2hfhZ2qA31sNeovJw65iUv7sDKX3LVWgJ\r
+10jlywcDwqlU8CO7WC9lGixYTbnOkYZpXCGEl8e6Jbs79l42YFo4ogYpFK1NXFhV\r
+kOXRmDf/wmfj+c/ld3L2PkvwlgofhCudOQknZbo3ub1gjiTn7L+lMGHIj/3suMIl\r
+ID4EUxAXScIM1ZEz2fjtW5jATlqYcLjLTbf/olw6HFyPNH+9IssqXeZNKnGwPUB9\r
+3lTXsg0tpzl+x7F/2WjEw1DSNhjC0KnHt1vEYNMkUGDGFdN9y3ERLqX/FIgiASUb\r
+bTvAVupnAK3raBezGmhrs6LsQtLS9P0VvQiLU3uDhMqw8Z4SISLpcD+NnVBHzQqm\r
+6W5Qn/8xsCL6av18yUVTi2G3igt3QCNoYx9evt2ZcIkNoyyagUVjfZe5GHXh8Dnz\r
+GaBXW/hg3HlXLRGaQu4RYCzBMJILcO25OhZOg6jbkCLiEexQlm2e9krB5cXR49Al\r
+UN4fiB0KR9JyG2ayUdNJVkXZSZLnHyRgiaadlpUo16LVvw==\r
+=b5Kp\r
+-----END PGP MESSAGE-----\r
+\r
+"""
+
+    assert check_armored_payload(payload) == True