random: Eliminate unneeded memcpy invocations in the DRBG.
authorStephan Mueller <smueller@chronox.de>
Sat, 3 Dec 2016 18:18:01 +0000 (19:18 +0100)
committerWerner Koch <wk@gnupg.org>
Thu, 8 Dec 2016 08:06:30 +0000 (09:06 +0100)
* random/random-drbg.c (drbg_hash): Remove arg 'outval' and return a
pointer instead.
(drbg_instantiate): Reduce size of scratchpad.
(drbg_hmac_update): Avoid use of scratch buffers for the hash.
(drbg_hmac_generate, drbg_hash_df): Ditto.
(drbg_hash_process_addtl): Ditto.
(drbg_hash_hashgen): Ditto.
(drbg_hash_generate): Ditto.

--
The gcry_md_read returns a pointer to the hash which can be directly
used instead of copying it into a scratch buffer. This eliminates a
number of memcpy invocations for HMAC and Hash DRBG and reduces the
memory footprint of the Hash DRBG by the block size of the used hash.

The performance increase is between 1 and 3 MB/s depending on the output
buffer size.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
ChangeLog entries above written by -wk.

random/random-drbg.c

index dc8e8f3..e2fe861 100644 (file)
@@ -374,9 +374,7 @@ static gpg_err_code_t drbg_hmac_init (drbg_state_t drbg);
 static gpg_err_code_t drbg_hmac_setkey (drbg_state_t drbg,
                                        const unsigned char *key);
 static void drbg_hash_fini (drbg_state_t drbg);
-static gpg_err_code_t drbg_hash (drbg_state_t drbg,
-                                 unsigned char *outval,
-                                 const drbg_string_t *buf);
+static byte *drbg_hash (drbg_state_t drbg, const drbg_string_t *buf);
 static gpg_err_code_t drbg_sym_init (drbg_state_t drbg);
 static void drbg_sym_fini (drbg_state_t drbg);
 static gpg_err_code_t drbg_sym_setkey (drbg_state_t drbg,
@@ -1042,24 +1040,21 @@ drbg_hmac_update (drbg_state_t drbg, drbg_string_t *seed, int reseed)
   /* we execute two rounds of V/K massaging */
   for (i = 2; 0 < i; i--)
     {
+      byte *retval;
       /* first round uses 0x0, second 0x1 */
       unsigned char prefix = DRBG_PREFIX0;
       if (1 == i)
        prefix = DRBG_PREFIX1;
       /* 10.1.2.2 step 1 and 4 -- concatenation and HMAC for key */
       seed2.buf = &prefix;
-      ret = drbg_hash (drbg, drbg->C, &seed1);
-      if (ret)
-       return ret;
-
-      ret = drbg_hmac_setkey (drbg, drbg->C);
+      retval = drbg_hash (drbg, &seed1);
+      ret = drbg_hmac_setkey (drbg, retval);
       if (ret)
        return ret;
 
       /* 10.1.2.2 step 2 and 5 -- HMAC for V */
-      ret = drbg_hash (drbg, drbg->V, &cipherin);
-      if (ret)
-       return ret;
+      retval = drbg_hash (drbg, &cipherin);
+      memcpy(drbg->V, retval, drbg_blocklen (drbg));
 
       /* 10.1.2.2 step 3 */
       if (!seed || 0 == seed->len)
@@ -1091,9 +1086,8 @@ drbg_hmac_generate (drbg_state_t drbg, unsigned char *buf, unsigned int buflen,
     {
       unsigned int outlen = 0;
       /* 10.1.2.5 step 4.1 */
-      ret = drbg_hash (drbg, drbg->V, &data);
-      if (ret)
-       return ret;
+      byte *retval = drbg_hash (drbg, &data);
+      memcpy(drbg->V, retval, drbg_blocklen (drbg));
       outlen = (drbg_blocklen (drbg) < (buflen - len)) ?
        drbg_blocklen (drbg) : (buflen - len);
 
@@ -1137,14 +1131,10 @@ drbg_hash_df (drbg_state_t drbg,
               unsigned char *outval, size_t outlen,
               drbg_string_t *entropy)
 {
-  gpg_err_code_t ret = 0;
   size_t len = 0;
   unsigned char input[5];
-  unsigned char *tmp = drbg->scratchpad + drbg_statelen (drbg);
   drbg_string_t data1;
 
-  memset (tmp, 0, drbg_blocklen (drbg));
-
   /* 10.4.1 step 3 */
   input[0] = 1;
   drbg_cpu_to_be32 ((outlen * 8), &input[1]);
@@ -1158,20 +1148,16 @@ drbg_hash_df (drbg_state_t drbg,
     {
       short blocklen = 0;
       /* 10.4.1 step 4.1 */
-      ret = drbg_hash (drbg, tmp, &data1);
-      if (ret)
-       goto out;
+      byte *retval = drbg_hash (drbg, &data1);
       /* 10.4.1 step 4.2 */
       input[0]++;
       blocklen = (drbg_blocklen (drbg) < (outlen - len)) ?
        drbg_blocklen (drbg) : (outlen - len);
-      memcpy (outval + len, tmp, blocklen);
+      memcpy (outval + len, retval, blocklen);
       len += blocklen;
     }
 
- out:
-  memset (tmp, 0, drbg_blocklen (drbg));
-  return ret;
+  return 0;
 }
 
 /* update function for Hash DRBG as defined in 10.1.1.2 / 10.1.1.3 */
@@ -1227,13 +1213,10 @@ drbg_hash_update (drbg_state_t drbg, drbg_string_t *seed, int reseed)
 static gpg_err_code_t
 drbg_hash_process_addtl (drbg_state_t drbg, drbg_string_t *addtl)
 {
-  gpg_err_code_t ret = 0;
   drbg_string_t data1, data2;
   drbg_string_t *data3;
   unsigned char prefix = DRBG_PREFIX2;
-
-  /* this is value w as per documentation */
-  memset (drbg->scratchpad, 0, drbg_blocklen (drbg));
+  byte *retval;
 
   /* 10.1.1.4 step 2 */
   if (!addtl || 0 == addtl->len)
@@ -1247,37 +1230,25 @@ drbg_hash_process_addtl (drbg_state_t drbg, drbg_string_t *addtl)
   data2.next = data3;
   data3->next = NULL;
   /* 10.1.1.4 step 2a -- cipher invocation */
-  ret = drbg_hash (drbg, drbg->scratchpad, &data1);
-  if (ret)
-    goto out;
+  retval = drbg_hash (drbg, &data1);
 
   /* 10.1.1.4 step 2b */
-  drbg_add_buf (drbg->V, drbg_statelen (drbg),
-               drbg->scratchpad, drbg_blocklen (drbg));
+  drbg_add_buf (drbg->V, drbg_statelen (drbg), retval, drbg_blocklen (drbg));
 
- out:
-  memset (drbg->scratchpad, 0, drbg_blocklen (drbg));
-  return ret;
+  return 0;
 }
 
 /*
  * Hashgen defined in 10.1.1.4
  */
 static gpg_err_code_t
-drbg_hash_hashgen (drbg_state_t drbg,
-                   unsigned char *buf, unsigned int buflen)
+drbg_hash_hashgen (drbg_state_t drbg, unsigned char *buf, unsigned int buflen)
 {
-  gpg_err_code_t ret = 0;
   unsigned int len = 0;
   unsigned char *src = drbg->scratchpad;
-  unsigned char *dst = drbg->scratchpad + drbg_statelen (drbg);
   drbg_string_t data;
   unsigned char prefix = DRBG_PREFIX1;
 
-  /* use the scratchpad as a lookaside buffer */
-  memset (src, 0, drbg_statelen (drbg));
-  memset (dst, 0, drbg_blocklen (drbg));
-
   /* 10.1.1.4 step hashgen 2 */
   memcpy (src, drbg->V, drbg_statelen (drbg));
 
@@ -1286,44 +1257,36 @@ drbg_hash_hashgen (drbg_state_t drbg,
     {
       unsigned int outlen = 0;
       /* 10.1.1.4 step hashgen 4.1 */
-      ret = drbg_hash (drbg, dst, &data);
-      if (ret)
-       goto out;
+      byte *retval = drbg_hash (drbg, &data);
       outlen = (drbg_blocklen (drbg) < (buflen - len)) ?
        drbg_blocklen (drbg) : (buflen - len);
       /* 10.1.1.4 step hashgen 4.2 */
-      memcpy (buf + len, dst, outlen);
+      memcpy (buf + len, retval, outlen);
       len += outlen;
       /* 10.1.1.4 hashgen step 4.3 */
       if (len < buflen)
        drbg_add_buf (src, drbg_statelen (drbg), &prefix, 1);
     }
 
- out:
-  memset (drbg->scratchpad, 0,
-         (drbg_statelen (drbg) + drbg_blocklen (drbg)));
-  return ret;
+  memset (drbg->scratchpad, 0, drbg_statelen (drbg));
+  return 0;
 }
 
 /* Generate function for Hash DRBG as defined in 10.1.1.4  */
 static gpg_err_code_t
-drbg_hash_generate (drbg_state_t drbg,
-                        unsigned char *buf, unsigned int buflen,
-                        drbg_string_t *addtl)
+drbg_hash_generate (drbg_state_t drbg, unsigned char *buf, unsigned int buflen,
+                   drbg_string_t *addtl)
 {
-  gpg_err_code_t ret = 0;
+  gpg_err_code_t ret;
   unsigned char prefix = DRBG_PREFIX3;
   drbg_string_t data1, data2;
+  byte *retval;
   union
   {
     unsigned char req[8];
     u64 req_int;
   } u;
 
-  /*
-   * scratchpad usage: drbg_hash_process_addtl uses the scratchpad, but
-   * fully completes before returning. Thus, we can reuse the scratchpad
-   */
   /* 10.1.1.4 step 2 */
   ret = drbg_hash_process_addtl (drbg, addtl);
   if (ret)
@@ -1334,27 +1297,20 @@ drbg_hash_generate (drbg_state_t drbg,
   if (ret)
     return ret;
 
-  /* this is the value H as documented in 10.1.1.4 */
-  memset (drbg->scratchpad, 0, drbg_blocklen (drbg));
   /* 10.1.1.4 step 4 */
   drbg_string_fill (&data1, &prefix, 1);
   drbg_string_fill (&data2, drbg->V, drbg_statelen (drbg));
   data1.next = &data2;
-  ret = drbg_hash (drbg, drbg->scratchpad, &data1);
-  if (ret)
-    goto out;
+
+  /* this is the value H as documented in 10.1.1.4 */
+  retval = drbg_hash (drbg, &data1);
 
   /* 10.1.1.4 step 5 */
-  drbg_add_buf (drbg->V, drbg_statelen (drbg),
-                    drbg->scratchpad, drbg_blocklen (drbg));
-  drbg_add_buf (drbg->V, drbg_statelen (drbg), drbg->C,
-                    drbg_statelen (drbg));
+  drbg_add_buf (drbg->V, drbg_statelen (drbg), retval, drbg_blocklen (drbg));
+  drbg_add_buf (drbg->V, drbg_statelen (drbg), drbg->C, drbg_statelen (drbg));
   u.req_int = be_bswap64 (drbg->reseed_ctr);
-  drbg_add_buf (drbg->V, drbg_statelen (drbg), u.req,
-                    sizeof (u.req));
+  drbg_add_buf (drbg->V, drbg_statelen (drbg), u.req, sizeof (u.req));
 
- out:
-  memset (drbg->scratchpad, 0, drbg_blocklen (drbg));
   return ret;
 }
 
@@ -1699,7 +1655,7 @@ drbg_instantiate (drbg_state_t drbg,
       drbg_blocklen (drbg) +   /* iv */
       drbg_statelen (drbg) + drbg_blocklen (drbg);     /* temp */
   else
-    sb_size = drbg_statelen (drbg) + drbg_blocklen (drbg);
+    sb_size = drbg_statelen (drbg);
 
   if (0 < sb_size)
     {
@@ -2626,8 +2582,8 @@ drbg_hash_fini (drbg_state_t drbg)
   _gcry_md_close (hd);
 }
 
-static gpg_err_code_t
-drbg_hash (drbg_state_t drbg, unsigned char *outval, const drbg_string_t *buf)
+static byte *
+drbg_hash (drbg_state_t drbg, const drbg_string_t *buf)
 {
   gcry_md_hd_t hd = (gcry_md_hd_t)drbg->priv_data;
 
@@ -2635,9 +2591,7 @@ drbg_hash (drbg_state_t drbg, unsigned char *outval, const drbg_string_t *buf)
   for (; NULL != buf; buf = buf->next)
     _gcry_md_write (hd, buf->buf, buf->len);
   _gcry_md_final (hd);
-  memcpy (outval, _gcry_md_read (hd, drbg->core->backend_cipher),
-         drbg_blocklen (drbg));
-  return 0;
+  return _gcry_md_read (hd, drbg->core->backend_cipher);
 }
 
 static void