[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Fri Jan 25 15:32:04 UTC 2019


The branch, master has been updated
       via  e37f9956c1f smbd: uid: Don't crash if 'force group' is added to an existing share connection.
       via  7b21b4c1f53 s3: tests: Add regression test for smbd crash on share force group change with existing connection.
       via  e903d37ea4a s3:rpclient: rpclient help is not very helpful
      from  67fc683a3ee CI: move target "build_nt4" to private gitlab runners

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit e37f9956c1f2416408bad048a4618f6366086b6a
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Jan 18 14:24:30 2019 -0800

    smbd: uid: Don't crash if 'force group' is added to an existing share connection.
    
    smbd could crash if "force group" is added to a
    share definition whilst an existing connection
    to that share exists. In that case, don't change
    the existing credentials for force group, only
    do so for new connections.
    
    Remove knownfail from regression test.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13690
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Fri Jan 25 16:31:27 CET 2019 on sn-devel-144

commit 7b21b4c1f538650f23ec77fb3c02fe1e224d89aa
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Jan 24 10:15:56 2019 -0800

    s3: tests: Add regression test for smbd crash on share force group change with existing connection.
    
    Mark as known fail for now.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13690
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit e903d37ea4a8372f819f8bdbec7dfeef1799f612
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Wed Jan 23 11:07:42 2019 +1300

    s3:rpclient: rpclient help is not very helpful
    
    The help was not telling me that there was a mandatory 'server' argument
    that I needed to specify. After trying several different combinations
    of parameters, I eventually had to run the tool in gdb to work out why
    it was complaining.
    
    This is the output I was getting:
    
    bin/rpcclient -U$USERNAME%$PASSWORD -I $SERVER_IP
    Usage: rpcclient [OPTION...]
      -c, --command=COMMANDS                 Execute semicolon separated
    cmds
      -I, --dest-ip=IP                       Specify destination IP address
      -p, --port=PORT                        Specify port number
    ...
    
    New help output is:
    
    Usage: rpcclient [OPTION...] <server>
    Options:
      -c, --command=COMMANDS                 Execute semicolon separated
    cmds
    ...
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Ralph Boehme <slow at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 selftest/selftesthelpers.py                     |  1 +
 selftest/target/Samba3.pm                       |  5 ++
 source3/rpcclient/rpcclient.c                   |  2 +
 source3/script/tests/test_force_group_change.sh | 73 +++++++++++++++++++++++++
 source3/selftest/tests.py                       |  4 ++
 source3/smbd/uid.c                              | 35 +++++++++++-
 6 files changed, 118 insertions(+), 2 deletions(-)
 create mode 100755 source3/script/tests/test_force_group_change.sh


Changeset truncated at 500 lines:

diff --git a/selftest/selftesthelpers.py b/selftest/selftesthelpers.py
index ebdae12866a..acce6d24cce 100644
--- a/selftest/selftesthelpers.py
+++ b/selftest/selftesthelpers.py
@@ -207,3 +207,4 @@ smbcquotas = binpath('smbcquotas')
 smbget = binpath('smbget')
 rpcclient = binpath('rpcclient')
 smbcacls = binpath('smbcacls')
+smbcontrol = binpath('smbcontrol')
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 49bdd2ac885..f11bb9312df 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -984,6 +984,11 @@ sub setup_fileserver
 	comment = inherit only unix owner
 	inherit owner = unix only
 	acl_xattr:ignore system acls = yes
+# BUG: https://bugzilla.samba.org/show_bug.cgi?id=13690
+[force_group_test]
+	path = $share_dir
+	comment = force group test
+#	force group = everyone
 [homes]
 	comment = Home directories
 	browseable = No
diff --git a/source3/rpcclient/rpcclient.c b/source3/rpcclient/rpcclient.c
index 9f95f1a7a8c..6cfacb19cfe 100644
--- a/source3/rpcclient/rpcclient.c
+++ b/source3/rpcclient/rpcclient.c
@@ -982,6 +982,8 @@ out_free:
 	pc = poptGetContext("rpcclient", argc, const_argv,
 			    long_options, 0);
 
+	poptSetOtherOptionHelp(pc, "[OPTION...] <server>\nOptions:");
+
 	if (argc == 1) {
 		poptPrintHelp(pc, stderr, 0);
 		goto done;
diff --git a/source3/script/tests/test_force_group_change.sh b/source3/script/tests/test_force_group_change.sh
new file mode 100755
index 00000000000..6cb1ab4e048
--- /dev/null
+++ b/source3/script/tests/test_force_group_change.sh
@@ -0,0 +1,73 @@
+#!/bin/sh
+
+# Copyright (c) Jeremy Allison <jra at samba.org>
+# License: GPLv3
+# Regression test for BUG:https://bugzilla.samba.org/show_bug.cgi?id=13690
+
+if [ $# -lt 6 ]; then
+	echo "Usage: test_force_group_change.sh SERVER USERNAME PASSWORD LOCAL_PATH SMBCLIENT SMBCONTROL"
+	exit 1
+fi
+
+SERVER="${1}"
+USERNAME="${2}"
+PASSWORD="${3}"
+LOCAL_PATH="${4}"
+SMBCLIENT="${5}"
+SMBCONTROL="${6}"
+shift 6
+
+incdir=`dirname $0`/../../../testprogs/blackbox
+. $incdir/subunit.sh
+
+failed=0
+
+test_force_group_change()
+{
+#
+# A SMB_CONF variable passed in here is the client smb.conf.
+# We need to convert to the server.conf file from
+# the LOCAL_PATH variable.
+#
+SERVER_CONFIG=`dirname $LOCAL_PATH`/lib/server.conf
+SERVER_CONFIG_SAVE=${SERVER_CONFIG}.bak
+SERVER_CONFIG_NEW=${SERVER_CONFIG}.new
+cp $SERVER_CONFIG $SERVER_CONFIG_SAVE
+
+sed -e 's/#\tforce group = everyone/\tforce group = everyone/' <${SERVER_CONFIG} >${SERVER_CONFIG_NEW}
+
+    tmpfile=$PREFIX/smbclient_force_group_change_commands
+    cat > $tmpfile <<EOF
+ls
+!cp ${SERVER_CONFIG_NEW} ${SERVER_CONFIG}
+!${SMBCONTROL} --configfile=${SERVER_CONFIG} all reload-config
+ls
+!cp ${SERVER_CONFIG_SAVE} ${SERVER_CONFIG}
+!${SMBCONTROL} --configfile=${SERVER_CONFIG} all reload-config
+quit
+EOF
+
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/force_group_test $CONFIGURATION < $tmpfile 2>&1'
+    eval echo "$cmd"
+    out=$(eval $cmd)
+    ret=$?
+    rm -f $tmpfile
+    rm -f $SERVER_CONFIG_SAVE
+    rm -f $SERVER_CONFIG_NEW
+
+    echo "$out" | grep 'NT_STATUS_CONNECTION_DISCONNECTED'
+    ret=$?
+    if [ $ret -eq 0 ] ; then
+       # Client was disconnected as server crashed.
+       echo "$out"
+       return 1
+    fi
+
+    return 0
+}
+
+testit "test force group change" \
+    test_force_group_change || \
+    failed=`expr $failed + 1`
+
+testok $0 $failed
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 46f078759e1..30a93a2ee42 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -29,6 +29,7 @@ from selftesthelpers import net, wbinfo, dbwrap_tool, rpcclient, python
 from selftesthelpers import smbget, smbcacls, smbcquotas, ntlm_auth3
 from selftesthelpers import valgrindify, smbtorture4_testsuites
 from selftesthelpers import smbtorture4_options
+from selftesthelpers import smbcontrol
 smbtorture4_options.extend([
     '--option=torture:sharedelay=100000',
    '--option=torture:writetimeupdatedelay=500000',
@@ -327,6 +328,9 @@ for env in ["fileserver"]:
     plantestsuite("samba3.blackbox.large_acl.SMB3", env, [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls, '-m', 'SMB3'])
     plantestsuite("samba3.blackbox.give_owner", env, [os.path.join(samba3srcdir, "script/tests/test_give_owner.sh"), '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, net, 'tmp'])
     plantestsuite("samba3.blackbox.homes", env, [os.path.join(samba3srcdir, "script/tests/test_homes.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', '$PREFIX', smbclient3, configuration])
+    plantestsuite("samba3.blackbox.force_group_change", env,
+		[os.path.join(samba3srcdir, "script/tests/test_force_group_change.sh"),
+		'$SERVER', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', smbclient3, smbcontrol])
 
     #
     # tar command tests
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 7aecea5f857..a4bcb747d37 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -291,6 +291,7 @@ static bool change_to_user_internal(connection_struct *conn,
 	int snum;
 	gid_t gid;
 	uid_t uid;
+	const char *force_group_name;
 	char group_c;
 	int num_groups = 0;
 	gid_t *group_list = NULL;
@@ -330,9 +331,39 @@ static bool change_to_user_internal(connection_struct *conn,
 	 * See if we should force group for this service. If so this overrides
 	 * any group set in the force user code.
 	 */
-	if((group_c = *lp_force_group(talloc_tos(), snum))) {
+	force_group_name = lp_force_group(talloc_tos(), snum);
+	group_c = *force_group_name;
 
-		SMB_ASSERT(conn->force_group_gid != (gid_t)-1);
+	if ((group_c != '\0') && (conn->force_group_gid == (gid_t)-1)) {
+		/*
+		 * This can happen if "force group" is added to a
+		 * share definition whilst an existing connection
+		 * to that share exists. In that case, don't change
+		 * the existing credentials for force group, only
+		 * do so for new connections.
+		 *
+		 * BUG: https://bugzilla.samba.org/show_bug.cgi?id=13690
+		 */
+		DBG_INFO("Not forcing group %s on existing connection to "
+			"share %s for SMB user %s (unix user %s)\n",
+			force_group_name,
+			lp_const_servicename(snum),
+			session_info->unix_info->sanitized_username,
+			session_info->unix_info->unix_name);
+	}
+
+	if((group_c != '\0') && (conn->force_group_gid != (gid_t)-1)) {
+		/*
+		 * Only force group for connections where
+		 * conn->force_group_gid has already been set
+		 * to the correct value (i.e. the connection
+		 * happened after the 'force group' definition
+		 * was added to the share definition. Connections
+		 * that were made before force group was added
+		 * should stay with their existing credentials.
+		 *
+		 * BUG: https://bugzilla.samba.org/show_bug.cgi?id=13690
+		 */
 
 		if (group_c == '+') {
 			int i;


-- 
Samba Shared Repository



More information about the samba-cvs mailing list