vfs_fruit bug in ACL get/set - [PATCH] attached.

[Ralph Böhme]

On Wed, Mar 07, 2018 at 01:47:54PM -0800, Jeremy Allison wrote:
> On Wed, Mar 07, 2018 at 09:53:13PM +0100, Ralph Böhme wrote:
> > Hi!
> > 
> > On Fri, Mar 02, 2018 at 02:35:13PM -0800, Jeremy Allison wrote:
> > > On Fri, Mar 02, 2018 at 11:17:00PM +0100, Ralph Böhme wrote:
> > > > 
> > > > I was thinking about moving setting *and* getting to a lower layer. Whichever
> > > > layer ends up doing it, should be fixed to correctly filter of course. I'm just
> > > > questioning whether keeping this in fruit is the right thing to do when we're
> > > > starting to use this more broadly.
> > > 
> > > Well it's still pretty fruit-specific. For example,
> > > I don't want the SMB2 unix extensions to return the
> > > global_sid_Unix_NFS_Users or global_sid_Unix_NFS_Groups,
> > > as that's info that the client shouldn't have (IMHO),
> > > as it's already covered by the owner and group SIDs
> > > in the ACL returned. My current prototype code for
> > > SMB2 unix only returns global_sid_Unix_NFS_Mode.
> > > 
> > > I understand why they did it for NFS or fruit, but
> > > I really want the SMB2 unix design to be cleaner
> > > than that - and one of the mandates I set myself
> > > was "No UID's/GID's" - only SIDs.
> > 
> > what is the rationale? Why return the mode but not the uid/gid. Are we *sure*
> > that no SMB2 POSIX extensions client will ever need this in the future?
> 
> Yes I'm sure.
> 
> SMB2 is designed around SIDs - built into all
> ACL and ownership and in all the RPC calls and pipes
> (LSA/SAMR etc.). So for POSIX you *have* to have a local-server
> uid/gid to SID mapping in order to work at all.
> 
> Returning uids/gid to a client exposes that internal
> mapping over the network to clients. This is duplicate
> (and possibly incorrect) information. What if you decide
> to change your SID -> uid/gid mapping and change the
> uids on the server side. That can happen with company
> splits/merges with overlapping uid spaces. If you've
> exposed both SIDs (which you already have to do) as
> well as uids/gids to the client, then you've got to change
> this mapping on *all* clients too, rather than making a
> server-contained change.
> 
> The uid/gid <--> SID mapping needs to remain hidden
> and server dependent. SIDs *only* on the wire, and
> clients can store and depend on one user representation
> and not have to cope with two. As I recall, that's why
> NFSv4 uses only a string name format (although it's
> broken in that I think it can be a numeric string :-).

Ok, sounds reasonable. I guess we should write down the design somewhere.

> But this isn't needed to review the specific patch
> I sent (I think :-). That's only about preventing
> round-tripping adding more and more extraneous
> ACE entries into the ACL :-).

yeah, currently reviewing. Generally looks good. Goint to push with a few
cleanups.

Guess we need another README.Coding guideline for multiline statements
indentation:

  Stamement spanning multiple lines shall be indented by aligning at the scoping
  enclosing parenthesis, eg

  bool foo = false;
  bool bar = true;
  bool okey_dokey = (check_that_does_something_with_foo(foo) ||
                     check_that_does_something_with_foo(bar));


  Function invocations can also be split at the initial parenthesis like this:

  foo = this_function_made_my_day(
          bar, baz, oink, grunz, 42, slow, no_more_args);

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46