From c0e6fb81579126a8a7525a3ac377b9afad8bff4f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 27 Feb 2014 09:29:36 +0100 Subject: [PATCH 1/9] s4:dsdb/ldb_modules: avoid declaration after code warnings Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit de773f3785d8fedba605437fbd434a49b9d18b0e) --- source4/dsdb/samdb/ldb_modules/dirsync.c | 5 ++++- source4/dsdb/samdb/ldb_modules/operational.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c index e769948..0d68b59 100644 --- a/source4/dsdb/samdb/ldb_modules/dirsync.c +++ b/source4/dsdb/samdb/ldb_modules/dirsync.c @@ -336,10 +336,13 @@ skip: * if not we remove the attribute. */ for (i = msg->num_elements - 1; i >= 0; i--) { + const char *ldapattrname; + el = &(msg->elements[i]); + ldapattrname = el->name; + attr = dsdb_attribute_by_lDAPDisplayName(dsc->schema, el->name); - const char *ldapattrname = el->name; keep = false; if (attr->linkID & 1) { diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c index 9337faa..2ebefac 100644 --- a/source4/dsdb/samdb/ldb_modules/operational.c +++ b/source4/dsdb/samdb/ldb_modules/operational.c @@ -131,7 +131,7 @@ static int construct_token_groups(struct ldb_module *module, struct ldb_message *msg, enum ldb_scope scope, struct ldb_request *parent) { - struct ldb_context *ldb = ldb_module_get_ctx(module);; + struct ldb_context *ldb = ldb_module_get_ctx(module); TALLOC_CTX *tmp_ctx = talloc_new(msg); unsigned int i; int ret; -- 1.9.1 From dfa81abf98968072bfed92a7b34c6798fd579af2 Mon Sep 17 00:00:00 2001 From: Christian Ambach Date: Tue, 10 Dec 2013 17:53:15 +0100 Subject: [PATCH 2/9] s4:dsdb fix compiler warnings about potentially uninitialized variables Signed-off-by: Christian Ambach Reviewed-by: Jeremy Allison (cherry picked from commit 9c2951a9ca8228c714a1c1c834392077d050b569) --- source4/dsdb/samdb/ldb_modules/dirsync.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/dsdb/samdb/ldb_modules/dirsync.c b/source4/dsdb/samdb/ldb_modules/dirsync.c index 0d68b59..c93189e 100644 --- a/source4/dsdb/samdb/ldb_modules/dirsync.c +++ b/source4/dsdb/samdb/ldb_modules/dirsync.c @@ -70,7 +70,7 @@ static int dirsync_filter_entry(struct ldb_request *req, bool referral) { struct ldb_context *ldb; - uint64_t val; + uint64_t val = 0; enum ndr_err_code ndr_err; uint32_t n; int i; -- 1.9.1 From 8360fe6fe9a34dda25e6848290212d9dcf3fa413 Mon Sep 17 00:00:00 2001 From: Christian Ambach Date: Tue, 10 Dec 2013 17:52:29 +0100 Subject: [PATCH 3/9] s4:dsdb fix compiler warnings about potentially uninitialized variables Signed-off-by: Christian Ambach Reviewed-by: Jeremy Allison (cherry picked from commit e5cb10f59122acc56a465c19885fe74a39985700) --- source4/dsdb/samdb/ldb_modules/descriptor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c index ceac8db..cc0a9c2 100644 --- a/source4/dsdb/samdb/ldb_modules/descriptor.c +++ b/source4/dsdb/samdb/ldb_modules/descriptor.c @@ -501,7 +501,7 @@ static int descriptor_search_callback(struct ldb_request *req, struct ldb_reply struct ldb_val *sd_val = NULL; struct ldb_message_element *sd_el; DATA_BLOB *show_sd; - int ret; + int ret = LDB_SUCCESS; ac = talloc_get_type(req->context, struct descriptor_context); -- 1.9.1 From 465084d15b893d5386c568621e5ad577efc40891 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Tue, 12 Nov 2013 14:09:56 +0100 Subject: [PATCH 4/9] s4-dsdb: Fix a use after free segfault. Signed-off-by: Andreas Schneider Reviewed-by: David Disseldorp Autobuild-User(master): David Disseldorp Autobuild-Date(master): Tue Nov 12 19:22:28 CET 2013 on sn-devel-104 (cherry picked from commit 744abc882284bfde41b087bc06e13160b915f371) --- source4/dsdb/samdb/ldb_modules/rootdse.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/rootdse.c b/source4/dsdb/samdb/ldb_modules/rootdse.c index f905aa2..7e1d277 100644 --- a/source4/dsdb/samdb/ldb_modules/rootdse.c +++ b/source4/dsdb/samdb/ldb_modules/rootdse.c @@ -1373,6 +1373,7 @@ static void rootdse_fsmo_transfer_callback(struct tevent_req *treq) int ret; struct ldb_request *req = fsmo->req; struct ldb_context *ldb = fsmo->ldb; + struct ldb_module *module = fsmo->module; status = dcerpc_drepl_takeFSMORole_recv(treq, fsmo, &werr); talloc_free(fsmo); @@ -1382,7 +1383,7 @@ static void rootdse_fsmo_transfer_callback(struct tevent_req *treq) * Now that it is failed, start the transaction up * again so the wrappers can close it without additional error */ - ldb_next_start_trans(fsmo->module); + ldb_next_start_trans(module); ldb_module_done(req, NULL, NULL, LDB_ERR_UNAVAILABLE); return; } @@ -1392,7 +1393,7 @@ static void rootdse_fsmo_transfer_callback(struct tevent_req *treq) * Now that it is failed, start the transaction up * again so the wrappers can close it without additional error */ - ldb_next_start_trans(fsmo->module); + ldb_next_start_trans(module); ldb_module_done(req, NULL, NULL, LDB_ERR_UNAVAILABLE); return; } @@ -1401,7 +1402,7 @@ static void rootdse_fsmo_transfer_callback(struct tevent_req *treq) * Now that it is done, start the transaction up again so the * wrappers can close it without error */ - ret = ldb_next_start_trans(fsmo->module); + ret = ldb_next_start_trans(module); ldb_module_done(req, NULL, NULL, ret); } -- 1.9.1 From 18ec41155182fb0300223615d557151826ff1f5d Mon Sep 17 00:00:00 2001 From: Christian Ambach Date: Tue, 10 Dec 2013 17:52:29 +0100 Subject: [PATCH 5/9] s4:dsdb fix compiler warnings about potentially uninitialized variables Signed-off-by: Christian Ambach Reviewed-by: Jeremy Allison (cherry picked from commit 2bd15d1b830b177ea234aa29ff696379abbcd683) --- source4/dsdb/samdb/ldb_modules/samldb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index a63a44e..3118751 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -2002,7 +2002,7 @@ static int samldb_service_principal_names_change(struct samldb_ctx *ac) /* Create a temporary message for fetching the "sAMAccountName" */ if (el2 != NULL) { - char *tempstr, *tempstr2; + char *tempstr, *tempstr2 = NULL; const char *acct_attrs[] = { "sAMAccountName", NULL }; msg = ldb_msg_new(ac->msg); -- 1.9.1 From 5ba24597a894ec9873a517f95c10cc3a2e3edb04 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 27 Feb 2014 09:29:36 +0100 Subject: [PATCH 6/9] s4:dsdb/ldb_modules: avoid invalid pointer type warnings Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit ee06cbce30ccdb057ffc5b3da2d7fb1010a13837) --- source4/dsdb/samdb/ldb_modules/samldb.c | 1 - source4/dsdb/samdb/ldb_modules/update_keytab.c | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c index 3118751..b73e45e 100644 --- a/source4/dsdb/samdb/ldb_modules/samldb.c +++ b/source4/dsdb/samdb/ldb_modules/samldb.c @@ -2757,7 +2757,6 @@ static int check_rename_constraints(struct ldb_message *msg, static int samldb_rename_search_base_callback(struct ldb_request *req, struct ldb_reply *ares) { - struct ldb_request *rename_req; struct samldb_ctx *ac; int ret; diff --git a/source4/dsdb/samdb/ldb_modules/update_keytab.c b/source4/dsdb/samdb/ldb_modules/update_keytab.c index bec4a83..3291a8e 100644 --- a/source4/dsdb/samdb/ldb_modules/update_keytab.c +++ b/source4/dsdb/samdb/ldb_modules/update_keytab.c @@ -397,7 +397,7 @@ static int update_kt_prepare_commit(struct ldb_module *module) const char *realm; char *upper_realm; struct ldb_message_element *spn_el = ldb_msg_find_element(p->msg, "servicePrincipalName"); - char **SPNs = NULL; + const char **SPNs = NULL; int num_SPNs = 0; int i; @@ -411,13 +411,13 @@ static int update_kt_prepare_commit(struct ldb_module *module) } num_SPNs = spn_el->num_values; - SPNs = talloc_array(tmp_ctx, char *, num_SPNs); + SPNs = talloc_array(tmp_ctx, const char *, num_SPNs); if (!SPNs) { ldb_oom(ldb); goto fail; } for (i = 0; i < num_SPNs; i++) { - SPNs[i] = talloc_asprintf(tmp_ctx, "%*.*s@%s", + SPNs[i] = talloc_asprintf(SPNs, "%*.*s@%s", (int)spn_el->values[i].length, (int)spn_el->values[i].length, (const char *)spn_el->values[i].data, @@ -432,7 +432,7 @@ static int update_kt_prepare_commit(struct ldb_module *module) krb5_ret = smb_krb5_update_keytab(tmp_ctx, smb_krb5_context->krb5_context, keytab_name_from_msg(tmp_ctx, ldb, p->msg), ldb_msg_find_attr_as_string(p->msg, "samAccountName", NULL), - realm, (const char **)SPNs, num_SPNs, + realm, SPNs, num_SPNs, ldb_msg_find_attr_as_string(p->msg, "saltPrincipal", NULL), ldb_msg_find_attr_as_string(p->msg, "secret", NULL), ldb_msg_find_attr_as_string(p->msg, "priorSecret", NULL), -- 1.9.1 From f56d65396d99f6562e8e237add4cac94c0f3a0fd Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Sun, 18 Aug 2013 19:37:56 +0000 Subject: [PATCH 7/9] samdb: Fix CID 1034910 Dereference before null check strncmp("tdb://", sam_name, 6) dereferences sam_name. Check for NULL before that. Signed-off-by: Volker Lendecke Reviewed-by: Andrew Bartlett (cherry picked from commit 8c4e6f0cba164c91661a654e2ccc13c265a06953) --- source4/dsdb/samdb/ldb_modules/schema_load.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c index 3397669..75e065f 100644 --- a/source4/dsdb/samdb/ldb_modules/schema_load.c +++ b/source4/dsdb/samdb/ldb_modules/schema_load.c @@ -68,13 +68,13 @@ static int schema_metadata_open(struct ldb_module *module) } sam_name = (const char *)ldb_get_opaque(ldb, "ldb_url"); - if (strncmp("tdb://", sam_name, 6) == 0) { - sam_name += 6; - } if (!sam_name) { talloc_free(tmp_ctx); return ldb_operr(ldb); } + if (strncmp("tdb://", sam_name, 6) == 0) { + sam_name += 6; + } filename = talloc_asprintf(tmp_ctx, "%s.d/metadata.tdb", sam_name); if (!filename) { talloc_free(tmp_ctx); -- 1.9.1 From 2844aefc0c2bf6a35a38b620220ca528293762b4 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Sun, 18 Aug 2013 19:37:56 +0000 Subject: [PATCH 8/9] samdb: Fix CID 1034910 Dereference before null check strncmp("tdb://", secrets_ldb, 6) dereferences secrets_ldb. Check for NULL before that. Signed-off-by: Volker Lendecke Reviewed-by: Andrew Bartlett (cherry picked from commit 35330aa2c8b255d74e94bc9dd742e621953c21f9) --- source4/dsdb/samdb/ldb_modules/secrets_tdb_sync.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/secrets_tdb_sync.c b/source4/dsdb/samdb/ldb_modules/secrets_tdb_sync.c index e3d8485..284aa1b 100644 --- a/source4/dsdb/samdb/ldb_modules/secrets_tdb_sync.c +++ b/source4/dsdb/samdb/ldb_modules/secrets_tdb_sync.c @@ -489,12 +489,12 @@ static int secrets_tdb_sync_init(struct ldb_module *module) ldb_module_set_private(module, data); secrets_ldb = (const char *)ldb_get_opaque(ldb, "ldb_url"); - if (strncmp("tdb://", secrets_ldb, 6) == 0) { - secrets_ldb += 6; - } if (!secrets_ldb) { return ldb_operr(ldb); } + if (strncmp("tdb://", secrets_ldb, 6) == 0) { + secrets_ldb += 6; + } private_dir = talloc_strdup(data, secrets_ldb); p = strrchr(private_dir, '/'); if (p) { -- 1.9.1 From eaa94eb818d4763848d8332559f121fea3fc682e Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 13 May 2014 17:47:03 +1200 Subject: [PATCH 9/9] passdb: Do not routinely clear the global memory returned by get_global_sam_sid() This avoids use-after-free errors and tdb database churn. Andrew Bartlett Change-Id: If7ab2e24556d9dffc7ad22c0489d665dd75a0cab Signed-off-by: Andrew Bartlett Reviewed-by: Kamen Mazdrashki (cherry picked from commit cda32d4e47aa3efb040eb60f1a0332ea8dd58417) --- source3/passdb/machine_account_secrets.c | 10 ++++--- source3/passdb/pdb_samba_dsdb.c | 46 +++++++++++++++++++++++--------- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c index 5758efe..4e35a72 100644 --- a/source3/passdb/machine_account_secrets.c +++ b/source3/passdb/machine_account_secrets.c @@ -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; } diff --git a/source3/passdb/pdb_samba_dsdb.c b/source3/passdb/pdb_samba_dsdb.c index 3fc266c..cbeb332 100644 --- a/source3/passdb/pdb_samba_dsdb.c +++ b/source3/passdb/pdb_samba_dsdb.c @@ -2193,6 +2193,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); @@ -2200,20 +2204,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: -- 1.9.1