From 40166d7ecbc6db9e7d7ce761a0c770e025c3895f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 27 Dec 2005 07:49:34 +0000 Subject: r12506: Fix up issues shown up by the expanded RPC-SAMR testsuite, and add ldb transactions to the SAMR password change code. Andrew Bartlett (This used to be commit dc091c6c06b5e5488bcc475e88a9f18ead545c85) --- source4/rpc_server/samr/dcesrv_samr.c | 20 ++-- source4/rpc_server/samr/samr_password.c | 163 +++++++++++++++++++++++++------- 2 files changed, 140 insertions(+), 43 deletions(-) (limited to 'source4') diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c index 11d9bca068..de08695502 100644 --- a/source4/rpc_server/samr/dcesrv_samr.c +++ b/source4/rpc_server/samr/dcesrv_samr.c @@ -752,7 +752,8 @@ static NTSTATUS samr_CreateUser2(struct dcesrv_call_state *dce_call, TALLOC_CTX ret = ldb_transaction_start(d_state->sam_ctx); if (ret != 0) { - DEBUG(0,("Failed to start a transaction for user creation\n")); + DEBUG(0,("Failed to start a transaction for user creation: %s\n", + ldb_errstring(d_state->sam_ctx))); return NT_STATUS_INTERNAL_DB_CORRUPTION; } @@ -825,8 +826,9 @@ static NTSTATUS samr_CreateUser2(struct dcesrv_call_state *dce_call, TALLOC_CTX ret = samdb_add(d_state->sam_ctx, mem_ctx, msg); if (ret != 0) { ldb_transaction_cancel(d_state->sam_ctx); - DEBUG(0,("Failed to create user record %s\n", - ldb_dn_linearize(mem_ctx, msg->dn))); + DEBUG(0,("Failed to create user record %s: %s\n", + ldb_dn_linearize(mem_ctx, msg->dn), + ldb_errstring(d_state->sam_ctx))); return NT_STATUS_INTERNAL_DB_CORRUPTION; } @@ -885,18 +887,20 @@ static NTSTATUS samr_CreateUser2(struct dcesrv_call_state *dce_call, TALLOC_CTX /* modify the samdb record */ ret = samdb_replace(a_state->sam_ctx, mem_ctx, msg); if (ret != 0) { - DEBUG(0,("Failed to modify account record %s to set userAccountControl\n", - ldb_dn_linearize(mem_ctx, msg->dn))); + DEBUG(0,("Failed to modify account record %s to set userAccountControl: %s\n", + ldb_dn_linearize(mem_ctx, msg->dn), + ldb_errstring(d_state->sam_ctx))); ldb_transaction_cancel(d_state->sam_ctx); /* we really need samdb.c to return NTSTATUS */ return NT_STATUS_UNSUCCESSFUL; } - ldb_transaction_commit(d_state->sam_ctx); + ret = ldb_transaction_commit(d_state->sam_ctx); if (ret != 0) { - DEBUG(0,("Failed to commit transaction to add and modify account record %s\n", - ldb_dn_linearize(mem_ctx, msg->dn))); + DEBUG(0,("Failed to commit transaction to add and modify account record %s: %s\n", + ldb_dn_linearize(mem_ctx, msg->dn), + ldb_errstring(d_state->sam_ctx))); return NT_STATUS_INTERNAL_DB_CORRUPTION; } diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index 95524c22a9..a83a8f6080 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -50,21 +50,7 @@ NTSTATUS samr_ChangePasswordUser(struct dcesrv_call_state *dce_call, TALLOC_CTX a_state = h->data; - /* To change a password we need to open as system */ - sam_ctx = samdb_connect(mem_ctx, system_session(mem_ctx)); - if (sam_ctx == NULL) { - return NT_STATUS_INVALID_SYSTEM_SERVICE; - } - - /* fetch the old hashes */ - ret = gendb_search_dn(sam_ctx, mem_ctx, - a_state->account_dn, &res, attrs); - if (ret != 1) { - return NT_STATUS_INTERNAL_DB_CORRUPTION; - } - msg = res[0]; - - /* basic sanity checking on parameters */ + /* basic sanity checking on parameters. Do this before any database ops */ if (!r->in.lm_present || !r->in.nt_present || !r->in.old_lm_crypted || !r->in.new_lm_crypted || !r->in.old_nt_crypted || !r->in.new_nt_crypted) { @@ -79,8 +65,30 @@ NTSTATUS samr_ChangePasswordUser(struct dcesrv_call_state *dce_call, TALLOC_CTX return NT_STATUS_LM_CROSS_ENCRYPTION_REQUIRED; } + /* To change a password we need to open as system */ + sam_ctx = samdb_connect(mem_ctx, system_session(mem_ctx)); + if (sam_ctx == NULL) { + return NT_STATUS_INVALID_SYSTEM_SERVICE; + } + + ret = ldb_transaction_start(sam_ctx); + if (ret) { + DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); + return NT_STATUS_INTERNAL_DB_CORRUPTION; + } + + /* fetch the old hashes */ + ret = gendb_search_dn(sam_ctx, mem_ctx, + a_state->account_dn, &res, attrs); + if (ret != 1) { + ldb_transaction_cancel(sam_ctx); + return NT_STATUS_WRONG_PASSWORD; + } + msg = res[0]; + status = samdb_result_passwords(mem_ctx, msg, &lm_pwd, &nt_pwd); if (!NT_STATUS_IS_OK(status) || !lm_pwd || !nt_pwd) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_WRONG_PASSWORD; } @@ -88,6 +96,7 @@ NTSTATUS samr_ChangePasswordUser(struct dcesrv_call_state *dce_call, TALLOC_CTX D_P16(lm_pwd->hash, r->in.new_lm_crypted->hash, new_lmPwdHash.hash); D_P16(new_lmPwdHash.hash, r->in.old_lm_crypted->hash, checkHash.hash); if (memcmp(checkHash.hash, lm_pwd, 16) != 0) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_WRONG_PASSWORD; } @@ -95,31 +104,38 @@ NTSTATUS samr_ChangePasswordUser(struct dcesrv_call_state *dce_call, TALLOC_CTX D_P16(nt_pwd->hash, r->in.new_nt_crypted->hash, new_ntPwdHash.hash); D_P16(new_ntPwdHash.hash, r->in.old_nt_crypted->hash, checkHash.hash); if (memcmp(checkHash.hash, nt_pwd, 16) != 0) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_WRONG_PASSWORD; } /* check the nt cross hash */ D_P16(lm_pwd->hash, r->in.nt_cross->hash, checkHash.hash); if (memcmp(checkHash.hash, new_ntPwdHash.hash, 16) != 0) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_WRONG_PASSWORD; } /* check the lm cross hash */ D_P16(nt_pwd->hash, r->in.lm_cross->hash, checkHash.hash); if (memcmp(checkHash.hash, new_lmPwdHash.hash, 16) != 0) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_WRONG_PASSWORD; } msg = ldb_msg_new(mem_ctx); if (msg == NULL) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_NO_MEMORY; } msg->dn = ldb_dn_copy(msg, a_state->account_dn); if (!msg->dn) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_NO_MEMORY; } + /* set the password on the user DN specified. This may fail + * due to password policies */ status = samdb_set_password(sam_ctx, mem_ctx, a_state->account_dn, a_state->domain_state->domain_dn, msg, NULL, &new_lmPwdHash, &new_ntPwdHash, @@ -128,15 +144,27 @@ NTSTATUS samr_ChangePasswordUser(struct dcesrv_call_state *dce_call, TALLOC_CTX NULL, NULL); if (!NT_STATUS_IS_OK(status)) { + ldb_transaction_cancel(sam_ctx); return status; } - /* modify the samdb record */ + /* The above call only setup the modifications, this actually + * makes the write to the database. */ ret = samdb_replace(sam_ctx, mem_ctx, msg); if (ret != 0) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_UNSUCCESSFUL; } + /* And this confirms it in a transaction commit */ + ret = ldb_transaction_commit(sam_ctx); + if (ret != 0) { + DEBUG(0,("Failed to commit transaction to change password on %s: %s\n", + ldb_dn_linearize(mem_ctx, a_state->account_dn), + ldb_errstring(sam_ctx))); + return NT_STATUS_INTERNAL_DB_CORRUPTION; + } + return NT_STATUS_OK; } @@ -170,6 +198,12 @@ NTSTATUS samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call, TALLOC_ return NT_STATUS_INVALID_SYSTEM_SERVICE; } + ret = ldb_transaction_start(sam_ctx); + if (ret) { + DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); + return NT_STATUS_INTERNAL_DB_CORRUPTION; + } + /* we need the users dn and the domain dn (derived from the user SID). We also need the current lm password hash in order to decrypt the incoming password */ @@ -178,13 +212,16 @@ NTSTATUS samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call, TALLOC_ "(&(sAMAccountName=%s)(objectclass=user))", r->in.account->string); if (ret != 1) { - return NT_STATUS_NO_SUCH_USER; + ldb_transaction_cancel(sam_ctx); + /* Don't give the game away: (don't allow anonymous users to prove the existance of usernames) */ + return NT_STATUS_WRONG_PASSWORD; } user_dn = res[0]->dn; status = samdb_result_passwords(mem_ctx, res[0], &lm_pwd, NULL); if (!NT_STATUS_IS_OK(status) || !lm_pwd) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_WRONG_PASSWORD; } @@ -195,33 +232,38 @@ NTSTATUS samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call, TALLOC_ if (!decode_pw_buffer(pwbuf->data, new_pass, sizeof(new_pass), &new_pass_len, STR_ASCII)) { + ldb_transaction_cancel(sam_ctx); DEBUG(3,("samr: failed to decode password buffer\n")); return NT_STATUS_WRONG_PASSWORD; } /* check LM verifier */ if (lm_pwd == NULL || r->in.hash == NULL) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_WRONG_PASSWORD; } E_deshash(new_pass, new_lm_hash); E_old_pw_hash(new_lm_hash, lm_pwd->hash, lm_verifier.hash); if (memcmp(lm_verifier.hash, r->in.hash->hash, 16) != 0) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_WRONG_PASSWORD; } mod = ldb_msg_new(mem_ctx); if (mod == NULL) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_NO_MEMORY; } mod->dn = ldb_dn_copy(mod, user_dn); if (!mod->dn) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_NO_MEMORY; } - /* set the password - samdb needs to know both the domain and user DNs, - so the domain password policy can be used */ + /* set the password on the user DN specified. This may fail + * due to password policies */ status = samdb_set_password(sam_ctx, mem_ctx, user_dn, NULL, mod, new_pass, @@ -231,15 +273,27 @@ NTSTATUS samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call, TALLOC_ NULL, NULL); if (!NT_STATUS_IS_OK(status)) { + ldb_transaction_cancel(sam_ctx); return status; } - /* modify the samdb record */ + /* The above call only setup the modifications, this actually + * makes the write to the database. */ ret = samdb_replace(sam_ctx, mem_ctx, mod); if (ret != 0) { + ldb_transaction_cancel(sam_ctx); return NT_STATUS_UNSUCCESSFUL; } + /* And this confirms it in a transaction commit */ + ret = ldb_transaction_commit(sam_ctx); + if (ret != 0) { + DEBUG(0,("Failed to commit transaction to change password on %s: %s\n", + ldb_dn_linearize(mem_ctx, user_dn), + ldb_errstring(sam_ctx))); + return NT_STATUS_INTERNAL_DB_CORRUPTION; + } + return NT_STATUS_OK; } @@ -254,16 +308,16 @@ NTSTATUS samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, NTSTATUS status; char new_pass[512]; uint32_t new_pass_len; - struct ldb_context *sam_ctx; + struct ldb_context *sam_ctx = NULL; const struct ldb_dn *user_dn; int ret; struct ldb_message **res, *mod; const char * const attrs[] = { "ntPwdHash", "lmPwdHash", "unicodePwd", NULL }; struct samr_Password *nt_pwd, *lm_pwd; DATA_BLOB nt_pwd_blob; - struct samr_DomInfo1 *dominfo; - struct samr_ChangeReject *reject; - uint32_t reason = 0; + struct samr_DomInfo1 *dominfo = NULL; + struct samr_ChangeReject *reject = NULL; + enum samr_RejectReason reason = SAMR_REJECT_OTHER; uint8_t new_nt_hash[16], new_lm_hash[16]; struct samr_Password nt_verifier, lm_verifier; @@ -271,15 +325,20 @@ NTSTATUS samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, if (r->in.nt_password == NULL || r->in.nt_verifier == NULL) { - status = NT_STATUS_INVALID_PARAMETER; - goto failed; + return NT_STATUS_INVALID_PARAMETER; } /* To change a password we need to open as system */ sam_ctx = samdb_connect(mem_ctx, system_session(mem_ctx)); if (sam_ctx == NULL) { return NT_STATUS_INVALID_SYSTEM_SERVICE; - goto failed; + } + + ret = ldb_transaction_start(sam_ctx); + if (ret) { + talloc_free(sam_ctx); + DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); + return NT_STATUS_INTERNAL_DB_CORRUPTION; } /* we need the users dn and the domain dn (derived from the @@ -290,7 +349,8 @@ NTSTATUS samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, "(&(sAMAccountName=%s)(objectclass=user))", r->in.account->string); if (ret != 1) { - status = NT_STATUS_NO_SUCH_USER; + /* Don't give the game away: (don't allow anonymous users to prove the existance of usernames) */ + status = NT_STATUS_WRONG_PASSWORD; goto failed; } @@ -353,8 +413,8 @@ NTSTATUS samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, goto failed; } - /* set the password - samdb needs to know both the domain and user DNs, - so the domain password policy can be used */ + /* set the password on the user DN specified. This may fail + * due to password policies */ status = samdb_set_password(sam_ctx, mem_ctx, user_dn, NULL, mod, new_pass, @@ -367,18 +427,34 @@ NTSTATUS samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, goto failed; } - /* modify the samdb record */ + /* The above call only setup the modifications, this actually + * makes the write to the database. */ ret = samdb_replace(sam_ctx, mem_ctx, mod); if (ret != 0) { status = NT_STATUS_UNSUCCESSFUL; goto failed; } + /* And this confirms it in a transaction commit */ + ret = ldb_transaction_commit(sam_ctx); + if (ret != 0) { + DEBUG(0,("Failed to commit transaction to change password on %s: %s\n", + ldb_dn_linearize(mem_ctx, user_dn), + ldb_errstring(sam_ctx))); + status = NT_STATUS_INTERNAL_DB_CORRUPTION; + goto failed; + } + return NT_STATUS_OK; failed: + ldb_transaction_cancel(sam_ctx); + talloc_free(sam_ctx); reject = talloc(mem_ctx, struct samr_ChangeReject); + r->out.dominfo = dominfo; + r->out.reject = reject; + if (reject == NULL) { return status; } @@ -386,9 +462,6 @@ failed: reject->reason = reason; - r->out.dominfo = dominfo; - r->out.reject = reject; - return status; } @@ -433,6 +506,8 @@ static BOOL samdb_password_complexity_ok(const char *pass) elements to setup the correct change when samdb_replace() is called. This allows the caller to combine the change with other changes (as is needed by some of the set user info levels) + + The caller should probably have a transaction wrapping this */ NTSTATUS samdb_set_password(struct ldb_context *ctx, TALLOC_CTX *mem_ctx, const struct ldb_dn *user_dn, @@ -443,7 +518,7 @@ NTSTATUS samdb_set_password(struct ldb_context *ctx, TALLOC_CTX *mem_ctx, struct samr_Password *ntNewHash, BOOL user_change, BOOL restrictions, - uint32_t *reject_reason, + enum samr_RejectReason *reject_reason, struct samr_DomInfo1 **_dominfo) { const char * const user_attrs[] = { "userAccountControl", "lmPwdHistory", @@ -838,10 +913,17 @@ NTSTATUS samdb_set_password_sid(struct ldb_context *ctx, TALLOC_CTX *mem_ctx, struct ldb_message *msg; int ret; + ret = ldb_transaction_start(ctx); + if (ret) { + DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(ctx))); + return NT_STATUS_INTERNAL_DB_CORRUPTION; + } + user_dn = samdb_search_dn(ctx, mem_ctx, NULL, "(&(objectSid=%s)(objectClass=user))", ldap_encode_ndr_dom_sid(mem_ctx, user_sid)); if (!user_dn) { + ldb_transaction_cancel(ctx); DEBUG(3, ("samdb_set_password_sid: SID %s not found in samdb, returning NO_SUCH_USER\n", dom_sid_string(mem_ctx, user_sid))); return NT_STATUS_NO_SUCH_USER; @@ -849,11 +931,13 @@ NTSTATUS samdb_set_password_sid(struct ldb_context *ctx, TALLOC_CTX *mem_ctx, msg = ldb_msg_new(mem_ctx); if (msg == NULL) { + ldb_transaction_cancel(ctx); return NT_STATUS_NO_MEMORY; } msg->dn = ldb_dn_copy(msg, user_dn); if (!msg->dn) { + ldb_transaction_cancel(ctx); return NT_STATUS_NO_MEMORY; } @@ -865,14 +949,23 @@ NTSTATUS samdb_set_password_sid(struct ldb_context *ctx, TALLOC_CTX *mem_ctx, restrictions, /* run restriction tests */ reject_reason, _dominfo); if (!NT_STATUS_IS_OK(nt_status)) { + ldb_transaction_cancel(ctx); return nt_status; } /* modify the samdb record */ ret = samdb_replace(ctx, mem_ctx, msg); if (ret != 0) { + ldb_transaction_cancel(ctx); return NT_STATUS_ACCESS_DENIED; } + ret = ldb_transaction_commit(ctx); + if (ret != 0) { + DEBUG(0,("Failed to commit transaction to change password on %s: %s\n", + ldb_dn_linearize(mem_ctx, msg->dn), + ldb_errstring(ctx))); + return NT_STATUS_INTERNAL_DB_CORRUPTION; + } return NT_STATUS_OK; } -- cgit