From b7d9500a6d1f54d29338929dd8d0f0863ad995b4 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra@samba.org>
Date: Fri, 12 Jul 2019 12:10:35 -0700
Subject: [PATCH 1/6] CVE-2019-10197: smbd: separate out impersonation debug
 info into a new function.

Will be called on elsewhere on successful impersonation.

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
---
 source3/smbd/uid.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index ced2d450f8e7..89f539ed4300 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -284,6 +284,28 @@ static bool check_user_ok(connection_struct *conn,
 	return(True);
 }
 
+static void print_impersonation_info(connection_struct *conn)
+{
+	struct smb_filename *cwdfname = NULL;
+
+	if (!CHECK_DEBUGLVL(DBGLVL_INFO)) {
+		return;
+	}
+
+	cwdfname = vfs_GetWd(talloc_tos(), conn);
+	if (cwdfname == NULL) {
+		return;
+	}
+
+	DBG_INFO("Impersonated user: uid=(%d,%d), gid=(%d,%d), cwd=[%s]\n",
+		 (int)getuid(),
+		 (int)geteuid(),
+		 (int)getgid(),
+		 (int)getegid(),
+		 cwdfname->base_name);
+	TALLOC_FREE(cwdfname);
+}
+
 /****************************************************************************
  Become the user of a connection number without changing the security context
  stack, but modify the current_user entries.
@@ -420,20 +442,7 @@ static bool change_to_user_internal(connection_struct *conn,
 		current_user.done_chdir = true;
 	}
 
-	if (CHECK_DEBUGLVL(DBGLVL_INFO)) {
-		struct smb_filename *cwdfname = vfs_GetWd(talloc_tos(), conn);
-		if (cwdfname == NULL) {
-			return false;
-		}
-		DBG_INFO("Impersonated user: uid=(%d,%d), gid=(%d,%d), cwd=[%s]\n",
-			 (int)getuid(),
-			 (int)geteuid(),
-			 (int)getgid(),
-			 (int)getegid(),
-			 cwdfname->base_name);
-		TALLOC_FREE(cwdfname);
-	}
-
+	print_impersonation_info(conn);
 	return true;
 }
 
-- 
2.17.1


From 632fdf8947285c70650ce5a849349a86c4dc294e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Thu, 11 Jul 2019 17:01:29 +0200
Subject: [PATCH 2/6] CVE-2019-10197: smbd: make sure that
 change_to_user_internal() always resets current_user.done_chdir

We should not leave current_user.done_chdir as true if we didn't call
chdir_current_service() with success.

This caused problems in when calling vfs_ChDir() in pop_conn_ctx() when
chdir_current_service() worked once on one share but later failed on another
share.

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>
---
 source3/smbd/uid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 89f539ed4300..b6ef02a36b3a 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -432,6 +432,7 @@ static bool change_to_user_internal(connection_struct *conn,
 	current_user.conn = conn;
 	current_user.vuid = vuid;
 	current_user.need_chdir = conn->tcon_done;
+	current_user.done_chdir = false;
 
 	if (current_user.need_chdir) {
 		ok = chdir_current_service(conn);
-- 
2.17.1


From 830dfe13ccdf5d12e3f04a2f6068ae2d40fb71cf Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 18 Jun 2019 14:04:08 +0200
Subject: [PATCH 3/6] CVE-2019-10197: smbd: make sure we reset
 current_user.{need,done}_chdir in become_root()

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 source3/smbd/uid.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index b6ef02a36b3a..9cf09c11a622 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -629,6 +629,9 @@ void smbd_become_root(void)
 	}
 	push_conn_ctx();
 	set_root_sec_ctx();
+
+	current_user.need_chdir = false;
+	current_user.done_chdir = false;
 }
 
 /* Unbecome the root user */
-- 
2.17.1


From 6ff1e7677208774e0b973457c50f48965871f117 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 30 Jul 2019 17:16:59 +0200
Subject: [PATCH 4/6] CVE-2019-10197: selftest: make fsrvp_share its own
 independent subdirectory

The next patch will otherwise break the fsrvp related tests.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 selftest/target/Samba3.pm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 142523441751..5e9daf3eec46 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1474,6 +1474,9 @@ sub provision($$$$$$$$$)
 	my $widelinks_linkdir="$shrdir/widelinks_foo";
 	push(@dirs,$widelinks_linkdir);
 
+	my $fsrvp_shrdir="$shrdir/fsrvp";
+	push(@dirs,$fsrvp_shrdir);
+
 	my $shadow_tstdir="$shrdir/shadow";
 	push(@dirs,$shadow_tstdir);
 	my $shadow_mntdir="$shadow_tstdir/mount";
@@ -2009,14 +2012,14 @@ sub provision($$$$$$$$$)
 	guest ok = yes
 
 [fsrvp_share]
-	path = $shrdir
+	path = $fsrvp_shrdir
 	comment = fake shapshots using rsync
 	vfs objects = shell_snap shadow_copy2
 	shell_snap:check path command = $fake_snap_pl --check
 	shell_snap:create command = $fake_snap_pl --create
 	shell_snap:delete command = $fake_snap_pl --delete
 	# a relative path here fails, the snapshot dir is no longer found
-	shadow:snapdir = $shrdir/.snapshots
+	shadow:snapdir = $fsrvp_shrdir/.snapshots
 
 [shadow1]
 	path = $shadow_shrdir
-- 
2.17.1


From 99352a814b086fef25af184a357cb027cabc7b59 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 16 Jul 2019 15:40:38 +0200
Subject: [PATCH 5/6] CVE-2019-10197: test_smbclient_s3.sh: add regression test
 for the no permission on share root problem

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 selftest/knownfail.d/CVE-2019-10197       |  1 +
 selftest/target/Samba3.pm                 | 12 +++++++++
 source3/script/tests/test_smbclient_s3.sh | 30 +++++++++++++++++++++++
 3 files changed, 43 insertions(+)
 create mode 100644 selftest/knownfail.d/CVE-2019-10197

diff --git a/selftest/knownfail.d/CVE-2019-10197 b/selftest/knownfail.d/CVE-2019-10197
new file mode 100644
index 000000000000..f7056bbf3ad4
--- /dev/null
+++ b/selftest/knownfail.d/CVE-2019-10197
@@ -0,0 +1 @@
+^samba3.blackbox.smbclient_s3.*.noperm.share.regression
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 5e9daf3eec46..22e5035b0794 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1450,6 +1450,9 @@ sub provision($$$$$$$$$)
 	my $ro_shrdir="$shrdir/root-tmp";
 	push(@dirs,$ro_shrdir);
 
+	my $noperm_shrdir="$shrdir/noperm-tmp";
+	push(@dirs,$noperm_shrdir);
+
 	my $msdfs_shrdir="$shrdir/msdfsshare";
 	push(@dirs,$msdfs_shrdir);
 
@@ -1520,6 +1523,11 @@ sub provision($$$$$$$$$)
 	chmod 0755, $piddir;
 
 
+	##
+	## Create a directory without permissions to enter
+	##
+	chmod 0000, $noperm_shrdir;
+
 	##
 	## create ro and msdfs share layout
 	##
@@ -1828,6 +1836,10 @@ sub provision($$$$$$$$$)
 [ro-tmp]
 	path = $ro_shrdir
 	guest ok = yes
+[noperm]
+	path = $noperm_shrdir
+	wide links = yes
+	guest ok = yes
 [write-list-tmp]
 	path = $shrdir
         read only = yes
diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index bf033ccd2fbf..0bae1d78fac9 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -1329,6 +1329,32 @@ EOF
     fi
 }
 
+#
+# Regression test for CVE-2019-10197
+# we should always get ACCESS_DENIED
+#
+test_noperm_share_regression()
+{
+    cmd='$SMBCLIENT -U$USERNAME%$PASSWORD //$SERVER/noperm -I $SERVER_IP $LOCAL_ADDARGS -c "ls;ls"  2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+    if [ $ret -eq 0 ] ; then
+       echo "$out"
+       echo "failed accessing no perm share should not work"
+       return 1
+    fi
+
+    num=`echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' | wc -l`
+    if [ "$num" -ne "2" ] ; then
+       echo "$out"
+       echo "failed num[$num] - two NT_STATUS_ACCESS_DENIED lines expected"
+       return 1
+    fi
+
+    return 0
+}
+
 # Test smbclient deltree command
 test_deltree()
 {
@@ -1857,6 +1883,10 @@ testit "follow local symlinks" \
     test_local_symlinks || \
     failed=`expr $failed + 1`
 
+testit "noperm share regression" \
+    test_noperm_share_regression || \
+    failed=`expr $failed + 1`
+
 testit "smbclient deltree command" \
     test_deltree || \
     failed=`expr $failed + 1`
-- 
2.17.1


From 9c8ebfb8488a3583a3a42b49d6cdadb923f35c3e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Thu, 11 Jul 2019 17:02:15 +0200
Subject: [PATCH 6/6] 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>
---
 selftest/knownfail.d/CVE-2019-10197 |  1 -
 source3/smbd/uid.c                  | 21 +++++++++++++++++----
 2 files changed, 17 insertions(+), 5 deletions(-)
 delete mode 100644 selftest/knownfail.d/CVE-2019-10197

diff --git a/selftest/knownfail.d/CVE-2019-10197 b/selftest/knownfail.d/CVE-2019-10197
deleted file mode 100644
index f7056bbf3ad4..000000000000
--- 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 9cf09c11a622..f65e3091dfd9 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -311,9 +311,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;
@@ -326,7 +326,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");
@@ -431,6 +430,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;
 
-- 
2.17.1

