The branch, v4-6-stable has been updated via 36d0070 VERSION: Disable GIT_SNAPSHOTS for the 4.6.2 release. via 8f35980 WHATSNEW: Add release notes for 4.6.2. via 2b9a812 s3: Test for CVE-2017-2619 regression with "follow symlinks = no" - part 2 via 9e81c83 s3: smbd: Fix "follow symlink = no" regression part 2. via 9e2ce69 s3: smbd: Fix "follow symlink = no" regression part 2. via 076f01e s3: Fixup test for CVE-2017-2619 regression with "follow symlinks = no" via 5a573c2 s3: Test for CVE-2017-2619 regression with "follow symlinks = no". via faea234 s3: smbd: Fix incorrect logic exposed by fix for the security bug 12496 (CVE-2017-2619). via 7b7f6a0 VERSION: Re-enable GIT_SNAPSHOTS. via 6cd0b59 VERSION: Bump version up to 4.6.2. from 1a8f3cf VERSION: Disable GIT_SNAPSHOTS for the 4.6.1 release.
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-6-stable - Log ----------------------------------------------------------------- commit 36d0070a6a7b021804a81fe5313cf6678769c7ae Author: Karolin Seeger <ksee...@samba.org> Date: Fri Mar 31 08:34:16 2017 +0200 VERSION: Disable GIT_SNAPSHOTS for the 4.6.2 release. Signed-off-by: Karolin Seeger <ksee...@samba.org> commit 8f359809bbd21a8e63bee10139db51104819820d Author: Karolin Seeger <ksee...@samba.org> Date: Fri Mar 31 08:33:25 2017 +0200 WHATSNEW: Add release notes for 4.6.2. Signed-off-by: Karolin Seeger <ksee...@samba.org> commit 2b9a812c14f4a9599ba71c99fc28fa94e8f63fcf Author: Jeremy Allison <j...@samba.org> Date: Mon Mar 27 22:10:29 2017 -0700 s3: Test for CVE-2017-2619 regression with "follow symlinks = no" - part 2 Add tests for regular access. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> Autobuild-User(master): Ralph Böhme <s...@samba.org> Autobuild-Date(master): Tue Mar 28 17:05:27 CEST 2017 on sn-devel-144 (cherry picked from commit 4e734fcd1bf82c08aa303ce44e9735acccffcf06) commit 9e81c832f9c90d63569d614edfe655182522abdb Author: Jeremy Allison <j...@samba.org> Date: Mon Mar 27 17:09:38 2017 -0700 s3: smbd: Fix "follow symlink = no" regression part 2. Use the cwd_name parameter to reconstruct the original client name for symlink testing. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> (cherry picked from commit e182a4d39e86c9694e255efdf6ee2ea3ccb9af4a) commit 9e2ce6939861e51e5e626426aaf2b7b1075b31bf Author: Jeremy Allison <j...@samba.org> Date: Mon Mar 27 17:04:58 2017 -0700 s3: smbd: Fix "follow symlink = no" regression part 2. Add an extra paramter to cwd_name to check_reduced_name(). If cwd_name == NULL then fname is a client given path relative to the root path of the share. If cwd_name != NULL then fname is a client given path relative to cwd_name. cwd_name is relative to the root path of the share. Not yet used, logic added in the next commit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> (cherry picked from commit 83e30cb48859b412b76572b6a3ba84d8fde167af) commit 076f01e55a1d5ad77f975dc397b50c9f620e6959 Author: Jeremy Allison <j...@samba.org> Date: Mon Mar 27 22:07:50 2017 -0700 s3: Fixup test for CVE-2017-2619 regression with "follow symlinks = no" Use correct bash operators (not string operators). Add missing "return". BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Boehme <s...@samba.org> (cherry picked from commit 037297a1c50e90a0092e3b94f472623f41ccc015) commit 5a573c2285e42777282ace19b9b83f27858a4c55 Author: Jeremy Allison <j...@samba.org> Date: Mon Mar 27 11:48:25 2017 -0700 s3: Test for CVE-2017-2619 regression with "follow symlinks = no". BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Uri Simchoni <u...@samba.org> Back-ported from commit 782172a9bef0040981d20e49519b13dd744df6a0 commit faea23484be55dad2c0e6eafbcb8ba7d05692e6c Author: Jeremy Allison <j...@samba.org> Date: Mon Mar 27 10:46:47 2017 -0700 s3: smbd: Fix incorrect logic exposed by fix for the security bug 12496 (CVE-2017-2619). In a UNIX filesystem, the names "." and ".." by definition can *never* be symlinks - they are already reserved names. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12721 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Uri Simchoni <u...@samba.org> (cherry picked from commit ae17bebd250bdde5614b2ac17e53512f19fe9b68) commit 7b7f6a02046ab9fb710277c4bdb64e5458f56c2a Author: Karolin Seeger <ksee...@samba.org> Date: Fri Mar 31 08:31:37 2017 +0200 VERSION: Re-enable GIT_SNAPSHOTS. Signed-off-by: Karolin Seeger <ksee...@samba.org> commit 6cd0b59eadf2bdb6d86195b076c2ef3bec00bbee Author: Karolin Seeger <ksee...@samba.org> Date: Thu Mar 23 10:17:00 2017 +0100 VERSION: Bump version up to 4.6.2. Signed-off-by: Karolin Seeger <ksee...@samba.org> (cherry picked from commit c47fee64a6419894713fde18907aff68c7d4c000) ----------------------------------------------------------------------- Summary of changes: VERSION | 2 +- WHATSNEW.txt | 45 +++++++++++- selftest/target/Samba3.pm | 7 ++ source3/script/tests/test_smbclient_s3.sh | 111 ++++++++++++++++++++++++++++++ source3/smbd/filename.c | 2 +- source3/smbd/open.c | 2 +- source3/smbd/proto.h | 4 +- source3/smbd/vfs.c | 40 ++++++++++- 8 files changed, 204 insertions(+), 9 deletions(-) Changeset truncated at 500 lines: diff --git a/VERSION b/VERSION index 8632851..9668644 100644 --- a/VERSION +++ b/VERSION @@ -25,7 +25,7 @@ ######################################################## SAMBA_VERSION_MAJOR=4 SAMBA_VERSION_MINOR=6 -SAMBA_VERSION_RELEASE=1 +SAMBA_VERSION_RELEASE=2 ######################################################## # If a official release has a serious bug # diff --git a/WHATSNEW.txt b/WHATSNEW.txt index 02935d7..a5feff8 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -1,4 +1,45 @@ ============================= + Release Notes for Samba 4.6.2 + March 31, 2017 + ============================= + + +This is a bug fix release to address a regression introduced by the security +fixes for CVE-2017-2619 (Symlink race allows access outside share definition). +Please see https://bugzilla.samba.org/show_bug.cgi?id=12721 for details. + + +Changes since 4.6.1: +-------------------- + +o Jeremy Allison <j...@samba.org> + * BUG 12721: Fix regression with "follow symlinks = no". + + +####################################### +Reporting bugs & Development Discussion +####################################### + +Please discuss this release on the samba-technical mailing list or by +joining the #samba-technical IRC channel on irc.freenode.net. + +If you do report problems then please try to send high quality +feedback. If you don't provide vital information to help us track down +the problem then you will probably be ignored. All bug reports should +be filed under the "Samba 4.1 and newer" product in the project's Bugzilla +database (https://bugzilla.samba.org/). + + +====================================================================== +== Our Code, Our Bugs, Our Responsibility. +== The Samba Team +====================================================================== + + +Release notes for older releases follow: +---------------------------------------- + + ============================= Release Notes for Samba 4.6.1 March 23, 2017 ============================= @@ -66,8 +107,8 @@ database (https://bugzilla.samba.org/). ====================================================================== -Release notes for older releases follow: ----------------------------------------- +---------------------------------------------------------------------- + ============================== Release Notes for Samba 4.6.0 diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 013e8d5..d080a91 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1245,6 +1245,9 @@ sub provision($$$$$$$$) my $shadow_shrdir="$shadow_basedir/share"; push(@dirs,$shadow_shrdir); + my $nosymlinks_shrdir="$shrdir/nosymlinks"; + push(@dirs,$nosymlinks_shrdir); + # this gets autocreated by winbindd my $wbsockdir="$prefix_abs/winbindd"; my $wbsockprivdir="$lockdir/winbindd_privileged"; @@ -1858,6 +1861,10 @@ sub provision($$$$$$$$) copy = tmp acl_xattr:ignore system acls = yes acl_xattr:default acl style = posix +[nosymlinks] + copy = tmp + path = $nosymlinks_shrdir + follow symlinks = no [acl_xattr_ign_sysacl_windows] copy = tmp acl_xattr:ignore system acls = yes diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index 22849bd..9bff883 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -1096,6 +1096,113 @@ EOF fi } +# Test follow symlinks can't access symlinks +test_nosymlinks() +{ +# Setup test dirs. + slink_name="$LOCAL_PATH/nosymlinks/source" + slink_target="$LOCAL_PATH/nosymlinks/target" + mkdir_target="$LOCAL_PATH/nosymlinks/a" + dir1="$LOCAL_PATH/nosymlinks/foo" + dir2="$LOCAL_PATH/nosymlinks/foo/bar" + get_target="$LOCAL_PATH/nosymlinks/foo/bar/testfile" + + rm -f $slink_target + rm -f $slink_name + rm -rf $mkdir_target + rm -rf $dir1 + + touch $slink_target + ln -s $slink_target $slink_name + + mkdir $dir1 + mkdir $dir2 + touch $get_target + +# Getting a file through a symlink name should fail. + tmpfile=$PREFIX/smbclient_interactive_prompt_commands + cat > $tmpfile <<EOF +get source +quit +EOF + cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/nosymlinks -I $SERVER_IP $ADDARGS < $tmpfile 2>&1' + eval echo "$cmd" + out=`eval $cmd` + ret=$? + rm -f $tmpfile + + if [ $ret -ne 0 ] ; then + echo "$out" + echo "failed accessing nosymlinks with error $ret" + false + return + fi + + echo "$out" | grep 'NT_STATUS_ACCESS_DENIED' + ret=$? + if [ $ret -ne 0 ] ; then + echo "$out" + echo "failed - should get NT_STATUS_ACCESS_DENIED getting \\nosymlinks\\source" + false + return + fi + +# But we should be able to create and delete directories. + cat > $tmpfile <<EOF +mkdir a +mkdir a\\b +quit +EOF + cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/nosymlinks -I $SERVER_IP $ADDARGS < $tmpfile 2>&1' + eval echo "$cmd" + out=`eval $cmd` + ret=$? + rm -f $tmpfile + + if [ $ret -ne 0 ] ; then + echo "$out" + echo "failed accessing nosymlinks with error $ret" + false + return + fi + + echo "$out" | grep 'NT_STATUS' + ret=$? + if [ $ret -eq 0 ] ; then + echo "$out" + echo "failed - NT_STATUS_XXXX doing mkdir a; mkdir a\\b on \\nosymlinks" + false + fi + +# Ensure regular file/directory access also works. + cat > $tmpfile <<EOF +cd foo\\bar +ls +get testfile - +quit +EOF + cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/nosymlinks -I $SERVER_IP $ADDARGS < $tmpfile 2>&1' + eval echo "$cmd" + out=`eval $cmd` + ret=$? + rm -f $tmpfile + + if [ $ret -ne 0 ] ; then + echo "$out" + echo "failed accessing nosymlinks with error $ret" + false + return + fi + + echo "$out" | grep 'NT_STATUS' + ret=$? + if [ $ret -eq 0 ] ; then + echo "$out" + echo "failed - NT_STATUS_XXXX doing cd foo\\bar; get testfile on \\nosymlinks" + false + return + fi +} LOGDIR_PREFIX=test_smbclient_s3 @@ -1196,6 +1303,10 @@ testit "streams_depot can delete correctly" \ test_streams_depot_delete || \ failed=`expr $failed + 1` +testit "follow symlinks = no" \ + test_nosymlinks || \ + failed=`expr $failed + 1` + testit "rm -rf $LOGDIR" \ rm -rf $LOGDIR || \ failed=`expr $failed + 1` diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index efe52a0..2d85e8d 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1242,7 +1242,7 @@ NTSTATUS check_name(connection_struct *conn, const char *name) } if (!lp_widelinks(SNUM(conn)) || !lp_follow_symlinks(SNUM(conn))) { - status = check_reduced_name(conn,name); + status = check_reduced_name(conn, NULL, name); if (!NT_STATUS_IS_OK(status)) { DEBUG(5,("check_name: name %s failed with %s\n",name, nt_errstr(status))); diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 08d14cb..c62ffe4 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -549,7 +549,7 @@ static int non_widelink_open(struct connection_struct *conn, } /* Ensure the relative path is below the share. */ - status = check_reduced_name(conn, final_component); + status = check_reduced_name(conn, parent_dir, final_component); if (!NT_STATUS_IS_OK(status)) { saved_errno = map_errno_from_nt_status(status); goto out; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 50ede9d..61bd33a 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1227,7 +1227,9 @@ const char *vfs_readdirname(connection_struct *conn, void *p, SMB_STRUCT_STAT *sbuf, char **talloced); int vfs_ChDir(connection_struct *conn, const char *path); char *vfs_GetWd(TALLOC_CTX *ctx, connection_struct *conn); -NTSTATUS check_reduced_name(connection_struct *conn, const char *fname); +NTSTATUS check_reduced_name(connection_struct *conn, + const char *cwd_name, + const char *fname); NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, const char *fname, struct smb_request *smbreq); diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index 35f560b..b7364b7 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -1179,11 +1179,20 @@ NTSTATUS check_reduced_name_with_privilege(connection_struct *conn, /******************************************************************* Reduce a file name, removing .. elements and checking that it is below dir in the heirachy. This uses realpath. + + If cwd_name == NULL then fname is a client given path relative + to the root path of the share. + + If cwd_name != NULL then fname is a client given path relative + to cwd_name. cwd_name is relative to the root path of the share. ********************************************************************/ -NTSTATUS check_reduced_name(connection_struct *conn, const char *fname) +NTSTATUS check_reduced_name(connection_struct *conn, + const char *cwd_name, + const char *fname) { char *resolved_name = NULL; + char *new_fname = NULL; bool allow_symlinks = true; bool allow_widelinks = false; @@ -1307,8 +1316,11 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname) /* fname can't have changed in resolved_path. */ const char *p = &resolved_name[rootdir_len]; - /* *p can be '\0' if fname was "." */ - if (*p == '\0' && ISDOT(fname)) { + /* + * UNIX filesystem semantics, names consisting + * only of "." or ".." CANNOT be symlinks. + */ + if (ISDOT(fname) || ISDOTDOT(fname)) { goto out; } @@ -1322,11 +1334,32 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname) } p++; + + /* + * If cwd_name is present and not ".", + * then fname is relative to that, not + * the root of the share. Make sure the + * path we check is the one the client + * sent (cwd_name+fname). + */ + if (cwd_name != NULL && !ISDOT(cwd_name)) { + new_fname = talloc_asprintf(talloc_tos(), + "%s/%s", + cwd_name, + fname); + if (new_fname == NULL) { + SAFE_FREE(resolved_name); + return NT_STATUS_NO_MEMORY; + } + fname = new_fname; + } + if (strcmp(fname, p)!=0) { DEBUG(2, ("check_reduced_name: Bad access " "attempt: %s is a symlink to %s\n", fname, p)); SAFE_FREE(resolved_name); + TALLOC_FREE(new_fname); return NT_STATUS_ACCESS_DENIED; } } @@ -1336,6 +1369,7 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname) DBG_INFO("%s reduced to %s\n", fname, resolved_name); SAFE_FREE(resolved_name); + TALLOC_FREE(new_fname); return NT_STATUS_OK; } -- Samba Shared Repository