tidying

[exim.git] / src / src / pdkim / pdkim.c

diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index 36e559b59db292478da38104006b79c5a7037657..b2caa81abcf8025e47bda0efdae8e3ec78ff8a36 100644 (file)
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -1,7 +1,7 @@
 /*
  *  PDKIM - a RFC4871 (DKIM) implementation
  *
 /*
  *  PDKIM - a RFC4871 (DKIM) implementation
  *

- *  Copyright (c) The Exim Maintainers 2021 - 2023
+ *  Copyright (c) The Exim Maintainers 2021 - 2024

  *  Copyright (C) 2016 - 2020  Jeremy Harris <jgh@exim.org>
  *  Copyright (C) 2009 - 2016  Tom Kistner <tom@duncanthrax.net>
  *  SPDX-License-Identifier: GPL-2.0-or-later
  *  Copyright (C) 2016 - 2020  Jeremy Harris <jgh@exim.org>
  *  Copyright (C) 2009 - 2016  Tom Kistner <tom@duncanthrax.net>
  *  SPDX-License-Identifier: GPL-2.0-or-later


@@ -468,15 +468,12 @@ return b64encode(CUS b->data, b->len);
 static pdkim_signature *
 pdkim_parse_sig_header(pdkim_ctx * ctx, uschar * raw_hdr)
 {
 static pdkim_signature *
 pdkim_parse_sig_header(pdkim_ctx * ctx, uschar * raw_hdr)
 {

-pdkim_signature * sig;
-uschar *q;
-gstring * cur_tag = NULL;
-gstring * cur_val = NULL;
-BOOL past_hname = FALSE;
-BOOL in_b_val = FALSE;
+pdkim_signature * sig = store_get(sizeof(pdkim_signature), GET_UNTAINTED);
+uschar * q;
+gstring * cur_tag = NULL, * cur_val = NULL;
+BOOL past_hname = FALSE, in_b_val = FALSE;

 int where = PDKIM_HDR_LIMBO;
 
 int where = PDKIM_HDR_LIMBO;
 

-sig = store_get(sizeof(pdkim_signature), GET_UNTAINTED);

 memset(sig, 0, sizeof(pdkim_signature));
 sig->bodylength = -1;
 
 memset(sig, 0, sizeof(pdkim_signature));
 sig->bodylength = -1;
 


@@ -512,10 +509,6 @@ for (uschar * p = raw_hdr; ; p++)
     }
 
   if (where == PDKIM_HDR_TAG)
     }
 
   if (where == PDKIM_HDR_TAG)

-    {
-    if (c >= 'a' && c <= 'z')
-      cur_tag = string_catn(cur_tag, p, 1);
-

     if (c == '=')
       {
       if (Ustrcmp(string_from_gstring(cur_tag), "b") == 0)
     if (c == '=')
       {
       if (Ustrcmp(string_from_gstring(cur_tag), "b") == 0)


@@ -526,7 +519,8 @@ for (uschar * p = raw_hdr; ; p++)
       where = PDKIM_HDR_VALUE;
       goto NEXT_CHAR;
       }
       where = PDKIM_HDR_VALUE;
       goto NEXT_CHAR;
       }

-    }
+    else if (!isspace(c))
+      cur_tag = string_catn(cur_tag, p, 1);

 
   if (where == PDKIM_HDR_VALUE)
     {
 
   if (where == PDKIM_HDR_VALUE)
     {


@@ -553,17 +547,17 @@ for (uschar * p = raw_hdr; ; p++)
            switch (cur_tag->s[1])
              {
              case '\0': pdkim_decode_base64(cur_val->s, &sig->sighash); break;
            switch (cur_tag->s[1])
              {
              case '\0': pdkim_decode_base64(cur_val->s, &sig->sighash); break;

-             case 'h':  if (cur_tag->ptr == 2)
-                          pdkim_decode_base64(cur_val->s, &sig->bodyhash);
+             case 'h':  if (cur_tag->ptr != 2) goto bad_tag;
+                        pdkim_decode_base64(cur_val->s, &sig->bodyhash);

                         break;
                         break;

-             default:   break;
+             default:   goto bad_tag;

              }
            break;
          case 'v':                                     /* version */
              /* We only support version 1, and that is currently the
                 only version there is. */
            sig->version =
              }
            break;
          case 'v':                                     /* version */
              /* We only support version 1, and that is currently the
                 only version there is. */
            sig->version =

-             Ustrcmp(cur_val->s, PDKIM_SIGNATURE_VERSION) == 0 ? 1 : -1;
+               Ustrcmp(cur_val->s, PDKIM_SIGNATURE_VERSION) == 0 ? 1 : -1;

            break;
          case 'a':                                     /* algorithm */
            {
            break;
          case 'a':                                     /* algorithm */
            {


@@ -578,6 +572,7 @@ for (uschar * p = raw_hdr; ; p++)
                if (Ustrcmp(elem, pdkim_hashes[i].dkim_hashname) == 0)
                  { sig->hashtype = i; break; }
            }
                if (Ustrcmp(elem, pdkim_hashes[i].dkim_hashname) == 0)
                  { sig->hashtype = i; break; }
            }

+           break;

 
          case 'c':                                     /* canonicalization */
            pdkim_cstring_to_canons(cur_val->s, 0,
 
          case 'c':                                     /* canonicalization */
            pdkim_cstring_to_canons(cur_val->s, 0,


@@ -586,15 +581,15 @@ for (uschar * p = raw_hdr; ; p++)
          case 'q':                             /* Query method (for pubkey)*/
            for (int i = 0; pdkim_querymethods[i]; i++)
              if (Ustrcmp(cur_val->s, pdkim_querymethods[i]) == 0)
          case 'q':                             /* Query method (for pubkey)*/
            for (int i = 0; pdkim_querymethods[i]; i++)
              if (Ustrcmp(cur_val->s, pdkim_querymethods[i]) == 0)

-               {
+               {

                sig->querymethod = i;   /* we never actually use this */
                break;
                }
            break;
          case 's':                                     /* Selector */
                sig->querymethod = i;   /* we never actually use this */
                break;
                }
            break;
          case 's':                                     /* Selector */

-           sig->selector = string_copyn(cur_val->s, cur_val->ptr); break;
+           sig->selector = string_copy_from_gstring(cur_val); break;

          case 'd':                                     /* SDID */
          case 'd':                                     /* SDID */

-           sig->domain = string_copyn(cur_val->s, cur_val->ptr); break;
+           sig->domain = string_copy_from_gstring(cur_val); break;

          case 'i':                                     /* AUID */
            sig->identity = pdkim_decode_qp(cur_val->s); break;
          case 't':                                     /* Timestamp */
          case 'i':                                     /* AUID */
            sig->identity = pdkim_decode_qp(cur_val->s); break;
          case 't':                                     /* Timestamp */


@@ -604,16 +599,18 @@ for (uschar * p = raw_hdr; ; p++)
          case 'l':                                     /* Body length count */
            sig->bodylength = strtol(CS cur_val->s, NULL, 10); break;
          case 'h':                                     /* signed header fields */
          case 'l':                                     /* Body length count */
            sig->bodylength = strtol(CS cur_val->s, NULL, 10); break;
          case 'h':                                     /* signed header fields */

-           sig->headernames = string_copyn(cur_val->s, cur_val->ptr); break;
+           sig->headernames = string_copy_from_gstring(cur_val); break;

          case 'z':                                     /* Copied headfields */
            sig->copiedheaders = pdkim_decode_qp(cur_val->s); break;
 /*XXX draft-ietf-dcrup-dkim-crypto-05 would need 'p' tag support
 for rsafp signatures.  But later discussion is dropping those. */
          default:
          case 'z':                                     /* Copied headfields */
            sig->copiedheaders = pdkim_decode_qp(cur_val->s); break;
 /*XXX draft-ietf-dcrup-dkim-crypto-05 would need 'p' tag support
 for rsafp signatures.  But later discussion is dropping those. */
          default:

-           DEBUG(D_acl) debug_printf(" Unknown tag encountered\n");
-           break;
+           goto bad_tag;

          }
        }
          }
        }

+       else
+bad_tag:  DEBUG(D_acl) debug_printf(" Unknown tag encountered: %Y\n", cur_tag);
+

       cur_tag = cur_val = NULL;
       in_b_val = FALSE;
       where = PDKIM_HDR_LIMBO;
       cur_tag = cur_val = NULL;
       in_b_val = FALSE;
       where = PDKIM_HDR_LIMBO;


@@ -659,25 +656,36 @@ return sig;
 /* -------------------------------------------------------------------------- */
 
 pdkim_pubkey *
 /* -------------------------------------------------------------------------- */
 
 pdkim_pubkey *

-pdkim_parse_pubkey_record(const uschar *raw_record)
+pdkim_parse_pubkey_record(const uschar * raw_record)

 {
 {

-const uschar * ele;
-int sep = ';';
-pdkim_pubkey * pub;
+pdkim_pubkey * pub = store_get(sizeof(pdkim_pubkey), GET_TAINTED);

 
 

-pub = store_get(sizeof(pdkim_pubkey), GET_TAINTED);

 memset(pub, 0, sizeof(pdkim_pubkey));
 
 memset(pub, 0, sizeof(pdkim_pubkey));
 

-while ((ele = string_nextinlist(&raw_record, &sep, NULL, 0)))
+for (const uschar * ele = raw_record, * tspec, * end, * val; *ele; ele = end)

   {
   {

-  const uschar * val;
+  Uskip_whitespace(&ele);
+  end = US strchrnul(CS ele, ';');
+  tspec = string_copyn(ele, end - ele);
+  if (*end) end++;                     /* skip the ; */

 
 

-  if ((val = Ustrchr(ele, '=')))
+  if ((val = Ustrchr(tspec, '=')))

     {
     {

-    int taglen = val++ - ele;
+    int taglen = val++ - tspec;
+
+    DEBUG(D_acl) debug_printf(" %.*s=%s\n", taglen, tspec, val);
+    while (taglen > 1 && isspace(tspec[taglen-1]))
+      taglen--;                        /* Ignore whitespace before = */
+    Uskip_whitespace(&val);    /* Ignore whitespace after = */
+    if (isspace(val[ Ustrlen(val)-1 ]))
+      {                                /* Ignore whitespace after value */
+      gstring * g = string_cat(NULL, val);
+      while (isspace(gstring_last_char(g)))
+       gstring_trim(g, 1);
+      val = string_from_gstring(g);
+      }

 
 

-    DEBUG(D_acl) debug_printf(" %.*s=%s\n", taglen, ele, val);
-    switch (ele[0])
+    if (taglen == 1) switch (tspec[0])

       {
       case 'v': pub->version = val;                    break;
       case 'h': pub->hashes = val;                     break;
       {
       case 'v': pub->version = val;                    break;
       case 'h': pub->hashes = val;                     break;


@@ -689,8 +697,11 @@ while ((ele = string_nextinlist(&raw_record, &sep, NULL, 0)))
       case 't': if (Ustrchr(val, 'y')) pub->testing = 1;
                if (Ustrchr(val, 's')) pub->no_subdomaining = 1;
                break;
       case 't': if (Ustrchr(val, 'y')) pub->testing = 1;
                if (Ustrchr(val, 's')) pub->no_subdomaining = 1;
                break;

-      default:  DEBUG(D_acl) debug_printf(" Unknown tag encountered\n"); break;
+      default:  goto bad_tag;

       }
       }

+    else
+bad_tag:
+       DEBUG(D_acl) debug_printf(" Unknown tag encountered\n");

     }
   }
 
     }
   }
 


@@ -1868,9 +1879,9 @@ for (pdkim_signature * sig = ctx->sig; sig; sig = sig->next)
     if (*dkim_verify_min_keysizes)
       {
       unsigned minbits;
     if (*dkim_verify_min_keysizes)
       {
       unsigned minbits;

-      uschar * ss = expand_getkeyed(US pdkim_keytypes[sig->keytype],
+      const uschar * ss = expand_getkeyed(US pdkim_keytypes[sig->keytype],

                                    dkim_verify_min_keysizes);
                                    dkim_verify_min_keysizes);

-      if (ss &&  (minbits = atoi(CS ss)) > sig->keybits)
+      if (ss &&  (minbits = atoi(CCS ss)) > sig->keybits)

        {
        DEBUG(D_acl) debug_printf("Key too short: Actual: %s %u  Minima '%s'\n",
          pdkim_keytypes[sig->keytype], sig->keybits, dkim_verify_min_keysizes);
        {
        DEBUG(D_acl) debug_printf("Key too short: Actual: %s %u  Minima '%s'\n",
          pdkim_keytypes[sig->keytype], sig->keybits, dkim_verify_min_keysizes);


@@ -1885,11 +1896,17 @@ for (pdkim_signature * sig = ctx->sig; sig; sig = sig->next)
       {
       sig->verify_status = PDKIM_VERIFY_PASS;
       verify_pass = TRUE;
       {
       sig->verify_status = PDKIM_VERIFY_PASS;
       verify_pass = TRUE;

-      if (dkim_verify_minimal) break;
+      /*XXX We used to "break" here if dkim_verify_minimal, but that didn't
+      stop the ACL being called.  So move that test.  Unfortunately, we
+      need to eval all the sigs here only to possibly ignore some later,
+      because we don't know what verify options might say.
+      Could we change to a later eval of the sig?
+      Both bits are called from receive_msg().
+      Moving the test is also suboptimal for the case of no ACL (or no
+      signers to check!) so keep it for that case, but after debug output */

       }
 
 NEXT_VERIFY:
       }
 
 NEXT_VERIFY:

-

     DEBUG(D_acl)
       {
       debug_printf("DKIM [%s] %s signature status: %s",
     DEBUG(D_acl)
       {
       debug_printf("DKIM [%s] %s signature status: %s",


@@ -1901,6 +1918,10 @@ NEXT_VERIFY:
       else
        debug_printf("\n");
       }
       else
        debug_printf("\n");
       }

+
+    if (  verify_pass && dkim_verify_minimal
+       && !(acl_smtp_dkim && dkim_verify_signers && *dkim_verify_signers))
+      break;

     }
   }
 
     }
   }