Bug 8950 - mount.cifs unable to access shares with read restricted at root level.
mount.cifs unable to access shares with read restricted at root level.
Status: NEW
Product: CifsVFS
Classification: Unclassified
Component: kernel fs
2.5
x64 Linux
: P5 critical
: ---
Assigned To: Steve French
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-21 15:31 UTC by Scott Purcell (mail address dead)
Modified: 2016-01-13 20:18 UTC (History)
14 users (show)

See Also:


Attachments
create placeholder inodes (1018 bytes, patch)
2013-11-04 16:59 UTC, shirishpargaonkar@gmail.com
no flags Details
create placeholder inodes for intermediate dentries (2.18 KB, patch)
2013-11-04 17:05 UTC, shirishpargaonkar@gmail.com
no flags Details
allow to mount a share with intermediate inaccessible paths (2.42 KB, patch)
2013-11-13 17:27 UTC, shirishpargaonkar@gmail.com
no flags Details
allow to mount a share with intermediate inaccessible paths using d_obtain and d_splice alias (3.84 KB, patch)
2014-02-26 20:30 UTC, shirishpargaonkar@gmail.com
no flags Details
408-thil-lss-noauth.pcap (2.36 KB, application/vnd.tcpdump.pcap)
2014-03-18 11:28 UTC, alex_y_xu
no flags Details
shared superblock mount changes for mounts without access to intermediate path (4.19 KB, patch)
2014-05-24 21:12 UTC, shirishpargaonkar@gmail.com
no flags Details
make shares unaccessible at root level mountable (15.43 KB, patch)
2016-01-08 15:08 UTC, Aurélien Aptel
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Scott Purcell (mail address dead) 2012-05-21 15:31:17 UTC
I need to access a share where I have restricted access at the root level, but r/w access to a subdirectory of the root.

mount.cifs and mount.smbfs recently lost the ability to access such shares (I was able to do so with the versions installed on Ubuntu 11.04 and am unable to do so with with the versions in Ubuntu 12.04).

smbclient, by contrast, is able to mount these shares.

A more full account of this issue can be found in the Samba mailing list beginning on 5/16/2012.  Network traffic captures were provided in off-list email exchanges with Steve French, Shirish Pargaonkar, and Gunter Kukkukk.

Jeff Layton described the problem as follows:

   Known problem since the superblock sharing patches went in. cifs.ko
   needs to establish a dentry and inode for the root of the share and
   then walks down to the "prefixpath" for the mount. Unfortunately if you
   don't have access to any point along that path, the mount will fail.

   There have been a couple of proposals to fix it, but they've had their
   own problems. What probably needs to happen is to do something like
   what NFS does in its superblock sharing model. Allow several trees of
   dentries within a superblock and only connect them later if we happen
   to stumble across the right entry. See commit 54ceac45159 for an
   explanation of the model NFS uses for this.

   -- 
   Jeff Layton <jlayton@samba.org>

Opening this bug report at the suggestion of Steve French so that it does not get overlooked.
Comment 1 Jonathan Nieder 2012-06-28 00:02:46 UTC
Reference: http://thread.gmane.org/gmane.network.samba.general/124822/focus=124872
Comment 2 Richard Ash 2012-11-05 17:03:24 UTC
This is still biting me in Linux 3.4.9, with exactly the same problem:
* I don't have access to the share root
* I do have access to a lower directory.
* smbclient can access the lower levels fine, but I can't mount them with mount.cifs.
* Getting the server administrator to fix this will be a long, tedious, process because it works just fine for all the Windows clients, and the administrators don't touch anything else.
Comment 3 Richard Ash 2012-11-06 22:06:14 UTC
I found at least some degree of work-around (although the performance is pretty poor): usmb can connect to and mount these shares through FUSE:
 http://ametros.net/code.html#usmb
all the other FUSE/libsmbclient tools I found insisted upon doing the same directory walk (for different reasons) as the cifs kernel module, and thus failing in the same way.
Comment 4 Suresh Jayaraman (mail address is dead) 2013-01-30 11:38:35 UTC
This sounds like a regression to me, isn't? May be we rushed through the current shared superblock patches..?
Comment 5 Shirish S. Pargaonkar 2013-02-11 16:10:46 UTC
I do not see this happening with 2.0 version of cifs vfs client.
For example, I have a Windows 2003 server which a share exported
that is owned by Administrator with a security descriptor with only
Administrator access.
As an another user (not Administrator), I can authenticate and
mount a directory, which is owned by this user, under this share.
There is no access denied error while browsing the mounted directory.
Comment 6 Shirish S. Pargaonkar 2013-09-01 16:17:24 UTC
I really do not see the problem.  I would like to work on the fix
but having hard time recreating the problem.

Here are the steps I do and do not see the problem in accessing
files/directories that an authenticated user has permissions
even if he/she does not have access permissions on prior directories.


I am using 

sudo modinfo cifs
filename:       /lib/modules/3.10.0+/kernel/fs/cifs/cifs.ko
version:        2.01

and a Windows 8 Pro server with sharename public2 and a directory 
underneath named sspdir.

---------------
I think this is somewhat consistent with what has been repoted.

$ sudo smbclient //192.168.1.67/public2 -U ssp%<password>
Domain=[SSPWIN8] OS=[Windows 8 Pro with Media Center 9200] Server=[Windows 8 Pro with Media Center 6.2]
smb: \> ls
NT_STATUS_ACCESS_DENIED listing \*
smb: \> ls sspdir
NT_STATUS_ACCESS_DENIED listing \sspdir
smb: \> cd sspdir
smb: \sspdir\> ls
  .                                   D        0  Sun Sep  1 09:36:14 2013
  ..                                  D        0  Sun Sep  1 09:36:14 2013
  sspfile1                            A       25  Sun Sep  1 09:36:14 2013

		57624 blocks of size 8388608. 51639 blocks available
smb: \sspdir\> 

---------------
But I do not see the same problem with cifs client (with vers=3.0 and vers=1.0)

$ sudo mount -t cifs //192.168.1.67/public2 /mnt/smb_a -o sec=ntlmsspi,vers=3.0,user=ssp,pass=<password>

$ ls /mnt/smb_a
ls: reading directory /mnt/smb_a: Permission denied

$ ls /mnt/smb_a/sspdir
sspfile1

$ cd /mnt/smb_a/sspdir

$ ls
sspfile1


--------------------
I can also do this:

$ sudo mount -t cifs //192.168.1.67/public2/sspdir /mnt/smb_a -o sec=ntlmsspi,vers=3.0,user=ssp,pass=<password>

$ ls /mnt/smb_a
sspfile1


--------------

These are the permissions at the server:

$ smbcacls //192.168.1.67/public2 sspdir -U ssp%<password>
REVISION:1
CONTROL:0x9404
OWNER:sspwin8\ssp
GROUP:sspwin8\None
ACL:sspwin8\ssp:ALLOWED/OI|CI/FULL

I can not list (smb)cacls of the share, public2, but if I could, they
are something like this:


REVISION:1
CONTROL:0x9404
OWNER:sspwin8\Shirish
GROUP:sspwin8\None
ACL:nt authority\system:ALLOWED/OI|CI/FULL
ACL:builtin\Administrators:ALLOWED/OI|CI/FULL
ACL:sspwin8\Shirish:ALLOWED/OI|CI/FULL
Comment 7 Jeff Layton 2013-09-01 18:43:20 UTC
(In reply to comment #6)
> I really do not see the problem.  I would like to work on the fix
> but having hard time recreating the problem.

[...]

> 
> $ sudo mount -t cifs //192.168.1.67/public2 /mnt/smb_a -o
> sec=ntlmsspi,vers=3.0,user=ssp,pass=<password>
> 
> $ ls /mnt/smb_a
> ls: reading directory /mnt/smb_a: Permission denied
> 
> $ ls /mnt/smb_a/sspdir
> sspfile1
> 

Assuming that you have the same problem with SMB1, if you're able to mount the root of the share, then you're almost certainly able to issue a QUERY_PATH_INFO against the root as well (a network capture would confirm this), though you may not be able to do a FIND_FIRST/NEXT in that directory.

The problem in this bug crops up when you're not allowed to even do a QPathInfo against an intermediate directory between the root of the share and the directory within that share that you want to serve as the root of the mount.

To reproduce this, you may need to tweak the ACLs a bit more, or perhaps have another level of directory in there that has very restrictive permissions.
Comment 8 Shirish S. Pargaonkar 2013-09-08 04:45:05 UTC
ok, I tweaked this acl


REVISION:1
CONTROL:0x9404
OWNER:sspwin8\Shirish
GROUP:sspwin8\None
ACL:nt authority\system:ALLOWED/OI|CI/FULL
ACL:builtin\Administrators:ALLOWED/OI|CI/FULL
ACL:sspwin8\Shirish:ALLOWED/OI|CI/FULL

to


REVISION:1
CONTROL:0x9404
OWNER:sspwin8\Shirish
GROUP:sspwin8\None
ACL:sspwin8\Shirish:ALLOWED/OI|CI/FULL

and now user ssp can't mount the share (wither public2 or
public2/sspdir) since it is denied access to the root of the share
itself when doing create request file.

That is what happens to a Windows 7 client when it tries to mount
and access the share or a subdirectory under the share.

smbclient can mount the share and can cd to the sspdir but
that does not mean much since as it tries to access files under
sspdir, it is denied access.

So none of the clients, Windows 7, smbclient, and cifs vfs,
as authenticated user ssp can go past the (root of the) share over to a
directory which user ssp owns/has access to.

This behaviour is against the Windows 8 Pro server.

So if this is how a Windows server is responding to all kinds
of client, I am still not sure what a cifs client is expected to do
that a server does not permit.
Comment 9 shirishpargaonkar@gmail.com 2013-09-08 05:09:19 UTC
I think I need to re-read and understand these lines that Jeff added
in prior comments.

"The problem in this bug crops up when you're not allowed to even do a QPathInfo
against an intermediate directory between the root of the share and the
directory within that share that you want to serve as the root of the mount."

Will try to recreate this instead of what I was doing (denying a user
access to the root of the share instead of denying access to an
intermediate directory).
Comment 10 alex_y_xu 2013-09-08 13:40:44 UTC
My use case (doesn't work with cifs):

We have networked home directories (not profiles), which are on a shared server (call it server) and inside a share (call it share$).

So, on Windows, H: -> \\server\share$\username

We are not permitted to list the contents of the share$, but are free to read/write/delete inside username.

This works with smbclient (smbclient -U username '\\server\share$', cd username, ls) and on Windows (obviously), but not with CIFS.
Comment 11 shirishpargaonkar@gmail.com 2013-09-08 15:56:09 UTC
OK, I finally have the set up right, with correct ACLs aligned so that
I can see the problem using cifs client but not with Windows client and 
smbclient.
On to understanding various proposed solutions and implementing one.
Comment 12 shirishpargaonkar@gmail.com 2013-10-13 22:01:24 UTC
"1) if the dentries to get down to the root of the mount don't already
exist, then attach some sort of "placeholder" inode that can be fleshed
out later if and when the dentry is accessed via other means."

Once a dentry for intermediate inaccessible path(s) is/are created
with "placeholder" inode, that dentry may/will need to be revalidated.
For a shared superblock, not sure how a dentry with "placeholder" inode
can be accessed to flesh out the inode.

I think the challenge here is to create a "placeholder" inode
(in cifs_get_root()) and once that is created to sometimes validate
that dentry/inode.
Comment 13 shirishpargaonkar@gmail.com 2013-10-21 20:41:37 UTC
I think it is possible to handle lookup/revalidate errors/failures on
inaccessible intermediate paths but I am not sure how to handle
revalidates on these intermediate paths in other places.


For example, we could have paths b and d inaccessible.
So we can hanele mount of /a/b/c/d/e on mountpoint /mnt/m and
mount of /a/b/c on mount point /mnt/n.
But a stat of /mnt/n/d could end up purging dentry for d and
then we have disconnected paths to e.
Comment 14 shirishpargaonkar@gmail.com 2013-10-21 20:44:17 UTC
Addng the same note with some corrections..

----
I think it is possible to handle lookup/revalidate errors/failures on
inaccessible intermediate paths in cifs_get_root() but I am not sure how to 
handle revalidates on these intermediate paths in other places.


For example, we could have paths b and d inaccessible.
So we can handle mount of /a/b/c/d/e on mountpoint /mnt/m and
mount of /a/b/c on mount point /mnt/n.
But a stat of /mnt/n/d could end up purging dentry for d and
then we have disconnected paths to e.
Comment 15 shirishpargaonkar@gmail.com 2013-11-04 16:59:19 UTC
Created attachment 9369 [details]
create placeholder inodes

So I came up with something like this and it works.

But I think basic problem/idea is:

--
Once we create these dentries with "placeholder" inodes, we want them to
remain in the dcache.
Thus either the "placeholder" inodes for these dentries should not be
revalidated and if they should be revalidatable, if they do not get validated 
(as they would not) and dentry turns negative, it should somehow remain in 
dcache.
--

So perhaps either unset the bit DCACHE_OP_REVALIDATE of d_flags of the
dentries with placeholder inode or set their d_op to NULL.
Comment 16 shirishpargaonkar@gmail.com 2013-11-04 17:05:00 UTC
Created attachment 9370 [details]
create placeholder inodes for intermediate dentries

oops, wrong attachment in the previous note....

So I came up with something like this and it works.

But I think basic problem/idea is:

--
Once we create these dentries with "placeholder" inodes, we want them to
remain in the dcache.
Thus either the "placeholder" inodes for these dentries should not be
revalidated and if they should be revalidatable, if they do not get validated 
(as they would not) and dentry turns negative, it should somehow remain in 
dcache.
--

So perhaps either unset the bit DCACHE_OP_REVALIDATE of d_flags of the
dentries with placeholder inode or set their d_op to NULL.
Comment 17 shirishpargaonkar@gmail.com 2013-11-06 16:19:14 UTC
Thanks Jeff.

As pointed out on the linux-fsdevel mailing list that should not
manipulate DCACHE_OP_REVALIDATE since it does not apply here.
So I think one solution is to have for the intermediate paths,
dentry d_ops with a d_revalidate method such that if a dentry can't be 
validated, keep it instantiated with placeholder inode and if it can be, with 
the real inode (info).
Comment 18 shirishpargaonkar@gmail.com 2013-11-13 17:27:15 UTC
Created attachment 9414 [details]
allow to mount a share with intermediate inaccessible paths

I was able to make mounts work with this patch.
If anybody would care to test, it would be nice.

Basically, I tested thus:

mount a share /<sharename>/a/b on /mnt/m as user x on the client
where user x can't access a on the server
mount a share /<sharename>     on /mnt/n as user x
this sharing superblock.

stat /mnt/n/a without permission on a
stat /mnt/n/a after changing / with permissions on a
stat /mnt/n/a without permission on a
Comment 19 shirishpargaonkar@gmail.com 2013-11-15 17:52:32 UTC
I did further testing like this:

For the same authenticated user:

mount /<share>/a/b on /mnt/smb_m
mount /<share>     on /mnt/smb_n

stat /mnt/smb_n/a    - which can't validate but d_count of dentry for a is > 0

sleep 5

mount /<share>/a/c on /mnt/smb_o
stat /mnt/smb_n/a

umount /mnt/smb_m
stat /mnt/smb_n/a
sleep 5
mount /<share>/a/b on /mnt/smb_m
ls -l /mnt/smb_o
ls -l /mnt/smb_m

So d_count greater than one keep the dentry even if can't be invalidated.

I think I feel comfortable with this patch enough so will stop further
testing and post this patch on the mailing list sometimes today or so
and await for any comments.
Comment 20 shirishpargaonkar@gmail.com 2013-11-29 16:00:46 UTC
I had to  DENY   Read Attribute in Special Permissions for a directory
for an user to make smb2 create request with access mask 0x80 to fail.
The set up I can use to generate EACCES, finally.
Comment 21 shirishpargaonkar@gmail.com 2014-01-06 06:01:31 UTC
Need to mimic what nfs does in such cases.  That is the only solution,
that is what it looks like.
Spent quite some time over the break, have a faint idea now how to 
implement it but not fully.  Will spend time on this next week and 
thereafter along with rest of the work.

Basically trying to figure out how to create multiple vfsmount and root
dentry pairs (sub-trees) within a shared superblock and how a sub-tree
can become/materialise/splice (after being identified with a certainty)
as a branch in another (sub)tree when otherwise inacessbile parent of a
sub-tree becomes accessible.  Not sure whether we need to create
a false root for root dentry of the superblock (what would be the inode
for this fake dentry?) Something like that...
Comment 22 alex_y_xu 2014-02-12 15:23:36 UTC
Still doesn't work with patch:

# cd /tmp
# mkdir dir
# mount -t cifs -o user=[snip],domain=[snip] //[snip]/[snip]\ [snip]$/[snip] dir
Password for [snip]@//[snip]/[snip] [snip]$/[snip]
mount error(13): Permission denied
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)
# dmesg | tail -n 5
address conversion returned 1 for [snip]
Max buf = 16644
CIFS VFS: cifs_read_super: get root inode failed
SendRcvNoRsp flags 64 rc 0
SendRcvNoRsp flags 64 rc 0
Comment 23 shirishpargaonkar@gmail.com 2014-02-12 15:26:06 UTC
(In reply to comment #22)
> Still doesn't work with patch:

No it would not, the patch is incorrect. About to resume work on this
in a day or two.
Comment 24 shirishpargaonkar@gmail.com 2014-02-18 17:17:07 UTC
I think I understand creating several trees within a superblock.
Working on figuring out how to connect/splice them together.
Comment 25 shirishpargaonkar@gmail.com 2014-02-18 22:22:14 UTC
I think it is basically down to calling d_obtain_alias() in
cifs_get_root() and calling d_materialise_unique() in cifs_lookup().
I do not think d_splice_alias() would be useful for this problem.

Following the discussion on mailing lists related to these function
calls.
Comment 26 shirishpargaonkar@gmail.com 2014-02-26 20:30:19 UTC
Created attachment 9727 [details]
allow to mount a share with intermediate inaccessible paths using d_obtain and d_splice alias

Second attempt with this patch posted on the mailing list.
Waiting for the feedback.
Comment 27 shirishpargaonkar@gmail.com 2014-03-05 07:23:39 UTC
Somehow, for a (root) disconnected dentry obtained by d_obtain_alias,
inspite of having DCACHE_OP_DELETE (d_flag value is 0x100002c and lockref.count
is 1), d_delete method does not get invoked during unmount (as if dput does not 
get invoked).

For noserverino case, I rely on d_delete method of such disconnected entries 
to clean-up list elements maintained in cifs_sb_info structure.

Will resume debugging tomorrow, hopefully this is the last hurdle to
overcome to make superblock sharing code to work for all the cases.
Comment 28 alex_y_xu 2014-03-06 22:05:12 UTC
Still doesn't work with patch "try #2".
Comment 29 alex_y_xu 2014-03-06 22:06:22 UTC
P.S. see comment 22.

if cifsFYI etc is needed, feel free to ask.
Comment 30 shirishpargaonkar@gmail.com 2014-03-07 05:38:32 UTC
(In reply to comment #29)
> P.S. see comment 22.
> 
> if cifsFYI etc is needed, feel free to ask.

Hi, Thanks for testing. Although the patch needs more work and working
on that, the basic mount should have worked. I did try against a Windows
8.1 server and it did mount shares for smb1 and 2 and 3.

Would you be able to attach a wireshark trace and the command you attempt
to mount the share?
Comment 31 alex_y_xu 2014-03-07 11:40:06 UTC
(In reply to comment #30)
> (In reply to comment #29)
> > P.S. see comment 22.
> > 
> > if cifsFYI etc is needed, feel free to ask.
> 
> Hi, Thanks for testing. Although the patch needs more work and working
> on that, the basic mount should have worked. I did try against a Windows
> 8.1 server and it did mount shares for smb1 and 2 and 3.
> 
> Would you be able to attach a wireshark trace and the command you attempt
> to mount the share?

The latter is available in comment 22 ([snip]ped of internal information).

The former will have to wait until Monday, as I do not have access to the server at the moment.

I do know that the server is likely Windows Server 2000 or Server 2003, but may be 2008 or R2.

I tried both with and without vers=1.0, but I don't know what the server actually supports, so that may or may not have actually made a difference.
Comment 32 shirishpargaonkar@gmail.com 2014-03-10 05:50:37 UTC
(In reply to comment #27)
> Somehow, for a (root) disconnected dentry obtained by d_obtain_alias,
> inspite of having DCACHE_OP_DELETE (d_flag value is 0x100002c and lockref.count
> is 1), d_delete method does not get invoked during unmount (as if dput does not 
> get invoked).
> 

Not a correct observation. does not matter if the root dentry is a regular or
anonymous.

Need to figure out a way to clean up entries added to cifs_sb_info
during d_obtain_alias for anonymous root dentries for the case of noserverino 
kind of servers if d_splice_alias does not happen before umount
so the respective/added entries to cifs_sb_info are cleaned up.
Comment 33 shirishpargaonkar@gmail.com 2014-03-10 05:59:02 UTC
What I need to figure out is how does a filesystem (in this case, cifs)
know when one of its mounts is being unmounted!
Comment 34 shirishpargaonkar@gmail.com 2014-03-11 19:10:26 UTC
d_release of a dentry works.

During umount, d_delete may not always be called on the root dentries
because one may have (child) dentries referring to it (including negative 
dentries).

d_release will always be called, so the bookkeeping will happen while
getting rid of hte superblock.

The entries will always be added to begining of the list in cifs_sb_info,
so that will take care of finding the correct inode for the matched
path for a disconnected/anonymous root dentry (mount).

Coding all of this and testing.

And (hopefully) waiting for the wireshark trace to debug the
reported failure.
Comment 35 alex_y_xu 2014-03-11 19:16:51 UTC
(In reply to comment #34)
> d_release of a dentry works.
> 
> During umount, d_delete may not always be called on the root dentries
> because one may have (child) dentries referring to it (including negative 
> dentries).
> 
> d_release will always be called, so the bookkeeping will happen while
> getting rid of hte superblock.
> 
> The entries will always be added to begining of the list in cifs_sb_info,
> so that will take care of finding the correct inode for the matched
> path for a disconnected/anonymous root dentry (mount).
> 
> Coding all of this and testing.
> 
> And (hopefully) waiting for the wireshark trace to debug the
> reported failure.

I lied, I'm actually on break this week. I'll get it next week; ping me via email around 9:00 EDT Monday if you want to remind me.
Comment 36 alex_y_xu 2014-03-18 11:28:57 UTC
Created attachment 9787 [details]
408-thil-lss-noauth.pcap

pcap file attached. I have filtered by smb and not smb.security_blob to prevent offline cracking of my password.
Comment 37 shirishpargaonkar@gmail.com 2014-03-18 17:44:10 UTC
(In reply to comment #36)
> Created attachment 9787 [details]
> 408-thil-lss-noauth.pcap
> 
> pcap file attached. I have filtered by smb and not smb.security_blob to prevent
> offline cracking of my password.

Looks like user has access to directory/path 0727417143 under the share
(name) 408 students$ but has no/restricted access/permissions on the share
itself.
Can this user mount just the share i.e. instead of 408 students$/0727417143,
mount just 408 students$?
Comment 38 shirishpargaonkar@gmail.com 2014-03-19 16:02:12 UTC
Looked at the wireshark trace again.

The access denied is returned while querrying the share (directory/folder)
on the server during the process to create root dentry for the superblock.

I am not sure yet how to have such a setup where a share allows an
authenticated user to tree connect but returns access denied to query
the share (directory/folder).

But although similar, this is a different problem than the one where
a user does not have access to an intermediate path to a directory/folder
being mounted.

So the patch I have posted will not work for this case.

I am not sure how to handle a case where we can't obtain (by querying)
information to create an inode for the root dentry for a superblock
itself.
Comment 39 shirishpargaonkar@gmail.com 2014-04-07 04:19:06 UTC
Asked this question on MSDN.  Hope someone responds...
If there is a way to do this on a Samba server, that would be useful too.

"
I have a share (for a folder named public) where Authenticated User (group) has Sharing Permission as Read and Security Permissions as

Allow Traverse Folder / Execute File.  There is a folder (named user1folder) under the folder public, to which an user name user1 has Full Access.

So I expect user1 as an autheticated user over network trying to mount a folder user1folder to do a successful query path info

but acces denied to query path info on share/folder public.

But that does not happen.  Query path info on share/folder public also succeeds.

I also tried Everyone (group) and user1 instead of Authenticated User (group) but no luck.

Basically, on this Windows 7 SMB server, I would like to have an authenticated user, denied access if it attempts to mount

the shared folder but be able to access a folder under this shared folder to which this authenticated user has full access.
"
Comment 40 shirishpargaonkar@gmail.com 2014-04-14 02:54:47 UTC
How can query path info on shared folder C:\public2 succeed
is really puzzling inspite of these Sharing and Security permissions.


C:\>net share public2
Share name        public2
Path              C:\public2
Remark
Maximum users     No limit
Users
Caching           Manual caching of documents
Permission        server-PC\owner1, FULL
                  server-PC\user1, READ
The command completed successfully.

C:\>cacls public2
C:\public2 server-PC\user1:(DENY)(special access:)
                             DELETE
                             READ_CONTROL
                             WRITE_DAC
                             WRITE_OWNER
                             STANDARD_RIGHTS_REQUIRED
                             FILE_READ_DATA
                             FILE_WRITE_DATA
                             FILE_APPEND_DATA
                             FILE_READ_EA
                             FILE_WRITE_EA
                             FILE_DELETE_CHILD
                             FILE_READ_ATTRIBUTES
                             FILE_WRITE_ATTRIBUTES
           server-PC\user1:(special access:)
                             SYNCHRONIZE
                             FILE_EXECUTE
           server-PC\owner1:(OI)(CI)F

C:\>icacls public2
public2 server-PC\user1:(DENY)(R,W,D,WDAC,WO,DC)
        server-PC\user1:(S,X)
        server-PC\owner1:(OI)(CI)(F)
Comment 41 shirishpargaonkar@gmail.com 2014-05-14 00:26:35 UTC
I think I have the setup.  I had to change permissions (deny ReadData) on the 
parent directory of the leaf of the share path so that 
query path info fails.
Looking at events in the log (event 5145 on Windows 2012 server) was helpful.
Will work on a meaningful patch and try to post soon.
Comment 42 shirishpargaonkar@gmail.com 2014-05-14 00:53:32 UTC
This is the setup that works, thought will just log it here for reference...

PS C:\Users\Administrator> net share public3$
Share name        public3$
Path              C:\Shares\public3$
Remark
Maximum users     No limit
Users
Caching           Manual caching of documents
Permission        Everyone, READ

The command completed successfully.

PS C:\> net user user1
User name                    user1
Full Name                    user1
Comment
User's comment
Country/region code          000 (System Default)
Account active               Yes
Account expires              Never

Password last set            4/26/2014 11:01:45 PM
Password expires             Never
Password changeable          4/26/2014 11:01:45 PM
Password required            Yes
User may change password     Yes

Workstations allowed         All
Logon script
User profile
Home directory
Last logon                   5/12/2014 6:53:16 PM

Logon hours allowed          All

Local Group Memberships      *Users
Global Group memberships     *None
The command completed successfully.


PS C:\Users\Administrator> cacls c:\shares
c:\Shares BUILTIN\Users:(DENY)(special access:)
                        FILE_READ_DATA

          NT AUTHORITY\SYSTEM:(OI)(CI)(ID)F
          BUILTIN\Administrators:(OI)(CI)(ID)F
          BUILTIN\Users:(OI)(CI)(ID)R
          BUILTIN\Users:(CI)(ID)(special access
                                FILE_APPEND_DAT

          BUILTIN\Users:(CI)(ID)(special access
                                FILE_WRITE_DATA

          CREATOR OWNER:(OI)(CI)(IO)(ID)F

PS C:\> cacls c:\shares\public3$
c:\shares\public3$ WIN-VA77GADM8UM\user1:(DENY)(special access:)
                                           DELETE
                                           READ_CONTROL
                                           WRITE_DAC
                                           WRITE_OWNER
                                           STANDARD_RIGHTS_REQUIRED
                                           FILE_READ_DATA
                                           FILE_WRITE_DATA
                                           FILE_APPEND_DATA
                                           FILE_READ_EA
                                           FILE_WRITE_EA
                                           FILE_DELETE_CHILD
                                           FILE_READ_ATTRIBUTES
                                           FILE_WRITE_ATTRIBUTES

                   WIN-VA77GADM8UM\Administrator:(OI)(CI)F
                   BUILTIN\Administrators:(OI)(CI)F
                   WIN-VA77GADM8UM\user1:(special access:)
                                           SYNCHRONIZE
                                           FILE_EXECUTE


PS C:\> cacls c:\shares\public3$\user1dir3
c:\shares\public3$\user1dir3 WIN-VA77GADM8UM\user1:F
                               WIN-VA77GADM8UM\Administrator:F
                               BUILTIN\Administrators:R
Comment 43 shirishpargaonkar@gmail.com 2014-05-24 21:12:30 UTC
Created attachment 9976 [details]
shared superblock mount changes for mounts without access to intermediate path

So this should work for a mount where a SMB server furbishes inode numbers.

Now working on the remaining patch where a SMB server does not provide
inode numbers.

Still some issues to hash out but hopefully getting there...
Comment 44 shirishpargaonkar@gmail.com 2014-08-31 20:19:57 UTC
If anybody wants, the five patches are available at
https://github.com/shirishpargaonkar/sspcifs/commits/for-next-sb1

Would be nice to receive any feedback.
Comment 45 Michal Zatloukal 2014-09-19 16:32:15 UTC
(In reply to comment #44)
> If anybody wants, the five patches are available at
> https://github.com/shirishpargaonkar/sspcifs/commits/for-next-sb1

Should this also fix the situation, where the user is denied access on the share itself, or just the other situation where user is denied access on some intermediate directories?

I ran into this problem today on CentOS 7 - the situation where both the share and all directories above the last one were denying access to the user. Interestingly, such setup works not only with Windows clients, but OS X 10.9 as well.
Comment 46 shirishpargaonkar@gmail.com 2014-09-22 16:40:56 UTC
(In reply to comment #45)
> (In reply to comment #44)
> > If anybody wants, the five patches are available at
> > https://github.com/shirishpargaonkar/sspcifs/commits/for-next-sb1
> 
> Should this also fix the situation, where the user is denied access on the
> share itself, or just the other situation where user is denied access on some
> intermediate directories?
> 
> I ran into this problem today on CentOS 7 - the situation where both the share
> and all directories above the last one were denying access to the user.
> Interestingly, such setup works not only with Windows clients, but OS X 10.9 as
> well.

It should work for both the cases.
The first patch takes care of when user is denied access on the share itself
and the rest four, when user is denied access on the intermediate paths.
Comment 47 Michal Zatloukal 2014-09-22 21:15:00 UTC
Hm, the patch won't apply to sources for CentOS 3.10 kernel - problem in cifs_fs_sb.h: struct cifs_sb_info is missing rcu_head. What do you suggest - modify the patch, or apply to newer (vanilla?) kernel?
Comment 48 shirishpargaonkar@gmail.com 2014-09-22 22:05:43 UTC
no, you can modify the patch, just the 
 struct rcu_head rcu;
does not exist in 3.10.
Rest of the patches should apply.
Comment 49 David Disseldorp 2015-01-05 14:03:46 UTC
(In reply to Michal Zatloukal from comment #47)

I've rebased Shirish's five comment#44 patches atop current mainline, and v3.10.10 (if needed for CentOS). The bso8950_mount_restricted_root_subdir[_v3.10.10] branches can be found in my repo at git://git.samba.org/ddiss/linux.git:

- https://git.samba.org/?p=ddiss/linux.git;a=shortlog;h=refs/heads/bso8950_mount_restricted_root_subdir
- https://git.samba.org/?p=ddiss/linux.git;a=shortlog;h=refs/heads/bso8950_mount_restricted_root_subdir

@Michal and Alex: It'd be great if you could both test the changes in your local environment and report back.
Comment 50 David Disseldorp 2015-01-07 19:09:37 UTC
@Shirish, I've been taking a look at your comment#44 patch-set, and have a few minor (mostly cosmetic) comments so far:

 708 static struct cifs_rdelem *
 709 find_rdelem_by_inode(struct inode *rdinode, struct cifs_sb_info * cifs_sb)

The non-static->static change of these functions should be collapsed
into the initial "various routines to search disconnected root dentry"
commit. Also, drop the spaces in "* cifs_sb".

 743 static void
 744 find_rdelem_by_path(char *full_path, struct inode **newInode,

full_path should probably be const here.

 745                         struct cifs_sb_info * cifs_sb)
 746 {
 747         struct cifs_rdelem *rdelem;
 748 
 749         spin_lock(&cifs_sb->rtdislock);
 750         list_for_each_entry(rdelem, &cifs_sb->rtdislist, rdlist) {
 751                 if (!strcmp(rdelem->rdname, full_path)) {
 752                         *newInode = ilookup(rdelem->rdinode->i_sb,
 753                                                 rdelem->rdinode->i_ino);
 754                         break;

The other find_rdelem_* functions remove the rtdislist entry when the
item is found, however this function retains it. I find this a little
confusing/inconsistent.

 888 cifs_d_common_releasedelete(const struct dentry *dentry)
 889 {
...
 895         /* disconnected root dentries that did not get spliced */
 896         if (IS_ROOT(dentry) && dentry->d_flags & DCACHE_DISCONNECTED) {

Extra parentheses would be nice here, e.g.
	if (IS_ROOT(dentry) && (dentry->d_flags & DCACHE_DISCONNECTED)) {


 618 create_root_dis_dentry(struct super_block *sb, struct inode *rinode,
 619                         char *fpath)

fpath should probably be const here too.

 620 {
...
 642                         rdelem =
 643                                 cifs_alloc_rdelem(fpath, dentry, rinode);

No need to split this across lines.

 661 static struct dentry *
 662 cifs_get_root(struct smb_vol *vol, struct super_block *sb)
...
 708                 child = lookup_one_len(p, dentry, s - p);
 709                 mutex_unlock(&dir->i_mutex);
 710                 dput(dentry);
 711                 dentry = child;
 712         } while (!IS_ERR(dentry));
 713 
 714         if (IS_ERR(dentry) && (PTR_ERR(dentry) == -EACCES) && *s)
 715                 dentry = create_root_dis_dentry(sb, rinode, full_path);

lookup_one_len() can return -EACCES if a path component corresponds to
"." or ".." - I expect a disconnected root dentry shouldn't be created in
such a case?

 760 struct dentry *
 761 cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
 762             unsigned int flags)
...
 820                 if (pTcon->unix_ext) {
 821                         rc = cifs_get_inode_info_unix(&newInode, full_path,
 822                                         parent_dir_inode->i_sb, xid);
 823                 } else
 824                         rc = cifs_get_inode_info(&newInode, full_path, NULL,
 825                                         parent_dir_inode->i_sb, xid, NULL);

The use of braces here is inconsistent.
Comment 51 shirishpargaonkar@gmail.com 2015-01-08 04:33:06 UTC
(In reply to David Disseldorp from comment #50)

David, Thanks fore the review and comments.

Regarding find_rdelem_by_path() function, it is just a search function
unlike the two functions find_rdelem_by_inode() and find_rdelem_by_dentry()
which are like search and purge functions to search and purge spliced
and unspliced rdelem entries respectively.
At the time of calling find_rdelem_by_path(), we have not spliced 
the dentry and two lookups could be racing in cifs_lookup() function.

So, perhaps a function rename could be more explanatory:
find_rdelem_by_path() to search_unspliced_rdelem_by_path(),
find_rdelem_by_dentry() to purge_unspliced_rdelem_by_dentry()
find_rdelem_by_inode() to purge_spliced_rdelem_by_inode().

Regarding . and .., I think this applies to the full path such as 
/a/b/c/d etc. during the mount. If someone were to specify a path 
such as /a/b/./c or /a/b/../b/c, I think . or .. would not matter
since we would have encountered -EACCES for either current
or parent directory respectively, already.

Do these explanations make sense?

Rest of the comments are valid and will fix them.
But at this point, I do not have a Windows 2012 server handy
to recreate the setup (I could never recreate this using a Samba server),
am working on securing one though.
I hope one or more of the subscribers to this bug is/are able to test
the patchset.
Comment 52 alex_y_xu 2015-05-04 20:22:22 UTC
still doesn't work with either the attached patch or those from https://git.samba.org/?p=ddiss/linux.git;a=shortlog;h=refs/heads/bso8950_mount_restricted_root_subdir

tested now against Windows Server 2012 R2 (as reported by server). now I am using sec=krb5, but that shouldn't make a difference in this part.
Comment 53 alex_y_xu 2015-05-05 12:47:34 UTC
never mind, now access without root works.

but I have a similar problem now. I want to use sec=krb5,multiuser when the mounting creds have no access anywhere in the tree, but the real users do.
Comment 54 shirishpargaonkar@gmail.com 2015-05-12 01:46:15 UTC
Hi Alex, Thanks for testing. Let me gather my thoughts on this, it has
been a while, will respond shortly...
Comment 55 shirishpargaonkar@gmail.com 2015-05-12 01:46:23 UTC
Hi Alex, Thanks for testing. Let me gather my thoughts on this, it has
been a while, will respond shortly...
Comment 56 David Disseldorp 2015-06-02 11:23:05 UTC
(In reply to alex_y_xu from comment #53)
Indeed, thanks for the test feedback Alex. Which kernel did you use?

@shirish: your proposed rename sounds good. Did you get around to making the other changes?
Comment 57 alex_y_xu 2015-06-02 13:11:26 UTC
(In reply to David Disseldorp from comment #56)
> (In reply to alex_y_xu from comment #53)
> Indeed, thanks for the test feedback Alex. Which kernel did you use?

can't be bothered to fish for the date in my reflog, but it was linux.git master at the time (around 4.0.0)
Comment 58 shirishpargaonkar@gmail.com 2015-06-22 10:47:21 UTC
(In reply to David Disseldorp from comment #56)

David, will work on making those changes.
Sorry for really late reply, was busy at work deadline and then on
vacation for two weeks.
Comment 59 Aurélien Aptel 2016-01-08 15:08:17 UTC
Created attachment 11762 [details]
make shares unaccessible at root level mountable

This patch should fix it. See relevant thread on the linux-cifs mailing list [1].

My only concern is when a connexion is shared between mount points. I need to go back to it and test some more...

1: http://thread.gmane.org/gmane.linux.kernel.cifs/11244