security-and-privacy/gnupg
security-and-privacy/gnupg
1
0
Fork 0
mirror of git://git.gnupg.org/gnupg.git synced 2024-12-22 10:19:57 +01:00
Code Activity

gpg: Avoid writing a zero length last chunk in AEAD mode.

Browse Source 
* 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@gnupg.org>
This commit is contained in:
Werner Koch 2018-02-28 09:31:39 +01:00
parent 047506a03d
commit f2c09203b9
No known key found for this signature in database
GPG Key ID: E3FDFF218E45B72B
2 changed files with 55 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
g10/cipher-aead.c
View File

@ -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)

16
g10/decrypt-data.c
View File

@ -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);