From 5e94fe726e9af81374c697ce603b3728ccaaebf3 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 a4bcb747d37e..ce8e8d92131c 100644

--- a/source3/smbd/uid.c

+++ b/source3/smbd/uid.c

@@ -279,6 +279,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.

@@ -415,20 +437,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 b4cd0dcbc38ae61cfb075e5f659384df889e99f7 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 ce8e8d92131c..77a81f602988 100644

--- a/source3/smbd/uid.c

+++ b/source3/smbd/uid.c

@@ -427,6 +427,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 b1496ce793129302c9959ebc6330219c6a3143f0 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 77a81f602988..50868ba8572a 100644

--- a/source3/smbd/uid.c

+++ b/source3/smbd/uid.c

@@ -624,6 +624,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 03a0719d6d5c1a81b44bc3cedc76563a1eb04491 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 9d88253c9fe7..f7eb314138a0 100755

--- a/selftest/target/Samba3.pm

+++ b/selftest/target/Samba3.pm

@@ -1540,6 +1540,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";

@@ -2083,14 +2086,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 409447f3258b87745a2248570278b1c6da8991f4 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 f7eb314138a0..2f491441815f 100755

--- a/selftest/target/Samba3.pm

+++ b/selftest/target/Samba3.pm

@@ -1516,6 +1516,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);

@@ -1586,6 +1589,11 @@ sub provision($$$$$$$$$)

 	chmod 0755, $piddir;

+	##

+	## Create a directory without permissions to enter

+	##

+	chmod 0000, $noperm_shrdir;

+

 	##

 	## create ro and msdfs share layout

 	##

@@ -1902,6 +1910,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 501e034aa5b6ba50bf14e41c59674fbbc28a2e9c 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 50868ba8572a..5c39baade5cf 100644

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

-- 

2.17.1