The Samba-Bugzilla – Attachment 9382 Details for
Bug 10229
No access check verification on stream files.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am patchset for 4.0.x
bug-10229-4.0.patchset (text/plain), 10.95 KB, created by
Jeremy Allison
on 2013-11-05 19:24:36 UTC
(
hide
)
Description:
git-am patchset for 4.0.x
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2013-11-05 19:24:36 UTC
Size:
10.95 KB
patch
obsolete
>From 3d9f9fb9249a2179d792ce9042cebbffeb4088f3 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Mon, 28 Oct 2013 16:59:20 -0700 >Subject: [PATCH 1/2] Fix bug #10229 - No access check verification on stream > files. > >https://bugzilla.samba.org/show_bug.cgi?id=10229 > >We need to check if the requested access mask >could be used to open the underlying file (if >it existed), as we're passing in zero for the >access mask to the base filename. > >Back-ported for 4.0.x. > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> > >(Based on master commit 60f922bf1bd8816eacbb32c24793ad1f97a1d9f2) >--- > source3/smbd/open.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 57 insertions(+) > >diff --git a/source3/smbd/open.c b/source3/smbd/open.c >index a1b4e43..0282722 100644 >--- a/source3/smbd/open.c >+++ b/source3/smbd/open.c >@@ -300,6 +300,44 @@ static NTSTATUS check_parent_access(struct connection_struct *conn, > } > > /**************************************************************************** >+ Ensure when opening a base file for a stream open that we have permissions >+ to do so given the access mask on the base file. >+****************************************************************************/ >+ >+static NTSTATUS check_base_file_access(struct connection_struct *conn, >+ struct smb_filename *smb_fname, >+ uint32_t access_mask) >+{ >+ NTSTATUS status; >+ >+ status = smbd_calculate_access_mask(conn, smb_fname, >+ access_mask, >+ &access_mask); >+ if (!NT_STATUS_IS_OK(status)) { >+ DEBUG(10, ("smbd_calculate_access_mask " >+ "on file %s returned %s\n", >+ smb_fname_str_dbg(smb_fname), >+ nt_errstr(status))); >+ return status; >+ } >+ >+ if (access_mask & (FILE_WRITE_DATA|FILE_APPEND_DATA)) { >+ uint32_t dosattrs; >+ if (!CAN_WRITE(conn)) { >+ return NT_STATUS_ACCESS_DENIED; >+ } >+ dosattrs = dos_mode(conn, smb_fname); >+ if (IS_DOS_READONLY(dosattrs)) { >+ return NT_STATUS_ACCESS_DENIED; >+ } >+ } >+ >+ return smbd_check_access_rights(conn, >+ smb_fname, >+ access_mask); >+} >+ >+/**************************************************************************** > fd support routines - attempt to do a dos_open. > ****************************************************************************/ > >@@ -3749,6 +3787,25 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, > if (SMB_VFS_STAT(conn, smb_fname_base) == -1) { > DEBUG(10, ("Unable to stat stream: %s\n", > smb_fname_str_dbg(smb_fname_base))); >+ } else { >+ /* >+ * https://bugzilla.samba.org/show_bug.cgi?id=10229 >+ * We need to check if the requested access mask >+ * could be used to open the underlying file (if >+ * it existed), as we're passing in zero for the >+ * access mask to the base filename. >+ */ >+ status = check_base_file_access(conn, >+ smb_fname_base, >+ access_mask); >+ >+ if (!NT_STATUS_IS_OK(status)) { >+ DEBUG(10, ("Permission check " >+ "for base %s failed: " >+ "%s\n", smb_fname->base_name, >+ nt_errstr(status))); >+ goto fail; >+ } > } > > /* Open the base file. */ >-- >1.8.1.2 > > >From a6d1dffee24c80852c8f0dffd65fd2c83ab06b1a Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 29 Oct 2013 15:57:01 -0700 >Subject: [PATCH 2/2] Add regression test for bug #10229 - No access check > verification on stream files. > >Checks against a file with attribute READONLY, and >a security descriptor denying WRITE_DATA access. > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Stefan Metzmacher <metze@samba.org> >Reviewed-by: David Disseldorp <ddiss@suse.de> > >Autobuild-User(master): Jeremy Allison <jra@samba.org> >Autobuild-Date(master): Mon Nov 4 23:10:10 CET 2013 on sn-devel-104 >(cherry picked from commit 65882152cc7ccaba0e7903862b99ca93594ed080) >--- > selftest/knownfail | 1 + > source4/torture/raw/streams.c | 181 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 182 insertions(+) > >diff --git a/selftest/knownfail b/selftest/knownfail >index d249a25..e393635 100644 >--- a/selftest/knownfail >+++ b/selftest/knownfail >@@ -128,6 +128,7 @@ > ^samba4.raw.streams.*.delete > ^samba4.raw.streams.*.createdisp > ^samba4.raw.streams.*.sumtab >+^samba4.raw.streams.*.perms > ^samba4.raw.acls.INHERITFLAGS > ^samba4.raw.acls.*.create_dir > ^samba4.raw.acls.*.create_file >diff --git a/source4/torture/raw/streams.c b/source4/torture/raw/streams.c >index 1611c64..61852f5 100644 >--- a/source4/torture/raw/streams.c >+++ b/source4/torture/raw/streams.c >@@ -23,6 +23,8 @@ > #include "system/locale.h" > #include "torture/torture.h" > #include "libcli/raw/libcliraw.h" >+#include "libcli/security/dom_sid.h" >+#include "libcli/security/security_descriptor.h" > #include "system/filesys.h" > #include "libcli/libcli.h" > #include "torture/util.h" >@@ -1885,6 +1887,184 @@ static bool test_stream_summary_tab(struct torture_context *tctx, > return ret; > } > >+/* Test how streams interact with base file permissions */ >+/* Regression test for bug: >+ https://bugzilla.samba.org/show_bug.cgi?id=10229 >+ bug #10229 - No access check verification on stream files. >+*/ >+static bool test_stream_permissions(struct torture_context *tctx, >+ struct smbcli_state *cli) >+{ >+ NTSTATUS status; >+ bool ret = true; >+ union smb_open io; >+ const char *fname = BASEDIR "\\stream_permissions.txt"; >+ const char *stream = "Stream One:$DATA"; >+ const char *fname_stream; >+ union smb_fileinfo finfo; >+ union smb_setfileinfo sfinfo; >+ int fnum = -1; >+ union smb_fileinfo q; >+ union smb_setfileinfo set; >+ struct security_ace ace; >+ struct security_descriptor *sd; >+ >+ torture_assert(tctx, torture_setup_dir(cli, BASEDIR), >+ "Failed to setup up test directory: " BASEDIR); >+ >+ torture_comment(tctx, "(%s) testing permissions on streams\n", __location__); >+ >+ fname_stream = talloc_asprintf(tctx, "%s:%s", fname, stream); >+ >+ /* Create a file with a stream with attribute FILE_ATTRIBUTE_ARCHIVE. */ >+ ret = create_file_with_stream(tctx, cli, fname_stream); >+ if (!ret) { >+ goto done; >+ } >+ >+ ZERO_STRUCT(finfo); >+ finfo.generic.level = RAW_FILEINFO_BASIC_INFO; >+ finfo.generic.in.file.path = fname; >+ status = smb_raw_pathinfo(cli->tree, tctx, &finfo); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ torture_assert_int_equal_goto(tctx, >+ finfo.all_info.out.attrib & ~FILE_ATTRIBUTE_NONINDEXED, >+ FILE_ATTRIBUTE_ARCHIVE, ret, done, "attrib incorrect"); >+ >+ /* Change the attributes on the base file name. */ >+ ZERO_STRUCT(sfinfo); >+ sfinfo.generic.level = RAW_SFILEINFO_SETATTR; >+ sfinfo.generic.in.file.path = fname; >+ sfinfo.setattr.in.attrib = FILE_ATTRIBUTE_READONLY; >+ >+ status = smb_raw_setpathinfo(cli->tree, &sfinfo); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* Try and open the stream name for WRITE_DATA. Should >+ fail with ACCESS_DENIED. */ >+ >+ ZERO_STRUCT(io); >+ io.generic.level = RAW_OPEN_NTCREATEX; >+ io.ntcreatex.in.root_fid.fnum = 0; >+ io.ntcreatex.in.flags = 0; >+ io.ntcreatex.in.access_mask = SEC_FILE_WRITE_DATA; >+ io.ntcreatex.in.create_options = 0; >+ io.ntcreatex.in.file_attr = 0; >+ io.ntcreatex.in.share_access = 0; >+ io.ntcreatex.in.alloc_size = 0; >+ io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN; >+ io.ntcreatex.in.impersonation = NTCREATEX_IMPERSONATION_ANONYMOUS; >+ io.ntcreatex.in.security_flags = 0; >+ io.ntcreatex.in.fname = fname_stream; >+ >+ status = smb_raw_open(cli->tree, tctx, &io); >+ CHECK_STATUS(status, NT_STATUS_ACCESS_DENIED); >+ >+ /* Change the attributes on the base file back. */ >+ ZERO_STRUCT(sfinfo); >+ sfinfo.generic.level = RAW_SFILEINFO_SETATTR; >+ sfinfo.generic.in.file.path = fname; >+ sfinfo.setattr.in.attrib = 0; >+ >+ status = smb_raw_setpathinfo(cli->tree, &sfinfo); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* Re-open the file name. */ >+ >+ ZERO_STRUCT(io); >+ io.generic.level = RAW_OPEN_NTCREATEX; >+ io.ntcreatex.in.root_fid.fnum = 0; >+ io.ntcreatex.in.flags = 0; >+ io.ntcreatex.in.access_mask = (SEC_FILE_READ_DATA|SEC_FILE_WRITE_DATA| >+ SEC_STD_READ_CONTROL|SEC_STD_WRITE_DAC| >+ SEC_FILE_WRITE_ATTRIBUTE); >+ io.ntcreatex.in.create_options = 0; >+ io.ntcreatex.in.file_attr = 0; >+ io.ntcreatex.in.share_access = 0; >+ io.ntcreatex.in.alloc_size = 0; >+ io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN; >+ io.ntcreatex.in.impersonation = NTCREATEX_IMPERSONATION_ANONYMOUS; >+ io.ntcreatex.in.security_flags = 0; >+ io.ntcreatex.in.fname = fname; >+ >+ status = smb_raw_open(cli->tree, tctx, &io); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ fnum = io.ntcreatex.out.file.fnum; >+ >+ /* Get the existing security descriptor. */ >+ ZERO_STRUCT(q); >+ q.query_secdesc.level = RAW_FILEINFO_SEC_DESC; >+ q.query_secdesc.in.file.fnum = fnum; >+ q.query_secdesc.in.secinfo_flags = >+ SECINFO_OWNER | >+ SECINFO_GROUP | >+ SECINFO_DACL; >+ status = smb_raw_fileinfo(cli->tree, tctx, &q); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ sd = q.query_secdesc.out.sd; >+ >+ /* Now add a DENY WRITE security descriptor for Everyone. */ >+ torture_comment(tctx, "add a new ACE to the DACL\n"); >+ >+ ace.type = SEC_ACE_TYPE_ACCESS_DENIED; >+ ace.flags = 0; >+ ace.access_mask = SEC_FILE_WRITE_DATA; >+ ace.trustee = *dom_sid_parse_talloc(tctx, SID_WORLD); >+ >+ status = security_descriptor_dacl_add(sd, &ace); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ /* security_descriptor_dacl_add adds to the *end* of >+ the ace array, we need it at the start. Swap.. */ >+ ace = sd->dacl->aces[0]; >+ sd->dacl->aces[0] = sd->dacl->aces[sd->dacl->num_aces-1]; >+ sd->dacl->aces[sd->dacl->num_aces-1] = ace; >+ >+ ZERO_STRUCT(set); >+ set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC; >+ set.set_secdesc.in.file.fnum = fnum; >+ set.set_secdesc.in.secinfo_flags = SECINFO_DACL; >+ set.set_secdesc.in.sd = sd; >+ >+ status = smb_raw_setfileinfo(cli->tree, &set); >+ CHECK_STATUS(status, NT_STATUS_OK); >+ >+ smbcli_close(cli->tree, fnum); >+ fnum = -1; >+ >+ /* Try and open the stream name for WRITE_DATA. Should >+ fail with ACCESS_DENIED. */ >+ >+ ZERO_STRUCT(io); >+ io.generic.level = RAW_OPEN_NTCREATEX; >+ io.ntcreatex.in.root_fid.fnum = 0; >+ io.ntcreatex.in.flags = 0; >+ io.ntcreatex.in.access_mask = SEC_FILE_WRITE_DATA; >+ io.ntcreatex.in.create_options = 0; >+ io.ntcreatex.in.file_attr = 0; >+ io.ntcreatex.in.share_access = 0; >+ io.ntcreatex.in.alloc_size = 0; >+ io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN; >+ io.ntcreatex.in.impersonation = NTCREATEX_IMPERSONATION_ANONYMOUS; >+ io.ntcreatex.in.security_flags = 0; >+ io.ntcreatex.in.fname = fname_stream; >+ >+ status = smb_raw_open(cli->tree, tctx, &io); >+ CHECK_STATUS(status, NT_STATUS_ACCESS_DENIED); >+ >+ done: >+ >+ if (fnum != -1) { >+ smbcli_close(cli->tree, fnum); >+ } >+ smbcli_unlink(cli->tree, fname); >+ >+ smbcli_deltree(cli->tree, BASEDIR); >+ return ret; >+} >+ > /* > basic testing of streams calls > */ >@@ -1905,6 +2085,7 @@ struct torture_suite *torture_raw_streams(TALLOC_CTX *tctx) > test_stream_create_disposition); > torture_suite_add_1smb_test(suite, "attr", test_stream_attributes); > torture_suite_add_1smb_test(suite, "sumtab", test_stream_summary_tab); >+ torture_suite_add_1smb_test(suite, "perms", test_stream_permissions); > > #if 0 > torture_suite_add_1smb_test(suite, "LARGESTREAMINFO", >-- >1.8.1.2 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
metze
:
review+
Actions:
View
Attachments on
bug 10229
:
9351
|
9352
|
9381
| 9382 |
9397