PDKIM: Fix use of private-keys having trailing '=' in the base-64. Bug 1781
authorJeremy Harris <jgh146exb@wizmail.org>
Fri, 22 Jan 2016 13:17:34 +0000 (13:17 +0000)
committerJeremy Harris <jgh146exb@wizmail.org>
Fri, 22 Jan 2016 13:17:34 +0000 (13:17 +0000)
doc/doc-txt/ChangeLog
src/src/base64.c
src/src/dkim.c
src/src/pdkim/pdkim.c
src/src/pdkim/pdkim.h

index c4a2f00..1c862b5 100644 (file)
@@ -164,6 +164,9 @@ JH/38 Fix cutthrough bug with body lines having a single dot. The dot was
       received - but deduplicating mailstores were liable to retain only the
       initial truncated version.
 
+JH/39 Bug 1781: Fix use of private-keys having trailing '=' in the base-64.
+
+
 
 Exim version 4.86
 -----------------
index ca6466b..031beb9 100644 (file)
@@ -129,7 +129,7 @@ compact loop is messy and would probably run more slowly.
 Arguments:
   code        points to the coded string, zero-terminated
   ptr         where to put the pointer to the result, which is in
-              dynamic store, and zero-terminated
+              allocated store, and zero-terminated
 
 Returns:      the number of bytes in the result,
               or -1 if the input was malformed
@@ -163,35 +163,50 @@ quantum this may decode to 1, 2, or 3 output bytes. */
 while ((x = *code++) != 0)
   {
   if (isspace(x)) continue;
+  /* debug_printf("b64d: '%c'\n", x); */
 
   if (x > 127 || (x = dec64table[x]) == 255) return -1;
 
   while (isspace(y = *code++)) ;
+  /* debug_printf("b64d: '%c'\n", y); */
   if (y == 0 || (y = dec64table[y]) == 255)
     return -1;
 
   *result++ = (x << 2) | (y >> 4);
+  /* debug_printf("b64d:      -> %02x\n", result[-1]); */
 
   while (isspace(x = *code++)) ;
-  if (x == '=')
+  /* debug_printf("b64d: '%c'\n", x); */
+  if (x == '=')                /* endmarker, but there should be another */
     {
     while (isspace(x = *code++)) ;
-    if (x != '=' || *code != 0) return -1;
+    /* debug_printf("b64d: '%c'\n", x); */
+    if (x != '=') return -1;
+    while (isspace(y = *code++)) ;
+    if (y != 0) return -1;
+    /* debug_printf("b64d: DONE\n"); */
+    break;
     }
   else
     {
     if (x > 127 || (x = dec64table[x]) == 255) return -1;
     *result++ = (y << 4) | (x >> 2);
+    /* debug_printf("b64d:      -> %02x\n", result[-1]); */
 
     while (isspace(y = *code++)) ;
+    /* debug_printf("b64d: '%c'\n", y); */
     if (y == '=')
       {
-      if (*code != 0) return -1;
+      while (isspace(y = *code++)) ;
+      if (y != 0) return -1;
+      /* debug_printf("b64d: DONE\n"); */
+      break;
       }
     else
       {
       if (y > 127 || (y = dec64table[y]) == 255) return -1;
       *result++ = (x << 6) | y;
+      /* debug_printf("b64d:      -> %02x\n", result[-1]); */
       }
     }
   }
@@ -202,23 +217,6 @@ return result - *ptr;
 
 
 /*************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************
- *************************************************/
-
-/*************************************************
 *          Encode byte-string in base 64         *
 *************************************************/
 
index 33d1e97..36b103e 100644 (file)
@@ -158,7 +158,8 @@ for (sig = dkim_signatures; sig; sig = sig->next)
                        "overlong public key record]");
          break;
 
-       case PDKIM_VERIFY_INVALID_PUBKEY_PARSING:
+       case PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD:
+       case PDKIM_VERIFY_INVALID_PUBKEY_IMPORT:
          logmsg = string_append(logmsg, &size, &ptr, 1,
                       "syntax error in public key record]");
          break;
@@ -395,7 +396,8 @@ switch (what)
       {
       case PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE:
                                                return US"pubkey_unavailable";
-      case PDKIM_VERIFY_INVALID_PUBKEY_PARSING:        return US"pubkey_syntax";
+      case PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD:return US"pubkey_dns_syntax";
+      case PDKIM_VERIFY_INVALID_PUBKEY_IMPORT: return US"pubkey_der_syntax";
       case PDKIM_VERIFY_FAIL_BODY:             return US"bodyhash_mismatch";
       case PDKIM_VERIFY_FAIL_MESSAGE:          return US"signature_incorrect";
       }
index a47b694..6e471a6 100644 (file)
@@ -146,7 +146,8 @@ pdkim_verify_ext_status_str(int ext_status)
     case PDKIM_VERIFY_FAIL_MESSAGE: return "PDKIM_VERIFY_FAIL_MESSAGE";
     case PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE: return "PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE";
     case PDKIM_VERIFY_INVALID_BUFFER_SIZE: return "PDKIM_VERIFY_INVALID_BUFFER_SIZE";
-    case PDKIM_VERIFY_INVALID_PUBKEY_PARSING: return "PDKIM_VERIFY_INVALID_PUBKEY_PARSING";
+    case PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD: return "PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD";
+    case PDKIM_VERIFY_INVALID_PUBKEY_IMPORT: return "PDKIM_VERIFY_INVALID_PUBKEY_IMPORT";
     default: return "PDKIM_VERIFY_UNKNOWN";
   }
 }
@@ -1881,13 +1882,23 @@ while (sig)
     if (  !(p = Ustrstr(sig->rsa_privkey, "-----BEGIN RSA PRIVATE KEY-----"))
        || !(q = Ustrstr(p+=31, "-----END RSA PRIVATE KEY-----"))
        )
-      return PDKIM_ERR_RSA_PRIVKEY;
+      return PDKIM_SIGN_PRIVKEY_WRAP;
     *q = '\0';
-    if (  (len = b64decode(p, &p)) < 0
-       || !(rsa = d2i_RSAPrivateKey(NULL, CUSS &p, len))
-       )
-      /*XXX todo: get errstring from library */
+    if ((len = b64decode(p, &p)) < 0)
+      {
+      DEBUG(D_acl) debug_printf("b64decode failed\n");
+      return PDKIM_SIGN_PRIVKEY_B64D;
+      }
+    if (!(rsa = d2i_RSAPrivateKey(NULL, CUSS &p, len)))
+      {
+      DEBUG(D_acl)
+       {
+       char ssl_errstring[256];
+       ERR_error_string(ERR_get_error(), ssl_errstring);
+       debug_printf("d2i_RSAPrivateKey: %s\n", ssl_errstring);
+       }
       return PDKIM_ERR_RSA_PRIVKEY;
+      }
 
 #elif defined(RSA_GNUTLS)
 
@@ -2013,7 +2024,7 @@ while (sig)
     if (!(sig->pubkey = pdkim_parse_pubkey_record(ctx, dns_txt_reply)))
       {
       sig->verify_status =      PDKIM_VERIFY_INVALID;
-      sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_PARSING;
+      sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD;
 
       DEBUG(D_acl) debug_printf(
          " Error while parsing public key record\n"
@@ -2052,7 +2063,7 @@ while (sig)
        }
 
       sig->verify_status =      PDKIM_VERIFY_INVALID;
-      sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_PARSING;
+      sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_IMPORT;
       goto NEXT_VERIFY;
       }
 
index 97a03ea..313afd5 100644 (file)
@@ -34,6 +34,8 @@
 #define PDKIM_ERR_RSA_SIGNING      -102
 #define PDKIM_ERR_LONG_LINE        -103
 #define PDKIM_ERR_BUFFER_TOO_SMALL -104
+#define PDKIM_SIGN_PRIVKEY_WRAP    -105
+#define PDKIM_SIGN_PRIVKEY_B64D    -106
 
 /* -------------------------------------------------------------------------- */
 /* Main/Extended verification status */
@@ -46,7 +48,8 @@
 #define PDKIM_VERIFY_FAIL_MESSAGE               2
 #define PDKIM_VERIFY_INVALID_PUBKEY_UNAVAILABLE 3
 #define PDKIM_VERIFY_INVALID_BUFFER_SIZE        4
-#define PDKIM_VERIFY_INVALID_PUBKEY_PARSING     5
+#define PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD   5
+#define PDKIM_VERIFY_INVALID_PUBKEY_IMPORT      6
 
 /* -------------------------------------------------------------------------- */
 /* Some parameter values */