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@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

diff --git a/selftest/knownfail b/selftest/knownfail
index 9678883924ef8d59b6111a75da1b6b3425532d2a..abbbd889c7109f96766f67b833e164980230c531 100644 (file)
--- 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 7aecea5f857ae0a3151010decd56d150fd3a5d5b..a4bcb747d37e61b1856a10f4e173cea6c2847762 100644 (file)
--- 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;