From: Andrew Bartlett Date: Wed, 9 Feb 2022 03:33:23 +0000 (+1300) Subject: dsdb: No longer supply exact password hashes in a control to indicate password changes X-Git-Tag: tevent-0.12.0~411 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1144addec5043d39fc5149aa2b93fe6b974cab7d;p=thirdparty%2Fsamba.git dsdb: No longer supply exact password hashes in a control to indicate password changes This returns the API for password changes via (eg) kpasswd to the previous design as at 7eebcebbab8f62935bd1d5460e58b0a8f2cc30e8 where a control but no partiuclar values were specified. This avoids the issues that were attempted to be addressed between 7eebcebbab8f62935bd1d5460e58b0a8f2cc30e8 and 786c41b0954b541518d1096019e1ce7ca11e5e98 by still keeping the ACL check from 23bd3a74176be4a1f8d6d70b148ababee397cf8c. The purpose of this change is to move away from the NT hash (unicodePwd) being the primary password in Samba, to allow installations to operate without this unsalted hash. Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher --- diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 5da75f2d28f..5601474b446 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -2257,8 +2257,7 @@ static NTSTATUS samdb_set_password_internal(struct ldb_context *ldb, TALLOC_CTX const DATA_BLOB *new_password, const struct samr_Password *lmNewHash, const struct samr_Password *ntNewHash, - const struct samr_Password *lmOldHash, - const struct samr_Password *ntOldHash, + enum dsdb_password_checked old_password_checked, enum samPwdChangeReason *reject_reason, struct samr_DomInfo1 **_dominfo, bool permit_interdomain_trust) @@ -2320,7 +2319,7 @@ static NTSTATUS samdb_set_password_internal(struct ldb_context *ldb, TALLOC_CTX } /* A password change operation */ - if ((ntOldHash != NULL) || (lmOldHash != NULL)) { + if (old_password_checked == DSDB_PASSWORD_CHECKED_AND_CORRECT) { struct dsdb_control_password_change *change; change = talloc(req, struct dsdb_control_password_change); @@ -2330,8 +2329,7 @@ static NTSTATUS samdb_set_password_internal(struct ldb_context *ldb, TALLOC_CTX return NT_STATUS_NO_MEMORY; } - change->old_nt_pwd_hash = ntOldHash; - change->old_lm_pwd_hash = lmOldHash; + change->old_password_checked = old_password_checked; ret = ldb_request_add_control(req, DSDB_CONTROL_PASSWORD_CHANGE_OID, @@ -2453,8 +2451,7 @@ NTSTATUS samdb_set_password(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, const DATA_BLOB *new_password, const struct samr_Password *lmNewHash, const struct samr_Password *ntNewHash, - const struct samr_Password *lmOldHash, - const struct samr_Password *ntOldHash, + enum dsdb_password_checked old_password_checked, enum samPwdChangeReason *reject_reason, struct samr_DomInfo1 **_dominfo) { @@ -2462,7 +2459,7 @@ NTSTATUS samdb_set_password(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, user_dn, domain_dn, new_password, lmNewHash, ntNewHash, - lmOldHash, ntOldHash, + old_password_checked, reject_reason, _dominfo, false); /* reject trusts */ } @@ -2491,8 +2488,7 @@ NTSTATUS samdb_set_password_sid(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, const DATA_BLOB *new_password, const struct samr_Password *lmNewHash, const struct samr_Password *ntNewHash, - const struct samr_Password *lmOldHash, - const struct samr_Password *ntOldHash, + enum dsdb_password_checked old_password_checked, enum samPwdChangeReason *reject_reason, struct samr_DomInfo1 **_dominfo) { @@ -2858,7 +2854,7 @@ NTSTATUS samdb_set_password_sid(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, user_msg->dn, NULL, new_password, lmNewHash, ntNewHash, - lmOldHash, ntOldHash, + old_password_checked, reject_reason, _dominfo, true); /* permit trusts */ if (!NT_STATUS_IS_OK(nt_status)) { diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c index 3978d82a79a..207a002556b 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -2815,8 +2815,13 @@ static int check_password_restrictions(struct setup_password_fields_io *io, WERR return LDB_SUCCESS; } - /* First check the old password is correct, for password changes */ - if (!io->ac->pwd_reset) { + /* + * First check the old password is correct, for password + * changes when this hasn't already been checked by a + * trustwrothy layer above + */ + if (!io->ac->pwd_reset && !(io->ac->change + && io->ac->change->old_password_checked == DSDB_PASSWORD_CHECKED_AND_CORRECT)) { bool nt_hash_checked = false; /* we need the old nt or lm hash given by the client */ @@ -3627,17 +3632,7 @@ static int setup_io(struct ph_context *ac, */ if (ac->change != NULL) { io->og.nt_hash = NULL; - if (ac->change->old_nt_pwd_hash != NULL) { - io->og.nt_hash = talloc_memdup(io->ac, - ac->change->old_nt_pwd_hash, - sizeof(struct samr_Password)); - } io->og.lm_hash = NULL; - if (lpcfg_lanman_auth(lp_ctx) && (ac->change->old_lm_pwd_hash != NULL)) { - io->og.lm_hash = talloc_memdup(io->ac, - ac->change->old_lm_pwd_hash, - sizeof(struct samr_Password)); - } } /* refuse the change if someone wants to change the clear- @@ -3677,13 +3672,16 @@ static int setup_io(struct ph_context *ac, return LDB_ERR_UNWILLING_TO_PERFORM; } - /* refuse the change if someone wants to compare against a plaintext - or hash at the same time for a "password modify" operation... */ + /* + * refuse the change if someone wants to compare against a + * plaintext or dsdb_control_password_change at the same time + * for a "password modify" operation... + */ if ((io->og.cleartext_utf8 || io->og.cleartext_utf16) - && (io->og.nt_hash || io->og.lm_hash)) { + && ac->change) { ldb_asprintf_errstring(ldb, "setup_io: " - "it's only allowed to provide the old password in form of cleartext attributes or as hashes"); + "it's only allowed to provide the old password in form of cleartext attributes or as the dsdb_control_password_change"); return LDB_ERR_UNWILLING_TO_PERFORM; } @@ -3730,9 +3728,12 @@ static int setup_io(struct ph_context *ac, */ ac->pwd_reset = pav->pwd_reset; } else if (io->og.cleartext_utf8 || io->og.cleartext_utf16 - || io->og.nt_hash || io->og.lm_hash) { - /* If we have an old password specified then for sure it - * is a user "password change" */ + || ac->change) { + /* + * If we have an old password specified or the + * dsdb_control_password_change then for sure + * it is a user "password change" + */ ac->pwd_reset = false; } else { /* Otherwise we have also here a "password reset" */ diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h index b0fdfeb3967..c646ccd9f77 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -31,6 +31,12 @@ struct tevent_context; struct tsocket_address; struct dsdb_trust_routing_table; +enum dsdb_password_checked { + DSDB_PASSWORD_NOT_CHECKED = 0, /* unused */ + DSDB_PASSWORD_RESET, + DSDB_PASSWORD_CHECKED_AND_CORRECT +}; + #include "librpc/gen_ndr/security.h" #include #include "lib/ldb-samba/ldif_handlers.h" @@ -97,8 +103,7 @@ struct dsdb_control_password_change_status { #define DSDB_CONTROL_PASSWORD_CHANGE_OID "1.3.6.1.4.1.7165.4.3.10" struct dsdb_control_password_change { - const struct samr_Password *old_nt_pwd_hash; - const struct samr_Password *old_lm_pwd_hash; + enum dsdb_password_checked old_password_checked; }; /** diff --git a/source4/kdc/kpasswd-helper.c b/source4/kdc/kpasswd-helper.c index 995f54825b5..65ddb7e444c 100644 --- a/source4/kdc/kpasswd-helper.c +++ b/source4/kdc/kpasswd-helper.c @@ -223,8 +223,7 @@ NTSTATUS kpasswd_samdb_set_password(TALLOC_CTX *mem_ctx, password, NULL, /* lmNewHash */ NULL, /* ntNewHash */ - NULL, /* lmOldHash */ - NULL, /* ntOldHash */ + DSDB_PASSWORD_RESET, reject_reason, dominfo); if (NT_STATUS_IS_OK(status)) { diff --git a/source4/kdc/kpasswd_glue.c b/source4/kdc/kpasswd_glue.c index 0addfdf58ea..3f985bbfc71 100644 --- a/source4/kdc/kpasswd_glue.c +++ b/source4/kdc/kpasswd_glue.c @@ -103,8 +103,7 @@ NTSTATUS samdb_kpasswd_change_password(TALLOC_CTX *mem_ctx, password, NULL, NULL, - oldLmHash, - oldNtHash, /* this is a user password change */ + DSDB_PASSWORD_CHECKED_AND_CORRECT, reject_reason, dominfo); if (!NT_STATUS_IS_OK(status)) { diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c index cfd6d148b0a..1f788ec9cc7 100644 --- a/source4/rpc_server/netlogon/dcerpc_netlogon.c +++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c @@ -787,7 +787,7 @@ static NTSTATUS dcesrv_netr_ServerPasswordSet(struct dcesrv_call_state *dce_call NULL, /* Don't have version */ NULL, /* Don't have plaintext */ NULL, r->in.new_password, - NULL, oldNtHash, /* Password change */ + DSDB_PASSWORD_CHECKED_AND_CORRECT, /* Password change */ NULL, NULL); return nt_status; } @@ -948,7 +948,7 @@ static NTSTATUS dcesrv_netr_ServerPasswordSet2(struct dcesrv_call_state *dce_cal new_version, &new_password, /* we have plaintext */ NULL, NULL, - oldLmHash, oldNtHash, /* Password change */ + DSDB_PASSWORD_CHECKED_AND_CORRECT, /* Password change */ NULL, NULL); return nt_status; } diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index c0fbaacd4d8..0a349f401ff 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -276,7 +276,7 @@ NTSTATUS dcesrv_samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call, user_dn, NULL, &new_unicode_password, NULL, NULL, - lm_pwd, NULL, /* this is a user password change */ + DSDB_PASSWORD_CHECKED_AND_CORRECT, NULL, NULL); if (!NT_STATUS_IS_OK(status)) { @@ -499,7 +499,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, user_dn, NULL, &new_password, NULL, NULL, - lm_pwd, nt_pwd, /* this is a user password change */ + DSDB_PASSWORD_CHECKED_AND_CORRECT, &reason, &dominfo); @@ -661,8 +661,7 @@ NTSTATUS samr_set_password(struct dcesrv_call_state *dce_call, &new_password, NULL, NULL, - NULL, - NULL, /* This is a password set, not change */ + DSDB_PASSWORD_RESET, NULL, NULL); out: @@ -733,8 +732,7 @@ NTSTATUS samr_set_password_ex(struct dcesrv_call_state *dce_call, &new_password, NULL, NULL, - NULL, - NULL, /* This is a password set, not change */ + DSDB_PASSWORD_RESET, NULL, NULL); ZERO_ARRAY_LEN(new_password.data, @@ -810,7 +808,7 @@ NTSTATUS samr_set_password_buffers(struct dcesrv_call_state *dce_call, nt_status = samdb_set_password(sam_ctx, mem_ctx, account_dn, domain_dn, NULL, d_lm_pwd_hash, d_nt_pwd_hash, - NULL, NULL, /* this is a password set */ + DSDB_PASSWORD_RESET, NULL, NULL); }