[PATCH] Fix 'smbpasswd' as local user in domain member case

Andreas Schneider asn at samba.org
Tue Aug 22 11:43:43 UTC 2017


On Tuesday, 22 August 2017 12:33:39 CEST Andrew Bartlett via samba-technical 
wrote:
> On Tue, 2017-08-22 at 08:22 +0200, Andreas Schneider wrote:
> > On Monday, 21 August 2017 21:21:27 CEST Andrew Bartlett wrote:
> > > On Mon, 2017-08-21 at 16:11 +0200, Andreas Schneider wrote:
> > > > On Friday, 18 August 2017 21:20:21 CEST Andrew Bartlett wrote:
> > > > > On Fri, 2017-08-18 at 16:39 +0200, Andreas Schneider via samba-
> > > > > 
> > > > > technical wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > the attached patch fixes calling 'smbpasswd' as a local user if
> > > > > > the
> > > > > > machine is a domain member.
> > > > > > 
> > > > > > Before we authenticated with the workgroup as the domain name, so
> > > > > > we
> > > > > > contacted winbind instead of our SAM.
> > > > > > 
> > > > > > 
> > > > > > Review and push apprecaited!
> > > > > 
> > > > > The patch looks good, but can we get a test so we don't regress on
> > > > > this
> > > > > again?
> > > > 
> > > > It took me quite a few hours to develop the test for this, but thanks
> > > > to
> > > > the new uid_wrapper features, we can do such things now :-)
> > > 
> > > Nice!
> > > 
> > > I had expected you would just use the -r option, but this certainly
> > > tests the entire problem!
> > > 
> > > Does it still work correctly on a DC, given you force the domain to the
> > > netbios name?  I'm assuming so, I think you can log in with the netbios
> > > name to domain account, but can we just run the test additionally in
> > > nt4_dc to be sure (reworking the test to run on the AD DC is more work
> > > than I want to ask).
> > 
> > See attached.
> 
> I'm not convinced this is in the right spot.  Doesn't the 
> 
> domain = lp_netbios_name()
> 
> bit need to be in the
> 
> if (remote_machine == NULL) {
> 
> chunk?

Yes, that's probably better, but then we cannot write a test for it. 127.0.0.1 
is not avaiable in a cwrap environment.

> 
> Otherwise don't we end up in the same muddle for 
> 
> smbpasswd -r $OTHERHOST?

I think we need an option to specify the domain name then.


See attached


-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org
-------------- next part --------------
>From 6eb41e9fd5c133da9fe6565cefc9908cb2bbb39e Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 18 Aug 2017 16:08:46 +0200
Subject: [PATCH 1/6] s3:libsmb: Pass domain to remote_password_change()

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

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/include/proto.h     | 3 ++-
 source3/libsmb/passchange.c | 5 +++--
 source3/utils/smbpasswd.c   | 3 ++-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index c8f6c282b68..976f3f92156 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -823,7 +823,8 @@ bool get_dc_name(const char *domain,
 
 /* The following definitions come from libsmb/passchange.c  */
 
-NTSTATUS remote_password_change(const char *remote_machine, const char *user_name, 
+NTSTATUS remote_password_change(const char *remote_machine,
+				const char *domain, const char *user_name,
 				const char *old_passwd, const char *new_passwd,
 				char **err_str);
 
diff --git a/source3/libsmb/passchange.c b/source3/libsmb/passchange.c
index c89b7ca85d1..48ffba8036f 100644
--- a/source3/libsmb/passchange.c
+++ b/source3/libsmb/passchange.c
@@ -30,7 +30,8 @@
  Change a password on a remote machine using IPC calls.
 *************************************************************/
 
-NTSTATUS remote_password_change(const char *remote_machine, const char *user_name, 
+NTSTATUS remote_password_change(const char *remote_machine,
+				const char *domain, const char *user_name,
 				const char *old_passwd, const char *new_passwd,
 				char **err_str)
 {
@@ -55,7 +56,7 @@ NTSTATUS remote_password_change(const char *remote_machine, const char *user_nam
 
 	creds = cli_session_creds_init(cli,
 				       user_name,
-				       NULL, /* domain */
+				       domain,
 				       NULL, /* realm */
 				       old_passwd,
 				       false, /* use_kerberos */
diff --git a/source3/utils/smbpasswd.c b/source3/utils/smbpasswd.c
index ae9862606f1..00d2acf2a5f 100644
--- a/source3/utils/smbpasswd.c
+++ b/source3/utils/smbpasswd.c
@@ -258,7 +258,8 @@ static NTSTATUS password_change(const char *remote_mach, char *username,
 			fprintf(stderr, "Invalid remote operation!\n");
 			return NT_STATUS_UNSUCCESSFUL;
 		}
-		ret = remote_password_change(remote_mach, username,
+		ret = remote_password_change(remote_mach,
+					     NULL, username,
 					     old_passwd, new_pw, &err_str);
 	} else {
 		ret = local_password_change(username, local_flags, new_pw,
-- 
2.14.0


>From 9cd8090d688e1fffb758b084face8c10610d0424 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 18 Aug 2017 16:10:06 +0200
Subject: [PATCH 2/6] s3:libsmb: Move prototye of remote_password_change()

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

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/include/proto.h   |  7 -------
 source3/libsmb/proto.h    | 10 ++++++++++
 source3/utils/smbpasswd.c |  1 +
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 976f3f92156..b2c3a03a193 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -821,13 +821,6 @@ bool get_dc_name(const char *domain,
 		fstring srv_name,
 		struct sockaddr_storage *ss_out);
 
-/* The following definitions come from libsmb/passchange.c  */
-
-NTSTATUS remote_password_change(const char *remote_machine,
-				const char *domain, const char *user_name,
-				const char *old_passwd, const char *new_passwd,
-				char **err_str);
-
 /* The following definitions come from libsmb/smberr.c  */
 
 const char *smb_dos_err_name(uint8_t e_class, uint16_t num);
diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index 05d91f79f83..a74433f623e 100644
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -30,6 +30,9 @@
 
 struct smb_trans_enc_state;
 struct cli_credentials;
+struct cli_state;
+struct file_info;
+struct print_job_info;
 
 /* The following definitions come from libsmb/cliconnect.c  */
 
@@ -970,4 +973,11 @@ NTSTATUS cli_readlink(struct cli_state *cli, const char *fname,
 		       TALLOC_CTX *mem_ctx, char **psubstitute_name,
 		      char **pprint_name, uint32_t *pflags);
 
+/* The following definitions come from libsmb/passchange.c  */
+
+NTSTATUS remote_password_change(const char *remote_machine,
+				const char *domain, const char *user_name,
+				const char *old_passwd, const char *new_passwd,
+				char **err_str);
+
 #endif /* _LIBSMB_PROTO_H_ */
diff --git a/source3/utils/smbpasswd.c b/source3/utils/smbpasswd.c
index 00d2acf2a5f..ec162fdbfb0 100644
--- a/source3/utils/smbpasswd.c
+++ b/source3/utils/smbpasswd.c
@@ -21,6 +21,7 @@
 #include "secrets.h"
 #include "../librpc/gen_ndr/samr.h"
 #include "../lib/util/util_pw.h"
+#include "libsmb/proto.h"
 #include "passdb.h"
 
 /*
-- 
2.14.0


>From e9eebc652e0930fea3254224718effae08f1b489 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 18 Aug 2017 16:13:15 +0200
Subject: [PATCH 3/6] s3:utils: Make strings const passed to password_change()
 in smbpasswd

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

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/utils/smbpasswd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/source3/utils/smbpasswd.c b/source3/utils/smbpasswd.c
index ec162fdbfb0..5c75f48189d 100644
--- a/source3/utils/smbpasswd.c
+++ b/source3/utils/smbpasswd.c
@@ -243,8 +243,9 @@ static char *prompt_for_new_password(bool stdin_get)
  Change a password either locally or remotely.
 *************************************************************/
 
-static NTSTATUS password_change(const char *remote_mach, char *username, 
-				char *old_passwd, char *new_pw,
+static NTSTATUS password_change(const char *remote_mach,
+				const char *username,
+				const char *old_passwd, const char *new_pw,
 				int local_flags)
 {
 	NTSTATUS ret;
-- 
2.14.0


>From bfb0542d22f162e4a59588bf1106fe3aa1fc63c4 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 18 Aug 2017 16:14:57 +0200
Subject: [PATCH 4/6] s3:utils: Pass domain to password_change() in smbpasswd

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

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/utils/smbpasswd.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/source3/utils/smbpasswd.c b/source3/utils/smbpasswd.c
index 5c75f48189d..b8a8e9c71b6 100644
--- a/source3/utils/smbpasswd.c
+++ b/source3/utils/smbpasswd.c
@@ -244,7 +244,7 @@ static char *prompt_for_new_password(bool stdin_get)
 *************************************************************/
 
 static NTSTATUS password_change(const char *remote_mach,
-				const char *username,
+				const char *domain, const char *username,
 				const char *old_passwd, const char *new_pw,
 				int local_flags)
 {
@@ -261,7 +261,7 @@ static NTSTATUS password_change(const char *remote_mach,
 			return NT_STATUS_UNSUCCESSFUL;
 		}
 		ret = remote_password_change(remote_mach,
-					     NULL, username,
+					     domain, username,
 					     old_passwd, new_pw, &err_str);
 	} else {
 		ret = local_password_change(username, local_flags, new_pw,
@@ -466,7 +466,8 @@ static int process_root(int local_flags)
 		}
 	}
 
-	if (!NT_STATUS_IS_OK(password_change(remote_machine, user_name,
+	if (!NT_STATUS_IS_OK(password_change(remote_machine,
+					     NULL, user_name,
 					     old_passwd, new_passwd,
 					     local_flags))) {
 		result = 1;
@@ -566,8 +567,9 @@ static int process_nonroot(int local_flags)
 		exit(1);
 	}
 
-	if (!NT_STATUS_IS_OK(password_change(remote_machine, user_name, old_pw,
-					     new_pw, 0))) {
+	if (!NT_STATUS_IS_OK(password_change(remote_machine,
+					     NULL, user_name,
+					     old_pw, new_pw, 0))) {
 		result = 1;
 		goto done;
 	}
-- 
2.14.0


>From cce783925e9cedbe47e9db9db00f812a23c6df30 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 18 Aug 2017 16:17:08 +0200
Subject: [PATCH 5/6] s3:utils: Make sure we authenticate against our SAM name
 in smbpasswd

If a local user wants to change his password using smbpasswd and the
machine is a domain member, we need to make sure we authenticate against
our SAM and not ask winbind.

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

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/utils/smbpasswd.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/source3/utils/smbpasswd.c b/source3/utils/smbpasswd.c
index b8a8e9c71b6..8c54b023917 100644
--- a/source3/utils/smbpasswd.c
+++ b/source3/utils/smbpasswd.c
@@ -35,6 +35,7 @@ extern int optind;
 static bool got_username = False;
 static bool stdin_passwd_get = False;
 static fstring user_name;
+static fstring domain_name;
 static char *new_passwd = NULL;
 static const char *remote_machine = NULL;
 
@@ -59,6 +60,7 @@ static void usage(void)
 	printf("  -D LEVEL             debug level\n");
 	printf("  -r MACHINE           remote machine\n");
 	printf("  -U USER              remote username\n");
+	printf("  -S SAMNAME           remote hostname (SAM name for unix users)\n");
 
 	printf("extra options when run by root or in local mode:\n");
 	printf("  -a                   add user\n");
@@ -95,7 +97,7 @@ static int process_options(int argc, char **argv, int local_flags)
 
 	user_name[0] = '\0';
 
-	while ((ch = getopt(argc, argv, "c:axdehminjr:sw:R:D:U:LW")) != EOF) {
+	while ((ch = getopt(argc, argv, "c:axdehminjr:sw:R:D:U:LWS:")) != EOF) {
 		switch(ch) {
 		case 'L':
 			if (getuid() != 0) {
@@ -104,6 +106,9 @@ static int process_options(int argc, char **argv, int local_flags)
 			}
 			local_flags |= LOCAL_AM_ROOT;
 			break;
+		case 'S':
+			fstrcpy(domain_name, optarg);
+			break;
 		case 'c':
 			configfile = optarg;
 			set_dyn_CONFIGFILE(optarg);
@@ -519,6 +524,7 @@ static int process_nonroot(int local_flags)
 	int result = 0;
 	char *old_pw = NULL;
 	char *new_pw = NULL;
+	const char *domain = NULL;
 
 	if (local_flags & ~(LOCAL_AM_ROOT | LOCAL_SET_PASSWORD)) {
 		/* Extra flags that we can't honor non-root */
@@ -546,6 +552,12 @@ static int process_nonroot(int local_flags)
 
 	if (remote_machine == NULL) {
 		remote_machine = "127.0.0.1";
+
+		/*
+		 * If we deal with a local user, change the password for the
+		 * user in our SAM.
+		 */
+		domain = lp_netbios_name();
 	}
 
 	if (remote_machine != NULL) {
@@ -567,8 +579,12 @@ static int process_nonroot(int local_flags)
 		exit(1);
 	}
 
+	if (domain_name[0] != '\0') {
+		domain = domain_name;
+	}
+
 	if (!NT_STATUS_IS_OK(password_change(remote_machine,
-					     NULL, user_name,
+					     domain, user_name,
 					     old_pw, new_pw, 0))) {
 		result = 1;
 		goto done;
-- 
2.14.0


>From 41fd0e214a49673cd8cf804decd46eb50b0f37cc Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Mon, 21 Aug 2017 12:23:56 +0200
Subject: [PATCH 6/6] s3:tests: Add test for changing the local user password
 with smbpasswd

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

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/script/tests/test_smbpasswd.sh | 132 +++++++++++++++++++++++++++++++++
 source3/selftest/tests.py              |   3 +
 2 files changed, 135 insertions(+)
 create mode 100755 source3/script/tests/test_smbpasswd.sh

diff --git a/source3/script/tests/test_smbpasswd.sh b/source3/script/tests/test_smbpasswd.sh
new file mode 100755
index 00000000000..a607ebb2f76
--- /dev/null
+++ b/source3/script/tests/test_smbpasswd.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+#
+# Blackbox tests for smbpasswd
+#
+# Copyright (c) 2015-2016 Andreas Schneider <asn at samba.org>
+#
+
+if [ $# -lt 4 ]; then
+cat <<EOF
+Usage: test_smbpasswd.sh SERVER USERNAME PASSWORD
+EOF
+exit 1;
+fi
+
+SERVER=$1
+SERVER_IP=$2
+USERNAME=$3
+PASSWORD=$4
+shift 4
+
+incdir=$(dirname $0)/../../../testprogs/blackbox
+source $incdir/subunit.sh
+
+failed=0
+
+samba_bindir="$BINDIR"
+samba_srcdir="$SRCDIR"
+
+samba_texpect="$samba_bindir/texpect"
+samba_smbpasswd="$samba_bindir/smbpasswd"
+
+samba_test_user="alice_smbpasswd"
+samba_test_user_pwd="Secret007"
+samba_test_user_new_pwd="Secret008"
+
+create_local_smb_user()
+{
+	user=$1
+	password=$2
+
+	tmpfile=$PREFIX/smbpasswd_create_user_script
+	cat > $tmpfile <<EOF
+expect New SMB password:
+send $password\n
+expect Retype new SMB password:
+send $password\n
+EOF
+
+	cmd='UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 $samba_texpect $tmpfile $samba_smbpasswd -c $SMB_CONF_PATH -a $user 2>&1'
+	eval echo "$cmd"
+	out=$(eval $cmd)
+	ret=$?
+
+	rm -f $tmpfile
+
+	if [ $ret -ne 0 ]; then
+		echo "Failed to create smb user $user"
+		return 1
+	fi
+
+	getent passwd $user
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "Failed to create smb user $user"
+		return 1
+	fi
+}
+
+delete_local_smb_user()
+{
+	user=$1
+
+	# This also deletes the unix account!
+	UID_WRAPPER_INITIAL_RUID=0 UID_WRAPPER_INITIAL_EUID=0 $samba_smbpasswd -c $SMB_CONF_PATH -x $user
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "Failed to delete smb user $user"
+		return 1
+	fi
+}
+
+test_smbpasswd()
+{
+	user=$1
+	oldpwd=$2
+	newpwd=$3
+
+	user_id=$(id -u $user)
+
+	tmpfile=$PREFIX/smbpasswd_change_password_script
+	cat > $tmpfile <<EOF
+expect Old SMB password:
+send $oldpwd\n
+expect New SMB password:
+send $newpwd\n
+expect Retype new SMB password:
+send $newpwd\n
+EOF
+
+	cmd='UID_WRAPPER_INITIAL_RUID=$user_id UID_WRAPPER_INITIAL_EUID=$user_id $samba_texpect $tmpfile $samba_smbpasswd -c $SMB_CONF_PATH -r $SERVER_IP -S $SERVER 2>&1'
+	eval echo "$cmd"
+	out=$(eval $cmd)
+	ret=$?
+	rm -f $tmpfile
+	if [ $ret -ne 0 ]; then
+		echo "Failed to change user password $user"
+		return 1
+	fi
+
+	prompt="Password changed for user $user"
+	echo "$out" | grep "$prompt" >/dev/null 2>&1
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		echo "Failed to change password for user $user"
+		echo "$out"
+		return 1
+	fi
+}
+
+testit "Create user $samba_test_user" \
+	create_local_smb_user $samba_test_user $samba_test_user_pwd \
+	|| failed=$(expr $failed + 1)
+
+testit "Change user password" \
+	test_smbpasswd $samba_test_user $samba_test_user_pwd $samba_test_user_new_pwd \
+	|| failed=$(expr $failed + 1)
+
+testit "Delete user $samba_test_user" \
+	delete_local_smb_user $samba_test_user \
+	|| failed=$(expr $failed + 1)
+
+exit $failed
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index a624f85b8f6..7e1f2135044 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -188,6 +188,9 @@ for options in ["--option=clientntlmv2auth=no", "--option=clientusespnego=no --o
 env="ad_dc"
 plantestsuite("samba3.blackbox.smbspool", env, [os.path.join(samba3srcdir, "script/tests/test_smbspool.sh"), '$SERVER', '$SERVER_IP', '$DC_USERNAME', '$DC_PASSWORD', env])
 
+for env in ["ad_member:local", "nt4_dc:local"]:
+    plantestsuite("samba3.blackbox.smbpasswd", env, [os.path.join(samba3srcdir, "script/tests/test_smbpasswd.sh"), '$SERVER', '$SERVER_IP', '$DC_USERNAME', '$DC_PASSWORD'])
+
 env="nt4_dc"
 plantestsuite("samba3.blackbox.smbclient_auth.plain (%s) ipv6" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IPV6', '$SERVER/$USERNAME', '$PASSWORD', smbclient3, configuration])
 
-- 
2.14.0



More information about the samba-technical mailing list