Attachment 14802 Details for Bug 13690 – git-am fix for 4.10.rcNext - cherry-pick from master

[patch] git-am fix for 4.10.rcNext - cherry-pick from master

bug-13690-4.10.patch (text/plain), 9.56 KB, created by Jeremy Allison on 2019-01-25 17:23:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2019-01-25 17:23:13 UTC

Size: 9.56 KB

patch

obsolete

>From 58212b72715510c87b906ce906f0cf8ccb5044e8 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Thu, 24 Jan 2019 10:15:56 -0800
>Subject: [PATCH 1/2] 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@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 7b21b4c1f538650f23ec77fb3c02fe1e224d89aa)
>---
> selftest/knownfail                            |  2 +
> selftest/selftesthelpers.py                   |  1 +
> selftest/target/Samba3.pm                     |  5 ++
> .../script/tests/test_force_group_change.sh   | 73 +++++++++++++++++++
> source3/selftest/tests.py                     |  4 +
> 5 files changed, 85 insertions(+)
> create mode 100755 source3/script/tests/test_force_group_change.sh
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index abbbd889c71..9678883924e 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -363,3 +363,5 @@
> ^samba.tests.ntlmdisabled.python$ktest$.python2.ntlmdisabled.NtlmDisabledTests.test_samr_change_password$ktest$
> ^samba.tests.ntlmdisabled.python$ad_dc_no_ntlm$.python3.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection$ad_dc_no_ntlm$
> ^samba.tests.ntlmdisabled.python$ad_dc_no_ntlm$.python2.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection$ad_dc_no_ntlm$
>+# BUG:https://bugzilla.samba.org/show_bug.cgi?id=13690
>+^samba3.blackbox.force_group_change.*
>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/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@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
>-- 
>2.20.1.495.gaa96b0ce6b-goog
>
>
>From d368697393c9cf47935f4770b2477c7fa9d6ad1b Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Fri, 18 Jan 2019 14:24:30 -0800
>Subject: [PATCH 2/2] smbd: uid: Don't crash if 'force group' is added to an
> existing share connection.
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>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@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>
>Autobuild-User(master): Ralph BÃ¶hme <slow@samba.org>
>Autobuild-Date(master): Fri Jan 25 16:31:27 CET 2019 on sn-devel-144
>
>(cherry picked from commit e37f9956c1f2416408bad048a4618f6366086b6a)
>---
> selftest/knownfail |  2 --
> source3/smbd/uid.c | 35 +++++++++++++++++++++++++++++++++--
> 2 files changed, 33 insertions(+), 4 deletions(-)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 9678883924e..abbbd889c71 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -363,5 +363,3 @@
> ^samba.tests.ntlmdisabled.python$ktest$.python2.ntlmdisabled.NtlmDisabledTests.test_samr_change_password$ktest$
> ^samba.tests.ntlmdisabled.python$ad_dc_no_ntlm$.python3.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection$ad_dc_no_ntlm$
> ^samba.tests.ntlmdisabled.python$ad_dc_no_ntlm$.python2.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection$ad_dc_no_ntlm$
>-# BUG:https://bugzilla.samba.org/show_bug.cgi?id=13690
>-^samba3.blackbox.force_group_change.*
>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;
>-- 
>2.20.1.495.gaa96b0ce6b-goog
>

Flags:

slow: review+
asn: review+

Actions: View

Attachments on bug 13690: 14790 | 14797 | 14802 | 14803