[git] GnuPG - branch, master, updated. gnupg-2.2.5-109-gb703ba7
by Werner Koch
cvs at cvs.gnupg.org
Tue Feb 27 14:04:29 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 b703ba725dadca8298a0c69365225f9a7ff60ae2 (commit)
via ebb0fcf6e0bd6997eff4097ddda94955134212af (commit)
from cbc7bacf2ff95aebb427bb244c719143a9001f3c (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 b703ba725dadca8298a0c69365225f9a7ff60ae2
Author: Werner Koch <wk at gnupg.org>
Date: Tue Feb 27 13:57:24 2018 +0100
gpg: Rename cipher.c to cipher-cfb.c
* g10/cipher.c: Rename to ...
* g10/cipher-cfb.c: this.
--
Signed-off-by: Werner Koch <wk at gnupg.org>
diff --git a/g10/Makefile.am b/g10/Makefile.am
index cba65b2..b8b92d7 100644
--- a/g10/Makefile.am
+++ b/g10/Makefile.am
@@ -131,7 +131,7 @@ gpg_sources = server.c \
passphrase.c \
decrypt.c \
decrypt-data.c \
- cipher.c \
+ cipher-cfb.c \
cipher-aead.c \
encrypt.c \
sign.c \
diff --git a/g10/cipher.c b/g10/cipher-cfb.c
similarity index 98%
rename from g10/cipher.c
rename to g10/cipher-cfb.c
index ad7399d..79b21bd 100644
--- a/g10/cipher.c
+++ b/g10/cipher-cfb.c
@@ -1,4 +1,4 @@
-/* cipher.c - Enciphering filter for the old CFB mode.
+/* cipher-cfb.c - Enciphering filter for the old CFB mode.
* Copyright (C) 1998-2003, 2006, 2009 Free Software Foundation, Inc.
* Copyright (C) 1998-2003, 2006, 2009, 2017 Werner koch
*
commit ebb0fcf6e0bd6997eff4097ddda94955134212af
Author: Werner Koch <wk at gnupg.org>
Date: Tue Feb 27 13:53:52 2018 +0100
gpg: Fix corner cases in AEAD encryption.
* g10/cipher-aead.c (write_final_chunk): Do not bump up the chunk
index if the previous chunk was empty.
* g10/decrypt-data.c (aead_underflow): Likewise. Also handle a other
corner cases. Add more debug output.
--
GnuPG-bug-id: 3774
This fixes the reported case when the encrypted data is a multiple of
the chunk size. Then the chunk index for the final chunk was wrongly
incremented by 2. The actual fix makes use of the fact that the
current dfx->CHUNKLEN is 0 in this case. There is also some other
reorganizing to help with debugging. The thing seems to work now but
the code is not very clean - should be reworked. Creating test files
can be done with this script:
--8<---------------cut here---------------start------------->8---
csize=6
for len in 0 55 56 57; do
awk </dev/null -v i=$len 'BEGIN{while(i){i--;printf"~"}}' \
| gpg --no-options -v --rfc4880bis --batch --passphrase "abc" \
--s2k-count 1025 --s2k-digest-algo sha256 -z0 \
--force-aead --aead-algo eax --cipher aes -a \
--chunk-size $csize -c >symenc-aead-eax-c$csize-$len.asc
done
--8<---------------cut here---------------end--------------->8---
A LEN of 56 triggered the bug which can be seen by looking at the
"authdata:" line in the --debug=crypt,filter output.
Signed-off-by: Werner Koch <wk at gnupg.org>
diff --git a/g10/cipher-aead.c b/g10/cipher-aead.c
index 573bb43..cc306f9 100644
--- a/g10/cipher-aead.c
+++ b/g10/cipher-aead.c
@@ -244,7 +244,8 @@ write_final_chunk (cipher_filter_context_t *cfx, iobuf_t a)
gpg_error_t err;
char dummy[1];
- cfx->chunkindex++;
+ if (cfx->chunklen)
+ cfx->chunkindex++;
err = set_nonce (cfx);
if (err)
diff --git a/g10/decrypt-data.c b/g10/decrypt-data.c
index afdedcb..0b0051a 100644
--- a/g10/decrypt-data.c
+++ b/g10/decrypt-data.c
@@ -541,6 +541,7 @@ aead_underflow (decode_filter_ctx_t dfx, iobuf_t a, byte *buf, size_t *ret_len)
size_t totallen = 0; /* The number of bytes to return on success or EOF. */
size_t off = 0; /* The offset into the buffer. */
size_t len; /* The current number of bytes in BUF+OFF. */
+ int last_chunk_done = 0; /* Flag that we processed the last chunk. */
int c;
log_assert (size > 48); /* Our code requires at least this size. */
@@ -551,16 +552,16 @@ aead_underflow (decode_filter_ctx_t dfx, iobuf_t a, byte *buf, size_t *ret_len)
memcpy (buf, dfx->holdback, len);
if (DBG_FILTER)
- log_debug ("aead_underflow: size=%zu len=%zu%s\n",
- size, len, dfx->eof_seen? " eof":"");
+ log_debug ("aead_underflow: size=%zu len=%zu%s%s\n", size, len,
+ dfx->partial? " partial":"", dfx->eof_seen? " eof":"");
- /* Read and fill up BUF. We need to watchout for an EOF so that we
+ /* Read and fill up BUF. We need to watch out for an EOF so that we
* can detect the last chunk which is commonly shorter than the
* chunksize. After the last data byte from the last chunk 32 more
* bytes are expected for the last chunk's tag and the following
- * final chunk's tag. To detect the EOF we need to read at least
+ * final chunk's tag. To detect the EOF we need to try reading at least
* one further byte; however we try to ready 16 extra bytes to avoid
- * singel byte reads in some lower layers. The outcome is that we
+ * single byte reads in some lower layers. The outcome is that we
* have up to 48 extra extra octets which we will later put into the
* holdback buffer for the next invocation (which handles the EOF
* case). */
@@ -648,56 +649,72 @@ aead_underflow (decode_filter_ctx_t dfx, iobuf_t a, byte *buf, size_t *ret_len)
len -= n;
if (DBG_FILTER)
- log_debug ("bytes left: %zu at off=%zu\n", len, off);
+ log_debug ("ndecrypted: %zu (nchunk=%zu) bytes left: %zu at off=%zu\n",
+ totallen, dfx->chunklen, len, off);
/* Check the tag. */
if (len < 16)
{
/* The tag is not entirely in the buffer. Read the rest of
- * the tag from the holdback buffer. The shift the holdback
+ * the tag from the holdback buffer. Then shift the holdback
* buffer and fill it up again. */
memcpy (tagbuf, buf+off, len);
memcpy (tagbuf + len, dfx->holdback, 16 - len);
dfx->holdbacklen -= 16-len;
memmove (dfx->holdback, dfx->holdback + (16-len), dfx->holdbacklen);
- len = dfx->holdbacklen;
- if (dfx->partial)
+ if (dfx->eof_seen)
{
- for (; len < 48; len++ )
+ /* We should have the last chunk's tag in TAGBUF and the
+ * final tag in HOLDBACKBUF. */
+ if (len || dfx->holdbacklen != 16)
{
- if ((c = iobuf_get (a)) == -1)
- {
- dfx->eof_seen = 1; /* Normal EOF. */
- break;
- }
- dfx->holdback[len] = c;
+ /* Not enough data for the last two tags. */
+ err = gpg_error (GPG_ERR_TRUNCATED);
+ goto leave;
}
+ len = 0;
+ last_chunk_done = 1;
}
else
{
- for (; len < 48 && dfx->length; len++, dfx->length--)
+ len = dfx->holdbacklen;
+ if (dfx->partial)
{
- c = iobuf_get (a);
- if (c == -1)
+ for (; len < 48; len++ )
{
- dfx->eof_seen = 3; /* Premature EOF. */
- break;
+ if ((c = iobuf_get (a)) == -1)
+ {
+ dfx->eof_seen = 1; /* Normal EOF. */
+ break;
+ }
+ dfx->holdback[len] = c;
}
- dfx->holdback[len] = c;
}
- if (!dfx->length)
- dfx->eof_seen = 1; /* Normal EOF. */
- }
- if (len < 32)
- {
- /* Not enough data for the last two tags. */
- err = gpg_error (GPG_ERR_TRUNCATED);
- goto leave;
+ else
+ {
+ for (; len < 48 && dfx->length; len++, dfx->length--)
+ {
+ c = iobuf_get (a);
+ if (c == -1)
+ {
+ dfx->eof_seen = 3; /* Premature EOF. */
+ break;
+ }
+ dfx->holdback[len] = c;
+ }
+ if (!dfx->length)
+ dfx->eof_seen = 1; /* Normal EOF. */
+ }
+ if (len < 32)
+ {
+ /* Not enough data for the last two tags. */
+ err = gpg_error (GPG_ERR_TRUNCATED);
+ goto leave;
+ }
+ dfx->holdbacklen = len;
+ len = 0;
}
- dfx->holdbacklen = len;
- /* log_printhex (dfx->holdback, dfx->holdbacklen, "holdback:"); */
- len = 0;
}
else /* We already have the full tag. */
{
@@ -716,54 +733,73 @@ aead_underflow (decode_filter_ctx_t dfx, iobuf_t a, byte *buf, size_t *ret_len)
gpg_strerror (err));
goto leave;
}
+ if (DBG_FILTER)
+ log_debug ("tag is valid\n");
/* Prepare a new chunk. */
- dfx->chunklen = 0;
- dfx->chunkindex++;
- err = aead_set_nonce (dfx);
- if (err)
- goto leave;
- err = aead_set_ad (dfx, 0);
- if (err)
- goto leave;
+ if (!last_chunk_done)
+ {
+ dfx->chunklen = 0;
+ dfx->chunkindex++;
+ err = aead_set_nonce (dfx);
+ if (err)
+ goto leave;
+ err = aead_set_ad (dfx, 0);
+ if (err)
+ goto leave;
+ }
continue;
}
- if (dfx->eof_seen)
- {
- /* This is the last block of the last chunk. Its length may
- * not be a multiple of the block length. */
- gcry_cipher_final (dfx->cipher_hd);
- }
- err = gcry_cipher_decrypt (dfx->cipher_hd, buf + off, len, NULL, 0);
- if (err)
+ if (!last_chunk_done)
{
- log_error ("gcry_cipher_decrypt failed (2): %s\n", gpg_strerror (err));
- goto leave;
+ if (dfx->eof_seen)
+ {
+ /* This is the last block of the last chunk. Its length may
+ * not be a multiple of the block length. */
+ gcry_cipher_final (dfx->cipher_hd);
+ }
+ err = gcry_cipher_decrypt (dfx->cipher_hd, buf + off, len, NULL, 0);
+ if (err)
+ {
+ log_error ("gcry_cipher_decrypt failed (2): %s\n",
+ gpg_strerror (err));
+ goto leave;
+ }
+ totallen += len;
+ dfx->chunklen += len;
+ dfx->total += len;
+ if (DBG_FILTER)
+ log_debug ("ndecrypted: %zu (nchunk=%zu)\n", totallen, dfx->chunklen);
}
- totallen += len;
- dfx->chunklen += len;
- dfx->total += len;
+
if (dfx->eof_seen)
{
if (DBG_FILTER)
- log_debug ("eof seen: holdback buffer has the tags.\n");
+ log_debug ("eof seen: holdback buffer has the %s.\n",
+ last_chunk_done? "final tag":"last and final tag");
- log_assert (dfx->holdbacklen >= 32);
-
- if (DBG_FILTER)
- log_printhex (dfx->holdback, 16, "tag:");
- err = gcry_cipher_checktag (dfx->cipher_hd, dfx->holdback, 16);
- if (err)
+ if (!last_chunk_done)
{
- log_error ("gcry_cipher_checktag failed (2): %s\n",
- gpg_strerror (err));
- goto leave;
+ log_assert (dfx->holdbacklen >= 32);
+
+ if (DBG_FILTER)
+ log_printhex (dfx->holdback, 16, "tag:");
+ err = gcry_cipher_checktag (dfx->cipher_hd, dfx->holdback, 16);
+ if (err)
+ {
+ log_error ("gcry_cipher_checktag failed (2): %s\n",
+ gpg_strerror (err));
+ goto leave;
+ }
+ if (DBG_FILTER)
+ log_debug ("tag is valid\n");
}
/* Check the final chunk. */
- dfx->chunkindex++;
+ if (dfx->chunklen)
+ dfx->chunkindex++;
err = aead_set_nonce (dfx);
if (err)
goto leave;
@@ -771,7 +807,7 @@ aead_underflow (decode_filter_ctx_t dfx, iobuf_t a, byte *buf, size_t *ret_len)
if (err)
goto leave;
gcry_cipher_final (dfx->cipher_hd);
- /* decrypt an empty string. */
+ /* Decrypt an empty string. */
err = gcry_cipher_decrypt (dfx->cipher_hd, dfx->holdback, 0, NULL, 0);
if (err)
{
@@ -779,8 +815,10 @@ aead_underflow (decode_filter_ctx_t dfx, iobuf_t a, byte *buf, size_t *ret_len)
gpg_strerror (err));
goto leave;
}
- /* log_printhex (dfx->holdback+16, 16, "tag:"); */
- err = gcry_cipher_checktag (dfx->cipher_hd, dfx->holdback+16, 16);
+ if (DBG_CRYPTO)
+ log_printhex (dfx->holdback+(last_chunk_done?0:16), 16, "tag:");
+ err = gcry_cipher_checktag (dfx->cipher_hd,
+ dfx->holdback+(last_chunk_done?0:16), 16);
if (err)
{
if (DBG_FILTER)
@@ -788,6 +826,8 @@ aead_underflow (decode_filter_ctx_t dfx, iobuf_t a, byte *buf, size_t *ret_len)
gpg_strerror (err));
goto leave;
}
+ if (DBG_FILTER)
+ log_debug ("final tag is valid\n");
err = gpg_error (GPG_ERR_EOF);
}
-----------------------------------------------------------------------
Summary of changes:
g10/Makefile.am | 2 +-
g10/cipher-aead.c | 3 +-
g10/{cipher.c => cipher-cfb.c} | 2 +-
g10/decrypt-data.c | 176 +++++++++++++++++++++++++----------------
4 files changed, 112 insertions(+), 71 deletions(-)
rename g10/{cipher.c => cipher-cfb.c} (98%)
hooks/post-receive
--
The GNU Privacy Guard
http://git.gnupg.org
More information about the Gnupg-commits
mailing list