Re: symbolic link curiousity in 3.6.0

2025-04-11 Thread Paul Eggert

On 2025-04-10 15:47, Bruno Haible wrote:

But what I could do is to change Gnulib's file-has-acl module so
that if the environment variable CYGWINLY_PEDANTIC is set,


Oh, let's not do that. We have too many environment variables already.

Thanks for explaining.



Re: symbolic link curiousity in 3.6.0

2025-04-10 Thread Bruno Haible via Gnulib discussion list
Paul Eggert wrote:
> On 4/9/25 04:09, Corinna Vinschen wrote:
> > I would like to ask you to reconsider and to remove the
> > Cygwin-special code and let Cygwin decide by itself, what a trivial vs.
> > non-trivial ACL is by using the POSIXy functions Cygwin provides.
> > Just use acl_extended_file(), please.
> 
> Sorry, I've lost context. I assume you're not suggesting that we revert 
> the patch I recently installed

No, that's not what Corinna means.

It's about whether Gnulib's file-has-acl module (and, by consequence,
the coreutils 'ls' program) shows freshly created files and directories
(e.g. "mkdir foo", "echo > bar") as having ACLs.

My answer is "it should not", and my arguments are in
.

Her answer is "it should", and her arguments are in
,
referring to
.

These points of view are not immediately reconcilable.

But what I could do is to change Gnulib's file-has-acl module so
that if the environment variable CYGWINLY_PEDANTIC is set, it
implements Corinna's point of view. (It's not a large change.)
The effects for a user who has set this environment variable would be:
  - "ls -l" shows the presence of an ACL on nearly all files.
  - The gnulib unit tests of this module and of 'copy-acl' fail.

The CYGWINLY_PEDANTIC name is meant to imitate POSIXLY_CORRECT, but here
we are not talking about POSIX (since POSIX:2024 does not specify ACLs [1]),
and what is "correct" is subjective.

Then users can choose which behaviour of 'ls' they prefer...

Bruno

[1] https://en.wikipedia.org/wiki/Access-control_list#POSIX_ACL






Re: symbolic link curiousity in 3.6.0

2025-04-10 Thread Paul Eggert

On 4/9/25 04:09, Corinna Vinschen wrote:

I would like to ask you to reconsider and to remove the
Cygwin-special code and let Cygwin decide by itself, what a trivial vs.
non-trivial ACL is by using the POSIXy functions Cygwin provides.
Just use acl_extended_file(), please.


Sorry, I've lost context. I assume you're not suggesting that we revert 
the patch I recently installed, which I thought came from your suggestion:


https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=41e7b7e0d159d8ac0eb385964119f350ac9dfc3f

Are you suggesting that we remove all the code in 
gnulib/lib/file-has-acl.c that depends __CYGWIN__ being defined? Or are 
you suggesting that we arrange for USE_LINUX_ATTR to not be defined when 
running on Cygwin? Or something else?


A proposed patch would help clarify what you're suggesting.



Re: symbolic link curiousity in 3.6.0

2025-04-09 Thread Corinna Vinschen
Hi Bruno,

On Mar 29 15:02, Bruno Haible via Cygwin wrote:
> Corinna Vinschen wrote:
> > > Regarding what acl_extended_file() does, there is the man page by
> > > Andreas Grünbacher:
> > > https://www.kernel.org/doc/man-pages/online/pages/man3/acl_extended_file.3.html
> > > Gnulib is not the only user of acl_extended_file(); therefore I would
> > > suggest that Cygwin should follow that man page — regardless of Gnulib.
> > 
> > It already does!  The acl_extended_file() change for directories we just
> > talked about will actually be a deviation from Andreas' man page.
> 
> OK, then Cygwin's acl_extended_file should not change.
> 
> > > An ACL implementation that shows a '+' sign on 99.9% of the files is
> > > not useful. A user can't practically run 'getfacl' on all files and
> > > understand the result. In other words, ACLs need to be the special case,
> > > not the common case.
> > 
> > Yeah, that's a good point.  If the user is beaten with a stick with
> > a '+' sign written on it, it's basically no help.
> 
> Glad to have an agreement here. Then, gnulib won't change: it will not
> use Cygwin's acl_extended_file() function and instead use the function
> acl_access_nontrivial(), defined in Gnulib.

Actually, after having some time to think more about this, I disagree.

First of all, Cygwin provides the acl_extended_file() function and it's
doing the POSIXly correct thing.

Second, your code mixes Windows and POSIX points of view as to what
constitutes a extended vs. trivial ACL.  However, Cygwin is
*deliberatly* using the POSIX point of view and *deliberatly*
providing the user with the POSIX POV.

Third, the filtering of Administrator and SYSTEM GROUP entries is
already performed by Cygwin itself.  Your code is duplicating what
Cygwin does and only adds a test for the Users group, which is
kinda questionable.
If we decide to change the ACL functions to ignore, for example, the
Domain Users group to evaluate an ACL for triviality, this would not be
reflected by ls(1), which is IMHO not a good thing.  And if we did that
accidentally, due to a bug, it might even be covered by ls(1), which is
even worse.

If you create a native Windows version, it's ok to check the ACL inside
gnulib, because there's no such thing as a Windows function returning a
triviality indicator.  But Cygwin is *not* Windows, it's POSIX on
Windows.  And it has a matching function.

Forth, by not trusting Cygwin to do the right thing and adding a lot
of unnecessary code for each single ACL, you slow down ls(1) even more
on Cygwin, which already gets a beating for being slow.

I think it would be nice if you would accept how we do things and
why we do things.  Gnulib supports the target system and fills the gaps
where the target system is lacking, and I'm very grateful for gnulib.
But I'm not talking about bugs here, rather about what we think is right
or wrong.  And in this case, I think we're doing the right thing, and
gnulib "knows better" and is trampling over it.

Therefore I would like to ask you to reconsider and to remove the
Cygwin-special code and let Cygwin decide by itself, what a trivial vs.
non-trivial ACL is by using the POSIXy functions Cygwin provides.
Just use acl_extended_file(), please.


Thanks,
Corinna



Re: symbolic link curiousity in 3.6.0

2025-04-07 Thread Paul Eggert

On 2025-04-04 10:40, Pádraig Brady wrote:


Is this patch is OK to be applied to gnulib,
with the tweaks Corinna mentioned?


Yes, I did that just now, and propagated it into coreutils.



Re: symbolic link curiousity in 3.6.0

2025-04-07 Thread Paul Eggert

On 2025-03-30 05:50, Corinna Vinschen wrote:


The definition of O_PATH requires an additional

   #include 


Thanks, I added that and installed the patch into Gnulib.



However, assuming not only Cygwin is affected, shouldn't the patch
rather use O_PATH if it's available, O_RDONLY if not?


I hope only Cygwin is affected. The only other platforms mentioned in 
the code that might also be affected are IRIX and Tru64, neither of 
which is supported these days. I don't think the code would work on a 
Linux kernel, as acl_get_fd doesn't work with O_PATH fds as I recall; 
however this code should never be compiled with Linux so it should be OK.


If we run into the problem on a non-Cygwin platform we can complicate 
the patch along the lines that you suggest. Any such complication would 
involve more than merely using O_RDONLY, though, as the code should work 
even on unreadable files.




-#if HAVE_ACL_GET_LINK_NP /* FreeBSD, NetBSD >= 10 */
+#if HAVE_ACL_GET_LINK_NP /* FreeBSD, NetBSD >= 10, Cygwin >= 2.5 */


Changing the comment is wrong, IMHO.  We're adding a local version of
HAVE_ACL_GET_LINK_NP specificially because Cygwin (or, FWIW, any system
only providing the POSIX.1e draft 17 function) does not provide
acl_get_link_np, isn't it?


As far as I know only Cygwin has the problem.



For Cygwin, I will add this function nevertheless, but it will only be
available in some upcoming version, either 3.6.1 or 3.7.0.


It's funny that so many operating systems are adding an 
"acl_get_link_np" function to mean the same thing, when "_np" means "not 
portable". This API is badly designed but I guess we're stuck with it.




In terms of coreutils, I think either ls(1) gobble_file() or
file_has_aclinfo_cache() should still handle ENOENT from
file_has_aclinfo() and not print any error message.


I'm not so sure about this, but one issue at a time.



Re: symbolic link curiousity in 3.6.0

2025-04-05 Thread Bruno Haible via Gnulib discussion list
Corinna Vinschen wrote:
> > Regarding what acl_extended_file() does, there is the man page by
> > Andreas Grünbacher:
> > https://www.kernel.org/doc/man-pages/online/pages/man3/acl_extended_file.3.html
> > Gnulib is not the only user of acl_extended_file(); therefore I would
> > suggest that Cygwin should follow that man page — regardless of Gnulib.
> 
> It already does!  The acl_extended_file() change for directories we just
> talked about will actually be a deviation from Andreas' man page.

OK, then Cygwin's acl_extended_file should not change.

> > An ACL implementation that shows a '+' sign on 99.9% of the files is
> > not useful. A user can't practically run 'getfacl' on all files and
> > understand the result. In other words, ACLs need to be the special case,
> > not the common case.
> 
> Yeah, that's a good point.  If the user is beaten with a stick with
> a '+' sign written on it, it's basically no help.

Glad to have an agreement here. Then, gnulib won't change: it will not
use Cygwin's acl_extended_file() function and instead use the function
acl_access_nontrivial(), defined in Gnulib.

> But your question was specificially about files created in the above
> dir:
> 
> - Files created by a Cygwin process inside that dir will have only the
>   usual three ACL entries constituting standard POSIX permission bits,
>   combined with their umask, i.e.:
> 
>   $ cd /tmp/glo1FkFx/tmpdir0
>   $ umask
>   22
>   $ touch foo
>   $ getfacl foo
>   # file: foo
>   # owner: corinna
>   # group: vinschen
>   user::rw-
>   group::r--
>   other::r--
> 
> - Files created by non-Cygwin processes inside that dir will have by
>   default(*) only the usual three ACL entries constituting standard
>   POSIX permission bits, but there's no umask handling in the non-Cygwin
>   process, i.e.
> 
>   $ cd /tmp/glo1FkFx/tmpdir0
>   $ cmd
>   C:\cygwin64\tmp\glQatIoG\tmpdir0>echo foo > bar
>   C:\cygwin64\tmp\glQatIoG\tmpdir0>exit
>   $ getfacl bar
>   # file: bar
>   # owner: corinna
>   # group: vinschen
>   user::rwx
>   group::r-x
>   other::r-x
> 
>   (*) Most Windows processes rely entirely on the permissions in
>   the ACLs and the Windows ACL inheritance rules.
> 
> Does that answer your question?

Thanks for explaining. This matches my earlier experiments on Cygwin;
see these comments in the acl_access_nontrivial function:
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/acl-internal.c;h=6c50feacbb811da92da9a68d544132d87ad8dbf3;hb=HEAD#l80

So, in summary, I too see the action item (regarding the behaviour
of 'ls' on symlinks) more on the coreutils side.

Bruno






Re: symbolic link curiousity in 3.6.0

2025-04-05 Thread Paul Eggert

On 3/29/25 04:30, Corinna Vinschen wrote:

What it should do if only the POSIX.1e draft 17 functions are available
is something along these lines:


Yes, that sounds like a better approach. However, shouldn't it use 
O_PATH not O_RDONLY? We might lack read access.


Does the attached Gnulib patch work for you? I haven't tested or 
installed it (I don't use Cygwin).From e245ab6ac865c7ff723837645886eb717c53a754 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 29 Mar 2025 10:27:01 -0600
Subject: [PATCH] file-has-acl: port symlink code to Cygwin

Problem reported by Corinna Vinschen in:
https://lists.gnu.org/r/bug-gnulib/2025-03/msg00112.html
* lib/file-has-acl.c (acl_get_link_np): New static function,
defined only if needed.
(HAVE_ACL_GET_LINK_NP): Define this if defining acl_get_link_np.
---
 ChangeLog  |  9 +
 lib/file-has-acl.c | 21 -
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 58195260cf..a7fa40dc33 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2025-03-29  Paul Eggert  
+
+	file-has-acl: port symlink code to Cygwin
+	Problem reported by Corinna Vinschen in:
+	https://lists.gnu.org/r/bug-gnulib/2025-03/msg00112.html
+	* lib/file-has-acl.c (acl_get_link_np): New static function,
+	defined only if needed.
+	(HAVE_ACL_GET_LINK_NP): Define this if defining acl_get_link_np.
+
 2025-03-29  Bruno Haible  
 
 	acl-permissions: Update comments regarding NetBSD.
diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
index 179e805bd4..2538b61944 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -362,6 +362,25 @@ acl_nfs4_nontrivial (uint32_t *xattr, ssize_t nbytes)
 }
 #endif
 
+#if (!USE_LINUX_XATTR && USE_ACL && HAVE_ACL_GET_FD \
+ && !HAVE_ACL_EXTENDED_FILE && !HAVE_ACL_TYPE_EXTENDED \
+ && !HAVE_ACL_GET_LINK_NP && defined O_PATH)
+/* Like acl_get_file, but do not follow symbolic links.  */
+static acl_t
+acl_get_link_np (char const *name, acl_type_t type)
+{
+  int fd = open (name, O_PATH | O_NOFOLLOW);
+  if (fd < 0)
+return NULL;
+  acl_t r = acl_get_fd (fd);
+  int err = errno;
+  close (fd);
+  errno = err;
+  return r;
+}
+# define HAVE_ACL_GET_LINK_NP 1
+#endif
+
 /* Return 1 if NAME has a nontrivial access control list,
0 if ACLs are not supported, or if NAME has no or only a base ACL,
and -1 (setting errno) on error.  Note callers can determine
@@ -467,7 +486,7 @@ file_has_aclinfo (MAYBE_UNUSED char const *restrict name,
   ret = -1;
 #   else /* FreeBSD, NetBSD >= 10, IRIX, Tru64, Cygwin >= 2.5 */
 acl_t (*acl_get_file_or_link) (char const *, acl_type_t) = acl_get_file;
-#if HAVE_ACL_GET_LINK_NP /* FreeBSD, NetBSD >= 10 */
+#if HAVE_ACL_GET_LINK_NP /* FreeBSD, NetBSD >= 10, Cygwin >= 2.5 */
 if (! (flags & ACL_SYMLINK_FOLLOW))
   acl_get_file_or_link = acl_get_link_np;
 #endif
-- 
2.34.1



Re: symbolic link curiousity in 3.6.0

2025-04-05 Thread Bruno Haible via Gnulib discussion list
Corinna Vinschen wrote:
> Btw., while I was testing test-file-has-acl.sh, I found two more bugs,
> one in test-file-has-acl.sh, and one (a problem of account handling in
> Windows) in Cygwin's setfacl(1).  Together with the above change to
> acl_extended_file(), fixing the bug in setfacl(1) is sufficient to run
> test-file-has-acl.sh successfully.
> 
> 
> However:
> 
> I ran the script with VERBOSE=1 and this is what is printed:
> 
>   [...]
>   + chmod 600 tmpfile0
>   + acl_flavor=none
>   + acl_flavor=linux
>   [...]
> 
> Oops, acl_flavor=linux? 
> 
> Turns out, the script checks the output for a --set-file option,
> which is supported by Cygwin's setfacl since commit ed4d919c24646
> ("setfacl: Rename the option --file to --set-file, as on Linux").

This should have been fixed two years ago already:
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=2d4adbff973ee2cc186cb6256b61d246c254fe93
You must be running an old copy of test-file-has-acl.sh.

Bruno






Re: symbolic link curiousity in 3.6.0

2025-04-05 Thread Corinna Vinschen
Hi Bruno,

thanks for your quick reply!

On Mar 28 15:30, Bruno Haible via Cygwin wrote:
> [CCing bug-gnulib]
> 
> Corinna Vinschen wrote in
> :
> > I found the problem, it's in a gnulib header. See below.
> > ...
> > Gnulib's acl-internal.h contains this:
> > 
> >   /* Linux-specific */
> >   /* Cygwin >= 2.5 implements this function, but it returns 1 for all
> >  directories, thus is unusable.  */
> >#  if !defined HAVE_ACL_EXTENDED_FILE || defined __CYGWIN__
> >#   undef HAVE_ACL_EXTENDED_FILE
> >#   define HAVE_ACL_EXTENDED_FILE false
> >#   define acl_extended_file(name) (-1)
> >#  endif
> > 
> > This is simply not true.  Cygwin's acl_extended_file only returns
> > 1 on dirs, if they actually contain more than the 3 default entries
> > to emulate POSIX access.  I just tried it and it works exactly
> > as required.
> > 
> > Can this be fixed, please?
> 
> If I comment out that part:
> /* || defined __CYGWIN__ */
> I get three test failures
> 
> FAIL: test-file-has-acl.sh
> ==
> 
> file_has_acl("tmpdir0") returned yes, expected no
> FAIL test-file-has-acl.sh (exit status: 1)
> [...]
> from a testdir created with
> $ ./gnulib-tool --create-testdir --dir=../testdir1 --single-configure acl 
> acl-permissions file-has-acl copy-file
> 
> This is reproducible with both Cygwin 2.9 and 3.6.0.
> 
> So, from my point of view, the situation is still the same as when
> I introduced this workaround, in
> https://lists.gnu.org/archive/html/bug-gnulib/2018-09/msg00089.html
> 
> Therefore, I see two possible ways forward:
>   a) The Cygwin function acl_extended_file gets modified so that it
>  is actually usable by Gnulib (i.e. does not cause test failures),
> or
>   b) Investigate how to deal with the "Not supported" error in coreutils.
>  (Maybe silence and ignore this error?)

Option b would be a change in coreutils, let's put that aside for now
and stick to gnulib, becasue that decides over the actual library
function used on Cygwin.

In terms of gnulib, there's option a or an option c:

  c) The expectations of test-file-has-acl.sh are wrong.

I only tested test-file-has-acl.sh so far, because it doesn't make
sense to continue without clarifying the basics first.

Here's what happens:

- When you create a dir in Cygwin, the dir will get a Windows ACL
  which is equivalent to a POSIX ACL with 6 entries:

  $ mkdir /tmp/glo1FkFx/tmpdir0
  $ getfacl /tmp/glo1FkFx/tmpdir0
  # file: /tmp/glo1FkFx/tmpdir0/
  # owner: corinna
  # group: vinschen
  user::rwx
  group::r-x
  other::r-x
  default:user::rwx
  default:group::r-x
  default:other::r-x

  We have to do this to make sure that native (i. e. non-Cygwin)
  processes creating files inside of the new dir will by default create
  ACLs which conform to the POSIX rules.  Note that Cygwin processes
  don't need the default ACL entries, but, alas, we're not alone in the
  world.

- However, being the POSIX-conformant citizen we try to be, Cygwin's
  acl_extended_file() *only* returns 0 if the ACL contains three
  entries.  Any ACL of three entries consists of the default POSIX
  permissions for user/group/other only.

- Therefore, a Cygwin-generated dir with default permissions is always
  an extended ACL by default.

- Outside of a Cygwin-generated directory tree, plain Windows rules
  apply, and Windows ACLs generated under Windows rules are always
  extended ACLs (with a 99.9% probability) from a POSIX POV.

- Therefore, 99.9% of the time, acl_extended_file("a-dir") returns 1,
  and *correctly* so from the POSIX POV.

- If you want acl_extended_file("a-dir") to return 0, run

setfacl -bk a-dir

So, how can we go forward here?

- As far as I can see, acl_extended_file() is doing the right thing, so
  at first glance option c applies.

- But I can also see the point that a directory created with mkdir
  should start out with a standard POSIX ACL.

  We can't do that literally for the reason cited above, but we *could*
  change acl_extended_file().  It could check a directory ACL, if it
  only consists of 3 or 6 ACL entries, and if it consists of 6 entries,
  the entries 4 to 6 *must* be the default entries for user/group/other.

  That would be option a.

  I created a patch for Cygwin to behave as just outlined, and it
  makes test-file-has-acl.sh behave as desired, because
  acl_extended_file("a-dir") now returns 0 after mkdir.

The ultimate question is, what is right or wrong?

Cygwin can't change the way mkdir creates the default ACL without
getting into trouble with non-Cygwin executables, so we either have
to go with

  a) acl_extended_file() changing its behaviour to handle dirs
 created by mkdir as non-extended ACLs, or with

  c) Changing test-file-has-acl.sh to take the unique situation
 in Cygwin into account and acknowledge that Cygwin's
 acl_extended_file() returns a correct value.

So, what is it, option a or c?


Re: symbolic link curiousity in 3.6.0

2025-04-05 Thread Bruno Haible via Gnulib discussion list
Corinna Vinschen wrote:
> Btw., there's still a small bug in test-file-has-acl.sh.  It tries to
> create an entry with gid 0:
> 
>   setfacl -m group:0:1 tmpfile0
> 
> But that's not possible, because there's no Windows group mapped to
> uid or gid 0, unless you create your own /etc/passwd and /etc/group
> files.
> 
> There's deliberately no default mapping from any Windows SID to user or
> group 0, i.e., root, because there's no equivalent Windows account.
> Administrator, Administrators, SYSTEM, Domain Admins, Backup Operators,
> etc, etc... there's just no direct match possible, but the uid/gid must
> map to a valid Windows SID.
> 
> What you can do is use group 1.  This group always exists, because
> it maps to the group NT AUTHORITY\DIALUP, SID S-1-5-1.

Thanks for reporting this! Fixed like you suggest:


2025-03-31  Bruno Haible  

acl, file-has-acl tests: Strengthen tests on Cygwin.
Suggested by Corinna Vinschen in
.
* tests/test-set-mode-acl.sh: On Cygwin, use group 1 instead of the
non-existent group 0.
* tests/test-copy-acl.sh: Likewise.
* tests/test-file-has-acl.sh: Likewise.

diff --git a/tests/test-copy-acl.sh b/tests/test-copy-acl.sh
index 8efe202afb..061755f124 100755
--- a/tests/test-copy-acl.sh
+++ b/tests/test-copy-acl.sh
@@ -310,7 +310,9 @@ cd "$builddir" ||
   cygwin)
 
 # Set an ACL for a group.
-setfacl -m group:0:1 tmpfile0
+# Group 1 in Cygwin corresponds to the DIALUP users (cf.
+# 
).
+setfacl -m group:1:1 tmpfile0
 
 func_test_copy tmpfile0 tmpfile2
 
@@ -320,7 +322,7 @@ cd "$builddir" ||
 func_test_copy tmpfile0 tmpfile4
 
 # Remove the ACL for the group.
-setfacl -d group:0 tmpfile0
+setfacl -d group:1 tmpfile0
 
 func_test_copy tmpfile0 tmpfile5
 
diff --git a/tests/test-file-has-acl.sh b/tests/test-file-has-acl.sh
index 1e791f3ae8..1ce388bbc0 100755
--- a/tests/test-file-has-acl.sh
+++ b/tests/test-file-has-acl.sh
@@ -255,12 +255,14 @@ cd "$builddir" ||
   cygwin)
 
 # Set an ACL for a group.
-if setfacl -m group:0:1 tmpfile0; then
+# Group 1 in Cygwin corresponds to the DIALUP users (cf.
+# 
).
+if setfacl -m group:1:1 tmpfile0; then
 
   func_test_has_acl tmpfile0 yes
 
   # Remove the ACL for the group.
-  setfacl -d group:0 tmpfile0
+  setfacl -d group:1 tmpfile0
 
   func_test_has_acl tmpfile0 no
 
diff --git a/tests/test-set-mode-acl.sh b/tests/test-set-mode-acl.sh
index fa9771e034..18eb72acf8 100755
--- a/tests/test-set-mode-acl.sh
+++ b/tests/test-set-mode-acl.sh
@@ -183,7 +183,9 @@ cd "$builddir" ||
 setfacl -m user:$auid:1 tmpfile0
 ;;
   cygwin)
-setfacl -m group:0:1 tmpfile0
+# Group 1 in Cygwin corresponds to the DIALUP users (cf.
+# 
).
+setfacl -m group:1:1 tmpfile0
 ;;
   hpux)
 orig=`lsacl tmpfile0 | sed -e 's/ tmpfile0$//'`






Re: symbolic link curiousity in 3.6.0

2025-04-04 Thread Bruno Haible via Gnulib discussion list
Hi Corinna,

>   c) The expectations of test-file-has-acl.sh are wrong.

The Cygwin-specific part of that unit test has minimal expectations:

# Set an ACL for a group.
if setfacl -m group:0:1 tmpfile0; then

  func_test_has_acl tmpfile0 yes

  # Remove the ACL for the group.
  setfacl -d group:0 tmpfile0

  func_test_has_acl tmpfile0 no

fi

Namely:
  - After we set an ACL for a group, there is an ACL.
  - After we remove this ACL, there is no ACL any more.

> Here's what happens:
> 
> - When you create a dir in Cygwin, the dir will get a Windows ACL
>   which is equivalent to a POSIX ACL with 6 entries:
> 
>   $ mkdir /tmp/glo1FkFx/tmpdir0
>   $ getfacl /tmp/glo1FkFx/tmpdir0
>   # file: /tmp/glo1FkFx/tmpdir0/
>   # owner: corinna
>   # group: vinschen
>   user::rwx
>   group::r-x
>   other::r-x
>   default:user::rwx
>   default:group::r-x
>   default:other::r-x
> 
>   We have to do this to make sure that native (i. e. non-Cygwin)
>   processes creating files inside of the new dir will by default create
>   ACLs which conform to the POSIX rules.  Note that Cygwin processes
>   don't need the default ACL entries, but, alas, we're not alone in the
>   world.

OK, and what does this mean for the *files* created in such a directory?

> - However, being the POSIX-conformant citizen we try to be, Cygwin's
>   acl_extended_file() *only* returns 0 if the ACL contains three
>   entries.  Any ACL of three entries consists of the default POSIX
>   permissions for user/group/other only.
> 
> - Therefore, a Cygwin-generated dir with default permissions is always
>   an extended ACL by default.
> 
> - Outside of a Cygwin-generated directory tree, plain Windows rules
>   apply, and Windows ACLs generated under Windows rules are always
>   extended ACLs (with a 99.9% probability) from a POSIX POV.
> 
> - Therefore, 99.9% of the time, acl_extended_file("a-dir") returns 1,
>   and *correctly* so from the POSIX POV.

This sounds all reasonable from the points of view of
  - Windows interoperability,
  - compliance with the never-standardized POSIX ACL specification
(see ).

The point of view that I'm using in Gnulib is that it must make sense
for the end user, that is, for the user who creates files and shares
files with other users on the same machine. For such a user
  - the absence of an ACL means "the usual chmod based permissions matter",
  - the presence of an ACL means "attention! special rules! run getfacl
to make sure you understand what you are doing".
The GNU coreutils 'ls' program helps the user making this distinction,
by displaying a '+' sign after the permissions field.

An ACL implementation that shows a '+' sign on 99.9% of the files is
not useful. A user can't practically run 'getfacl' on all files and
understand the result. In other words, ACLs need to be the special case,
not the common case.

> - But I can also see the point that a directory created with mkdir
>   should start out with a standard POSIX ACL.
> 
>   We can't do that literally for the reason cited above, but we *could*
>   change acl_extended_file().

Yes, if you change Cygwin's acl_extended_file(), Gnulib might be able
to use this function.

>   It could check a directory ACL, if it
>   only consists of 3 or 6 ACL entries, and if it consists of 6 entries,
>   the entries 4 to 6 *must* be the default entries for user/group/other.

I'm not so bothered with directories (because directory ACLs are one level
of complexity above the file ACLs, and Gnulib never got to the point of
having a reasonable cross-platform behaviour of directory ACLs), rather
with files. What is the change to acl_extended_file() on files that you
are considering?

> The ultimate question is, what is right or wrong?

Regarding what Gnulib does, I explained above.

Regarding what acl_extended_file() does, there is the man page by
Andreas Grünbacher:
https://www.kernel.org/doc/man-pages/online/pages/man3/acl_extended_file.3.html
Gnulib is not the only user of acl_extended_file(); therefore I would
suggest that Cygwin should follow that man page — regardless of Gnulib.

Bruno






Re: symbolic link curiousity in 3.6.0

2025-04-04 Thread Pádraig Brady

On 29/03/2025 16:31, Paul Eggert wrote:

On 3/29/25 04:30, Corinna Vinschen wrote:

What it should do if only the POSIX.1e draft 17 functions are available
is something along these lines:


Yes, that sounds like a better approach. However, shouldn't it use
O_PATH not O_RDONLY? We might lack read access.

Does the attached Gnulib patch work for you? I haven't tested or
installed it (I don't use Cygwin).


Is this patch is OK to be applied to gnulib,
with the tweaks Corinna mentioned?

thanks,
Pádraig



Re: symbolic link curiousity in 3.6.0

2025-03-31 Thread Paul Eggert

On 3/31/25 12:26, Corinna Vinschen wrote:

ls(1) always potentially shows a past state anyway.


Sure, but traditionally (and I'm talking about 7th edition Unix) a 
single output line of 'ls' corresponded to a state obtained atomically 
from the file system. I realize we can't always do that nowadays but the 
further we depart from it, the worse 'ls' users will be.




Re: symbolic link curiousity in 3.6.0

2025-03-31 Thread Corinna Vinschen
On Mar 31 12:12, Paul Eggert via Cygwin wrote:
> On 3/31/25 11:27, Pádraig Brady wrote:
> > The file could be deleted at any time.
> > We're just suppressing errors in the edge case it's deleted
> 
> More generally, though, the file could be renamed and another put in its
> place, which means that an attacker could cause 'ls' to generate a line that
> does not correspond to any state of any file.
> 
> For this sort of attack an O_PATH solution is the only defense I can think
> of (for systems with O_PATH and /proc/self/fd; I don't know of solutions
> elsewhere.) And if we use O_PATH for this, we've solved the problem for the
> file-being-deleted case too.

I see what you're up to, but is that really an issue?

ls(1) always potentially shows a past state anyway.  The user can
*never* rely on the fact that the files it just printed out still exist
in the same state  or at all after ls finished.  And the worst case
which might happen in terms of the problem at hand is that ls -l doesn't
print the '+' after the permission bits.


Corinna



Re: symbolic link curiousity in 3.6.0

2025-03-31 Thread Paul Eggert

On 3/31/25 11:27, Pádraig Brady wrote:

The file could be deleted at any time.
We're just suppressing errors in the edge case it's deleted


More generally, though, the file could be renamed and another put in its 
place, which means that an attacker could cause 'ls' to generate a line 
that does not correspond to any state of any file.


For this sort of attack an O_PATH solution is the only defense I can 
think of (for systems with O_PATH and /proc/self/fd; I don't know of 
solutions elsewhere.) And if we use O_PATH for this, we've solved the 
problem for the file-being-deleted case too.




Re: symbolic link curiousity in 3.6.0

2025-03-31 Thread Pádraig Brady

On 31/03/2025 17:32, Paul Eggert wrote:

On 2025-03-30 07:26, Pádraig Brady wrote:

On 30/03/2025 13:50, Corinna Vinschen wrote:

In terms of coreutils, I think either ls(1) gobble_file() or
file_has_aclinfo_cache() should still handle ENOENT from
file_has_aclinfo() and not print any error message.  After all, due to
the preconditions for building acl_get_link_np, we can't be sure
acl_get_link_np has really been built into file-has-acl.c, and the
problem persists.


I tend to agree. I'll apply this later:


commit 88385a0d6d56197d3c180432c8a4bca07241e90b (HEAD -> master)
Author: Pádraig Brady 
Date:   Sun Mar 30 15:16:54 2025 +0100

      ls: suppress ENOENT errors when reading ACL info

      * src/ls.c (gobble_file): Indicating unknown ACL info with '?'
      suffices for the edge case of a file being removed while reading,
      or older cygwin when reading through dangling symlinks.
      Reported by Corinna Vinschen.


Not sure this is a good idea, as a file being removed while getting its
attributes indicates a serious issue that's worth bringing to the user's
attention more directly.


I don't think this is a serious issue.
The file could be deleted at any time.
We're just suppressing errors in the edge case it's deleted
in the small window between the stat() and the acl_get_file().
We of course don't guarantee a file is present after we list it,
so we shouldn't jump through hoops for this edge case I think.


How about if we instead use O_PATH on the file first, and then use fstat
etc. on the result? That should fix the
file-removed-while-getting-attributes situation, at least on platforms
with /proc/self/fd (which I assume includes Cygnus). I realize that
flistxattr etc. don't work on fds opened via O_PATH (why? this seems
silly, but it's not my bailiwick) but /proc/self/fd works around that
problem.


cheers,
Pádraig



Re: symbolic link curiousity in 3.6.0

2025-03-31 Thread Paul Eggert

On 2025-03-30 07:26, Pádraig Brady wrote:

On 30/03/2025 13:50, Corinna Vinschen wrote:

In terms of coreutils, I think either ls(1) gobble_file() or
file_has_aclinfo_cache() should still handle ENOENT from
file_has_aclinfo() and not print any error message.  After all, due to
the preconditions for building acl_get_link_np, we can't be sure
acl_get_link_np has really been built into file-has-acl.c, and the
problem persists.


I tend to agree. I'll apply this later:


commit 88385a0d6d56197d3c180432c8a4bca07241e90b (HEAD -> master)
Author: Pádraig Brady 
Date:   Sun Mar 30 15:16:54 2025 +0100

     ls: suppress ENOENT errors when reading ACL info

     * src/ls.c (gobble_file): Indicating unknown ACL info with '?'
     suffices for the edge case of a file being removed while reading,
     or older cygwin when reading through dangling symlinks.
     Reported by Corinna Vinschen.


Not sure this is a good idea, as a file being removed while getting its 
attributes indicates a serious issue that's worth bringing to the user's 
attention more directly.


How about if we instead use O_PATH on the file first, and then use fstat 
etc. on the result? That should fix the 
file-removed-while-getting-attributes situation, at least on platforms 
with /proc/self/fd (which I assume includes Cygnus). I realize that 
flistxattr etc. don't work on fds opened via O_PATH (why? this seems 
silly, but it's not my bailiwick) but /proc/self/fd works around that 
problem.




Re: symbolic link curiousity in 3.6.0

2025-03-30 Thread Corinna Vinschen
Hi Bruno,

On Mar 29 15:02, Bruno Haible via Cygwin wrote:
> Corinna Vinschen wrote:
> > > Regarding what acl_extended_file() does, there is the man page by
> > > Andreas Grünbacher:
> > > https://www.kernel.org/doc/man-pages/online/pages/man3/acl_extended_file.3.html
> > > Gnulib is not the only user of acl_extended_file(); therefore I would
> > > suggest that Cygwin should follow that man page — regardless of Gnulib.
> > 
> > It already does!  The acl_extended_file() change for directories we just
> > talked about will actually be a deviation from Andreas' man page.
> 
> OK, then Cygwin's acl_extended_file should not change.

I'm not entirely sure here...

The three default perm entries are only effective outside Cygwin, so from
a POSIX point of view it's not really an extended ACL...

Btw., there's still a small bug in test-file-has-acl.sh.  It tries to
create an entry with gid 0:

  setfacl -m group:0:1 tmpfile0

But that's not possible, because there's no Windows group mapped to
uid or gid 0, unless you create your own /etc/passwd and /etc/group
files.

There's deliberately no default mapping from any Windows SID to user or
group 0, i.e., root, because there's no equivalent Windows account.
Administrator, Administrators, SYSTEM, Domain Admins, Backup Operators,
etc, etc... there's just no direct match possible, but the uid/gid must
map to a valid Windows SID.

What you can do is use group 1.  This group always exists, because
it maps to the group NT AUTHORITY\DIALUP, SID S-1-5-1.


Thanks,
Corinna




Re: symbolic link curiousity in 3.6.0

2025-03-30 Thread Bruno Haible via Gnulib discussion list
> > > An ACL implementation that shows a '+' sign on 99.9% of the files is
> > > not useful. A user can't practically run 'getfacl' on all files and
> > > understand the result. In other words, ACLs need to be the special case,
> > > not the common case.
> > 
> > Yeah, that's a good point.  If the user is beaten with a stick with
> > a '+' sign written on it, it's basically no help.

Let me summarize these arguments in a comment:


2025-03-30  Bruno Haible  

file-has-acl: Update comments regarding Cygwin.
* lib/acl-internal.h (HAVE_ACL_EXTENDED_FILE, acl_extended_file): Add
more comments.

diff --git a/lib/acl-internal.h b/lib/acl-internal.h
index b32a1cdafd..cb969e9797 100644
--- a/lib/acl-internal.h
+++ b/lib/acl-internal.h
@@ -118,8 +118,13 @@ rpl_acl_set_fd (int fd, acl_t acl)
 #  endif
 
 /* Linux-specific */
-/* Cygwin >= 2.5 implements this function, but it returns 1 for all
-   directories, thus is unusable.  */
+/* Cygwin >= 2.5 implements acl_extended_file(), but it returns 1 for nearly 
all
+   directories — for reasons explained in
+    —, thus 
is
+   unusable.  For the user, 'ls' should not print a '+' sign, indicating the
+   presence of an ACL, for 99,9% of the files; this would not be useful.
+   Therefore, on Cygwin, we ignore the acl_extended_file function and instead
+   use our own acl_access_nontrivial function.  */
 #  if !defined HAVE_ACL_EXTENDED_FILE || defined __CYGWIN__
 #   undef HAVE_ACL_EXTENDED_FILE
 #   define HAVE_ACL_EXTENDED_FILE false






Re: symbolic link curiousity in 3.6.0

2025-03-30 Thread Pádraig Brady

On 30/03/2025 13:50, Corinna Vinschen wrote:

Hi Paul,

thanks for the patch.

On Mar 29 10:31, Paul Eggert via Cygwin wrote:

On 3/29/25 04:30, Corinna Vinschen wrote:

What it should do if only the POSIX.1e draft 17 functions are available
is something along these lines:


Yes, that sounds like a better approach. However, shouldn't it use O_PATH
not O_RDONLY? We might lack read access.

Does the attached Gnulib patch work for you? I haven't tested or installed
it (I don't use Cygwin).



 From e245ab6ac865c7ff723837645886eb717c53a754 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 29 Mar 2025 10:27:01 -0600
Subject: [PATCH] file-has-acl: port symlink code to Cygwin

Problem reported by Corinna Vinschen in:
https://lists.gnu.org/r/bug-gnulib/2025-03/msg00112.html
* lib/file-has-acl.c (acl_get_link_np): New static function,
defined only if needed.
(HAVE_ACL_GET_LINK_NP): Define this if defining acl_get_link_np.
---
  ChangeLog  |  9 +
  lib/file-has-acl.c | 21 -
  2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 58195260cf..a7fa40dc33 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2025-03-29  Paul Eggert  
+
+   file-has-acl: port symlink code to Cygwin
+   Problem reported by Corinna Vinschen in:
+   https://lists.gnu.org/r/bug-gnulib/2025-03/msg00112.html
+   * lib/file-has-acl.c (acl_get_link_np): New static function,
+   defined only if needed.
+   (HAVE_ACL_GET_LINK_NP): Define this if defining acl_get_link_np.
+
  2025-03-29  Bruno Haible  
  
  	acl-permissions: Update comments regarding NetBSD.

diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
index 179e805bd4..2538b61944 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -362,6 +362,25 @@ acl_nfs4_nontrivial (uint32_t *xattr, ssize_t nbytes)
  }
  #endif
  
+#if (!USE_LINUX_XATTR && USE_ACL && HAVE_ACL_GET_FD \

+ && !HAVE_ACL_EXTENDED_FILE && !HAVE_ACL_TYPE_EXTENDED \
+ && !HAVE_ACL_GET_LINK_NP && defined O_PATH)


The definition of O_PATH requires an additional

   #include 

Adding that to gllib/file-has-acl.c, the patch works as desired on
Cygwin.

However, assuming not only Cygwin is affected, shouldn't the patch
rather use O_PATH if it's available, O_RDONLY if not?


-#if HAVE_ACL_GET_LINK_NP /* FreeBSD, NetBSD >= 10 */
+#if HAVE_ACL_GET_LINK_NP /* FreeBSD, NetBSD >= 10, Cygwin >= 2.5 */


Changing the comment is wrong, IMHO.  We're adding a local version of
HAVE_ACL_GET_LINK_NP specificially because Cygwin (or, FWIW, any system
only providing the POSIX.1e draft 17 function) does not provide
acl_get_link_np, isn't it?

For Cygwin, I will add this function nevertheless, but it will only be
available in some upcoming version, either 3.6.1 or 3.7.0.

In terms of coreutils, I think either ls(1) gobble_file() or
file_has_aclinfo_cache() should still handle ENOENT from
file_has_aclinfo() and not print any error message.  After all, due to
the preconditions for building acl_get_link_np, we can't be sure
acl_get_link_np has really been built into file-has-acl.c, and the
problem persists.


I tend to agree. I'll apply this later:


commit 88385a0d6d56197d3c180432c8a4bca07241e90b (HEAD -> master)
Author: Pádraig Brady 
Date:   Sun Mar 30 15:16:54 2025 +0100

ls: suppress ENOENT errors when reading ACL info

* src/ls.c (gobble_file): Indicating unknown ACL info with '?'
suffices for the edge case of a file being removed while reading,
or older cygwin when reading through dangling symlinks.
Reported by Corinna Vinschen.

diff --git a/src/ls.c b/src/ls.c
index 46ec42037..2cf4ee444 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3538,8 +3538,14 @@ gobble_file (char const *name, enum filetype type, ino_t 
inode,

   /* Let the user know via '?' if errno is EACCES, which can
  happen with Linux kernel 6.12 on an NFS file system.
- That's better than a longwinded diagnostic.  */
-  bool cannot_access_acl = n < 0 && errno == EACCES;
+ That's better than a longwinded diagnostic.
+
+ Similarly, ignore ENOENT which may happen on some versions
+ of cygwin when processing dangling symlinks for example.
+ Also if a file is removed while we're reading ACL info,
+ ACL_T_UNKNOWN is sufficient indication for that edge case.  */
+  bool cannot_access_acl = n < 0
+   && (errno == EACCES || errno == ENOENT);

   f->acl_type = (!have_scontext && !have_acl
  ? (cannot_access_acl ? ACL_T_UNKNOWN : ACL_T_NONE)

cheers,
Pádraig.



Re: symbolic link curiousity in 3.6.0

2025-03-30 Thread Corinna Vinschen
Hi Paul,

thanks for the patch.

On Mar 29 10:31, Paul Eggert via Cygwin wrote:
> On 3/29/25 04:30, Corinna Vinschen wrote:
> > What it should do if only the POSIX.1e draft 17 functions are available
> > is something along these lines:
> 
> Yes, that sounds like a better approach. However, shouldn't it use O_PATH
> not O_RDONLY? We might lack read access.
> 
> Does the attached Gnulib patch work for you? I haven't tested or installed
> it (I don't use Cygwin).

> From e245ab6ac865c7ff723837645886eb717c53a754 Mon Sep 17 00:00:00 2001
> From: Paul Eggert 
> Date: Sat, 29 Mar 2025 10:27:01 -0600
> Subject: [PATCH] file-has-acl: port symlink code to Cygwin
> 
> Problem reported by Corinna Vinschen in:
> https://lists.gnu.org/r/bug-gnulib/2025-03/msg00112.html
> * lib/file-has-acl.c (acl_get_link_np): New static function,
> defined only if needed.
> (HAVE_ACL_GET_LINK_NP): Define this if defining acl_get_link_np.
> ---
>  ChangeLog  |  9 +
>  lib/file-has-acl.c | 21 -
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 58195260cf..a7fa40dc33 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,12 @@
> +2025-03-29  Paul Eggert  
> +
> + file-has-acl: port symlink code to Cygwin
> + Problem reported by Corinna Vinschen in:
> + https://lists.gnu.org/r/bug-gnulib/2025-03/msg00112.html
> + * lib/file-has-acl.c (acl_get_link_np): New static function,
> + defined only if needed.
> + (HAVE_ACL_GET_LINK_NP): Define this if defining acl_get_link_np.
> +
>  2025-03-29  Bruno Haible  
>  
>   acl-permissions: Update comments regarding NetBSD.
> diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
> index 179e805bd4..2538b61944 100644
> --- a/lib/file-has-acl.c
> +++ b/lib/file-has-acl.c
> @@ -362,6 +362,25 @@ acl_nfs4_nontrivial (uint32_t *xattr, ssize_t nbytes)
>  }
>  #endif
>  
> +#if (!USE_LINUX_XATTR && USE_ACL && HAVE_ACL_GET_FD \
> + && !HAVE_ACL_EXTENDED_FILE && !HAVE_ACL_TYPE_EXTENDED \
> + && !HAVE_ACL_GET_LINK_NP && defined O_PATH)

The definition of O_PATH requires an additional

  #include 

Adding that to gllib/file-has-acl.c, the patch works as desired on
Cygwin.

However, assuming not only Cygwin is affected, shouldn't the patch
rather use O_PATH if it's available, O_RDONLY if not?

> -#if HAVE_ACL_GET_LINK_NP /* FreeBSD, NetBSD >= 10 */
> +#if HAVE_ACL_GET_LINK_NP /* FreeBSD, NetBSD >= 10, Cygwin >= 2.5 */

Changing the comment is wrong, IMHO.  We're adding a local version of
HAVE_ACL_GET_LINK_NP specificially because Cygwin (or, FWIW, any system
only providing the POSIX.1e draft 17 function) does not provide
acl_get_link_np, isn't it?

For Cygwin, I will add this function nevertheless, but it will only be
available in some upcoming version, either 3.6.1 or 3.7.0.

In terms of coreutils, I think either ls(1) gobble_file() or
file_has_aclinfo_cache() should still handle ENOENT from
file_has_aclinfo() and not print any error message.  After all, due to
the preconditions for building acl_get_link_np, we can't be sure
acl_get_link_np has really been built into file-has-acl.c, and the
problem persists.

>  if (! (flags & ACL_SYMLINK_FOLLOW))
>acl_get_file_or_link = acl_get_link_np;
>  #endif


Thanks,
Corinna



Re: symbolic link curiousity in 3.6.0

2025-03-29 Thread Brian Inglis

On 2025-03-29 12:14, Brian Inglis via Cygwin wrote:

On 2025-03-29 08:02, Bruno Haible via Cygwin wrote:

Corinna Vinschen wrote:

Regarding what acl_extended_file() does, there is the man page by
Andreas Grünbacher:
https://www.kernel.org/doc/man-pages/online/pages/man3/acl_extended_file.3.html
Gnulib is not the only user of acl_extended_file(); therefore I would
suggest that Cygwin should follow that man page — regardless of Gnulib.


It already does!  The acl_extended_file() change for directories we just
talked about will actually be a deviation from Andreas' man page.


OK, then Cygwin's acl_extended_file should not change.


An ACL implementation that shows a '+' sign on 99.9% of the files is
not useful. A user can't practically run 'getfacl' on all files and
understand the result. In other words, ACLs need to be the special case,
not the common case.


Yeah, that's a good point.  If the user is beaten with a stick with
a '+' sign written on it, it's basically no help.


Glad to have an agreement here. Then, gnulib won't change: it will not
use Cygwin's acl_extended_file() function and instead use the function
acl_access_nontrivial(), defined in Gnulib.


But your question was specificially about files created in the above
dir:

- Files created by a Cygwin process inside that dir will have only the
   usual three ACL entries constituting standard POSIX permission bits,
   combined with their umask, i.e.:

   $ cd /tmp/glo1FkFx/tmpdir0
   $ umask
   22
   $ touch foo
   $ getfacl foo
   # file: foo
   # owner: corinna
   # group: vinschen
   user::rw-
   group::r--
   other::r--

- Files created by non-Cygwin processes inside that dir will have by
   default(*) only the usual three ACL entries constituting standard
   POSIX permission bits, but there's no umask handling in the non-Cygwin
   process, i.e.

   $ cd /tmp/glo1FkFx/tmpdir0
   $ cmd
   C:\cygwin64\tmp\glQatIoG\tmpdir0>echo foo > bar
   C:\cygwin64\tmp\glQatIoG\tmpdir0>exit
   $ getfacl bar
   # file: bar
   # owner: corinna
   # group: vinschen
   user::rwx
   group::r-x
   other::r-x

   (*) Most Windows processes rely entirely on the permissions in
   the ACLs and the Windows ACL inheritance rules.

Does that answer your question?


Thanks for explaining. This matches my earlier experiments on Cygwin;
see these comments in the acl_access_nontrivial function:
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/acl- 
internal.c;h=6c50feacbb811da92da9a68d544132d87ad8dbf3;hb=HEAD#l80


So, in summary, I too see the action item (regarding the behaviour
of 'ls' on symlinks) more on the coreutils side.


Please reiterate what you see needing to be patched in coreutils 9.6 - just your 
patch 0001-file-has-acl-port-symlink-code-to-Cygwin.patch or something else?


Dogfooding coreutils 9.6 since release - now also under Cygwin 3.6 - permissions 
on build symlinks:


$ uname -srvmo
CYGWIN_NT-10.0-19045 3.6.0-1.x86_64 2025-03-18 17:01 UTC x86_64 Cygwin
$ ls --version
ls (GNU coreutils) 9.6
Packaged by Cygwin (9.6-1)
Copyright (C) 2025 Free Software Foundation, Inc.
...


Sorry - will make more sense with symlink entry:

 $ ls -l coreutils-9.6-1.x86_64/build/GNUmakefile | cyg-sanitize-output.sed
 lrwxrwxrwx 1 $USER None 71 Feb 10 14:45 
coreutils-9.6-1.x86_64/build/GNUmakefile -> 
/usr/src/coreutils/coreutils-9.6-1.x86_64/src/coreutils-9.6/GNUmakefile


[and if I cc Bruno and bug-gnulib]


$ lsp coreutils-9.6-1.x86_64/build/GNUmakefile # ls, getfacl, icacls
-rw-r--r-- 1 $USER None 4589 Jan 17 07:46 .../build/GNUmakefile

# file: coreutils-9.6-1.x86_64/build/GNUmakefile
# owner: $USER
# group: None
user::rw-
group::r--
other::r--

.../src/coreutils-9.6/GNUmakefile    $HOSTNAME/$USER:(R,W,D,WDAC,WO)
     $HOSTNAME/None:(R)
     Everyone:(R)

Successfully processed 1 files; Failed processing 0 files

--
Take care. Thanks, Brian Inglis  Calgary, Alberta, Canada

La perfection est atteinte   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retrancher  but when there is no more to cut
-- Antoine de Saint-Exupéry



Re: symbolic link curiousity in 3.6.0

2025-03-29 Thread Corinna Vinschen
On Mar 29 12:58, Bruno Haible via Cygwin wrote:
> Corinna Vinschen wrote:
> > Btw., while I was testing test-file-has-acl.sh, I found two more bugs,
> > one in test-file-has-acl.sh, and one (a problem of account handling in
> > Windows) in Cygwin's setfacl(1).  Together with the above change to
> > acl_extended_file(), fixing the bug in setfacl(1) is sufficient to run
> > test-file-has-acl.sh successfully.
> > 
> > 
> > However:
> > 
> > I ran the script with VERBOSE=1 and this is what is printed:
> > 
> >   [...]
> >   + chmod 600 tmpfile0
> >   + acl_flavor=none
> >   + acl_flavor=linux
> >   [...]
> > 
> > Oops, acl_flavor=linux? 
> > 
> > Turns out, the script checks the output for a --set-file option,
> > which is supported by Cygwin's setfacl since commit ed4d919c24646
> > ("setfacl: Rename the option --file to --set-file, as on Linux").
> 
> This should have been fixed two years ago already:
> https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=2d4adbff973ee2cc186cb6256b61d246c254fe93
> You must be running an old copy of test-file-has-acl.sh.

Oh drat.  I did a `git fetch' but missed the `git merge' afterwards.

Sigh.  Let me recheck with the latest version...


Corinna



Re: symbolic link curiousity in 3.6.0

2025-03-29 Thread Corinna Vinschen
On Mar 29 13:12, Corinna Vinschen via Cygwin wrote:
> On Mar 29 12:43, Bruno Haible via Cygwin wrote:
> > OK, and what does this mean for the *files* created in such a directory?
> 
> Just for clarity, permissions in Windows are *always* defined by an ACL.
> There's no such thing as default POSIX perms.  Don't try to look at
> non-Cygwin-created files from a POSIX permission POV, it's a lost cause.
> 
> But your question was specificially about files created in the above
> dir:
> 
> - Files created by a Cygwin process inside that dir will have only the
>   usual three ACL entries constituting standard POSIX permission bits,
>   combined with their umask, i.e.:
> 
>   $ cd /tmp/glo1FkFx/tmpdir0
>   $ umask
>   22
>   $ touch foo
>   $ getfacl foo
>   # file: foo
>   # owner: corinna
>   # group: vinschen
>   user::rw-
>   group::r--
>   other::r--
> 
> - Files created by non-Cygwin processes inside that dir will have by
>   default(*) only the usual three ACL entries constituting standard
>   POSIX permission bits, but there's no umask handling in the non-Cygwin
>   process, i.e.
> 
>   $ cd /tmp/glo1FkFx/tmpdir0
>   $ cmd
>   C:\cygwin64\tmp\glQatIoG\tmpdir0>echo foo > bar
>   C:\cygwin64\tmp\glQatIoG\tmpdir0>exit
>   $ getfacl bar
>   # file: bar
>   # owner: corinna
>   # group: vinschen
>   user::rwx
>   group::r-x
>   other::r-x
> 
>   (*) Most Windows processes rely entirely on the permissions in
>   the ACLs and the Windows ACL inheritance rules.
> 
> Does that answer your question?
> [...]
> For non-Cygwin-generated files, there's no such thing as a standard ACL,
> and the Cygwin user, looking from the POSIX perspective when using it,
> should see the '+'-sign to notice this file has no POSIX permissions.

Erm... except for non-Cygwin-generated files created in a Cygwin-created
dir, as outlined in the above example, in which case the inherited
default permissions emulate POSIX permissions and acl_extended_file() on
them returns 0.

I wasn't sure this was clear from the example...


Corinna



Re: symbolic link curiousity in 3.6.0

2025-03-29 Thread Corinna Vinschen
On Mar 29 12:43, Bruno Haible via Cygwin wrote:
> Hi Corinna,
> 
> >   c) The expectations of test-file-has-acl.sh are wrong.
> 
> The Cygwin-specific part of that unit test has minimal expectations:
> 
> # Set an ACL for a group.
> if setfacl -m group:0:1 tmpfile0; then
> 
>   func_test_has_acl tmpfile0 yes
> 
>   # Remove the ACL for the group.
>   setfacl -d group:0 tmpfile0
> 
>   func_test_has_acl tmpfile0 no
> 
> fi
> 
> Namely:
>   - After we set an ACL for a group, there is an ACL.
>   - After we remove this ACL, there is no ACL any more.

That is affected by the setfacl problem I mentioned in my previous mail.
I don't know if it's worth discussing.  If you think so, I'm happy to
oblige.

> > Here's what happens:
> > 
> > - When you create a dir in Cygwin, the dir will get a Windows ACL
> >   which is equivalent to a POSIX ACL with 6 entries:
> > 
> >   $ mkdir /tmp/glo1FkFx/tmpdir0
> >   $ getfacl /tmp/glo1FkFx/tmpdir0
> >   # file: /tmp/glo1FkFx/tmpdir0/
> >   # owner: corinna
> >   # group: vinschen
> >   user::rwx
> >   group::r-x
> >   other::r-x
> >   default:user::rwx
> >   default:group::r-x
> >   default:other::r-x
> > 
> >   We have to do this to make sure that native (i. e. non-Cygwin)
> >   processes creating files inside of the new dir will by default create
> >   ACLs which conform to the POSIX rules.  Note that Cygwin processes
> >   don't need the default ACL entries, but, alas, we're not alone in the
> >   world.
> 
> OK, and what does this mean for the *files* created in such a directory?

Just for clarity, permissions in Windows are *always* defined by an ACL.
There's no such thing as default POSIX perms.  Don't try to look at
non-Cygwin-created files from a POSIX permission POV, it's a lost cause.

But your question was specificially about files created in the above
dir:

- Files created by a Cygwin process inside that dir will have only the
  usual three ACL entries constituting standard POSIX permission bits,
  combined with their umask, i.e.:

  $ cd /tmp/glo1FkFx/tmpdir0
  $ umask
  22
  $ touch foo
  $ getfacl foo
  # file: foo
  # owner: corinna
  # group: vinschen
  user::rw-
  group::r--
  other::r--

- Files created by non-Cygwin processes inside that dir will have by
  default(*) only the usual three ACL entries constituting standard
  POSIX permission bits, but there's no umask handling in the non-Cygwin
  process, i.e.

  $ cd /tmp/glo1FkFx/tmpdir0
  $ cmd
  C:\cygwin64\tmp\glQatIoG\tmpdir0>echo foo > bar
  C:\cygwin64\tmp\glQatIoG\tmpdir0>exit
  $ getfacl bar
  # file: bar
  # owner: corinna
  # group: vinschen
  user::rwx
  group::r-x
  other::r-x

  (*) Most Windows processes rely entirely on the permissions in
  the ACLs and the Windows ACL inheritance rules.

Does that answer your question?

> > - However, being the POSIX-conformant citizen we try to be, Cygwin's
> >   acl_extended_file() *only* returns 0 if the ACL contains three
> >   entries.  Any ACL of three entries consists of the default POSIX
> >   permissions for user/group/other only.
> > 
> > - Therefore, a Cygwin-generated dir with default permissions is always
> >   an extended ACL by default.
> > 
> > - Outside of a Cygwin-generated directory tree, plain Windows rules
> >   apply, and Windows ACLs generated under Windows rules are always
> >   extended ACLs (with a 99.9% probability) from a POSIX POV.
> > 
> > - Therefore, 99.9% of the time, acl_extended_file("a-dir") returns 1,
> >   and *correctly* so from the POSIX POV.
> 
> This sounds all reasonable from the points of view of
>   - Windows interoperability,
>   - compliance with the never-standardized POSIX ACL specification
> (see ).
> 
> The point of view that I'm using in Gnulib is that it must make sense
> for the end user, that is, for the user who creates files and shares
> files with other users on the same machine. For such a user
>   - the absence of an ACL means "the usual chmod based permissions matter",
>   - the presence of an ACL means "attention! special rules! run getfacl
> to make sure you understand what you are doing".
> The GNU coreutils 'ls' program helps the user making this distinction,
> by displaying a '+' sign after the permissions field.
> 
> An ACL implementation that shows a '+' sign on 99.9% of the files is
> not useful. A user can't practically run 'getfacl' on all files and
> understand the result. In other words, ACLs need to be the special case,
> not the common case.

Yeah, that's a good point.  If the user is beaten with a stick with
a '+' sign written on it, it's basically no help.

> > - But I can also see the point that a directory created with mkdir
> >   should start out with a standard POSIX ACL.
> > 
> >   We can't do that literally for the reason cited above, but we *could*
> >   change acl_extended_file().
> 
> Yes, if you change Cygwin's acl_extended_file(), Gnulib might be a

Re: symbolic link curiousity in 3.6.0

2025-03-29 Thread Corinna Vinschen
Hi Pádraig,

thanks for your reply!

On Mar 28 20:30, Pádraig Brady via Cygwin wrote:
> On 28/03/2025 14:30, Bruno Haible via Gnulib discussion list wrote:
> > [CCing bug-gnulib]
> > 
> > Corinna Vinschen wrote in
> > :
> 
> Responding to previous messages in the above thread,
> I think the triggering commit in coreutils 9.6 was:
> https://github.com/coreutils/coreutils/commit/4ce432ad8
> 
> This displays ai->u.err on any issue getting acls etc.
> However in the non USE_LINUX_XATTR case in file_has_aclinfo() in gnulib
> we only initialize ai->u.err=ENOTSUP, but never set it otherwise.
> 
> So that means in coreutils we shouldn't be inspecting u.err at all,
> and just printing errno like:
> 
> diff --git a/src/ls.c b/src/ls.c
> index 244484439..46ec42037 100644
> --- a/src/ls.c
> +++ b/src/ls.c
> @@ -3549,7 +3549,7 @@ gobble_file (char const *name, enum filetype type, 
> ino_t inode,
>any_has_acl |= f->acl_type != ACL_T_NONE;
> 
>if (format == long_format && n < 0 && !cannot_access_acl)
> -error (0, ai.u.err, "%s", quotef (full_name));
> +error (0, errno, "%s", quotef (full_name));
>else
>  {
>/* When requesting security context information, don't make
> 
> 
> Note the coreutils code seemed to always output all errors from 
> file_has_acl(),
> so I'm presuming on earlier versions of coreutils we did output an ENOENT
> error for the dangling symlink on cygwin?

No, it never did, up to and including coreutils 9.5.  AFAICS, the
culprit is commit b58e321c8d5dd ("ls: suppress "Permission denied"errors
on NFS")

What happens is this:

- gobble_file() calls file_has_aclinfo_cache() on a symlink
  without the ACL_SYMLINK_FOLLOW flag.

- file_has_aclinfo_cache() calls file_has_aclinfo() from gnulib.

- On systems only providing the ACL functions defined in POSIX.1e draft 17,
  it calls acl_get_file().

The problem is that acl_get_file() always follows symlinks.  There's no
option to not follow symlinks.  Given the target file doesn't exist, it
returns ENOENT.

IMHO this is a bug in gnulib's file_has_aclinfo() function.

What it should do if only the POSIX.1e draft 17 functions are available
is something along these lines:

  if (flags & ACL_SYMLINK_FOLLOW
fd = open ("file-or-dir", O_RDONLY);
  else
fd = open ("file-or-dir", O_RDONLY | O_NOFOLLOW);
  if (fd >= 0)
ret = acl_get_fd (fd);

However, under the assumption we stick to the current implementation
of file_has_aclinfo(), gobble_file() would have to check if the
errno returned is ENOENT, and the file itself is a symlink.  Don't
print an error message in that case.


Corinna



Re: symbolic link curiousity in 3.6.0

2025-03-28 Thread Pádraig Brady

On 28/03/2025 14:30, Bruno Haible via Gnulib discussion list wrote:

[CCing bug-gnulib]

Corinna Vinschen wrote in
:


Responding to previous messages in the above thread,
I think the triggering commit in coreutils 9.6 was:
https://github.com/coreutils/coreutils/commit/4ce432ad8

This displays ai->u.err on any issue getting acls etc.
However in the non USE_LINUX_XATTR case in file_has_aclinfo() in gnulib
we only initialize ai->u.err=ENOTSUP, but never set it otherwise.

So that means in coreutils we shouldn't be inspecting u.err at all,
and just printing errno like:

diff --git a/src/ls.c b/src/ls.c
index 244484439..46ec42037 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3549,7 +3549,7 @@ gobble_file (char const *name, enum filetype type, ino_t 
inode,
   any_has_acl |= f->acl_type != ACL_T_NONE;

   if (format == long_format && n < 0 && !cannot_access_acl)
-error (0, ai.u.err, "%s", quotef (full_name));
+error (0, errno, "%s", quotef (full_name));
   else
 {
   /* When requesting security context information, don't make


Note the coreutils code seemed to always output all errors from file_has_acl(),
so I'm presuming on earlier versions of coreutils we did output an ENOENT
error for the dangling symlink on cygwin?
That may be avoided with the adjustments discussed below.



I found the problem, it's in a gnulib header. See below.
...
Gnulib's acl-internal.h contains this:

   /* Linux-specific */
   /* Cygwin >= 2.5 implements this function, but it returns 1 for all
  directories, thus is unusable.  */
#  if !defined HAVE_ACL_EXTENDED_FILE || defined __CYGWIN__
#   undef HAVE_ACL_EXTENDED_FILE
#   define HAVE_ACL_EXTENDED_FILE false
#   define acl_extended_file(name) (-1)
#  endif

This is simply not true.  Cygwin's acl_extended_file only returns
1 on dirs, if they actually contain more than the 3 default entries
to emulate POSIX access.  I just tried it and it works exactly
as required.

Can this be fixed, please?


If I comment out that part:
 /* || defined __CYGWIN__ */
I get three test failures

FAIL: test-file-has-acl.sh
==

file_has_acl("tmpdir0") returned yes, expected no
FAIL test-file-has-acl.sh (exit status: 1)

FAIL: test-file-has-acl-1.sh


file_has_acl("tmpdir0") returned yes, expected no
FAIL test-file-has-acl-1.sh (exit status: 1)

FAIL: test-file-has-acl-2.sh


file_has_acl("tmpdir0") returned yes, expected no
FAIL test-file-has-acl-2.sh (exit status: 1)


from a testdir created with
$ ./gnulib-tool --create-testdir --dir=../testdir1 --single-configure acl 
acl-permissions file-has-acl copy-file

This is reproducible with both Cygwin 2.9 and 3.6.0.

So, from my point of view, the situation is still the same as when
I introduced this workaround, in
https://lists.gnu.org/archive/html/bug-gnulib/2018-09/msg00089.html

Therefore, I see two possible ways forward:
   a) The Cygwin function acl_extended_file gets modified so that it
  is actually usable by Gnulib (i.e. does not cause test failures),
or
   b) Investigate how to deal with the "Not supported" error in coreutils.
  (Maybe silence and ignore this error?)

Bruno









Re: symbolic link curiousity in 3.6.0

2025-03-28 Thread Brian Inglis

On 2025-03-28 08:30, Bruno Haible via Cygwin wrote:

[CCing bug-gnulib]

Corinna Vinschen wrote in
:

I found the problem, it's in a gnulib header. See below.
...
Gnulib's acl-internal.h contains this:

   /* Linux-specific */
   /* Cygwin >= 2.5 implements this function, but it returns 1 for all
  directories, thus is unusable.  */
#  if !defined HAVE_ACL_EXTENDED_FILE || defined __CYGWIN__
#   undef HAVE_ACL_EXTENDED_FILE
#   define HAVE_ACL_EXTENDED_FILE false
#   define acl_extended_file(name) (-1)
#  endif

This is simply not true.  Cygwin's acl_extended_file only returns
1 on dirs, if they actually contain more than the 3 default entries
to emulate POSIX access.  I just tried it and it works exactly
as required.

Can this be fixed, please?


If I comment out that part:
 /* || defined __CYGWIN__ */
I get three test failures

FAIL: test-file-has-acl.sh
==

file_has_acl("tmpdir0") returned yes, expected no
FAIL test-file-has-acl.sh (exit status: 1)

FAIL: test-file-has-acl-1.sh


file_has_acl("tmpdir0") returned yes, expected no
FAIL test-file-has-acl-1.sh (exit status: 1)

FAIL: test-file-has-acl-2.sh


file_has_acl("tmpdir0") returned yes, expected no
FAIL test-file-has-acl-2.sh (exit status: 1)


Expectations are incorrect: all NTFS files under Cygwin have ACLs and 
directories have ACLS and DACLs, and probably also NFS files and directories, 
unless you can access a FAT or other filesystem; but not on my local, nor 
probably our CI, whether GH or Appveyor?


Some package tests are skipped because they can not find suitable files or 
directories: for example, accessible but not owned by the build owner IIRC.

Perhaps these should also be such cases
If any test does any ftw, it should test all the cases required, as ftw seems to 
crawl all over our filesystems, taking ages, possibly because symlinks are maybe 
not checked for everywhere, or cygdrive is mapped to /mnt a la Linux?



from a testdir created with
$ ./gnulib-tool --create-testdir --dir=../testdir1 --single-configure acl 
acl-permissions file-has-acl copy-file

This is reproducible with both Cygwin 2.9 and 3.6.0.

So, from my point of view, the situation is still the same as when
I introduced this workaround, in
https://lists.gnu.org/archive/html/bug-gnulib/2018-09/msg00089.html

Therefore, I see two possible ways forward:
   a) The Cygwin function acl_extended_file gets modified so that it
  is actually usable by Gnulib (i.e. does not cause test failures),
or
   b) Investigate how to deal with the "Not supported" error in coreutils.
  (Maybe silence and ignore this error?)


Or could translate "readlink/at" ENOENT to EACCESS, although POSIX says EACCESS 
means "Search permission is denied for a component of the path prefix of path" 
or "The access mode of the open file description associated with fd is not 
O_SEARCH and the permissions of the directory underlying fd do not permit 
directory searches."?


I already maintain a bunch of Cygwin coreutils patches: one more will just delay 
starting to build a while longer, while I work around the latest releases 
changes! ;^>


--
Take care. Thanks, Brian Inglis  Calgary, Alberta, Canada

La perfection est atteinte   Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter  not when there is no more to add
mais lorsqu'il n'y a plus rien à retrancher  but when there is no more to cut
-- Antoine de Saint-Exupéry



Re: symbolic link curiousity in 3.6.0

2025-03-28 Thread Bruno Haible via Gnulib discussion list
[CCing bug-gnulib]

Corinna Vinschen wrote in
:
> I found the problem, it's in a gnulib header. See below.
> ...
> Gnulib's acl-internal.h contains this:
> 
>   /* Linux-specific */
>   /* Cygwin >= 2.5 implements this function, but it returns 1 for all
>  directories, thus is unusable.  */
>#  if !defined HAVE_ACL_EXTENDED_FILE || defined __CYGWIN__
>#   undef HAVE_ACL_EXTENDED_FILE
>#   define HAVE_ACL_EXTENDED_FILE false
>#   define acl_extended_file(name) (-1)
>#  endif
> 
> This is simply not true.  Cygwin's acl_extended_file only returns
> 1 on dirs, if they actually contain more than the 3 default entries
> to emulate POSIX access.  I just tried it and it works exactly
> as required.
> 
> Can this be fixed, please?

If I comment out that part:
/* || defined __CYGWIN__ */
I get three test failures

FAIL: test-file-has-acl.sh
==

file_has_acl("tmpdir0") returned yes, expected no
FAIL test-file-has-acl.sh (exit status: 1)

FAIL: test-file-has-acl-1.sh


file_has_acl("tmpdir0") returned yes, expected no
FAIL test-file-has-acl-1.sh (exit status: 1)

FAIL: test-file-has-acl-2.sh


file_has_acl("tmpdir0") returned yes, expected no
FAIL test-file-has-acl-2.sh (exit status: 1)


from a testdir created with
$ ./gnulib-tool --create-testdir --dir=../testdir1 --single-configure acl 
acl-permissions file-has-acl copy-file

This is reproducible with both Cygwin 2.9 and 3.6.0.

So, from my point of view, the situation is still the same as when
I introduced this workaround, in
https://lists.gnu.org/archive/html/bug-gnulib/2018-09/msg00089.html

Therefore, I see two possible ways forward:
  a) The Cygwin function acl_extended_file gets modified so that it
 is actually usable by Gnulib (i.e. does not cause test failures),
or
  b) Investigate how to deal with the "Not supported" error in coreutils.
 (Maybe silence and ignore this error?)

Bruno