[PATCH] allow passdb backend to change trusted domain object password with clear text
Jeremy Allison
jra at samba.org
Tue Apr 18 19:06:20 UTC 2017
On Tue, Apr 18, 2017 at 06:39:49PM +0300, Alexander Bokovoy wrote:
> On ti, 18 huhti 2017, Alexander Bokovoy via samba-technical wrote:
> > On ti, 18 huhti 2017, Alexander Bokovoy via samba-technical wrote:
> > > On ti, 18 huhti 2017, Andrew Bartlett wrote:
> > > > On Tue, 2017-04-11 at 08:11 +0300, Alexander Bokovoy via samba-
> > > > technical wrote:
> > > > >
> > > > > Re-sent with updated comment.
> > > >
> > > > Reviewed-by: Andrew Bartlett <abartlet at samba.org>
> > > >
> > > > Please push.
> > > Pushed, thanks.
> > Failed, I'm checking what's wrong, found already some bugs in a
> > net_change_cred test...
> I forgot to decode the buffer, not just extract as it was before with NT
> Hash for netr_ServerPasswordSet2. Now all tests pass.
>
> I also fixed an error in samba3.blackbox.net_cred_change that caused
> shell error due to wrong shell syntax. This patch is attached too.
>
> Jeremy, could you please review it one more time and push?
RB+. Pushed. Thanks !
> From c022f83e8e6a333247e9dc7194de31071780c42b Mon Sep 17 00:00:00 2001
> From: Alexander Bokovoy <ab at samba.org>
> Date: Fri, 31 Mar 2017 12:44:58 +0300
> Subject: [PATCH 1/2] _netr_ServerPasswordSet2: use info level 26 to set plain
> text machine password
>
> To support password change for machine or trusted domain accounts in Active
> Directory environment we need to pass down actual plain text password
> instead of NT hashes. This would allow a backend like ipasam to update
> Kerberos keys as well as NT hashes.
>
> By calling samr_SetUserInfo2 info level 26 we ensure PASSDB layer can
> actually get the plain text password. If PASSDB backend implements
> pdb_update_sam_account() callback, it then gets the plain text password
> from samr_SetUserInfo2.
>
> A plain text password is a data blob represented as up to 256 WCHARs. It
> is UTF-16 coded on wire and we have its length from the buffer.
> SetUserInfo2 SAMR call chain in decode_pw_buffer() does explicitly
> expect 512+4 bytes in the buffer. It then calls convert_string_talloc()
> to convert it to UNIX charset passing the correct value of the plaintext
> password length. However, convert_string_talloc() expects the length of
> input string *including* the terminating null and we pass just the
> string length.
>
> convert_string_talloc() then explicitly null-terminates the resulting
> string by adding two nulls. In most cases UNIX charset is UTF-8, so we
> get null-terminated UTF-8 string down to PASSDB layer.
>
> MS-SAMR does not limit what does the password should contain. It says
> it is 'userPassword' value. Either 'userPassword' or 'unicodePwd' cannot
> contain null characters according to MS-ADTS 3.1.1.3.1.5 because they
> must be proper UTF-8 and UTF-16 strings accordingly.
>
> We are talking to our own SAMR service here.
>
> Signed-off-by: Alexander Bokovoy <ab at samba.org>
> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
> ---
> source3/rpc_server/netlogon/srv_netlog_nt.c | 79 +++++++++++++++++++++++------
> 1 file changed, 64 insertions(+), 15 deletions(-)
>
> diff --git a/source3/rpc_server/netlogon/srv_netlog_nt.c b/source3/rpc_server/netlogon/srv_netlog_nt.c
> index 6a42f34..c36e659 100644
> --- a/source3/rpc_server/netlogon/srv_netlog_nt.c
> +++ b/source3/rpc_server/netlogon/srv_netlog_nt.c
> @@ -33,6 +33,7 @@
> #include "../librpc/gen_ndr/ndr_lsa_c.h"
> #include "rpc_client/cli_lsarpc.h"
> #include "rpc_client/init_lsa.h"
> +#include "rpc_client/init_samr.h"
> #include "rpc_server/rpc_ncacn_np.h"
> #include "../libcli/security/security.h"
> #include "../libcli/security/dom_sid.h"
> @@ -1140,14 +1141,27 @@ static NTSTATUS netr_creds_server_step_check(struct pipes_struct *p,
> return status;
> }
>
> +
> /*************************************************************************
> *************************************************************************/
>
> +struct _samr_Credentials_t {
> + enum {
> + CRED_TYPE_NT_HASH,
> + CRED_TYPE_PLAIN_TEXT,
> + } cred_type;
> + union {
> + struct samr_Password *nt_hash;
> + const char *password;
> + } creds;
> +};
> +
> +
> static NTSTATUS netr_set_machine_account_password(TALLOC_CTX *mem_ctx,
> struct auth_session_info *session_info,
> struct messaging_context *msg_ctx,
> const char *account_name,
> - struct samr_Password *nt_hash)
> + struct _samr_Credentials_t *cr)
> {
> NTSTATUS status;
> NTSTATUS result = NT_STATUS_OK;
> @@ -1157,9 +1171,11 @@ static NTSTATUS netr_set_machine_account_password(TALLOC_CTX *mem_ctx,
> uint32_t acct_ctrl;
> union samr_UserInfo *info;
> struct samr_UserInfo18 info18;
> + struct samr_UserInfo26 info26;
> DATA_BLOB in,out;
> int rc;
> DATA_BLOB session_key;
> + enum samr_UserInfoLevel infolevel;
>
> ZERO_STRUCT(user_handle);
>
> @@ -1232,22 +1248,44 @@ static NTSTATUS netr_set_machine_account_password(TALLOC_CTX *mem_ctx,
> goto out;
> }
>
> - ZERO_STRUCT(info18);
> + switch(cr->cred_type) {
> + case CRED_TYPE_NT_HASH:
> + ZERO_STRUCT(info18);
> +
> + infolevel = UserInternal1Information;
> +
> + in = data_blob_const(cr->creds.nt_hash, 16);
> + out = data_blob_talloc_zero(mem_ctx, 16);
> + sess_crypt_blob(&out, &in, &session_key, true);
> + memcpy(info18.nt_pwd.hash, out.data, out.length);
> +
> + info18.nt_pwd_active = true;
> +
> + info->info18 = info18;
> + break;
> + case CRED_TYPE_PLAIN_TEXT:
> + ZERO_STRUCT(info26);
>
> - in = data_blob_const(nt_hash->hash, 16);
> - out = data_blob_talloc_zero(mem_ctx, 16);
> - sess_crypt_blob(&out, &in, &session_key, true);
> - memcpy(info18.nt_pwd.hash, out.data, out.length);
> + infolevel = UserInternal5InformationNew;
>
> - info18.nt_pwd_active = true;
> + init_samr_CryptPasswordEx(cr->creds.password,
> + &session_key,
> + &info26.password);
>
> - info->info18 = info18;
> + info26.password_expired = PASS_DONT_CHANGE_AT_NEXT_LOGON;
> + info->info26 = info26;
> + break;
> + default:
> + status = NT_STATUS_INTERNAL_ERROR;
> + goto out;
> + break;
> + }
>
> become_root();
> status = dcerpc_samr_SetUserInfo2(h,
> mem_ctx,
> &user_handle,
> - UserInternal1Information,
> + infolevel,
> info,
> &result);
> unbecome_root();
> @@ -1277,6 +1315,7 @@ NTSTATUS _netr_ServerPasswordSet(struct pipes_struct *p,
> NTSTATUS status = NT_STATUS_OK;
> int i;
> struct netlogon_creds_CredentialState *creds = NULL;
> + struct _samr_Credentials_t cr = { CRED_TYPE_NT_HASH, {0}};
>
> DEBUG(5,("_netr_ServerPasswordSet: %d\n", __LINE__));
>
> @@ -1311,11 +1350,12 @@ NTSTATUS _netr_ServerPasswordSet(struct pipes_struct *p,
> DEBUG(100,("%02X ", r->in.new_password->hash[i]));
> DEBUG(100,("\n"));
>
> + cr.creds.nt_hash = r->in.new_password;
> status = netr_set_machine_account_password(p->mem_ctx,
> p->session_info,
> p->msg_ctx,
> creds->account_name,
> - r->in.new_password);
> + &cr);
> return status;
> }
>
> @@ -1330,7 +1370,7 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
> struct netlogon_creds_CredentialState *creds = NULL;
> DATA_BLOB plaintext;
> struct samr_CryptPassword password_buf;
> - struct samr_Password nt_hash;
> + struct _samr_Credentials_t cr = { CRED_TYPE_PLAIN_TEXT, {0}};
>
> become_root();
> status = netr_creds_server_step_check(p, p->mem_ctx,
> @@ -1353,6 +1393,10 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
> return status;
> }
>
> + DEBUG(3,("_netr_ServerPasswordSet2: Server Password Seti2 by remote "
> + "machine:[%s] on account [%s]\n",
> + r->in.computer_name, creds->computer_name));
> +
> memcpy(password_buf.data, r->in.new_password->data, 512);
> SIVAL(password_buf.data, 512, r->in.new_password->length);
>
> @@ -1362,18 +1406,23 @@ NTSTATUS _netr_ServerPasswordSet2(struct pipes_struct *p,
> netlogon_creds_arcfour_crypt(creds, password_buf.data, 516);
> }
>
> - if (!extract_pw_from_buffer(p->mem_ctx, password_buf.data, &plaintext)) {
> + if (!decode_pw_buffer(p->mem_ctx,
> + password_buf.data,
> + (char**) &plaintext.data,
> + &plaintext.length,
> + CH_UTF16)) {
> + DEBUG(2,("_netr_ServerPasswordSet2: unable to extract password "
> + "from a buffer. Rejecting auth request as a wrong password\n"));
> TALLOC_FREE(creds);
> return NT_STATUS_WRONG_PASSWORD;
> }
>
> - mdfour(nt_hash.hash, plaintext.data, plaintext.length);
> -
> + cr.creds.password = (const char*) plaintext.data;
> status = netr_set_machine_account_password(p->mem_ctx,
> p->session_info,
> p->msg_ctx,
> creds->account_name,
> - &nt_hash);
> + &cr);
> TALLOC_FREE(creds);
> return status;
> }
> --
> 2.9.3
>
>
> From 5cdc7927e7cef473252ccf34b0055b4749c88cc3 Mon Sep 17 00:00:00 2001
> From: Alexander Bokovoy <ab at samba.org>
> Date: Tue, 18 Apr 2017 18:28:29 +0300
> Subject: [PATCH 2/2] s3-tests: assignement in shell shall have no spaces
> around equal sign
>
> When assigning value to 'failed', no spaces should be around '=' sign.
>
> Signed-off-by: Alexander Bokovoy <ab at samba.org>
> ---
> source3/script/tests/test_net_cred_change.sh | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/source3/script/tests/test_net_cred_change.sh b/source3/script/tests/test_net_cred_change.sh
> index 9013d07..de56be5 100755
> --- a/source3/script/tests/test_net_cred_change.sh
> +++ b/source3/script/tests/test_net_cred_change.sh
> @@ -9,8 +9,8 @@ fi
>
> incdir=`dirname $0`/../../../testprogs/blackbox
> . $incdir/subunit.sh
> -testit "first change" $VALGRIND $BINDIR/wbinfo -c || failed =`expr $failed + 1`
> -testit "first join" $VALGRIND $BINDIR/net rpc testjoin $@ || failed =`expr $failed + 1`
> -testit "second change" $VALGRIND $BINDIR/wbinfo -c || failed =`expr $failed + 1`
> +testit "first change" $VALGRIND $BINDIR/wbinfo -c || failed=`expr $failed + 1`
> +testit "first join" $VALGRIND $BINDIR/net rpc testjoin $@ || failed=`expr $failed + 1`
> +testit "second change" $VALGRIND $BINDIR/wbinfo -c || failed=`expr $failed + 1`
>
> testok $0 $failed
> --
> 2.9.3
>
More information about the samba-technical
mailing list