Skip Menu |
 

Subject: aes s2k for > 64 character returns ptr to stack memory
Cc: raeburn@mit.edu
Download (untitled) / with headers
text/plain 1.5KiB
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...
#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);
}
From: raeburn@mit.edu
Subject: CVS Commit
* pbkdf2.c (hmac1): Make a local copy of the supplied keyblock structure, in
case we want to modify it.


To generate a diff of this commit:



cvs diff -r5.153 -r5.154 krb5/src/lib/crypto/ChangeLog
cvs diff -r5.7 -r5.8 krb5/src/lib/crypto/pbkdf2.c
From: tlyu@mit.edu
Subject: CVS Commit
pullup from trunk


To generate a diff of this commit:



cvs diff -r5.136.2.10 -r5.136.2.11 krb5/src/lib/crypto/ChangeLog
cvs diff -r5.4.2.2 -r5.4.2.3 krb5/src/lib/crypto/pbkdf2.c