passdb: Do not routinely clear the global memory returned by get_global_sam_sid()
authorAndrew Bartlett <abartlet@samba.org>
Tue, 13 May 2014 05:47:03 +0000 (17:47 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Wed, 11 Jun 2014 08:18:26 +0000 (10:18 +0200)
This avoids use-after-free errors and tdb database churn.

Andrew Bartlett

Change-Id: If7ab2e24556d9dffc7ad22c0489d665dd75a0cab
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Kamen Mazdrashki <kamenim@samba.org>
source3/passdb/machine_account_secrets.c
source3/passdb/pdb_samba_dsdb.c

index 5758efe819c548e52ddc6d6d5817d905b8ce07d0..4e35a726382c16678ac76ceba416aeb12b6f9712 100644 (file)
@@ -29,6 +29,7 @@
 #include "dbwrap/dbwrap.h"
 #include "../librpc/ndr/libndr.h"
 #include "util_tdb.h"
+#include "libcli/security/security.h"
 
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_PASSDB
@@ -106,9 +107,12 @@ bool secrets_store_domain_sid(const char *domain, const struct dom_sid  *sid)
 
        ret = secrets_store(domain_sid_keystr(domain), sid, sizeof(struct dom_sid ));
 
-       /* Force a re-query, in case we modified our domain */
-       if (ret)
-               reset_global_sam_sid();
+       /* Force a re-query, in the case where we modified our domain */
+       if (ret) {
+               if (dom_sid_equal(get_global_sam_sid(), sid) == false) {
+                       reset_global_sam_sid();
+               }
+       }
        return ret;
 }
 
index febf21ceb27ed635b516b7519a4a63f637410689..e9255c7cfc551ec86fc3b6c4ab7d586e03f2e655 100644 (file)
@@ -2218,6 +2218,10 @@ static void free_private_data(void **vp)
 static NTSTATUS pdb_samba_dsdb_init_secrets(struct pdb_methods *m)
 {
        struct pdb_domain_info *dom_info;
+       struct dom_sid stored_sid;
+       struct GUID stored_guid;
+       bool sid_exists_and_matches = false;
+       bool guid_exists_and_matches = false;
        bool ret;
 
        dom_info = pdb_samba_dsdb_get_domain_info(m, m);
@@ -2225,20 +2229,38 @@ static NTSTATUS pdb_samba_dsdb_init_secrets(struct pdb_methods *m)
                return NT_STATUS_UNSUCCESSFUL;
        }
 
-       secrets_clear_domain_protection(dom_info->name);
-       ret = secrets_store_domain_sid(dom_info->name,
-                                      &dom_info->sid);
-       if (!ret) {
-               goto done;
+       ret = secrets_fetch_domain_sid(dom_info->name, &stored_sid);
+       if (ret) {
+               if (dom_sid_equal(&stored_sid, &dom_info->sid)) {
+                       sid_exists_and_matches = true;
+               }
        }
-       ret = secrets_store_domain_guid(dom_info->name,
-                                       &dom_info->guid);
-       if (!ret) {
-               goto done;
+
+       if (sid_exists_and_matches == false) {
+               secrets_clear_domain_protection(dom_info->name);
+               ret = secrets_store_domain_sid(dom_info->name,
+                                              &dom_info->sid);
+               ret &= secrets_mark_domain_protected(dom_info->name);
+               if (!ret) {
+                       goto done;
+               }
        }
-       ret = secrets_mark_domain_protected(dom_info->name);
-       if (!ret) {
-               goto done;
+
+       ret = secrets_fetch_domain_guid(dom_info->name, &stored_guid);
+       if (ret) {
+               if (GUID_equal(&stored_guid, &dom_info->guid)) {
+                       guid_exists_and_matches = true;
+               }
+       }
+
+       if (guid_exists_and_matches == false) {
+               secrets_clear_domain_protection(dom_info->name);
+               ret = secrets_store_domain_guid(dom_info->name,
+                                              &dom_info->guid);
+               ret &= secrets_mark_domain_protected(dom_info->name);
+               if (!ret) {
+                       goto done;
+               }
        }
 
 done: