[Patch] Regression fix for roaming profiles (bug #10297)

Stefan Metzmacher metze at samba.org
Sat Oct 22 08:15:08 UTC 2016


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.

Thanks!
metze
-------------- next part --------------
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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20161022/a006d5ab/signature.sig>


More information about the samba-technical mailing list