Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_make_access_from_sd assumes that an "everyone" ACE exists, which sometimes isn't true on Windows #9

Closed
telamonian opened this issue Apr 14, 2020 · 2 comments · Fixed by #11
Assignees

Comments

@telamonian
Copy link
Contributor

telamonian commented Apr 14, 2020

jupyter-fs is built on top of fs.smbfs, and I've been working to make it compatible with Windows. Right now I'm getting an error in my pytest CI on Windows durring SMBFS._make_access_from_sd:

cls = <class 'fs.smbfs.smbfs.SMBFS'>, sd = <smb.security_descriptors.SecurityDescriptor object at 0x04DDD7F0>

    @classmethod
    def _make_access_from_sd(cls, sd):

    ...

>       other_ace = next(ace for ace in sd.dacl.aces
            if str(ace.sid).startswith(smb.security_descriptors.SID_EVERYONE))
E       StopIteration

I know the file that _make_access_from_sd is choking on, so I dug in and examined it more deeply using pysmb. The problem appears to be that the "everyone" SID does not correspond to any of the file's ACEs. In fact, of the ACEs that _make_access_from_sd checks for, in my particular case only the "owner" ACE exists:

In [10]: smb.security_descriptors.SID_EVERYONE
Out[10]: 'S-1-1-0'

In [11]: conn.getSecurity('test', 'foo.txt').owner
Out[11]: SID('S-1-5-21-3526669579-2242266465-3136906013-1006')

In [12]: conn.getSecurity('test', 'foo.txt').group
Out[12]: SID('S-1-5-21-3526669579-2242266465-3136906013-513')
In [8]: [a.sid for a in conn.getSecurity('test', 'foo.txt').dacl.aces]
Out[8]:
[SID('S-1-5-18'),
 SID('S-1-5-32-544'),
 SID('S-1-5-32-545'),
 SID('S-1-5-21-3526669579-2242266465-3136906013-1006')]
@telamonian
Copy link
Contributor Author

telamonian commented Apr 14, 2020

_make_access_from_sd makes a hard assumption that the "everyone" ACE exists. One way to fix this would be to explicitly check for the existence of each of the relevant ACEs (ie "everyone", "owner", and "group") separately. If one of these ACEs does not exist, you would then interpret the corresponding Posix-like permission as none or empty.

I think this would also be more correct than the current behavior of _make_access_from_sd on Linux. Currently, _make_access_from_sd assumes that any missing ACE has at least the same permissions as "everyone". However, this fails to capture permission settings such as 007, which is perfectly valid on Linux.

I retract my previous suggestion, based on my experiments with ACEs in Windows. The way that ACE rights combine is surprisingly complex, but one thing that seems to definitely be true is that any user (including a file owner) has at least as much rights as the "everyone" access control entry.

@althonos althonos added the bug label Apr 14, 2020
@telamonian
Copy link
Contributor Author

telamonian commented Apr 14, 2020

New suggestion:

  • If an ACE does not exist, set its mask to 0x0
  • Elevate all rights to at least those of "everyone" by combining masks using bitwise OR |

Update

I took a crack at implementing the above in #11

telamonian added a commit to telamonian/fs.smbfs that referenced this issue Apr 14, 2020
- also elevates all rights to at least those of everyone by combining
  masks via bitwise or
telamonian added a commit to telamonian/fs.smbfs that referenced this issue Apr 14, 2020
- also elevates all rights to at least those of everyone by combining
  masks via bitwise or
@althonos althonos self-assigned this Apr 14, 2020
@althonos althonos added the fixed label May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants