CVE-2019-10197: smbd: split change_to_user_impersonate() out of change_to_user_internal()

This makes sure we always call chdir_current_service() even
when we still impersonated the user. Which is important
in order to run the SMB* request within the correct working directory
and only if the user has permissions to enter that directory.

It makes sure we always update conn->lastused_count
in chdir_current_service() for each request.

Note that vfs_ChDir() (called from chdir_current_service())
maintains its own cache and avoids calling SMB_VFS_CHDIR()
if possible.

It means we still avoid syscalls if we get a multiple requests
for the same session/tcon tuple.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14035

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Karolin Seeger <kseeger@samba.org>
Autobuild-Date(master): Tue Sep  3 09:27:22 UTC 2019 on sn-devel-184

diff --git a/selftest/knownfail.d/CVE-2019-10197 b/selftest/knownfail.d/CVE-2019-10197
deleted file mode 100644 (file)
index f7056bb..0000000
--- a/selftest/knownfail.d/CVE-2019-10197
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.blackbox.smbclient_s3.*.noperm.share.regression

diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index ac2f645e6ca803b0feeee0df5b08b90f47f6433f..5425449d42c33e6650af15dd0ae1066cf134f7a4 100644 (file)
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -306,9 +306,9 @@ static void print_impersonation_info(connection_struct *conn)
  stack, but modify the current_user entries.
 ****************************************************************************/
 
-static bool change_to_user_internal(connection_struct *conn,
-                                   const struct auth_session_info *session_info,
-                                   uint64_t vuid)
+static bool change_to_user_impersonate(connection_struct *conn,
+                                      const struct auth_session_info *session_info,
+                                      uint64_t vuid)
 {
        int snum;
        gid_t gid;
@@ -321,7 +321,6 @@ static bool change_to_user_internal(connection_struct *conn,
 
        if ((current_user.conn == conn) &&
            (current_user.vuid == vuid) &&
-           (current_user.need_chdir == conn->tcon_done) &&
            (current_user.ut.uid == session_info->unix_token->uid))
        {
                DBG_INFO("Skipping user change - already user\n");
@@ -426,6 +425,20 @@ static bool change_to_user_internal(connection_struct *conn,
 
        current_user.conn = conn;
        current_user.vuid = vuid;
+       return true;
+}
+
+static bool change_to_user_internal(connection_struct *conn,
+                                   const struct auth_session_info *session_info,
+                                   uint64_t vuid)
+{
+       bool ok;
+
+       ok = change_to_user_impersonate(conn, session_info, vuid);
+       if (!ok) {
+               return false;
+       }
+
        current_user.need_chdir = conn->tcon_done;
        current_user.done_chdir = false;