[SCM] Samba Shared Repository - branch master updated

Matthias Dieter Wallnöfer mdw at samba.org
Thu Dec 2 04:36:01 MST 2010


The branch, master has been updated
       via  6f42da7 s4:dsdb/samdb/cracknames.c - fix various KRB5 memory leaks
       via  573389c s4:password_hash LDB module - allow empty ("") passwords
      from  f62972b s4/scripting/ktpass: make shell code portable and eliminate another bash requirement

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 6f42da795e5cce29c122aea1a6818c1ae1d99ae6
Author: Matthias Dieter Wallnöfer <mdw at samba.org>
Date:   Thu Dec 2 11:49:56 2010 +0100

    s4:dsdb/samdb/cracknames.c - fix various KRB5 memory leaks
    
    Autobuild-User: Matthias Dieter Wallnöfer <mdw at samba.org>
    Autobuild-Date: Thu Dec  2 12:35:03 CET 2010 on sn-devel-104

commit 573389c8cc8d74ad8560bc3531be5e2207d1bdaa
Author: Matthias Dieter Wallnöfer <mdw at samba.org>
Date:   Thu Dec 2 09:55:56 2010 +0100

    s4:password_hash LDB module - allow empty ("") passwords
    
    This seems to have been broken some time ago - till someone on the
    mailing list noticed it.
    
    I've also added a testsuite (and some additional SamDB python helpers) which
    should prove this.

-----------------------------------------------------------------------

Summary of changes:
 source4/dsdb/samdb/cracknames.c                |   20 ++++++---
 source4/dsdb/samdb/ldb_modules/password_hash.c |   53 ++++++++++++++----------
 source4/dsdb/tests/python/passwords.py         |   19 ++++++++
 source4/scripting/python/samba/samdb.py        |   30 +++++++++++++
 4 files changed, 93 insertions(+), 29 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source4/dsdb/samdb/cracknames.c b/source4/dsdb/samdb/cracknames.c
index 6df140f..0b7b6ed 100644
--- a/source4/dsdb/samdb/cracknames.c
+++ b/source4/dsdb/samdb/cracknames.c
@@ -206,6 +206,7 @@ static WERROR DsCrackNameSPNAlias(struct ldb_context *sam_ctx, TALLOC_CTX *mem_c
 	/* This is checked for in callers, but be safe */
 	if (principal->name.name_string.len < 2) {
 		info1->status = DRSUAPI_DS_NAME_STATUS_NOT_FOUND;
+		krb5_free_principal(smb_krb5_context->krb5_context, principal);
 		return WERR_OK;
 	}
 	service = principal->name.name_string.val[0];
@@ -217,13 +218,14 @@ static WERROR DsCrackNameSPNAlias(struct ldb_context *sam_ctx, TALLOC_CTX *mem_c
 					  service, &new_service);
 
 	if (namestatus == DRSUAPI_DS_NAME_STATUS_NOT_FOUND) {
+		wret = WERR_OK;
 		info1->status		= DRSUAPI_DS_NAME_STATUS_DOMAIN_ONLY;
 		info1->dns_domain_name	= talloc_strdup(mem_ctx, dns_name);
 		if (!info1->dns_domain_name) {
-			krb5_free_principal(smb_krb5_context->krb5_context, principal);
-			return WERR_NOMEM;
+			wret = WERR_NOMEM;
 		}
-		return WERR_OK;
+		krb5_free_principal(smb_krb5_context->krb5_context, principal);
+		return wret;
 	} else if (namestatus != DRSUAPI_DS_NAME_STATUS_OK) {
 		info1->status = namestatus;
 		krb5_free_principal(smb_krb5_context->krb5_context, principal);
@@ -293,7 +295,8 @@ static WERROR DsCrackNameUPN(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx,
 		return WERR_OK;
 	}
 
-	realm = krb5_principal_get_realm(smb_krb5_context->krb5_context, principal);
+	realm = krb5_principal_get_realm(smb_krb5_context->krb5_context,
+					 principal);
 
 	ldb_ret = ldb_search(sam_ctx, mem_ctx, &domain_res,
 				     samdb_partitions_dn(sam_ctx, mem_ctx), 
@@ -306,6 +309,7 @@ static WERROR DsCrackNameUPN(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx,
 	if (ldb_ret != LDB_SUCCESS) {
 		DEBUG(2, ("DsCrackNameUPN domain ref search failed: %s", ldb_errstring(sam_ctx)));
 		info1->status = DRSUAPI_DS_NAME_STATUS_RESOLVE_ERROR;
+		krb5_free_principal(smb_krb5_context->krb5_context, principal);
 		return WERR_OK;
 	}
 
@@ -313,10 +317,12 @@ static WERROR DsCrackNameUPN(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx,
 	case 1:
 		break;
 	case 0:
+		krb5_free_principal(smb_krb5_context->krb5_context, principal);
 		return dns_domain_from_principal(mem_ctx, smb_krb5_context, 
 						 name, info1);
 	default:
 		info1->status = DRSUAPI_DS_NAME_STATUS_NOT_UNIQUE;
+		krb5_free_principal(smb_krb5_context->krb5_context, principal);
 		return WERR_OK;
 	}
 
@@ -616,12 +622,12 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx,
 			info1->status = DRSUAPI_DS_NAME_STATUS_NOT_FOUND;
 			krb5_free_principal(smb_krb5_context->krb5_context, principal);
 			return WERR_OK;
+		} else if (ret == 0) {
+			krb5_free_principal(smb_krb5_context->krb5_context, principal);
 		}
 		ret = krb5_parse_name_flags(smb_krb5_context->krb5_context, name, 
 					    KRB5_PRINCIPAL_PARSE_NO_REALM, &principal);
 		if (ret) {
-			krb5_free_principal(smb_krb5_context->krb5_context, principal);
-
 			return dns_domain_from_principal(mem_ctx, smb_krb5_context,
 							 name, info1);
 		}
@@ -642,6 +648,7 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx,
 			computer_name = talloc_strndup(mem_ctx, principal->name.name_string.val[1], 
 						      strcspn(principal->name.name_string.val[1], "."));
 			if (computer_name == NULL) {
+				krb5_free_principal(smb_krb5_context->krb5_context, principal);
 				return WERR_NOMEM;
 			}
 
@@ -662,7 +669,6 @@ WERROR DsCrackNameOneName(struct ldb_context *sam_ctx, TALLOC_CTX *mem_ctx,
 		info1->status = DRSUAPI_DS_NAME_STATUS_NOT_FOUND;
 		return WERR_OK;
 	}
-
 	}
 
 	if (format_flags & DRSUAPI_DS_NAME_FLAG_SYNTACTICAL_ONLY) {
diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c
index 35d2729..4e2e96f 100644
--- a/source4/dsdb/samdb/ldb_modules/password_hash.c
+++ b/source4/dsdb/samdb/ldb_modules/password_hash.c
@@ -1298,18 +1298,22 @@ static int setup_given_passwords(struct setup_password_fields_io *io,
 					   (void *)&cleartext_utf16_blob->data,
 					   &cleartext_utf16_blob->length,
 					   false)) {
-			talloc_free(cleartext_utf16_blob);
-			ldb_asprintf_errstring(ldb,
-					       "setup_password_fields: "
-					       "failed to generate UTF16 password from cleartext UTF8 password for user %s", io->u.sAMAccountName);
-			return LDB_ERR_CONSTRAINT_VIOLATION;
-		} else {
-			g->cleartext_utf16 = cleartext_utf16_blob;
+			if (g->cleartext_utf8->length != 0) {
+				talloc_free(cleartext_utf16_blob);
+				ldb_asprintf_errstring(ldb,
+						       "setup_password_fields: "
+						       "failed to generate UTF16 password from cleartext UTF8 one for user '%s'!",
+						       io->u.sAMAccountName);
+				return LDB_ERR_CONSTRAINT_VIOLATION;
+			} else {
+				/* passwords with length "0" are valid! */
+				cleartext_utf16_blob->data = NULL;
+				cleartext_utf16_blob->length = 0;
+			}
 		}
+		g->cleartext_utf16 = cleartext_utf16_blob;
 	} else if (g->cleartext_utf16) {
-		char *cleartext_utf8_str;
 		struct ldb_val *cleartext_utf8_blob;
-		size_t converted_pw_len;
 
 		cleartext_utf8_blob = talloc(io->ac, struct ldb_val);
 		if (!cleartext_utf8_blob) {
@@ -1319,20 +1323,25 @@ static int setup_given_passwords(struct setup_password_fields_io *io,
 					   CH_UTF16MUNGED, CH_UTF8,
 					   g->cleartext_utf16->data,
 					   g->cleartext_utf16->length,
-					   (void *)&cleartext_utf8_str,
-					   &converted_pw_len, false)) {
-			/* We must bail out here, the input wasn't even a
-			 * multiple of 2 bytes */
-			talloc_free(cleartext_utf8_blob);
-			ldb_asprintf_errstring(ldb,
-					       "setup_password_fields: "
-					       "UTF16 password for user %s had odd length (length must be a multiple of 2)", io->u.sAMAccountName);
-			return LDB_ERR_CONSTRAINT_VIOLATION;
-		} else {
-			*cleartext_utf8_blob = data_blob_const(cleartext_utf8_str,
-							       converted_pw_len);
-			g->cleartext_utf8 = cleartext_utf8_blob;
+					   (void *)&cleartext_utf8_blob->data,
+					   &cleartext_utf8_blob->length,
+					   false)) {
+			if (g->cleartext_utf16->length != 0) {
+				/* We must bail out here, the input wasn't even
+				 * a multiple of 2 bytes */
+				talloc_free(cleartext_utf8_blob);
+				ldb_asprintf_errstring(ldb,
+						       "setup_password_fields: "
+						       "failed to generate UTF8 password from cleartext UTF 16 one for user '%s' - the latter had odd length (length must be a multiple of 2)!",
+						       io->u.sAMAccountName);
+				return LDB_ERR_CONSTRAINT_VIOLATION;
+			} else {
+				/* passwords with length "0" are valid! */
+				cleartext_utf8_blob->data = NULL;
+				cleartext_utf8_blob->length = 0;
+			}
 		}
+		g->cleartext_utf8 = cleartext_utf8_blob;
 	}
 
 	if (g->cleartext_utf16) {
diff --git a/source4/dsdb/tests/python/passwords.py b/source4/dsdb/tests/python/passwords.py
index 198b314..e43298e 100755
--- a/source4/dsdb/tests/python/passwords.py
+++ b/source4/dsdb/tests/python/passwords.py
@@ -887,6 +887,25 @@ userPassword: thatsAcomplPASS4
         # Reset the test "dSHeuristics" (reactivate "userPassword" pwd changes)
         ldb.set_dsheuristics("000000001")
 
+    def test_zero_length(self):
+        # Get the old "minPwdLength"
+        minPwdLength = ldb.get_minPwdLength()
+        # Set it temporarely to "0"
+        ldb.set_minPwdLength("0")
+
+        # Get the old "pwdProperties"
+        pwdProperties = ldb.get_pwdProperties()
+        # Set them temporarely to "0" (to deactivate eventually the complexity)
+        ldb.set_pwdProperties("0")
+
+        ldb.setpassword("(sAMAccountName=testuser)", "")
+
+        # Reset the "pwdProperties" as they were before
+        ldb.set_pwdProperties(pwdProperties)
+
+        # Reset the "minPwdLength" as it was before
+        ldb.set_minPwdLength(minPwdLength)
+
     def tearDown(self):
         super(PasswordTests, self).tearDown()
         delete_force(self.ldb, "cn=testuser,cn=users," + self.base_dn)
diff --git a/source4/scripting/python/samba/samdb.py b/source4/scripting/python/samba/samdb.py
index a7f474d..99f141e 100644
--- a/source4/scripting/python/samba/samdb.py
+++ b/source4/scripting/python/samba/samdb.py
@@ -637,6 +637,36 @@ accountExpires: %u
         else:
             return res[0]["minPwdAge"][0]
 
+    def set_minPwdLength(self, value):
+        m = ldb.Message()
+        m.dn = ldb.Dn(self, self.domain_dn())
+        m["minPwdLength"] = ldb.MessageElement(value, ldb.FLAG_MOD_REPLACE, "minPwdLength")
+        self.modify(m)
+
+    def get_minPwdLength(self):
+        res = self.search(self.domain_dn(), scope=ldb.SCOPE_BASE, attrs=["minPwdLength"])
+        if len(res) == 0:
+            return None
+        elif not "minPwdLength" in res[0]:
+            return None
+        else:
+            return res[0]["minPwdLength"][0]
+
+    def set_pwdProperties(self, value):
+        m = ldb.Message()
+        m.dn = ldb.Dn(self, self.domain_dn())
+        m["pwdProperties"] = ldb.MessageElement(value, ldb.FLAG_MOD_REPLACE, "pwdProperties")
+        self.modify(m)
+
+    def get_pwdProperties(self):
+        res = self.search(self.domain_dn(), scope=ldb.SCOPE_BASE, attrs=["pwdProperties"])
+        if len(res) == 0:
+            return None
+        elif not "pwdProperties" in res[0]:
+            return None
+        else:
+            return res[0]["pwdProperties"][0]
+
     def set_dsheuristics(self, dsheuristics):
         m = ldb.Message()
         m.dn = ldb.Dn(self, "CN=Directory Service,CN=Windows NT,CN=Services,%s"


-- 
Samba Shared Repository


More information about the samba-cvs mailing list