CVE-2021-20251 s3:rpc_server: Split change_oem_password() call out of samr_set_password_aes()

Now samr_set_password_aes() just returns the new password in a similar
manner to check_oem_password(). This simplifies the logic for the
following change to recheck whether the account is locked out, and to
update the bad password count.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14611

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

3 files changed, 36 insertions, 32 deletions

diff --git a/source3/rpc_server/samr/srv_samr_chgpasswd.c b/source3/rpc_server/samr/srv_samr_chgpasswd.c
index d720641f05c..8563beb48a7 100644
--- a/source3/rpc_server/samr/srv_samr_chgpasswd.c
+++ b/source3/rpc_server/samr/srv_samr_chgpasswd.c
@@ -1072,10 +1072,10 @@ NTSTATUS check_password_complexity(const char *username,
  is correct before calling. JRA.
 ************************************************************/
 
-static NTSTATUS change_oem_password(struct samu *hnd, const char *rhost,
-				    char *old_passwd, char *new_passwd,
-				    bool as_root,
-				    enum samPwdChangeReason *samr_reject_reason)
+NTSTATUS change_oem_password(struct samu *hnd, const char *rhost,
+			     char *old_passwd, char *new_passwd,
+			     bool as_root,
+			     enum samPwdChangeReason *samr_reject_reason)
 {
 	uint32_t min_len;
 	uint32_t refuse;
@@ -1369,21 +1369,20 @@ done:
 }
 
 NTSTATUS samr_set_password_aes(TALLOC_CTX *mem_ctx,
-			       struct samu *sampass,
-			       const char *rhost,
 			       const DATA_BLOB *cdk,
 			       struct samr_EncryptedPasswordAES *pwbuf,
-			       enum samPwdChangeReason *reject_reason)
+			       char **new_password_str)
 {
 	DATA_BLOB pw_data = data_blob_null;
 	DATA_BLOB new_password = data_blob_null;
 	const DATA_BLOB ciphertext =
 		data_blob_const(pwbuf->cipher, pwbuf->cipher_len);
 	DATA_BLOB iv = data_blob_const(pwbuf->salt, sizeof(pwbuf->salt));
-	char *new_password_str = NULL;
 	NTSTATUS status;
 	bool ok;
 
+	*new_password_str = NULL;
+
 	status = samba_gnutls_aead_aes_256_cbc_hmac_sha512_decrypt(
 		mem_ctx,
 		&ciphertext,
@@ -1407,23 +1406,14 @@ NTSTATUS samr_set_password_aes(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_WRONG_PASSWORD;
 	}
 
-	new_password_str = talloc_strndup(mem_ctx,
-					  (char *)new_password.data,
-					  new_password.length);
+	*new_password_str = talloc_strndup(mem_ctx,
+					   (char *)new_password.data,
+					   new_password.length);
 	TALLOC_FREE(new_password.data);
-	if (new_password_str == NULL) {
+	if (*new_password_str == NULL) {
 		return NT_STATUS_NO_MEMORY;
 	}
+	talloc_keep_secret(*new_password_str);
 
-	become_root();
-	status = change_oem_password(sampass,
-				     rhost,
-				     NULL,
-				     new_password_str,
-				     true,
-				     reject_reason);
-	unbecome_root();
-	TALLOC_FREE(new_password_str);
-
-	return status;
+	return NT_STATUS_OK;
 }
diff --git a/source3/rpc_server/samr/srv_samr_nt.c b/source3/rpc_server/samr/srv_samr_nt.c
index 14bff5ecd7a..1036410f534 100644
--- a/source3/rpc_server/samr/srv_samr_nt.c
+++ b/source3/rpc_server/samr/srv_samr_nt.c
@@ -7681,7 +7681,6 @@ NTSTATUS _samr_ChangePasswordUser4(struct pipes_struct *p,
 	struct dcesrv_connection *dcesrv_conn = dce_call->conn;
 	const struct tsocket_address *remote_address =
 		dcesrv_connection_get_remote_address(dcesrv_conn);
-	enum samPwdChangeReason reject_reason;
 	char *rhost = NULL;
 	struct samu *sampass = NULL;
 	char *username = NULL;
@@ -7697,6 +7696,7 @@ NTSTATUS _samr_ChangePasswordUser4(struct pipes_struct *p,
 		.data = cdk_data,
 		.length = sizeof(cdk_data),
 	};
+	char *new_passwd = NULL;
 	NTSTATUS status = NT_STATUS_WRONG_PASSWORD;
 	bool ok;
 	int rc;
@@ -7766,19 +7766,31 @@ NTSTATUS _samr_ChangePasswordUser4(struct pipes_struct *p,
 	}
 
 	status = samr_set_password_aes(frame,
-				       sampass,
-				       rhost,
 				       &cdk,
 				       r->in.password,
-				       &reject_reason);
+				       &new_passwd);
 	BURN_DATA(cdk_data);
-	if (NT_STATUS_EQUAL(status, NT_STATUS_NO_SUCH_USER)) {
-		return NT_STATUS_WRONG_PASSWORD;
+	if (!NT_STATUS_IS_OK(status)) {
+		goto done;
 	}
 
+	become_root();
+	status = change_oem_password(sampass,
+				     rhost,
+				     NULL,
+				     new_passwd,
+				     true,
+				     NULL);
+	unbecome_root();
+	TALLOC_FREE(new_passwd);
+
 done:
 	TALLOC_FREE(frame);
 
+	if (NT_STATUS_EQUAL(status, NT_STATUS_NO_SUCH_USER)) {
+		return NT_STATUS_WRONG_PASSWORD;
+	}
+
 	return status;
 #else  /* HAVE_GNUTLS_PBKDF2 */
 	p->fault_state = DCERPC_FAULT_OP_RNG_ERROR;
diff --git a/source3/rpc_server/samr/srv_samr_util.h b/source3/rpc_server/samr/srv_samr_util.h
index a702d5d449c..5e839ac77c0 100644
--- a/source3/rpc_server/samr/srv_samr_util.h
+++ b/source3/rpc_server/samr/srv_samr_util.h
@@ -69,6 +69,10 @@ void copy_pwd_expired_to_sam_passwd(struct samu *to,
 
 bool chgpasswd(const char *name, const char *rhost, const struct passwd *pass,
 	       const char *oldpass, const char *newpass, bool as_root);
+NTSTATUS change_oem_password(struct samu *hnd, const char *rhost,
+			     char *old_passwd, char *new_passwd,
+			     bool as_root,
+			     enum samPwdChangeReason *samr_reject_reason);
 NTSTATUS pass_oem_change(char *user, const char *rhost,
 			 uchar password_encrypted_with_lm_hash[516],
 			 const uchar old_lm_hash_encrypted[16],
@@ -80,8 +84,6 @@ NTSTATUS check_password_complexity(const char *username,
 				   const char *password,
 				   enum samPwdChangeReason *samr_reject_reason);
 NTSTATUS samr_set_password_aes(TALLOC_CTX *mem_ctx,
-			       struct samu *sampass,
-			       const char *rhost,
 			       const DATA_BLOB *cdk,
 			       struct samr_EncryptedPasswordAES *pwbuf,
-			       enum samPwdChangeReason *reject_reason);
+			       char **new_password_str);