wrong parameter micalg with mutt 1.5.15 and gpgme

[lespocky]

I compiled a clean mutt 1.5.15 with gcc 3.4.5 as follows:

./configure     --prefix=/usr \
                --build=i386-pc-linux-gnu \
                --sysconfdir=/etc \
                --localstatedir=/var/lib \
                --libdir=/usr/lib \
                --libexecdir=/usr/local/mutt \
                --enable-imap \
                --enable-gpgme \
                --disable-pgp \
                --disable-smime \
                --with-ssl \
                --with-mailpath=/var/spool/mail

I configured crypto as follows:

set crypt_autoencrypt = no
set crypt_autopgp = yes
set crypt_autosign = yes # default: no
set crypt_autosmime = no # default: yes
set crypt_replyencrypt = yes
set crypt_replysign = yes # default: no
set crypt_replysignencrypted = yes # default: no
set crypt_timestamp = yes
set crypt_use_gpgme = yes # default: no
set crypt_verify_sig = yes

I don't use S/MIME but GnuPG (parallel 1.4.7, 2.0.4) with GPGME 1.1.4.

Now if I send messages signed (not encrypted) a part of the header looks like:

Subject: Mail mit mutt und so
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=SHA1;
	protocol="application/pgp-signature"; boundary="/9DWx/yDrRhgMJTb"
Content-Disposition: inline
User-Agent: Mutt/1.5.15 (2007-04-06)

The Problem ist the parameter "micalg=SHA1". Mozilla Thunderbird/Enigmail (2.0.0.0 WinXP, Enigmail 0.95.0) doesn't recognizes this. I asked in newsgroup public.mozdev.enigmail and got this answer:

The problem is with mutt. The content-type that mutt sends is: Content-Type: multipart/signed; micalg=SHA1; [etc]

However, according to RFC 3156 (and RFC 2015), "the 'micalg' parameter for the 'application/pgp-signature' protocol MUST contain exactly one hash-symbol of the format 'pgp-<hash-identifier>', where <hash-identifier> identifies the Message Integrity Check (MIC) algorithm used to generate the signature.". I.e. the content-type should be:

Content-Type: multipart/signed; micalg=pgp-sha1; [etc]

Conclusion: mutt sets the wrong parameter for micalg, is "SHA1", should be "pgp-sha1".

[lespocky]

I looked around a bit to find hints to solve this problem. First is RFC3156 saying the following:

The "micalg" parameter for the "application/pgp-signature" protocol MUST contain exactly one hash-symbol of the format "pgp-<hash- identifier>", where <hash-identifier> identifies the Message Integrity Check (MIC) algorithm used to generate the signature. Hash-symbols are constructed from the text names registered in RFC2440 or according to the mechanism defined in that document by converting the text name to lower case and prefixing it with the four characters "pgp-".

Currently defined values are "pgp-md5", "pgp-sha1", "pgp-ripemd160", "pgp-md2", "pgp-tiger192", and "pgp-haval-5-160".

Second is what GPGME (1.1.4) returns and what is put into micalg by mutt (1.5.15) in crypt-gpgme.c in row 759--780. This is in gpgme.c in rows 535--582:

const char *
gpgme_hash_algo_name (gpgme_hash_algo_t algo)
{
  switch (algo)
    {
    case GPGME_MD_MD5:
      return "MD5";

    case GPGME_MD_SHA1:
      return "SHA1";

    case GPGME_MD_RMD160:
      return "RIPEMD160";

    case GPGME_MD_MD2:
      return "MD2";

    case GPGME_MD_TIGER:
      return "TIGER192";

    case GPGME_MD_HAVAL:
      return "HAVAL";

    case GPGME_MD_SHA256:
      return "SHA256";

    case GPGME_MD_SHA384:
      return "SHA384";

    case GPGME_MD_SHA512:
      return "SHA512";

    case GPGME_MD_MD4:
      return "MD4";

    case GPGME_MD_CRC32:
      return "CRC32";

    case GPGME_MD_CRC32_RFC1510:
      return "CRC32RFC1510";

    case GPGME_MD_CRC24_RFC2440:
      return "CRC24RFC2440";

    default:
      return NULL;
    }
}

This should be somehow translated.

[MrTux]

The following patch solves the problem:

Index: crypt-gpgme.c
===================================================================
RCS file: /home/roessler/cvs/mutt/crypt-gpgme.c,v
retrieving revision 3.11
diff -u -r3.11 crypt-gpgme.c
--- crypt-gpgme.c	24 Feb 2007 06:12:20 -0000	3.11
+++ crypt-gpgme.c	5 Jun 2007 15:26:55 -0000
@@ -802,6 +802,7 @@
   char *sigfile;
   int err = 0;
   char buf[100];
+  int i;
   gpgme_ctx_t ctx;
   gpgme_data_t message, signature;
 
@@ -868,8 +869,18 @@
                       &t->parameter);
   /* Get the micalg from gpgme.  Old gpgme versions don't support this
      for S/MIME so we assume sha-1 in this case. */
-  if (!get_micalg (ctx, buf, sizeof buf))
-    mutt_set_parameter ("micalg", buf, &t->parameter);
+  if (!get_micalg (ctx, buf+4, sizeof buf - 4)) 
+    {
+      /*
+       * Convert result according to RFC3156 
+       */
+      for (i = 4; buf[i] && i < sizeof buf; i++) 
+      	buf[i] = tolower(buf[i]);
+       
+      strncpy(buf, "pgp-", 4);
+      
+      mutt_set_parameter ("micalg", buf, &t->parameter);
+    } 
   else if (use_smime)
     mutt_set_parameter ("micalg", "sha1", &t->parameter);
   gpgme_release (ctx);

[MrTux]

This patch fixes the problem.

[Paul Walker]

On Tue, Jun 05, 2007 at 03:38:45PM -0000, Mutt wrote:

>  +      for (i = 4; buf[i] && i < sizeof buf; i++)
>  +       buf[i] = tolower(buf[i]);

If I've understood it right, if sizeof(buf) < 4 that loop will dereference
off the end of the array before checking the boundary condition. Maybe
switch the checks around?

[MrTux]

Replying to Paul Walker:

If I've understood it right, if sizeof(buf) < 4 that loop will dereference off the end of the array before checking the boundary condition. Maybe switch the checks around?

Yes, would be better. Whoever applies the patch should change this.

But, in this case, I do not consider it critical; to create the situation you refer to, the source code must be altered in an unlikely way.

Having a buffer with less than 4 bytes will most probably yield other problems first. ;)

[Paul Walker]

On Tue, Jun 05, 2007 at 04:00:56PM -0000, Mutt wrote:

>  But, in this case, I do not consider it critical; to create the situation
>  you refer to, the source code must be altered in an unlikely way. Having
>  a buffer with less than 4 bytes will most probably yield other problems
>  first. ;)

Oh, I agree. Just thought it was worth fixing on-spec, rather than because
it would cause a real problem.

[Brendan Cully]

On Tuesday, 05 June 2007 at 15:38, Mutt wrote:
> #2901: wrong parameter micalg with mutt 1.5.15 and gpgme
> 
> Changes (by MrTux):
> 
>   * status:  new => closed
>   * resolution:  => fixed

You probably don't want to close the bug until the patch has actually
been applied. Otherwise there's a good chance the fix will get lost.

[MrTux]

Replying to Brendan Cully:

You probably don't want to close the bug until the patch has actually been applied. Otherwise there's a good chance the fix will get lost.

Well then....ticket reopened.

[Sertaç Ö. Yıldız]

[05.Haz.07 15:38 -0000] Mutt:
> The following patch solves the problem:
[…]
> +  if (!get_micalg (ctx, buf+4, sizeof buf - 4))
> +    {
> +      /*
> +       * Convert result according to RFC3156
> +       */
> +      for (i = 4; buf[i] && i < sizeof buf; i++)
> +       buf[i] = tolower(buf[i]);

tolower() is not locale safe so this might not give what you intend to 
get on some (Turkish) locales. ascii_tolower() might be more 
appropriate.

[MrTux]

Improved patch including the suggestions from the discussion.

[MrTux]

Replying to Sertaç Ö. Yıldız:

tolower() is not locale safe so this might not give what you intend to get on some (Turkish) locales. ascii_tolower() might be more appropriate.

Thanks for the suggestion. I am not that familar with mutt, so I did not know about function.

Patch version 4 (second attachment) uses ascii_tolower and follows the hint given by Paul.

[LeSpocky]

Replying to MrTux:

Thanks for the suggestion. I am not that familar with mutt, so I did not know about function. Patch version 4 (second attachment) uses ascii_tolower and follows the hint given by Paul.

I tested this with the configuration from initial request and it works fine for me with a patched 1.5.15. Thunderbird/Enigmail now correctly verifies signatures made with mutt/gpgme. :-)

[brendan]

(In [e6f958b093b6]) Make GPGME backend generate a RFC3156-compliant micalg parameter (blush). Based on a patch by Stefan Haun. Closes #2901.