[OTR-dev] Issues with libgcrypt 1.5
Ian Goldberg
ian at cypherpunks.ca
Mon Apr 11 20:20:23 CEST 2011
On Mon, Apr 11, 2011 at 07:31:55PM +0200, Werner Koch wrote:
> On Mon, 11 Apr 2011 19:03, ian at cypherpunks.ca said:
>
> > If you do this with x = 0,1,7,8,9,15,16,17,22, that should be a good set
> > of test cases, I think.
>
> I was more thinking of a testcase you use in libotr. Anyway, I added a
> simple truncation tests which was easy to add to the existsing tests.
> Find attached a patch.
Excellent, thanks! I'll have Rob test OTR with this patch, and I'll let
you know whether it all works now.
- Ian
> >From 3c18377a55085faf4df745034056bac53565effa Mon Sep 17 00:00:00 2001
> From: Werner Koch <wk at gnupg.org>
> Date: Mon, 11 Apr 2011 19:21:47 +0200
> Subject: [PATCH] Allow for truncation in CTR mode.
>
> This re-enables the behaviour of Libgcrypt 1.4. Such truncation is
> used by libotr and the current error-ed out here. The bug was
> introduced due to a rewrite of the function and the undocumented
> feature of truncating OTR data.
> ---
> cipher/ChangeLog | 5 ++
> cipher/cipher.c | 12 ++--
> tests/ChangeLog | 6 ++
> tests/basic.c | 136 ++++++++++++++++++++++++++++++++++++++++++-----------
> 4 files changed, 124 insertions(+), 35 deletions(-)
>
> diff --git a/cipher/ChangeLog b/cipher/ChangeLog
> index df27bab..4cde857 100644
> --- a/cipher/ChangeLog
> +++ b/cipher/ChangeLog
> @@ -1,3 +1,8 @@
> +2011-04-11 Werner Koch <wk at g10code.com>
> +
> + * cipher.c (do_ctr_encrypt): Allow arbitrary length inputs to
> + match the 1.4 behaviour.
> +
> 2011-04-04 Werner Koch <wk at g10code.com>
>
> * ecc.c (compute_keygrip): Release L1 while parsing "curve".
> diff --git a/cipher/cipher.c b/cipher/cipher.c
> index a2f8bb9..e5bb2e0 100644
> --- a/cipher/cipher.c
> +++ b/cipher/cipher.c
> @@ -1453,22 +1453,22 @@ do_ctr_encrypt (gcry_cipher_hd_t c,
> unsigned int blocksize = c->cipher->blocksize;
> unsigned int nblocks;
>
> - /* FIXME: This code does only work on complete blocks. */
> -
> if (outbuflen < inbuflen)
> return GPG_ERR_BUFFER_TOO_SHORT;
>
> - if ((inbuflen % blocksize))
> - return GPG_ERR_INV_LENGTH;
> -
> + /* Use a bulk method if available. */
> nblocks = inbuflen / blocksize;
> if (nblocks && c->bulk.ctr_enc)
> {
> c->bulk.ctr_enc (&c->context.c, c->u_ctr.ctr, outbuf, inbuf, nblocks);
> inbuf += nblocks * blocksize;
> outbuf += nblocks * blocksize;
> + inbuflen -= nblocks * blocksize;
> }
> - else
> +
> + /* If we don't have a bulk method use the standard method. We also
> + use this method for the a remaining partial block. */
> + if (inbuflen)
> {
> unsigned char tmp[MAX_BLOCKSIZE];
>
> diff --git a/tests/ChangeLog b/tests/ChangeLog
> index 0f5918a..3793149 100644
> --- a/tests/ChangeLog
> +++ b/tests/ChangeLog
> @@ -1,3 +1,9 @@
> +2011-04-11 Werner Koch <wk at g10code.com>
> +
> + * basic.c (mismatch): New.
> + (check_ctr_cipher): Remove length error code checks. Add
> + truncation checks.
> +
> 2011-04-04 Werner Koch <wk at g10code.com>
>
> * keygrip.c (main): Add option --repetitions.
> diff --git a/tests/basic.c b/tests/basic.c
> index 185091e..a20e731 100644
> --- a/tests/basic.c
> +++ b/tests/basic.c
> @@ -69,6 +69,22 @@ fail (const char *format, ...)
> }
>
> static void
> +mismatch (const void *expected, size_t expectedlen,
> + const void *computed, size_t computedlen)
> +{
> + const unsigned char *p;
> +
> + fprintf (stderr, "expected:");
> + for (p = expected; expectedlen; p++, expectedlen--)
> + fprintf (stderr, " %02x", *p);
> + fprintf (stderr, "\ncomputed:");
> + for (p = computed; computedlen; p++, computedlen--)
> + fprintf (stderr, " %02x", *p);
> + fprintf (stderr, "\n");
> +}
> +
> +
> +static void
> die (const char *format, ...)
> {
> va_list arg_ptr;
> @@ -349,8 +365,7 @@ check_ctr_cipher (void)
> unsigned char plaintext[MAX_DATA_LEN];
> int inlen;
> char out[MAX_DATA_LEN];
> - }
> - data[MAX_DATA_LEN];
> + } data[5];
> } tv[] =
> {
> /* http://csrc.nist.gov/publications/nistpubs/800-38a/sp800-38a.pdf */
> @@ -369,6 +384,8 @@ check_ctr_cipher (void)
> { "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
> 16,
> "\x1e\x03\x1d\xda\x2f\xbe\x03\xd1\x79\x21\x70\xa0\xf3\x00\x9c\xee" },
> +
> + { "", 0, "" }
> }
> },
> { GCRY_CIPHER_AES192,
> @@ -387,6 +404,7 @@ check_ctr_cipher (void)
> { "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
> 16,
> "\x4f\x78\xa7\xf6\xd2\x98\x09\x58\x5a\x97\xda\xec\x58\xc6\xb0\x50" },
> + { "", 0, "" }
> }
> },
> { GCRY_CIPHER_AES256,
> @@ -404,7 +422,80 @@ check_ctr_cipher (void)
> "\x2b\x09\x30\xda\xa2\x3d\xe9\x4c\xe8\x70\x17\xba\x2d\x84\x98\x8d" },
> { "\xf6\x9f\x24\x45\xdf\x4f\x9b\x17\xad\x2b\x41\x7b\xe6\x6c\x37\x10",
> 16,
> - "\xdf\xc9\xc5\x8d\xb6\x7a\xad\xa6\x13\xc2\xdd\x08\x45\x79\x41\xa6" }
> + "\xdf\xc9\xc5\x8d\xb6\x7a\xad\xa6\x13\xc2\xdd\x08\x45\x79\x41\xa6" },
> + { "", 0, "" }
> + }
> + },
> + /* Some truncation tests. With a truncated second block and
> + also with a single truncated block. */
> + { GCRY_CIPHER_AES,
> + "\x2b\x7e\x15\x16\x28\xae\xd2\xa6\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
> + "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff",
> + {{"\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96\xe9\x3d\x7e\x11\x73\x93\x17\x2a",
> + 16,
> + "\x87\x4d\x61\x91\xb6\x20\xe3\x26\x1b\xef\x68\x64\x99\x0d\xb6\xce" },
> + {"\xae\x2d\x8a\x57\x1e\x03\xac\x9c\x9e\xb7\x6f\xac\x45\xaf\x8e",
> + 15,
> + "\x98\x06\xf6\x6b\x79\x70\xfd\xff\x86\x17\x18\x7b\xb9\xff\xfd" },
> + {"", 0, "" }
> + }
> + },
> + { GCRY_CIPHER_AES,
> + "\x2b\x7e\x15\x16\x28\xae\xd2\xa6\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
> + "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff",
> + {{"\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96\xe9\x3d\x7e\x11\x73\x93\x17\x2a",
> + 16,
> + "\x87\x4d\x61\x91\xb6\x20\xe3\x26\x1b\xef\x68\x64\x99\x0d\xb6\xce" },
> + {"\xae",
> + 1,
> + "\x98" },
> + {"", 0, "" }
> + }
> + },
> + { GCRY_CIPHER_AES,
> + "\x2b\x7e\x15\x16\x28\xae\xd2\xa6\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
> + "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff",
> + {{"\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96\xe9\x3d\x7e\x11\x73\x93\x17",
> + 15,
> + "\x87\x4d\x61\x91\xb6\x20\xe3\x26\x1b\xef\x68\x64\x99\x0d\xb6" },
> + {"", 0, "" }
> + }
> + },
> + { GCRY_CIPHER_AES,
> + "\x2b\x7e\x15\x16\x28\xae\xd2\xa6\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
> + "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff",
> + {{"\x6b",
> + 1,
> + "\x87" },
> + {"", 0, "" }
> + }
> + },
> +#if USE_CAST5
> + /* A selfmade test vector using an 64 bit block cipher. */
> + { GCRY_CIPHER_CAST5,
> + "\x2b\x7e\x15\x16\x28\xae\xd2\xa6\xab\xf7\x15\x88\x09\xcf\x4f\x3c",
> + "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8",
> + {{"\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96\xe9\x3d\x7e\x11\x73\x93\x17\x2a",
> + 16,
> + "\xe8\xa7\xac\x68\xca\xca\xa0\x20\x10\xcb\x1b\xcc\x79\x2c\xc4\x48" },
> + {"\xae\x2d\x8a\x57\x1e\x03\xac\x9c",
> + 8,
> + "\x16\xe8\x72\x77\xb0\x98\x29\x68" },
> + {"\x9e\xb7\x6f\xac\x45\xaf\x8e\x51",
> + 8,
> + "\x9a\xb3\xa8\x03\x3b\xb4\x14\xba" },
> + {"\xae\x2d\x8a\x57\x1e\x03\xac\x9c\xa1\x00",
> + 10,
> + "\x31\x5e\xd3\xfb\x1b\x8d\xd1\xf9\xb0\x83" },
> + { "", 0, "" }
> + }
> + },
> +#endif /*USE_CAST5*/
> + { 0,
> + "",
> + "",
> + {
> + {"", 0, "" }
> }
> }
> };
> @@ -417,6 +508,9 @@ check_ctr_cipher (void)
> fprintf (stderr, " Starting CTR cipher checks.\n");
> for (i = 0; i < sizeof (tv) / sizeof (tv[0]); i++)
> {
> + if (!tv[i].algo)
> + continue;
> +
> err = gcry_cipher_open (&hde, tv[i].algo, GCRY_CIPHER_MODE_CTR, 0);
> if (!err)
> err = gcry_cipher_open (&hdd, tv[i].algo, GCRY_CIPHER_MODE_CTR, 0);
> @@ -485,7 +579,11 @@ check_ctr_cipher (void)
> }
>
> if (memcmp (tv[i].data[j].out, out, tv[i].data[j].inlen))
> - fail ("aes-ctr, encrypt mismatch entry %d:%d\n", i, j);
> + {
> + fail ("aes-ctr, encrypt mismatch entry %d:%d\n", i, j);
> + mismatch (tv[i].data[j].out, tv[i].data[j].inlen,
> + out, tv[i].data[j].inlen);
> + }
>
> err = gcry_cipher_decrypt (hdd, out, tv[i].data[j].inlen, NULL, 0);
> if (err)
> @@ -498,7 +596,11 @@ check_ctr_cipher (void)
> }
>
> if (memcmp (tv[i].data[j].plaintext, out, tv[i].data[j].inlen))
> - fail ("aes-ctr, decrypt mismatch entry %d:%d\n", i, j);
> + {
> + fail ("aes-ctr, decrypt mismatch entry %d:%d\n", i, j);
> + mismatch (tv[i].data[j].plaintext, tv[i].data[j].inlen,
> + out, tv[i].data[j].inlen);
> + }
>
> }
>
> @@ -509,18 +611,6 @@ check_ctr_cipher (void)
> if (err)
> fail ("aes-ctr, encryption failed for valid input");
>
> - err = gcry_cipher_encrypt (hde, out, MAX_DATA_LEN,
> - "1234567890123456", 15);
> - if (gpg_err_code (err) != GPG_ERR_INV_LENGTH)
> - fail ("aes-ctr, too short input returned wrong error: %s\n",
> - gpg_strerror (err));
> -
> - err = gcry_cipher_encrypt (hde, out, MAX_DATA_LEN,
> - "12345678901234567", 17);
> - if (gpg_err_code (err) != GPG_ERR_INV_LENGTH)
> - fail ("aes-ctr, too long input returned wrong error: %s\n",
> - gpg_strerror (err));
> -
> err = gcry_cipher_encrypt (hde, out, 15,
> "1234567890123456", 16);
> if (gpg_err_code (err) != GPG_ERR_BUFFER_TOO_SHORT)
> @@ -545,18 +635,6 @@ check_ctr_cipher (void)
> if (err)
> fail ("aes-ctr, decryption failed for valid input");
>
> - err = gcry_cipher_decrypt (hde, out, MAX_DATA_LEN,
> - "1234567890123456", 15);
> - if (gpg_err_code (err) != GPG_ERR_INV_LENGTH)
> - fail ("aes-ctr, too short input returned wrong error: %s\n",
> - gpg_strerror (err));
> -
> - err = gcry_cipher_decrypt (hde, out, MAX_DATA_LEN,
> - "12345678901234567", 17);
> - if (gpg_err_code (err) != GPG_ERR_INV_LENGTH)
> - fail ("aes-ctr, too long input returned wrong error: %s\n",
> - gpg_strerror (err));
> -
> err = gcry_cipher_decrypt (hde, out, 15,
> "1234567890123456", 16);
> if (gpg_err_code (err) != GPG_ERR_BUFFER_TOO_SHORT)
> --
> 1.7.2.3
>
More information about the Gcrypt-devel
mailing list