The Samba-Bugzilla – Attachment 14803 Details for
Bug 13690
Adding `force group = ...` to an active SMB sessoin causes PANIC
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for 4.9.next, 4.8.next
bug-13690-4.x.patch (text/plain), 8.97 KB, created by
Jeremy Allison
on 2019-01-25 17:25:07 UTC
(
hide
)
Description:
git-am fix for 4.9.next, 4.8.next
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2019-01-25 17:25:07 UTC
Size:
8.97 KB
patch
obsolete
>From ce475c1395b1917f3f96702ddda25b151ee76b25 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 | 6 ++ > .../script/tests/test_force_group_change.sh | 73 +++++++++++++++++++ > source3/selftest/tests.py | 3 + > 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 baf3d57a31a..ca2f79d768c 100644 >--- a/selftest/knownfail >+++ b/selftest/knownfail >@@ -349,3 +349,5 @@ > # Disabling NTLM means you can't use samr to change the password > ^samba.tests.ntlmdisabled.python\(ktest\).ntlmdisabled.NtlmDisabledTests.test_samr_change_password\(ktest\) > ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).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 0d8014c7d13..6e73f9f2673 100644 >--- a/selftest/selftesthelpers.py >+++ b/selftest/selftesthelpers.py >@@ -197,3 +197,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 314aae55bc5..47b9c8cbc48 100755 >--- a/selftest/target/Samba3.pm >+++ b/selftest/target/Samba3.pm >@@ -904,6 +904,12 @@ sub setup_fileserver > force group = everyone > write list = force_user > >+# BUG: https://bugzilla.samba.org/show_bug.cgi?id=13690 >+[force_group_test] >+ path = $share_dir >+ comment = force group test >+# force group = everyone >+ > [smbget] > path = $smbget_sharedir > comment = smb username is [%U] >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 f91cb5a6cd9..96524d1748f 100755 >--- a/source3/selftest/tests.py >+++ b/source3/selftest/tests.py >@@ -291,6 +291,9 @@ for env in ["fileserver"]: > plantestsuite("samba3.blackbox.large_acl.NT1", env, [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls, '-m', 'NT1']) > 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.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 fdc98f74d016bcfd9673f4bc011ba7ede59bdf48 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 ca2f79d768c..baf3d57a31a 100644 >--- a/selftest/knownfail >+++ b/selftest/knownfail >@@ -349,5 +349,3 @@ > # Disabling NTLM means you can't use samr to change the password > ^samba.tests.ntlmdisabled.python\(ktest\).ntlmdisabled.NtlmDisabledTests.test_samr_change_password\(ktest\) > ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).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 9d5321cf4cc..ced2d450f8e 100644 >--- a/source3/smbd/uid.c >+++ b/source3/smbd/uid.c >@@ -296,6 +296,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; >@@ -335,9 +336,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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
slow
:
review+
asn
:
review+
Actions:
View
Attachments on
bug 13690
:
14790
|
14797
|
14802
| 14803