Subject: | aes s2k for > 64 character returns ptr to stack memory |
Cc: | raeburn@mit.edu |
Already discussed with Ken... This is so that there could be a record
of the problem.
Discovered by using valgrind on kadmind w/ aes keytypes.
If one uses the aes string_to_key function with a password that
is > 64 characters, which is executed when adding a random key
to the database (see kadmin/cli/kadmin.c - a 256 character password)
Eventually, the code path comes down to lib/crypto/pbkdf2.c hmac1()
function. A pointer to a krb5_keyblock is passed in. The elements of
this keyblock are pointers to the password string and a length.
If the keylength > hashsize, a pre-hashed key is calculated into the
local variable tmp[40] - and the pointers in the krb5_keyblock updated
to point to this pre-hashed value.
This has the benefit in terms of performance that in the s2k loop,
hmac1 is called numerous times, but only the pre-hash calculation is
done once. The problem is that a pointer to local stack memory is
returned!!!
There are two possible fixes - with the idea of threads in mind -
declaring tmp static is not an option.
a) Use a local krb5_keyblock - do the prehash calculation if needed -
but don't update the original contents.
b) Allocate memory and update the original keyblock pointers. Then
krb5int_pbkdf2 needs to be modified to test if the password pointers
have changed, restore them at the end and free memory.
(a) is easier to code - but (b) might be a performance win...
Attached - find a small test program that exercises the bug. valgrind
will flag it as uninitialized memory in use...
of the problem.
Discovered by using valgrind on kadmind w/ aes keytypes.
If one uses the aes string_to_key function with a password that
is > 64 characters, which is executed when adding a random key
to the database (see kadmin/cli/kadmin.c - a 256 character password)
Eventually, the code path comes down to lib/crypto/pbkdf2.c hmac1()
function. A pointer to a krb5_keyblock is passed in. The elements of
this keyblock are pointers to the password string and a length.
If the keylength > hashsize, a pre-hashed key is calculated into the
local variable tmp[40] - and the pointers in the krb5_keyblock updated
to point to this pre-hashed value.
This has the benefit in terms of performance that in the s2k loop,
hmac1 is called numerous times, but only the pre-hash calculation is
done once. The problem is that a pointer to local stack memory is
returned!!!
There are two possible fixes - with the idea of threads in mind -
declaring tmp static is not an option.
a) Use a local krb5_keyblock - do the prehash calculation if needed -
but don't update the original contents.
b) Allocate memory and update the original keyblock pointers. Then
krb5int_pbkdf2 needs to be modified to test if the password pointers
have changed, restore them at the end and free memory.
(a) is easier to code - but (b) might be a performance win...
Attached - find a small test program that exercises the bug. valgrind
will flag it as uninitialized memory in use...
#include "k5-int.h"
int main()
{
krb5_context context;
krb5_keyblock key;
krb5_keysalt key_salt;
char dummybuf[256];
char *passwd;
int i;
krb5_data pwd;
krb5_error_code err;
/* Taken from kadmin/cli/kadmin.c - \0 terminated */
for (i = 0; i < 256; i++)
dummybuf[i] = (i+1) % 256;
passwd = dummybuf;
key_salt.data.data = malloc(32);
key_salt.data.length = 32;
memcpy(key_salt.data.data, "KRBTEST.COMgssservicedummy.dummy", 32);
pwd.data = passwd;
pwd.length = strlen(passwd);
err = krb5_c_string_to_key(context, 18, &pwd, &key_salt.data, &key);
printf("Err is %d\n", err);
return (0);
}
int main()
{
krb5_context context;
krb5_keyblock key;
krb5_keysalt key_salt;
char dummybuf[256];
char *passwd;
int i;
krb5_data pwd;
krb5_error_code err;
/* Taken from kadmin/cli/kadmin.c - \0 terminated */
for (i = 0; i < 256; i++)
dummybuf[i] = (i+1) % 256;
passwd = dummybuf;
key_salt.data.data = malloc(32);
key_salt.data.length = 32;
memcpy(key_salt.data.data, "KRBTEST.COMgssservicedummy.dummy", 32);
pwd.data = passwd;
pwd.length = strlen(passwd);
err = krb5_c_string_to_key(context, 18, &pwd, &key_salt.data, &key);
printf("Err is %d\n", err);
return (0);
}