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>

1 files changed, 65 insertions, 47 deletions

diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c
index 8963b0436e..5caf4b9e97 100644
--- 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) {