DKIM: fix relaxed body verify for a newline-only body. Bug 963
[exim.git] / src / src / pdkim / pdkim.c
index fa5d88d599927fd11a41f78b90587c94ae804475..94328f7ee2f039350b5e1024672d74bc4b0e1175 100644 (file)
@@ -1,7 +1,7 @@
 /*
  *  PDKIM - a RFC4871 (DKIM) implementation
  *
- *  Copyright (C) 2009  Tom Kistner <tom@duncanthrax.net>
+ *  Copyright (C) 2009 - 2015  Tom Kistner <tom@duncanthrax.net>
  *
  *  http://duncanthrax.net/pdkim/
  *
  *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-/* $Cambridge: exim/src/src/pdkim/pdkim.c,v 1.14 2010/05/29 19:14:06 nm4 Exp $ */
-
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
 #include <ctype.h>
 
 #include "pdkim.h"
+#include "pdkim-rsa.h"
 
-#include "sha1.h"
-#include "sha2.h"
-#include "rsa.h"
-#include "base64.h"
+#include "polarssl/sha1.h"
+#include "polarssl/sha2.h"
+#include "polarssl/rsa.h"
+#include "polarssl/base64.h"
 
 #define PDKIM_SIGNATURE_VERSION     "1"
 #define PDKIM_PUB_RECORD_VERSION    "DKIM1"
@@ -53,6 +52,7 @@
 /* -------------------------------------------------------------------------- */
 struct pdkim_stringlist {
   char *value;
+  int  tag;
   void *next;
 };
 
@@ -65,32 +65,32 @@ struct pdkim_str {
 
 /* -------------------------------------------------------------------------- */
 /* A bunch of list constants */
-char *pdkim_querymethods[] = {
+const char *pdkim_querymethods[] = {
   "dns/txt",
   NULL
 };
-char *pdkim_algos[] = {
+const char *pdkim_algos[] = {
   "rsa-sha256",
   "rsa-sha1",
   NULL
 };
-char *pdkim_canons[] = {
+const char *pdkim_canons[] = {
   "simple",
   "relaxed",
   NULL
 };
-char *pdkim_hashes[] = {
+const char *pdkim_hashes[] = {
   "sha256",
   "sha1",
   NULL
 };
-char *pdkim_keytypes[] = {
+const char *pdkim_keytypes[] = {
   "rsa",
   NULL
 };
 
 typedef struct pdkim_combined_canon_entry {
-  char *str;
+  const char *str;
   int canon_headers;
   int canon_body;
 } pdkim_combined_canon_entry;
@@ -105,7 +105,7 @@ pdkim_combined_canon_entry pdkim_combined_canons[] = {
 };
 
 
-char *pdkim_verify_status_str(int status) {
+const char *pdkim_verify_status_str(int status) {
   switch(status) {
     case PDKIM_VERIFY_NONE:    return "PDKIM_VERIFY_NONE";
     case PDKIM_VERIFY_INVALID: return "PDKIM_VERIFY_INVALID";
@@ -114,7 +114,7 @@ char *pdkim_verify_status_str(int status) {
     default:                   return "PDKIM_VERIFY_UNKNOWN";
   }
 }
-char *pdkim_verify_ext_status_str(int ext_status) {
+const char *pdkim_verify_ext_status_str(int ext_status) {
   switch(ext_status) {
     case PDKIM_VERIFY_FAIL_BODY: return "PDKIM_VERIFY_FAIL_BODY";
     case PDKIM_VERIFY_FAIL_MESSAGE: return "PDKIM_VERIFY_FAIL_MESSAGE";
@@ -129,12 +129,12 @@ char *pdkim_verify_ext_status_str(int ext_status) {
 /* -------------------------------------------------------------------------- */
 /* Print debugging functions */
 #ifdef PDKIM_DEBUG
-void pdkim_quoteprint(FILE *stream, char *data, int len, int lf) {
+void pdkim_quoteprint(FILE *stream, const char *data, int len, int lf) {
   int i;
-  unsigned char *p = (unsigned char *)data;
+  const unsigned char *p = (const unsigned char *)data;
 
   for (i=0;i<len;i++) {
-    int c = p[i];
+    const int c = p[i];
     switch (c) {
       case ' ' : fprintf(stream,"{SP}"); break;
       case '\t': fprintf(stream,"{TB}"); break;
@@ -153,12 +153,12 @@ void pdkim_quoteprint(FILE *stream, char *data, int len, int lf) {
   if (lf)
     fputc('\n',stream);
 }
-void pdkim_hexprint(FILE *stream, char *data, int len, int lf) {
+void pdkim_hexprint(FILE *stream, const char *data, int len, int lf) {
   int i;
-  unsigned char *p = (unsigned char *)data;
+  const unsigned char *p = (const unsigned char *)data;
 
   for (i=0;i<len;i++) {
-    int c = p[i];
+    const int c = p[i];
     fprintf(stream,"%02x",c);
   }
   if (lf)
@@ -198,7 +198,7 @@ pdkim_stringlist *pdkim_prepend_stringlist(pdkim_stringlist *base, char *str) {
 
 /* -------------------------------------------------------------------------- */
 /* A small "growing string" implementation to escape malloc/realloc hell */
-pdkim_str *pdkim_strnew (char *cstr) {
+pdkim_str *pdkim_strnew (const char *cstr) {
   unsigned int len = cstr?strlen(cstr):0;
   pdkim_str *p = malloc(sizeof(pdkim_str));
   if (p == NULL) return NULL;
@@ -214,7 +214,7 @@ pdkim_str *pdkim_strnew (char *cstr) {
   else p->str[p->len] = '\0';
   return p;
 }
-char *pdkim_strncat(pdkim_str *str, char *data, int len) {
+char *pdkim_strncat(pdkim_str *str, const char *data, int len) {
   if ((str->allocated - str->len) < (len+1)) {
     /* Extend the buffer */
     int num_frags = ((len+1)/PDKIM_STR_ALLOC_FRAG)+1;
@@ -229,9 +229,10 @@ char *pdkim_strncat(pdkim_str *str, char *data, int len) {
   str->str[str->len] = '\0';
   return str->str;
 }
-char *pdkim_strcat(pdkim_str *str, char *cstr) {
+char *pdkim_strcat(pdkim_str *str, const char *cstr) {
   return pdkim_strncat(str, cstr, strlen(cstr));
 }
+
 char *pdkim_numcat(pdkim_str *str, unsigned long num) {
   char minibuf[20];
   snprintf(minibuf,20,"%lu",num);
@@ -303,7 +304,6 @@ void pdkim_free_sig(pdkim_signature *sig) {
     if (sig->signature_header != NULL) free(sig->signature_header);
     if (sig->sha1_body        != NULL) free(sig->sha1_body);
     if (sig->sha2_body        != NULL) free(sig->sha2_body);
-    if (sig->hnames_check     != NULL) free(sig->hnames_check);
 
     if (sig->pubkey != NULL) pdkim_free_pubkey(sig->pubkey);
 
@@ -316,6 +316,13 @@ void pdkim_free_sig(pdkim_signature *sig) {
 /* -------------------------------------------------------------------------- */
 DLLEXPORT void pdkim_free_ctx(pdkim_ctx *ctx) {
   if (ctx) {
+    pdkim_stringlist *e = ctx->headers;
+    while(e != NULL) {
+      pdkim_stringlist *c = e;
+      if (e->value != NULL) free(e->value);
+      e = e->next;
+      free(c);
+    }
     pdkim_free_sig(ctx->sig);
     pdkim_strfree(ctx->cur_header);
     free(ctx);
@@ -328,9 +335,9 @@ DLLEXPORT void pdkim_free_ctx(pdkim_ctx *ctx) {
    the passed colon-separated "list", starting at entry
    "start". Returns the position of the header name in
    the list. */
-int header_name_match(char *header,
-                      char *tick,
-                      int   do_tick) {
+int header_name_match(const char *header,
+                      char       *tick,
+                      int         do_tick) {
   char *hname;
   char *lcopy;
   char *p;
@@ -420,6 +427,7 @@ char *pdkim_relax_header (char *header, int crlf) {
     p++;
     q++;
   }
+  if ((q>relaxed) && (*(q-1) == ' ')) q--; /* Squash eventual trailing SP */
   *q = '\0';
   if (crlf) strcat(relaxed,"\r\n");
   return relaxed;
@@ -589,7 +597,7 @@ pdkim_signature *pdkim_parse_sig_header(pdkim_ctx *ctx, char *raw_hdr) {
           pdkim_strtrim(cur_val);
           #ifdef PDKIM_DEBUG
           if (ctx->debug_stream)
-            fprintf(ctx->debug_stream, "%s=%s\n", cur_tag->str, cur_val->str);
+            fprintf(ctx->debug_stream, " %s=%s\n", cur_tag->str, cur_val->str);
           #endif
           switch (cur_tag->str[0]) {
             case 'b':
@@ -667,7 +675,7 @@ pdkim_signature *pdkim_parse_sig_header(pdkim_ctx *ctx, char *raw_hdr) {
             default:
               #ifdef PDKIM_DEBUG
               if (ctx->debug_stream)
-                fprintf(ctx->debug_stream, "Unknown tag encountered\n");
+                fprintf(ctx->debug_stream, " Unknown tag encountered\n");
               #endif
             break;
           }
@@ -702,9 +710,6 @@ pdkim_signature *pdkim_parse_sig_header(pdkim_ctx *ctx, char *raw_hdr) {
     return NULL;
   }
 
-  /* Copy header list to 'tick-off' header list */
-  sig->hnames_check = strdup(sig->headernames);
-
   *q = '\0';
   /* Chomp raw header. The final newline must not be added to the signature. */
   q--;
@@ -795,7 +800,7 @@ pdkim_pubkey *pdkim_parse_pubkey_record(pdkim_ctx *ctx, char *raw_record) {
           pdkim_strtrim(cur_val);
           #ifdef PDKIM_DEBUG
           if (ctx->debug_stream)
-            fprintf(ctx->debug_stream, "%s=%s\n", cur_tag->str, cur_val->str);
+            fprintf(ctx->debug_stream, " %s=%s\n", cur_tag->str, cur_val->str);
           #endif
           switch (cur_tag->str[0]) {
             case 'v':
@@ -829,7 +834,7 @@ pdkim_pubkey *pdkim_parse_pubkey_record(pdkim_ctx *ctx, char *raw_record) {
             default:
               #ifdef PDKIM_DEBUG
               if (ctx->debug_stream)
-                fprintf(ctx->debug_stream, "Unknown tag encountered\n");
+                fprintf(ctx->debug_stream, " Unknown tag encountered\n");
               #endif
             break;
           }
@@ -864,7 +869,7 @@ pdkim_pubkey *pdkim_parse_pubkey_record(pdkim_ctx *ctx, char *raw_record) {
 
 
 /* -------------------------------------------------------------------------- */
-int pdkim_update_bodyhash(pdkim_ctx *ctx, char *data, int len) {
+int pdkim_update_bodyhash(pdkim_ctx *ctx, const char *data, int len) {
   pdkim_signature *sig = ctx->sig;
   /* Cache relaxed version of data */
   char *relaxed_data = NULL;
@@ -873,14 +878,14 @@ int pdkim_update_bodyhash(pdkim_ctx *ctx, char *data, int len) {
   /* Traverse all signatures, updating their hashes. */
   while (sig != NULL) {
     /* Defaults to simple canon (no further treatment necessary) */
-    char *canon_data = data;
-    int   canon_len = len;
+    const char *canon_data = data;
+    int         canon_len = len;
 
     if (sig->canon_body == PDKIM_CANON_RELAXED) {
       /* Relax the line if not done already */
       if (relaxed_data == NULL) {
         int seen_wsp = 0;
-        char *p = data;
+        const char *p = data;
         int q = 0;
         relaxed_data = malloc(len+1);
         if (relaxed_data == NULL) return PDKIM_ERR_OOM;
@@ -921,7 +926,7 @@ int pdkim_update_bodyhash(pdkim_ctx *ctx, char *data, int len) {
       sig->signed_body_bytes += canon_len;
 #ifdef PDKIM_DEBUG
       if (ctx->debug_stream!=NULL)
-        pdkim_quoteprint(ctx->debug_stream,canon_data,canon_len,0);
+        pdkim_quoteprint(ctx->debug_stream,canon_data,canon_len,1);
 #endif
     }
 
@@ -982,11 +987,11 @@ int pdkim_finish_bodyhash(pdkim_ctx *ctx) {
       else {
         #ifdef PDKIM_DEBUG
         if (ctx->debug_stream) {
-          fprintf(ctx->debug_stream, "PDKIM [%s] Body hash did NOT verify\n",
-                  sig->domain);
           fprintf(ctx->debug_stream, "PDKIM [%s] bh signature: ", sig->domain);
           pdkim_hexprint(ctx->debug_stream, sig->bodyhash,
                            (sig->algo == PDKIM_ALGO_RSA_SHA1)?20:32,1);
+          fprintf(ctx->debug_stream, "PDKIM [%s] Body hash did NOT verify\n",
+                  sig->domain);
         }
         #endif
         sig->verify_status     = PDKIM_VERIFY_FAIL;
@@ -1017,6 +1022,12 @@ int pdkim_bodyline_complete(pdkim_ctx *ctx) {
   if (ctx->input_mode == PDKIM_INPUT_SMTP) {
     /* Terminate on EOD marker */
     if (memcmp(p,".\r\n",3) == 0) {
+      /* In simple body mode, if any empty lines were buffered,
+      replace with one. rfc 4871 3.4.3 */
+      if (ctx->sig && ctx->sig->canon_body == PDKIM_CANON_SIMPLE
+        && ctx->num_buffered_crlf > 0)
+       pdkim_update_bodyhash(ctx,"\r\n",2);
+
       ctx->seen_eod = 1;
       goto BAIL;
     }
@@ -1033,6 +1044,23 @@ int pdkim_bodyline_complete(pdkim_ctx *ctx) {
     goto BAIL;
   }
 
+  if (  ctx->sig
+     && ctx->sig->canon_body == PDKIM_CANON_RELAXED) {
+    /* Lines with just spaces need to be buffered too */
+    char *check = p;
+    while(memcmp(check,"\r\n",2) != 0) {
+      char c = *check;
+
+      if (c != '\t' && c != ' ')
+       goto PROCESS;
+      check++;
+    }
+
+    ctx->num_buffered_crlf++;
+    goto BAIL;
+  }
+
+  PROCESS:
   /* At this point, we have a non-empty line, so release the buffered ones. */
   while (ctx->num_buffered_crlf) {
     pdkim_update_bodyhash(ctx,"\r\n",2);
@@ -1063,65 +1091,69 @@ int pdkim_header_complete(pdkim_ctx *ctx) {
   ctx->num_headers++;
   if (ctx->num_headers > PDKIM_MAX_HEADERS) goto BAIL;
 
-  /* Traverse all signatures */
-  while (sig != NULL) {
-    pdkim_stringlist *list;
+  /* SIGNING -------------------------------------------------------------- */
+  if (ctx->mode == PDKIM_MODE_SIGN) {
+    /* Traverse all signatures */
+    while (sig != NULL) {
+      pdkim_stringlist *list;
 
-    /* SIGNING -------------------------------------------------------------- */
-    if (ctx->mode == PDKIM_MODE_SIGN) {
       if (header_name_match(ctx->cur_header->str,
                             sig->sign_headers?
                               sig->sign_headers:
                               PDKIM_DEFAULT_SIGN_HEADERS, 0) != PDKIM_OK) goto NEXT_SIG;
-    }
-    /* VERIFICATION --------------------------------------------------------- */
-    else {
-      /* Header is not included or all instances were already 'ticked off' */
-      if (header_name_match(ctx->cur_header->str,
-                            sig->hnames_check, 1) != PDKIM_OK) goto NEXT_SIG;
-    }
 
-    /* Add header to the signed headers list (in reverse order) */
-    list = pdkim_prepend_stringlist(sig->headers,
-                                    ctx->cur_header->str);
-    if (list == NULL) return PDKIM_ERR_OOM;
-    sig->headers = list;
+      /* Add header to the signed headers list (in reverse order) */
+      list = pdkim_prepend_stringlist(sig->headers,
+                                      ctx->cur_header->str);
+      if (list == NULL) return PDKIM_ERR_OOM;
+      sig->headers = list;
 
-    NEXT_SIG:
-    sig = sig->next;
+      NEXT_SIG:
+      sig = sig->next;
+    }
   }
 
   /* DKIM-Signature: headers are added to the verification list */
-  if ( (ctx->mode == PDKIM_MODE_VERIFY) &&
-       (strncasecmp(ctx->cur_header->str,
+  if (ctx->mode == PDKIM_MODE_VERIFY) {
+    if (strncasecmp(ctx->cur_header->str,
                     DKIM_SIGNATURE_HEADERNAME,
-                    strlen(DKIM_SIGNATURE_HEADERNAME)) == 0) ) {
-     pdkim_signature *new_sig;
-    /* Create and chain new signature block */
-    #ifdef PDKIM_DEBUG
-    if (ctx->debug_stream)
-      fprintf(ctx->debug_stream,
-        "PDKIM >> Found sig, trying to parse >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
-    #endif
-    new_sig = pdkim_parse_sig_header(ctx, ctx->cur_header->str);
-    if (new_sig != NULL) {
-      pdkim_signature *last_sig = ctx->sig;
-      if (last_sig == NULL) {
-        ctx->sig = new_sig;
+                    strlen(DKIM_SIGNATURE_HEADERNAME)) == 0) {
+      pdkim_signature *new_sig;
+      /* Create and chain new signature block */
+      #ifdef PDKIM_DEBUG
+      if (ctx->debug_stream)
+        fprintf(ctx->debug_stream,
+          "PDKIM >> Found sig, trying to parse >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
+      #endif
+      new_sig = pdkim_parse_sig_header(ctx, ctx->cur_header->str);
+      if (new_sig != NULL) {
+        pdkim_signature *last_sig = ctx->sig;
+        if (last_sig == NULL) {
+          ctx->sig = new_sig;
+        }
+        else {
+          while (last_sig->next != NULL) { last_sig = last_sig->next; }
+          last_sig->next = new_sig;
+        }
       }
       else {
-        while (last_sig->next != NULL) { last_sig = last_sig->next; }
-        last_sig->next = new_sig;
+        #ifdef PDKIM_DEBUG
+        if (ctx->debug_stream) {
+          fprintf(ctx->debug_stream,"Error while parsing signature header\n");
+          fprintf(ctx->debug_stream,
+            "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
+        }
+        #endif
       }
     }
+    /* every other header is stored for signature verification */
     else {
-      #ifdef PDKIM_DEBUG
-      if (ctx->debug_stream) {
-        fprintf(ctx->debug_stream,"Error while parsing signature header\n");
-        fprintf(ctx->debug_stream,
-          "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
-      }
-      #endif
+      pdkim_stringlist *list;
+
+      list = pdkim_prepend_stringlist(ctx->headers,
+                                      ctx->cur_header->str);
+      if (list == NULL) return PDKIM_ERR_OOM;
+      ctx->headers = list;
     }
   }
 
@@ -1188,86 +1220,216 @@ DLLEXPORT int pdkim_feed (pdkim_ctx *ctx,
   return PDKIM_OK;
 }
 
+/*
+ * RFC 5322 specifies that header line length SHOULD be no more than 78
+ * lets make it so!
+ *  pdkim_headcat
+ * returns char*
+ *
+ * col: this int holds and receives column number (octets since last '\n')
+ * str: partial string to append to
+ * pad: padding, split line or space after before or after eg: ";"
+ * intro: - must join to payload eg "h=", usually the tag name
+ * payload: eg base64 data - long data can be split arbitrarily.
+ *
+ * this code doesn't fold the header in some of the places that RFC4871
+ * allows: As per RFC5322(2.2.3) it only folds before or after tag-value
+ * pairs and inside long values. it also always spaces or breaks after the
+ * "pad"
+ *
+ * no guarantees are made for output given out-of range input. like tag
+ * names loinger than 78, or bogus col. Input is assumed to be free of line breaks.
+ */
+
+static char *pdkim_headcat(int *col, pdkim_str *str, const char*pad,const char *intro, const char *payload ) {
+  size_t l;
+  if( pad)
+  {
+    l = strlen(pad);
+    if( *col + l > 78 )
+    {
+      pdkim_strcat(str, "\r\n\t");
+      *col=1;
+    }
+    pdkim_strncat(str, pad,l);
+    *col +=l;
+  }
+
+  l=(pad?1:0) + (intro?strlen(intro):0 );
+
+  if( *col + l > 78 )
+  { /*can't fit intro - start a new line to make room.*/
+    pdkim_strcat(str, "\r\n\t");
+    *col=1;
+    l= intro?strlen(intro):0;
+  }
+
+  l += payload ? strlen(payload):0 ;
+
+  while(l>77)
+  { /* this fragment will not fit on a single line */
+    if( pad )
+    {
+      pdkim_strcat(str, " ");
+      *col +=1;
+      pad=NULL; // only want this once
+      l--;
+    }
+    if( intro )
+    {
+      size_t sl=strlen(intro);
+      pdkim_strncat(str, intro,sl);
+      *col +=sl;
+      l-=sl;
+      intro=NULL; // only want this once
+    }
+    if(payload)
+    {
+      size_t sl=strlen(payload);
+      size_t chomp = *col+sl < 77 ? sl : 78-*col;
+      pdkim_strncat(str, payload,chomp);
+      *col +=chomp;
+      payload+=chomp;
+      l-=chomp-1;
+    }
+    // the while precondition tells us it didn't fit.
+    pdkim_strcat(str, "\r\n\t");
+    *col=1;
+  }
+  if( *col + l > 78 )
+  {
+    pdkim_strcat(str, "\r\n\t");
+    *col=1;
+    pad=NULL;
+  }
+
+  if( pad )
+  {
+    pdkim_strcat(str, " ");
+    *col +=1;
+    pad=NULL;
+  }
+
+  if( intro )
+  {
+    size_t sl=strlen(intro);
+    pdkim_strncat(str, intro,sl);
+    *col +=sl;
+    l-=sl;
+    intro=NULL;
+  }
+  if(payload)
+  {
+    size_t sl=strlen(payload);
+    pdkim_strncat(str, payload,sl);
+    *col +=sl;
+  }
+
+  return str->str;
+}
 
 /* -------------------------------------------------------------------------- */
 char *pdkim_create_header(pdkim_signature *sig, int final) {
   char *rc = NULL;
   char *base64_bh = NULL;
   char *base64_b  = NULL;
+  int col=0;
   pdkim_str *hdr = pdkim_strnew("DKIM-Signature: v="PDKIM_SIGNATURE_VERSION);
   if (hdr == NULL) return NULL;
+  pdkim_str *canon_all = pdkim_strnew(pdkim_canons[sig->canon_headers]);
+  if (canon_all == NULL) goto BAIL;
 
   base64_bh = pdkim_encode_base64(sig->bodyhash, sig->bodyhash_len);
   if (base64_bh == NULL) goto BAIL;
 
+  col=strlen(hdr->str);
+
   /* Required and static bits */
   if (
-        pdkim_strcat(hdr,"; a=")                                &&
-        pdkim_strcat(hdr,pdkim_algos[sig->algo])                &&
-        pdkim_strcat(hdr,"; q=")                                &&
-        pdkim_strcat(hdr,pdkim_querymethods[sig->querymethod])  &&
-        pdkim_strcat(hdr,"; c=")                                &&
-        pdkim_strcat(hdr,pdkim_canons[sig->canon_headers])      &&
-        pdkim_strcat(hdr,"/")                                   &&
-        pdkim_strcat(hdr,pdkim_canons[sig->canon_body])         &&
-        pdkim_strcat(hdr,"; d=")                                &&
-        pdkim_strcat(hdr,sig->domain)                           &&
-        pdkim_strcat(hdr,"; s=")                                &&
-        pdkim_strcat(hdr,sig->selector)                         &&
-        pdkim_strcat(hdr,";\r\n\th=")                           &&
-        pdkim_strcat(hdr,sig->headernames)                      &&
-        pdkim_strcat(hdr,"; bh=")                               &&
-        pdkim_strcat(hdr,base64_bh)                             &&
-        pdkim_strcat(hdr,";\r\n\t")
+        pdkim_headcat(&col,hdr,";","a=",pdkim_algos[sig->algo]) &&
+        pdkim_headcat(&col,hdr,";","q=",pdkim_querymethods[sig->querymethod])  &&
+          pdkim_strcat(canon_all,"/")                           &&
+          pdkim_strcat(canon_all,pdkim_canons[sig->canon_body]) &&
+        pdkim_headcat(&col,hdr,";","c=",canon_all->str)         &&
+        pdkim_headcat(&col,hdr,";","d=",sig->domain)            &&
+        pdkim_headcat(&col,hdr,";","s=",sig->selector)
      ) {
+    /* list of eader names can be split between items. */
+    {
+      char *n=strdup(sig->headernames);
+      char *f=n;
+      char *i="h=";
+      char *s=";";
+      if(!n) goto BAIL;
+      while (*n)
+      {
+          char *c=strchr(n,':');
+          if(c) *c='\0';
+          if(!i)
+          {
+            if (!pdkim_headcat(&col,hdr,NULL,NULL,":"))
+            {
+              free(f);
+              goto BAIL;
+            }
+          }
+          if( !pdkim_headcat(&col,hdr,s,i,n))
+          {
+            free(f);
+            goto BAIL;
+          }
+          if(c) n=c+1 ; else break;
+          s=NULL;
+          i=NULL;
+      }
+      free(f);
+    }
+    if(!pdkim_headcat(&col,hdr,";","bh=",base64_bh))
+        goto BAIL;
+
     /* Optional bits */
     if (sig->identity != NULL) {
-      if (!( pdkim_strcat(hdr,"i=")                             &&
-             pdkim_strcat(hdr,sig->identity)                    &&
-             pdkim_strcat(hdr,";") ) ) {
+      if(!pdkim_headcat(&col,hdr,";","i=",sig->identity)){
         goto BAIL;
       }
     }
+
     if (sig->created > 0) {
-      if (!( pdkim_strcat(hdr,"t=")                             &&
-             pdkim_numcat(hdr,sig->created)                     &&
-             pdkim_strcat(hdr,";") ) ) {
+      char minibuf[20];
+      snprintf(minibuf,20,"%lu",sig->created);
+      if(!pdkim_headcat(&col,hdr,";","t=",minibuf)) {
         goto BAIL;
       }
     }
     if (sig->expires > 0) {
-      if (!( pdkim_strcat(hdr,"x=")                             &&
-             pdkim_numcat(hdr,sig->expires)                     &&
-             pdkim_strcat(hdr,";") ) ) {
+      char minibuf[20];
+      snprintf(minibuf,20,"%lu",sig->expires);
+      if(!pdkim_headcat(&col,hdr,";","x=",minibuf)) {
         goto BAIL;
       }
     }
     if (sig->bodylength >= 0) {
-      if (!( pdkim_strcat(hdr,"l=")                             &&
-             pdkim_numcat(hdr,sig->bodylength)                  &&
-             pdkim_strcat(hdr,";") ) ) {
+      char minibuf[20];
+      snprintf(minibuf,20,"%lu",sig->bodylength);
+      if(!pdkim_headcat(&col,hdr,";","l=",minibuf)) {
         goto BAIL;
       }
     }
-    /* Extra linebreak */
-    if (hdr->str[(hdr->len)-1] == ';') {
-      if (!pdkim_strcat(hdr," \r\n\t")) goto BAIL;
-    }
+
     /* Preliminary or final version? */
     if (final) {
       base64_b = pdkim_encode_base64(sig->sigdata, sig->sigdata_len);
       if (base64_b == NULL) goto BAIL;
-      if (
-            pdkim_strcat(hdr,"b=")                              &&
-            pdkim_strcat(hdr,base64_b)                          &&
-            pdkim_strcat(hdr,";")
-         ) goto DONE;
+      if(!pdkim_headcat(&col,hdr,";","b=",base64_b)) goto BAIL;
     }
     else {
-      if (pdkim_strcat(hdr,"b=;")) goto DONE;
+      if(!pdkim_headcat(&col,hdr,";","b=","")) goto BAIL;
     }
 
-    goto BAIL;
+    /* add trailing semicolon: I'm not sure if this is actually needed */
+    if(!pdkim_headcat(&col,hdr,NULL,";","")) goto BAIL;
+
+    goto DONE;
   }
 
   DONE:
@@ -1275,6 +1437,7 @@ char *pdkim_create_header(pdkim_signature *sig, int final) {
 
   BAIL:
   pdkim_strfree(hdr);
+  if (canon_all != NULL) pdkim_strfree(canon_all);
   if (base64_bh != NULL) free(base64_bh);
   if (base64_b  != NULL) free(base64_b);
   return rc;
@@ -1376,14 +1539,23 @@ DLLEXPORT int pdkim_feed_finish(pdkim_ctx *ctx, pdkim_signature **return_signatu
       char *b = strdup(sig->headernames);
       char *p = b;
       char *q = NULL;
+      pdkim_stringlist *hdrs = ctx->headers;
+
       if (b == NULL) return PDKIM_ERR_OOM;
 
+      /* clear tags */
+      while (hdrs != NULL) {
+        hdrs->tag = 0;
+        hdrs = hdrs->next;
+      }
+
       while(1) {
-        pdkim_stringlist *hdrs = sig->headers;
+        hdrs = ctx->headers;
         q = strchr(p,':');
         if (q != NULL) *q = '\0';
         while (hdrs != NULL) {
-          if ( (strncasecmp(hdrs->value,p,strlen(p)) == 0) &&
+          if ( (hdrs->tag == 0) &&
+               (strncasecmp(hdrs->value,p,strlen(p)) == 0) &&
                ((hdrs->value)[strlen(p)] == ':') ) {
             char *rh = NULL;
             if (sig->canon_headers == PDKIM_CANON_RELAXED)
@@ -1401,7 +1573,7 @@ DLLEXPORT int pdkim_feed_finish(pdkim_ctx *ctx, pdkim_signature **return_signatu
               pdkim_quoteprint(ctx->debug_stream, rh, strlen(rh), 1);
             #endif
             free(rh);
-            (hdrs->value)[0] = '_';
+            hdrs->tag = 1;
             break;
           }
           hdrs = hdrs->next;
@@ -1481,7 +1653,7 @@ DLLEXPORT int pdkim_feed_finish(pdkim_ctx *ctx, pdkim_signature **return_signatu
     if (ctx->mode == PDKIM_MODE_SIGN) {
       rsa_context rsa;
 
-      rsa_init(&rsa,RSA_PKCS_V15,0,NULL,NULL);
+      rsa_init(&rsa,RSA_PKCS_V15,0);
 
       /* Perform private key operation */
       if (rsa_parse_key(&rsa, (unsigned char *)sig->rsa_privkey,
@@ -1520,7 +1692,7 @@ DLLEXPORT int pdkim_feed_finish(pdkim_ctx *ctx, pdkim_signature **return_signatu
       rsa_context rsa;
       char *dns_txt_name, *dns_txt_reply;
 
-      rsa_init(&rsa,RSA_PKCS_V15,0,NULL,NULL);
+      rsa_init(&rsa,RSA_PKCS_V15,0);
 
       dns_txt_name  = malloc(PDKIM_DNS_TXT_MAX_NAMELEN);
       if (dns_txt_name == NULL) return PDKIM_ERR_OOM;
@@ -1551,7 +1723,7 @@ DLLEXPORT int pdkim_feed_finish(pdkim_ctx *ctx, pdkim_signature **return_signatu
       if (ctx->debug_stream) {
         fprintf(ctx->debug_stream,
                 "PDKIM >> Parsing public key record >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>\n");
-        fprintf(ctx->debug_stream,"Raw record: ");
+        fprintf(ctx->debug_stream," Raw record: ");
         pdkim_quoteprint(ctx->debug_stream, dns_txt_reply, strlen(dns_txt_reply), 1);
       }
       #endif
@@ -1562,7 +1734,7 @@ DLLEXPORT int pdkim_feed_finish(pdkim_ctx *ctx, pdkim_signature **return_signatu
         sig->verify_ext_status =  PDKIM_VERIFY_INVALID_PUBKEY_PARSING;
         #ifdef PDKIM_DEBUG
         if (ctx->debug_stream) {
-          fprintf(ctx->debug_stream,"Error while parsing public key record\n");
+          fprintf(ctx->debug_stream," Error while parsing public key record\n");
           fprintf(ctx->debug_stream,
             "PDKIM <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n");
         }