I uploaded this to ticket 3742 a couple weeks ago, but I don't think I sent it to the list. This patch attempts to allow Mutt to decrypt the malformed multipart/encrypted emails that MS Exchange sometimes generates when used with MAPI.
I've been testing this for a while and so far haven't seen any problems. -- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA http://www.8t8.us/configs/gpg-key-transition-statement.txt
# HG changeset patch # User Kevin McCarthy <[email protected]> # Date 1433899111 25200 # Tue Jun 09 18:18:31 2015 -0700 # Node ID 0eb23baf5acd8976040482b236e8a3d03cacd53d # Parent 704e0622cc671a0ca3324c6119c3203c17e5a5a7 Handle malformed ms-exchange pgp-encrypted block. (closes #3742) In certain circumstances, Exchange corrupts a multipart/encrypted block into: <multipart/mixed> <text/plain> <application/pgp-encrypted> [BASE64-encoded] <application/octet-stream> [BASE64-encoded] This patch pulls the full detection of valid/invalid multiparts into mutt_body_handler(). It extracts a run_decode_and_handler() function, which is reused by new intermediate handlers to decode the application/octet-stream part before passing it directly to crypt_pgp_encrypted_handler. These intermediate handlers then check and set any GOODSIG flags back into the parent part. This change may result in less error messages for invalid multipart/encrypted parts. Instead, mutt will default to the multipart_handler if it isn't fully "correct". Viewing attachments uses crypt_pgp_decrypt_mime() which bypasses the handler mechanism. Add decoding to the decrypt_mime() functions for pgp and gpgme. Thanks to Vincent Brillault for his analysis and initial patch. diff --git a/crypt-gpgme.c b/crypt-gpgme.c --- a/crypt-gpgme.c +++ b/crypt-gpgme.c @@ -1794,45 +1794,92 @@ /* Decrypt a PGP/MIME message in FPIN and B and return a new body and the stream in CUR and FPOUT. Returns 0 on success. */ int pgp_gpgme_decrypt_mime (FILE *fpin, FILE **fpout, BODY *b, BODY **cur) { char tempfile[_POSIX_PATH_MAX]; STATE s; BODY *first_part = b; - int is_signed; + int is_signed = 0; + int need_decode = 0; + int saved_type; + LOFF_T saved_offset; + size_t saved_length; + FILE *decoded_fp = NULL; + int rv = 0; first_part->goodsig = 0; first_part->warnsig = 0; - if(!mutt_is_multipart_encrypted(b)) + if (mutt_is_valid_multipart_pgp_encrypted (b)) + b = b->parts->next; + else if (mutt_is_malformed_multipart_pgp_encrypted (b)) + { + b = b->parts->next->next; + need_decode = 1; + } + else return -1; - - if(!b->parts || !b->parts->next) - return -1; - - b = b->parts->next; memset (&s, 0, sizeof (s)); s.fpin = fpin; + + if (need_decode) + { + saved_type = b->type; + saved_offset = b->offset; + saved_length = b->length; + + mutt_mktemp (tempfile, sizeof (tempfile)); + if ((decoded_fp = safe_fopen (tempfile, "w+")) == NULL) + { + mutt_perror (tempfile); + return (-1); + } + unlink (tempfile); + + fseeko (s.fpin, b->offset, 0); + s.fpout = decoded_fp; + + mutt_decode_attachment (b, &s); + + fflush (decoded_fp); + b->length = ftello (decoded_fp); + b->offset = 0; + rewind (decoded_fp); + s.fpin = decoded_fp; + s.fpout = 0; + } + mutt_mktemp (tempfile, sizeof (tempfile)); if (!(*fpout = safe_fopen (tempfile, "w+"))) { mutt_perror (tempfile); - return -1; + rv = -1; + goto bail; } unlink (tempfile); - *cur = decrypt_part (b, &s, *fpout, 0, &is_signed); + if ((*cur = decrypt_part (b, &s, *fpout, 0, &is_signed)) == NULL) + rv = -1; rewind (*fpout); if (is_signed > 0) first_part->goodsig = 1; - - return *cur? 0:-1; + +bail: + if (need_decode) + { + b->type = saved_type; + b->length = saved_length; + b->offset = saved_offset; + safe_fclose (&decoded_fp); + } + + return rv; } /* Decrypt a S/MIME message in FPIN and B and return a new body and the stream in CUR and FPOUT. Returns 0 on success. */ int smime_gpgme_decrypt_mime (FILE *fpin, FILE **fpout, BODY *b, BODY **cur) { char tempfile[_POSIX_PATH_MAX]; @@ -2514,41 +2561,29 @@ return err; } /* * Implementation of `encrypted_handler'. */ -/* MIME handler for pgp/mime encrypted messages. */ +/* MIME handler for pgp/mime encrypted messages. + * This handler is passed the application/octet-stream directly. + * The caller must propagate a->goodsig to its parent. + */ int pgp_gpgme_encrypted_handler (BODY *a, STATE *s) { char tempfile[_POSIX_PATH_MAX]; FILE *fpout; BODY *tattach; - BODY *orig_body = a; int is_signed; int rc = 0; dprint (2, (debugfile, "Entering pgp_encrypted handler\n")); - a = a->parts; - if (!a || a->type != TYPEAPPLICATION || !a->subtype - || ascii_strcasecmp ("pgp-encrypted", a->subtype) - || !a->next || a->next->type != TYPEAPPLICATION || !a->next->subtype - || ascii_strcasecmp ("octet-stream", a->next->subtype) ) - { - if (s->flags & M_DISPLAY) - state_attach_puts (_("[-- Error: malformed PGP/MIME message! --]\n\n"), - s); - return -1; - } - - /* Move forward to the application/pgp-encrypted body. */ - a = a->next; mutt_mktemp (tempfile, sizeof (tempfile)); if (!(fpout = safe_fopen (tempfile, "w+"))) { if (s->flags & M_DISPLAY) state_attach_puts (_("[-- Error: could not create temporary file! " "--]\n"), s); return -1; @@ -2573,17 +2608,17 @@ } /* * if a multipart/signed is the _only_ sub-part of a * multipart/encrypted, cache signature verification * status. */ if (mutt_is_multipart_signed (tattach) && !tattach->next) - orig_body->goodsig |= tattach->goodsig; + a->goodsig |= tattach->goodsig; if (s->flags & M_DISPLAY) { state_puts ("\n", s); state_attach_puts (is_signed? _("[-- End of PGP/MIME signed and encrypted data --]\n"): _("[-- End of PGP/MIME encrypted data --]\n"), s); diff --git a/crypt.c b/crypt.c --- a/crypt.c +++ b/crypt.c @@ -309,23 +309,84 @@ char *p; if (!b || b->type != TYPEMULTIPART || !b->subtype || ascii_strcasecmp (b->subtype, "encrypted") || !(p = mutt_get_parameter ("protocol", b->parameter)) || ascii_strcasecmp (p, "application/pgp-encrypted")) return 0; - return PGPENCRYPT; + return PGPENCRYPT; } return 0; } +int mutt_is_valid_multipart_pgp_encrypted (BODY *b) +{ + if (! mutt_is_multipart_encrypted (b)) + return 0; + + b = b->parts; + if (!b || b->type != TYPEAPPLICATION || + !b->subtype || ascii_strcasecmp (b->subtype, "pgp-encrypted")) + return 0; + + b = b->next; + if (!b || b->type != TYPEAPPLICATION || + !b->subtype || ascii_strcasecmp (b->subtype, "octet-stream")) + return 0; + + return PGPENCRYPT; +} + + +/* + * This checks for the malformed layout caused by MS Exchange in + * some cases: + * <multipart/mixed> + * <text/plain> + * <application/pgp-encrypted> [BASE64-encoded] + * <application/octet-stream> [BASE64-encoded] + * See ticket #3742 + */ +int mutt_is_malformed_multipart_pgp_encrypted (BODY *b) +{ + if (!(WithCrypto & APPLICATION_PGP)) + return 0; + + if (!b || b->type != TYPEMULTIPART || + !b->subtype || ascii_strcasecmp (b->subtype, "mixed")) + return 0; + + b = b->parts; + if (!b || b->type != TYPETEXT || + !b->subtype || ascii_strcasecmp (b->subtype, "plain") || + b->length != 0) + return 0; + + b = b->next; + if (!b || b->type != TYPEAPPLICATION || + !b->subtype || ascii_strcasecmp (b->subtype, "pgp-encrypted")) + return 0; + + b = b->next; + if (!b || b->type != TYPEAPPLICATION || + !b->subtype || ascii_strcasecmp (b->subtype, "octet-stream")) + return 0; + + b = b->next; + if (b) + return 0; + + return PGPENCRYPT; +} + + int mutt_is_application_pgp (BODY *m) { int t = 0; char *p; if (m->type == TYPEAPPLICATION) { if (!ascii_strcasecmp (m->subtype, "pgp") || !ascii_strcasecmp (m->subtype, "x-pgp-message")) @@ -464,16 +525,17 @@ if (t && m->goodsig) t |= GOODSIGN; } if (m->type == TYPEMULTIPART) { t |= mutt_is_multipart_encrypted(m); t |= mutt_is_multipart_signed (m); + t |= mutt_is_malformed_multipart_pgp_encrypted (m); if (t && m->goodsig) t |= GOODSIGN; } if (m->type == TYPEMULTIPART || m->type == TYPEMESSAGE) { BODY *p; diff --git a/handler.c b/handler.c --- a/handler.c +++ b/handler.c @@ -1585,25 +1585,143 @@ state_puts (buf, s); state_putc ('\n', s); } FREE (&buf); return 0; } +static int run_decode_and_handler (BODY *b, STATE *s, handler_t handler, int plaintext) +{ + int origType; + char *savePrefix = NULL; + FILE *fp = NULL; + char tempfile[_POSIX_PATH_MAX]; + size_t tmplength = 0; + LOFF_T tmpoffset = 0; + int decode = 0; + int rc = 0; + + fseeko (s->fpin, b->offset, 0); + + /* see if we need to decode this part before processing it */ + if (b->encoding == ENCBASE64 || b->encoding == ENCQUOTEDPRINTABLE || + b->encoding == ENCUUENCODED || plaintext || + mutt_is_text_part (b)) /* text subtypes may + * require character + * set conversion even + * with 8bit encoding. + */ + { + origType = b->type; + + if (!plaintext) + { + /* decode to a tempfile, saving the original destination */ + fp = s->fpout; + mutt_mktemp (tempfile, sizeof (tempfile)); + if ((s->fpout = safe_fopen (tempfile, "w")) == NULL) + { + mutt_error _("Unable to open temporary file!"); + dprint (1, (debugfile, "Can't open %s.\n", tempfile)); + return -1; + } + /* decoding the attachment changes the size and offset, so save a copy + * of the "real" values now, and restore them after processing + */ + tmplength = b->length; + tmpoffset = b->offset; + + /* if we are decoding binary bodies, we don't want to prefix each + * line with the prefix or else the data will get corrupted. + */ + savePrefix = s->prefix; + s->prefix = NULL; + + decode = 1; + } + else + b->type = TYPETEXT; + + mutt_decode_attachment (b, s); + + if (decode) + { + b->length = ftello (s->fpout); + b->offset = 0; + safe_fclose (&s->fpout); + + /* restore final destination and substitute the tempfile for input */ + s->fpout = fp; + fp = s->fpin; + s->fpin = fopen (tempfile, "r"); + unlink (tempfile); + + /* restore the prefix */ + s->prefix = savePrefix; + } + + b->type = origType; + } + + /* process the (decoded) body part */ + if (handler) + { + rc = handler (b, s); + + if (rc) + { + dprint (1, (debugfile, "Failed on attachment of type %s/%s.\n", TYPE(b), NONULL (b->subtype))); + } + + if (decode) + { + b->length = tmplength; + b->offset = tmpoffset; + + /* restore the original source stream */ + safe_fclose (&s->fpin); + s->fpin = fp; + } + } + s->flags |= M_FIRSTDONE; + + return rc; +} + +static int valid_pgp_encrypted_handler (BODY *b, STATE *s) +{ + int rc; + BODY *octetstream; + + octetstream = b->parts->next; + rc = crypt_pgp_encrypted_handler (octetstream, s); + b->goodsig |= octetstream->goodsig; + + return rc; +} + +static int malformed_pgp_encrypted_handler (BODY *b, STATE *s) +{ + int rc; + BODY *octetstream; + + octetstream = b->parts->next->next; + /* exchange encodes the octet-stream, so re-run it through the decoder */ + rc = run_decode_and_handler (octetstream, s, crypt_pgp_encrypted_handler, 0); + b->goodsig |= octetstream->goodsig; + + return rc; +} + int mutt_body_handler (BODY *b, STATE *s) { - int decode = 0; int plaintext = 0; - FILE *fp = NULL; - char tempfile[_POSIX_PATH_MAX]; handler_t handler = NULL; - LOFF_T tmpoffset = 0; - size_t tmplength = 0; int rc = 0; int oflags = s->flags; /* first determine which handler to use to process this part */ if (mutt_is_autoview (b)) { @@ -1648,30 +1766,24 @@ { p = mutt_get_parameter ("protocol", b->parameter); if (!p) mutt_error _("Error: multipart/signed has no protocol."); else if (s->flags & M_VERIFY) handler = mutt_signed_handler; } - else if ((WithCrypto & APPLICATION_PGP) - && ascii_strcasecmp ("encrypted", b->subtype) == 0) - { - p = mutt_get_parameter ("protocol", b->parameter); - - if (!p) - mutt_error _("Error: multipart/encrypted has no protocol parameter!"); - else if (ascii_strcasecmp ("application/pgp-encrypted", p) == 0) - handler = crypt_pgp_encrypted_handler; - } + else if (mutt_is_valid_multipart_pgp_encrypted (b)) + handler = valid_pgp_encrypted_handler; + else if (mutt_is_malformed_multipart_pgp_encrypted (b)) + handler = malformed_pgp_encrypted_handler; if (!handler) handler = multipart_handler; - + if (b->encoding != ENC7BIT && b->encoding != ENC8BIT && b->encoding != ENCBINARY) { dprint (1, (debugfile, "Bad encoding type %d for multipart entity, " "assuming 7 bit\n", b->encoding)); b->encoding = ENC7BIT; } } @@ -1690,100 +1802,17 @@ } /* only respect disposition == attachment if we're not displaying from the attachment menu (i.e. pager) */ if ((!option (OPTHONORDISP) || (b->disposition != DISPATTACH || option(OPTVIEWATTACH))) && (plaintext || handler)) { - fseeko (s->fpin, b->offset, 0); - - /* see if we need to decode this part before processing it */ - if (b->encoding == ENCBASE64 || b->encoding == ENCQUOTEDPRINTABLE || - b->encoding == ENCUUENCODED || plaintext || - mutt_is_text_part (b)) /* text subtypes may - * require character - * set conversion even - * with 8bit encoding. - */ - { - int origType = b->type; - char *savePrefix = NULL; - - if (!plaintext) - { - /* decode to a tempfile, saving the original destination */ - fp = s->fpout; - mutt_mktemp (tempfile, sizeof (tempfile)); - if ((s->fpout = safe_fopen (tempfile, "w")) == NULL) - { - mutt_error _("Unable to open temporary file!"); - dprint (1, (debugfile, "Can't open %s.\n", tempfile)); - goto bail; - } - /* decoding the attachment changes the size and offset, so save a copy - * of the "real" values now, and restore them after processing - */ - tmplength = b->length; - tmpoffset = b->offset; - - /* if we are decoding binary bodies, we don't want to prefix each - * line with the prefix or else the data will get corrupted. - */ - savePrefix = s->prefix; - s->prefix = NULL; - - decode = 1; - } - else - b->type = TYPETEXT; - - mutt_decode_attachment (b, s); - - if (decode) - { - b->length = ftello (s->fpout); - b->offset = 0; - safe_fclose (&s->fpout); - - /* restore final destination and substitute the tempfile for input */ - s->fpout = fp; - fp = s->fpin; - s->fpin = fopen (tempfile, "r"); - unlink (tempfile); - - /* restore the prefix */ - s->prefix = savePrefix; - } - - b->type = origType; - } - - /* process the (decoded) body part */ - if (handler) - { - rc = handler (b, s); - - if (rc) - { - dprint (1, (debugfile, "Failed on attachment of type %s/%s.\n", TYPE(b), NONULL (b->subtype))); - } - - if (decode) - { - b->length = tmplength; - b->offset = tmpoffset; - - /* restore the original source stream */ - safe_fclose (&s->fpin); - s->fpin = fp; - } - } - s->flags |= M_FIRSTDONE; + rc = run_decode_and_handler (b, s, handler, plaintext); } /* print hint to use attachment menu for disposition == attachment if we're not already being called from there */ else if ((s->flags & M_DISPLAY) || (b->disposition == DISPATTACH && !option (OPTVIEWATTACH) && option (OPTHONORDISP) && (plaintext || handler))) { @@ -1800,17 +1829,16 @@ km_find_func (MENU_PAGER, OP_VIEW_ATTACHMENTS))) fprintf (s->fpout, _("(use '%s' to view this part)"), keystroke); else fputs (_("(need 'view-attachments' bound to key!)"), s->fpout); } fputs (" --]\n", s->fpout); } - bail: s->flags = oflags | (s->flags & M_FIRSTDONE); if (rc) { dprint (1, (debugfile, "Bailing on attachment of type %s/%s.\n", TYPE(b), NONULL (b->subtype))); } return rc; } diff --git a/mutt_crypt.h b/mutt_crypt.h --- a/mutt_crypt.h +++ b/mutt_crypt.h @@ -107,16 +107,20 @@ /* Some prototypes -- old crypt.h. */ int mutt_protect (HEADER *, char *); int mutt_is_multipart_encrypted (BODY *); +int mutt_is_valid_multipart_pgp_encrypted (BODY *b); + +int mutt_is_malformed_multipart_pgp_encrypted (BODY *b); + int mutt_is_multipart_signed (BODY *); int mutt_is_application_pgp (BODY *); int mutt_is_application_smime (BODY *); int mutt_signed_handler (BODY *, STATE *); diff --git a/pgp.c b/pgp.c --- a/pgp.c +++ b/pgp.c @@ -949,80 +949,110 @@ return (tattach); } int pgp_decrypt_mime (FILE *fpin, FILE **fpout, BODY *b, BODY **cur) { char tempfile[_POSIX_PATH_MAX]; STATE s; BODY *p = b; - - if(!mutt_is_multipart_encrypted(b)) + int need_decode = 0; + int saved_type; + LOFF_T saved_offset; + size_t saved_length; + FILE *decoded_fp = NULL; + int rv = 0; + + if (mutt_is_valid_multipart_pgp_encrypted (b)) + b = b->parts->next; + else if (mutt_is_malformed_multipart_pgp_encrypted (b)) + { + b = b->parts->next->next; + need_decode = 1; + } + else return -1; - if(!b->parts || !b->parts->next) - return -1; - - b = b->parts->next; - memset (&s, 0, sizeof (s)); s.fpin = fpin; + + if (need_decode) + { + saved_type = b->type; + saved_offset = b->offset; + saved_length = b->length; + + mutt_mktemp (tempfile, sizeof (tempfile)); + if ((decoded_fp = safe_fopen (tempfile, "w+")) == NULL) + { + mutt_perror (tempfile); + return (-1); + } + unlink (tempfile); + + fseeko (s.fpin, b->offset, 0); + s.fpout = decoded_fp; + + mutt_decode_attachment (b, &s); + + fflush (decoded_fp); + b->length = ftello (decoded_fp); + b->offset = 0; + rewind (decoded_fp); + s.fpin = decoded_fp; + s.fpout = 0; + } + mutt_mktemp (tempfile, sizeof (tempfile)); if ((*fpout = safe_fopen (tempfile, "w+")) == NULL) { mutt_perror (tempfile); - return (-1); + rv = -1; + goto bail; } unlink (tempfile); - *cur = pgp_decrypt_part (b, &s, *fpout, p); + if ((*cur = pgp_decrypt_part (b, &s, *fpout, p)) == NULL) + rv = -1; + rewind (*fpout); - rewind (*fpout); - - if (!*cur) - return -1; - - return (0); +bail: + if (need_decode) + { + b->type = saved_type; + b->length = saved_length; + b->offset = saved_offset; + safe_fclose (&decoded_fp); + } + + return rv; } +/* + * This handler is passed the application/octet-stream directly. + * The caller must propagate a->goodsig to its parent. + */ int pgp_encrypted_handler (BODY *a, STATE *s) { char tempfile[_POSIX_PATH_MAX]; FILE *fpout, *fpin; BODY *tattach; - BODY *p = a; int rc = 0; - - a = a->parts; - if (!a || a->type != TYPEAPPLICATION || !a->subtype || - ascii_strcasecmp ("pgp-encrypted", a->subtype) != 0 || - !a->next || a->next->type != TYPEAPPLICATION || !a->next->subtype || - ascii_strcasecmp ("octet-stream", a->next->subtype) != 0) - { - if (s->flags & M_DISPLAY) - state_attach_puts (_("[-- Error: malformed PGP/MIME message! --]\n\n"), s); - return -1; - } - - /* - * Move forward to the application/pgp-encrypted body. - */ - a = a->next; mutt_mktemp (tempfile, sizeof (tempfile)); if ((fpout = safe_fopen (tempfile, "w+")) == NULL) { if (s->flags & M_DISPLAY) state_attach_puts (_("[-- Error: could not create temporary file! --]\n"), s); return -1; } if (s->flags & M_DISPLAY) crypt_current_time (s, "PGP"); - if ((tattach = pgp_decrypt_part (a, s, fpout, p)) != NULL) + if ((tattach = pgp_decrypt_part (a, s, fpout, a)) != NULL) { if (s->flags & M_DISPLAY) state_attach_puts (_("[-- The following data is PGP/MIME encrypted --]\n\n"), s); fpin = s->fpin; s->fpin = fpout; rc = mutt_body_handler (tattach, s); s->fpin = fpin; @@ -1030,17 +1060,17 @@ /* * if a multipart/signed is the _only_ sub-part of a * multipart/encrypted, cache signature verification * status. * */ if (mutt_is_multipart_signed (tattach) && !tattach->next) - p->goodsig |= tattach->goodsig; + a->goodsig |= tattach->goodsig; if (s->flags & M_DISPLAY) { state_puts ("\n", s); state_attach_puts (_("[-- End of PGP/MIME encrypted data --]\n"), s); } mutt_free_body (&tattach); diff --git a/recvattach.c b/recvattach.c --- a/recvattach.c +++ b/recvattach.c @@ -991,17 +991,18 @@ safe_fclose (&_fp); } } else need_secured = 0; } if ((WithCrypto & APPLICATION_PGP) && (hdr->security & APPLICATION_PGP)) { - if (mutt_is_multipart_encrypted(hdr->content)) + if (mutt_is_multipart_encrypted(hdr->content) || + mutt_is_malformed_multipart_pgp_encrypted(hdr->content)) secured = !crypt_pgp_decrypt_mime (msg->fp, &fp, hdr->content, &cur); else need_secured = 0; } if (need_secured && !secured) { mx_close_message (&msg);
signature.asc
Description: PGP signature
