[PATCHES] smbclient - add support for superseding the destination file on rename

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCHES] smbclient - add support for superseding the destination file on rename

Samba - samba-technical mailing list
Hi,

This smbclient patch set supports asking the server to replace an
existing destination file when renaming. This is a feature of SMB2
protocol only - haven't found a parallel in SMB1.

Review appreciated,
Uri.

client-supersede.patch.txt (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHES] smbclient - add support for superseding the destination file on rename

Samba - samba-technical mailing list
Hi Uri,

> This smbclient patch set supports asking the server to replace an
> existing destination file when renaming. This is a feature of SMB2
> protocol only - haven't found a parallel in SMB1.

According to [MS-FSCC], there is a "2.4.34.1 FileRenameInformation for
SMB" that also contains a ReplaceIfExists flag. Or did you observe that
servers ignore the flag when given? The SMB1 path in smbd seems to
ignore it, see the hardcoded False value in reply.c:7391

Regards,
Christian


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHES] smbclient - add support for superseding the destination file on rename

Samba - samba-technical mailing list
On 03/22/2017 10:30 AM, Christian Ambach wrote:

> Hi Uri,
>
>> This smbclient patch set supports asking the server to replace an
>> existing destination file when renaming. This is a feature of SMB2
>> protocol only - haven't found a parallel in SMB1.
>
> According to [MS-FSCC], there is a "2.4.34.1 FileRenameInformation for
> SMB" that also contains a ReplaceIfExists flag. Or did you observe that
> servers ignore the flag when given? The SMB1 path in smbd seems to
> ignore it, see the hardcoded False value in reply.c:7391
>
> Regards,
> Christian
>
I haven't spotted renaming via TRANS2_SET_FILE_INFORMATION and
pass-through infolevels - I'll test it with Windows and see how it
works. This SMB1 protocol is certainly a mess...

Thanks!
Uri.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHES] smbclient - add support for superseding the destination file on rename

Samba - samba-technical mailing list
Hi Uri,

> I haven't spotted renaming via TRANS2_SET_FILE_INFORMATION and
> pass-through infolevels - I'll test it with Windows and see how it
> works. This SMB1 protocol is certainly a mess...

smb_file_rename_information looks at the flag and passes it down to
rename_internals_fsp, so it should work. The plain rename request in
CIFS does not have such a flag, but this call should not be in use any
more with current clients.

Regards,
Christian



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHES] smbclient - add support for superseding the destination file on rename

Samba - samba-technical mailing list
On 03/22/2017 10:46 AM, Christian Ambach via samba-technical wrote:

> Hi Uri,
>
>> I haven't spotted renaming via TRANS2_SET_FILE_INFORMATION and
>> pass-through infolevels - I'll test it with Windows and see how it
>> works. This SMB1 protocol is certainly a mess...
>
> smb_file_rename_information looks at the flag and passes it down to
> rename_internals_fsp, so it should work. The plain rename request in
> CIFS does not have such a flag, but this call should not be in use any
> more with current clients.
>
> Regards,
> Christian
>
>
>
Here's a modified patch set - the original 4 patches, plus 4 patches on
top that add rename with delete to SMB1, using FileRenameInformation.

As far as I can see, Windows still uses CIFS-style rename, making
rename-with-delete a non-atomic operation, so I left the default as-is.
This can easily be changed though.

BTW, the motive for adding this is gone - I wanted to use it to
rename-and-supersede a directory, and as it turns out, Windows doesn't
allow that (also documented in [MS-FSA] - search for ACCESS_DENIED under
that 10-page-long description of how it processes a rename). Samba seems
to allow that in SMB2, probably a bug, I'll follow up with a patch when
time allows.

Thanks,
Uri.

client-supersede-v2.patch.txt (33K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHES] smbclient - add support for superseding the destination file on rename

Samba - samba-technical mailing list
On Sun, Mar 26, 2017 at 09:43:23PM +0300, Uri Simchoni via samba-technical wrote:

> Here's a modified patch set - the original 4 patches, plus 4 patches on
> top that add rename with delete to SMB1, using FileRenameInformation.
>
> As far as I can see, Windows still uses CIFS-style rename, making
> rename-with-delete a non-atomic operation, so I left the default as-is.
> This can easily be changed though.
>
> BTW, the motive for adding this is gone - I wanted to use it to
> rename-and-supersede a directory, and as it turns out, Windows doesn't
> allow that (also documented in [MS-FSA] - search for ACCESS_DENIED under
> that 10-page-long description of how it processes a rename). Samba seems
> to allow that in SMB2, probably a bug, I'll follow up with a patch when
> time allows.

Really nice work Uri - that's something I've been planning to
add for a long time but never got the time to do it.

RB+ and pushed !

> From 4444c471e1ae2c51ab92ed9819e84e709d656d16 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <[hidden email]>
> Date: Tue, 21 Mar 2017 23:02:48 +0200
> Subject: [PATCH v2 1/8] s3: libsmb: add replace support to SMB2 rename
>
> SMB2 rename operation supports replacing the
> destination file if it exists.
>
> Signed-off-by: Uri Simchoni <[hidden email]>
> ---
>  source3/libsmb/cli_smb2_fnum.c | 9 +++++++--
>  source3/libsmb/cli_smb2_fnum.h | 5 +++--
>  source3/libsmb/clifile.c       | 4 +---
>  3 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
> index 848e077..351fccf 100644
> --- a/source3/libsmb/cli_smb2_fnum.c
> +++ b/source3/libsmb/cli_smb2_fnum.c
> @@ -2010,8 +2010,9 @@ NTSTATUS cli_smb2_set_security_descriptor(struct cli_state *cli,
>  ***************************************************************/
>  
>  NTSTATUS cli_smb2_rename(struct cli_state *cli,
> - const char *fname_src,
> - const char *fname_dst)
> + const char *fname_src,
> + const char *fname_dst,
> + bool replace)
>  {
>   NTSTATUS status;
>   DATA_BLOB inbuf = data_blob_null;
> @@ -2091,6 +2092,10 @@ NTSTATUS cli_smb2_rename(struct cli_state *cli,
>   goto fail;
>   }
>  
> + if (replace) {
> + SCVAL(inbuf.data, 0, 1);
> + }
> +
>   SIVAL(inbuf.data, 16, converted_size_bytes);
>   memcpy(inbuf.data + 20, converted_str, converted_size_bytes);
>  
> diff --git a/source3/libsmb/cli_smb2_fnum.h b/source3/libsmb/cli_smb2_fnum.h
> index 12c42a2..43e0471 100644
> --- a/source3/libsmb/cli_smb2_fnum.h
> +++ b/source3/libsmb/cli_smb2_fnum.h
> @@ -132,8 +132,9 @@ NTSTATUS cli_smb2_set_security_descriptor(struct cli_state *cli,
>   uint32_t sec_info,
>   const struct security_descriptor *sd);
>  NTSTATUS cli_smb2_rename(struct cli_state *cli,
> - const char *fname_src,
> - const char *fname_dst);
> + const char *fname_src,
> + const char *fname_dst,
> + bool replace);
>  NTSTATUS cli_smb2_set_ea_fnum(struct cli_state *cli,
>   uint16_t fnum,
>   const char *ea_name,
> diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
> index 6b32bf1..e6bc40c 100644
> --- a/source3/libsmb/clifile.c
> +++ b/source3/libsmb/clifile.c
> @@ -1082,9 +1082,7 @@ NTSTATUS cli_rename(struct cli_state *cli, const char *fname_src, const char *fn
>   NTSTATUS status = NT_STATUS_OK;
>  
>   if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
> - return cli_smb2_rename(cli,
> - fname_src,
> - fname_dst);
> + return cli_smb2_rename(cli, fname_src, fname_dst, false);
>   }
>  
>   frame = talloc_stackframe();
> --
> 2.9.3
>
>
> From ceb8aa9d23d695472d0092f7859c6cc088ca4499 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <[hidden email]>
> Date: Tue, 21 Mar 2017 23:13:07 +0200
> Subject: [PATCH v2 2/8] s3: libsmb: add replace support to cli_rename()
>
> Adds support for replacing the destination file at
> the higher-level cli_rename(). This is actually supported
> only by SMB2, and fails with invalid parameter with SMB1.
>
> Signed-off-by: Uri Simchoni <[hidden email]>
> ---
>  source3/client/client.c     |  2 +-
>  source3/libsmb/clifile.c    | 15 +++++++++++++--
>  source3/libsmb/libsmb_dir.c | 10 +++++++---
>  source3/libsmb/proto.h      |  5 ++++-
>  source3/torture/nbio.c      |  2 +-
>  source3/torture/torture.c   | 14 +++++++-------
>  6 files changed, 33 insertions(+), 15 deletions(-)
>
> diff --git a/source3/client/client.c b/source3/client/client.c
> index 226eb27..78945f9 100644
> --- a/source3/client/client.c
> +++ b/source3/client/client.c
> @@ -3870,7 +3870,7 @@ static int cmd_rename(void)
>   return 1;
>   }
>  
> - status = cli_rename(targetcli, targetsrc, targetdest);
> + status = cli_rename(targetcli, targetsrc, targetdest, false);
>   if (!NT_STATUS_IS_OK(status)) {
>   d_printf("%s renaming files %s -> %s \n",
>   nt_errstr(status),
> diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
> index e6bc40c..8f96e04 100644
> --- a/source3/libsmb/clifile.c
> +++ b/source3/libsmb/clifile.c
> @@ -1074,7 +1074,10 @@ NTSTATUS cli_rename_recv(struct tevent_req *req)
>   return tevent_req_simple_recv_ntstatus(req);
>  }
>  
> -NTSTATUS cli_rename(struct cli_state *cli, const char *fname_src, const char *fname_dst)
> +NTSTATUS cli_rename(struct cli_state *cli,
> +    const char *fname_src,
> +    const char *fname_dst,
> +    bool replace)
>  {
>   TALLOC_CTX *frame = NULL;
>   struct tevent_context *ev;
> @@ -1082,7 +1085,7 @@ NTSTATUS cli_rename(struct cli_state *cli, const char *fname_src, const char *fn
>   NTSTATUS status = NT_STATUS_OK;
>  
>   if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
> - return cli_smb2_rename(cli, fname_src, fname_dst, false);
> + return cli_smb2_rename(cli, fname_src, fname_dst, replace);
>   }
>  
>   frame = talloc_stackframe();
> @@ -1095,6 +1098,14 @@ NTSTATUS cli_rename(struct cli_state *cli, const char *fname_src, const char *fn
>   goto fail;
>   }
>  
> + if (replace) {
> + /*
> + * SMB1 doesn't support replace
> + */
> + status = NT_STATUS_INVALID_PARAMETER;
> + goto fail;
> + }
> +
>   ev = samba_tevent_context_init(frame);
>   if (ev == NULL) {
>   status = NT_STATUS_NO_MEMORY;
> diff --git a/source3/libsmb/libsmb_dir.c b/source3/libsmb/libsmb_dir.c
> index 8bf3c6b..4a4e084 100644
> --- a/source3/libsmb/libsmb_dir.c
> +++ b/source3/libsmb/libsmb_dir.c
> @@ -2032,12 +2032,16 @@ SMBC_rename_ctx(SMBCCTX *ocontext,
>   return -1;
>   }
>  
> - if (!NT_STATUS_IS_OK(cli_rename(targetcli1, targetpath1, targetpath2))) {
> + if (!NT_STATUS_IS_OK(
> + cli_rename(targetcli1, targetpath1, targetpath2, false))) {
>   int eno = SMBC_errno(ocontext, targetcli1);
>  
>   if (eno != EEXIST ||
> -    !NT_STATUS_IS_OK(cli_unlink(targetcli1, targetpath2, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN)) ||
> -    !NT_STATUS_IS_OK(cli_rename(targetcli1, targetpath1, targetpath2))) {
> +    !NT_STATUS_IS_OK(cli_unlink(targetcli1, targetpath2,
> + FILE_ATTRIBUTE_SYSTEM |
> +    FILE_ATTRIBUTE_HIDDEN)) ||
> +    !NT_STATUS_IS_OK(cli_rename(targetcli1, targetpath1,
> + targetpath2, false))) {
>  
>   errno = eno;
>   TALLOC_FREE(frame);
> diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
> index 764f3fc..57a45e3 100644
> --- a/source3/libsmb/proto.h
> +++ b/source3/libsmb/proto.h
> @@ -330,7 +330,10 @@ struct tevent_req *cli_rename_send(TALLOC_CTX *mem_ctx,
>                                  const char *fname_src,
>                                  const char *fname_dst);
>  NTSTATUS cli_rename_recv(struct tevent_req *req);
> -NTSTATUS cli_rename(struct cli_state *cli, const char *fname_src, const char *fname_dst);
> +NTSTATUS cli_rename(struct cli_state *cli,
> +    const char *fname_src,
> +    const char *fname_dst,
> +    bool replace);
>  struct tevent_req *cli_ntrename_send(TALLOC_CTX *mem_ctx,
>                                  struct tevent_context *ev,
>                                  struct cli_state *cli,
> diff --git a/source3/torture/nbio.c b/source3/torture/nbio.c
> index 6c87f9a..861f874 100644
> --- a/source3/torture/nbio.c
> +++ b/source3/torture/nbio.c
> @@ -261,7 +261,7 @@ void nb_rename(const char *oldname, const char *newname)
>  {
>   NTSTATUS status;
>  
> - status = cli_rename(c, oldname, newname);
> + status = cli_rename(c, oldname, newname, false);
>   if (!NT_STATUS_IS_OK(status)) {
>   printf("ERROR: rename %s %s failed (%s)\n",
>         oldname, newname, nt_errstr(status));
> diff --git a/source3/torture/torture.c b/source3/torture/torture.c
> index dbbd072..0d9a653 100644
> --- a/source3/torture/torture.c
> +++ b/source3/torture/torture.c
> @@ -4703,7 +4703,7 @@ static bool run_rename(int dummy)
>   return False;
>   }
>  
> - status = cli_rename(cli1, fname, fname1);
> + status = cli_rename(cli1, fname, fname1, false);
>   if (!NT_STATUS_IS_OK(status)) {
>   printf("First rename failed (SHARE_READ) (this is correct) - %s\n", nt_errstr(status));
>   } else {
> @@ -4731,7 +4731,7 @@ static bool run_rename(int dummy)
>   return False;
>   }
>  
> - status = cli_rename(cli1, fname, fname1);
> + status = cli_rename(cli1, fname, fname1, false);
>   if (!NT_STATUS_IS_OK(status)) {
>   printf("Second rename failed (SHARE_DELETE | SHARE_READ) - this should have succeeded - %s\n", nt_errstr(status));
>   correct = False;
> @@ -4778,7 +4778,7 @@ static bool run_rename(int dummy)
>    }
>  #endif
>  
> - status = cli_rename(cli1, fname, fname1);
> + status = cli_rename(cli1, fname, fname1, false);
>   if (!NT_STATUS_IS_OK(status)) {
>   printf("Third rename failed (SHARE_NONE) - this should have succeeded - %s\n", nt_errstr(status));
>   correct = False;
> @@ -4806,7 +4806,7 @@ static bool run_rename(int dummy)
>   return False;
>   }
>  
> - status = cli_rename(cli1, fname, fname1);
> + status = cli_rename(cli1, fname, fname1, false);
>   if (!NT_STATUS_IS_OK(status)) {
>   printf("Fourth rename failed (SHARE_READ | SHARE_WRITE) (this is correct) - %s\n", nt_errstr(status));
>   } else {
> @@ -4834,7 +4834,7 @@ static bool run_rename(int dummy)
>   return False;
>   }
>  
> - status = cli_rename(cli1, fname, fname1);
> + status = cli_rename(cli1, fname, fname1, false);
>   if (!NT_STATUS_IS_OK(status)) {
>   printf("Fifth rename failed (SHARE_READ | SHARE_WRITE | SHARE_DELETE) - this should have succeeded - %s ! \n", nt_errstr(status));
>   correct = False;
> @@ -5043,13 +5043,13 @@ static bool run_rename_access(int dummy)
>   * dst directory should fail.
>   */
>  
> - status = cli_rename(cli, src, dst);
> + status = cli_rename(cli, src, dst, false);
>   if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
>   printf("rename of %s -> %s should be ACCESS denied, was %s\n",
>   src, dst, nt_errstr(status));
>   goto fail;
>   }
> - status = cli_rename(cli, dsrc, ddst);
> + status = cli_rename(cli, dsrc, ddst, false);
>   if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
>   printf("rename of %s -> %s should be ACCESS denied, was %s\n",
>   src, dst, nt_errstr(status));
> --
> 2.9.3
>
>
> From dedf46a6d03f7c9637cdf8ea3f9a7669edcda7d3 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <[hidden email]>
> Date: Tue, 21 Mar 2017 23:26:05 +0200
> Subject: [PATCH v2 3/8] smbclient: add -f option to rename command
>
> This option causes the rename to request that the
> destination file / directory be replaced if it exists.
>
> Supported only in SMB2 and higher protocol.
>
> Signed-off-by: Uri Simchoni <[hidden email]>
> ---
>  source3/client/client.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/source3/client/client.c b/source3/client/client.c
> index 78945f9..d2ebea0 100644
> --- a/source3/client/client.c
> +++ b/source3/client/client.c
> @@ -3833,10 +3833,11 @@ static int cmd_rename(void)
>   char *targetsrc;
>   char *targetdest;
>          NTSTATUS status;
> + bool replace = false;
>  
>   if (!next_token_talloc(ctx, &cmd_ptr,&buf,NULL) ||
>      !next_token_talloc(ctx, &cmd_ptr,&buf2,NULL)) {
> - d_printf("rename <src> <dest>\n");
> + d_printf("rename <src> <dest> [-f]\n");
>   return 1;
>   }
>  
> @@ -3856,6 +3857,11 @@ static int cmd_rename(void)
>   return 1;
>   }
>  
> + if (next_token_talloc(ctx, &cmd_ptr, &buf, NULL) &&
> +    strcsequal(buf, "-f")) {
> + replace = true;
> + }
> +
>   status = cli_resolve_path(ctx, "", auth_info, cli, src, &targetcli,
>    &targetsrc);
>   if (!NT_STATUS_IS_OK(status)) {
> @@ -3870,7 +3876,7 @@ static int cmd_rename(void)
>   return 1;
>   }
>  
> - status = cli_rename(targetcli, targetsrc, targetdest, false);
> + status = cli_rename(targetcli, targetsrc, targetdest, replace);
>   if (!NT_STATUS_IS_OK(status)) {
>   d_printf("%s renaming files %s -> %s \n",
>   nt_errstr(status),
> --
> 2.9.3
>
>
> From bbc4176a3bce85f45b2ada6ee55884d25bd097f0 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <[hidden email]>
> Date: Tue, 21 Mar 2017 23:56:35 +0200
> Subject: [PATCH v2 4/8] manpages: update smbclient manpage with rename -f
>  option
>
> Document the -f option of the rename command.
>
> Signed-off-by: Uri Simchoni <[hidden email]>
> ---
>  docs-xml/manpages/smbclient.1.xml | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/docs-xml/manpages/smbclient.1.xml b/docs-xml/manpages/smbclient.1.xml
> index c8f4e40..059e14c 100644
> --- a/docs-xml/manpages/smbclient.1.xml
> +++ b/docs-xml/manpages/smbclient.1.xml
> @@ -1000,10 +1000,13 @@
>   </varlistentry>
>  
>   <varlistentry>
> - <term>rename &lt;old filename&gt; &lt;new filename&gt;</term>
> + <term>rename &lt;old filename&gt; &lt;new filename&gt; [-f]</term>
>   <listitem><para>Rename files in the current working directory on the
>   server from <replaceable>old filename</replaceable> to
> - <replaceable>new filename</replaceable>. </para></listitem>
> + <replaceable>new filename</replaceable>. The optional
> + -f switch is supported only by SMB2 protocol and beyond,
> + and allows for superseding the destination file,
> + if it exists.</para></listitem>
>   </varlistentry>
>  
>   <varlistentry>
> --
> 2.9.3
>
>
> From 1dd24602fc5f0c9d7d5920ef40b3dfd8030f7078 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <[hidden email]>
> Date: Sun, 26 Mar 2017 08:10:34 +0300
> Subject: [PATCH v2 5/8] libcli: introduce smbXcli_conn_support_passthrough()
>
> This routine queries the client connenction whether
> it supports query/set InfoLevels beyond 1000 (which,
> in Windows OS, is a pass-through mechanism to the
> file system).
>
> Signed-off-by: Uri Simchoni <[hidden email]>
> ---
>  libcli/smb/smbXcli_base.c | 17 +++++++++++++++++
>  libcli/smb/smbXcli_base.h |  1 +
>  2 files changed, 18 insertions(+)
>
> diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
> index 9521fc6..1fcf11e 100644
> --- a/libcli/smb/smbXcli_base.c
> +++ b/libcli/smb/smbXcli_base.c
> @@ -468,6 +468,23 @@ bool smbXcli_conn_use_unicode(struct smbXcli_conn *conn)
>   return false;
>  }
>  
> +/*
> + * [MS-SMB] 2.2.2.3.5 - SMB1 support for passing through
> + * query/set commands to the file system
> + */
> +bool smbXcli_conn_support_passthrough(struct smbXcli_conn *conn)
> +{
> + if (conn->protocol >= PROTOCOL_SMB2_02) {
> + return true;
> + }
> +
> + if (conn->smb1.capabilities & CAP_W2K_SMBS) {
> + return true;
> + }
> +
> + return false;
> +}
> +
>  void smbXcli_conn_set_sockopt(struct smbXcli_conn *conn, const char *options)
>  {
>   set_socket_options(conn->sock_fd, options);
> diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h
> index 84f4a6b..1abeb2b 100644
> --- a/libcli/smb/smbXcli_base.h
> +++ b/libcli/smb/smbXcli_base.h
> @@ -47,6 +47,7 @@ bool smbXcli_conn_dfs_supported(struct smbXcli_conn *conn);
>  
>  enum protocol_types smbXcli_conn_protocol(struct smbXcli_conn *conn);
>  bool smbXcli_conn_use_unicode(struct smbXcli_conn *conn);
> +bool smbXcli_conn_support_passthrough(struct smbXcli_conn *conn);
>  
>  void smbXcli_conn_set_sockopt(struct smbXcli_conn *conn, const char *options);
>  const struct sockaddr_storage *smbXcli_conn_local_sockaddr(struct smbXcli_conn *conn);
> --
> 2.9.3
>
>
> From 0af96ce668c993b7fcd92601d1a3e54092c7a86a Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <[hidden email]>
> Date: Sun, 26 Mar 2017 08:54:42 +0300
> Subject: [PATCH v2 6/8] s3-libsmb: cli_cifs_rename_send()
>
> Pure refactoring - current rename is [MS-CIFS] - style
> rename. In later patch we'll introduce [MS-SMB] rename.
>
> Signed-off-by: Uri Simchoni <[hidden email]>
> ---
>  source3/libsmb/clifile.c | 37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
> index 8f96e04..b5f29b6 100644
> --- a/source3/libsmb/clifile.c
> +++ b/source3/libsmb/clifile.c
> @@ -992,25 +992,40 @@ NTSTATUS cli_posix_chown(struct cli_state *cli,
>   Rename a file.
>  ****************************************************************************/
>  
> -static void cli_rename_done(struct tevent_req *subreq);
> +static struct tevent_req *cli_cifs_rename_send(TALLOC_CTX *mem_ctx,
> +       struct tevent_context *ev,
> +       struct cli_state *cli,
> +       const char *fname_src,
> +       const char *fname_dst);
>  
> -struct cli_rename_state {
> +struct tevent_req *cli_rename_send(TALLOC_CTX *mem_ctx,
> +   struct tevent_context *ev,
> +   struct cli_state *cli,
> +   const char *fname_src,
> +   const char *fname_dst)
> +{
> + return cli_cifs_rename_send(mem_ctx, ev, cli, fname_src, fname_dst);
> +}
> +
> +static void cli_cifs_rename_done(struct tevent_req *subreq);
> +
> +struct cli_cifs_rename_state {
>   uint16_t vwv[1];
>  };
>  
> -struct tevent_req *cli_rename_send(TALLOC_CTX *mem_ctx,
> - struct tevent_context *ev,
> - struct cli_state *cli,
> - const char *fname_src,
> - const char *fname_dst)
> +static struct tevent_req *cli_cifs_rename_send(TALLOC_CTX *mem_ctx,
> +       struct tevent_context *ev,
> +       struct cli_state *cli,
> +       const char *fname_src,
> +       const char *fname_dst)
>  {
>   struct tevent_req *req = NULL, *subreq = NULL;
> - struct cli_rename_state *state = NULL;
> + struct cli_cifs_rename_state *state = NULL;
>   uint8_t additional_flags = 0;
>   uint16_t additional_flags2 = 0;
>   uint8_t *bytes = NULL;
>  
> - req = tevent_req_create(mem_ctx, &state, struct cli_rename_state);
> + req = tevent_req_create(mem_ctx, &state, struct cli_cifs_rename_state);
>   if (req == NULL) {
>   return NULL;
>   }
> @@ -1051,11 +1066,11 @@ struct tevent_req *cli_rename_send(TALLOC_CTX *mem_ctx,
>   if (tevent_req_nomem(subreq, req)) {
>   return tevent_req_post(req, ev);
>   }
> - tevent_req_set_callback(subreq, cli_rename_done, req);
> + tevent_req_set_callback(subreq, cli_cifs_rename_done, req);
>   return req;
>  }
>  
> -static void cli_rename_done(struct tevent_req *subreq)
> +static void cli_cifs_rename_done(struct tevent_req *subreq)
>  {
>   struct tevent_req *req = tevent_req_callback_data(
>   subreq, struct tevent_req);
> --
> 2.9.3
>
>
> From 460c04c2f801a1425857bb014cc1337a12626251 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <[hidden email]>
> Date: Sun, 26 Mar 2017 09:14:43 +0300
> Subject: [PATCH v2 7/8] s3-libsmb: fail rename and replace inside cifs variant
>
> Another refactoring step - fail request to rename and
> replace existing file from within the CIFS version,
> allowing the soon-to-be-added SMB version to succeed.
>
> Signed-off-by: Uri Simchoni <[hidden email]>
> ---
>  source3/libsmb/clifile.c | 30 +++++++++++++++++-------------
>  source3/libsmb/proto.h   |  9 +++++----
>  2 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
> index b5f29b6..44fde38 100644
> --- a/source3/libsmb/clifile.c
> +++ b/source3/libsmb/clifile.c
> @@ -996,15 +996,18 @@ static struct tevent_req *cli_cifs_rename_send(TALLOC_CTX *mem_ctx,
>         struct tevent_context *ev,
>         struct cli_state *cli,
>         const char *fname_src,
> -       const char *fname_dst);
> +       const char *fname_dst,
> +       bool replace);
>  
>  struct tevent_req *cli_rename_send(TALLOC_CTX *mem_ctx,
>     struct tevent_context *ev,
>     struct cli_state *cli,
>     const char *fname_src,
> -   const char *fname_dst)
> +   const char *fname_dst,
> +   bool replace)
>  {
> - return cli_cifs_rename_send(mem_ctx, ev, cli, fname_src, fname_dst);
> + return cli_cifs_rename_send(mem_ctx, ev, cli, fname_src, fname_dst,
> +    replace);
>  }
>  
>  static void cli_cifs_rename_done(struct tevent_req *subreq);
> @@ -1017,7 +1020,8 @@ static struct tevent_req *cli_cifs_rename_send(TALLOC_CTX *mem_ctx,
>         struct tevent_context *ev,
>         struct cli_state *cli,
>         const char *fname_src,
> -       const char *fname_dst)
> +       const char *fname_dst,
> +       bool replace)
>  {
>   struct tevent_req *req = NULL, *subreq = NULL;
>   struct cli_cifs_rename_state *state = NULL;
> @@ -1030,6 +1034,14 @@ static struct tevent_req *cli_cifs_rename_send(TALLOC_CTX *mem_ctx,
>   return NULL;
>   }
>  
> + if (replace) {
> + /*
> + * CIFS doesn't support replace
> + */
> + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
> + return tevent_req_post(req, ev);
> + }
> +
>   SSVAL(state->vwv+0, 0, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_DIRECTORY);
>  
>   bytes = talloc_array(state, uint8_t, 1);
> @@ -1113,21 +1125,13 @@ NTSTATUS cli_rename(struct cli_state *cli,
>   goto fail;
>   }
>  
> - if (replace) {
> - /*
> - * SMB1 doesn't support replace
> - */
> - status = NT_STATUS_INVALID_PARAMETER;
> - goto fail;
> - }
> -
>   ev = samba_tevent_context_init(frame);
>   if (ev == NULL) {
>   status = NT_STATUS_NO_MEMORY;
>   goto fail;
>   }
>  
> - req = cli_rename_send(frame, ev, cli, fname_src, fname_dst);
> + req = cli_rename_send(frame, ev, cli, fname_src, fname_dst, replace);
>   if (req == NULL) {
>   status = NT_STATUS_NO_MEMORY;
>   goto fail;
> diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
> index 57a45e3..b453733 100644
> --- a/source3/libsmb/proto.h
> +++ b/source3/libsmb/proto.h
> @@ -325,10 +325,11 @@ NTSTATUS cli_posix_chown(struct cli_state *cli,
>   uid_t uid,
>   gid_t gid);
>  struct tevent_req *cli_rename_send(TALLOC_CTX *mem_ctx,
> -                                struct tevent_context *ev,
> -                                struct cli_state *cli,
> -                                const char *fname_src,
> -                                const char *fname_dst);
> +   struct tevent_context *ev,
> +   struct cli_state *cli,
> +   const char *fname_src,
> +   const char *fname_dst,
> +   bool replace);
>  NTSTATUS cli_rename_recv(struct tevent_req *req);
>  NTSTATUS cli_rename(struct cli_state *cli,
>      const char *fname_src,
> --
> 2.9.3
>
>
> From c0b958c1a128d63519001bfe6d180340e7bc9c80 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <[hidden email]>
> Date: Sun, 26 Mar 2017 12:02:09 +0300
> Subject: [PATCH v2 8/8] s3-libsmb: support rename and replace for SMB1
>
> Add cli_smb1_rename_send() which renames a file via
> setting FileRenameInformation.
>
> Curretly this path is invoked only if replacing
> an existing file is requested. This is because as far
> as I can see, Windows uses CIFS rename for anything below
> SMB2.
>
> Signed-off-by: Uri Simchoni <[hidden email]>
> ---
>  source3/libsmb/clifile.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 92 insertions(+), 2 deletions(-)
>
> diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
> index 44fde38..cc1d1e4 100644
> --- a/source3/libsmb/clifile.c
> +++ b/source3/libsmb/clifile.c
> @@ -999,6 +999,13 @@ static struct tevent_req *cli_cifs_rename_send(TALLOC_CTX *mem_ctx,
>         const char *fname_dst,
>         bool replace);
>  
> +static struct tevent_req *cli_smb1_rename_send(TALLOC_CTX *mem_ctx,
> +       struct tevent_context *ev,
> +       struct cli_state *cli,
> +       const char *fname_src,
> +       const char *fname_dst,
> +       bool replace);
> +
>  struct tevent_req *cli_rename_send(TALLOC_CTX *mem_ctx,
>     struct tevent_context *ev,
>     struct cli_state *cli,
> @@ -1006,8 +1013,91 @@ struct tevent_req *cli_rename_send(TALLOC_CTX *mem_ctx,
>     const char *fname_dst,
>     bool replace)
>  {
> - return cli_cifs_rename_send(mem_ctx, ev, cli, fname_src, fname_dst,
> -    replace);
> + if (replace && smbXcli_conn_support_passthrough(cli->conn)) {
> + return cli_smb1_rename_send(mem_ctx, ev, cli, fname_src,
> +    fname_dst, replace);
> + } else {
> + return cli_cifs_rename_send(mem_ctx, ev, cli, fname_src,
> +    fname_dst, replace);
> + }
> +}
> +
> +struct cli_smb1_rename_state {
> + uint8_t *data;
> +};
> +
> +static void cli_smb1_rename_done(struct tevent_req *subreq);
> +
> +static struct tevent_req *cli_smb1_rename_send(TALLOC_CTX *mem_ctx,
> +       struct tevent_context *ev,
> +       struct cli_state *cli,
> +       const char *fname_src,
> +       const char *fname_dst,
> +       bool replace)
> +{
> + NTSTATUS status;
> + struct tevent_req *req = NULL, *subreq = NULL;
> + struct cli_smb1_rename_state *state = NULL;
> + smb_ucs2_t *converted_str = NULL;
> + size_t converted_size_bytes = 0;
> +
> + req = tevent_req_create(mem_ctx, &state, struct cli_smb1_rename_state);
> + if (req == NULL) {
> + return NULL;
> + }
> +
> + if (!push_ucs2_talloc(talloc_tos(), &converted_str, fname_dst,
> +      &converted_size_bytes)) {
> + status = NT_STATUS_INVALID_PARAMETER;
> + goto fail;
> + }
> +
> + /* W2K8 insists the dest name is not null
> +   terminated. Remove the last 2 zero bytes
> +   and reduce the name length. */
> +
> + if (converted_size_bytes < 2) {
> + status = NT_STATUS_INVALID_PARAMETER;
> + goto fail;
> + }
> + converted_size_bytes -= 2;
> +
> + state->data =
> +    talloc_zero_array(state, uint8_t, 12 + converted_size_bytes);
> + if (state->data == NULL) {
> + status = NT_STATUS_NO_MEMORY;
> + goto fail;
> + }
> +
> + if (replace) {
> + SCVAL(state->data, 0, 1);
> + }
> +
> + SIVAL(state->data, 8, converted_size_bytes);
> + memcpy(state->data + 12, converted_str, converted_size_bytes);
> +
> + TALLOC_FREE(converted_str);
> +
> + subreq = cli_setpathinfo_send(
> +    state, ev, cli, SMB_FILE_RENAME_INFORMATION, fname_src, state->data,
> +    talloc_get_size(state->data));
> + if (tevent_req_nomem(subreq, req)) {
> + status = NT_STATUS_NO_MEMORY;
> + goto fail;
> + }
> + tevent_req_set_callback(subreq, cli_smb1_rename_done, req);
> + return req;
> +
> +fail:
> + TALLOC_FREE(converted_str);
> + tevent_req_nterror(req, status);
> + return tevent_req_post(req, ev);
> +}
> +
> +static void cli_smb1_rename_done(struct tevent_req *subreq)
> +{
> + NTSTATUS status = cli_setpathinfo_recv(subreq);
> + tevent_req_simple_finish_ntstatus(subreq, status);
>  }
>  
>  static void cli_cifs_rename_done(struct tevent_req *subreq);
> --
> 2.9.3
>


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHES] smbclient - add support for superseding the destination file on rename

Samba - samba-technical mailing list
Hi,

sorry for coming back on this, but I think the following hunk should have
been updated with the patches that went in on top of the original patches.

> - <replaceable>new filename</replaceable>. </para></listitem>
> + <replaceable>new filename</replaceable>. The optional
> + -f switch is supported only by SMB2 protocol and beyond,
> + and allows for superseding the destination file,
> + if it exists.</para></listitem>
>   </varlistentry>

Cheers,
Christian


---
Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
https://www.avast.com/antivirus


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHES] smbclient - add support for superseding the destination file on rename

Samba - samba-technical mailing list
On 04/07/2017 06:47 PM, Christian Ambach via samba-technical wrote:

> Hi,
>
> sorry for coming back on this, but I think the following hunk should have
> been updated with the patches that went in on top of the original patches.
>
>> - <replaceable>new filename</replaceable>. </para></listitem>
>> + <replaceable>new filename</replaceable>. The optional
>> + -f switch is supported only by SMB2 protocol and beyond,
>> + and allows for superseding the destination file,
>> + if it exists.</para></listitem>
>>   </varlistentry>
>
> Cheers,
> Christian
>
>
> ---
> Diese E-Mail wurde von Avast Antivirus-Software auf Viren geprüft.
> https://www.avast.com/antivirus
>
>
Right. Here's the fixup patch.

Thanks!
Uri.

doc-fixup.patch.txt (1K) Download Attachment
Loading...