Attachment 9168 Details for Bug 10114 – Patch

[patch] Patch

10114-4.1.patch (text/plain), 8.47 KB, created by Volker Lendecke on 2013-08-28 13:29:46 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Volker Lendecke

Created: 2013-08-28 13:29:46 UTC

Size: 8.47 KB

patch

obsolete

>From a8fa2fc0e987b80ffeaa239afbfa8b4b6cb16a8d Mon Sep 17 00:00:00 2001
>From: Volker Lendecke <Volker.Lendecke@SerNet.DE>
>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 <Volker.Lendecke@SerNet.DE>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>---
> 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 <vl@samba.org>
>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 <vl@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Volker Lendecke <vl@samba.org>
>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
>

Flags:

jra: review+

Actions: View

Attachments on bug 10114: 9168 | 9170