ACL type does not converge if permissions include capital X

ungleich-gitea commented

2021-11-20 13:23:52 +00:00

Owner

Created by: jimis

__acl /path/to/dir/ --default --user username:r-X

Note the capital X in the permisisons part. No matter how many times I execute this, the type re-sets the ACL every time.

From setfacl manual:

The perms field is a combination of characters that indicate the read (r), write (w), execute (x) permissions. Dash characters in the perms field (-) are ignored. The character X stands for the execute permission if the file is a directory or already has execute permission for some user.

The problem lies in the gencode-remote script, that expects to find the string "r-X" in the output of getfacl (explorer acl_is). Apparently getfacl will never return a capital X. From the end of gencode-remote (the comment is mine):

for acl in $acl_should
do
    ### NOTE $acl_is might contain capital X but $acl_should never does, since
    ###     it comes from getfacl
    if ! echo "$acl_is" | grep -Eq "^$acl"
    then echo "$setfacl_exec -m \"$acl\" \"$acl_path\""
    fi
done

Fixing it properly is complicated, it would require logic while comparing $acl_is and $acl_should, expecting "x" if it is a directory or if it is a file with the executable permissions bit set.

My workaround is to add -i to that grep command line. Apparently this has drawbacks.

*Created by: jimis* ```__acl /path/to/dir/ --default --user username:r-X``` Note the capital X in the permisisons part. No matter how many times I execute this, the type re-sets the ACL every time. From setfacl manual: > The perms field is a combination of characters that indicate the read (r), write (w), execute (x) permissions. Dash characters in the perms field (-) are ignored. The character X stands for the execute permission if the file is a directory or already has execute permission for some user. The problem lies in the `gencode-remote` script, that expects to find the string "r-X" in the output of `getfacl` (explorer `acl_is`). Apparently `getfacl` will never return a capital X. From the end of `gencode-remote` (the comment is mine): ``` for acl in $acl_should do ### NOTE $acl_is might contain capital X but $acl_should never does, since ### it comes from getfacl if ! echo "$acl_is" | grep -Eq "^$acl" then echo "$setfacl_exec -m \"$acl\" \"$acl_path\"" fi done ``` Fixing it properly is complicated, it would require logic while comparing `$acl_is` and `$acl_should`, expecting "x" if it is a directory or if it is a file with the executable permissions bit set. My workaround is to add `-i` to that grep command line. Apparently this has drawbacks.

ungleich-gitea closed this issue

2021-11-20 13:23:52 +00:00

ungleich-gitea commented

2021-11-20 15:38:37 +00:00

Author

Owner

Created by: jimis

Yes it is solved, thank you!

*Created by: jimis* Yes it is solved, thank you!

ungleich-gitea commented

2021-11-20 15:38:39 +00:00

Author

Owner

Created by: 4nd3r

this should be closed if #760 solved an issue

*Created by: 4nd3r* this should be closed if #760 solved an issue

ungleich-gitea commented

2021-11-20 15:38:40 +00:00

Author

Owner

Created by: jimis

if that is the case, then we could just do simple test and sed 's/x/X/ on wanted line in acl_is output...

Sounds good! :-) But also skip the test entirely if it's a file and the "x" bit is not set for any user.

*Created by: jimis* > if that is the case, then we could just do simple test and sed 's/x/X/ on wanted line in acl_is output... Sounds good! :-) But also skip the test entirely if it's a file and the "x" bit is not set for any user.

ungleich-gitea commented

2021-11-20 15:38:41 +00:00

Author

Owner

Created by: 4nd3r

wait... non-recursive test on path?
you don't know whether path, you are defining for __acl, is directory or file?
i'm confused.

if that is the case, then we could just do simple test and sed 's/x/X/ on wanted line in acl_is output...

*Created by: 4nd3r* wait... non-recursive test on path? you don't know whether path, you are defining for `__acl`, is directory or file? i'm confused. if that is the case, then we could just do simple test and `sed 's/x/X/` on wanted line in acl_is output...

ungleich-gitea commented

2021-11-20 15:38:43 +00:00

Author

Owner

Created by: 4nd3r

separate explorer(s) with find -perms -type if X is used?

*Created by: 4nd3r* separate explorer(s) with `find -perms -type` if `X` is used?

ungleich-gitea commented

2021-11-20 15:38:44 +00:00

Author

Owner

Created by: jimis

maybe we still should revisit that idea. if --recursive is set then get current state of entire directory contents and comapre it against wanted state with file type check? this way we could solve issue with X. but i can already imagine the complexity of this kind of type...

I definitely do not want recursive comparisons, the way it is now is fine regarding recursion, I agree that the overhead would be too big otherwise.

Non-recursive test for capital X permission is what I was thinking of:

Is it a directory and the ACL x bit is set? Then the ACL is correct.
Is it a file and the x mode bit is set, and the x ACL bit is set? Then the ACL is correct.
Otherwise the ACL is incorrect and needs to be set using "setfacl" .

*Created by: jimis* > maybe we still should revisit that idea. if --recursive is set then get current state of entire directory contents and comapre it against wanted state with file type check? this way we could solve issue with X. but i can already imagine the complexity of this kind of type... I definitely do not want recursive comparisons, the way it is now is fine regarding recursion, I agree that the overhead would be too big otherwise. Non-recursive test for capital X permission is what I was thinking of: * Is it a directory and the ACL `x` bit is set? Then the ACL is correct. * Is it a file and the `x` mode bit is set, and the `x` ACL bit is set? Then the ACL is correct. * Otherwise the ACL is incorrect and needs to be set using "setfacl" .

ungleich-gitea commented

2021-11-20 15:38:45 +00:00

Author

Owner

Created by: 4nd3r

I use find with -type and -exec 😄

jokes a side, how do you know if some perms inside deep directory needs a change? I personally don't like types which do something even when there's no need to, but that's may personal belief 👼

the thing with recursive operations is that they are hard. when i wrote __acl, i gave it a lot of thought. for example, getting recursively state of entire directory content and comparing that against wanted state is somewhat difficult, but doable. and when you have directory with 100k files then it doesn't scale.

since I only use __acl to set up new directories and only care about acl of parent, i just made it this way and forgot it. now your situation gets my attention again and... maybe we still should revisit that idea. if --recursive is set then get current state of entire directory contents and comapre it against wanted state with file type check? this way we could solve issue with X. but i can already imagine the complexity of this kind of type...

also, it's sorta admin choice - if you are going to use that type on directory with 100k files, then you maybe have very bad times...

*Created by: 4nd3r* I use `find` with `-type` and ` -exec` :smile: jokes a side, how do you know if some perms inside deep directory needs a change? I personally don't like types which do something even when there's no need to, but that's may personal belief :angel: the thing with recursive operations is that they are hard. when i wrote `__acl`, i gave it a lot of thought. for example, getting recursively state of entire directory content and comparing that against wanted state is somewhat difficult, but doable. and when you have directory with 100k files then it doesn't scale. since I only use `__acl` to set up new directories and only care about acl of parent, i just made it this way and forgot it. now your situation gets my attention again and... maybe we still should revisit that idea. if `--recursive` is set then get current state of entire directory contents and comapre it against wanted state with file type check? this way we could solve issue with `X`. but i can already imagine the complexity of this kind of type... also, it's sorta admin choice - if you are going to use that type on directory with 100k files, then you maybe have very bad times...

ungleich-gitea commented

2021-11-20 15:38:47 +00:00

Author

Owner

Created by: jimis

@4nd3r I find it really useful. Otherwise how do you set recursively the ACL to "r-x" for directories but to "r--" for files?

*Created by: jimis* @4nd3r I find it really useful. Otherwise how do you set recursively the ACL to "r-x" for directories but to "r--" for files?

ungleich-gitea commented

2021-11-20 15:38:48 +00:00

Author

Owner

Created by: 4nd3r

this is though one. i, myself, would never use such ambiguous feature, but I can't think of good solution for those, who would like to use it. at least we should add note about that in man.rst.

*Created by: 4nd3r* this is though one. i, myself, would never use such ambiguous feature, but I can't think of good solution for those, who would like to use it. at least we should add note about that in man.rst.

ungleich-gitea commented

2021-11-20 15:38:49 +00:00

Author

Owner

Created by: jimis

@4nd3r maybe you have an opinion on this one?

*Created by: jimis* @4nd3r maybe you have an opinion on this one?

ACL type does not converge if permissions include capital X #98