From a8fa2fc0e987b80ffeaa239afbfa8b4b6cb16a8d Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 9 Jul 2013 11:02:39 -0700 Subject: [PATCH 1/2] smbd: Fix a profile problem When trying to read a profile, under certain circumstances Windows tries to read with its machine account first. The profile previously written was stored with an ACL that only allows access for the user and not the machine. Windows should get an NT_STATUS_ACCESS_DENIED when using the machine account, making it retry with the user account (which would then succeed). Samba under these circumstances erroneously gives NT_STATUS_OBJECT_PATH_NOT_FOUND, which makes Windows give up and not retry. The reasons is the "dropbox" patch in unix_convert, turning EACCESS on the last path component to OBJECT_PATH_NOT_FOUND. This patch makes the dropbox behaviour only kick in when we are creating a file. I think this is an abstraction violation. unix_convert() should not have to know about the create_disposition, but given that we have pathname resolution separated from the core open code right now this is the best we can do. Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison --- source3/smbd/filename.c | 3 ++- source3/smbd/nttrans.c | 6 ++++-- source3/smbd/reply.c | 48 ++++++++++++++++++++++++---------------------- source3/smbd/smb2_create.c | 3 ++- source3/smbd/smbd.h | 1 + 5 files changed, 34 insertions(+), 27 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 03e1d2d..4384f5a 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -723,7 +723,8 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx, * can only put stuff with permission -wx. */ if ((errno != 0) && (errno != ENOENT) - && (errno != EACCES)) { + && ((ucf_flags & UCF_CREATING_FILE) && + (errno != EACCES))) { /* * ENOTDIR and ELOOP both map to * NT_STATUS_OBJECT_PATH_NOT_FOUND diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c index f4b0456..bcba29a 100644 --- a/source3/smbd/nttrans.c +++ b/source3/smbd/nttrans.c @@ -538,7 +538,8 @@ void reply_ntcreate_and_X(struct smb_request *req) conn, req->flags2 & FLAGS2_DFS_PATHNAMES, fname, - 0, + (create_disposition == FILE_CREATE) + ? UCF_CREATING_FILE : 0, NULL, &smb_fname); @@ -1118,7 +1119,8 @@ static void call_nt_transact_create(connection_struct *conn, conn, req->flags2 & FLAGS2_DFS_PATHNAMES, fname, - 0, + (create_disposition == FILE_CREATE) + ? UCF_CREATING_FILE : 0, NULL, &smb_fname); diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 07b144e..2ae3ff4 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1906,11 +1906,20 @@ void reply_open(struct smb_request *req) goto out; } + if (!map_open_params_to_ntcreate(fname, deny_mode, + OPENX_FILE_EXISTS_OPEN, &access_mask, + &share_mode, &create_disposition, + &create_options, &private_flags)) { + reply_force_doserror(req, ERRDOS, ERRbadaccess); + goto out; + } + status = filename_convert(ctx, conn, req->flags2 & FLAGS2_DFS_PATHNAMES, fname, - 0, + (create_disposition == FILE_CREATE) + ? UCF_CREATING_FILE : 0, NULL, &smb_fname); if (!NT_STATUS_IS_OK(status)) { @@ -1924,14 +1933,6 @@ void reply_open(struct smb_request *req) goto out; } - if (!map_open_params_to_ntcreate(smb_fname->base_name, deny_mode, - OPENX_FILE_EXISTS_OPEN, &access_mask, - &share_mode, &create_disposition, - &create_options, &private_flags)) { - reply_force_doserror(req, ERRDOS, ERRbadaccess); - goto out; - } - status = SMB_VFS_CREATE_FILE( conn, /* conn */ req, /* req */ @@ -2081,11 +2082,22 @@ void reply_open_and_X(struct smb_request *req) goto out; } + if (!map_open_params_to_ntcreate(fname, deny_mode, + smb_ofun, + &access_mask, &share_mode, + &create_disposition, + &create_options, + &private_flags)) { + reply_force_doserror(req, ERRDOS, ERRbadaccess); + goto out; + } + status = filename_convert(ctx, conn, req->flags2 & FLAGS2_DFS_PATHNAMES, fname, - 0, + (create_disposition == FILE_CREATE) + ? UCF_CREATING_FILE : 0, NULL, &smb_fname); if (!NT_STATUS_IS_OK(status)) { @@ -2099,16 +2111,6 @@ void reply_open_and_X(struct smb_request *req) goto out; } - if (!map_open_params_to_ntcreate(smb_fname->base_name, deny_mode, - smb_ofun, - &access_mask, &share_mode, - &create_disposition, - &create_options, - &private_flags)) { - reply_force_doserror(req, ERRDOS, ERRbadaccess); - goto out; - } - status = SMB_VFS_CREATE_FILE( conn, /* conn */ req, /* req */ @@ -2328,7 +2330,7 @@ void reply_mknew(struct smb_request *req) conn, req->flags2 & FLAGS2_DFS_PATHNAMES, fname, - 0, + UCF_CREATING_FILE, NULL, &smb_fname); if (!NT_STATUS_IS_OK(status)) { @@ -2469,7 +2471,7 @@ void reply_ctemp(struct smb_request *req) status = filename_convert(ctx, conn, req->flags2 & FLAGS2_DFS_PATHNAMES, fname, - 0, + UCF_CREATING_FILE, NULL, &smb_fname); if (!NT_STATUS_IS_OK(status)) { @@ -5828,7 +5830,7 @@ void reply_mkdir(struct smb_request *req) status = filename_convert(ctx, conn, req->flags2 & FLAGS2_DFS_PATHNAMES, directory, - 0, + UCF_CREATING_FILE, NULL, &smb_dname); if (!NT_STATUS_IS_OK(status)) { diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c index 168beac..4f2edfc 100644 --- a/source3/smbd/smb2_create.c +++ b/source3/smbd/smb2_create.c @@ -846,7 +846,8 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx, smb1req->conn, smb1req->flags2 & FLAGS2_DFS_PATHNAMES, fname, - 0, /* unix_convert flags */ + (in_create_disposition == FILE_CREATE) ? + UCF_CREATING_FILE : 0, NULL, /* ppath_contains_wcards */ &smb_fname); if (!NT_STATUS_IS_OK(status)) { diff --git a/source3/smbd/smbd.h b/source3/smbd/smbd.h index a5b211a..e769157 100644 --- a/source3/smbd/smbd.h +++ b/source3/smbd/smbd.h @@ -73,5 +73,6 @@ struct trans_state { #define UCF_COND_ALLOW_WCARD_LCOMP 0x00000004 #define UCF_POSIX_PATHNAMES 0x00000008 #define UCF_UNIX_NAME_LOOKUP 0x00000010 +#define UCF_CREATING_FILE 0x00000020 #endif /* _SMBD_SMBD_H */ -- 1.8.1.2 From abe2a3682a2ceb4183c0cba0185f33e3a1b1e036 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 19 Aug 2013 10:26:00 +0000 Subject: [PATCH 2/2] smbd: Simplify dropbox special case in unix_convert EACCESS needs special treatment: If we want to create a fresh file, return OBJECT_PATH_NOT_FOUND, so that the client will continue creating the file. If the client wants us to open a potentially existing file, we need to correctly return ACCESS_DENIED. This patch makes this behaviour hopefully a bit clearer than the code before did. Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Mon Aug 26 12:14:26 CEST 2013 on sn-devel-104 --- source3/smbd/filename.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index 4384f5a..68321ee 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -718,13 +718,30 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx, /* * ENOENT/EACCESS are the only valid errors - * here. EACCESS needs handling here for - * "dropboxes", i.e. directories where users - * can only put stuff with permission -wx. + * here. */ - if ((errno != 0) && (errno != ENOENT) - && ((ucf_flags & UCF_CREATING_FILE) && - (errno != EACCES))) { + + if (errno == EACCES) { + if (ucf_flags & UCF_CREATING_FILE) { + /* + * This is the dropbox + * behaviour. A dropbox is a + * directory with only -wx + * permissions, so + * get_real_filename fails + * with EACCESS, it needs to + * list the directory. We + * nevertheless want to allow + * users creating a file. + */ + status = NT_STATUS_OBJECT_PATH_NOT_FOUND; + } else { + status = NT_STATUS_ACCESS_DENIED; + } + goto fail; + } + + if ((errno != 0) && (errno != ENOENT)) { /* * ENOTDIR and ELOOP both map to * NT_STATUS_OBJECT_PATH_NOT_FOUND -- 1.8.1.2