[PATCH] Fix 'smbpasswd' as local user in domain member case
Andreas Schneider
asn at samba.org
Tue Aug 22 14:04:37 UTC 2017
On Tuesday, 22 August 2017 13:43:43 CEST Andreas Schneider via samba-technical
wrote:
> 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.
I've solved it diffently now. If the remote machine is not an IP address, we
use it, else we use the domain part of the username if it has been specified.
Andreas
--
Andreas Schneider GPG-ID: CC014E3D
Samba Team asn at samba.org
www.samba.org
-------------- next part --------------
>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 1/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 2/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 3/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 6b67c89f1204fc0e44fc33cf772c5104cd800d38 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 4/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 | 37 ++++++++++++++++++++++++++++++++-----
1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/source3/utils/smbpasswd.c b/source3/utils/smbpasswd.c
index b8a8e9c71b6..6ae85b46f4f 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;
@@ -58,7 +59,7 @@ static void usage(void)
printf(" -c smb.conf file Use the given path to the smb.conf file\n");
printf(" -D LEVEL debug level\n");
printf(" -r MACHINE remote machine\n");
- printf(" -U USER remote username\n");
+ printf(" -U USER remote username (e.g. SAM/user)\n");
printf("extra options when run by root or in local mode:\n");
printf(" -a add user\n");
@@ -95,7 +96,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) {
@@ -519,6 +520,9 @@ static int process_nonroot(int local_flags)
int result = 0;
char *old_pw = NULL;
char *new_pw = NULL;
+ const char *username = user_name;
+ const char *domain = NULL;
+ char *p = NULL;
if (local_flags & ~(LOCAL_AM_ROOT | LOCAL_SET_PASSWORD)) {
/* Extra flags that we can't honor non-root */
@@ -536,6 +540,15 @@ static int process_nonroot(int local_flags)
}
}
+ /* Allow domain as part of the username */
+ if ((p = strchr_m(user_name, '\\')) ||
+ (p = strchr_m(user_name, '/')) ||
+ (p = strchr_m(user_name, *lp_winbind_separator()))) {
+ *p = '\0';
+ username = p + 1;
+ domain = user_name;
+ }
+
/*
* A non-root user is always setting a password
* via a remote machine (even if that machine is
@@ -544,8 +557,18 @@ static int process_nonroot(int local_flags)
load_interfaces(); /* Delayed from main() */
- if (remote_machine == NULL) {
+ if (remote_machine != NULL) {
+ if (!is_ipaddress(remote_machine)) {
+ domain = remote_machine;
+ }
+ } else {
remote_machine = "127.0.0.1";
+
+ /*
+ * If we deal with a local user, change the password for the
+ * user in our SAM.
+ */
+ domain = get_global_sam_name();
}
if (remote_machine != NULL) {
@@ -567,14 +590,18 @@ 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, username,
old_pw, new_pw, 0))) {
result = 1;
goto done;
}
- printf("Password changed for user %s\n", user_name);
+ printf("Password changed for user %s\n", username);
done:
SAFE_FREE(old_pw);
--
2.14.0
>From ea44a26c44aab78f3da34f55ae7f20167238ef79 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 22 Aug 2017 15:46:07 +0200
Subject: [PATCH 5/6] s3:utils: Remove pointless if-clause for remote_machine
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12975
Review with: git show -U20
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source3/utils/smbpasswd.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/source3/utils/smbpasswd.c b/source3/utils/smbpasswd.c
index 6ae85b46f4f..b7d9bc5785c 100644
--- a/source3/utils/smbpasswd.c
+++ b/source3/utils/smbpasswd.c
@@ -571,12 +571,10 @@ static int process_nonroot(int local_flags)
domain = get_global_sam_name();
}
- if (remote_machine != NULL) {
- old_pw = get_pass("Old SMB password:",stdin_passwd_get);
- if (old_pw == NULL) {
- fprintf(stderr, "Unable to get old password.\n");
- exit(1);
- }
+ old_pw = get_pass("Old SMB password:",stdin_passwd_get);
+ if (old_pw == NULL) {
+ fprintf(stderr, "Unable to get old password.\n");
+ exit(1);
}
if (!new_passwd) {
--
2.14.0
>From 2c5dc5f56414adfd402202956ff2ba6c1df2dfc6 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..e5aa95d51f9
--- /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 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