DKIM: fix signing bug induced by total size of parameter text
[exim.git] / src / src / pdkim / pdkim.c
index f26b745af31a84af274579eaeb18282526d49ef0..2ce0ff6c9be4c193b6f2a50ea854b389e4b1c48f 100644 (file)
@@ -132,6 +132,7 @@ switch(ext_status)
   {
   case PDKIM_VERIFY_FAIL_BODY: return "PDKIM_VERIFY_FAIL_BODY";
   case PDKIM_VERIFY_FAIL_MESSAGE: return "PDKIM_VERIFY_FAIL_MESSAGE";
+  case PDKIM_VERIFY_FAIL_SIG_ALGO_MISMATCH: return "PDKIM_VERIFY_FAIL_SIG_ALGO_MISMATCH";
   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_DNSRECORD: return "PDKIM_VERIFY_INVALID_PUBKEY_DNSRECORD";
@@ -142,20 +143,20 @@ switch(ext_status)
   }
 }
 
-const char *
+const uschar *
 pdkim_errstr(int status)
 {
 switch(status)
   {
-  case PDKIM_OK:               return "OK";
-  case PDKIM_FAIL:             return "FAIL";
-  case PDKIM_ERR_RSA_PRIVKEY:  return "RSA_PRIVKEY";
-  case PDKIM_ERR_RSA_SIGNING:  return "RSA SIGNING";
-  case PDKIM_ERR_LONG_LINE:    return "RSA_LONG_LINE";
-  case PDKIM_ERR_BUFFER_TOO_SMALL:     return "BUFFER_TOO_SMALL";
-  case PDKIM_SIGN_PRIVKEY_WRAP:        return "PRIVKEY_WRAP";
-  case PDKIM_SIGN_PRIVKEY_B64D:        return "PRIVKEY_B64D";
-  default: return "(unknown)";
+  case PDKIM_OK:               return US"OK";
+  case PDKIM_FAIL:             return US"FAIL";
+  case PDKIM_ERR_RSA_PRIVKEY:  return US"RSA_PRIVKEY";
+  case PDKIM_ERR_RSA_SIGNING:  return US"RSA SIGNING";
+  case PDKIM_ERR_LONG_LINE:    return US"RSA_LONG_LINE";
+  case PDKIM_ERR_BUFFER_TOO_SMALL:     return US"BUFFER_TOO_SMALL";
+  case PDKIM_SIGN_PRIVKEY_WRAP:        return US"PRIVKEY_WRAP";
+  case PDKIM_SIGN_PRIVKEY_B64D:        return US"PRIVKEY_B64D";
+  default: return US"(unknown)";
   }
 }
 
@@ -288,7 +289,7 @@ found:
 /* Performs "relaxed" canonicalization of a header. */
 
 static uschar *
-pdkim_relax_header(const uschar * header, int crlf)
+pdkim_relax_header(const uschar * header, BOOL append_crlf)
 {
 BOOL past_field_name = FALSE;
 BOOL seen_wsp = FALSE;
@@ -299,8 +300,8 @@ uschar * q = relaxed;
 for (p = header; *p; p++)
   {
   uschar c = *p;
-  /* Ignore CR & LF */
-  if (c == '\r' || c == '\n')
+
+  if (c == '\r' || c == '\n')  /* Ignore CR & LF */
     continue;
   if (c == '\t' || c == ' ')
     {
@@ -312,8 +313,8 @@ for (p = header; *p; p++)
   else
     if (!past_field_name && c == ':')
       {
-      if (seen_wsp) q--;       /* This removes WSP before the colon */
-      seen_wsp = TRUE;         /* This removes WSP after the colon */
+      if (seen_wsp) q--;       /* This removes WSP immediately before the colon */
+      seen_wsp = TRUE;         /* This removes WSP immediately after the colon */
       past_field_name = TRUE;
       }
     else
@@ -326,7 +327,7 @@ for (p = header; *p; p++)
 
 if (q > relaxed && q[-1] == ' ') q--; /* Squash eventual trailing SP */
 
-if (crlf) { *q++ = '\r'; *q++ = '\n'; }
+if (append_crlf) { *q++ = '\r'; *q++ = '\n'; }
 *q = '\0';
 return relaxed;
 }
@@ -335,10 +336,10 @@ return relaxed;
 /* -------------------------------------------------------------------------- */
 #define PDKIM_QP_ERROR_DECODE -1
 
-static uschar *
-pdkim_decode_qp_char(uschar *qp_p, int *c)
+static const uschar *
+pdkim_decode_qp_char(const uschar *qp_p, int *c)
 {
-uschar *initial_pos = qp_p;
+const uschar *initial_pos = qp_p;
 
 /* Advance one char */
 qp_p++;
@@ -361,11 +362,11 @@ return initial_pos;
 /* -------------------------------------------------------------------------- */
 
 static uschar *
-pdkim_decode_qp(uschar * str)
+pdkim_decode_qp(const uschar * str)
 {
 int nchar = 0;
 uschar * q;
-uschar * p = str;
+const uschar * p = str;
 uschar * n = store_get(Ustrlen(str)+1);
 
 *n = '\0';
@@ -393,7 +394,7 @@ return n;
 /* -------------------------------------------------------------------------- */
 
 static void
-pdkim_decode_base64(uschar *str, blob * b)
+pdkim_decode_base64(const uschar * str, blob * b)
 {
 int dlen;
 dlen = b64decode(str, &b->data);
@@ -597,103 +598,58 @@ return sig;
 static pdkim_pubkey *
 pdkim_parse_pubkey_record(pdkim_ctx *ctx, const uschar *raw_record)
 {
-pdkim_pubkey *pub;
-const uschar *p;
-uschar * cur_tag = NULL; int ts = 0, tl = 0;
-uschar * cur_val = NULL; int vs = 0, vl = 0;
-int where = PDKIM_HDR_LIMBO;
+const uschar * ele;
+int sep = ';';
+pdkim_pubkey * pub;
 
 pub = store_get(sizeof(pdkim_pubkey));
 memset(pub, 0, sizeof(pdkim_pubkey));
 
-for (p = raw_record; ; p++)
+while ((ele = string_nextinlist(&raw_record, &sep, NULL, 0)))
+  {
+  const uschar * val;
+
+  if ((val = Ustrchr(ele, '=')))
     {
-    uschar c = *p;
+    int taglen = val++ - ele;
 
-    /* Ignore FWS */
-    if (c != '\r' && c != '\n') switch (where)
+    DEBUG(D_acl) debug_printf(" %.*s=%s\n", taglen, ele, val);
+    switch (ele[0])
       {
-      case PDKIM_HDR_LIMBO:            /* In limbo, just wait for a tag-char to appear */
-       if (!(c >= 'a' && c <= 'z'))
-         break;
-       where = PDKIM_HDR_TAG;
-       /*FALLTHROUGH*/
-
-      case PDKIM_HDR_TAG:
-       if (c >= 'a' && c <= 'z')
-         cur_tag = string_catn(cur_tag, &ts, &tl, p, 1);
-
-       if (c == '=')
-         {
-         cur_tag[tl] = '\0';
-         where = PDKIM_HDR_VALUE;
-         }
-       break;
-
-      case PDKIM_HDR_VALUE:
-       if (c == ';' || c == '\0')
-         {
-         if (tl && vl)
-           {
-           cur_val[vl] = '\0';
-           pdkim_strtrim(cur_val);
-           DEBUG(D_acl) debug_printf(" %s=%s\n", cur_tag, cur_val);
-
-           switch (cur_tag[0])
-             {
-             case 'v':
-               pub->version = string_copy(cur_val); break;
-             case 'h':
-             case 'k':
-/* This field appears to never be used. Also, unclear why
-a 'k' (key-type_ would go in this field name.  There is a field
-"keytype", also never used.
-               pub->hashes = string_copy(cur_val);
-*/
-               break;
-             case 'g':
-               pub->granularity = string_copy(cur_val); break;
-             case 'n':
-               pub->notes = pdkim_decode_qp(cur_val); break;
-             case 'p':
-               pdkim_decode_base64(US cur_val, &pub->key); break;
-             case 's':
-               pub->srvtype = string_copy(cur_val); break;
-             case 't':
-               if (Ustrchr(cur_val, 'y') != NULL) pub->testing = 1;
-               if (Ustrchr(cur_val, 's') != NULL) pub->no_subdomaining = 1;
-               break;
-             default:
-               DEBUG(D_acl) debug_printf(" Unknown tag encountered\n");
+      case 'v': pub->version = val;                    break;
+      case 'h': pub->hashes = val;                     break;
+      case 'k': break;
+      case 'g': pub->granularity = val;                        break;
+      case 'n': pub->notes = pdkim_decode_qp(val);     break;
+      case 'p': pdkim_decode_base64(val, &pub->key);   break;
+      case 's': pub->srvtype = val;                    break;
+      case 't': if (Ustrchr(val, 'y')) pub->testing = 1;
+               if (Ustrchr(val, 's')) pub->no_subdomaining = 1;
                break;
-             }
-           }
-         tl = 0;
-         vl = 0;
-         where = PDKIM_HDR_LIMBO;
-         }
-       else
-         cur_val = string_catn(cur_val, &vs, &vl, p, 1);
-       break;
+      default:  DEBUG(D_acl) debug_printf(" Unknown tag encountered\n"); break;
       }
-
-    if (c == '\0') break;
     }
+  }
 
 /* Set fallback defaults */
 if (!pub->version    ) pub->version     = string_copy(PDKIM_PUB_RECORD_VERSION);
-else if (Ustrcmp(pub->version, PDKIM_PUB_RECORD_VERSION) != 0) return NULL;
+else if (Ustrcmp(pub->version, PDKIM_PUB_RECORD_VERSION) != 0)
+  {
+  DEBUG(D_acl) debug_printf(" Bad v= field\n");
+  return NULL;
+  }
 
-if (!pub->granularity) pub->granularity = string_copy(US"*");
+if (!pub->granularity) pub->granularity = US"*";
 /*
-if (!pub->keytype    ) pub->keytype     = string_copy(US"rsa");
+if (!pub->keytype    ) pub->keytype     = US"rsa";
 */
-if (!pub->srvtype    ) pub->srvtype     = string_copy(US"*");
+if (!pub->srvtype    ) pub->srvtype     = US"*";
 
 /* p= is required */
 if (pub->key.data)
   return pub;
 
+DEBUG(D_acl) debug_printf(" Missing p= field\n");
 return NULL;
 }
 
@@ -999,7 +955,7 @@ return PDKIM_OK;
 #define HEADER_BUFFER_FRAG_SIZE 256
 
 DLLEXPORT int
-pdkim_feed(pdkim_ctx *ctx, char *data, int len)
+pdkim_feed(pdkim_ctx * ctx, uschar * data, int len)
 {
 int p, rc;
 
@@ -1297,11 +1253,23 @@ if (sig->bodylength >= 0)
   }
 
 /* Preliminary or final version? */
-base64_b = final ? pdkim_encode_base64(&sig->sighash) : US"";
-hdr = pdkim_headcat(&col, hdr, &hdr_size, &hdr_len, US";", US"b=", base64_b);
+if (final)
+  {
+  base64_b = pdkim_encode_base64(&sig->sighash);
+  hdr = pdkim_headcat(&col, hdr, &hdr_size, &hdr_len, US";", US"b=", base64_b);
 
-/* add trailing semicolon: I'm not sure if this is actually needed */
-hdr = pdkim_headcat(&col, hdr, &hdr_size, &hdr_len, NULL, US";", US"");
+  /* add trailing semicolon: I'm not sure if this is actually needed */
+  hdr = pdkim_headcat(&col, hdr, &hdr_size, &hdr_len, NULL, US";", US"");
+  }
+else
+  {
+  /* To satisfy the rule "all surrounding whitespace [...] deleted"
+  ( RFC 6376 section 3.7 ) we ensure there is no whitespace here.  Otherwise
+  the headcat routine could insert a linebreak which the relaxer would reduce
+  to a single space preceding the terminating semicolon, resulting in an
+  incorrect header-hash. */
+  hdr = pdkim_headcat(&col, hdr, &hdr_size, &hdr_len, US";", US"b=;", US"");
+  }
 
 hdr[hdr_len] = '\0';
 return hdr;
@@ -1311,11 +1279,11 @@ return hdr;
 /* -------------------------------------------------------------------------- */
 
 static pdkim_pubkey *
-pdkim_key_from_dns(pdkim_ctx * ctx, pdkim_signature * sig, ev_ctx * vctx)
+pdkim_key_from_dns(pdkim_ctx * ctx, pdkim_signature * sig, ev_ctx * vctx,
+  const uschar ** errstr)
 {
 uschar * dns_txt_name, * dns_txt_reply;
 pdkim_pubkey * p;
-const uschar * errstr;
 
 /* Fetch public key for signing domain, from DNS */
 
@@ -1364,9 +1332,9 @@ DEBUG(D_acl) debug_printf(
       "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
 
 /* Import public key */
-if ((errstr = exim_rsa_verify_init(&p->key, vctx)))
+if ((*errstr = exim_rsa_verify_init(&p->key, vctx)))
   {
-  DEBUG(D_acl) debug_printf("verify_init: %s\n", errstr);
+  DEBUG(D_acl) debug_printf("verify_init: %s\n", *errstr);
   sig->verify_status =      PDKIM_VERIFY_INVALID;
   sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_IMPORT;
   return NULL;
@@ -1379,7 +1347,8 @@ return p;
 /* -------------------------------------------------------------------------- */
 
 DLLEXPORT int
-pdkim_feed_finish(pdkim_ctx *ctx, pdkim_signature **return_signatures)
+pdkim_feed_finish(pdkim_ctx * ctx, pdkim_signature ** return_signatures,
+  const uschar ** err)
 {
 pdkim_signature *sig = ctx->sig;
 
@@ -1418,7 +1387,7 @@ while (sig)
     }
 
   DEBUG(D_acl) debug_printf(
-      "PDKIM >> Header data for hash, canonicalized, in sequence >>>>>>>>>>>>>>\n");
+      "PDKIM >> Header data for hash, canonicalized, in sequence >>>>>>>>>>>>\n");
 
   /* SIGNING ---------------------------------------------------------------- */
   /* When signing, walk through our header list and add them to the hash. As we
@@ -1446,7 +1415,7 @@ while (sig)
                        p->value, (q - US p->value) + (p->next ? 1 : 0));
 
        rh = sig->canon_headers == PDKIM_CANON_RELAXED
-         ? pdkim_relax_header(p->value, 1) /* cook header for relaxed canon */
+         ? pdkim_relax_header(p->value, TRUE) /* cook header for relaxed canon */
          : string_copy(CUS p->value);      /* just copy it for simple canon */
 
        /* Feed header to the hash algorithm */
@@ -1507,7 +1476,7 @@ while (sig)
            /* cook header for relaxed canon, or just copy it for simple  */
 
            uschar * rh = sig->canon_headers == PDKIM_CANON_RELAXED
-             ? pdkim_relax_header(hdrs->value, 1)
+             ? pdkim_relax_header(hdrs->value, TRUE)
              : string_copy(CUS hdrs->value);
 
            /* Feed header to the hash algorithm */
@@ -1531,7 +1500,7 @@ while (sig)
 
   /* Relax header if necessary */
   if (sig->canon_headers == PDKIM_CANON_RELAXED)
-    sig_hdr = pdkim_relax_header(sig_hdr, 0);
+    sig_hdr = pdkim_relax_header(sig_hdr, FALSE);
 
   DEBUG(D_acl)
     {
@@ -1560,12 +1529,11 @@ while (sig)
   if (ctx->flags & PDKIM_MODE_SIGN)
     {
     es_ctx sctx;
-    const uschar * errstr;
 
     /* Import private key */
-    if ((errstr = exim_rsa_signing_init(US sig->rsa_privkey, &sctx)))
+    if ((*err = exim_rsa_signing_init(US sig->rsa_privkey, &sctx)))
       {
-      DEBUG(D_acl) debug_printf("signing_init: %s\n", errstr);
+      DEBUG(D_acl) debug_printf("signing_init: %s\n", *err);
       return PDKIM_ERR_RSA_PRIVKEY;
       }
 
@@ -1577,9 +1545,9 @@ while (sig)
     hdata = hhash;
 #endif
 
-    if ((errstr = exim_rsa_sign(&sctx, is_sha1, &hdata, &sig->sighash)))
+    if ((*err = exim_rsa_sign(&sctx, is_sha1, &hdata, &sig->sighash)))
       {
-      DEBUG(D_acl) debug_printf("signing: %s\n", errstr);
+      DEBUG(D_acl) debug_printf("signing: %s\n", *err);
       return PDKIM_ERR_RSA_SIGNING;
       }
 
@@ -1596,8 +1564,6 @@ while (sig)
   else
     {
     ev_ctx vctx;
-    const uschar * errstr;
-    pdkim_pubkey * p;
 
     /* Make sure we have all required signature tags */
     if (!(  sig->domain        && *sig->domain
@@ -1630,13 +1596,32 @@ while (sig)
       goto NEXT_VERIFY;
       }
 
-    if (!(sig->pubkey = pdkim_key_from_dns(ctx, sig, &vctx)))
+    if (!(sig->pubkey = pdkim_key_from_dns(ctx, sig, &vctx, err)))
       goto NEXT_VERIFY;
 
+    /* If the pubkey limits to a list of specific hashes, ignore sigs that
+    do not have the hash part of the sig algorithm matching */
+
+    if (sig->pubkey->hashes)
+      {
+      const uschar * list = sig->pubkey->hashes, * ele;
+      int sep = ':';
+      while ((ele = string_nextinlist(&list, &sep, NULL, 0)))
+       if (Ustrcmp(ele, pdkim_algos[sig->algo] + 4) == 0) break;
+      if (!ele)
+       {
+       DEBUG(D_acl) debug_printf("pubkey h=%s vs sig a=%s\n",
+                               sig->pubkey->hashes, pdkim_algos[sig->algo]);
+       sig->verify_status =      PDKIM_VERIFY_FAIL;
+       sig->verify_ext_status =  PDKIM_VERIFY_FAIL_SIG_ALGO_MISMATCH;
+       goto NEXT_VERIFY;
+       }
+      }
+
     /* Check the signature */
-    if ((errstr = exim_rsa_verify(&vctx, is_sha1, &hhash, &sig->sighash)))
+    if ((*err = exim_rsa_verify(&vctx, is_sha1, &hhash, &sig->sighash)))
       {
-      DEBUG(D_acl) debug_printf("headers verify: %s\n", errstr);
+      DEBUG(D_acl) debug_printf("headers verify: %s\n", *err);
       sig->verify_status =      PDKIM_VERIFY_FAIL;
       sig->verify_ext_status =  PDKIM_VERIFY_FAIL_MESSAGE;
       goto NEXT_VERIFY;
@@ -1694,7 +1679,8 @@ return ctx;
 
 DLLEXPORT pdkim_ctx *
 pdkim_init_sign(char * domain, char * selector, char * rsa_privkey, int algo,
-  BOOL dot_stuffed, int(*dns_txt_callback)(char *, char *))
+  BOOL dot_stuffed, int(*dns_txt_callback)(char *, char *),
+  const uschar ** errstr)
 {
 pdkim_ctx * ctx;
 pdkim_signature * sig;
@@ -1733,10 +1719,10 @@ DEBUG(D_acl)
   pdkim_signature s = *sig;
   ev_ctx vctx;
 
-  debug_printf("PDKIM (checking verify key)<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
-  if (!pdkim_key_from_dns(ctx, &s, &vctx))
+  debug_printf("PDKIM (checking verify key)>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
+  if (!pdkim_key_from_dns(ctx, &s, &vctx, errstr))
     debug_printf("WARNING: bad dkim key in dns\n");
-  debug_printf("PDKIM (finished checking verify key)<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
+  debug_printf("PDKIM (finished checking verify key)<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
   }
 return ctx;
 }