The branch, master has been updated
       via  efd4832c2cf CVE-2019-10197: smbd: split 
change_to_user_impersonate() out of change_to_user_internal()
       via  9ab5a51a6e7 CVE-2019-10197: test_smbclient_s3.sh: add regression 
test for the no permission on share root problem
       via  d7ed819d9f8 CVE-2019-10197: selftest: make fsrvp_share its own 
independent subdirectory
       via  509113fcffd CVE-2019-10197: smbd: make sure we reset 
current_user.{need,done}_chdir in become_root()
       via  6fae81312fa CVE-2019-10197: smbd: make sure that 
change_to_user_internal() always resets current_user.done_chdir
       via  e48c1189690 CVE-2019-10197: smbd: separate out impersonation debug 
info into a new function.
      from  16c3a00d2eb build: remove unneeded libceph-common dependency

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit efd4832c2cfc4092bafb93dd4caf32d04488919f
Author: Stefan Metzmacher <[email protected]>
Date:   Thu Jul 11 17:02:15 2019 +0200

    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 <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>
    
    Autobuild-User(master): Karolin Seeger <[email protected]>
    Autobuild-Date(master): Tue Sep  3 09:27:22 UTC 2019 on sn-devel-184

commit 9ab5a51a6e770c8bd95ceca892808cf2cf670852
Author: Stefan Metzmacher <[email protected]>
Date:   Tue Jul 16 15:40:38 2019 +0200

    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 <[email protected]>

commit d7ed819d9f83a89f436b13437a71c85335cdb5df
Author: Stefan Metzmacher <[email protected]>
Date:   Tue Jul 30 17:16:59 2019 +0200

    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 <[email protected]>

commit 509113fcffdd0f371f8b32efc900d2102ef0cef7
Author: Stefan Metzmacher <[email protected]>
Date:   Tue Jun 18 14:04:08 2019 +0200

    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 <[email protected]>

commit 6fae81312fad5caf948f0f0f48b84fd3f59dc944
Author: Stefan Metzmacher <[email protected]>
Date:   Thu Jul 11 17:01:29 2019 +0200

    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 <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>

commit e48c11896904fd063619a95d3fa7ceccbf2beb56
Author: Jeremy Allison <[email protected]>
Date:   Fri Jul 12 12:10:35 2019 -0700

    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 <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>
    Reviewed-by: Stefan Metzmacher <[email protected]>

-----------------------------------------------------------------------

Summary of changes:
 selftest/target/Samba3.pm                 | 19 +++++++++-
 source3/script/tests/test_smbclient_s3.sh | 30 +++++++++++++++
 source3/smbd/uid.c                        | 62 ++++++++++++++++++++++---------
 3 files changed, 91 insertions(+), 20 deletions(-)


Changeset truncated at 500 lines:

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 2587ea31253..1dfd543d6b5 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1434,6 +1434,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);
 
@@ -1458,6 +1461,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";
@@ -1501,6 +1507,11 @@ sub provision($$$$$$$$$)
        chmod 0755, $piddir;
 
 
+       ##
+       ## Create a directory without permissions to enter
+       ##
+       chmod 0000, $noperm_shrdir;
+
        ##
        ## create ro and msdfs share layout
        ##
@@ -1824,6 +1835,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
@@ -2033,14 +2048,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
diff --git a/source3/script/tests/test_smbclient_s3.sh 
b/source3/script/tests/test_smbclient_s3.sh
index bf033ccd2fb..0bae1d78fac 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`
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 53c7aa7b798..5425449d42c 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -279,14 +279,36 @@ 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.
 ****************************************************************************/
 
-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;
@@ -299,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");
@@ -404,7 +425,22 @@ 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;
 
        if (current_user.need_chdir) {
                ok = chdir_current_service(conn);
@@ -415,20 +451,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;
 }
 
@@ -614,6 +637,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 */


-- 
Samba Shared Repository

Reply via email to