Skip to content

Commit

Permalink
db/utils: fix line ending in PGP MIME validation
Browse files Browse the repository at this point in the history 
RFC 3156 uses CRLF as the canonical line ending sequence in the signed
content whereas the local eml store often uses UNIX LF.
  • Loading branch information
@pacien @pazz
pacien authored and pazz committed May 7, 2020
1 parent 3f74ed7 commit dff3ae0
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 78 deletions.
22 changes: 16 additions & 6 deletions alot/db/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,20 @@ def _handle_signatures(original_bytes, original, message, params):
if not mic_alg.startswith('pgp-'):
raise MessageError(f'expected micalg=pgp-..., got: {mic_alg}')

# manual part extraction from original message necessary because
# parsing and re-serialising a Message isn't byte-perfect,
# which interferes with signature validation.
# See RFC 1847 section 2.1.
# RFC 3156 section 5 says that "signed message and transmitted message
# MUST be identical". We therefore need to validate against the message
# as it was originally sent.

# The transmitted content and therefore the signed content are using
# CRLF as line delimiter, but our eml file has most likely been
# converted to UNIX LF line ending in the local storage.
if b'\r\n' not in original_bytes:
original_bytes = original_bytes.replace(b'\n', b'\r\n')

# The sender's signed canonical form often differs from the one
# produced by Python's standard lib (in the number of blank lines
# between multipart segments...). We therefore need to extract the
# signed part directly from the original byte string.
signed_boundary = b'\r\n--' + message.get_boundary().encode()
original_chunks = original_bytes.split(signed_boundary)
nb_chunks = len(original_chunks)
Expand All @@ -147,11 +157,11 @@ def _handle_signatures(original_bytes, original, message, params):
f'unexpected number of multipart chunks, got {nb_chunks}')

signed_chunk = original_chunks[1]
if len(signed_chunk) < 2:
if len(signed_chunk) < len(b'\r\n'):
raise MessageError('signed chunk has an invalid length')

sigs = crypto.verify_detached(
signed_chunk[len('\r\n'):],
signed_chunk[len(b'\r\n'):],
signature_part.get_payload(decode=True))

add_signature_headers(original, sigs, None)
Expand Down
51 changes: 17 additions & 34 deletions tests/db/test_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,40 +425,35 @@ def _make_signed(self):
def test_signed_headers_included(self):
"""Headers are added to the message."""
m = self._make_signed()
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn(utils.X_SIGNATURE_VALID_HEADER, m)
self.assertIn(utils.X_SIGNATURE_MESSAGE_HEADER, m)

def test_signed_valid(self):
"""Test that the signature is valid."""
m = self._make_signed()
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertEqual(m[utils.X_SIGNATURE_VALID_HEADER], 'True')

def test_signed_correct_from(self):
"""Test that the signature is valid."""
m = self._make_signed()
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
# Don't test for valid/invalid since that might change
self.assertIn(
'ambig <ambig@example.com>', m[utils.X_SIGNATURE_MESSAGE_HEADER])

def test_signed_wrong_mimetype_second_payload(self):
m = self._make_signed()
m.get_payload(1).set_type('text/plain')
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn('expected Content-Type: ',
m[utils.X_SIGNATURE_MESSAGE_HEADER])

def test_signed_wrong_micalg(self):
m = self._make_signed()
m.set_param('micalg', 'foo')
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn('expected micalg=pgp-...',
m[utils.X_SIGNATURE_MESSAGE_HEADER])

Expand All @@ -480,8 +475,7 @@ def test_signed_micalg_cap(self):
"""
m = self._make_signed()
m.set_param('micalg', 'PGP-SHA1')
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn('expected micalg=pgp-',
m[utils.X_SIGNATURE_MESSAGE_HEADER])

Expand All @@ -496,8 +490,7 @@ def test_signed_more_than_two_messages(self):
"""
m = self._make_signed()
m.attach(email.mime.text.MIMEText('foo'))
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn('expected exactly two messages, got 3',
m[utils.X_SIGNATURE_MESSAGE_HEADER])

Expand Down Expand Up @@ -528,14 +521,12 @@ def test_encrypted_length(self):
# of the mail, rather than replacing the whole encrypted payload with
# it's unencrypted equivalent
m = self._make_encrypted()
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertEqual(len(m.get_payload()), 3)

def test_encrypted_unsigned_is_decrypted(self):
m = self._make_encrypted()
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
# Check using m.walk, since we're not checking for ordering, just
# existence.
self.assertIn('This is some text', [n.get_payload() for n in m.walk()])
Expand All @@ -545,24 +536,21 @@ def test_encrypted_unsigned_doesnt_add_signed_headers(self):
that there is a signature.
"""
m = self._make_encrypted()
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertNotIn(utils.X_SIGNATURE_VALID_HEADER, m)
self.assertNotIn(utils.X_SIGNATURE_MESSAGE_HEADER, m)

def test_encrypted_signed_is_decrypted(self):
m = self._make_encrypted(True)
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn('This is some text', [n.get_payload() for n in m.walk()])

def test_encrypted_signed_headers(self):
"""Since the message is signed, it should have headers saying that
there is a signature.
"""
m = self._make_encrypted(True)
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn(utils.X_SIGNATURE_MESSAGE_HEADER, m)
self.assertIn(
'ambig <ambig@example.com>', m[utils.X_SIGNATURE_MESSAGE_HEADER])
Expand All @@ -572,16 +560,14 @@ def test_encrypted_signed_headers(self):
def test_encrypted_wrong_mimetype_first_payload(self):
m = self._make_encrypted()
m.get_payload(0).set_type('text/plain')
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn('Malformed OpenPGP message:',
m.get_payload(2).get_payload())

def test_encrypted_wrong_mimetype_second_payload(self):
m = self._make_encrypted()
m.get_payload(1).set_type('text/plain')
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn('Malformed OpenPGP message:',
m.get_payload(2).get_payload())

Expand All @@ -591,8 +577,7 @@ def test_signed_in_multipart_mixed(self):
"""
s = self._make_signed()
m = email.mime.multipart.MIMEMultipart('mixed', None, [s])
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertEqual(m[utils.X_SIGNATURE_VALID_HEADER], 'True')
self.assertIn(utils.X_SIGNATURE_MESSAGE_HEADER, m)

Expand All @@ -616,8 +601,7 @@ def test_encrypted_unsigned_in_multipart_mixed(self):
"""
s = self._make_encrypted()
m = email.mime.multipart.MIMEMultipart('mixed', None, [s])
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn('This is some text', [n.get_payload() for n in m.walk()])
self.assertNotIn(utils.X_SIGNATURE_VALID_HEADER, m)
self.assertNotIn(utils.X_SIGNATURE_MESSAGE_HEADER, m)
Expand All @@ -629,8 +613,7 @@ def test_encrypted_signed_in_multipart_mixed(self):
"""
s = self._make_encrypted(True)
m = email.mime.multipart.MIMEMultipart('mixed', None, [s])
mb = m.as_bytes(policy=email.policy.SMTP)
m = utils.decrypted_message_from_bytes(mb)
m = utils.decrypted_message_from_bytes(m.as_bytes())
self.assertIn('This is some text', [n.get_payload() for n in m.walk()])
self.assertIn(utils.X_SIGNATURE_VALID_HEADER, m)
self.assertIn(utils.X_SIGNATURE_MESSAGE_HEADER, m)
Expand Down
76 changes: 38 additions & 38 deletions tests/static/mail/protonmail-signed.eml
View file
Original file line number Diff line number Diff line change
@@ -1,38 +1,38 @@
To: test <test@example.com>
From: test <test@example.com>
Subject: test
MIME-Version: 1.0
Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg=pgp-sha256; boundary="---------------------b1a1d581e2f9973dec483e5b8dea8467"; charset=UTF-8

This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
-----------------------b1a1d581e2f9973dec483e5b8dea8467
Content-Type: multipart/mixed;boundary=---------------------103eabeea9de680baf0de1110c9de38f
-----------------------103eabeea9de680baf0de1110c9de38f
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;charset=utf-8
This is a signed multipart message sent from ProtonMail.
-----------------------103eabeea9de680baf0de1110c9de38f--

-----------------------b1a1d581e2f9973dec483e5b8dea8467
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBcBAEBCAAGBQJejPBTAAoJEBl07FX7wtZg2XYH/3E0TLJ44OA36J55tv+i
FAnXy0ncTsqSJk2RloMI+rgJW6Ky1nkLMdPFuT7imBkuMGEFOv5NtWyuF6R+
ZTxML7JNEzK1kj/egvWMWgjAoKQ/OXhGEANwmeV90LseOlbPp6sppxgHcGVv
LymaQHPs4M5IDScum0jnywQ017esxQ4CuINuQsv3Tbp9E/nl5PrmwXsqB1ov
Z38n7RkJD523sPFuk6E6XrDaiod8J/Jd+0y7iWAVuxRyOVBnbHmMwkU8wvpI
VWkTLGNB2azregJp678DL7vfp5483tn4PqgSH5RO86glgo87uEin0cTmvdhp
+UQoeT6rE/dza6g2dGYG+co=
=h6OL
-----END PGP SIGNATURE-----


-----------------------b1a1d581e2f9973dec483e5b8dea8467--

To: test <test@example.com>
From: test <test@example.com>
Subject: test
MIME-Version: 1.0
Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg=pgp-sha256; boundary="---------------------b1a1d581e2f9973dec483e5b8dea8467"; charset=UTF-8

This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
-----------------------b1a1d581e2f9973dec483e5b8dea8467
Content-Type: multipart/mixed;boundary=---------------------103eabeea9de680baf0de1110c9de38f
-----------------------103eabeea9de680baf0de1110c9de38f
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;charset=utf-8
This is a signed multipart message sent from ProtonMail.
-----------------------103eabeea9de680baf0de1110c9de38f--

-----------------------b1a1d581e2f9973dec483e5b8dea8467
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: ProtonMail

wsBcBAEBCAAGBQJejPBTAAoJEBl07FX7wtZg2XYH/3E0TLJ44OA36J55tv+i
FAnXy0ncTsqSJk2RloMI+rgJW6Ky1nkLMdPFuT7imBkuMGEFOv5NtWyuF6R+
ZTxML7JNEzK1kj/egvWMWgjAoKQ/OXhGEANwmeV90LseOlbPp6sppxgHcGVv
LymaQHPs4M5IDScum0jnywQ017esxQ4CuINuQsv3Tbp9E/nl5PrmwXsqB1ov
Z38n7RkJD523sPFuk6E6XrDaiod8J/Jd+0y7iWAVuxRyOVBnbHmMwkU8wvpI
VWkTLGNB2azregJp678DL7vfp5483tn4PqgSH5RO86glgo87uEin0cTmvdhp
+UQoeT6rE/dza6g2dGYG+co=
=h6OL
-----END PGP SIGNATURE-----


-----------------------b1a1d581e2f9973dec483e5b8dea8467--

0 comments on commit dff3ae0

Please sign in to comment.