From 83d82e735bb6399142b7e18cf83aa81abefeba33 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 18 Jan 2019 14:24:30 -0800 Subject: [PATCH] smbd: uid: Don't crash if 'force group' is added to an existing share connection. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 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 Reviewed-by: Ralph Boehme Autobuild-User(master): Ralph Böhme 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 52b179a83c2..84776d4f35d 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -348,5 +348,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 b24ae3cc3b0..598327eba03 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -288,6 +288,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; @@ -314,9 +315,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.47.2