summaryrefslogtreecommitdiffstats
path: root/ldap/servers/plugins
diff options
context:
space:
mode:
authorRich Megginson <rmeggins@redhat.com>2009-07-08 09:57:04 -0600
committerRich Megginson <rmeggins@redhat.com>2009-07-14 12:35:11 -0600
commita4240192f344a1a172cfdf8609661b90435b5db3 (patch)
treec6ea1519c184971c66f171252e1ebd5493a8b610 /ldap/servers/plugins
parent386ba57d421ee2d59a267d52d63bd88cbf20c435 (diff)
downloadds-a4240192f344a1a172cfdf8609661b90435b5db3.tar.gz
ds-a4240192f344a1a172cfdf8609661b90435b5db3.tar.xz
ds-a4240192f344a1a172cfdf8609661b90435b5db3.zip
Reduce noise reported by valgrind
valgrind is a very useful tool - however, the directory server produces a lot of false positives that have to be suppressed in order to get to the useful information. These patches attempt to reduce some of that noise. 1) aclparse - should calculate the length of the string _after_ trimming the spaces 2) something about random number generation causes some of the bits to be uninitialized, and valgrind doesn't like it - this patch doesn't eliminate the error, just reduces it 3) use initialized memory when generating hashes - also remove "magic numbers" 4) bin.c - slapi_value_get_string must not be used with unterminated (binary) values 5) we get these odd valgrind reports from deep within bdb about invalid reads and uninitialized memory - I thought perhaps because we were initializing DBT structures with = {0} which the bdb docs says is not sufficient - they recommend memset or bzero 6) There are some small memory leaks during attrcrypt initialization and in error cases 7) error message in ldif2ldbm.c was attempting to print the Slapi_DN structure rather than getting the char *dn 8) After we call NSS_Initialize, we must call the NSS shutdown functions to clean up the caches and other data structures, otherwise NSS will leak memory. This is harmless since it happens at exit, but valgrind reports hundreds of memory leaks. The solution is to make sure we go through a single exit point after NSS_Initialize. This means many places that just called exit() must instead return with a real return value. This mostly affected main.c, detach.c, and a couple of other places called during startup. 9) minor memory leaks in mapping tree initialization 10) sasl_map.c - should not call this in referral mode 11) minor memory leaks during ssl init Reviewed by: nkinder, nhosoi (Thanks!)
Diffstat (limited to 'ldap/servers/plugins')
-rw-r--r--ldap/servers/plugins/acl/aclparse.c3
-rw-r--r--ldap/servers/plugins/acl/aclutil.c11
-rw-r--r--ldap/servers/plugins/pwdstorage/sha_pwd.c35
-rw-r--r--ldap/servers/plugins/pwdstorage/ssha_pwd.c28
-rw-r--r--ldap/servers/plugins/syntaxes/bin.c6
5 files changed, 47 insertions, 36 deletions
diff --git a/ldap/servers/plugins/acl/aclparse.c b/ldap/servers/plugins/acl/aclparse.c
index 8855623e..57fd5ae7 100644
--- a/ldap/servers/plugins/acl/aclparse.c
+++ b/ldap/servers/plugins/acl/aclparse.c
@@ -609,11 +609,10 @@ normalize_nextACERule:
** for deny rule. We will never need more 2 times
** the len.
*/
+ __acl_strip_leading_space(&tmp_str);
len = strlen (tmp_str);
s_acestr = acestr = slapi_ch_calloc ( 1, 2 * len);
- __acl_strip_leading_space(&tmp_str);
-
/*
* Now it's something like:
* allow (all) groupdn = "ldap:///cn=Domain Administrators, o=$dn.o, o=ISP";
diff --git a/ldap/servers/plugins/acl/aclutil.c b/ldap/servers/plugins/acl/aclutil.c
index 599fdbd0..a93a53a3 100644
--- a/ldap/servers/plugins/acl/aclutil.c
+++ b/ldap/servers/plugins/acl/aclutil.c
@@ -442,10 +442,13 @@ acl_gen_err_msg(int access, char *edn, char *attr, char **errbuf)
short
aclutil_gen_signature ( short c_signature )
{
- short o_signature;
- o_signature = c_signature ^ (slapi_rand() % 32768);
- if (!o_signature)
- o_signature = c_signature ^ (slapi_rand() % 32768);
+ short o_signature = 0;
+ short randval = (short)slapi_rand();
+ o_signature = c_signature ^ (randval % 32768);
+ if (!o_signature) {
+ randval = (short)slapi_rand();
+ o_signature = c_signature ^ (randval % 32768);
+ }
return o_signature;
}
diff --git a/ldap/servers/plugins/pwdstorage/sha_pwd.c b/ldap/servers/plugins/pwdstorage/sha_pwd.c
index e54feab7..8e9d60cf 100644
--- a/ldap/servers/plugins/pwdstorage/sha_pwd.c
+++ b/ldap/servers/plugins/pwdstorage/sha_pwd.c
@@ -54,6 +54,7 @@
#include <sechash.h>
#define SHA_SALT_LENGTH 8 /* number of bytes of data in salt */
+#define OLD_SALT_LENGTH 8
#define NOT_FIRST_TIME (time_t)1 /* not the first logon */
static char *hasherrmsg = "pw_cmp: %s userPassword \"%s\" is the wrong length or is not properly encoded BASE64\n";
@@ -82,7 +83,7 @@ sha_pw_cmp (const char *userpwd, const char *dbpwd, unsigned int shaLen )
unsigned int secOID;
char *schemeName;
char *hashresult = NULL;
-
+
/* Determine which algorithm we're using */
switch (shaLen) {
case SHA1_LENGTH:
@@ -111,8 +112,10 @@ sha_pw_cmp (const char *userpwd, const char *dbpwd, unsigned int shaLen )
*/
hash_len = (strlen(dbpwd) * 3) / 4; /* includes the trailing = if any */
if ( hash_len > sizeof(quick_dbhash) ) { /* get more space: */
- dbhash = (char*) slapi_ch_malloc( hash_len );
+ dbhash = (char*) slapi_ch_calloc( hash_len, sizeof(char) );
if ( dbhash == NULL ) goto loser;
+ } else {
+ memset( quick_dbhash, 0, sizeof(quick_dbhash) );
}
hashresult = PL_Base64Decode( dbpwd, 0, dbhash );
if (NULL == hashresult) {
@@ -123,23 +126,25 @@ sha_pw_cmp (const char *userpwd, const char *dbpwd, unsigned int shaLen )
salt.bv_len = SHA_SALT_LENGTH;
} else if ( hash_len >= DS40B1_SALTED_SHA_LENGTH ) {
salt.bv_val = (void*)dbhash;
- salt.bv_len = 8;
+ salt.bv_len = OLD_SALT_LENGTH;
} else { /* unsupported, invalid BASE64 (hash_len < 0), or similar */
slapi_log_error( SLAPI_LOG_PLUGIN, plugin_name, hasherrmsg, schemeName, dbpwd );
goto loser;
}
-
+
/* hash the user's key */
+ memset( userhash, 0, sizeof(userhash) );
if ( sha_salted_hash( userhash, userpwd, &salt, secOID ) != SECSuccess ) {
slapi_log_error( SLAPI_LOG_PLUGIN, plugin_name, "sha_pw_cmp: sha_salted_hash() failed\n");
goto loser;
}
-
+
/* the proof is in the comparison... */
result = ( hash_len >= shaLen ) ?
- ( memcmp( userhash, dbhash, shaLen )) : /* include salt */
- ( memcmp( userhash, dbhash + 8, hash_len - 8 )); /* exclude salt */
-
+ ( memcmp( userhash, dbhash, shaLen ) ) : /* include salt */
+ ( memcmp( userhash, dbhash + OLD_SALT_LENGTH,
+ hash_len - OLD_SALT_LENGTH ) ); /* exclude salt */
+
loser:
if ( dbhash && dbhash != quick_dbhash ) slapi_ch_free_string( &dbhash );
return result;
@@ -153,7 +158,8 @@ sha_pw_enc( const char *pwd, unsigned int shaLen )
char *schemeName;
unsigned int schemeNameLen;
unsigned int secOID;
-
+ size_t enclen;
+
/* Determine which algorithm we're using */
switch (shaLen) {
case SHA1_LENGTH:
@@ -182,19 +188,20 @@ sha_pw_enc( const char *pwd, unsigned int shaLen )
}
/* hash the user's key */
+ memset( hash, 0, sizeof(hash) );
if ( sha_salted_hash( hash, pwd, NULL, secOID ) != SECSuccess ) {
return( NULL );
}
-
- if (( enc = slapi_ch_malloc( 3 + schemeNameLen +
- LDIF_BASE64_LEN( shaLen ))) == NULL ) {
+
+ enclen = 3 + schemeNameLen + LDIF_BASE64_LEN( shaLen );
+ if (( enc = slapi_ch_calloc( enclen, sizeof(char) )) == NULL ) {
return( NULL );
}
-
+
sprintf( enc, "%c%s%c", PWD_HASH_PREFIX_START, schemeName,
PWD_HASH_PREFIX_END );
(void)PL_Base64Encode( hash, shaLen, enc + 2 + schemeNameLen );
-
+
return( enc );
}
diff --git a/ldap/servers/plugins/pwdstorage/ssha_pwd.c b/ldap/servers/plugins/pwdstorage/ssha_pwd.c
index 14b8d443..6f09d5e9 100644
--- a/ldap/servers/plugins/pwdstorage/ssha_pwd.c
+++ b/ldap/servers/plugins/pwdstorage/ssha_pwd.c
@@ -131,10 +131,11 @@ salted_sha_pw_enc( const char *pwd, unsigned int shaLen )
char *salt = hash + shaLen;
struct berval saltval;
char *enc;
+ size_t encsize;
char *schemeName;
unsigned int schemeNameLen;
unsigned int secOID;
-
+
/* Determine which algorithm we're using */
switch (shaLen) {
case SHA1_LENGTH:
@@ -161,31 +162,30 @@ salted_sha_pw_enc( const char *pwd, unsigned int shaLen )
/* An unknown shaLen was passed in. We shouldn't get here. */
return( NULL );
}
-
+
+ memset(hash, 0, sizeof(hash));
saltval.bv_val = (void*)salt;
saltval.bv_len = SHA_SALT_LENGTH;
-
+
/* generate a new random salt */
- /* Note: the uninitialized salt array provides a little extra entropy
- * to the random array generation, but it is not really needed since
- * PK11_GenerateRandom takes care of seeding. In any case, it doesn't
- * hurt. */
- ssha_rand_array( salt, SHA_SALT_LENGTH );
-
+ ssha_rand_array( salt, SHA_SALT_LENGTH );
+
/* hash the user's key */
if ( sha_salted_hash( hash, pwd, &saltval, secOID ) != SECSuccess ) {
return( NULL );
}
-
- if (( enc = slapi_ch_malloc( 3 + schemeNameLen +
- LDIF_BASE64_LEN(shaLen + SHA_SALT_LENGTH))) == NULL ) {
+
+ encsize = 3 + schemeNameLen +
+ LDIF_BASE64_LEN(shaLen + SHA_SALT_LENGTH);
+ if ( ( enc = slapi_ch_calloc( encsize, sizeof(char) ) ) == NULL ) {
return( NULL );
}
-
+
sprintf( enc, "%c%s%c", PWD_HASH_PREFIX_START, schemeName,
PWD_HASH_PREFIX_END );
(void)PL_Base64Encode( hash, (shaLen + SHA_SALT_LENGTH), enc + 2 + schemeNameLen );
-
+ PR_ASSERT(0 == enc[encsize-1]); /* must be null terminated */
+
return( enc );
}
diff --git a/ldap/servers/plugins/syntaxes/bin.c b/ldap/servers/plugins/syntaxes/bin.c
index b7be0d1a..2d0b6f8a 100644
--- a/ldap/servers/plugins/syntaxes/bin.c
+++ b/ldap/servers/plugins/syntaxes/bin.c
@@ -165,8 +165,10 @@ bin_filter_ava( Slapi_PBlock *pb, struct berval *bvfilter,
int i;
for ( i = 0; bvals[i] != NULL; i++ ) {
- if ( slapi_value_get_length(bvals[i]) == bvfilter->bv_len &&
- 0 == memcmp( slapi_value_get_string(bvals[i]), bvfilter->bv_val, bvfilter->bv_len ))
+ const struct berval *bv = slapi_value_get_berval(bvals[i]);
+
+ if ( ( bv->bv_len == bvfilter->bv_len ) &&
+ ( 0 == memcmp( bv->bv_val, bvfilter->bv_val, bvfilter->bv_len ) ) )
{
if(retVal!=NULL)
{