From: Michael Adam <obnox@samba.org>
Date: Tue, 5 Aug 2008 21:14:05 +0000 (+0200)
Subject: secrets: fix replacemend random seed generator (security issue).
X-Git-Tag: samba-3.3.0pre1~275
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=bfc5d34a196f667276ce1e173821db478d01258b;p=thirdparty%2Fsamba.git

secrets: fix replacemend random seed generator (security issue).

This is a regression introduced by the change to dbwrap.
The replacement dbwrap_change_int32_atomic() does not
correctly mimic the behaviour of tdb_change_int32_atomic():
The intended behaviour is to use *oldval  as an initial
value when the entry does not yet exist in the db and to
return the old value in *oldval.

The effect was that:
1. get_rand_seed() always returns sys_getpid() in *new_seed
   instead of the incremented seed from the secrets.tdb.
2. the seed stored in the tdb is always starting at 0 instead
   of sys_getpid() + 1 and incremented in subsequent calls.

In principle this is a security issue, but i think the danger is
low, since this is only used as a fallback when there is no useable
/dev/urandom, and this is at most called on startup or via
reinit_after_fork.

Michael
---

diff --git a/source/lib/dbwrap_util.c b/source/lib/dbwrap_util.c
index 3bf312d0d09..7789f692230 100644
--- a/source/lib/dbwrap_util.c
+++ b/source/lib/dbwrap_util.c
@@ -150,9 +150,13 @@ int32 dbwrap_change_int32_atomic(struct db_context *db, const char *keystr,
 		return -1;
 	}
 
-	if ((rec->value.dptr != NULL)
-	    && (rec->value.dsize == sizeof(val))) {
+	if (rec->value.dptr == NULL) {
+		val = *oldval;
+	} else if (rec->value.dsize == sizeof(val)) {
 		val = IVAL(rec->value.dptr, 0);
+		*oldval = val;
+	} else {
+		return -1;
 	}
 
 	val += change_val;