ACL problem with NFSv4 and DELETE_ACCESS - Samba

This is a discussion on ACL problem with NFSv4 and DELETE_ACCESS - Samba ; Hi! Attached find two patches that attempt to fix a bug we have when "real" ACLs and not just posix mode bits are used. With "real" I right now mean NFSv4, but others like for example the AFS ACL module ...

+ Reply to Thread
Results 1 to 11 of 11

Thread: ACL problem with NFSv4 and DELETE_ACCESS

  1. ACL problem with NFSv4 and DELETE_ACCESS

    Hi!

    Attached find two patches that attempt to fix a bug we have
    when "real" ACLs and not just posix mode bits are used. With
    "real" I right now mean NFSv4, but others like for example
    the AFS ACL module are also affected.

    The problem is in can_delete_file_in_directory(). It right
    now looks at the posix mode only if the owner of a directory
    wants to delete a file within it. This is wrong in all the
    more enhanced ACL schemes. It might be obvious, but it took
    a while for me to understand how this should really work: We
    are allowed to delete a file when either we have a direct
    DELETE right on the object or if that is not there we have a
    DELETE_CHILD right on the containing directory. The first
    attached patch implements this.

    The second patch is necessary because the default rwxr-xr-x
    right on a normal file would map to the owner's DELETE bit
    on a file that is about to be deleted. This is wrong, in the
    non-acl case the right to delete a file is not determined by
    the permissions on the file itself. The changed checks in
    can_delete_file_in_directory take care of it by separately
    looking at the directory permissions.

    The downside of this patch is that we don't use the fast
    path anymore in the non-acl case. I will measure next now
    much we actually lose. And, I'm not sure if all mappings in
    posix_acls.c are correct enough to actually make sure that
    we get the checks right if we push it through
    posix_get_nt_acl.

    I'm not checking these patches in right away, because I need
    to do more tests, but I'd like to hear some feedback
    nevertheless, in particular from people who have worked with
    Posix and NFSv4 ACLs in production.

    Thanks,

    Volker

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.5 (GNU/Linux)

    iD8DBQFIV80sUzqjrWwMRl0RAsYrAJ0T7WXkGNjdO2f3nNMT7w DgmEzyHgCgncEJ
    2pjv6Rom6mZf7B+fMfpxiMI=
    =n6DN
    -----END PGP SIGNATURE-----


  2. Re: ACL problem with NFSv4 and DELETE_ACCESS


    Hi Volker,


    Well, I tried to work on this :

    https://bugzilla.samba.org/show_bug.cgi?id=5135


    not really easy when you never worked on Samba sources.

    When I chat with obnox, we talked about the issue around calls to
    posix_acl.c instead of using vfs implementation.

    Your patch may fix some cases, but is it really sufficient ? Take a
    look to the list I wrote in my bug report.


    Nicolas

    Le 17 juin 08 à 16:41, Volker Lendecke a écrit :

    > Hi!
    >
    > Attached find two patches that attempt to fix a bug we have
    > when "real" ACLs and not just posix mode bits are used. With
    > "real" I right now mean NFSv4, but others like for example
    > the AFS ACL module are also affected.
    >
    > The problem is in can_delete_file_in_directory(). It right
    > now looks at the posix mode only if the owner of a directory
    > wants to delete a file within it. This is wrong in all the
    > more enhanced ACL schemes. It might be obvious, but it took
    > a while for me to understand how this should really work: We
    > are allowed to delete a file when either we have a direct
    > DELETE right on the object or if that is not there we have a
    > DELETE_CHILD right on the containing directory. The first
    > attached patch implements this.
    >
    > The second patch is necessary because the default rwxr-xr-x
    > right on a normal file would map to the owner's DELETE bit
    > on a file that is about to be deleted. This is wrong, in the
    > non-acl case the right to delete a file is not determined by
    > the permissions on the file itself. The changed checks in
    > can_delete_file_in_directory take care of it by separately
    > looking at the directory permissions.
    >
    > The downside of this patch is that we don't use the fast
    > path anymore in the non-acl case. I will measure next now
    > much we actually lose. And, I'm not sure if all mappings in
    > posix_acls.c are correct enough to actually make sure that
    > we get the checks right if we push it through
    > posix_get_nt_acl.
    >
    > I'm not checking these patches in right away, because I need
    > to do more tests, but I'd like to hear some feedback
    > nevertheless, in particular from people who have worked with
    > Posix and NFSv4 ACLs in production.
    >
    > Thanks,
    >
    > Volker
    > <0001-Fix-checks-in-can_delete_file_in_directory.patch><0002-RWX-on-
    > a-file-does-not-imply-DELETE-access.patch>



  3. Re: ACL problem with NFSv4 and DELETE_ACCESS

    On Tue, Jun 17, 2008 at 08:45:52PM +0200, Nicolas Dorfsman wrote:
    > Your patch may fix some cases, but is it really sufficient ? Take a
    > look to the list I wrote in my bug report.


    Thanks, I had forgotten that one.

    Volker

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.5 (GNU/Linux)

    iD8DBQFIWBS6UzqjrWwMRl0RAklOAJ4gBZAbz7CifABE0gaG66 I4qteajQCfWsWY
    vUd6kST/4O4zzSqE5tJYB+U=
    =nmTR
    -----END PGP SIGNATURE-----


  4. Re: ACL problem with NFSv4 and DELETE_ACCESS

    On Tue, Jun 17, 2008 at 08:45:52PM +0200, Nicolas Dorfsman wrote:
    > Well, I tried to work on this :
    >
    > https://bugzilla.samba.org/show_bug.cgi?id=5135
    >
    >
    > not really easy when you never worked on Samba sources.
    >
    > When I chat with obnox, we talked about the issue around calls to
    > posix_acl.c instead of using vfs implementation.
    >
    > Your patch may fix some cases, but is it really sufficient ? Take a
    > look to the list I wrote in my bug report.


    Just found in the SUN docs of the acl(2) system call that is
    evolving or uncommitted. This is really brillant, because
    according to their docs it means:

    > No commitment is made about either source or binary
    > compatibility of these interfaces from one Minor release to
    > the next. Even the drastic incompatible change of removal of
    > the interface in a Minor release is possible.


    Can we use this AT ALL?

    Volker

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.5 (GNU/Linux)

    iD8DBQFIWNrEUzqjrWwMRl0RAoMJAKCW/YMqroJs/0jqXItLsnnHh2Zl3wCgmFW5
    ekJHr8nx1r3v7lpwdJxl12k=
    =fKd7
    -----END PGP SIGNATURE-----


  5. Re: ACL problem with NFSv4 and DELETE_ACCESS


    Le 18 juin 08 à 11:52, Volker Lendecke a écrit :
    >>

    >
    > Just found in the SUN docs of the acl(2) system call that is
    > evolving or uncommitted. This is really brillant, because
    > according to their docs it means:
    >
    >> No commitment is made about either source or binary
    >> compatibility of these interfaces from one Minor release to
    >> the next. Even the drastic incompatible change of removal of
    >> the interface in a Minor release is possible.

    >
    > Can we use this AT ALL?


    Where do you find that ? Which Solaris distro are you using ?

    I can't read anything like this in S10 release :
    http://docs.sun.com/app/docs/doc/816-5167/acl-2?a=view



    Nicolas

  6. Re: ACL problem with NFSv4 and DELETE_ACCESS

    Le 18 juin 08 à 13:52, Nicolas Dorfsman a écrit :

    > Le 18 juin 08 à 11:52, Volker Lendecke a écrit :
    >>>

    >>
    >> Just found in the SUN docs of the acl(2) system call that is
    >> evolving or uncommitted. This is really brillant, because
    >> according to their docs it means:
    >>
    >>> No commitment is made about either source or binary
    >>> compatibility of these interfaces from one Minor release to
    >>> the next. Even the drastic incompatible change of removal of
    >>> the interface in a Minor release is possible.

    >>
    >> Can we use this AT ALL?

    >
    > Where do you find that ? Which Solaris distro are you using ?
    >
    > I can't read anything like this in S10 release :
    > http://docs.sun.com/app/docs/doc/816-5167/acl-2?a=view


    Oops, sorry. I see. I read too quickly your post.

    I'm going to try to find more infos on this.

    Nicolas

  7. Re: ACL problem with NFSv4 and DELETE_ACCESS

    On Tue, Jun 17, 2008 at 04:41:48PM +0200, Volker Lendecke wrote:
    > Hi!
    >
    > Attached find two patches that attempt to fix a bug we have
    > when "real" ACLs and not just posix mode bits are used. With
    > "real" I right now mean NFSv4, but others like for example
    > the AFS ACL module are also affected.
    >
    > The problem is in can_delete_file_in_directory(). It right
    > now looks at the posix mode only if the owner of a directory
    > wants to delete a file within it. This is wrong in all the
    > more enhanced ACL schemes. It might be obvious, but it took
    > a while for me to understand how this should really work: We
    > are allowed to delete a file when either we have a direct
    > DELETE right on the object or if that is not there we have a
    > DELETE_CHILD right on the containing directory. The first
    > attached patch implements this.
    >
    > The second patch is necessary because the default rwxr-xr-x
    > right on a normal file would map to the owner's DELETE bit
    > on a file that is about to be deleted. This is wrong, in the
    > non-acl case the right to delete a file is not determined by
    > the permissions on the file itself. The changed checks in
    > can_delete_file_in_directory take care of it by separately
    > looking at the directory permissions.
    >
    > The downside of this patch is that we don't use the fast
    > path anymore in the non-acl case. I will measure next now
    > much we actually lose. And, I'm not sure if all mappings in
    > posix_acls.c are correct enough to actually make sure that
    > we get the checks right if we push it through
    > posix_get_nt_acl.
    >
    > I'm not checking these patches in right away, because I need
    > to do more tests, but I'd like to hear some feedback
    > nevertheless, in particular from people who have worked with
    > Posix and NFSv4 ACLs in production.


    As discussed, they look really good with the change from
    UNIX_ACCESS_RWX to (UNIX_ACCESS_RWX & ~DELETE_ACCESS) and
    the re-arrangement to check the directory acl first.

    Cheers,

    Jeremy.


  8. Re: ACL problem with NFSv4 and DELETE_ACCESS

    On Wed, Jun 18, 2008 at 04:37:26PM -0700, Jeremy Allison wrote:
    > As discussed, they look really good with the change from
    > UNIX_ACCESS_RWX to (UNIX_ACCESS_RWX & ~DELETE_ACCESS) and
    > the re-arrangement to check the directory acl first.


    Ok, pushed. Thanks for looking at it :-)

    Volker

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.5 (GNU/Linux)

    iD8DBQFIWmXjUzqjrWwMRl0RAjfwAJ98sRqSsrggGj6OPPIXJQ hzklOWegCggsxq
    X5oXnwLLX+4OOquw4XGAI2A=
    =Sv53
    -----END PGP SIGNATURE-----


  9. Re: ACL problem with NFSv4 and DELETE_ACCESS


    Hi Volker,

    Le 18 juin 08 à 11:52, Volker Lendecke a écrit :
    >
    > Just found in the SUN docs of the acl(2) system call that is
    > evolving or uncommitted. This is really brillant, because
    > according to their docs it means:
    >
    >> No commitment is made about either source or binary
    >> compatibility of these interfaces from one Minor release to
    >> the next. Even the drastic incompatible change of removal of
    >> the interface in a Minor release is possible.

    >
    > Can we use this AT ALL?


    man -s5 attributes extract :

    Evolving

    An Evolving interface may eventually become Standard or
    Stable but is still in transition.

    Sun will make reasonable efforts to ensure compatibility
    with previous releases as it evolves. When non-upwards
    compatible changes become necessary, they will occur in
    minor and major releases; such changes will be avoided
    in micro releases whenever possible. If such a change is
    necessary, it will be documented in the release notes
    for the affected release, and when feasible, Sun will
    provide migration aids for binary compatibility and con-
    tinued source development.


    Are you still afraid ?

  10. Re: ACL problem with NFSv4 and DELETE_ACCESS

    On Tue, Jun 24, 2008 at 06:28:25AM +0200, Nicolas Dorfsman wrote:
    >
    > Hi Volker,
    >
    > Le 18 juin 08 à 11:52, Volker Lendecke a écrit :
    > >
    > >Just found in the SUN docs of the acl(2) system call that is
    > >evolving or uncommitted. This is really brillant, because
    > >according to their docs it means:
    > >
    > >>No commitment is made about either source or binary
    > >>compatibility of these interfaces from one Minor release to
    > >>the next. Even the drastic incompatible change of removal of
    > >>the interface in a Minor release is possible.

    > >
    > >Can we use this AT ALL?

    >
    > man -s5 attributes extract :
    >
    > Evolving


    Sounds better :-)

    Volker

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.5 (GNU/Linux)

    iD8DBQFIYJZ3UzqjrWwMRl0RArXcAKCFz8fOb4X2VNUPDvmebh nSW159ggCeNwkY
    V14pf8j+4ymYaukTy5dV0GU=
    =rmBc
    -----END PGP SIGNATURE-----


  11. Re: ACL problem with NFSv4 and DELETE_ACCESS

    Nicolas Dorfsman wrote:
    >
    > Hi Volker,
    >
    > Le 18 juin 08 à 11:52, Volker Lendecke a écrit :
    >>
    >> Just found in the SUN docs of the acl(2) system call that is
    >> evolving or uncommitted. This is really brillant, because
    >> according to their docs it means:
    >>
    >>> No commitment is made about either source or binary
    >>> compatibility of these interfaces from one Minor release to
    >>> the next. Even the drastic incompatible change of removal of
    >>> the interface in a Minor release is possible.

    >>
    >> Can we use this AT ALL?

    >
    > man -s5 attributes extract :
    >
    > Evolving
    >
    > An Evolving interface may eventually become Standard or
    > Stable but is still in transition.
    >
    > Sun will make reasonable efforts to ensure compatibility
    > with previous releases as it evolves. When non-upwards
    > compatible changes become necessary, they will occur in
    > minor and major releases; such changes will be avoided
    > in micro releases whenever possible. If such a change is
    > necessary, it will be documented in the release notes
    > for the affected release, and when feasible, Sun will
    > provide migration aids for binary compatibility and con-
    > tinued source development.
    >
    >
    > Are you still afraid ?


    "Evolving" really mens the interface is uncommitted and can be changed
    by Sun but in case of the acl(2) you should have on your mind the ace_t
    structure is using layout of the "NFSv4 wire format" for the flags so
    the RFC 3530 should be rewritten first in this case. Also the ZFS is
    using the NFSv4 extended ACL interface so here is a lot of code in
    Solaris which should be rewritten in such case :-)

    Jiri


+ Reply to Thread