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

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>

diff --git a/source3/rpc_server/samr/srv_samr_chgpasswd.c b/source3/rpc_server/samr/srv_samr_chgpasswd.c
index d720641f05c089063c6e839428b108237090bb30..8563beb48a7248dd4f6d6d1201380261401b7809 100644 (file)
--- 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 14bff5ecd7a7916379530df52c55ae2c12f24215..1036410f534c6d6d7841cedeff91f55bd0ab3560 100644 (file)
--- 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 a702d5d449c752038438117459cd3813a9c5feec..5e839ac77c011a8735a00fb245ada5ac366356e7 100644 (file)
--- 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);