[git] GnuPG - branch, master, updated. gnupg-2.2.5-113-gf2c0920

by Werner Koch cvs at cvs.gnupg.org
Wed Feb 28 09:50:57 CET 2018


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "The GNU Privacy Guard".

The branch, master has been updated
       via  f2c09203b98b83669a460dc8161283de96022536 (commit)
       via  047506a03d21739b5b922f6b3fd9f059b0b137c5 (commit)
      from  618b86325f776f7250ad2bb09680e4bb427d7e50 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit f2c09203b98b83669a460dc8161283de96022536
Author: Werner Koch <wk at gnupg.org>
Date:   Wed Feb 28 09:31:39 2018 +0100

    gpg: Avoid writing a zero length last chunk in AEAD mode.
    
    * g10/cipher-aead.c (write_header): Do not call set_nonce_and_ad.
    (write_final_chunk): Do not increase chunkindex.
    (do_flush): Call set_nonce_and_ad immediately before the first
    encryption of a chunk.  Bump up the chunkindex after writing the tag.
    (do_free): Do not insert a zero length last chunk.
    * g10/decrypt-data.c (aead_underflow): Fix the corresponding bug.
    --
    
    This fixes a bug in writing a zero length last chunk right before the
    final chunk (which has by design a zero length).  We also need to
    adjust the decryption part because that assumed this zero length last
    chunk.
    
    Note that we use the term "last chunk" for the chunk which directly
    precedes the "final chunk" which ends the entire encryption.
    
    GnuPG-bug-id: 3774
    Signed-off-by: Werner Koch <wk at gnupg.org>

diff --git a/g10/cipher-aead.c b/g10/cipher-aead.c
index 1457baa..f9a996c 100644
--- a/g10/cipher-aead.c
+++ b/g10/cipher-aead.c
@@ -200,9 +200,6 @@ write_header (cipher_filter_context_t *cfx, iobuf_t a)
   if (err)
     return err;
 
-  err = set_nonce_and_ad (cfx, 0);
-  if (err)
-    return err;
   cfx->wrote_header = 1;
 
  leave:
@@ -238,9 +235,6 @@ write_final_chunk (cipher_filter_context_t *cfx, iobuf_t a)
   gpg_error_t err;
   char dummy[1];
 
-  if (cfx->chunklen)
-    cfx->chunkindex++;
-
   err = set_nonce_and_ad (cfx, 1);
   if (err)
     goto leave;
@@ -297,8 +291,18 @@ do_flush (cipher_filter_context_t *cfx, iobuf_t a, byte *buf, size_t size)
       if (cfx->buflen == cfx->bufsize || finalize)
         {
           if (DBG_FILTER)
-            log_debug ("encrypting: buflen=%zu %s n=%zu\n",
-                       cfx->buflen, finalize?"(finalize)":"", n);
+            log_debug ("encrypting: size=%zu buflen=%zu %s n=%zu\n",
+                       size, cfx->buflen, finalize?"(finalize)":"", n);
+
+          if (!cfx->chunklen)
+            {
+              if (DBG_FILTER)
+                log_debug ("start encrypting a new chunk\n");
+              err = set_nonce_and_ad (cfx, 0);
+              if (err)
+                goto leave;
+            }
+
           if (finalize)
             gcry_cipher_final (cfx->cipher_hd);
           if (DBG_FILTER)
@@ -314,8 +318,10 @@ do_flush (cipher_filter_context_t *cfx, iobuf_t a, byte *buf, size_t size)
            * be called after gcry_cipher_final and before
            * gcry_cipher_gettag - at least with libgcrypt 1.8 and OCB
            * mode.  */
-          gcry_cipher_encrypt (cfx->cipher_hd, cfx->buffer, cfx->buflen,
-                               NULL, 0);
+          err = gcry_cipher_encrypt (cfx->cipher_hd, cfx->buffer, cfx->buflen,
+                                     NULL, 0);
+          if (err)
+            goto leave;
           if (finalize && DBG_FILTER)
             log_printhex (cfx->buffer, cfx->buflen, "ciphr(1):");
           err = my_iobuf_write (a, cfx->buffer, cfx->buflen);
@@ -328,19 +334,14 @@ do_flush (cipher_filter_context_t *cfx, iobuf_t a, byte *buf, size_t size)
           if (finalize)
             {
               if (DBG_FILTER)
-                log_debug ("chunklen=%ju  total=%ju\n",
+                log_debug ("writing tag: chunklen=%ju total=%ju\n",
                            (uintmax_t)cfx->chunklen, (uintmax_t)cfx->total);
               err = write_auth_tag (cfx, a);
               if (err)
                 goto leave;
 
-              if (DBG_FILTER)
-                log_debug ("starting a new chunk (cur size=%zu)\n", size);
               cfx->chunkindex++;
               cfx->chunklen = 0;
-              err = set_nonce_and_ad (cfx, 0);
-              if (err)
-                goto leave;
               finalize = 0;
             }
         }
@@ -361,38 +362,42 @@ do_free (cipher_filter_context_t *cfx, iobuf_t a)
   if (DBG_FILTER)
     log_debug ("do_free: buflen=%zu\n", cfx->buflen);
 
-  /* FIXME: Check what happens if we just wrote the last chunk and no
-   * more bytes were to encrypt.  We should then not call finalize and
-   * write the auth tag again, right?  May this at all happen?  */
-
-  /* Call finalize which will also allow us to flush out and encrypt
-   * the last arbitrary length buffer.  */
-  gcry_cipher_final (cfx->cipher_hd);
-
-  /* Encrypt any remaining bytes.  */
   if (cfx->buflen)
     {
       if (DBG_FILTER)
-        log_debug ("processing last %zu bytes of the last chunk\n",
-                   cfx->buflen);
-      gcry_cipher_encrypt (cfx->cipher_hd, cfx->buffer, cfx->buflen, NULL, 0);
+        log_debug ("encrypting last %zu bytes of the last chunk\n",cfx->buflen);
+
+      if (!cfx->chunklen)
+        {
+          if (DBG_FILTER)
+            log_debug ("start encrypting a new chunk\n");
+          err = set_nonce_and_ad (cfx, 0);
+          if (err)
+            goto leave;
+        }
+
+      gcry_cipher_final (cfx->cipher_hd);
+      err = gcry_cipher_encrypt (cfx->cipher_hd, cfx->buffer, cfx->buflen,
+                                 NULL, 0);
+      if (err)
+        goto leave;
       err = my_iobuf_write (a, cfx->buffer, cfx->buflen);
       if (err)
         goto leave;
       /* log_printhex (cfx->buffer, cfx->buflen, "wrote:"); */
       cfx->chunklen += cfx->buflen;
       cfx->total += cfx->buflen;
-    }
-  else /* Dummy encryption.  */
-    gcry_cipher_encrypt (cfx->cipher_hd, cfx->buffer, 0, NULL, 0);
 
-  /* Get and write the authentication tag.  */
-  if (DBG_FILTER)
-    log_debug ("chunklen=%ju  total=%ju\n",
-               (uintmax_t)cfx->chunklen, (uintmax_t)cfx->total);
-  err = write_auth_tag (cfx, a);
-  if (err)
-    goto leave;
+      /* Get and write the authentication tag.  */
+      if (DBG_FILTER)
+        log_debug ("writing tag: chunklen=%ju total=%ju\n",
+                   (uintmax_t)cfx->chunklen, (uintmax_t)cfx->total);
+      err = write_auth_tag (cfx, a);
+      if (err)
+        goto leave;
+      cfx->chunkindex++;
+      cfx->chunklen = 0;
+    }
 
   /* Write the final chunk.  */
   if (DBG_FILTER)
diff --git a/g10/decrypt-data.c b/g10/decrypt-data.c
index 5594d90..a3151b5 100644
--- a/g10/decrypt-data.c
+++ b/g10/decrypt-data.c
@@ -770,17 +770,25 @@ aead_underflow (decode_filter_ctx_t dfx, iobuf_t a, byte *buf, size_t *ret_len)
 
   if (dfx->eof_seen)
     {
-      if (DBG_FILTER)
-        log_debug ("eof seen: holdback buffer has the last and final tag\n");
 
-      log_assert (dfx->holdbacklen >= 32);
       if (dfx->chunklen)
         {
+          if (DBG_FILTER)
+            log_debug ("eof seen: holdback has the last and final tag\n");
+          log_assert (dfx->holdbacklen >= 32);
           err = aead_checktag (dfx, 0, dfx->holdback);
           if (err)
             goto leave;
           dfx->chunklen = 0;
           dfx->chunkindex++;
+          off = 16;
+        }
+      else
+        {
+          if (DBG_FILTER)
+            log_debug ("eof seen: holdback has the final tag\n");
+          log_assert (dfx->holdbacklen >= 16);
+          off = 0;
         }
 
       /* Check the final chunk.  */
@@ -796,7 +804,7 @@ aead_underflow (decode_filter_ctx_t dfx, iobuf_t a, byte *buf, size_t *ret_len)
                      gpg_strerror (err));
           goto leave;
         }
-      err = aead_checktag (dfx, 1, dfx->holdback+16);
+      err = aead_checktag (dfx, 1, dfx->holdback+off);
       if (err)
         goto leave;
       err = gpg_error (GPG_ERR_EOF);

commit 047506a03d21739b5b922f6b3fd9f059b0b137c5
Author: Werner Koch <wk at gnupg.org>
Date:   Wed Feb 28 08:36:46 2018 +0100

    gpg: Merge two functions in cipher-aead.c
    
    * g10/cipher-aead.c (set_nonce, set_additional_data): Merge into ...
    (set_nonce_and_ad): new function.
    (write_auth_tag): Print error message here.
    (do_flush): Rename var newchunk to finalize.
    --
    
    There is no need to have separate functions here.  We should also
    print a error message for writing the final tag.
    
    Signed-off-by: Werner Koch <wk at gnupg.org>

diff --git a/g10/cipher-aead.c b/g10/cipher-aead.c
index cc306f9..1457baa 100644
--- a/g10/cipher-aead.c
+++ b/g10/cipher-aead.c
@@ -55,49 +55,16 @@ my_iobuf_write (iobuf_t a, const void *buffer, size_t buflen)
 }
 
 
-/* Set the additional data for the current chunk.  If FINAL is set the
- * final AEAD chunk is processed.  */
+/* Set the nonce and the additional data for the current chunk.  If
+ * FINAL is set the final AEAD chunk is processed.  This also reset
+ * the encryption machinery so that the handle can be used for a new
+ * chunk.  */
 static gpg_error_t
-set_additional_data (cipher_filter_context_t *cfx, int final)
-{
-  unsigned char ad[21];
-
-  ad[0] = (0xc0 | PKT_ENCRYPTED_AEAD);
-  ad[1] = 1;
-  ad[2] = cfx->dek->algo;
-  ad[3] = cfx->dek->use_aead;
-  ad[4] = cfx->chunkbyte;
-  ad[5] = cfx->chunkindex >> 56;
-  ad[6] = cfx->chunkindex >> 48;
-  ad[7] = cfx->chunkindex >> 40;
-  ad[8] = cfx->chunkindex >> 32;
-  ad[9] = cfx->chunkindex >> 24;
-  ad[10]= cfx->chunkindex >> 16;
-  ad[11]= cfx->chunkindex >>  8;
-  ad[12]= cfx->chunkindex;
-  if (final)
-    {
-      ad[13] = cfx->total >> 56;
-      ad[14] = cfx->total >> 48;
-      ad[15] = cfx->total >> 40;
-      ad[16] = cfx->total >> 32;
-      ad[17] = cfx->total >> 24;
-      ad[18] = cfx->total >> 16;
-      ad[19] = cfx->total >>  8;
-      ad[20] = cfx->total;
-    }
-  if (DBG_CRYPTO)
-    log_printhex (ad, final? 21 : 13, "authdata:");
-  return gcry_cipher_authenticate (cfx->cipher_hd, ad, final? 21 : 13);
-}
-
-
-/* Set the nonce.  This also reset the encryption machinery so that
- * the handle can be used for a new chunk.  */
-static gpg_error_t
-set_nonce (cipher_filter_context_t *cfx)
+set_nonce_and_ad (cipher_filter_context_t *cfx, int final)
 {
+  gpg_error_t err;
   unsigned char nonce[16];
+  unsigned char ad[21];
   int i;
 
   switch (cfx->dek->use_aead)
@@ -127,7 +94,37 @@ set_nonce (cipher_filter_context_t *cfx)
 
   if (DBG_CRYPTO)
     log_printhex (nonce, 15, "nonce:");
-  return gcry_cipher_setiv (cfx->cipher_hd, nonce, i);
+  err = gcry_cipher_setiv (cfx->cipher_hd, nonce, i);
+  if (err)
+    return err;
+
+  ad[0] = (0xc0 | PKT_ENCRYPTED_AEAD);
+  ad[1] = 1;
+  ad[2] = cfx->dek->algo;
+  ad[3] = cfx->dek->use_aead;
+  ad[4] = cfx->chunkbyte;
+  ad[5] = cfx->chunkindex >> 56;
+  ad[6] = cfx->chunkindex >> 48;
+  ad[7] = cfx->chunkindex >> 40;
+  ad[8] = cfx->chunkindex >> 32;
+  ad[9] = cfx->chunkindex >> 24;
+  ad[10]= cfx->chunkindex >> 16;
+  ad[11]= cfx->chunkindex >>  8;
+  ad[12]= cfx->chunkindex;
+  if (final)
+    {
+      ad[13] = cfx->total >> 56;
+      ad[14] = cfx->total >> 48;
+      ad[15] = cfx->total >> 40;
+      ad[16] = cfx->total >> 32;
+      ad[17] = cfx->total >> 24;
+      ad[18] = cfx->total >> 16;
+      ad[19] = cfx->total >>  8;
+      ad[20] = cfx->total;
+    }
+  if (DBG_CRYPTO)
+    log_printhex (ad, final? 21 : 13, "authdata:");
+  return gcry_cipher_authenticate (cfx->cipher_hd, ad, final? 21 : 13);
 }
 
 
@@ -203,14 +200,9 @@ write_header (cipher_filter_context_t *cfx, iobuf_t a)
   if (err)
     return err;
 
-  err = set_nonce (cfx);
+  err = set_nonce_and_ad (cfx, 0);
   if (err)
     return err;
-
-  err = set_additional_data (cfx, 0);
-  if (err)
-    return err;
-
   cfx->wrote_header = 1;
 
  leave:
@@ -233,6 +225,8 @@ write_auth_tag (cipher_filter_context_t *cfx, iobuf_t a)
     goto leave;
 
  leave:
+  if (err)
+    log_error ("write_auth_tag failed: %s\n", gpg_strerror (err));
   return err;
 }
 
@@ -247,10 +241,7 @@ write_final_chunk (cipher_filter_context_t *cfx, iobuf_t a)
   if (cfx->chunklen)
     cfx->chunkindex++;
 
-  err = set_nonce (cfx);
-  if (err)
-    goto leave;
-  err = set_additional_data (cfx, 1);
+  err = set_nonce_and_ad (cfx, 1);
   if (err)
     goto leave;
 
@@ -273,7 +264,7 @@ static gpg_error_t
 do_flush (cipher_filter_context_t *cfx, iobuf_t a, byte *buf, size_t size)
 {
   gpg_error_t err = 0;
-  int newchunk = 0;
+  int finalize = 0;
   size_t n;
 
   /* Put the data into a buffer, flush and encrypt as needed.  */
@@ -289,7 +280,7 @@ do_flush (cipher_filter_context_t *cfx, iobuf_t a, byte *buf, size_t size)
       if (cfx->chunklen + cfx->buflen + n >= cfx->chunksize)
         {
           size_t n1 = cfx->chunksize - (cfx->chunklen + cfx->buflen);
-          newchunk = 1;
+          finalize = 1;
           if (DBG_FILTER)
             log_debug ("chunksize %ju reached;"
                        " cur buflen=%zu using %zu of %zu\n",
@@ -303,20 +294,21 @@ do_flush (cipher_filter_context_t *cfx, iobuf_t a, byte *buf, size_t size)
       buf  += n;
       size -= n;
 
-      if (cfx->buflen == cfx->bufsize || newchunk)
+      if (cfx->buflen == cfx->bufsize || finalize)
         {
           if (DBG_FILTER)
             log_debug ("encrypting: buflen=%zu %s n=%zu\n",
-                       cfx->buflen, newchunk?"(newchunk)":"", n);
-          if (newchunk)
+                       cfx->buflen, finalize?"(finalize)":"", n);
+          if (finalize)
             gcry_cipher_final (cfx->cipher_hd);
-          if (!DBG_FILTER)
-            ;
-          else if (newchunk)
-            log_printhex (cfx->buffer, cfx->buflen, "plain(1):");
-          else if (cfx->buflen > 32)
-            log_printhex (cfx->buffer + cfx->buflen - 32, 32,
-                          "plain(last 32):");
+          if (DBG_FILTER)
+            {
+              if (finalize)
+                log_printhex (cfx->buffer, cfx->buflen, "plain(1):");
+              else if (cfx->buflen > 32)
+                log_printhex (cfx->buffer + cfx->buflen - 32, 32,
+                              "plain(last32):");
+            }
 
           /* Take care: even with a buflen of zero an encrypt needs to
            * be called after gcry_cipher_final and before
@@ -324,7 +316,7 @@ do_flush (cipher_filter_context_t *cfx, iobuf_t a, byte *buf, size_t size)
            * mode.  */
           gcry_cipher_encrypt (cfx->cipher_hd, cfx->buffer, cfx->buflen,
                                NULL, 0);
-          if (newchunk && DBG_FILTER)
+          if (finalize && DBG_FILTER)
             log_printhex (cfx->buffer, cfx->buflen, "ciphr(1):");
           err = my_iobuf_write (a, cfx->buffer, cfx->buflen);
           if (err)
@@ -333,30 +325,23 @@ do_flush (cipher_filter_context_t *cfx, iobuf_t a, byte *buf, size_t size)
           cfx->total += cfx->buflen;
           cfx->buflen = 0;
 
-          if (newchunk)
+          if (finalize)
             {
               if (DBG_FILTER)
                 log_debug ("chunklen=%ju  total=%ju\n",
                            (uintmax_t)cfx->chunklen, (uintmax_t)cfx->total);
               err = write_auth_tag (cfx, a);
               if (err)
-                {
-                  log_error ("gcry_cipher_gettag failed: %s\n",
-                             gpg_strerror (err));
-                  goto leave;
-                }
+                goto leave;
 
               if (DBG_FILTER)
                 log_debug ("starting a new chunk (cur size=%zu)\n", size);
               cfx->chunkindex++;
               cfx->chunklen = 0;
-              err = set_nonce (cfx);
-              if (err)
-                goto leave;
-              err = set_additional_data (cfx, 0);
+              err = set_nonce_and_ad (cfx, 0);
               if (err)
                 goto leave;
-              newchunk = 0;
+              finalize = 0;
             }
         }
     }

-----------------------------------------------------------------------

Summary of changes:
 g10/cipher-aead.c  | 210 +++++++++++++++++++++++++----------------------------
 g10/decrypt-data.c |  16 +++-
 2 files changed, 112 insertions(+), 114 deletions(-)


hooks/post-receive
-- 
The GNU Privacy Guard
http://git.gnupg.org




More information about the Gnupg-commits mailing list