I *hate* global variables...

OK, what was happening here was that we would invalidate global_sam_sid
when we set the sid into secrets.tdb, to force a re-read.

The problem was, we would do *two* writes into the TDB, and the second one
(in the PDC/BDC case) would be of a NULL pointer.  This caused smbd startups
to fail, on a blank TDB.

By using a local variable in the pdb_generate_sam_sid() code, we avoid this
particular trap.

I've also added better debugging for the case where this all matters, which
is particularly for LDAP, where it finds out a domain SID from the sambaDomain
object.

Andrew Bartlett
(This used to be commit 86ad04d26d3065a99b08afaaf2914968a9e701c5)

2 files changed, 45 insertions, 31 deletions

diff --git a/source3/passdb/machine_sid.c b/source3/passdb/machine_sid.c
index 866ae30362..f52e7116b0 100644
--- a/source3/passdb/machine_sid.c
+++ b/source3/passdb/machine_sid.c
@@ -76,15 +76,15 @@ static void generate_random_sid(DOM_SID *sid)
  Generate the global machine sid.
 ****************************************************************************/
 
-static BOOL pdb_generate_sam_sid(void)
+static DOM_SID *pdb_generate_sam_sid(void)
 {
 	DOM_SID domain_sid;
 	char *fname = NULL;
 	BOOL is_dc = False;
-
-	if(global_sam_sid==NULL)
-		if(!(global_sam_sid=(DOM_SID *)malloc(sizeof(DOM_SID))))
-			return False;
+	DOM_SID *sam_sid;
+	
+	if(!(sam_sid=(DOM_SID *)malloc(sizeof(DOM_SID))))
+		return NULL;
 			
 	generate_wellknown_sids();
 
@@ -100,86 +100,93 @@ static BOOL pdb_generate_sam_sid(void)
 
 	if (is_dc) {
 		if (secrets_fetch_domain_sid(lp_workgroup(), &domain_sid)) {
-			sid_copy(global_sam_sid, &domain_sid);
-			return True;
+			sid_copy(sam_sid, &domain_sid);
+			return sam_sid;
 		}
 	}
 
-	if (secrets_fetch_domain_sid(global_myname(), global_sam_sid)) {
+	if (secrets_fetch_domain_sid(global_myname(), sam_sid)) {
 
 		/* We got our sid. If not a pdc/bdc, we're done. */
 		if (!is_dc)
-			return True;
+			return sam_sid;
 
 		if (!secrets_fetch_domain_sid(lp_workgroup(), &domain_sid)) {
 
 			/* No domain sid and we're a pdc/bdc. Store it */
 
-			if (!secrets_store_domain_sid(lp_workgroup(), global_sam_sid)) {
+			if (!secrets_store_domain_sid(lp_workgroup(), sam_sid)) {
 				DEBUG(0,("pdb_generate_sam_sid: Can't store domain SID as a pdc/bdc.\n"));
-				return False;
+				SAFE_FREE(sam_sid);
+				return NULL;
 			}
-			return True;
+			return sam_sid;
 		}
 
-		if (!sid_equal(&domain_sid, global_sam_sid)) {
+		if (!sid_equal(&domain_sid, sam_sid)) {
 
 			/* Domain name sid doesn't match global sam sid. Re-store domain sid as 'local' sid. */
 
 			DEBUG(0,("pdb_generate_sam_sid: Mismatched SIDs as a pdc/bdc.\n"));
 			if (!secrets_store_domain_sid(global_myname(), &domain_sid)) {
 				DEBUG(0,("pdb_generate_sam_sid: Can't re-store domain SID for local sid as PDC/BDC.\n"));
-				return False;
+				SAFE_FREE(sam_sid);
+				return NULL;
 			}
-			return True;
+			return sam_sid;
 		}
 
-		return True;
+		return sam_sid;
 		
 	}
 
 	/* check for an old MACHINE.SID file for backwards compatibility */
 	asprintf(&fname, "%s/MACHINE.SID", lp_private_dir());
 
-	if (read_sid_from_file(fname, global_sam_sid)) {
+	if (read_sid_from_file(fname, sam_sid)) {
 		/* remember it for future reference and unlink the old MACHINE.SID */
-		if (!secrets_store_domain_sid(global_myname(), global_sam_sid)) {
+		if (!secrets_store_domain_sid(global_myname(), sam_sid)) {
 			DEBUG(0,("pdb_generate_sam_sid: Failed to store SID from file.\n"));
 			SAFE_FREE(fname);
-			return False;
+			SAFE_FREE(sam_sid);
+			return NULL;
 		}
 		unlink(fname);
 		if (is_dc) {
-			if (!secrets_store_domain_sid(lp_workgroup(), global_sam_sid)) {
+			if (!secrets_store_domain_sid(lp_workgroup(), sam_sid)) {
 				DEBUG(0,("pdb_generate_sam_sid: Failed to store domain SID from file.\n"));
 				SAFE_FREE(fname);
-				return False;
+				SAFE_FREE(sam_sid);
+				return NULL;
 			}
 		}
 
 		/* Stored the old sid from MACHINE.SID successfully.*/
 		SAFE_FREE(fname);
-		return True;
+		SAFE_FREE(sam_sid);
+		return sam_sid;
 	}
 
 	SAFE_FREE(fname);
 
 	/* we don't have the SID in secrets.tdb, we will need to
            generate one and save it */
-	generate_random_sid(global_sam_sid);
+	generate_random_sid(sam_sid);
 
-	if (!secrets_store_domain_sid(global_myname(), global_sam_sid)) {
+	if (!secrets_store_domain_sid(global_myname(), sam_sid)) {
 		DEBUG(0,("pdb_generate_sam_sid: Failed to store generated machine SID.\n"));
-		return False;
+		SAFE_FREE(sam_sid);
+		return NULL;
 	}
 	if (is_dc) {
-		if (!secrets_store_domain_sid(lp_workgroup(), global_sam_sid)) {
+		if (!secrets_store_domain_sid(lp_workgroup(), sam_sid)) {
 			DEBUG(0,("pdb_generate_sam_sid: Failed to store generated domain SID.\n"));
-			return False;
+			SAFE_FREE(sam_sid);
+			return NULL;
 		}
 	}
 
-	return True;
+	return sam_sid;
 }   
 
 /* return our global_sam_sid */
@@ -191,10 +198,10 @@ DOM_SID *get_global_sam_sid(void)
 	/* memory for global_sam_sid is allocated in 
 	   pdb_generate_sam_sid() as needed */
 
-	if (!pdb_generate_sam_sid()) {
+	if (!(global_sam_sid = pdb_generate_sam_sid())) {
 		smb_panic("Could not generate a machine SID\n");
 	}
-	
+
 	return global_sam_sid;
 }
 
diff --git a/source3/passdb/pdb_ldap.c b/source3/passdb/pdb_ldap.c
index 3db0702c92..eefd302d42 100644
--- a/source3/passdb/pdb_ldap.c
+++ b/source3/passdb/pdb_ldap.c
@@ -2353,7 +2353,7 @@ static NTSTATUS pdb_init_ldapsam(PDB_CONTEXT *pdb_context, PDB_METHODS **pdb_met
 	/* Try to setup the Domain Name, Domain SID, algorithmic rid base */
 	
 	nt_status = smbldap_search_domain_info(ldap_state->smbldap_state, &result, 
-		ldap_state->domain_name, True);
+					       ldap_state->domain_name, True);
 	
 	if ( !NT_STATUS_IS_OK(nt_status) ) {
 		DEBUG(2, ("pdb_init_ldapsam: WARNING: Could not get domain info, nor add one to the domain\n"));
@@ -2382,8 +2382,15 @@ and will risk BDCs having inconsistant SIDs\n"));
 		}
 		found_sid = secrets_fetch_domain_sid(ldap_state->domain_name, &secrets_domain_sid);
 		if (!found_sid || !sid_equal(&secrets_domain_sid, &ldap_domain_sid)) {
+			fstring new_sid_str, old_sid_str;
+			DEBUG(1, ("pdb_init_ldapsam: Resetting SID for domain %s based on pdb_ldap results %s -> %s\n",
+				  ldap_state->domain_name, 
+				  sid_to_string(old_sid_str, &secrets_domain_sid),
+				  sid_to_string(new_sid_str, &ldap_domain_sid)));
+			
 			/* reset secrets.tdb sid */
 			secrets_store_domain_sid(ldap_state->domain_name, &ldap_domain_sid);
+			DEBUG(1, ("New global sam SID: %s\n", sid_to_string(new_sid_str, get_global_sam_sid())));
 		}
 		sid_copy(&ldap_state->domain_sid, &ldap_domain_sid);
 	}