[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