diff mbox series

[SMB3] add mount option to allow retrieving POSIX mode from special ACE

Message ID CAH2r5mvN2LQG_eWhfes3_tpBwhmg-Q=+L7U+=xFHb4W01_wVJg@mail.gmail.com
State New
Headers show
Series [SMB3] add mount option to allow retrieving POSIX mode from special ACE | expand

Commit Message

Steve French June 24, 2019, 7:11 a.m. UTC 
See e.g. https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2008-R2-and-2008/hh509017(v=ws.10)

where it describes use of an ACE with special SID S-1-5-88-3 to store the mode.

Followon patches will add the support for chmod and query_info (stat)

Comments

Steve French June 24, 2019, 6:23 p.m. UTC | #1 
I missed a couple lines in the earlier version of this patch that I
sent last night - updated one attached.


On Mon, Jun 24, 2019 at 2:11 AM Steve French <smfrench@gmail.com> wrote:
>
> See e.g. https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2008-R2-and-2008/hh509017(v=ws.10)
>
> where it describes use of an ACE with special SID S-1-5-88-3 to store the mode.
>
> Followon patches will add the support for chmod and query_info (stat)
>
>
>
> --
> Thanks,
>
> Steve
Pavel Shilovsky June 24, 2019, 7:06 p.m. UTC | #2 
Can't we use the existing idfromsid for this purpose? We already have
a plenty of mount options and the list keeps growing.

--
Best regards,
Pavel Shilovsky

пн, 24 июн. 2019 г. в 00:20, Steve French <smfrench@gmail.com>:
>
> See e.g. https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2008-R2-and-2008/hh509017(v=ws.10)
>
> where it describes use of an ACE with special SID S-1-5-88-3 to store the mode.
>
> Followon patches will add the support for chmod and query_info (stat)
>
>
>
> --
> Thanks,
>
> Steve
Steve French June 24, 2019, 8:25 p.m. UTC | #3 
On Mon, Jun 24, 2019 at 2:07 PM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>
> Can't we use the existing idfromsid for this purpose? We already have
> a plenty of mount options and the list keeps growing.

That is a good question - and I am open to suggestions to remove some
mount options but
the general problem is that that mount option name could be very confusing -
"idsfromsid" doesn't really imply anything about how we handle
mode bits (we could save mode bits even if saving uid owner without
using the "idsfromsid"
mechanism) we want to allow:

1) query mode from special sid if present
or
2) query mode from ACL (only check for perms on the three
user-owner/group-owner/EVERYONE SIDs), in this case we may chose to
mount noperm
or
3) the default today - we set mode for files and directories to the
permissions supplied as "file_mode" and "dir_mode")
We by default do:
      vol->dir_mode = vol->file_mode = S_IRUGO | S_IXUGO | S_IWUSR;
and we can mount with noperm to disable the client perm checks if the
checks on the client are not useful
or
4) set the permissions (temporarily) locally only and cache them
(dynperm) - typically not recommended.

Where I would like to get to is that we focus strongly on only the
first two common use cases:
1) "client focused perm checks"   -  get/set mode from special SID
(server permission checks are not important in this case)
2) "server focused perm checks" - get/set the three ACEs
(user-owner/group-owner/EVERYONE) in the ACL

I would like to default to idsfromsid (setting the owner with  if
looking up owner from Winbind or SSSD or falling back
to S-1-22-1 (Unmapped user's special SID) or S-1-5-88-1  (MS-NFS and
Apple style unmapped user's special SID).

In a way I would like to remove "idsfromsid" (and do it by default),
and add the new mount point to distinguish between

"client centric" mode bit evaluation (special mode SID)
vs.
"server centric" ACL evaluation (where mode bits are mapped into the 3
usual ACEs - user/group/other)



> пн, 24 июн. 2019 г. в 00:20, Steve French <smfrench@gmail.com>:
> >
> > See e.g. https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2008-R2-and-2008/hh509017(v=ws.10)
> >
> > where it describes use of an ACE with special SID S-1-5-88-3 to store the mode.
> >
> > Followon patches will add the support for chmod and query_info (stat)
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
ronnie sahlberg June 24, 2019, 9:36 p.m. UTC | #4 
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>

On Tue, Jun 25, 2019 at 7:22 AM Steve French <smfrench@gmail.com> wrote:
>
> I missed a couple lines in the earlier version of this patch that I
> sent last night - updated one attached.
>
>
> On Mon, Jun 24, 2019 at 2:11 AM Steve French <smfrench@gmail.com> wrote:
> >
> > See e.g. https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2008-R2-and-2008/hh509017(v=ws.10)
> >
> > where it describes use of an ACE with special SID S-1-5-88-3 to store the mode.
> >
> > Followon patches will add the support for chmod and query_info (stat)
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
>
>
>
> --
> Thanks,
>
> Steve
Pavel Shilovsky July 4, 2019, 8:52 p.m. UTC | #5 
These are good points and I agree with the plan.

I would rename the option:

"modefromace" -> ""modefromsid"

to make the naming consistent with the existing "idsfromsid" and match
the behavior closely: a mode is still technically from the special SID
and that SID is from the special ACE. Other than that the patch looks
good.

--
Best regards,
Pavel Shilovsky

пн, 24 июн. 2019 г. в 13:25, Steve French <smfrench@gmail.com>:
>
> On Mon, Jun 24, 2019 at 2:07 PM Pavel Shilovsky <piastryyy@gmail.com> wrote:
> >
> > Can't we use the existing idfromsid for this purpose? We already have
> > a plenty of mount options and the list keeps growing.
>
> That is a good question - and I am open to suggestions to remove some
> mount options but
> the general problem is that that mount option name could be very confusing -
> "idsfromsid" doesn't really imply anything about how we handle
> mode bits (we could save mode bits even if saving uid owner without
> using the "idsfromsid"
> mechanism) we want to allow:
>
> 1) query mode from special sid if present
> or
> 2) query mode from ACL (only check for perms on the three
> user-owner/group-owner/EVERYONE SIDs), in this case we may chose to
> mount noperm
> or
> 3) the default today - we set mode for files and directories to the
> permissions supplied as "file_mode" and "dir_mode")
> We by default do:
>       vol->dir_mode = vol->file_mode = S_IRUGO | S_IXUGO | S_IWUSR;
> and we can mount with noperm to disable the client perm checks if the
> checks on the client are not useful
> or
> 4) set the permissions (temporarily) locally only and cache them
> (dynperm) - typically not recommended.
>
> Where I would like to get to is that we focus strongly on only the
> first two common use cases:
> 1) "client focused perm checks"   -  get/set mode from special SID
> (server permission checks are not important in this case)
> 2) "server focused perm checks" - get/set the three ACEs
> (user-owner/group-owner/EVERYONE) in the ACL
>
> I would like to default to idsfromsid (setting the owner with  if
> looking up owner from Winbind or SSSD or falling back
> to S-1-22-1 (Unmapped user's special SID) or S-1-5-88-1  (MS-NFS and
> Apple style unmapped user's special SID).
>
> In a way I would like to remove "idsfromsid" (and do it by default),
> and add the new mount point to distinguish between
>
> "client centric" mode bit evaluation (special mode SID)
> vs.
> "server centric" ACL evaluation (where mode bits are mapped into the 3
> usual ACEs - user/group/other)
>
>
>
> > пн, 24 июн. 2019 г. в 00:20, Steve French <smfrench@gmail.com>:
> > >
> > > See e.g. https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2008-R2-and-2008/hh509017(v=ws.10)
> > >
> > > where it describes use of an ACE with special SID S-1-5-88-3 to store the mode.
> > >
> > > Followon patches will add the support for chmod and query_info (stat)
> > >
> > >
> > >
> > > --
> > > Thanks,
> > >
> > > Steve
>
>
>
> --
> Thanks,
>
> Steve
Aurélien Aptel July 4, 2019, 10:03 p.m. UTC | #6 
Steve French <smfrench@gmail.com> writes:
> Where I would like to get to is that we focus strongly on only the
> first two common use cases:
> 1) "client focused perm checks"   -  get/set mode from special SID
> (server permission checks are not important in this case)
> 2) "server focused perm checks" - get/set the three ACEs
> (user-owner/group-owner/EVERYONE) in the ACL

The 2) part is not really documented and is more complex than it
seems. We know how the SID are created but not the actual ACL/ACE for
each SID. I've almost completely reversed engineered it (except for the
one bit).

I've documented all here: https://github.com/aaptel/nfs-acl-test

The is one permission -- the S (SYNCHRONIZE) flag -- which doesn't seem
to be consistent in how it is granted/denied. But its purpose is not
clear on files/dir so it's probably irrelevant: we just need to
reimplement the unix_to_acl() func that I wrote.

I've contacted dochelp regarding this, and here is what they have to
say (note "UUUA" is Unmapped UNIX User Access):

>  I have been combing the source and could not find where a mapping is done yet for the mode
>  you are asking about.
>
> ...
>
>  After conferring with our NFS experts, the key perspective that I’d like to share is that
>  we do document (outside of the protocol documents) that UUUA is intended for an end-to-end
>  NFS-only access.
>  The UUUA mode is intended for use when the Windows NFS Server is the only accessor to the
>  files. We make no statements as to the behavior of any other accessor or how they
>  can/should decode the DACL. 
>  We do not expect a client to ever come across the NFS specific DACL in a well configured
>  system. 
>  With that perspective, we had some archived content which describes some “Mapping of NFS
>  Mode Bits to Windows ACL”. To understand part of what Windows does under the hood, you
>  many find useful to consult the obsoleted [MS-FSSO] which is under Windows Protocols
>  Archive Documents. Keep in mind that archived documents are for “convenience” only. We do
>  not answer questions or service those types of documents. 
>  [MS-FSSO]: File Access Services System Overview
>  https://winprotocoldoc.blob.core.windows.net/productionwindowsarchives/WinArchive/%5bMS-FSSO%5d.pdf

Cheers,
Steve French July 6, 2019, 2:11 a.m. UTC | #7 
Updated "modefromace" to "modefromsid"

On Thu, Jul 4, 2019 at 3:52 PM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>
> These are good points and I agree with the plan.
>
> I would rename the option:
>
> "modefromace" -> ""modefromsid"
>
> to make the naming consistent with the existing "idsfromsid" and match
> the behavior closely: a mode is still technically from the special SID
> and that SID is from the special ACE. Other than that the patch looks
> good.
>
> --
> Best regards,
> Pavel Shilovsky
>
> пн, 24 июн. 2019 г. в 13:25, Steve French <smfrench@gmail.com>:
> >
> > On Mon, Jun 24, 2019 at 2:07 PM Pavel Shilovsky <piastryyy@gmail.com> wrote:
> > >
> > > Can't we use the existing idfromsid for this purpose? We already have
> > > a plenty of mount options and the list keeps growing.
> >
> > That is a good question - and I am open to suggestions to remove some
> > mount options but
> > the general problem is that that mount option name could be very confusing -
> > "idsfromsid" doesn't really imply anything about how we handle
> > mode bits (we could save mode bits even if saving uid owner without
> > using the "idsfromsid"
> > mechanism) we want to allow:
> >
> > 1) query mode from special sid if present
> > or
> > 2) query mode from ACL (only check for perms on the three
> > user-owner/group-owner/EVERYONE SIDs), in this case we may chose to
> > mount noperm
> > or
> > 3) the default today - we set mode for files and directories to the
> > permissions supplied as "file_mode" and "dir_mode")
> > We by default do:
> >       vol->dir_mode = vol->file_mode = S_IRUGO | S_IXUGO | S_IWUSR;
> > and we can mount with noperm to disable the client perm checks if the
> > checks on the client are not useful
> > or
> > 4) set the permissions (temporarily) locally only and cache them
> > (dynperm) - typically not recommended.
> >
> > Where I would like to get to is that we focus strongly on only the
> > first two common use cases:
> > 1) "client focused perm checks"   -  get/set mode from special SID
> > (server permission checks are not important in this case)
> > 2) "server focused perm checks" - get/set the three ACEs
> > (user-owner/group-owner/EVERYONE) in the ACL
> >
> > I would like to default to idsfromsid (setting the owner with  if
> > looking up owner from Winbind or SSSD or falling back
> > to S-1-22-1 (Unmapped user's special SID) or S-1-5-88-1  (MS-NFS and
> > Apple style unmapped user's special SID).
> >
> > In a way I would like to remove "idsfromsid" (and do it by default),
> > and add the new mount point to distinguish between
> >
> > "client centric" mode bit evaluation (special mode SID)
> > vs.
> > "server centric" ACL evaluation (where mode bits are mapped into the 3
> > usual ACEs - user/group/other)
> >
> >
> >
> > > пн, 24 июн. 2019 г. в 00:20, Steve French <smfrench@gmail.com>:
> > > >
> > > > See e.g. https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2008-R2-and-2008/hh509017(v=ws.10)
> > > >
> > > > where it describes use of an ACE with special SID S-1-5-88-3 to store the mode.
> > > >
> > > > Followon patches will add the support for chmod and query_info (stat)
> > > >
> > > >
> > > >
> > > > --
> > > > Thanks,
> > > >
> > > > Steve
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
diff mbox series

Patch

From 952f30b31c903f8f6f4ca023061c108d16cc6df6 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Mon, 24 Jun 2019 02:01:42 -0500
Subject: [PATCH] smb3: add new mount option to retrieve mode from special ACE

There is a special ACE used by some servers to allow the mode
bits to be stored.  This can be especially helpful in scenarios
in which the client is trusted, and access checking on the
client vs the POSIX mode bits is sufficient.

Add mount option to allow enabling this behavior.
Follow on patch will add the support to add chmod and queryinfo
(stat) support for retrieving the POSIX mode bits from the
special ACE, SID: S-1-5-88-3

See e.g.
https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2008-R2-and-2008/hh509017(v=ws.10)

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifs_fs_sb.h | 1 +
 fs/cifs/cifsfs.c     | 2 ++
 fs/cifs/cifsglob.h   | 2 +-
 fs/cifs/connect.c    | 6 ++++++
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
index afa56237a0c3..744e48bdcb6c 100644
--- a/fs/cifs/cifs_fs_sb.h
+++ b/fs/cifs/cifs_fs_sb.h
@@ -52,6 +52,7 @@ 
 #define CIFS_MOUNT_UID_FROM_ACL 0x2000000 /* try to get UID via special SID */
 #define CIFS_MOUNT_NO_HANDLE_CACHE 0x4000000 /* disable caching dir handles */
 #define CIFS_MOUNT_NO_DFS 0x8000000 /* disable DFS resolving */
+#define CIFS_MOUNT_MODE_FROM_ACE 0x10000000 /* retrieve mode from special ACE */
 
 struct cifs_sb_info {
 	struct rb_root tlink_tree;
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index dc5fd7a648f0..e33da73bd300 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -526,6 +526,8 @@  cifs_show_options(struct seq_file *s, struct dentry *root)
 		seq_puts(s, ",nobrl");
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_HANDLE_CACHE)
 		seq_puts(s, ",nohandlecache");
+	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MODE_FROM_ACE)
+		seq_puts(s, ",modefromace");
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL)
 		seq_puts(s, ",cifsacl");
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_DYNPERM)
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 16f240911192..2c87547e42ab 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -618,7 +618,7 @@  struct smb_vol {
 			 CIFS_MOUNT_MULTIUSER | CIFS_MOUNT_STRICT_IO | \
 			 CIFS_MOUNT_CIFS_BACKUPUID | CIFS_MOUNT_CIFS_BACKUPGID | \
 			 CIFS_MOUNT_UID_FROM_ACL | CIFS_MOUNT_NO_HANDLE_CACHE | \
-			 CIFS_MOUNT_NO_DFS)
+			 CIFS_MOUNT_NO_DFS | CIFS_MOUNT_MODE_FROM_ACE)
 
 /**
  * Generic VFS superblock mount flags (s_flags) to consider when
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index b8a60060d329..f7bc85775f96 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -175,6 +175,7 @@  static const match_table_t cifs_mount_option_tokens = {
 	{ Opt_serverino, "serverino" },
 	{ Opt_noserverino, "noserverino" },
 	{ Opt_rwpidforward, "rwpidforward" },
+	{ Opt_modeace, "modefromace" },
 	{ Opt_cifsacl, "cifsacl" },
 	{ Opt_nocifsacl, "nocifsacl" },
 	{ Opt_acl, "acl" },
@@ -1830,6 +1831,9 @@  cifs_parse_mount_options(const char *mountdata, const char *devname,
 		case Opt_rwpidforward:
 			vol->rwpidforward = 1;
 			break;
+		case Opt_modeace:
+			vol->mode_ace = 1;
+			break;
 		case Opt_cifsacl:
 			vol->cifs_acl = 1;
 			break;
@@ -3976,6 +3980,8 @@  int cifs_setup_cifs_sb(struct smb_vol *pvolume_info,
 		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_NOPOSIXBRL;
 	if (pvolume_info->rwpidforward)
 		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_RWPIDFORWARD;
+	if (pvolume_info->mode_ace)
+		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_MODE_FROM_ACE;
 	if (pvolume_info->cifs_acl)
 		cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_CIFS_ACL;
 	if (pvolume_info->backupuid_specified) {
-- 
2.20.1