From 1df75c0bf8155f6367814817edf1a40d1b95f05e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 10 May 2018 10:26:52 -0700 Subject: [PATCH 1/2] s3: smbd: Fix SMB2-FLUSH against directories. Directories opened with either FILE_ADD_FILE or FILE_ADD_SUBDIRECTORY can be flushed even if they're not writable in the conventional sense. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13428 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme (cherry picked from commit 42aadf42f27053e621f2a6b72448afebb3f5082a) --- source3/smbd/smb2_flush.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/source3/smbd/smb2_flush.c b/source3/smbd/smb2_flush.c index d1ab3a09839..ef9b7fddcf9 100644 --- a/source3/smbd/smb2_flush.c +++ b/source3/smbd/smb2_flush.c @@ -23,6 +23,7 @@ #include "smbd/globals.h" #include "../libcli/smb/smb_common.h" #include "../lib/util/tevent_ntstatus.h" +#include "libcli/security/security.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_SMB2 @@ -147,8 +148,29 @@ static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx, } if (!CHECK_WRITE(fsp)) { - tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); - return tevent_req_post(req, ev); + bool allow_dir_flush = false; + uint32_t flush_access = FILE_ADD_FILE | FILE_ADD_SUBDIRECTORY; + + if (!fsp->is_directory) { + tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); + return tevent_req_post(req, ev); + } + + /* + * Directories are not writable in the conventional + * sense, but if opened with *either* + * FILE_ADD_FILE or FILE_ADD_SUBDIRECTORY + * they can be flushed. + */ + + if ((fsp->access_mask & flush_access) != 0) { + allow_dir_flush = true; + } + + if (allow_dir_flush == false) { + tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); + return tevent_req_post(req, ev); + } } if (fsp->fh->fd == -1) { -- 2.17.0.441.gb46fe60e1d-goog From eb2b352ae136102e6c6ab33b6d021af54364b014 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 10 May 2018 11:30:24 -0700 Subject: [PATCH 2/2] s3: smbtorture: Add new SMB2-DIR-FSYNC test to show behavior of FSYNC on directories. Tests against a directory handle on the root of a share, and a directory handle on a sub-directory in a share. Check SEC_DIR_ADD_FILE and SEC_DIR_ADD_SUBDIR separately, either allows flush to succeed. Passes against Windows. Regression test for: BUG: https://bugzilla.samba.org/show_bug.cgi?id=13428 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri May 18 02:38:50 CEST 2018 on sn-devel-144 (cherry picked from commit d42f467a25e75e5487a00378609a24809ddc83ee) --- selftest/knownfail | 1 + source3/selftest/tests.py | 2 +- source3/torture/proto.h | 1 + source3/torture/test_smb2.c | 270 ++++++++++++++++++++++++++++++++++++ source3/torture/torture.c | 1 + 5 files changed, 274 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail b/selftest/knownfail index dd23c7d204f..267c928955e 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -8,6 +8,7 @@ .*driver.add_driver_timestamps # we only can store dates, not timestamps ^samba3.smbtorture_s3.crypt_server\(nt4_dc\).SMB2-SESSION-REAUTH # expected to give ACCESS_DENIED SMB2.1 doesn't have encryption ^samba3.smbtorture_s3.crypt_server\(nt4_dc\).SMB2-SESSION-RECONNECT # expected to give CONNECTION_DISCONNECTED, we need to fix the test +^samba3.smbtorture_s3.*ad_dc_ntvfs.*SMB2-DIR-FSYNC.* ^samba3.smb2.session enc.reconnect # expected to give CONNECTION_DISCONNECTED, we need to fix the test ^samba3.raw.session enc # expected to give ACCESS_DENIED as SMB1 encryption isn't used ^samba3.smbtorture_s3.crypt_server # expected to give ACCESS_DENIED as SMB1 encryption isn't used diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 20e1c03fa66..c2a4014b289 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -78,7 +78,7 @@ tests = ["FDPASS", "LOCK1", "LOCK2", "LOCK3", "LOCK4", "LOCK5", "LOCK6", "LOCK7" "GETADDRINFO", "UID-REGRESSION-TEST", "SHORTNAME-TEST", "CASE-INSENSITIVE-CREATE", "SMB2-BASIC", "NTTRANS-FSCTL", "SMB2-NEGPROT", "SMB2-SESSION-REAUTH", "SMB2-SESSION-RECONNECT", "SMB2-FTRUNCATE", - "SMB2-ANONYMOUS", + "SMB2-ANONYMOUS", "SMB2-DIR-FSYNC", "CLEANUP1", "CLEANUP2", "CLEANUP4", diff --git a/source3/torture/proto.h b/source3/torture/proto.h index 6f12ff7c2b9..a8400382e10 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -101,6 +101,7 @@ bool run_smb2_tcon_dependence(int dummy); bool run_smb2_multi_channel(int dummy); bool run_smb2_session_reauth(int dummy); bool run_smb2_ftruncate(int dummy); +bool run_smb2_dir_fsync(int dummy); bool run_chain3(int dummy); bool run_local_conv_auth_info(int dummy); bool run_local_sprintf_append(int dummy); diff --git a/source3/torture/test_smb2.c b/source3/torture/test_smb2.c index 897d034f6a9..094a9b84d6e 100644 --- a/source3/torture/test_smb2.c +++ b/source3/torture/test_smb2.c @@ -2065,3 +2065,273 @@ bool run_smb2_ftruncate(int dummy) } return correct; } + +/* Ensure SMB2 flush on directories behaves correctly. */ + +static bool test_dir_fsync(struct cli_state *cli, const char *path) +{ + NTSTATUS status; + uint64_t fid_persistent, fid_volatile; + uint8_t *dir_data = NULL; + uint32_t dir_data_length = 0; + + /* Open directory - no write abilities. */ + status = smb2cli_create(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, path, + SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */ + SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */ + SEC_STD_SYNCHRONIZE| + SEC_DIR_LIST| + SEC_DIR_READ_ATTRIBUTE, /* desired_access, */ + 0, /* file_attributes, */ + FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */ + FILE_OPEN, /* create_disposition, */ + FILE_SYNCHRONOUS_IO_NONALERT|FILE_DIRECTORY_FILE, /* create_options, */ + NULL, /* smb2_create_blobs *blobs */ + &fid_persistent, + &fid_volatile, + NULL, NULL, NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_create '%s' (readonly) returned %s\n", + path, + nt_errstr(status)); + return false; + } + + status = smb2cli_query_directory( + cli->conn, cli->timeout, cli->smb2.session, cli->smb2.tcon, + 1, 0, 0, fid_persistent, fid_volatile, "*", 0xffff, + talloc_tos(), &dir_data, &dir_data_length); + + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_query_directory returned %s\n", + nt_errstr(status)); + return false; + } + + /* Open directory no write access. Flush should fail. */ + + status = smb2cli_flush(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, fid_persistent, fid_volatile); + if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + printf("smb2cli_flush on a read-only directory returned %s\n", + nt_errstr(status)); + return false; + } + + status = smb2cli_close(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, 0, fid_persistent, fid_volatile); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_close returned %s\n", nt_errstr(status)); + return false; + } + + /* Open directory write-attributes only. Flush should still fail. */ + + status = smb2cli_create(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, path, + SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */ + SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */ + SEC_STD_SYNCHRONIZE| + SEC_DIR_LIST| + SEC_DIR_WRITE_ATTRIBUTE| + SEC_DIR_READ_ATTRIBUTE, /* desired_access, */ + 0, /* file_attributes, */ + FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */ + FILE_OPEN, /* create_disposition, */ + FILE_SYNCHRONOUS_IO_NONALERT|FILE_DIRECTORY_FILE, /* create_options, */ + NULL, /* smb2_create_blobs *blobs */ + &fid_persistent, + &fid_volatile, + NULL, NULL, NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_create '%s' (write attr) returned %s\n", + path, + nt_errstr(status)); + return false; + } + + status = smb2cli_query_directory( + cli->conn, cli->timeout, cli->smb2.session, cli->smb2.tcon, + 1, 0, 0, fid_persistent, fid_volatile, "*", 0xffff, + talloc_tos(), &dir_data, &dir_data_length); + + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_query_directory returned %s\n", nt_errstr(status)); + return false; + } + + status = smb2cli_flush(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, fid_persistent, fid_volatile); + if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + printf("smb2cli_flush on a write-attributes directory " + "returned %s\n", + nt_errstr(status)); + return false; + } + + status = smb2cli_close(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, 0, fid_persistent, fid_volatile); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_close returned %s\n", nt_errstr(status)); + return false; + } + + /* Open directory with SEC_DIR_ADD_FILE access. Flush should now succeed. */ + + status = smb2cli_create(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, path, + SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */ + SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */ + SEC_STD_SYNCHRONIZE| + SEC_DIR_LIST| + SEC_DIR_ADD_FILE, /* desired_access, */ + 0, /* file_attributes, */ + FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */ + FILE_OPEN, /* create_disposition, */ + FILE_SYNCHRONOUS_IO_NONALERT|FILE_DIRECTORY_FILE, /* create_options, */ + NULL, /* smb2_create_blobs *blobs */ + &fid_persistent, + &fid_volatile, + NULL, NULL, NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_create '%s' (write FILE access) returned %s\n", + path, + nt_errstr(status)); + return false; + } + + status = smb2cli_query_directory( + cli->conn, cli->timeout, cli->smb2.session, cli->smb2.tcon, + 1, 0, 0, fid_persistent, fid_volatile, "*", 0xffff, + talloc_tos(), &dir_data, &dir_data_length); + + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_query_directory returned %s\n", nt_errstr(status)); + return false; + } + + status = smb2cli_flush(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, fid_persistent, fid_volatile); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_flush on a directory returned %s\n", + nt_errstr(status)); + return false; + } + + status = smb2cli_close(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, 0, fid_persistent, fid_volatile); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_close returned %s\n", nt_errstr(status)); + return false; + } + + /* Open directory with SEC_DIR_ADD_FILE access. Flush should now succeed. */ + + status = smb2cli_create(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, path, + SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */ + SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */ + SEC_STD_SYNCHRONIZE| + SEC_DIR_LIST| + SEC_DIR_ADD_SUBDIR, /* desired_access, */ + 0, /* file_attributes, */ + FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */ + FILE_OPEN, /* create_disposition, */ + FILE_SYNCHRONOUS_IO_NONALERT|FILE_DIRECTORY_FILE, /* create_options, */ + NULL, /* smb2_create_blobs *blobs */ + &fid_persistent, + &fid_volatile, + NULL, NULL, NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_create '%s' (write DIR access) returned %s\n", + path, + nt_errstr(status)); + return false; + } + + status = smb2cli_query_directory( + cli->conn, cli->timeout, cli->smb2.session, cli->smb2.tcon, + 1, 0, 0, fid_persistent, fid_volatile, "*", 0xffff, + talloc_tos(), &dir_data, &dir_data_length); + + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_query_directory returned %s\n", nt_errstr(status)); + return false; + } + + status = smb2cli_flush(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, fid_persistent, fid_volatile); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_flush on a directory returned %s\n", + nt_errstr(status)); + return false; + } + + status = smb2cli_close(cli->conn, cli->timeout, cli->smb2.session, + cli->smb2.tcon, 0, fid_persistent, fid_volatile); + if (!NT_STATUS_IS_OK(status)) { + printf("smb2cli_close returned %s\n", nt_errstr(status)); + return false; + } + + + return true; +} + +bool run_smb2_dir_fsync(int dummy) +{ + struct cli_state *cli = NULL; + NTSTATUS status; + bool bret = false; + const char *dname = "fsync_test_dir"; + + printf("Starting SMB2-DIR-FSYNC\n"); + + if (!torture_init_connection(&cli)) { + return false; + } + + status = smbXcli_negprot(cli->conn, cli->timeout, + PROTOCOL_SMB2_02, PROTOCOL_SMB2_02); + if (!NT_STATUS_IS_OK(status)) { + printf("smbXcli_negprot returned %s\n", nt_errstr(status)); + return false; + } + + status = cli_session_setup_creds(cli, torture_creds); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_session_setup returned %s\n", nt_errstr(status)); + return false; + } + + status = cli_tree_connect(cli, share, "?????", NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_tree_connect returned %s\n", nt_errstr(status)); + return false; + } + + (void)cli_rmdir(cli, dname); + status = cli_mkdir(cli, dname); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_mkdir(%s) returned %s\n", + dname, + nt_errstr(status)); + return false; + } + + /* Test on a subdirectory. */ + bret = test_dir_fsync(cli, dname); + if (bret == false) { + (void)cli_rmdir(cli, dname); + return false; + } + (void)cli_rmdir(cli, dname); + + /* Test on the root handle of a share. */ + bret = test_dir_fsync(cli, ""); + if (bret == false) { + return false; + } + return true; +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 1709e94289c..eb007063ee1 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -11650,6 +11650,7 @@ static struct { { "SMB2-MULTI-CHANNEL", run_smb2_multi_channel }, { "SMB2-SESSION-REAUTH", run_smb2_session_reauth }, { "SMB2-FTRUNCATE", run_smb2_ftruncate }, + { "SMB2-DIR-FSYNC", run_smb2_dir_fsync }, { "CLEANUP1", run_cleanup1 }, { "CLEANUP2", run_cleanup2 }, { "CLEANUP3", run_cleanup3 }, -- 2.17.0.441.gb46fe60e1d-goog