[PATCHSET] Fix regression for a directory dropbox (dir with perm -wx).
Jeremy Allison
jra at samba.org
Tue Dec 3 14:49:15 MST 2013
On Tue, Dec 03, 2013 at 01:20:15PM +0100, Andreas Schneider wrote:
> Hi Jeremy,
>
> with Samba 4.0 we introduced a regression. We are not able to write to a
> dropbox (directory with permission -wx, 0733). I've debugged the code and
> discussed it with Volker. We came up with the attached patches. We need your
> input to this code, please review and comment!
Ok, here is the modified version of the patch.
The key change to the previous version is that we
now ignore ACCESS_DENIED returned from a filename_convert()
call in any of the SMB_VFS_CREATE_FILE() paths, allowing
the real underlying open call to catch any access
denieds that should be returned here.
Removing a 'r' permission on a directory should
only prohibit the reading of the filenames inside
a directory, not any other activity (such as
creating or opening files) - creating a file
is controlled by the 'w' bit on the directory,
and opening a file is controlled by the ACL
on the underlying file itself.
So the core of the patch is changing UCF_CREATING_FILE
to UCF_OPENING_FILE, and ensuring this is passed
into filename_convert() on any path that leads
to a SMB_VFS_CREATE_FILE() call.
This patch passes make test for me, and also the
hand crafted tests I've done with smbclient over
SMB1 and SMB2/3.
Please review.
Thanks !
Jeremy.
-------------- next part --------------
From 8d401132c07e6531f4dda1f5f756700492895f74 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 3 Dec 2013 13:20:17 +0100
Subject: [PATCH 1/3] smbd: Fix regression for the dropbox case.
We need to allow to save a file to a directory with perm -wx.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=10297
Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
source3/smbd/filename.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 68321ee..3096a3e 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -722,7 +722,10 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
*/
if (errno == EACCES) {
- if (ucf_flags & UCF_CREATING_FILE) {
+ if ((ucf_flags & UCF_CREATING_FILE) == 0) {
+ status = NT_STATUS_ACCESS_DENIED;
+ goto fail;
+ } else {
/*
* This is the dropbox
* behaviour. A dropbox is a
@@ -734,11 +737,8 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
* nevertheless want to allow
* users creating a file.
*/
- status = NT_STATUS_OBJECT_PATH_NOT_FOUND;
- } else {
- status = NT_STATUS_ACCESS_DENIED;
+ errno = 0;
}
- goto fail;
}
if ((errno != 0) && (errno != ENOENT)) {
--
1.8.4.1
From 5a9b96735df0cd18a3e93f569a404cc803a23149 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 3 Dec 2013 10:19:09 -0800
Subject: [PATCH 2/3] smbd: change flag name from UCF_CREATING_FILE to
UCF_OPENING_FILE
In preparation to using it for all open calls.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=10297
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/filename.c | 2 +-
source3/smbd/nttrans.c | 4 ++--
source3/smbd/reply.c | 10 +++++-----
source3/smbd/smb2_create.c | 2 +-
source3/smbd/smbd.h | 2 +-
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
index 3096a3e..d0d36d0 100644
--- a/source3/smbd/filename.c
+++ b/source3/smbd/filename.c
@@ -722,7 +722,7 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx,
*/
if (errno == EACCES) {
- if ((ucf_flags & UCF_CREATING_FILE) == 0) {
+ if ((ucf_flags & UCF_OPENING_FILE) == 0) {
status = NT_STATUS_ACCESS_DENIED;
goto fail;
} else {
diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index e901ff2..c6dd541 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -539,7 +539,7 @@ void reply_ntcreate_and_X(struct smb_request *req)
req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
(create_disposition == FILE_CREATE)
- ? UCF_CREATING_FILE : 0,
+ ? UCF_OPENING_FILE : 0,
NULL,
&smb_fname);
@@ -1068,7 +1068,7 @@ static void call_nt_transact_create(connection_struct *conn,
req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
(create_disposition == FILE_CREATE)
- ? UCF_CREATING_FILE : 0,
+ ? UCF_OPENING_FILE : 0,
NULL,
&smb_fname);
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index ce1a127..6f7eb40 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -1918,7 +1918,7 @@ void reply_open(struct smb_request *req)
req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
(create_disposition == FILE_CREATE)
- ? UCF_CREATING_FILE : 0,
+ ? UCF_OPENING_FILE : 0,
NULL,
&smb_fname);
if (!NT_STATUS_IS_OK(status)) {
@@ -2087,7 +2087,7 @@ void reply_open_and_X(struct smb_request *req)
req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
(create_disposition == FILE_CREATE)
- ? UCF_CREATING_FILE : 0,
+ ? UCF_OPENING_FILE : 0,
NULL,
&smb_fname);
if (!NT_STATUS_IS_OK(status)) {
@@ -2320,7 +2320,7 @@ void reply_mknew(struct smb_request *req)
conn,
req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
- UCF_CREATING_FILE,
+ UCF_OPENING_FILE,
NULL,
&smb_fname);
if (!NT_STATUS_IS_OK(status)) {
@@ -2461,7 +2461,7 @@ void reply_ctemp(struct smb_request *req)
status = filename_convert(ctx, conn,
req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
- UCF_CREATING_FILE,
+ UCF_OPENING_FILE,
NULL,
&smb_fname);
if (!NT_STATUS_IS_OK(status)) {
@@ -5820,7 +5820,7 @@ void reply_mkdir(struct smb_request *req)
status = filename_convert(ctx, conn,
req->flags2 & FLAGS2_DFS_PATHNAMES,
directory,
- UCF_CREATING_FILE,
+ UCF_OPENING_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 38eba4f..c4f5c3c 100644
--- a/source3/smbd/smb2_create.c
+++ b/source3/smbd/smb2_create.c
@@ -892,7 +892,7 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx,
smb1req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
(in_create_disposition == FILE_CREATE) ?
- UCF_CREATING_FILE : 0,
+ UCF_OPENING_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 e769157..d1436e8 100644
--- a/source3/smbd/smbd.h
+++ b/source3/smbd/smbd.h
@@ -73,6 +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
+#define UCF_OPENING_FILE 0x00000020
#endif /* _SMBD_SMBD_H */
--
1.8.4.1
From 1acb6accc43bfab70c7624442b8963188b800ed0 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 3 Dec 2013 10:21:16 -0800
Subject: [PATCH 3/3] smbd: Always use UCF_OPENING_FILE for filename_convert
calls to resolve a path for open.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=10297
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/nttrans.c | 6 ++----
source3/smbd/reply.c | 6 ++----
source3/smbd/smb2_create.c | 3 +--
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index c6dd541..b67c2e2 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -538,8 +538,7 @@ void reply_ntcreate_and_X(struct smb_request *req)
conn,
req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
- (create_disposition == FILE_CREATE)
- ? UCF_OPENING_FILE : 0,
+ UCF_OPENING_FILE,
NULL,
&smb_fname);
@@ -1067,8 +1066,7 @@ static void call_nt_transact_create(connection_struct *conn,
conn,
req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
- (create_disposition == FILE_CREATE)
- ? UCF_OPENING_FILE : 0,
+ UCF_OPENING_FILE,
NULL,
&smb_fname);
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 6f7eb40..ca4674d 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -1917,8 +1917,7 @@ void reply_open(struct smb_request *req)
conn,
req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
- (create_disposition == FILE_CREATE)
- ? UCF_OPENING_FILE : 0,
+ UCF_OPENING_FILE,
NULL,
&smb_fname);
if (!NT_STATUS_IS_OK(status)) {
@@ -2086,8 +2085,7 @@ void reply_open_and_X(struct smb_request *req)
conn,
req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
- (create_disposition == FILE_CREATE)
- ? UCF_OPENING_FILE : 0,
+ UCF_OPENING_FILE,
NULL,
&smb_fname);
if (!NT_STATUS_IS_OK(status)) {
diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c
index c4f5c3c..9f0e103 100644
--- a/source3/smbd/smb2_create.c
+++ b/source3/smbd/smb2_create.c
@@ -891,8 +891,7 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx,
smb1req->conn,
smb1req->flags2 & FLAGS2_DFS_PATHNAMES,
fname,
- (in_create_disposition == FILE_CREATE) ?
- UCF_OPENING_FILE : 0,
+ UCF_OPENING_FILE,
NULL, /* ppath_contains_wcards */
&smb_fname);
if (!NT_STATUS_IS_OK(status)) {
--
1.8.4.1
More information about the samba-technical
mailing list