Bug 127023

Forgot to mention that the building platform was a FC2 system.

See bug #151602 for a related problem.

This is the culprit: perl-5.8.3-findbin-selinux.patch.

This patch, dated 09-Sep-2004, gotten from perl-5.8.6-5.src.rpm, is the
difference between a successful self-test and one that fails on the lib/FindBin
test.  Looking at the patch it is not clear to me what problem it is meant to
address, but it clearly introduces a new problem.

As this bug was originally submitted on 2004-06-30 I wonder if there are
multiple problems at work here.  Tested on a fully up-to-date FC2 system, using
both the latest Perl SRPM and a regular perl-5.8.6 tarball.  For the tarball
builds I added in Red Hat's patches one at a time until the self test broke - on
perl-5.8.3-findbin-selinux.patch.

* Sat Apr 03 2004 Colin Walters <walters> 3:5.8.3-17
- Apply patch to fix FindBin module when access to cwd is disallowed,
  should solve the MRTG/SELinux cron spam issue

Walters appears to be the author of this patch.  Maybe he would have some ideas
about this.

A couple releases of the Perl package have been made since this bug report and
still the problem continues.  Package perl-5.8.5-12.FC3.src.rpm, released
yesterday, continues to fail the FindBin self-test.  So does package
perl-5.8.6-8.src.rpm, from Rawhide.

If Colin Walters can't find the time to address the problem introduced by his
patch, could someone else please look at this situation?  Thanks.

We can probably just revert the patch.  It's probably better to work around this
in mrtg, and we don't really support the strict policy per se now.

I still haven't looked at mrtg code that causes the problem reported
in #118877 but the following thread in perl5-porters mailing list may
also be related:

Re: [perl #34252] Access rights in FindBin::Bin
http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2005-05/msg00042.html

Jose do you recommend we just remove that patch?

Warren,

I will try to give a closer look in the next 48 hours.

Jose, that problem you linked to looks *exactly* like the problem mrtg was
having; we were running it in a SELinux domain with no access to /root (but as
uid 0), and FindBin died on this.

Can we try adding the patch from that list instead of mine?

Created attachment 114111 [details]
FindBin.pm patch (from the perl5-porters mailing list)

Colin,

I have been having problems duplicating the problem:

1) I am unable to successfully boot a FC4test2 (+rawhide) system with
   the following selinux options (/etc/sysconfig/selinux):

      SELINUX=enforcing
      SELINUXTYPE=strict

It boots if I change
   SELINUX=enforcing => SELINUX=permissive

2) With SELinux in permissive mode I don't see any SELinux mrtg related
   messages (mrtg configuration?)

   kernel-2.6.11-1.1286_FC4
   selinux-policy-strict-1.23.14-2
   mrtg-2.11.1-3

   TODO: Check the all the SELinux mrtg rules (in the strict policy).

jpo

PS - Will try to duplicate the problem in a FC3 system.

Jose,

For changing to strict policy, did you use system-config-securitylevel?  Or
alternatively just "touch /.autorelabel; reboot".  If that still doesn't work,
please open a bug against selinux-policy-strict with the avc messages.

As for 2, I believe it was the periodic cron job that was failing.  However,
it's entirely possible that the mrtg policy has changed or that the mrtg code
has changed or cron has changed in a way that would fix this bug.

Anyways, you should not feel obligated to keep mrtg/strict policy working before
taking out my broken FindBin.pm patch, since it's not a very high priority bug
given we have less support for strict policy now.  If it's causing a test
failure let's just take it out now, and just work to make sure that the other
Perl patch gets upstreamed eventually.

(In reply to comment #13)
> For changing to strict policy, did you use system-config-securitylevel?  Or
> alternatively just "touch /.autorelabel; reboot".  If that still doesn't work,
> please open a bug against selinux-policy-strict with the avc messages.

Just edited the file /etc/sysconfig/selinux and reboot the machine.
 
> As for 2, I believe it was the periodic cron job that was failing.  However,
> it's entirely possible that the mrtg policy has changed or that the mrtg code
> has changed or cron has changed in a way that would fix this bug.
> 
> Anyways, you should not feel obligated to keep mrtg/strict policy working before
> taking out my broken FindBin.pm patch, since it's not a very high priority bug
> given we have less support for strict policy now.  If it's causing a test
> failure let's just take it out now, and just work to make sure that the other
> Perl patch gets upstreamed eventually.

I did the tests with a pristine FindBin.pm file (without the patch).

With the new patch (comment #11) the FinBin.pm test suite doesn't fail
(FinBin.t).

Regarding the new patch, I believe it is already uptream. I will try to
determine to which branch it was applied to: perl-5.8.{7,8}, perl-5.9.3 (perl
5.10 devel branch) or both.

jpo

The FindBin.pm patch is only available in the development branch (5.10 to be):

Current file:
http://public.activestate.com/gsar/APC/perl-current/lib/FindBin.pm

Last relevant FindBin.pm changes:

[19] 24379 on 2005/05/03 by rgs@bloom

	Error in the latest FindBin patch, noticed by Nicholas


[18] 24375 on 2005/05/03 by rgs@bloom

	Fix for [perl #34252] Access rights in FindBin::Bin
	At least on my platform, Cwd::getcwd doesn't find the current
	directory if it has no access to it. Try harder with Cwd::cwd.

Patches:
http://public.activestate.com/cgi-bin/perlbrowse?patch=24375
http://public.activestate.com/cgi-bin/perlbrowse?patch=24379

Managed to boot FC4test2+rawhide in strict/enforcing mode (relabelling resolved
the problem).  Now I have problems remote accessing the machine:

1) ssh root@fc4testmachine

   [root@localhost ~]# ls -l
   ls: .: Permission denied

2) ssh -X root@fc4testmachine
   Last login: Mon May  9 21:46:11 2005 from ...
   /usr/X11R6/bin/xauth:  timeout in locking authority file /root/.Xauthority
   [root@localhost ~]# ls
   ls: .: Permission denied

But this is another problem. Back to the current one.

Problem:

1) Using the pristine FindBin.pm module, I now have several mrtg mail messages
   exactly as reported in
   https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=118877.

2) Using the new patch (comment #11) doesn't resolve the problem.

   As far as I can tell the problem is located in the abs_path from
   the module Cwd.pm (see also http://search.cpan.org/dist/PathTools/).

   The above function - abs_path - is implemented in XS code and calls
   bsd_realpath (also implemented in XS code).  It appears to be this
   last function that fails under SELinux.

3) Using the current patch solves the problem.

   As the documentation states, the File::Spec->file_name_is_absolute test
   does not consult the local filesystem, is uses only a regular expression
   to see it the argument starts with a '/'.
   (http://search.cpan.org/dist/PathTools/lib/File/Spec/Unix.pm#METHODS)
 

I think the problem should be resolved with one or more SELinux rules
(I'm a newbie in this area).  Maybe forward this problem to Daniel Walsh?

In the meantime we could rediff the current patch to disable the FindBin.t
test.  It would allow to run the test suite for all other tests and be able
to catch other problems.

Created attachment 114188 [details]
Previous patch rediff

Changes from previous one:
1) Updated to perl 5.8.6
2) Also modifies the MANIFEST file (to disable the FindBin.t test)

Created attachment 114189 [details]
perl specfile patch (to apply new findbin-selinux patch)

Changes:

1) apply new findbin-selinux patch

2) build fails if there are more problems in the test suite

   -make test < /dev/null || /bin/true
   +make test

3) also disables the 5.8.2 ABI compat (#154295 comments 6 and 7)


Perl builds without problems in a FC3 system and in a FC4test2+ system.

Handling the FindBin.t test failure: another option
---------------------------------------------------

With the findbin-selinux patch applied to FindBin.pm the
variable $Bin inside the FindBin.t test suite ends with a
slash ('/').  The following patch allows the test 1 to pass.

----------
--- FindBin.t.orig      2003-12-27 14:52:04.000000000 +0000
+++ FindBin.t   2005-05-10 05:49:51.000000000 +0100
@@ -14,7 +14,7 @@
 if ($^O eq 'MacOS') {
     print "not " unless $Bin =~ m,:lib:$,;
 } else {
-    print "not " unless $Bin =~ m,[/.]lib\]?$,;
+    print "not " unless $Bin =~ m,[/.]lib[\]/]?$,;
 }
 print "ok 1\n";

----------

Comment #19 is the actual reason for the failure, meaning we don't need #17?

I would keep the patch from comment #17.  The findbin-selinux patch changes the
behaviour of of a core perl module: the variables exported by FindBin.pm now
have a different different value (with the patch applied both $Bin and $RealBin
are terminated by a slash; without the patch they don't have the ending slash
character).

If the patch breaks one of the FindBin.t tests, it may also break other code...

I will also forward this problem to the perl5-porters mailing list.  I believe
there is also room for improvement in the Cwd.pm module (in the abs_path code).

Committed and building.  I am not sure if selinux policy can solve this in a
different way.  dwalsh?

Not sure I totally understand the problem.  But we do not want to allow a
restricted domain access to /root in order to get the build to work.  So I think
Colin's patch is correct.

In this case shouldn't the MRTG cron job be executed under a different account
(maybe mrtg instead of root) ?

Building the package isn't the issue.  The selinux patch changing perl's
behavior in such a way that breaks programs is.

I am testing a new FindBin.pm patch that partially restores the default 
behaviour of FindBin.  It will be something along the lines:

...
$Bin = abs_path($Bin) unless (!$Bin || (File::Spec->file_name_is_absolute($Bin)
     && ($Bin = File::Spec->canonpath($Bin))));
...
(will do the same to the $RealBin variable)

The File::Spec canonpath function doesn't touch the filesystem and helps
sanitizing the path (removes duplicated slashs and in particular the trailling
one).  

Will do a couple more tests to see if how it goes.

Created attachment 114373 [details]
New FindBin.pm patch: passes the FindBin test suite

Partially restores the default behaviour of FindBin.pm (no longer necessary to
disable the FindBin test suite; it now passes all the tests).

Created attachment 114374 [details]
perl.spec: updates the changelog entry

Warren,

Could you replace the perl-5.8.6-findbin-selinux.patch by the one in comment #28
and update the changelog entry in the perl specfile?

This one no longer breaks the test suite. It manages to clean the trailing slash
that caused the test failure.

There is still at least a known problem as File::Spec->canonpath is not a
abs_path replacement. Examples:

Same behaviour
--------------
abs_path('/usr/bin/')                        -> /usr/bin
File::Spec->canonpath('/usr/bin/')           -> /usr/bin

Different behaviour
-------------------
abs_path('/usr/..//usr/bin///')              -> /usr/bin
File::Spec->canonpath('/usr/..//usr/bin///') -> /usr/../usr/bin

x86_64 build failure in beehive... this might be a resource limit issue in the
build environment?

t/op/fork.................................PROG:
pipe(RDR,WTR) or die $!;
my $pid = fork;
die "fork: $!" if !defined $pid;
if ($pid == 0) {
    my $rand_child = rand;
    close RDR;
    print WTR $rand_child, "\n";
    close WTR;
} else {
    my $rand_parent = rand;
    close WTR;
    chomp(my $rand_child  = <RDR>);
    close RDR;
    print $rand_child ne $rand_parent, "\n";
}
EXPECTED:
1
GOT:

FAILED at test 19

(Adding Ville for a sanity check, please see above conversation and last few CVS
checkins to perl/devel.)

x86_64 build succeeded on a different build server, but s390 failed this time
here.  Maybe failing on tests is too fragile with these build servers...

lib/Net/Ping/t/190_alarm..................# Failed test 6 in
../lib/Net/Ping/t/190_alarm.t at line 52
#  ../lib/Net/Ping/t/190_alarm.t line 52 is: ok $@ =~ /alarm works/ or die $@;
alarm failed at ../lib/Net/Ping/t/190_alarm.t line 45.
FAILED at test 6

i386 test failure... but it succeeded build previous time when s390 failed. 
Should we just add the || /bin/true again?

t/op/fork.................................PROG:
pipe(RDR,WTR) or die $!;
my $pid = fork;
die "fork: $!" if !defined $pid;
if ($pid == 0) {
    my $rand_child = rand;
    close RDR;
    print WTR $rand_child, "\n";
    close WTR;
} else {
    my $rand_parent = rand;
    close WTR;
    chomp(my $rand_child  = <RDR>);
    close RDR;
    print $rand_child ne $rand_parent, "\n";
}
EXPECTED:
1
GOT:

FAILED at test 19

Better add again '|| /bin/true' to the 'make test' line.

The fork problem appears to be a resource limitation problem but the ping test
failure in s390 should be looked into. See
http://search.cpan.org/~nwclark/perl-5.8.6/INSTALL#make_test .

The currently applied patch is unacceptable for $RealBin (but ok for $Bin).
The FindBin documentation says:

  $RealBin     - $Bin with all links resolved

Test case:

  $ mkdir /tmp/foo
  $ ln -s foo /tmp/bar
  $ cat > /tmp/foo/t.pl <<EOF
  print "Bin: \$FindBin::Bin\n";
  print "RealBin: \$FindBin::RealBin\n";
  EOF
  
With the vanilla upstream implementation:

  $ perl -MFindBin /tmp/bar/t.pl
  Bin: /tmp/foo
  RealBin: /tmp/foo

With the current patch applied:

  $ perl -MFindBin /tmp/bar/t.pl
  Bin: /tmp/bar
  RealBin: /tmp/bar

Now, the changed value of $Bin is insignificant, because the documentation does
not say that $Bin has all symlinks resolved (although due to use of abs_path(),
it does have in the upstream implementation, probably due to oversight).

However, the same change in $RealBin directly breaks documented behaviour, and
is thus not really acceptable.

Note that this isn't new breakage in the currently applied findbin patch, it was
already present in the previous one.

Created attachment 114407 [details]
FindBin.pm patch: restores behaviour under normal circunstances

New patch findbin patch:

1) restores FindBin.pm behaviour under normal circunstances
   (only calls File::Spec->canonpath if abs_path fails)

2) doesn't touch the $RealBin variable
   (the $RealBin variable isn't needed by the mrtg script; it only uses $Bin)

   /usr/bin/mrtg
   -------------
   76  ...
   77  use FindBin;
   78  use lib "${FindBin::Bin}";
   79  use lib "${FindBin::Bin}${main::SL}..${main::SL}lib${main::SL}mrtg2";
   80  ...

3) it passes all the FindBin.t tests

4) the mrtg script doesn't abort under the SELinux strict policy

Checked in and added make test || /bin/true
build will take a while because s390(x) is down

Yep, the patch from comment 37 looks less intrusive, but it of course leaves the
clock ticking for an app that uses $RealBin in a similar setup as where mrtg
uses $Bin... sigh.  Upstream should really replace the contents of FindBin.pm with:

  BEGIN {
    die "I don't exist";
  }
  1;

Patch from comment 37 would be suitable for FC3 and RHEL4 too?

Yes. It makes sense to queue it for a future FC3/RHEL4 perl updates (the new
patch is much less intrusive).

Patch (from comment #37) applied upstream as #24753

Perl patch
http://public.activestate.com/cgi-bin/perlbrowse?patch=24753

FindBin.pm in the perl blead branch
http://public.activestate.com/gsar/APC/perl-current/lib/FindBin.pm

jpo

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2005-674.html

Pedro,

Would the new patch in comment #37 be appropriate to be back-ported to Fedora
Core 2, since FC2 has SELINUX in it, and it uses the old
perl-5.8.3-findbin-selinux.patch?

David,

You should have no problems in backporting the patch attached to comment #37 to FC2.

jpo