Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

searching @INC shouldn't abort on a permissions error #14495

Open
p5pRT opened this issue Feb 11, 2015 · 9 comments
Open

searching @INC shouldn't abort on a permissions error #14495

p5pRT opened this issue Feb 11, 2015 · 9 comments
Labels

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Feb 11, 2015

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

Searchable as RT123795$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 11, 2015

From simon.foley@credit-suisse.com

Simon Foley
CREDIT SUISSE AG
Information Technology | Market Data Engineering - LDN, KFVK 143
One Cabot Square | E14 4QJ London | United Kingdom
Phone +44 20 7888 1569 | Mobile +44 79 7632 3270
simon.foley@​credit-suisse.com<mailto​:simon.foley@​credit-suisse.com> | www.credit-suisse.com<http​://www.credit-suisse.com>

===============================================================================
Please access the attached hyperlink for an important electronic communications disclaimer​:
http​://www.credit-suisse.com/legal/en/disclaimer_email_ib.html

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 11, 2015

From simon.foley@credit-suisse.com

Created by simon@simonfoley.net

Please see BUG ID​: 123270

It appears to be assumed by the developers that to DIE when one of the @​INC paths for modules exists but can not be read,
is acceptable behaviour. However I believe this to be irrational. Just because e.g. one of the PERL5LIB
paths exists but can not be read due to file permissions should not mean that perl dies and stops
iterating through the rest of the @​INC paths. One of the remaining @​INC paths may well have the
modules the perl scripts require just because one path can not be read in should not mean perl die's.
Older versions of perl (e.g. 5.12.3) did not implement such behaviour and iterated through the entire
@​INC Path. The problem is that in a production environment in large IT departments there are many teams that
control the OS envirnment and the PERL5LIB variable. e.g. SYBASE's SYBASE.sh\SYBASE.env include files.
somebody else adds a broken path to the PERL5LIB variable and you break all perl users!
Perl5 needs to be more robust to protect from human error and only warn not DIE.
Also the errorm message is not specific enough ... it needs to specify the PATH in the @​INC that it can not read,
noty just the module that it is currently searcing for.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.20.1:

Configured by m665134 at Fri Feb  6 15:58:20 GMT 2015.

Summary of my perl5 (revision 5 version 20 subversion 1) configuration:
   
  Platform:
    osname=linux, osvers=2.6.18-274.el5, archname=x86_64-linux
    uname='linux gbl20001624 2.6.18-274.el5 #1 smp fri jul 8 17:36:59 edt 2011 x86_64 x86_64 x86_64 gnulinux '
    config_args=''
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.1.2 20080704 (Red Hat 4.1.2-51)', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib /lib /lib64 /usr/lib64 /usr/local/lib64
    libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.5.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.5'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'



@INC for perl 5.20.1:
    /app/gmd/pm
    /cs/gmd/sw/perl5.20.1.linux/lib/site_perl/5.20.1/x86_64-linux
    /cs/gmd/sw/perl5.20.1.linux/lib/site_perl/5.20.1
    /cs/gmd/sw/perl5.20.1.linux/lib/5.20.1/x86_64-linux
    /cs/gmd/sw/perl5.20.1.linux/lib/5.20.1
    .


Environment for perl 5.20.1:
    HOME=/app/gmd/home/gmdadmin
    LANG=en_gb
    LANGUAGE=C
    LC_ALL=C
    LD_LIBRARY_PATH=/cs/oracle/product/11204/client_1/lib:/cs/oracle/product/11204/client_1/instantclient:
    LOGDIR (unset)
    PATH=/app/gmd/home/gmdadmin/bin:/app/gmd/scripts/:/app/gmd/bin:/app/gmd/rmds/site/scripts:/app/gmd/rmds/ref/scripts:/bin:/usr/ucb:/usr/local/bin:/usr/kerberos/bin:/usr/bin:/usr/X11R6/bin:/sbin:/usr/sbin:/app/gmd/monitor/scripts:/usr/local/site/bin
    PERL5LIB=/app/gmd/pm
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 11, 2015

From simon@simonfoley.net

Please see BUG ID​: 123270

It appears to be assumed by the developers that to DIE when one of the @​INC paths for modules exists but can not be read,is acceptable behaviour. However I believe this to be irrational. Just because e.g. one of the PERL5LIB paths exists but can not be read due to file permissions should not mean that perl dies and stops iterating through the rest of the @​INC paths. One of the remaining @​INC paths may well have the modules the perl scripts require just because one path can not be read in should not mean perl die's.Older versions of perl (e.g. 5.12.3) did not implement such behavior and iterated through the entire @​INC Path. The problem is that in a production environment in large IT departments there are many teams that control the OS environment and the PERL5LIB variable. e.g. SYBASE's SYBASE.sh\SYBASE.env include files. somebody else adds a broken path to the PERL5LIB variable and you break all perl users! Perl5 needs to be more robust to protect from human error and only warn not DIE. Also the error message is not specific enough ... it needs to specify the PATH in the @​INC that it can not read, not just the module that it is currently searching for.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 11, 2015

From [Unknown Contact. See original ticket]

Please see BUG ID​: 123270

It appears to be assumed by the developers that to DIE when one of the @​INC paths for modules exists but can not be read,is acceptable behaviour. However I believe this to be irrational. Just because e.g. one of the PERL5LIB paths exists but can not be read due to file permissions should not mean that perl dies and stops iterating through the rest of the @​INC paths. One of the remaining @​INC paths may well have the modules the perl scripts require just because one path can not be read in should not mean perl die's.Older versions of perl (e.g. 5.12.3) did not implement such behavior and iterated through the entire @​INC Path. The problem is that in a production environment in large IT departments there are many teams that control the OS environment and the PERL5LIB variable. e.g. SYBASE's SYBASE.sh\SYBASE.env include files. somebody else adds a broken path to the PERL5LIB variable and you break all perl users! Perl5 needs to be more robust to protect from human error and only warn not DIE. Also the error message is not specific enough ... it needs to specify the PATH in the @​INC that it can not read, not just the module that it is currently searching for.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 14, 2015

From @jkeenan

On Wed Feb 11 05​:37​:45 2015, simon@​simonfoley.net wrote​:

Please see BUG ID​: 123270

It appears to be assumed by the developers that to DIE when one of the
@​INC paths for modules exists but can not be read,is acceptable
behaviour. However I believe this to be irrational. Just because e.g.
one of the PERL5LIB paths exists but can not be read due to file
permissions should not mean that perl dies and stops iterating through
the rest of the @​INC paths. One of the remaining @​INC paths may well
have the modules the perl scripts require just because one path can
not be read in should not mean perl die's.
Older versions of perl (e.g.
5.12.3) did not implement such behavior and iterated through the
entire @​INC Path.

You are correct in thinking that perl's behavior has changed in this regard in recent versions. The change in behavior is documented in pod/perl5180delta.pod​:

#####
=head2 C<require> dies for unreadable files

When C<require> encounters an unreadable file, it now dies. It used to
ignore the file and continue searching the directories in C<@​INC>
[perl #113422].

#####

However, I would argue that this is an improvement, not a regression.

The problem is that in a production environment in
large IT departments there are many teams that control the OS
environment and the PERL5LIB variable.

I would argue that *that* is a *broken*, insecure production environment.

e.g. SYBASE's

SYBASE.sh\SYBASE.env include files. somebody else adds a broken path
to the PERL5LIB variable and you break all perl users! Perl5 needs to
be more robust to protect from human error and only warn not DIE. Also
the error message is not specific enough ... it needs to specify the
PATH in the @​INC that it can not read, not just the module that it is
currently searching for.

Different people and different organizations will have different opinions about this. In many organizations, if a user commits a permissions violation -- advertently or inadvertently -- the user gets an Access Denied error and no further information. To tell the user precisely how he violated permissions is to feed him information that can be used to build a map of what is permitted inside a certain system and what is not. I agree with that approach; hence, I agree with the approach perl has taken here since 5.18.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 14, 2015

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 16, 2015

From @iabyn

On Sat, Mar 14, 2015 at 04​:48​:31PM -0700, James E Keenan via RT wrote​:

Also

the error message is not specific enough ... it needs to specify the
PATH in the @​INC that it can not read, not just the module that it is
currently searching for.

Different people and different organizations will have different
opinions about this. In many organizations, if a user commits a
permissions violation -- advertently or inadvertently -- the user gets
an Access Denied error and no further information. To tell the user
precisely how he violated permissions is to feed him information that
can be used to build a map of what is permitted inside a certain system
and what is not. I agree with that approach; hence, I agree with the
approach perl has taken here since 5.18.

Note that since 5.21.7 we now include the path in the error message​:

  $ perl5216 -I/root -Mstrict -e 1
  Can't locate strict.pm​: Permission denied.
  BEGIN failed--compilation aborted.

  $ perl5217 -I/root -Mstrict -e 1
  Can't locate strict.pm​: /root/strict.pm​: Permission denied.
  BEGIN failed--compilation aborted.

--
"Foul and greedy Dwarf - you have eaten the last candle."
  -- "Hordes of the Things", BBC Radio.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 17, 2015

From @ap

On Wed Feb 11 05​:37​:45 2015, simon@​simonfoley.net wrote​:

One of the remaining @​INC paths may well have the modules the perl
scripts require just because one path can not be read in should not
mean perl die's. Older versions of perl (e.g. 5.12.3) did not
implement such behavior and iterated through the entire @​INC Path.

… which sometimes led to people being very, very confused.

The problem is, there are two cases if @​INC contains /some/restricted/path​: /some/restricted/path/Module.pm exists or it doesn’t exist. Now the ideal behaviour goes like this​:

• If /some/restricted/path/Module.pm exists but perl cannot read it due
  to a lack of permissions, perl should throw an error and stop. If it
  just goes on and loads a different Module.pm from somewhere else in
  @​INC, that can be bewildering to debug.

• If /some/restricted/path/Module.pm doesn’t exist, perl should just
  keep searching @​INC, as you would expect. If loading modules just
  broke any time someone added a path to @​INC which perl does not have
  permission to read, that will be painful in a shared install.

The problem is that you cannot distinguish these cases.

If perl doesn’t have permission on any of the directories in /some/restricted/path, then when it tries to open /some/restricted/path/Module.pm, it will get the *same* error both if the file exists *and* if it doesn’t.

So perl cannot know whether the file exists. All it knows is that for *some* reason it doesn’t have permission to read it. So you have to make a choice​: perl must either do the wrong thing if the file exists or do the wrong thing if the file doesn’t exist.

And in 5.20 it was changed to do the wrong thing in the other case.

Perl5 needs to be more robust to protect from human error
and only warn not DIE.

That doesn’t sound like a bad idea, actually. It should fix the original problem that some people got really confused trying to figure out why the changes they were making to a file weren’t having any effect whatsoever. (Solution​: perl didn’t have permissions to load the file, so it was loading another similar file from later in @​INC, so it seemed to somehow magically ignore changes to the file.) That is what the change in 5.20 was supposed to fix.

Just one warning wouldn’t be enough to fully cover that case though, I think​: perl should probably report *every* path involved, meaning not just the paths it skipped, but also the path it then ended up loading. So it would warn at least twice.

--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Mar 19, 2015

From @rjbs

* Aristotle Pagaltzis via RT <perlbug-followup@​perl.org> [2015-03-17T08​:10​:31]

Just one warning wouldn’t be enough to fully cover that case though, I think​:
perl should probably report *every* path involved, meaning not just the paths
it skipped, but also the path it then ended up loading. So it would warn at
least twice.

Without looking at the implementation, I'd like to think it could accumulate
the failed attempts and then provide them all in a single warning.

Something like, but better than​:

  Resolved %s to %s because earlier options could not be read​: %s

  Resolved X​::Y to /foo/bar/X/Y.pm because earlier options could not be read​:
  /foo/baz/X/Y.pm /foo/quux/X/Y.pm

...but maybe it's no big deal.

--
rjbs

@p5pRT p5pRT added the Severity Low label Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.