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 <abartlet@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 5da75f2d28f48018153d2f66cbdcbbecc9001bf1..5601474b446aa6fe889ce4554fb0315ea500289f 100644 (file)
--- 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 3978d82a79a201c5358c3c580de61dc77ebf6c66..207a002556bb3dc3982c970260b66b08f077b6a6 100644 (file)
--- 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 b0fdfeb3967245c2fb92971242f8f30361a735a5..c646ccd9f7780fce548f6e2881c9ac69b5bf84c8 100644 (file)
--- 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 <ldb.h>
 #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 995f54825b57cae773be9aa24553ebcd9dad980f..65ddb7e444c504e0612f9474b634f89891028f24 100644 (file)
--- 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 0addfdf58ea764d874ae7033eda9b0ca4d2acb34..3f985bbfc7140a34a182fb030392f1338ec89884 100644 (file)
--- 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 cfd6d148b0a37430f27f2323ee6ca847f6375d4e..1f788ec9cc70655d3e46f39e8a83dd4632c8d729 100644 (file)
--- 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 c0fbaacd4d8d2c0834903eb15138941729148bda..0a349f401ff1e40907b3c5f6fd1a0ee956c2d177 100644 (file)
--- 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);
        }