md: keep contexts for HMAC in GcryDigestEntry.
authorNIIBE Yutaka <gniibe@fsij.org>
Thu, 22 Oct 2015 00:58:24 +0000 (09:58 +0900)
committerNIIBE Yutaka <gniibe@fsij.org>
Thu, 22 Oct 2015 00:58:24 +0000 (09:58 +0900)
* cipher/md.c (struct gcry_md_context): Add flags.hmac.
Remove macpads and mcpads_Bsize.
(md_open): Initialize flags.hmac.  Remove macpads initialization.
(md_enable): Allocate contexts when flags.hmac is enabled.
(md_copy): Remove macpads copying.  Add copying contexts.
(_gcry_md_reset): When flags.hmac is enabled, restore precomputed
context with input pad
(md_close): Remove macpads wiping.
(md_final): When flags.hmac is enabled, compute hmac by precomputed
context with output pad.
(prepare_macpads): Prepare precomputed contexts with input pad and
output pad for each registered digest entry.
(_gcry_md_setkey): Just call prepare_macpads.

--

This change is making things straight in HMAC computation.  This makes
HMAC computation allow multple algorithms in future.

Libgcrypt's code has a potential to compute digests for multiple
algorithms at once (currently, it's not enabled).  HMAC code didn't
work well with multple algorithms, because the macpads were only
allocated for an algorithm.  Now, it's allocated for each algorithm.

We now precompute hash contexts, instead of keeping input pad and
output pad.  This can be performance improvement, which is described
in RFC 2104.

Thanks to:

   Andrea Visconti, Simone Bossi, Hany Ragab and Alexandro Calò

For the discussion and their paper of CANS2015, which titled:

   On the weaknesses of PBKDF2

cipher/md.c

index 19b2c9b..c6bf90d 100644 (file)
@@ -108,10 +108,9 @@ struct gcry_md_context
     unsigned int secure: 1;
     unsigned int finalized:1;
     unsigned int bugemu1:1;
+    unsigned int hmac:1;
   } flags;
   GcryDigestEntry *list;
-  byte *macpads;
-  int macpads_Bsize;             /* Blocksize as used for the HMAC pads. */
 };
 
 
@@ -331,43 +330,8 @@ md_open (gcry_md_hd_t *h, int algo, unsigned int flags)
       ctx->magic = secure ? CTX_MAGIC_SECURE : CTX_MAGIC_NORMAL;
       ctx->actual_handle_size = n + sizeof (struct gcry_md_context);
       ctx->flags.secure = secure;
+      ctx->flags.hmac = hmac;
       ctx->flags.bugemu1 = !!(flags & GCRY_MD_FLAG_BUGEMU1);
-
-      if (hmac)
-       {
-         switch (algo)
-            {
-              case GCRY_MD_SHA3_224:
-                ctx->macpads_Bsize = 1152 / 8;
-                break;
-              case GCRY_MD_SHA3_256:
-                ctx->macpads_Bsize = 1088 / 8;
-                break;
-              case GCRY_MD_SHA3_384:
-                ctx->macpads_Bsize = 832 / 8;
-                break;
-              case GCRY_MD_SHA3_512:
-                ctx->macpads_Bsize = 576 / 8;
-                break;
-              case GCRY_MD_SHA384:
-              case GCRY_MD_SHA512:
-                ctx->macpads_Bsize = 128;
-                break;
-              case GCRY_MD_GOSTR3411_94:
-              case GCRY_MD_GOSTR3411_CP:
-                ctx->macpads_Bsize = 32;
-                break;
-              default:
-                ctx->macpads_Bsize = 64;
-                break;
-            }
-          ctx->macpads = xtrymalloc_secure (2*(ctx->macpads_Bsize));
-         if (!ctx->macpads)
-           {
-             err = gpg_err_code_from_errno (errno);
-             md_close (hd);
-           }
-       }
     }
 
   if (! err)
@@ -447,7 +411,7 @@ md_enable (gcry_md_hd_t hd, int algorithm)
   if (!err)
     {
       size_t size = (sizeof (*entry)
-                     + spec->contextsize
+                     + spec->contextsize * (h->flags.hmac? 3 : 1)
                      - sizeof (entry->context));
 
       /* And allocate a new list entry. */
@@ -515,17 +479,6 @@ md_copy (gcry_md_hd_t ahd, gcry_md_hd_t *b_hd)
       memcpy (b, a, sizeof *a);
       b->list = NULL;
       b->debug = NULL;
-      if (a->macpads)
-       {
-         b->macpads = xtrymalloc_secure (2*(a->macpads_Bsize));
-         if (! b->macpads)
-           {
-             err = gpg_err_code_from_errno (errno);
-             md_close (bhd);
-           }
-         else
-           memcpy (b->macpads, a->macpads, (2*(a->macpads_Bsize)));
-       }
     }
 
   /* Copy the complete list of algorithms.  The copied list is
@@ -535,13 +488,9 @@ md_copy (gcry_md_hd_t ahd, gcry_md_hd_t *b_hd)
       for (ar = a->list; ar; ar = ar->next)
         {
           if (a->flags.secure)
-            br = xtrymalloc_secure (sizeof *br
-                                    + ar->spec->contextsize
-                                    - sizeof(ar->context));
+            br = xtrymalloc_secure (ar->actual_struct_size);
           else
-            br = xtrymalloc (sizeof *br
-                             + ar->spec->contextsize
-                             - sizeof (ar->context));
+            br = xtrymalloc (ar->actual_struct_size);
           if (!br)
             {
              err = gpg_err_code_from_errno (errno);
@@ -549,8 +498,7 @@ md_copy (gcry_md_hd_t ahd, gcry_md_hd_t *b_hd)
               break;
             }
 
-          memcpy (br, ar, (sizeof (*br) + ar->spec->contextsize
-                           - sizeof (ar->context)));
+          memcpy (br, ar, ar->actual_struct_size);
           br->next = b->list;
           b->list = br;
         }
@@ -591,14 +539,19 @@ _gcry_md_reset (gcry_md_hd_t a)
 
   a->bufpos = a->ctx->flags.finalized = 0;
 
-  for (r = a->ctx->list; r; r = r->next)
-    {
-      memset (r->context.c, 0, r->spec->contextsize);
-      (*r->spec->init) (&r->context.c,
-                        a->ctx->flags.bugemu1? GCRY_MD_FLAG_BUGEMU1:0);
-    }
-  if (a->ctx->macpads)
-    md_write (a, a->ctx->macpads, a->ctx->macpads_Bsize); /* inner pad */
+  if (a->ctx->flags.hmac)
+    for (r = a->ctx->list; r; r = r->next)
+      {
+        memcpy (r->context.c, r->context.c + r->spec->contextsize,
+                r->spec->contextsize);
+      }
+  else
+    for (r = a->ctx->list; r; r = r->next)
+      {
+        memset (r->context.c, 0, r->spec->contextsize);
+        (*r->spec->init) (&r->context.c,
+                          a->ctx->flags.bugemu1? GCRY_MD_FLAG_BUGEMU1:0);
+      }
 }
 
 
@@ -618,12 +571,6 @@ md_close (gcry_md_hd_t a)
       xfree (r);
     }
 
-  if (a->ctx->macpads)
-    {
-      wipememory (a->ctx->macpads, 2*(a->ctx->macpads_Bsize));
-      xfree(a->ctx->macpads);
-    }
-
   wipememory (a, a->ctx->actual_handle_size);
   xfree(a);
 }
@@ -686,66 +633,120 @@ md_final (gcry_md_hd_t a)
 
   a->ctx->flags.finalized = 1;
 
-  if (a->ctx->macpads)
+  if (!a->ctx->flags.hmac)
+    return;
+
+  for (r = a->ctx->list; r; r = r->next)
     {
-      /* Finish the hmac. */
-      int algo = md_get_algo (a);
-      byte *p = md_read (a, algo);
-      size_t dlen = md_digest_length (algo);
-      gcry_md_hd_t om;
+      byte *p = r->spec->read (&r->context.c);
+      size_t dlen = r->spec->mdlen;
+      byte *hash;
       gcry_err_code_t err;
 
-      err = md_open (&om, algo,
-                     ((a->ctx->flags.secure? GCRY_MD_FLAG_SECURE:0)
-                      | (a->ctx->flags.bugemu1? GCRY_MD_FLAG_BUGEMU1:0)));
-      if (err)
-       _gcry_fatal_error (err, NULL);
-      md_write (om,
-                (a->ctx->macpads)+(a->ctx->macpads_Bsize),
-                a->ctx->macpads_Bsize);
-      md_write (om, p, dlen);
-      md_final (om);
-      /* Replace our digest with the mac (they have the same size). */
-      memcpy (p, md_read (om, algo), dlen);
-      md_close (om);
+      if (a->ctx->flags.secure)
+        hash = xtrymalloc_secure (dlen);
+      else
+        hash = xtrymalloc (dlen);
+      if (!hash)
+        {
+          err = gpg_err_code_from_errno (errno);
+          _gcry_fatal_error (err, NULL);
+        }
+
+      memcpy (hash, p, dlen);
+      memcpy (r->context.c, r->context.c + r->spec->contextsize * 2,
+              r->spec->contextsize);
+      (*r->spec->write) (&r->context.c, hash, dlen);
+      (*r->spec->final) (&r->context.c);
+      xfree (hash);
     }
 }
 
 
 static gcry_err_code_t
-prepare_macpads (gcry_md_hd_t hd, const unsigned char *key, size_t keylen)
+prepare_macpads (gcry_md_hd_t a, const unsigned char *key, size_t keylen)
 {
-  int i;
-  int algo = md_get_algo (hd);
-  unsigned char *helpkey = NULL;
-  unsigned char *ipad, *opad;
+  GcryDigestEntry *r;
 
-  if (!algo)
+  if (!a->ctx->list)
     return GPG_ERR_DIGEST_ALGO; /* Might happen if no algo is enabled.  */
 
-  if ( keylen > hd->ctx->macpads_Bsize )
+  for (r = a->ctx->list; r; r = r->next)
     {
-      helpkey = xtrymalloc_secure (md_digest_length (algo));
-      if (!helpkey)
-        return gpg_err_code_from_errno (errno);
-      _gcry_md_hash_buffer (algo, helpkey, key, keylen);
-      key = helpkey;
-      keylen = md_digest_length (algo);
-      gcry_assert ( keylen <= hd->ctx->macpads_Bsize );
-    }
+      const unsigned char *k;
+      size_t k_len;
+      unsigned char *key_allocated = NULL;
+      int macpad_Bsize;
+      int i;
 
-  memset ( hd->ctx->macpads, 0, 2*(hd->ctx->macpads_Bsize) );
-  ipad = hd->ctx->macpads;
-  opad = (hd->ctx->macpads)+(hd->ctx->macpads_Bsize);
-  memcpy ( ipad, key, keylen );
-  memcpy ( opad, key, keylen );
-  for (i=0; i < hd->ctx->macpads_Bsize; i++ )
-    {
-      ipad[i] ^= 0x36;
-      opad[i] ^= 0x5c;
+      switch (r->spec->algo)
+        {
+        case GCRY_MD_SHA3_224:
+          macpad_Bsize = 1152 / 8;
+          break;
+        case GCRY_MD_SHA3_256:
+          macpad_Bsize = 1088 / 8;
+          break;
+        case GCRY_MD_SHA3_384:
+          macpad_Bsize = 832 / 8;
+          break;
+        case GCRY_MD_SHA3_512:
+          macpad_Bsize = 576 / 8;
+          break;
+        case GCRY_MD_SHA384:
+        case GCRY_MD_SHA512:
+          macpad_Bsize = 128;
+          break;
+        case GCRY_MD_GOSTR3411_94:
+        case GCRY_MD_GOSTR3411_CP:
+          macpad_Bsize = 32;
+          break;
+        default:
+          macpad_Bsize = 64;
+          break;
+        }
+
+      if ( keylen > macpad_Bsize )
+        {
+          k = key_allocated = xtrymalloc_secure (r->spec->mdlen);
+          if (!k)
+            return gpg_err_code_from_errno (errno);
+          _gcry_md_hash_buffer (r->spec->algo, key_allocated, key, keylen);
+          k_len = r->spec->mdlen;
+          gcry_assert ( k_len <= macpad_Bsize );
+        }
+      else
+        {
+          k = key;
+          k_len = keylen;
+        }
+
+      (*r->spec->init) (&r->context.c,
+                        a->ctx->flags.bugemu1? GCRY_MD_FLAG_BUGEMU1:0);
+      a->bufpos = 0;
+      for (i=0; i < k_len; i++ )
+        _gcry_md_putc (a, k[i] ^ 0x36);
+      for (; i < macpad_Bsize; i++ )
+        _gcry_md_putc (a, 0x36);
+      (*r->spec->write) (&r->context.c, a->buf, a->bufpos);
+      memcpy (r->context.c + r->spec->contextsize, r->context.c,
+              r->spec->contextsize);
+
+      (*r->spec->init) (&r->context.c,
+                        a->ctx->flags.bugemu1? GCRY_MD_FLAG_BUGEMU1:0);
+      a->bufpos = 0;
+      for (i=0; i < k_len; i++ )
+        _gcry_md_putc (a, k[i] ^ 0x5c);
+      for (; i < macpad_Bsize; i++ )
+        _gcry_md_putc (a, 0x5c);
+      (*r->spec->write) (&r->context.c, a->buf, a->bufpos);
+      memcpy (r->context.c + r->spec->contextsize*2, r->context.c,
+              r->spec->contextsize);
+
+      xfree (key_allocated);
     }
-  xfree (helpkey);
 
+  a->bufpos = 0;
   return 0;
 }
 
@@ -780,14 +781,9 @@ _gcry_md_setkey (gcry_md_hd_t hd, const void *key, size_t keylen)
 {
   gcry_err_code_t rc;
 
-  if (!hd->ctx->macpads)
-    rc = GPG_ERR_CONFLICT;
-  else
-    {
-      rc = prepare_macpads (hd, key, keylen);
-      if (!rc)
-       _gcry_md_reset (hd);
-    }
+  rc = prepare_macpads (hd, key, keylen);
+  if (!rc)
+    _gcry_md_reset (hd);
 
   return rc;
 }