[Patch] Regression fix for roaming profiles (bug #10297)
Jeremy Allison
jra at samba.org
Mon Oct 24 22:28:31 UTC 2016
On Sat, Oct 22, 2016 at 10:15:08AM +0200, Stefan Metzmacher wrote:
> Hi,
>
> here's a possible regression fix for
>
> https://bugzilla.samba.org/show_bug.cgi?id=10297
>
> Existing patches were supposed to fix the drop box share
> (-wx permissions) use case, but they broke the roaming profiles
> use case.
>
> Do we have regression tests for the drop box feature?
>
> I was hoping to add tests for roaming profiles, but it's
> harder than expected, because make test runs as just one
> real unix user and we won't get EACCES failures there.
>
> Please review and test that the drop box feature still
> works as desired.
Ah, OK - so the underlying change is to only set
UCF_PREP_CREATEFILE in the cases where O_CREAT
would get set in the underlying POSIX call.
Looks correct to me, but I'll check he
dropbox case before pushing.
> From 8e35a0cdb98e2e5f0d923d5259b0ccc279f0618e Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <metze at samba.org>
> Date: Thu, 13 Oct 2016 12:42:59 +0200
> Subject: [PATCH] s3:smbd: only pass UCF_PREP_CREATEFILE to filename_convert()
> if we may create a new file
>
> This fixes a regression introduced by commit
> f98d10af2a05f0261611f4cabdfe274cd9fe91c0
> (smbd: Always use UCF_PREP_CREATEFILE for filename_convert calls to resolve a path for open)
>
> The main problem was that Windows client seem to verify
> the access to user.V2\ntuser.ini is rejected with NT_STATUS_ACCESS_DENIED,
> using the machine credentials.
>
> Passing UCF_PREP_CREATEFILE to filename_convert() triggers a code path
> that implements a dropbox behaviour. A dropbox is a directory with only -wx permissions,
> so get_real_filename fails with EACCESS, it needs to list the directory.
> EACCESS is ignored with UCF_PREP_CREATEFILE.
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=10297
>
> Signed-off-by: Stefan Metzmacher <metze at samba.org>
> ---
> source3/smbd/filename.c | 23 +++++++++++++++++++++++
> source3/smbd/nttrans.c | 8 ++++----
> source3/smbd/proto.h | 1 +
> source3/smbd/reply.c | 38 ++++++++++++++++++++------------------
> source3/smbd/smb2_create.c | 10 ++++++----
> 5 files changed, 54 insertions(+), 26 deletions(-)
>
> diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
> index 7062060..a8cfb55 100644
> --- a/source3/smbd/filename.c
> +++ b/source3/smbd/filename.c
> @@ -30,6 +30,29 @@
> #include "smbd/smbd.h"
> #include "smbd/globals.h"
>
> +uint32_t filename_create_ucf_flags(struct smb_request *req, uint32_t create_disposition)
> +{
> + uint32_t ucf_flags = 0;
> +
> + if (req != NULL && req->posix_pathnames) {
> + ucf_flags |= UCF_POSIX_PATHNAMES;
> + }
> +
> + switch (create_disposition) {
> + case FILE_OPEN:
> + case FILE_OVERWRITE:
> + break;
> + case FILE_SUPERSEDE:
> + case FILE_CREATE:
> + case FILE_OPEN_IF:
> + case FILE_OVERWRITE_IF:
> + ucf_flags |= UCF_PREP_CREATEFILE;
> + break;
> + }
> +
> + return ucf_flags;
> +}
> +
> static NTSTATUS build_stream_path(TALLOC_CTX *mem_ctx,
> connection_struct *conn,
> struct smb_filename *smb_fname);
> diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
> index 05f3cae..5f122a9 100644
> --- a/source3/smbd/nttrans.c
> +++ b/source3/smbd/nttrans.c
> @@ -461,8 +461,7 @@ void reply_ntcreate_and_X(struct smb_request *req)
> int oplock_request;
> uint8_t oplock_granted = NO_OPLOCK_RETURN;
> struct case_semantics_state *case_state = NULL;
> - uint32_t ucf_flags = UCF_PREP_CREATEFILE |
> - (req->posix_pathnames ? UCF_POSIX_PATHNAMES : 0);
> + uint32_t ucf_flags;
> TALLOC_CTX *ctx = talloc_tos();
>
> START_PROFILE(SMBntcreateX);
> @@ -536,6 +535,7 @@ void reply_ntcreate_and_X(struct smb_request *req)
> }
> }
>
> + ucf_flags = filename_create_ucf_flags(req, create_disposition);
> status = filename_convert(ctx,
> conn,
> req->flags2 & FLAGS2_DFS_PATHNAMES,
> @@ -1024,8 +1024,7 @@ static void call_nt_transact_create(connection_struct *conn,
> int oplock_request;
> uint8_t oplock_granted;
> struct case_semantics_state *case_state = NULL;
> - uint32_t ucf_flags = UCF_PREP_CREATEFILE |
> - (req->posix_pathnames ? UCF_POSIX_PATHNAMES : 0);
> + uint32_t ucf_flags;
> TALLOC_CTX *ctx = talloc_tos();
>
> DEBUG(5,("call_nt_transact_create\n"));
> @@ -1106,6 +1105,7 @@ static void call_nt_transact_create(connection_struct *conn,
> }
> }
>
> + ucf_flags = filename_create_ucf_flags(req, create_disposition);
> status = filename_convert(ctx,
> conn,
> req->flags2 & FLAGS2_DFS_PATHNAMES,
> diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
> index fb30a9e..352d28c 100644
> --- a/source3/smbd/proto.h
> +++ b/source3/smbd/proto.h
> @@ -339,6 +339,7 @@ int fsp_stat(files_struct *fsp);
>
> /* The following definitions come from smbd/filename.c */
>
> +uint32_t filename_create_ucf_flags(struct smb_request *req, uint32_t create_disposition);
> NTSTATUS unix_convert(TALLOC_CTX *ctx,
> connection_struct *conn,
> const char *orig_path,
> diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
> index 4f1ecb1..0aec433 100644
> --- a/source3/smbd/reply.c
> +++ b/source3/smbd/reply.c
> @@ -2111,8 +2111,7 @@ void reply_open(struct smb_request *req)
> uint32_t create_options = 0;
> uint32_t private_flags = 0;
> NTSTATUS status;
> - uint32_t ucf_flags = UCF_PREP_CREATEFILE |
> - (req->posix_pathnames ? UCF_POSIX_PATHNAMES : 0);
> + uint32_t ucf_flags;
> TALLOC_CTX *ctx = talloc_tos();
>
> START_PROFILE(SMBopen);
> @@ -2141,6 +2140,8 @@ void reply_open(struct smb_request *req)
> goto out;
> }
>
> + ucf_flags = filename_create_ucf_flags(req, create_disposition);
> +
> status = filename_convert(ctx,
> conn,
> req->flags2 & FLAGS2_DFS_PATHNAMES,
> @@ -2265,8 +2266,7 @@ void reply_open_and_X(struct smb_request *req)
> uint32_t create_disposition;
> uint32_t create_options = 0;
> uint32_t private_flags = 0;
> - uint32_t ucf_flags = UCF_PREP_CREATEFILE |
> - (req->posix_pathnames ? UCF_POSIX_PATHNAMES : 0);
> + uint32_t ucf_flags;
> TALLOC_CTX *ctx = talloc_tos();
>
> START_PROFILE(SMBopenX);
> @@ -2313,6 +2313,8 @@ void reply_open_and_X(struct smb_request *req)
> goto out;
> }
>
> + ucf_flags = filename_create_ucf_flags(req, create_disposition);
> +
> status = filename_convert(ctx,
> conn,
> req->flags2 & FLAGS2_DFS_PATHNAMES,
> @@ -2525,8 +2527,7 @@ void reply_mknew(struct smb_request *req)
> uint32_t share_mode = FILE_SHARE_READ|FILE_SHARE_WRITE;
> uint32_t create_disposition;
> uint32_t create_options = 0;
> - uint32_t ucf_flags = UCF_PREP_CREATEFILE |
> - (req->posix_pathnames ? UCF_POSIX_PATHNAMES : 0);
> + uint32_t ucf_flags;
> TALLOC_CTX *ctx = talloc_tos();
>
> START_PROFILE(SMBcreate);
> @@ -2540,6 +2541,14 @@ void reply_mknew(struct smb_request *req)
> fattr = SVAL(req->vwv+0, 0);
> oplock_request = CORE_OPLOCK_REQUEST(req->inbuf);
>
> + if (req->cmd == SMBmknew) {
> + /* We should fail if file exists. */
> + create_disposition = FILE_CREATE;
> + } else {
> + /* Create if file doesn't exist, truncate if it does. */
> + create_disposition = FILE_OVERWRITE_IF;
> + }
> +
> /* mtime. */
> ft.mtime = convert_time_t_to_timespec(srv_make_unix_date3(req->vwv+1));
>
> @@ -2550,6 +2559,7 @@ void reply_mknew(struct smb_request *req)
> goto out;
> }
>
> + ucf_flags = filename_create_ucf_flags(req, create_disposition);
> status = filename_convert(ctx,
> conn,
> req->flags2 & FLAGS2_DFS_PATHNAMES,
> @@ -2574,14 +2584,6 @@ void reply_mknew(struct smb_request *req)
> smb_fname_str_dbg(smb_fname)));
> }
>
> - if(req->cmd == SMBmknew) {
> - /* We should fail if file exists. */
> - create_disposition = FILE_CREATE;
> - } else {
> - /* Create if file doesn't exist, truncate if it does. */
> - create_disposition = FILE_OVERWRITE_IF;
> - }
> -
> status = SMB_VFS_CREATE_FILE(
> conn, /* conn */
> req, /* req */
> @@ -2658,8 +2660,7 @@ void reply_ctemp(struct smb_request *req)
> char *s;
> NTSTATUS status;
> int i;
> - uint32_t ucf_flags = UCF_PREP_CREATEFILE |
> - (req->posix_pathnames ? UCF_POSIX_PATHNAMES : 0);
> + uint32_t ucf_flags;
> TALLOC_CTX *ctx = talloc_tos();
>
> START_PROFILE(SMBctemp);
> @@ -2696,6 +2697,7 @@ void reply_ctemp(struct smb_request *req)
> goto out;
> }
>
> + ucf_flags = filename_create_ucf_flags(req, FILE_CREATE);
> status = filename_convert(ctx, conn,
> req->flags2 & FLAGS2_DFS_PATHNAMES,
> fname,
> @@ -6133,8 +6135,7 @@ void reply_mkdir(struct smb_request *req)
> struct smb_filename *smb_dname = NULL;
> char *directory = NULL;
> NTSTATUS status;
> - uint32_t ucf_flags = UCF_PREP_CREATEFILE |
> - (req->posix_pathnames ? UCF_POSIX_PATHNAMES : 0);
> + uint32_t ucf_flags;
> TALLOC_CTX *ctx = talloc_tos();
>
> START_PROFILE(SMBmkdir);
> @@ -6146,6 +6147,7 @@ void reply_mkdir(struct smb_request *req)
> goto out;
> }
>
> + ucf_flags = filename_create_ucf_flags(req, FILE_CREATE);
> status = filename_convert(ctx, conn,
> req->flags2 & FLAGS2_DFS_PATHNAMES,
> directory,
> diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c
> index 75da8a1..8211991 100644
> --- a/source3/smbd/smb2_create.c
> +++ b/source3/smbd/smb2_create.c
> @@ -374,12 +374,12 @@ static bool smb2_lease_key_valid(const struct smb2_lease_key *key)
> return ((key->data[0] != 0) || (key->data[1] != 0));
> }
>
> -static NTSTATUS smbd_smb2_create_durable_lease_check(
> +static NTSTATUS smbd_smb2_create_durable_lease_check(struct smb_request *smb1req,
> const char *requested_filename, const struct files_struct *fsp,
> const struct smb2_lease *lease_ptr)
> {
> struct smb_filename *smb_fname = NULL;
> - uint32_t ucf_flags = UCF_PREP_CREATEFILE;
> + uint32_t ucf_flags;
> NTSTATUS status;
>
> if (lease_ptr == NULL) {
> @@ -404,6 +404,7 @@ static NTSTATUS smbd_smb2_create_durable_lease_check(
> return NT_STATUS_OBJECT_NAME_NOT_FOUND;
> }
>
> + ucf_flags = filename_create_ucf_flags(smb1req, FILE_OPEN);
> status = filename_convert(talloc_tos(), fsp->conn, false,
> requested_filename, ucf_flags,
> NULL, &smb_fname);
> @@ -1057,7 +1058,7 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx,
> (unsigned)result->oplock_type, lease_ptr));
>
> status = smbd_smb2_create_durable_lease_check(
> - fname, result, lease_ptr);
> + smb1req, fname, result, lease_ptr);
> if (!NT_STATUS_IS_OK(status)) {
> close_file(smb1req, result, SHUTDOWN_CLOSE);
> tevent_req_nterror(req, status);
> @@ -1078,7 +1079,7 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx,
> info = FILE_WAS_OPENED;
> } else {
> struct smb_filename *smb_fname = NULL;
> - uint32_t ucf_flags = UCF_PREP_CREATEFILE;
> + uint32_t ucf_flags;
>
> if (requested_oplock_level == SMB2_OPLOCK_LEVEL_LEASE) {
> if (lease_ptr == NULL) {
> @@ -1102,6 +1103,7 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx,
> }
> }
>
> + ucf_flags = filename_create_ucf_flags(smb1req, in_create_disposition);
> status = filename_convert(req,
> smb1req->conn,
> smb1req->flags2 & FLAGS2_DFS_PATHNAMES,
> --
> 1.9.1
>
More information about the samba-technical
mailing list