search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 560 forks source link

Perl second-guesses Cygwin for permission override, sometimes incorrectly #14845

Open p5pRT opened 9 years ago

p5pRT commented 9 years ago

Migrated from rt.perl.org#125788 (status was 'open')

Searchable as RT125788$

p5pRT commented 9 years ago

From vano@mail.mipt.ru

Created by vano@mail.mipt.ru

Follow-up of #125740.

When checking `-r/-w file'\, pp_ftrread (pp_sys.c​:3071) does a `stat'\, then\, in `Perl_cando' (doio.c​:2065) guesses whether to override its results.

Sometimes\, it guesses incorrectly​: 1) In some cases where Cygwin uses an old ID mapping (https://rt.perl.org/Public/Bug/Display.html?id=125740#txn-1360344) 2) When a user is not a member of Administrators but holds SeBackupPrivilege/SeRestorePrivilege (https://rt.perl.org/Public/Bug/Display.html?id=125740#txn-1359529). Moreover\, Cygwin correctly makes SeBackupPrivilege override only read and SeRestorePrivilege - only write.

Since commit https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=commit;h=2c1ffdbf5e6f2767ab63e67834530539d36c6c0b, Cygwin supports the overriding itself in its faccessat/eaccess/access. So it's better not to second-guess it.

Perl Info ``` Flags: category=core severity=low Site configuration information for perl 5.23.2: Configured by User at Fri Aug 7 11:37:29 AST 2015. Summary of my perl5 (revision 5 version 23 subversion 2) configuration: Derived from: 2245a2697d9662043aa9aa1fdfc9002e9438a089 Ancestor: 85d323d4d59cc15f5c76bca4c1df90317f88d0e8 Platform: osname=cygwin, osvers=2.2.0(0.28953), archname=cygwin-thread-multi-64int uname='cygwin_nt-5.1 iru-notebook 2.2.0(0.28953) 2015-08-03 12:49 i686 cygwin ' config_args='-Dprefix=/opt/perl5 -Dusedevel -Doptimize=-O0 -ggdb3 -Dldflags=-ggdb3 -DDEBUGGING -de' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define use64bitint=define, use64bitall=undef, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags ='-DPERL_USE_SAFE_PUTENV -U__STRICT_ANSI__ -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -D_FORTIFY_SOURCE=2', optimize='-O0 -ggdb3', cppflags='-DPERL_USE_SAFE_PUTENV -U__STRICT_ANSI__ -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong' ccversion='', gccversion='4.9.3', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678, doublekind=3 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12, longdblkind=3 ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='g++', ldflags ='-ggdb3 -Wl,--enable-auto-import -Wl,--export-all-symbols -Wl,--enable-auto-image-base -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib /lib libs=-lpthread -ldl -lcrypt perllibs=-lpthread -ldl -lcrypt libc=/usr/lib/libc.a, so=dll, useshrplib=true, libperl=cygperl5_23_2.dll gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags=' --shared -ggdb3 -Wl,--enable-auto-import -Wl,--export-all-symbols -Wl,--enable-auto-image-base -L/usr/local/lib -fstack-protector-strong' Locally applied patches: uncommitted-changes a90ad6d3cfecd7f26e2a8932159963ea31307bca 77890ada5ef197f9be3406cfe66be13e91f48b84 2245a2697d9662043aa9aa1fdfc9002e9438a089 @INC for perl 5.23.2: lib /opt/perl5/lib/site_perl/5.23.2/cygwin-thread-multi-64int /opt/perl5/lib/site_perl/5.23.2 /opt/perl5/lib/5.23.2/cygwin-thread-multi-64int /opt/perl5/lib/5.23.2 . Environment for perl 5.23.2: CYGWIN=nodosfilewarning HOME=/home/User LANG=RU LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin:/usr/bin:/c/bin/rk:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/system32/WBEM:/c/Py/Scripts:/c/Py:/usr/local/bin:/usr/bin:/:/c/bin:%JAVA_HOME%/bin:/c/Program Files/QT Lite/QTSystem:/c/Program Files/TortoiseSVN/bin:/c/Program Files/TortoiseGit/bin:/c/Program Files/Git/cmd:/c/Program Files/TortoiseHg:/c/program files/vim/vim71:/c/Program Files/PuTTY:/c/Program Files/Nmap PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 9 years ago

From @tonycoz

On Tue Aug 11 08​:31​:30 2015\, vano@​mail.mipt.ru wrote​:

Follow-up of #125740.

When checking `-r/-w file'\, pp_ftrread (pp_sys.c​:3071) does a `stat'\, then\, in `Perl_cando' (doio.c​:2065) guesses whether to override its results.

Sometimes\, it guesses incorrectly​: 1) In some cases where Cygwin uses an old ID mapping (https://rt.perl.org/Public/Bug/Display.html?id=125740#txn-1360344) 2) When a user is not a member of Administrators but holds SeBackupPrivilege/SeRestorePrivilege (https://rt.perl.org/Public/Bug/Display.html?id=125740#txn-1359529). Moreover\, Cygwin correctly makes SeBackupPrivilege override only read and SeRestorePrivilege - only write.

Since commit https://cygwin.com/git/gitweb.cgi?p=newlib- cygwin.git;a=commit;h=2c1ffdbf5e6f2767ab63e67834530539d36c6c0b\, Cygwin supports the overriding itself in its faccessat/eaccess/access. So it's better not to second-guess it.

That the permission check operators work off the mode is explicitly documented (from =item -X in perlfunc.pod)​:

The interpretation of the file permission operators C\<-r>\, C\<-R>\, C\<-w>\, C\<-W>\, C\<-x>\, and C\<-X> is by default based solely on the mode of the file and the uids and gids of the user. There may be other reasons you can't actually read\, write\, or execute the file​: for example network filesystem access controls\, ACLs (access control lists)\, read-only filesystems\, and unrecognized executable formats. Note that the use of these six specific operators to verify if some operation is possible is usually a mistake\, because it may be open to race conditions.

Also note that\, for the superuser on the local filesystems\, the C\<-r>\, C\<-R>\, C\<-w>\, and C\<-W> tests always return 1\, and C\<-x> and C\<-X> return 1 if any execute bit is set in the mode. Scripts run by the superuser may thus need to do a stat() to determine the actual mode of the file\, or temporarily set their effective uid to something else.

as is the mechanism to use access()​:

If you are using ACLs\, there is a pragma called C\ that may produce more accurate results than the bare stat() mode bits. When under C\<use filetest 'access'> the above-mentioned filetests test whether the permission can(not) be granted using the access(2) family of system calls. Also note that the C\<-x> and C\<-X> may under this pragma return true even if there are no execute permission bits set (nor any extra execute permission ACLs). This strangeness is due to the underlying system calls' definitions. Note also that\, due to the implementation of C\<use filetest 'access'>\, the C\<_> special filehandle won't cache the results of the file tests when this pragma is in effect. Read the documentation for the C\ pragma for more information.

Is this documented behaviour what you were trying to report? If it was something else please provide some sample code.

Thanks\, Tony

p5pRT commented 9 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 9 years ago

From vano@mail.mipt.ru

That the permission check operators work off the mode is explicitly documented (from =item -X in perlfunc.pod)​:

\<...> Also note that\, for the superuser on the local filesystems\, the C\<-r>\, C\<-R>\, C\<-w>\, and C\<-W> tests always return 1\, and C\<-x> and C\<-X> return 1 if any execute bit is set in the mode. Scripts run by the superuser may thus need to do a stat() to determine the actual mode of the file\, or temporarily set their effective uid to something else.

\<...> Is this documented behaviour what you were trying to report? If it was something else please provide some sample code.

In these terms\, Perl incorrectly detects if the current user is a "superuser" (=a user who overrides permissions) in the specified cases.

-- Best regards\, Ivan mailto​:vano@​mail.mipt.ru

p5pRT commented 9 years ago

From @tonycoz

On Thu Aug 13 04​:25​:25 2015\, vano@​mail.mipt.ru wrote​:

That the permission check operators work off the mode is explicitly documented (from =item -X in perlfunc.pod)​:

\<...> Also note that\, for the superuser on the local filesystems\, the C\<- r>\, C\<-R>\, C\<-w>\, and C\<-W> tests always return 1\, and C\<-x> and C\<-X> return 1 if any execute bit is set in the mode. Scripts run by the superuser may thus need to do a stat() to determine the actual mode of the file\, or temporarily set their effective uid to something else.

\<...> Is this documented behaviour what you were trying to report? If it was something else please provide some sample code.

In these terms\, Perl incorrectly detects if the current user is a "superuser" (=a user who overrides permissions) in the specified cases.

Is there a correct check?

Or is cygwin's emulation of mode flags (because of Win32) sufficiently non-POSIX that it isn't possible to get even close to making it work?

Are you suggesting a fix? If it's "use access()" then you already have an option to do so.

Tony

p5pRT commented 9 years ago

From Stromeko@nexgo.de

Tony Cook via RT \<perlbug-followup \ perl.org> writes​:

In these terms\, Perl incorrectly detects if the current user is a "superuser" (=a user who overrides permissions) in the specified cases.

Is there a correct check?

Perl doesn't have the distinction of checking for "permission overrides"\, it just checks for some sort of "superuser/root". That would be someone with group 544 in his security token on Windows\, kinda-sort-of.

Or is cygwin's emulation of mode flags (because of Win32) sufficiently non-POSIX that it isn't possible to get even close to making it work?

There's a few things that don't translate well at the moment if ACL are structured in a peculiar way (mostly visible on network shares). Work is underway to address these.

Are you suggesting a fix? If it's "use access()" then you already have an option to do so.

That option would only be viable if it filled the stat cache and allowed stacked file tests\, otherwise you would have to re-structure the file tests everywhere. There's some mumbling that this issue maybe addressable sometime in the future\, but in any case this is a different question than "when should Perl ignore the bits it gets from stat and assume it knows better".

Regards\, Achim.

p5pRT commented 9 years ago

From vano@mail.mipt.ru

Wednesday\, August 19\, 2015\, 9​:20​:14 Tony​:

Is there a correct check?

Or is cygwin's emulation of mode flags (because of Win32) sufficiently non-POSIX that it isn't possible to get even close to making it work?

Are you suggesting a fix? If it's "use access()" then you already have an option to do so.

It's not about using `access' or not using `access'. It's about implementing the function to return correct results (and as long as it does - who cares how it is implemented?). If using `access' is the only practical way - then what's wrong with it?

The only other way is\, well\, replicating Cygwin's logic implemented in its fhandler.cc​::fhandler_base​::fhaccess(). I don't want to do this because the copy will become invalid the next time they change their logic (hence the ticket's name).

-- Regards\, Ivan Pozdeev

p5pRT commented 9 years ago

From vano@mail.mipt.ru

The only other way is\, well\, replicating Cygwin's logic implemented in its fhandler.cc​::fhandler_base​::fhaccess(). I don't want to do this because the copy will become invalid the next time they change their logic (hence the ticket's name).

Yet another way is to declare -r/-w without "use filetest 'access'" officially broken in cases where `stat' bits aren't the only thing that governs access (cygwin/other nontrivial emulators\, selinux/similar modules\, network shares). Then the tests need to be rewritten to skip relevant checks in these cases.

This might actually be a good thing​: interpreting the bits by hand where there can be other arbitrary factors is a fundamentally wrong thing to do.

-- Best regards\, Ivan mailto​:vano@​mail.mipt.ru