asn is 'other' in case of 0733, so falling into the 3 case. /srv/samba/dropbox/dirmode733 is owned by root.

Created attachment 15788 [details]
A 'dropbox' test

Here is a reproducer.

Created attachment 15789 [details]
Add improved dropbox test

We can't revert cd66614a4e8a8f93e1debb8a87dd2afd46b02990. It's part of the move to always use file handles everywhere in the VFS.

We may have to abandon this functionality, it's not safe to use.

Thinking more about this, use of Samba as a dropbox into a directory we can't read is counter to good security practice.

Right now I don't see a way to fix this and still maintain secure access. Andreas, can you think of any ways this could work given we must have fd access to all directories we operate within ?

I'm close to "WILLNOTFIX"..

Please don't close it as WONTFIX for now. If we do not support this anymore we probably should write that down in WHATSNEW.txt at least for 4.12.

Sure, I *didn't* close it :-). I just don't currently see how this can be fixed.

Jeremy, wouldn't the O_PATH we were discussing elsewhere also allow this to work?

No, O_PATH doesn't allow reading directory contents.

I need to investigate closer, but after a conversation with Volker the issue is the difference between detecting if a path is valid (for which O_PATH would work) and opening a directory to read the contents (for which O_PATH doesn't, we need O_RDONLY|O_DIRECTORY).

Volker may be correct in that the directory open with read is being done in order to try and do case insensitive matching in the pathname lookup.

In which case, for the dropbox case we simply have to ignore case insensitive name matching and always use the name as given by the client.

(In reply to Jeremy Allison from comment #9)
we're not interested in listing directory content here. All we need is an fd referring to the parent dir, so we can fchdir() to dir.

This is basically the same problem brought up by Ralf Würthner.

I need to investigate, but if the ACCESS_DENIED error is coming from the "get_real_filename()" codepath inside source3/smbd/filename.c we may be able to fix this by deliberately going into the code here:

1026                                 /*
1027                                  * ENOENT/EACCESS are the only valid errors
1028                                  * here.
1029                                  */
1030 
1031                                 if (errno == EACCES) {
1032                                         if ((ucf_flags & UCF_PREP_CREATEFILE) == 0) {
1033                                                 status = NT_STATUS_ACCESS_DENIED;
1034                                                 goto fail;
1035                                         } else {
1036                                                 /*
1037                                                  * This is the dropbox
1038                                                  * behaviour. A dropbox is a
1039                                                  * directory with only -wx
1040                                                  * permissions, so
1041                                                  * get_real_filename fails
1042                                                  * with EACCESS, it needs to
1043                                                  * list the directory. We
1044                                                  * nevertheless want to allow
1045                                                  * users creating a file.
1046                                                  */
1047                                                 errno = 0;
1048                                         }
1049                                 }

Ralph, that's incorrect, in pathname lookup we *are* interesting in enumerating the directory contents in order to match case insensitive names.

(In reply to Jeremy Allison from comment #10)
Case sensitivity, right, we're screwed. :)

(In reply to Jeremy Allison from comment #12)
That smells like as if it would allow creating files only differing in case in a drobox. Is that what we want?

Andreas, we can split this into 2 problems.

Doing:

cd dirmode733

in smbclient calls:

source3/client/client.c:do_cd()

which does cli_resolve_path(), which over SMB2 I gets a handle to the directory. That is failing with ACCESS_DENIED as we're almost certainly asking for read access on the directory (we'll want to look up attributes etc).

For SMB2 POSIX that specific case could be fixed by using O_PATH in open_directory() when the open request doesn't include read directory contents access, only traverse. That way we'd still maintain a fd handle on the directory but the client would never read it (as they promised).

In order to do a dropbox using smbclient you don't have to cd into the directory. You can instead do:

put \full\pathname\to\dropbox\file.

I'll check this. It may still work as doing vfs_ChDir() on /full/pathname/to/dropbox should still succeed.

Ralph wrote:

> That smells like as if it would allow creating files only differing in case in
> a dropbox. Is that what we want?

Yes, that has always been the case. The current code inside source3/smbd/filename.c has always allowed this. If we can't look up existing filename how could it not ?

Historically this hasn't been a problem :-).

> I'll check this. It may still work as doing vfs_ChDir() on /full/pathname/to/dropbox should still succeed.

Yes, see the initial description. The vfs_ChDir() succeeds but the vfs_open(".", O_RDONLY|O_DIRECTORY, 0) fails with "Permisson denied".

Yes, this still works (just checked master).

Andreas, dropboxes still work. You just can't do a cd into the dropbox directory first. Just use the absolute path from the root of the share and you can create the file.

If you think about it, that makes perfect sense.

This may be a change in behavior, but I'm having a hard time seeing this as a bug.

Is the customer using smbclient to do this ?

If so, we *could* change the do_cd() command inside smbclient to ignore ACCESS_DENIED errors (we think the directory is good - we didn't get PATH_NOT_FOUND - we just can't get access anymore) and still set the internal string of the current directory to the requested cd target.

Then the put would work after that. It's a bit wonky though, as it leaves you in a current directory that's not accessible other than for "put" commands.

> Andreas, dropboxes still work. You just can't do a cd into the dropbox 
> directory first. Just use the absolute path from the root of the share and you
> can create the file.

Or cd into the directory above the dropbox if the dropbox isn't at the root of the share and then do:

put dropbox/file

will still work. It's the cd failing that is the actual change here. But from what the server sees the client asking for (read access on the cd directory) the server is doing the right thing.

Volker, for SMB2 POSIX we can make this work by changing the code in open_directory() in the server to use O_PATH on Linux if the client in a POSIX open only asks for SEC_DIR_TRAVERSE and no other flags.

This has been found by Red Hat QA as we have a regression test for it. It was broken in RHEL5 with Samba 3.5.x :-)

Looking at the original bug, the customer used smbclient to drop a file. Exactly the testcase I implemented.

I'm not 100% sure if we should ignore ACCESS_DENIED. We don't know if it is coming from the chdir or the open ...

Jeremy, putting the file in the dirmode733 directory from the share directory doesn't work either:

smb: \> put wurst.7623 dirmode733/wurst.7623
NT_STATUS_NOT_FOUND opening remote file \dirmode733\wurst.7623

(In reply to Andreas Schneider from comment #24)

Can you check in master first so I can see if it's a regression for 4.11 ? This works in master for me.

(In reply to Andreas Schneider from comment #23)

We can ignore the ACCESS_DENIED if it comes from the check_path code, as we know this is an attempt to get attributes on the directory. What we'd do if we get access denied is check the parent directory path, and if that works then we know the lower-level directory exists (otherwise we'd have gotten OBJECT_NOT_FOUND or NOT_A_DIRECTORY) so we can then set the smbclient internal path to the directory that returned ACCESS_DENIED. It would only then work for "put" commands though.

It's a horrible hack, and if the customer can just use the full path instead I'd rather not do it.

Created attachment 15790 [details]
dropbox test with 'put wurst dirmode733/wurst'

Yes, I'm testing on master. I've attached my modified test!

make -j20 test TESTS="samba3.blackbox.dropbox"

(In reply to Andreas Schneider from comment #27)

Can you try testing by hand first please. I tested by hand and it works fine. I don't want to end up debugging your test :-).

Note I'm testing against a share with no VFS modules loaded. Can you test by hand in the same way, and if it works for you then I think you need to look into your test environment.

Yep. Just confirmed testing by hand on master, it still works.

Share definition:

[tmp]
        path = /tmp
        read only = no

# mkdir /tmp/test
# chmod 733 /tmp/test
# ls -ld /tmp/test
drwx-wx-wx 2 root root 4096 Feb 14 09:16 /tmp/test

$ /usr/local/samba/bin/smbclient //127.0.0.1/tmp -Uuser%pass
smb: \> cd test
cd \test\: NT_STATUS_ACCESS_DENIED
smb: \> put look test/look
putting file look as \test\look (213.9 kb/s) (average 213.9 kb/s)

$ ls -l /tmp/test/look
-rw-rw-rw- 1 user roup 657 Feb 14 09:16 /tmp/test/look

I think you need to debug your test, sorry.

Test for this went into master as e2ea059e671140b32ea2ab1a35691a8e35d4a5f4

I think this shows it's not a bug, so closing as RESOLVED/WORKSFORME.