Skip to content
Permalink
Browse files

EFail mitigation: Disable decryption if message structure is unusual

  • Loading branch information...
BjarniRunar committed Aug 27, 2019
1 parent 8267248 commit 79551b17928a9f990ac5e1336386caab8b183323
Showing with 45 additions and 13 deletions.
  1. +36 −9 mailpile/crypto/mime.py
  2. +9 −4 mailpile/mailutils/emails.py
@@ -148,7 +148,8 @@ def MimeReplacePart(part, newpart, keep_old_headers=False):


def UnwrapMimeCrypto(part, protocols=None, psi=None, pei=None, charsets=None,
unwrap_attachments=True, require_MDC=True, depth=0):
unwrap_attachments=True, require_MDC=True,
depth=0, sibling=0, efail_unsafe=False, allow_decrypt=True):
"""
This method will replace encrypted and signed parts with their
contents and set part attributes describing the security properties
@@ -208,7 +209,9 @@ def UnwrapMimeCrypto(part, protocols=None, psi=None, pei=None, charsets=None,
charsets=charsets,
unwrap_attachments=unwrap_attachments,
require_MDC=require_MDC,
depth = depth + 1 )
depth=depth+1, sibling=sibling,
efail_unsafe=efail_unsafe,
allow_decrypt=allow_decrypt)

except (IOError, OSError, ValueError, IndexError, KeyError):
part.signature_info = SignatureInfo()
@@ -217,8 +220,9 @@ def UnwrapMimeCrypto(part, protocols=None, psi=None, pei=None, charsets=None,

elif part.is_multipart() and mimetype == 'multipart/encrypted':
try:
if not allow_decrypt:
raise ValueError('Decryption forbidden, MIME structure is weird')
preamble, payload = part.get_payload()

(part.signature_info, part.encryption_info, decrypted) = (
crypto_cls().decrypt(
payload.as_string(), require_MDC=require_MDC))
@@ -268,20 +272,33 @@ def UnwrapMimeCrypto(part, protocols=None, psi=None, pei=None, charsets=None,
charsets=charsets,
unwrap_attachments=unwrap_attachments,
require_MDC=require_MDC,
depth = depth + 1 )
depth=depth+1, sibling=sibling,
efail_unsafe=efail_unsafe,
allow_decrypt=allow_decrypt)

# If we are still multipart after the above shenanigans (perhaps due
# to an error state), recurse into our subparts and unwrap them too.
elif part.is_multipart():
for sp in part.get_payload():
for count, sp in enumerate(part.get_payload()):
# EFail mitigation: We decrypt attachments and the first part
# or a nested multipart structure, but not any subsequent parts.
# This allows rewriting of messages to *append* cleartext, but
# disallows rewriting that pushes "inline" encrypted content
# further down to where the recipient might not notice it.
sp_disp = (unwrap_attachments and sp['content-disposition']) or ""
allow_decrypt = (efail_unsafe
or (count == sibling == 0)
or sp_disp.startswith('attachment')) and allow_decrypt
UnwrapMimeCrypto(sp,
protocols=protocols,
psi=part.signature_info,
pei=part.encryption_info,
charsets=charsets,
unwrap_attachments=unwrap_attachments,
require_MDC=require_MDC,
depth = depth + 1 )
depth=depth+1, sibling=count,
efail_unsafe=efail_unsafe,
allow_decrypt=allow_decrypt)

elif disposition.startswith('attachment'):
# The sender can attach signed/encrypted/key files without following
@@ -299,6 +316,8 @@ def UnwrapMimeCrypto(part, protocols=None, psi=None, pei=None, charsets=None,
# are encrypted and signed, and files that are signed only.
payload = part.get_payload( None, True )
try:
if not allow_decrypt:
raise ValueError('Decryption forbidden, MIME structure is weird')
(part.signature_info, part.encryption_info, decrypted) = (
crypto_cls().decrypt(
payload, require_MDC=require_MDC))
@@ -330,7 +349,9 @@ def UnwrapMimeCrypto(part, protocols=None, psi=None, pei=None, charsets=None,
charsets=charsets,
unwrap_attachments=unwrap_attachments,
require_MDC=require_MDC,
depth = depth + 1 )
depth=depth+1, sibling=sibling,
efail_unsafe=efail_unsafe,
allow_decrypt=allow_decrypt)
else:
# FIXME: Best action for unsuccessful attachment processing?
pass
@@ -342,7 +363,9 @@ def UnwrapMimeCrypto(part, protocols=None, psi=None, pei=None, charsets=None,
pei=pei,
charsets=charsets,
require_MDC=require_MDC,
depth = depth + 1 )
depth=depth+1, sibling=sibling,
efail_unsafe=efail_unsafe,
allow_decrypt=allow_decrypt)

else:
# FIXME: This is where we would handle cryptoschemes that don't
@@ -360,7 +383,9 @@ def UnwrapMimeCrypto(part, protocols=None, psi=None, pei=None, charsets=None,


def UnwrapPlainTextCrypto(part, protocols=None, psi=None, pei=None,
charsets=None, require_MDC=True, depth=0):
charsets=None, require_MDC=True,
depth=0, sibling=0,
efail_unsafe=False, allow_decrypt=True):
"""
This method will replace encrypted and signed parts with their
contents and set part attributes describing the security properties
@@ -375,6 +400,8 @@ def UnwrapPlainTextCrypto(part, protocols=None, psi=None, pei=None,
if (payload.startswith(crypto.ARMOR_BEGIN_ENCRYPTED) and
payload.endswith(crypto.ARMOR_END_ENCRYPTED)):
try:
if not allow_decrypt:
raise ValueError('Decryption forbidden, MIME structure is weird')
si, ei, text = crypto.decrypt(payload, require_MDC=require_MDC)
_update_text_payload(part, text, charsets=charsets)
except (IOError, OSError, ValueError, IndexError, KeyError):
@@ -1304,8 +1304,8 @@ def parse_text_part(self, data, charset, crypto, mimetype, count):
parse = []
block = 'body'
clines = []
for line in data.splitlines(True):
block, ltype = self.parse_line_type(line, block)
for count, line in enumerate(data.splitlines(True)):
block, ltype = self.parse_line_type(line, block, count)
if ltype != current['type']:

# This is not great, it's a hack to move the preamble
@@ -1339,7 +1339,7 @@ def parse_text_part(self, data, charset, crypto, mimetype, count):
GIT_DIFF_STARTS = re.compile('^diff --git a/.*b/')
GIT_DIFF_LINE = re.compile('^([ +@-]|index |$)')

def parse_line_type(self, line, block):
def parse_line_type(self, line, block, line_count):
# FIXME: Detect forwarded messages, ...

if (block in ('body', 'quote', 'barequote')
@@ -1372,7 +1372,12 @@ def parse_line_type(self, line, block):
else:
return 'pgpsignature', 'pgpsignature'

if stripped == GnuPG.ARMOR_BEGIN_ENCRYPTED:
if (stripped == GnuPG.ARMOR_BEGIN_ENCRYPTED
# This is an EFail mitigation: do not decrypt content
# inlined somewhere well below a bunch of other stuff.
# The encrypted content must be high up enough that
# the user will plausibly see it when reading.
and line_count < 10 and block == 'body'):
return 'pgpbegin', 'pgpbegin'
if block == 'pgpbegin':
if ':' in line or stripped == '':

0 comments on commit 79551b1

Please sign in to comment.
You can’t perform that action at this time.