s4:rpc_server/samr: do WRONG_PASSWORD checks after the complexity checks

This matches the windows behavior.

Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>

Signed-off-by: Michael Adam <obnox@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>

diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c
index 8963b0436eb484bb4edeb00838a9c6dce897163d..5caf4b9e97e0d2c68af53feb8f73645472de229c 100644 (file)
--- a/source4/rpc_server/samr/samr_password.c
+++ b/source4/rpc_server/samr/samr_password.c
@@ -88,34 +88,22 @@ NTSTATUS dcesrv_samr_ChangePasswordUser(struct dcesrv_call_state *dce_call,
        if (lm_pwd) {
                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) {
-                       return NT_STATUS_WRONG_PASSWORD;
-               }
        }
 
        /* decrypt and check the new nt hash */
        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) {
-               return NT_STATUS_WRONG_PASSWORD;
-       }
 
        /* The NT Cross is not required by Win2k3 R2, but if present
           check the nt cross hash */
        if (r->in.cross1_present && r->in.nt_cross && lm_pwd) {
                D_P16(lm_pwd->hash, r->in.nt_cross->hash, checkHash.hash);
-               if (memcmp(checkHash.hash, new_ntPwdHash.hash, 16) != 0) {
-                       return NT_STATUS_WRONG_PASSWORD;
-               }
        }
 
        /* The LM Cross is not required by Win2k3 R2, but if present
           check the lm cross hash */
        if (r->in.cross2_present && r->in.lm_cross && lm_pwd) {
                D_P16(nt_pwd->hash, r->in.lm_cross->hash, checkHash.hash);
-               if (memcmp(checkHash.hash, new_lmPwdHash.hash, 16) != 0) {
-                       return NT_STATUS_WRONG_PASSWORD;
-               }
        }
 
        /* Start a SAM with user privileges for the password change */
@@ -148,6 +136,37 @@ NTSTATUS dcesrv_samr_ChangePasswordUser(struct dcesrv_call_state *dce_call,
                return status;
        }
 
+       /* decrypt and check the new lm hash */
+       if (lm_pwd) {
+               if (memcmp(checkHash.hash, lm_pwd, 16) != 0) {
+                       ldb_transaction_cancel(sam_ctx);
+                       return NT_STATUS_WRONG_PASSWORD;
+               }
+       }
+
+       if (memcmp(checkHash.hash, nt_pwd, 16) != 0) {
+               ldb_transaction_cancel(sam_ctx);
+               return NT_STATUS_WRONG_PASSWORD;
+       }
+
+       /* The NT Cross is not required by Win2k3 R2, but if present
+          check the nt cross hash */
+       if (r->in.cross1_present && r->in.nt_cross && lm_pwd) {
+               if (memcmp(checkHash.hash, new_ntPwdHash.hash, 16) != 0) {
+                       ldb_transaction_cancel(sam_ctx);
+                       return NT_STATUS_WRONG_PASSWORD;
+               }
+       }
+
+       /* The LM Cross is not required by Win2k3 R2, but if present
+          check the lm cross hash */
+       if (r->in.cross2_present && r->in.lm_cross && lm_pwd) {
+               if (memcmp(checkHash.hash, new_lmPwdHash.hash, 16) != 0) {
+                       ldb_transaction_cancel(sam_ctx);
+                       return NT_STATUS_WRONG_PASSWORD;
+               }
+       }
+
        /* And this confirms it in a transaction commit */
        ret = ldb_transaction_commit(sam_ctx);
        if (ret != LDB_SUCCESS) {
@@ -256,9 +275,6 @@ NTSTATUS dcesrv_samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call,
 
        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) {
-               return NT_STATUS_WRONG_PASSWORD;
-       }
 
        /* Connect to a SAMDB with user privileges for the password change */
        sam_ctx = samdb_connect(mem_ctx, dce_call->event_ctx,
@@ -290,6 +306,11 @@ NTSTATUS dcesrv_samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call,
                return status;
        }
 
+       if (memcmp(lm_verifier.hash, r->in.hash->hash, 16) != 0) {
+               ldb_transaction_cancel(sam_ctx);
+               return NT_STATUS_WRONG_PASSWORD;
+       }
+
        /* And this confirms it in a transaction commit */
        ret = ldb_transaction_commit(sam_ctx);
        if (ret != LDB_SUCCESS) {
@@ -379,8 +400,33 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call,
                goto failed;
        }
 
-       if (r->in.nt_verifier == NULL) {
-               status = NT_STATUS_WRONG_PASSWORD;
+       /* Connect to a SAMDB with user privileges for the password change */
+       sam_ctx = samdb_connect(mem_ctx, dce_call->event_ctx,
+                               dce_call->conn->dce_ctx->lp_ctx,
+                               dce_call->conn->auth_state.session_info, 0);
+       if (sam_ctx == NULL) {
+               return NT_STATUS_INVALID_SYSTEM_SERVICE;
+       }
+
+       ret = ldb_transaction_start(sam_ctx);
+       if (ret != LDB_SUCCESS) {
+               DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx)));
+               return NT_STATUS_TRANSACTION_ABORTED;
+       }
+
+       /* Performs the password modification. We pass the old hashes read out
+        * from the database since they were already checked against the user-
+        * provided ones. */
+       status = samdb_set_password(sam_ctx, mem_ctx,
+                                   user_dn, NULL,
+                                   &new_password,
+                                   NULL, NULL,
+                                   lm_pwd, nt_pwd, /* this is a user password change */
+                                   &reason,
+                                   &dominfo);
+
+       if (!NT_STATUS_IS_OK(status)) {
+               ldb_transaction_cancel(sam_ctx);
                goto failed;
        }
 
@@ -389,6 +435,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call,
 
        E_old_pw_hash(new_nt_hash, nt_pwd->hash, nt_verifier.hash);
        if (memcmp(nt_verifier.hash, r->in.nt_verifier->hash, 16) != 0) {
+               ldb_transaction_cancel(sam_ctx);
                status = NT_STATUS_WRONG_PASSWORD;
                goto failed;
        }
@@ -408,42 +455,13 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call,
                        E_deshash(new_pass, new_lm_hash);
                        E_old_pw_hash(new_nt_hash, lm_pwd->hash, lm_verifier.hash);
                        if (memcmp(lm_verifier.hash, r->in.lm_verifier->hash, 16) != 0) {
+                               ldb_transaction_cancel(sam_ctx);
                                status = NT_STATUS_WRONG_PASSWORD;
                                goto failed;
                        }
                }
        }
 
-       /* Connect to a SAMDB with user privileges for the password change */
-       sam_ctx = samdb_connect(mem_ctx, dce_call->event_ctx,
-                               dce_call->conn->dce_ctx->lp_ctx,
-                               dce_call->conn->auth_state.session_info, 0);
-       if (sam_ctx == NULL) {
-               return NT_STATUS_INVALID_SYSTEM_SERVICE;
-       }
-
-       ret = ldb_transaction_start(sam_ctx);
-       if (ret != LDB_SUCCESS) {
-               DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx)));
-               return NT_STATUS_TRANSACTION_ABORTED;
-       }
-
-       /* Performs the password modification. We pass the old hashes read out
-        * from the database since they were already checked against the user-
-        * provided ones. */
-       status = samdb_set_password(sam_ctx, mem_ctx,
-                                   user_dn, NULL,
-                                   &new_password,
-                                   NULL, NULL,
-                                   lm_pwd, nt_pwd, /* this is a user password change */
-                                   &reason,
-                                   &dominfo);
-
-       if (!NT_STATUS_IS_OK(status)) {
-               ldb_transaction_cancel(sam_ctx);
-               goto failed;
-       }
-
        /* And this confirms it in a transaction commit */
        ret = ldb_transaction_commit(sam_ctx);
        if (ret != LDB_SUCCESS) {