From: Kevin McCarthy <kevin@8t8.us>
Date: Mon, 4 Jun 2018 22:40:57 +0000 (-0700)
Subject: Add $pgp_check_gpg_decrypt_status_fd.
X-Git-Tag: mutt-1-10-1-rel~15
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8ec6d766c7557e42462d0db1ceeb7ccc83556345;p=mutt

Add $pgp_check_gpg_decrypt_status_fd.

If set (the default) mutt performs more thorough checking of the
$pgp_decrypt_command status output for GnuPG result codes.

Ticket #39 revealed that GnuPG (currently) does not protect against
messages that have been manipulated to contain an empty encryption
packet followed by a plaintext packet.

A huge thanks to Marcus Brinkmann for researching this issue, taking
the time to report it to us (and the GnuPG team), and taking even more
time to clarify exactly what needed to be checked for.   
---

diff --git a/contrib/gpg.rc b/contrib/gpg.rc
index 452eed2b..7adcbdb2 100644
--- a/contrib/gpg.rc
+++ b/contrib/gpg.rc
@@ -106,5 +106,7 @@ set pgp_list_secring_command="gpg --no-verbose --batch --quiet --with-colons --w
 set pgp_good_sign="^\\[GNUPG:\\] GOODSIG"
 
 # pattern to verify a decryption occurred
-set pgp_decryption_okay="^\\[GNUPG:\\] DECRYPTION_OKAY"
+# This is now deprecated by pgp_check_gpg_decrypt_status_fd:
+# set pgp_decryption_okay="^\\[GNUPG:\\] DECRYPTION_OKAY"
+set pgp_check_gpg_decrypt_status_fd
 
diff --git a/contrib/pgp2.rc b/contrib/pgp2.rc
index bd66cf10..21280453 100644
--- a/contrib/pgp2.rc
+++ b/contrib/pgp2.rc
@@ -20,6 +20,9 @@ set pgp_verify_command="pgp +language=mutt +verbose=0 +batchmode -t %s %f"
 # decrypt a pgp/mime attachment
 set pgp_decrypt_command="PGPPASSFD=0; export PGPPASSFD; cat - %f | pgp +language=mutt +verbose=0 +batchmode -f"
 
+# don't check for GnuPG decryption status codes
+unset pgp_check_gpg_decrypt_status_fd
+
 # create a pgp/mime signed attachment
 set pgp_sign_command="PGPPASSFD=0; export PGPPASSFD; cat - %f | pgp +language=mutt +verbose=0 +batchmode -abfst %?a? -u %a?"
 
diff --git a/contrib/pgp5.rc b/contrib/pgp5.rc
index d2e578fb..61d1c2c9 100644
--- a/contrib/pgp5.rc
+++ b/contrib/pgp5.rc
@@ -17,6 +17,9 @@ set pgp_good_sign = "Good signature"
 # decrypt a pgp/mime attachment
 set pgp_decrypt_command="PGPPASSFD=0; export PGPPASSFD; cat - %f | pgpv +language=mutt +verbose=0 +batchmode --OutputInformationFD=2 -f"
 
+# don't check for GnuPG decryption status codes
+unset pgp_check_gpg_decrypt_status_fd
+
 # create a pgp/mime signed attachment
 set pgp_sign_command="PGPPASSFD=0; export PGPPASSFD; cat - %f | pgps +language=mutt +verbose=0 +batchmode -abft %?a? -u %a?"
 
diff --git a/contrib/pgp6.rc b/contrib/pgp6.rc
index a317d4be..a5fca654 100644
--- a/contrib/pgp6.rc
+++ b/contrib/pgp6.rc
@@ -14,6 +14,9 @@ set pgp_verify_command="pgp6 +compatible   +verbose=0 +batchmode -t %s %f"
 # decrypt a pgp/mime attachment
 set pgp_decrypt_command="PGPPASSFD=0; export PGPPASSFD; cat - %f | pgp6 +compatible   +verbose=0 +batchmode -f"
 
+# don't check for GnuPG decryption status codes
+unset pgp_check_gpg_decrypt_status_fd
+
 # create a pgp/mime signed attachment
 set pgp_sign_command="PGPPASSFD=0; export PGPPASSFD; cat - %f | pgp6 +compatible   +verbose=0 +batchmode -abfst %?a? -u %a?"
 
diff --git a/init.h b/init.h
index 6c443945..44d7fabd 100644
--- a/init.h
+++ b/init.h
@@ -1941,6 +1941,20 @@ struct option_t MuttVars[] = {
   ** subprocess failed.
   ** (PGP only)
   */
+  { "pgp_check_gpg_decrypt_status_fd", DT_BOOL, R_NONE, OPTPGPCHECKGPGDECRYPTSTATUSFD, 1 },
+  /*
+  ** .pp
+  ** If \fIset\fP, mutt will check the status file descriptor output
+  ** of $$pgp_decrypt_command for GnuPG status codes indicating
+  ** successful decryption.  This will check for the presence of
+  ** DECRYPTION_OKAY, absence of DECRYPTION_FAILED, and that all
+  ** PLAINTEXT occurs between the BEGIN_DECRYPTION and END_DECRYPTION
+  ** status codes.
+  ** .pp
+  ** If \fIunset\fP, mutt will instead match the status fd output
+  ** against $$pgp_decryption_okay.
+  ** (PGP only)
+  */
   { "pgp_clearsign_command",	DT_STR,	R_NONE, UL &PgpClearSignCommand, 0 },
   /*
   ** .pp
@@ -1994,6 +2008,9 @@ struct option_t MuttVars[] = {
   ** protect against a spoofed encrypted message, with multipart/encrypted
   ** headers but containing a block that is not actually encrypted.
   ** (e.g. simply signed and ascii armored text).
+  ** .pp
+  ** Note that if $$pgp_check_gpg_decrypt_status_fd is set, this variable
+  ** is ignored.
   ** (PGP only)
   */
   { "pgp_self_encrypt_as",	DT_SYN,  R_NONE, UL "pgp_default_key", 0 },
diff --git a/mutt.h b/mutt.h
index 663beb4f..7f49c116 100644
--- a/mutt.h
+++ b/mutt.h
@@ -529,6 +529,7 @@ enum
   OPTSDEFAULTDECRYPTKEY,
   OPTPGPIGNORESUB,
   OPTPGPCHECKEXIT,
+  OPTPGPCHECKGPGDECRYPTSTATUSFD,
   OPTPGPLONGIDS,
   OPTPGPAUTODEC,
 #if 0
diff --git a/pgp.c b/pgp.c
index ea307997..c9a4c91f 100644
--- a/pgp.c
+++ b/pgp.c
@@ -231,7 +231,7 @@ static int pgp_copy_checksig (FILE *fpin, FILE *fpout)
  * This protects against messages with multipart/encrypted headers
  * but which aren't actually encrypted.  See ticket #3770
  */
-static int pgp_check_decryption_okay (FILE *fpin)
+static int pgp_check_pgp_decryption_okay_regexp (FILE *fpin)
 {
   int rv = -1;
 
@@ -245,26 +245,79 @@ static int pgp_check_decryption_okay (FILE *fpin)
     {
       if (regexec (PgpDecryptionOkay.rx, line, 0, NULL, 0) == 0)
       {
-        dprint (2, (debugfile, "pgp_check_decryption_okay: \"%s\" matches regexp.\n",
+        dprint (2, (debugfile, "pgp_check_pgp_decryption_okay_regexp: \"%s\" matches regexp.\n",
                     line));
         rv = 0;
         break;
       }
       else
-        dprint (2, (debugfile, "pgp_check_decryption_okay: \"%s\" doesn't match regexp.\n",
+        dprint (2, (debugfile, "pgp_check_pgp_decryption_okay_regexp: \"%s\" doesn't match regexp.\n",
                     line));
     }
     FREE (&line);
   }
   else
   {
-    dprint (2, (debugfile, "pgp_check_decryption_okay: No pattern.\n"));
+    dprint (2, (debugfile, "pgp_check_pgp_decryption_okay_regexp: No pattern.\n"));
     rv = 1;
   }
 
   return rv;
 }
 
+/* Checks GnuPGP status fd output for various status codes indicating
+ * an issue.  If $pgp_check_gpg_decrypt_status_fd is unset, it falls
+ * back to the old behavior of just scanning for $pgp_decryption_okay.
+ */
+static int pgp_check_decryption_okay (FILE *fpin)
+{
+  int rv = -1;
+  char *line = NULL, *s;
+  int lineno = 0;
+  size_t linelen;
+  int inside_decrypt = 0;
+
+  if (!option (OPTPGPCHECKGPGDECRYPTSTATUSFD))
+    return pgp_check_pgp_decryption_okay_regexp (fpin);
+
+  while ((line = mutt_read_line (line, &linelen, fpin, &lineno, 0)) != NULL)
+  {
+    if (strncmp (line, "[GNUPG:] ", 9) != 0)
+      continue;
+    s = line + 9;
+    dprint (2, (debugfile, "pgp_check_decryption_okay: checking \"%s\".\n",
+                  line));
+    if (mutt_strncmp (s, "BEGIN_DECRYPTION", 16) == 0)
+      inside_decrypt = 1;
+    else if (mutt_strncmp (s, "END_DECRYPTION", 14) == 0)
+      inside_decrypt = 0;
+    else if (mutt_strncmp (s, "PLAINTEXT", 9) == 0)
+    {
+      if (!inside_decrypt)
+      {
+        dprint (2, (debugfile, "\tPLAINTEXT encountered outside of DECRYPTION.  Failure.\n"));
+        rv = -1;
+        break;
+      }
+    }
+    else if (mutt_strncmp (s, "DECRYPTION_FAILED", 17) == 0)
+    {
+      dprint (2, (debugfile, "\tDECRYPTION_FAILED encountered.  Failure.\n"));
+      rv = -1;
+      break;
+    }
+    else if (mutt_strncmp (s, "DECRYPTION_OKAY", 15) == 0)
+    {
+      /* Don't break out because we still have to check for
+       * PLAINTEXT outside of the decryption boundaries. */
+      dprint (2, (debugfile, "\tDECRYPTION_OKAY encountered.\n"));
+      rv = 0;
+    }
+  }
+  FREE (&line);
+
+  return rv;
+}
 
 /* 
  * Copy a clearsigned message, and strip the signature and PGP's