Re: SFTP ACLs need inheritance support

[Joseph Galbraith <galb-list%vandyke.com@localhost>]

denis bider wrote:
> I don't have any major complaints about including the ACL
> flags as an extension to Attributes.
> 
> NT4 is also a platform that supports ACLs but not inheritance.
> We support it.
> 
> I don't think the presence of inheritance flags hurts any
> platform that doesn't support them. Such a platform can simply
> ignore those flags.

Hmm... well, lets try a proposal that doesn't use extensions,
and see how it comes out -- folks, let me know what you think!

ACE
      uint32   ace-type
      uint32   ace-flag
      uint32   ace-mask
      string   who [UTF-8]

ACL

uint32 acl-flags
uint32 ace-count
ACE    ace[ace-count]

acl-flags indicates information about the ACL
as a whole.

SFX_ACL_ACCESS_CONTROL_INCLUDED = 0x00000001
SFX_ACL_ACCESS_CONTROL_PRESENT  = 0x00000002
SFX_ACL_CONTROL_INHERITED       = 0x00000004
  If INCLUDE is set during a setstat operation, then the
  client intends to modify the ALLOWED/DENIED entries of
  the ACL.  Otherwise, the client intends for these entries
  to be preserved.

  If the PRESENT bit is not set, then the client wishes to
  remove control entries.  If the server doesn't
  support separate control and audit information, the client
  MUST not clear this bit without also clearing the
  AUDIT_ALARM_PRESENT bit.

  If the PRESENT bit is clear, then control of the file
  MAY be through the permissions mask.  The server MAY
  also grant full access to the file.

  If the both the INCLUDE and the PRESENT bit are set,
  but their are no ALLOW/DENY entries in the list, the
  client wishes to deny all access to the file or directory.
  The server may have to transform this into a ACL with
  a deny entry to implement it.

  If INHERITED is set, then ALLOW/DENY ACEs MAY be inherited
  from the parent directory.  If it is off, then they MUST
  not be INHERITED.  If the server does not support controlling
  inheritance, then the client MUST clear this bit; in this
  case the inheritance properties of the server are undefined.

SFX_ACL_AUDIT_ALARM_INCLUDED  = 0x00000010
SFX_ACL_AUDIT_ALARM_INHERITED = 0x00000020
  If INCLUDE is set during a setstat operation, then the
  client intends to modify the AUDIT/ALARM entries of
  the ACL.  Otherwise, the client intends for these entries
  to be preserved.

  If INHERITED is set, then AUDIT/ALARM ACEs MAY be inherited
  from the parent directory.  If it is off, then they MUST
  not be INHERITED.  If the server does not support controlling
  inheritance, then the client MUST clear this bit; in this
  case the inheritance properties of the server are undefined.

Because some server require special permissions / privileges
in order to modify the AUDIT/ALARM entries in the ACL, it
is important to communicate to the server the clients intent
to modify these entries.  The client MUST both use the
ACCESS_AUDIT_ALARM_ATTRIBUTES bit in the desired attribute
of the open request and must set the SFX_ACL_AUDIT_ALARM_INCLUDED
during the setstat operation.

Clients that do not intend specifically to modify the
AUDIT or ALARM entries SHOULD NOT request
ACCESS_PRIV_ACCESS_AUDIT_ALARM_ATTRIBUTES access and SHOULD NOT
set the SFX_ACL_AUDIT_ALARM_INCLUDED bit because these
operations are often privileged and will fail.

If the SFX_ACL_AUDIT_ALARM_INCLUDED is set, and the requested
change can not be made, the server MUST fail the request.

--

I believe rather than access flags, it makes more sense to
do these as open flags (since they can't be set in a ACL):

SSH_FXF_ACCESS_AUDIT_ALARM_INFORMATION = 0x00001000
  This bit indicates that the client wishes the server
  to enable any privileges or extra capabilities that
  the user may have in to allow the reading and writing
  of AUDIT or ALARM access control entries.

SSH_FXF_ACCESS_BACKUP               = 0x00002000
  This bit indicates that the client wishes the server to
  enable any privileges or extra capabilities that the user
  may have in order to bypass normal access checks for the
  purpose of backing up or restoring files.

SSH_FXF_ACCESS_BACKUP_STREAM        = 0x00004000
  This bit indicates that the client wishes to read or write
  a backup stream.  A backup stream is a system dependent
  structured data stream that encodes all the information
  that must be preserved in order to restore the file from
  backup medium.

  The only well defined use for backup stream data read in
  this fashion is to write it to the same server to a file
  also opened using the BACKUP_STREAM flag.  However,
  if the server has a well defined backup stream format, their
  may be other uses for this data outside the scope of this
  protocol.

ACCESS_OVERRIDE_OWNER                = 0x00008000
  This bit indicates that the client wishes the server
  to enable any privileges or extra capabilities that
  the user may have in order to gain access to the file
  with WRITE_OWNER permission.

  This bit MUST always be specified in combination with
  ACE4_WRITE_OWNER.

> It may however be useful if a server can advertise that it
> supports ACL inheritance via the "supported2" mechanism in the
> version packet.
>
> What would be a good way to advertise that?

Unfortunately people (myself included) have already implemented
and shipped "supported2" -- the "supported2" and the "vendor-id"
seem to be pretty popular :-)

I'd suggest the following:

"acl-supported"
uint32 capabilities

  ACL_CAP_ALLOW = 0x00000001
  ACL_CAP_DENY  = 0x00000002
  ACL_CAP_AUDIT = 0x00000004
  ACL_CAP_ALARM = 0x00000008
    The client may use the associated ACE type.

  ACL_CAP_INHERIT_ACCESS      = 0x00000010
  ACL_CAP_INHERIT_AUDIT_ALARM = 0x00000020
    If these bits are clear, the SFX_ACL_INHERIT_*
    flags must be false.

  ACL_CAP_SEPERATES_AUDIT_AND_CONTROL = 0x00000040
    If this bit is clear, then either both SFX_ACL_INHERIT_
    bits must bet set or both must be clear.  This is also
    true for the SFX_ACL_*_INCLUDED bits.

What do you think?

Thanks,

Joseph

> On Tue, 06 Dec 2005 08:32:09 -0700, Joseph Galbraith wrote:
>> denis bider wrote:
>> 
>>>> Do you think this should be added to SFTP v6, or do we
>>>> need a v7?
>>>> 
>>> I think the changes I recommended are central to any SFTP 
>>> client or server that implements ACLs, so I'd prefer them
>>> to be treated as such. For this reason, I would prefer a
>>> new ACL flags field into the Attributes structure.
>>> However, I could also live with the ACL flags field being
>>> an extension.
>>> 
>> Remember, currently, the draft has no concept of DACL and
>> SACL, and the other OS that does ACLs that I'm somewhat
>> familiar with (VMS) does not seperate these, and so forcing
>> them to emulate the behavior of being able to set these
>> separately increases the complexity of their servers.
>> 
>> Also, the meaning of a empty ACL is not universal; an empty
>> ACL in VMS is the same as no ACL, and both mean defer to the
>> permissions field.
>> 
>> In NT, it an empty ACL means grant no access; an absent ACL
>> means grant all access.
>> 
>> I don't know if VMS supports the concept of inheritable ACLs
>> (Richard, are you watching this thread?)  And if it does, if
>> it has a concept of "PROTECTED" like NT does.
>> 
>> It may be that we need some increased complexity anyway to
>> make it so that the client can predict what behavior will be
>> if it sets an ACL with no ALLOW/DENY entries.
>> 
>> Regardless I'm trying to balance increasing complexity for 
>> non-NT platforms with allowing NT platforms to communicate 
>> all the information needed.
>> 
>> Using an extension, I think, accomplishes this nicely. Under
>> NT, fetching the permissions is expensive enough that it
>> isn't done all the time; hence the additional overhead of an
>> extension isn't too much.  I also tried to define the
>> extension in a way such that it wouldn't need to be sent
>> more often than not-- but given the first point, I'm not
>> sure this is worth it.
>> 
>> Servers that don't separate the two won't advertise the
>> extension, and clients will know not to send them.
>> 
>> Thanks,
>> 
>> Joseph
> 
> 
>