From 4331de44227d655cb68a230ac4a0c3afcf629705 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 21 Jul 2017 09:56:45 -0700 Subject: [PATCH 1/4] s3: libsmbclient: Fix cli_setpathinfo_basic() to treat mode == -1 as no change. This is only called from SMBC_setatr(), so bring it into line with the specification for that function. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12913 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme (cherry picked from commit 812006fa8f26004609901b0ddef1c3ed05eff35e) --- source3/libsmb/clirap.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/source3/libsmb/clirap.c b/source3/libsmb/clirap.c index dd2c30e2258..f897df63053 100644 --- a/source3/libsmb/clirap.c +++ b/source3/libsmb/clirap.c @@ -732,8 +732,17 @@ NTSTATUS cli_setpathinfo_basic(struct cli_state *cli, const char *fname, put_long_date(p, change_time); p += 8; - /* Add attributes */ - SIVAL(p, 0, mode); + if (mode == (uint16_t)-1 || mode == FILE_ATTRIBUTE_NORMAL) { + /* No change. */ + mode = 0; + } else if (mode == 0) { + /* Clear all existing attributes. */ + mode = FILE_ATTRIBUTE_NORMAL; + } + + /* Add attributes */ + SIVAL(p, 0, mode); + p += 4; /* Add padding */ -- 2.14.0.rc0.400.g1c36432dff-goog From af206c2877b8d2808cd47d61af56d5afedbe258d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 21 Jul 2017 12:41:11 -0700 Subject: [PATCH 2/4] s3: libsmb: Add cli_smb2_setpathinfo(), to be called by cli_setpathinfo_basic(). Fix to prevent libsmbclient from accidently making SMB1 calls inside an SMB2 connection. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12913 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme (cherry picked from commit 2a15c70603bb23a68a2e3de0b00bfd98508f78e0) --- source3/libsmb/cli_smb2_fnum.c | 70 ++++++++++++++++++++++++++++++++++++++++++ source3/libsmb/cli_smb2_fnum.h | 5 +++ source3/libsmb/clirap.c | 14 +++++++++ 3 files changed, 89 insertions(+) diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c index 52c6960dd6f..6cdaace76aa 100644 --- a/source3/libsmb/cli_smb2_fnum.c +++ b/source3/libsmb/cli_smb2_fnum.c @@ -1501,6 +1501,75 @@ NTSTATUS cli_smb2_qpathinfo_streams(struct cli_state *cli, return status; } +/*************************************************************** + Wrapper that allows SMB2 to set SMB_FILE_BASIC_INFORMATION on + a pathname. + Synchronous only. +***************************************************************/ + +NTSTATUS cli_smb2_setpathinfo(struct cli_state *cli, + const char *name, + uint8_t in_info_type, + uint8_t in_file_info_class, + const DATA_BLOB *p_in_data) +{ + NTSTATUS status; + uint16_t fnum = 0xffff; + struct smb2_hnd *ph = NULL; + TALLOC_CTX *frame = talloc_stackframe(); + + if (smbXcli_conn_has_async_calls(cli->conn)) { + /* + * Can't use sync call while an async call is in flight + */ + status = NT_STATUS_INVALID_PARAMETER; + goto fail; + } + + if (smbXcli_conn_protocol(cli->conn) < PROTOCOL_SMB2_02) { + status = NT_STATUS_INVALID_PARAMETER; + goto fail; + } + + status = get_fnum_from_path(cli, + name, + FILE_WRITE_ATTRIBUTES, + &fnum); + + if (!NT_STATUS_IS_OK(status)) { + goto fail; + } + + status = map_fnum_to_smb2_handle(cli, + fnum, + &ph); + if (!NT_STATUS_IS_OK(status)) { + goto fail; + } + + status = smb2cli_set_info(cli->conn, + cli->timeout, + cli->smb2.session, + cli->smb2.tcon, + in_info_type, + in_file_info_class, + p_in_data, /* in_input_buffer */ + 0, /* in_additional_info */ + ph->fid_persistent, + ph->fid_volatile); + fail: + + if (fnum != 0xffff) { + cli_smb2_close_fnum(cli, fnum); + } + + cli->raw_status = status; + + TALLOC_FREE(frame); + return status; +} + + /*************************************************************** Wrapper that allows SMB2 to set pathname attributes. Synchronous only. @@ -1606,6 +1675,7 @@ NTSTATUS cli_smb2_setatr(struct cli_state *cli, return status; } + /*************************************************************** Wrapper that allows SMB2 to set file handle times. Synchronous only. diff --git a/source3/libsmb/cli_smb2_fnum.h b/source3/libsmb/cli_smb2_fnum.h index d8a517c30ee..435a5c28db3 100644 --- a/source3/libsmb/cli_smb2_fnum.h +++ b/source3/libsmb/cli_smb2_fnum.h @@ -107,6 +107,11 @@ NTSTATUS cli_smb2_qpathinfo_streams(struct cli_state *cli, TALLOC_CTX *mem_ctx, unsigned int *pnum_streams, struct stream_struct **pstreams); +NTSTATUS cli_smb2_setpathinfo(struct cli_state *cli, + const char *name, + uint8_t in_info_type, + uint8_t in_file_info_class, + const DATA_BLOB *p_in_data); NTSTATUS cli_smb2_setatr(struct cli_state *cli, const char *fname, uint16_t attr, diff --git a/source3/libsmb/clirap.c b/source3/libsmb/clirap.c index f897df63053..e80dfc92a77 100644 --- a/source3/libsmb/clirap.c +++ b/source3/libsmb/clirap.c @@ -29,6 +29,7 @@ #include "libsmb/clirap.h" #include "trans2.h" #include "../libcli/smb/smbXcli_base.h" +#include "cli_smb2_fnum.h" #define PIPE_LANMAN "\\PIPE\\LANMAN" @@ -751,6 +752,19 @@ NTSTATUS cli_setpathinfo_basic(struct cli_state *cli, const char *fname, data_len = PTR_DIFF(p, data); + if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) { + DATA_BLOB in_data = data_blob_const(data, data_len); + /* + * Split out SMB2 here as we need to select + * the correct info type and level. + */ + return cli_smb2_setpathinfo(cli, + fname, + 1, /* SMB2_SETINFO_FILE */ + SMB_FILE_BASIC_INFORMATION - 1000, + &in_data); + } + return cli_setpathinfo(cli, SMB_FILE_BASIC_INFORMATION, fname, (uint8_t *)data, data_len); } -- 2.14.0.rc0.400.g1c36432dff-goog From 844e4a5af8b6224c8192ff94d27f830f3a203664 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 21 Jul 2017 12:46:23 -0700 Subject: [PATCH 3/4] s3: libsmb: Implement cli_smb2_setatr() by calling cli_smb2_setpathinfo(). This removes duplicate code paths and ensures we have only one function calling the underlying smb2cli_set_info() for setting info levels by path. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12913 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme (cherry picked from commit b1e5b894b089433e59c96915a27559d179bdb6c5) --- source3/libsmb/cli_smb2_fnum.c | 55 ++++-------------------------------------- 1 file changed, 5 insertions(+), 50 deletions(-) diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c index 6cdaace76aa..fc99be23d03 100644 --- a/source3/libsmb/cli_smb2_fnum.c +++ b/source3/libsmb/cli_smb2_fnum.c @@ -1580,41 +1580,8 @@ NTSTATUS cli_smb2_setatr(struct cli_state *cli, uint16_t attr, time_t mtime) { - NTSTATUS status; - uint16_t fnum = 0xffff; - struct smb2_hnd *ph = NULL; uint8_t inbuf_store[40]; DATA_BLOB inbuf = data_blob_null; - TALLOC_CTX *frame = talloc_stackframe(); - - if (smbXcli_conn_has_async_calls(cli->conn)) { - /* - * Can't use sync call while an async call is in flight - */ - status = NT_STATUS_INVALID_PARAMETER; - goto fail; - } - - if (smbXcli_conn_protocol(cli->conn) < PROTOCOL_SMB2_02) { - status = NT_STATUS_INVALID_PARAMETER; - goto fail; - } - - status = get_fnum_from_path(cli, - name, - FILE_WRITE_ATTRIBUTES, - &fnum); - - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } - - status = map_fnum_to_smb2_handle(cli, - fnum, - &ph); - if (!NT_STATUS_IS_OK(status)) { - goto fail; - } /* setinfo on the handle with info_type SMB2_SETINFO_FILE (1), level 4 (SMB_FILE_BASIC_INFORMATION - 1000). */ @@ -1655,24 +1622,12 @@ NTSTATUS cli_smb2_setatr(struct cli_state *cli, SBVAL(inbuf.data, 8, 0xFFFFFFFFFFFFFFFFLL); SBVAL(inbuf.data, 24, 0xFFFFFFFFFFFFFFFFLL); - status = smb2cli_set_info(cli->conn, - cli->timeout, - cli->smb2.session, - cli->smb2.tcon, + return cli_smb2_setpathinfo(cli, + name, 1, /* in_info_type */ - SMB_FILE_BASIC_INFORMATION - 1000, /* in_file_info_class */ - &inbuf, /* in_input_buffer */ - 0, /* in_additional_info */ - ph->fid_persistent, - ph->fid_volatile); - fail: - - if (fnum != 0xffff) { - cli_smb2_close_fnum(cli, fnum); - } - - TALLOC_FREE(frame); - return status; + /* in_file_info_class */ + SMB_FILE_BASIC_INFORMATION - 1000, + &inbuf); } -- 2.14.0.rc0.400.g1c36432dff-goog From 4677643aca0cddec56c39582659eef9cb5cd4faf Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 21 Jul 2017 15:11:08 -0700 Subject: [PATCH 4/4] s3: torture: Add a test for cli_setpathinfo_basic() to smbtorture3. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12913 Signed-off-by: Jeremy Allison Reviewed-by: Ralph Boehme (cherry picked from commit bfa07323590357542eb06ad5faa2dc5a5736e3f1) --- source3/torture/torture.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/source3/torture/torture.c b/source3/torture/torture.c index 1a1a23a7570..c4ca8e86101 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -3158,6 +3158,29 @@ static bool run_browsetest(int dummy) } +static bool check_attributes(struct cli_state *cli, + const char *fname, + uint16_t expected_attrs) +{ + uint16_t attrs = 0; + NTSTATUS status = cli_getatr(cli, + fname, + &attrs, + NULL, + NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_getatr failed with %s\n", + nt_errstr(status)); + return false; + } + if (attrs != expected_attrs) { + printf("Attributes incorrect 0x%x, should be 0x%x\n", + (unsigned int)attrs, + (unsigned int)expected_attrs); + return false; + } + return true; +} /* This checks how the getatr calls works @@ -3218,6 +3241,120 @@ static bool run_attrtest(int dummy) cli_unlink(cli, fname, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); + /* Check cli_setpathinfo_basic() */ + /* Re-create the file. */ + status = cli_openx(cli, fname, + O_RDWR | O_CREAT | O_TRUNC, DENY_NONE, &fnum); + if (!NT_STATUS_IS_OK(status)) { + printf("Failed to recreate %s (%s)\n", + fname, nt_errstr(status)); + correct = false; + } + cli_close(cli, fnum); + + status = cli_setpathinfo_basic(cli, + fname, + 0, /* create */ + 0, /* access */ + 0, /* write */ + 0, /* change */ + FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN | + FILE_ATTRIBUTE_READONLY); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_setpathinfo_basic failed with %s\n", + nt_errstr(status)); + correct = false; + } + + /* Check attributes are correct. */ + correct = check_attributes(cli, + fname, + FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN | + FILE_ATTRIBUTE_READONLY); + if (correct == false) { + goto out; + } + + /* Setting to FILE_ATTRIBUTE_NORMAL should be ignored. */ + status = cli_setpathinfo_basic(cli, + fname, + 0, /* create */ + 0, /* access */ + 0, /* write */ + 0, /* change */ + FILE_ATTRIBUTE_NORMAL); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_setpathinfo_basic failed with %s\n", + nt_errstr(status)); + correct = false; + } + + /* Check attributes are correct. */ + correct = check_attributes(cli, + fname, + FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN | + FILE_ATTRIBUTE_READONLY); + if (correct == false) { + goto out; + } + + /* Setting to (uint16_t)-1 should also be ignored. */ + status = cli_setpathinfo_basic(cli, + fname, + 0, /* create */ + 0, /* access */ + 0, /* write */ + 0, /* change */ + (uint16_t)-1); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_setpathinfo_basic failed with %s\n", + nt_errstr(status)); + correct = false; + } + + /* Check attributes are correct. */ + correct = check_attributes(cli, + fname, + FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN | + FILE_ATTRIBUTE_READONLY); + if (correct == false) { + goto out; + } + + /* Setting to 0 should clear them all. */ + status = cli_setpathinfo_basic(cli, + fname, + 0, /* create */ + 0, /* access */ + 0, /* write */ + 0, /* change */ + 0); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_setpathinfo_basic failed with %s\n", + nt_errstr(status)); + correct = false; + } + + /* Check attributes are correct. */ + correct = check_attributes(cli, + fname, + FILE_ATTRIBUTE_NORMAL); + if (correct == false) { + goto out; + } + + out: + + cli_unlink(cli, + fname, + FILE_ATTRIBUTE_SYSTEM | + FILE_ATTRIBUTE_HIDDEN| + FILE_ATTRIBUTE_READONLY); + if (!torture_close_connection(cli)) { correct = False; } -- 2.14.0.rc0.400.g1c36432dff-goog