From 7bc0449cfeb65bbe3081176791082eaf837b31f3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 28 Mar 2018 12:42:20 -0700 Subject: [PATCH 1/6] s3: smbd: Files or directories can't be opened DELETE_ON_CLOSE without delete access. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13358 Signed-off-by: Jeremy Allison --- source3/smbd/open.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index be9e601bb15..643e074f399 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -5120,6 +5120,18 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, goto fail; } + /* + * Files or directories can't be opened DELETE_ON_CLOSE without + * delete access. + * BUG: https://bugzilla.samba.org/show_bug.cgi?id=13358 + */ + if (create_options & FILE_DELETE_ON_CLOSE) { + if ((access_mask & DELETE_ACCESS) == 0) { + status = NT_STATUS_INVALID_PARAMETER; + goto fail; + } + } + if ((conn->fs_capabilities & FILE_NAMED_STREAMS) && is_ntfs_stream_smb_fname(smb_fname) && (!(private_flags & NTCREATEX_OPTIONS_PRIVATE_STREAM_DELETE))) { -- 2.17.0.rc1.321.gba9d0f2565-goog From f2b8517fe1bd1d46f006173da5e4c4869a874006 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 28 Mar 2018 10:54:30 -0700 Subject: [PATCH 2/6] s4: torture: Ensure a failed file create doesn't create the file. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13358 Signed-off-by: Jeremy Allison --- source4/torture/basic/delete.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/source4/torture/basic/delete.c b/source4/torture/basic/delete.c index 54815b95512..bc23b5fd1d0 100644 --- a/source4/torture/basic/delete.c +++ b/source4/torture/basic/delete.c @@ -476,10 +476,18 @@ static bool deltest8(struct torture_context *tctx, struct smbcli_state *cli1, st static bool deltest9(struct torture_context *tctx, struct smbcli_state *cli1, struct smbcli_state *cli2) { int fnum1 = -1; + NTSTATUS status; del_clean_area(cli1, cli2); /* This should fail - we need to set DELETE_ACCESS. */ + + /* + * A file or directory create with DELETE_ON_CLOSE but + * without DELETE_ACCESS should fail with + * NT_STATUS_INVALID_PARAMETER. + */ + fnum1 = smbcli_nt_create_full(cli1->tree, fname, 0, SEC_FILE_READ_DATA|SEC_FILE_WRITE_DATA, FILE_ATTRIBUTE_NORMAL, @@ -491,6 +499,25 @@ static bool deltest9(struct torture_context *tctx, struct smbcli_state *cli1, st talloc_asprintf(tctx, "open of %s succeeded should have failed!", fname)); + /* Must fail with NT_STATUS_INVALID_PARAMETER. */ + status = smbcli_nt_error(cli1->tree); + torture_assert_ntstatus_equal(tctx, + status, + NT_STATUS_INVALID_PARAMETER, + talloc_asprintf(tctx, "create of %s should return " + "NT_STATUS_INVALID_PARAMETER, got %s", + fname, + smbcli_errstr(cli1->tree))); + + /* This should fail - the file should not have been created. */ + status = smbcli_getatr(cli1->tree, fname, NULL, NULL, NULL); + torture_assert_ntstatus_equal(tctx, + status, + NT_STATUS_OBJECT_NAME_NOT_FOUND, + talloc_asprintf(tctx, "getattr of %s succeeded should " + "not have been created !", + fname)); + return true; } -- 2.17.0.rc1.321.gba9d0f2565-goog From 5cda9e28a017e2a5156df7126c1cec3f240a0131 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 28 Mar 2018 11:00:59 -0700 Subject: [PATCH 3/6] s4: torture: Test all combinations of file create to ensure behavior is the same. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13358 Signed-off-by: Jeremy Allison --- source4/torture/basic/delete.c | 57 ++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/source4/torture/basic/delete.c b/source4/torture/basic/delete.c index bc23b5fd1d0..c56b5c834da 100644 --- a/source4/torture/basic/delete.c +++ b/source4/torture/basic/delete.c @@ -477,31 +477,39 @@ static bool deltest9(struct torture_context *tctx, struct smbcli_state *cli1, st { int fnum1 = -1; NTSTATUS status; + uint32_t disps[4] = { + NTCREATEX_DISP_SUPERSEDE, + NTCREATEX_DISP_OVERWRITE_IF, + NTCREATEX_DISP_CREATE, + NTCREATEX_DISP_OPEN_IF}; + unsigned int i; del_clean_area(cli1, cli2); - /* This should fail - we need to set DELETE_ACCESS. */ - - /* - * A file or directory create with DELETE_ON_CLOSE but - * without DELETE_ACCESS should fail with - * NT_STATUS_INVALID_PARAMETER. - */ - - fnum1 = smbcli_nt_create_full(cli1->tree, fname, 0, - SEC_FILE_READ_DATA|SEC_FILE_WRITE_DATA, - FILE_ATTRIBUTE_NORMAL, - NTCREATEX_SHARE_ACCESS_NONE, - NTCREATEX_DISP_OVERWRITE_IF, - NTCREATEX_OPTIONS_DELETE_ON_CLOSE, 0); - - torture_assert(tctx, fnum1 == -1, - talloc_asprintf(tctx, "open of %s succeeded should have failed!", - fname)); + for (i = 0; i < sizeof(disps)/sizeof(disps[0]); i++) { + /* This should fail - we need to set DELETE_ACCESS. */ + + /* + * A file or directory create with DELETE_ON_CLOSE but + * without DELETE_ACCESS should fail with + * NT_STATUS_INVALID_PARAMETER. + */ + + fnum1 = smbcli_nt_create_full(cli1->tree, fname, 0, + SEC_FILE_READ_DATA|SEC_FILE_WRITE_DATA, + FILE_ATTRIBUTE_NORMAL, + NTCREATEX_SHARE_ACCESS_NONE, + disps[i], + NTCREATEX_OPTIONS_DELETE_ON_CLOSE, 0); + + torture_assert(tctx, fnum1 == -1, + talloc_asprintf(tctx, "open of %s succeeded " + "should have failed!", + fname)); - /* Must fail with NT_STATUS_INVALID_PARAMETER. */ - status = smbcli_nt_error(cli1->tree); - torture_assert_ntstatus_equal(tctx, + /* Must fail with NT_STATUS_INVALID_PARAMETER. */ + status = smbcli_nt_error(cli1->tree); + torture_assert_ntstatus_equal(tctx, status, NT_STATUS_INVALID_PARAMETER, talloc_asprintf(tctx, "create of %s should return " @@ -509,14 +517,15 @@ static bool deltest9(struct torture_context *tctx, struct smbcli_state *cli1, st fname, smbcli_errstr(cli1->tree))); - /* This should fail - the file should not have been created. */ - status = smbcli_getatr(cli1->tree, fname, NULL, NULL, NULL); - torture_assert_ntstatus_equal(tctx, + /* This should fail - the file should not have been created. */ + status = smbcli_getatr(cli1->tree, fname, NULL, NULL, NULL); + torture_assert_ntstatus_equal(tctx, status, NT_STATUS_OBJECT_NAME_NOT_FOUND, talloc_asprintf(tctx, "getattr of %s succeeded should " "not have been created !", fname)); + } return true; } -- 2.17.0.rc1.321.gba9d0f2565-goog From 616c9a025bf89dec911b8bf078b4f847758f2086 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 28 Mar 2018 11:44:40 -0700 Subject: [PATCH 4/6] s4: torture: Test all combinations of file open with existing file to ensure behavior is the same. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13358 Signed-off-by: Jeremy Allison --- source4/torture/basic/delete.c | 67 ++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/source4/torture/basic/delete.c b/source4/torture/basic/delete.c index c56b5c834da..4500d9500e8 100644 --- a/source4/torture/basic/delete.c +++ b/source4/torture/basic/delete.c @@ -530,6 +530,72 @@ static bool deltest9(struct torture_context *tctx, struct smbcli_state *cli1, st return true; } +/* Test 9a ... */ +static bool deltest9a(struct torture_context *tctx, + struct smbcli_state *cli1, + struct smbcli_state *cli2) +{ + int fnum1 = -1; + NTSTATUS status; + uint32_t disps[4] = { + NTCREATEX_DISP_OVERWRITE_IF, + NTCREATEX_DISP_OPEN, + NTCREATEX_DISP_OVERWRITE, + NTCREATEX_DISP_OPEN_IF}; + + unsigned int i; + + del_clean_area(cli1, cli2); + + /* Create the file, and try with open calls. */ + fnum1 = smbcli_open(cli1->tree, fname, O_CREAT|O_RDWR, DENY_NONE); + torture_assert(tctx, + fnum1 != -1, + talloc_asprintf(tctx, "open of %s failed (%s)", + fname, + smbcli_errstr(cli1->tree))); + status = smbcli_close(cli1->tree, fnum1); + torture_assert_ntstatus_ok(tctx, + status, + talloc_asprintf(tctx, "close failed")); + + for (i = 0; i < sizeof(disps)/sizeof(disps[0]); i++) { + fnum1 = smbcli_nt_create_full(cli1->tree, fname, 0, + SEC_FILE_READ_DATA|SEC_FILE_WRITE_DATA, + FILE_ATTRIBUTE_NORMAL, + NTCREATEX_SHARE_ACCESS_NONE, + disps[i], + NTCREATEX_OPTIONS_DELETE_ON_CLOSE, 0); + + torture_assert(tctx, fnum1 == -1, + talloc_asprintf(tctx, "open of %s succeeded " + "should have failed!", + fname)); + + /* Must fail with NT_STATUS_INVALID_PARAMETER. */ + status = smbcli_nt_error(cli1->tree); + torture_assert_ntstatus_equal(tctx, + status, + NT_STATUS_INVALID_PARAMETER, + talloc_asprintf(tctx, "create of %s should return " + "NT_STATUS_INVALID_PARAMETER, got %s", + fname, + smbcli_errstr(cli1->tree))); + + /* + * This should succeed - the file should not have been deleted. + */ + status = smbcli_getatr(cli1->tree, fname, NULL, NULL, NULL); + torture_assert_ntstatus_ok(tctx, + status, + talloc_asprintf(tctx, "getattr of %s failed %s", + fname, + smbcli_errstr(cli1->tree))); + } + + return true; +} + /* Test 10 ... */ static bool deltest10(struct torture_context *tctx, struct smbcli_state *cli1, struct smbcli_state *cli2) { @@ -2305,6 +2371,7 @@ struct torture_suite *torture_test_delete(TALLOC_CTX *ctx) torture_suite_add_2smb_test(suite, "deltest7", deltest7); torture_suite_add_2smb_test(suite, "deltest8", deltest8); torture_suite_add_2smb_test(suite, "deltest9", deltest9); + torture_suite_add_2smb_test(suite, "deltest9a", deltest9a); torture_suite_add_2smb_test(suite, "deltest10", deltest10); torture_suite_add_2smb_test(suite, "deltest11", deltest11); torture_suite_add_2smb_test(suite, "deltest12", deltest12); -- 2.17.0.rc1.321.gba9d0f2565-goog From b918ac3f637ba3030997e10ace4f70263514773b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 28 Mar 2018 13:17:14 -0700 Subject: [PATCH 5/6] s4: torture: Test all combinations of directory create to ensure behavior is the same. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13358 Signed-off-by: Jeremy Allison --- source4/torture/basic/delete.c | 65 ++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/source4/torture/basic/delete.c b/source4/torture/basic/delete.c index 4500d9500e8..e48efaa481d 100644 --- a/source4/torture/basic/delete.c +++ b/source4/torture/basic/delete.c @@ -2354,6 +2354,70 @@ static bool deltest24(struct torture_context *tctx) return correct; } +/* Test 25 ... */ +static bool deltest25(struct torture_context *tctx, + struct smbcli_state *cli1, + struct smbcli_state *cli2) +{ + int fnum1 = -1; + NTSTATUS status; + uint32_t disps[4] = { + NTCREATEX_DISP_SUPERSEDE, + NTCREATEX_DISP_OVERWRITE_IF, + NTCREATEX_DISP_CREATE, + NTCREATEX_DISP_OPEN_IF}; + unsigned int i; + + del_clean_area(cli1, cli2); + + for (i = 0; i < sizeof(disps)/sizeof(disps[0]); i++) { + /* This should fail - we need to set DELETE_ACCESS. */ + + /* + * A file or directory create with DELETE_ON_CLOSE but + * without DELETE_ACCESS should fail with + * NT_STATUS_INVALID_PARAMETER. + */ + + fnum1 = smbcli_nt_create_full(cli1->tree, dname, 0, + SEC_FILE_READ_DATA, + FILE_ATTRIBUTE_DIRECTORY, + NTCREATEX_SHARE_ACCESS_NONE, + disps[i], + NTCREATEX_OPTIONS_DIRECTORY| + NTCREATEX_OPTIONS_DELETE_ON_CLOSE, 0); + + torture_assert(tctx, fnum1 == -1, + talloc_asprintf(tctx, "open of %s succeeded " + "should have failed!", + dname)); + + /* Must fail with NT_STATUS_INVALID_PARAMETER. */ + status = smbcli_nt_error(cli1->tree); + torture_assert_ntstatus_equal(tctx, + status, + NT_STATUS_INVALID_PARAMETER, + talloc_asprintf(tctx, "create of %s should return " + "NT_STATUS_INVALID_PARAMETER, got %s", + dname, + smbcli_errstr(cli1->tree))); + + /* + * This should fail - the directory + * should not have been created. + */ + status = smbcli_getatr(cli1->tree, dname, NULL, NULL, NULL); + torture_assert_ntstatus_equal(tctx, + status, + NT_STATUS_OBJECT_NAME_NOT_FOUND, + talloc_asprintf(tctx, "getattr of %s succeeded should " + "not have been created !", + dname)); + } + + return true; +} + /* Test delete on close semantics. */ @@ -2396,6 +2460,7 @@ struct torture_suite *torture_test_delete(TALLOC_CTX *ctx) torture_suite_add_simple_test(suite, "deltest22", deltest22); torture_suite_add_2smb_test(suite, "deltest23", deltest23); torture_suite_add_simple_test(suite, "deltest24", deltest24); + torture_suite_add_2smb_test(suite, "deltest25", deltest25); return suite; } -- 2.17.0.rc1.321.gba9d0f2565-goog From 6235bf76704845d9d9a5301de2537a882d634eb1 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 28 Mar 2018 13:19:12 -0700 Subject: [PATCH 6/6] s4: torture: Test all combinations of directory open with existing directory to ensure behavior is the same. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13358 Signed-off-by: Jeremy Allison --- source4/torture/basic/delete.c | 65 ++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/source4/torture/basic/delete.c b/source4/torture/basic/delete.c index e48efaa481d..0472ee5b27e 100644 --- a/source4/torture/basic/delete.c +++ b/source4/torture/basic/delete.c @@ -2418,6 +2418,70 @@ static bool deltest25(struct torture_context *tctx, return true; } +/* Test 25a... */ +static bool deltest25a(struct torture_context *tctx, + struct smbcli_state *cli1, + struct smbcli_state *cli2) +{ + int fnum1 = -1; + NTSTATUS status; + uint32_t disps[4] = { + NTCREATEX_DISP_OVERWRITE_IF, + NTCREATEX_DISP_OPEN, + NTCREATEX_DISP_OVERWRITE, + NTCREATEX_DISP_OPEN_IF}; + + unsigned int i; + + del_clean_area(cli1, cli2); + + /* Create the directory, and try with open calls. */ + status = smbcli_mkdir(cli1->tree, dname); + torture_assert_ntstatus_ok(tctx, + status, + talloc_asprintf(tctx, "mkdir of %s failed %s", + dname, + smbcli_errstr(cli1->tree))); + + for (i = 0; i < sizeof(disps)/sizeof(disps[0]); i++) { + fnum1 = smbcli_nt_create_full(cli1->tree, dname, 0, + SEC_FILE_READ_DATA, + FILE_ATTRIBUTE_DIRECTORY, + NTCREATEX_SHARE_ACCESS_NONE, + disps[i], + NTCREATEX_OPTIONS_DIRECTORY| + NTCREATEX_OPTIONS_DELETE_ON_CLOSE, 0); + + torture_assert(tctx, fnum1 == -1, + talloc_asprintf(tctx, "open of %s succeeded " + "should have failed!", + dname)); + + /* Must fail with NT_STATUS_INVALID_PARAMETER. */ + status = smbcli_nt_error(cli1->tree); + torture_assert_ntstatus_equal(tctx, + status, + NT_STATUS_INVALID_PARAMETER, + talloc_asprintf(tctx, "create of %s should return " + "NT_STATUS_INVALID_PARAMETER, got %s", + dname, + smbcli_errstr(cli1->tree))); + + /* + * This should succeed - the directory + * should not have been deleted. + */ + status = smbcli_getatr(cli1->tree, dname, NULL, NULL, NULL); + torture_assert_ntstatus_ok(tctx, + status, + talloc_asprintf(tctx, "getattr of %s failed %s", + fname, + smbcli_errstr(cli1->tree))); + } + + return true; +} + /* Test delete on close semantics. */ @@ -2461,6 +2525,7 @@ struct torture_suite *torture_test_delete(TALLOC_CTX *ctx) torture_suite_add_2smb_test(suite, "deltest23", deltest23); torture_suite_add_simple_test(suite, "deltest24", deltest24); torture_suite_add_2smb_test(suite, "deltest25", deltest25); + torture_suite_add_2smb_test(suite, "deltest25a", deltest25a); return suite; } -- 2.17.0.rc1.321.gba9d0f2565-goog