[openssl.org #3457] Possible Bugs in EVP_KeyToBytes?

2014-07-19 Thread Matt Caswell via RT
Closing this ticket in favour of PR#3462.

Matt

__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


[openssl.org #3457] Possible Bugs in EVP_KeyToBytes?

2014-07-15 Thread noloa...@gmail.com via RT
Below is from crypto/evp/evp_key.c.

Notice that `addmd` is never set to 1. *If* the routine needs to loop
back to the top to finish fulfilling a derivation request, then the
previous hash is *not* added back into the computation. That is, this
is never executed:

if (addmd++)
if (!EVP_DigestUpdate(c,(md_buf[0]),mds))

I believe that means the same bit pattern repeats. If MD5 is the
underlying hash (which appears to be used fairly regularly), then the
bits repeat after 16 bytes.

Also, the early out from `if (data == NULL) return(nkey);` should
probably return something other than a successful result. Since
nothing was derived, the function should return 0; and not nkey.

*

int EVP_BytesToKey(const EVP_CIPHER *type, const EVP_MD *md,
   const unsigned char *salt, const unsigned char
*data, int datal,
   int count, unsigned char *key, unsigned char *iv)
{
EVP_MD_CTX c;
unsigned char md_buf[EVP_MAX_MD_SIZE];
int niv,nkey,addmd=0;
unsigned int mds=0,i;
int rv = 0;
nkey=type-key_len;
niv=type-iv_len;
OPENSSL_assert(nkey = EVP_MAX_KEY_LENGTH);
OPENSSL_assert(niv = EVP_MAX_IV_LENGTH);

if (data == NULL) return(nkey);

EVP_MD_CTX_init(c);
for (;;)
{
if (!EVP_DigestInit_ex(c,md, NULL))
return 0;
if (addmd++)
if (!EVP_DigestUpdate(c,(md_buf[0]),mds))
goto err;
if (!EVP_DigestUpdate(c,data,datal))
goto err;
if (salt != NULL)
if (!EVP_DigestUpdate(c,salt,PKCS5_SALT_LEN))
goto err;
if (!EVP_DigestFinal_ex(c,(md_buf[0]),mds))
goto err;

for (i=1; i(unsigned int)count; i++)
{
if (!EVP_DigestInit_ex(c,md, NULL))
goto err;
if (!EVP_DigestUpdate(c,(md_buf[0]),mds))
goto err;
if (!EVP_DigestFinal_ex(c,(md_buf[0]),mds))
goto err;
}
i=0;
if (nkey)
{
for (;;)
{
if (nkey == 0) break;
if (i == mds) break;
if (key != NULL)
*(key++)=md_buf[i];
nkey--;
i++;
}
}
if (niv  (i != mds))
{
for (;;)
{
if (niv == 0) break;
if (i == mds) break;
if (iv != NULL)
*(iv++)=md_buf[i];
niv--;
i++;
}
}
if ((nkey == 0)  (niv == 0)) break;
}
rv = type-key_len;
err:
EVP_MD_CTX_cleanup(c);
OPENSSL_cleanse((md_buf[0]),EVP_MAX_MD_SIZE);
return rv;
}

*

$ git pull
   ...
$ git log
commit 199772e53427ca921c289471c1344d454781fcc0
   ...

__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: [openssl.org #3457] Possible Bugs in EVP_KeyToBytes?

2014-07-15 Thread Marcus Meissner
On Tue, Jul 15, 2014 at 07:31:59PM +0200, noloa...@gmail.com via RT wrote:
 Below is from crypto/evp/evp_key.c.
 
 Notice that `addmd` is never set to 1. *If* the routine needs to loop
 back to the top to finish fulfilling a derivation request, then the
 previous hash is *not* added back into the computation. That is, this
 is never executed:
 
 if (addmd++)
 if (!EVP_DigestUpdate(c,(md_buf[0]),mds))

Well, addmd will be 1 after the first loop iteration due to the post-increment
operator and then it will be folded in.
 
Ciao, Marcus
__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org