[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