Your patch nullifies the comment "Load only when required", "use" will always load the module even if not needed. If that is need to be used, then it should be moved to the file header.

-1 

Should also fix (remove) preceding comment, which no longer makes sense with ''require' switched to 'use'

Also, what is the reasoning for this change? Some startup vs runtime @INC is different? Should not that be fixed instead?

To expand on Henrik's comment in case it isn't clear to you, a require statement should never cause a permission denied error. What require does is attempt to load a module that it looks for in the directory path specified in @INC. What use does is load a module at compile time. One difference is that when the compiler sees the use statement it will load the module. As the comment before the require says, this code has require instead of use so that the module will only be loaded if the code is actually executed. Changing it to use will cause it to be always loaded when spamd first starts up.

This error indicates that @INC is not set correctly when the require statement is run, even if it is correct at compile time. Is it possible that you are calling spamd with perl called using -I options to get the library include path right? If so, that may be what bug 8030 addresses and you should see if the patch in there helps. You can also see if 4.0.0rc1 helps. I should be releasing that today. I don't off the top of my head see how a use instead of a require there would help with bug 8030, but it is in a related area, so maybe.

In any case, changing the require to use cannot be the correct thing to do even if it suppresses the error, there has to be a different way to fix it.

The proposed fix was just a way to better explain what's going on.
spamd(8) starts without issues, only regression tests are failing.
I think it may be one of recent changes to SATest.pm.
I will investigate more.

(In reply to Giovanni Bechis from comment #5)
> The proposed fix was just a way to better explain what's going on.
> spamd(8) starts without issues, only regression tests are failing.
> I think it may be one of recent changes to SATest.pm.
> I will investigate more.

Oh, sorry, my brain must have been fried when I read the bug report and I thought it was from some random person running some older build and who didn't know the difference between require and use, and I didn't notice your name.

Ok, confirm if this is before or after the commit for bug 8030 because it seems like something related. If it happens even with the commit for bug 8030 it might be a case that I missed. Please post the output, usi9g the same perl that you run the tests with, of

perl -e 'print "\@INC:\n".join("\n", @INC)."\nPerl env:\n"; map {print "$_ = $ENV{$_}\n" if (/^PERL/);} keys(%ENV)'

to show the default @INC and relevant environment variables that the patch is supposed to be working with.

One thing to look for in this in the debug logs is if it only happens after a SIGHUP restart, and secondly, if it only happens when spamd is starting up a copy of itself some other way. Those are the most likely places my changes could fail.

Created attachment 5802 [details]
SATest workaround

This workaround fixes the issue, I do not know if it break something on other systems and it's better to have it as local patch.

(In reply to Giovanni Bechis from comment #7)
> Created attachment 5802 [details]
> SATest workaround
> 
> This workaround fixes the issue, I do not know if it break something on
> other systems and it's better to have it as local patch.

(In reply to Sidney Markowitz from comment #6)
> (In reply to Giovanni Bechis from comment #5)
> > The proposed fix was just a way to better explain what's going on.
> > spamd(8) starts without issues, only regression tests are failing.
> > I think it may be one of recent changes to SATest.pm.
> > I will investigate more.
> 
> Oh, sorry, my brain must have been fried when I read the bug report and I
> thought it was from some random person running some older build and who
> didn't know the difference between require and use, and I didn't notice your
> name.
> 
> Ok, confirm if this is before or after the commit for bug 8030 because it
> seems like something related. If it happens even with the commit for bug
> 8030 it might be a case that I missed. Please post the output, usi9g the
> same perl that you run the tests with, of
> 
> perl -e 'print "\@INC:\n".join("\n", @INC)."\nPerl env:\n"; map {print "$_ =
> $ENV{$_}\n" if (/^PERL/);} keys(%ENV)'
> 
@INC:
/usr/local/cpanel
/usr/local/cpanel/3rdparty/perl/532/lib/perl5/cpanel_lib/x86_64-linux-64int
/usr/local/cpanel/3rdparty/perl/532/lib/perl5/cpanel_lib
/usr/local/cpanel/3rdparty/perl/532/lib/perl5/532/x86_64-linux-64int
/usr/local/cpanel/3rdparty/perl/532/lib/perl5/532
/opt/cpanel/perl5/532/site_lib/x86_64-linux-64int
/opt/cpanel/perl5/532/site_lib
Perl env

> to show the default @INC and relevant environment variables that the patch
> is supposed to be working with.
> 
> One thing to look for in this in the debug logs is if it only happens after
> a SIGHUP restart, and secondly, if it only happens when spamd is starting up
> a copy of itself some other way. Those are the most likely places my changes
> could fail.

reverting your latest change doesn't help.
Test output is:

+ /usr/bin/make test TEST_FILES=t/sa_check_spamd.t
"/usr/local/cpanel/3rdparty/perl/532/bin/perl" build/mkrules --exit_on_no_src --src rulesrc --out rules --manifest MANIFEST --manifestskip MANIFEST.SKIP
no source directory found: exiting
"/usr/local/cpanel/3rdparty/perl/532/bin/perl" build/preprocessor  -Mvars -DVERSION="4.000000" -DPREFIX="/usr/local/cpanel/3rdparty/perl/532/lib/perl5" -DDEF_RULES_DIR="/usr/share/spamassassin" -DLOCAL_RULES_DIR="/etc/mail/spamassassin" -DLOCAL_STATE_DIR="/var/lib/spamassassin" -DINSTALLSITELIB="/usr/local/cpanel/3rdparty/perl/532/lib/perl5/cpanel_lib" -DCONTACT_ADDRESS="root\@localhost" -DRE2C_BIN="/usr/local/cpanel/3rdparty/bin/re2c" -Msharpbang -Mconditional -DPERL_BIN=""/usr/local/cpanel/3rdparty/perl/532/bin/perl"" -DPERL_WARN="" -DPERL_TAINT="" -m755 -isa-update.raw -osa-update
cp sa-update blib/script/sa-update
"/usr/local/cpanel/3rdparty/perl/532/bin/perl" -MExtUtils::MY -e 'MY->fixin(shift)' -- blib/script/sa-update
PERL_DL_NONLAZY=1 "/usr/local/cpanel/3rdparty/perl/532/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/sa_check_spamd.t
t/sa_check_spamd.t .. 1/7       Not found: flag =  X-Spam-Flag: YES at t/sa_check_spamd.t line 20.

#   Failed test at t/SATest.pm line 926.
        Not found: status =  X-Spam-Status: Yes, score= at t/sa_check_spamd.t line 20.

#   Failed test at t/SATest.pm line 926.
Output can be examined in: log/sa_check_spamd.WH4bSS/d.sa_check_spamd/out.1
t/sa_check_spamd.t .. 6/7 # Looks like you failed 2 tests of 7.
t/sa_check_spamd.t .. Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/7 subtests 

Test Summary Report
-------------------
t/sa_check_spamd.t (Wstat: 512 Tests: 7 Failed: 2)
  Failed tests:  3-4
  Non-zero exit status: 2
Files=1, Tests=7, 10 wallclock secs ( 0.04 usr  0.01 sys +  1.02 cusr  0.20 csys =  1.27 CPU)
Result: FAIL
Failed 1/1 test programs. 2/7 subtests failed.

Which perl did you use to run perl Makefile.PL?

With your patch not installed, so when the test breaks, add this line to t/SATest.pl before the line that begins my $mode so it looks like this

warn "***DEBUG will call @args\n****\n";
my $mode = $opts{'M'};

The run the test and show what the debug output was

Giovanni, I just installed cpanel on a CentOS 7.9 VM to see if I could reproduce the problem. I checked out svn trunk and I built and tested it using

/usr/local/cpanel/3rdparty/bin/perl Makefile.PL < /dev/null
make
make test TEST_FILES="t/sa_check_spamd.t"

It worked. Is there anything that you see is different from your test setup?

I think I figured it out. It's the code in SATest.pm that has this comment:

  # If PERL5LIB is empty copy @INC instead because on some platforms like FreeBSD MakeMaker clears PER5LIB and sets @INC
  # Filter out relative paths, and canonicalize so no symlinks or /../ will be left in untainted result as a nod to security

I bet that somehow you can access directories as relative pathnames but not when they are canonicalized absolute pathnames.

I see in the error message in the first comment that you were building in /root/rpmbuild/BUILD/Mail-SpamAssassin-4.0.0

On the VM I installed Cpanel on, it ended up with /root having dr-xr-x--- permissions. How were you able to cd into the directory with those permissions?

It looks like I should not make the paths canonicalized and absolute there, just leave them relative. The less change made to them the less chance of unforseen problems, anyway.

(In reply to Sidney Markowitz from comment #11)
> I think I figured it out. It's the code in SATest.pm that has this comment:
> 
>   # If PERL5LIB is empty copy @INC instead because on some platforms like
> FreeBSD MakeMaker clears PER5LIB and sets @INC
>   # Filter out relative paths, and canonicalize so no symlinks or /../ will
> be left in untainted result as a nod to security
> 
> I bet that somehow you can access directories as relative pathnames but not
> when they are canonicalized absolute pathnames.
> 
> I see in the error message in the first comment that you were building in
> /root/rpmbuild/BUILD/Mail-SpamAssassin-4.0.0
> 
> On the VM I installed Cpanel on, it ended up with /root having dr-xr-x---
> permissions. How were you able to cd into the directory with those
> permissions?
> 
> It looks like I should not make the paths canonicalized and absolute there,
> just leave them relative. The less change made to them the less chance of
> unforseen problems, anyway.

As an unprivileged user, regression test works, as root fails.
I think the issue is that when spamd(8) starts as root and changes its user to "nobody", it cannot access the directory it started in and fails.

(In reply to Giovanni Bechis from comment #12)
[...]
> As an unprivileged user, regression test works, as root fails.

This is true for multiple tests on various platforms. E.g. on macOS if SA has been used previously a bunch of spamd/spamc tests fail because there's some sort of escape from the test environment that grabs the system-wide welcomelist. Historically I've run into similar problems on FreeBSD. I believe there's still an open bug that I opened before I was a contributor noting that 'make test' with an '/opt/local' prefix polluted the base OS (/etc). 

I don't think we should encourage anyone to build/test SA as root or support doing so. In fact, I think we should actively discourage it, not because of anything special in SA but because it's a spectacularly insecure behavior if you are working with untrusted code that you have not audited. 

Yes, I know that's an extreme position which virtually no one actually adheres to all of the time. Obviously it is a mistake I've made. I strive imperfectly toward virtue in a world of sin. 

I vote for WONTFIX.

Tests may not work if run as root, running code as root is discouraged.

(In reply to Giovanni Bechis from comment #12)
> As an unprivileged user, regression test works, as root fails.
> I think the issue is that when spamd(8) starts as root and changes its user
> to "nobody", it cannot access the directory it started in and fails.

Oh, that's different, and makes me less inclined to fix it. Although I'll still look at it some more. This could be tricky. Code at the comment
 
# Fix INC to point to absolute path of built SA

also ensures that there is an absolute path in @INC and that is there for a reason. Maybe something can be done conditional on running as root, and it would end up only breaking tests run on root on machines that rely on PERL5LIB for local::lib installation of perl libraries. I would gladly settle for that one as a known limitation if that's how it ends up.

Giovanni, I still can't reproduce the bug. What permissions and ownership do you have for /root and for /root/rpmbuild/BUILD/Mail-SpamAssassin-4.0.0/ and the tree beneath it? Do you go into a root shell, cd to /root/rpmbuild/BUILD/Mail-SpamAssassin-4.0.0/ and run the make test? When I tried that the test didn't fail to run as root. When I had the Mail-SpamAssassin-4.0.0 directory tree owned by a user, I could not run any tests as user because looking up modules in @INC failed even sooner when it could not search into anything under /root which is dr-xr-x---

Is there some configuration that makes spamd launch as root and run as nobody during the tests? If I just build from trunk and run as root is it not doing that?

(In reply to Giovanni Bechis from comment #14)
> Tests may not work if run as root, running code as root is discouraged.

Oh, I missed that you actually set this to WONTFIX. That's much easier :) I'll stop now.

I'm not sure I agree with the WONTFIX to be honest.  It's a test so it's not a blocker but can we reopen and kick to 4.0.1 target?

I don't mind looking at it, as I'm by now familiar with the issues I dealt with that are involved with this. What I need though are more specifics so I can reproduce it. I don't have to do it in cpanel now that I know that the real issue is running the test as root in a subdirectory under something like /root that has permissions dr-x--x--x

Giovanni, could you please provide these details about how you got to this point:

What are the permissions of /root directory, is it still the default dr-x--x--x ?

What is the ownership and permissions of the /root/rpmbuild/BUILD/Mail-SpamAssassin-4.0.0 directory tree?

As what user did you run perl Makefile.PL and make?

Which perl did you use to run perl Makefile.PL

Exactly what were the steps you used to run the test? Was it something like

  su -
  cd /root/rpmbuild/BUILD/SpamAssassin-4.0.0
  make test TEST_FILES="t/sa_check_spamd.t"

Thanks

Giovanni, the error message you showed and what you said about running spamd as root and it then doing a setuid to a non-root uid implies to me that you start out as root in /root/rpmbuild/BUILD/Mail-SpamAssassin-4.0.0 which can only be accessed by root, and you run the make test as root. When you do that spamd is started as root, and it later does a setuid nobody.

However I added the following diagnostic line to Util.pm before the require that is failing

dbg("util: DEBUG require hostname now running as: ruid=$< euid=$> rgid=$( egid=$)");

When I use the same directories as above and su to a root shell, cd to the directory and run the make test, I don't get an error. If I edit the sa_check_spamd.t to change an ok pattern to force it to fail so the spamd error log file is not deleted, it shows that the dbg line I added says that it was still running as root when the require is run, and the setuid nobody shows up in the log later.

Are you running the test differently? Could you add that dbg line and run the test and attach the spamd.err.0.timestamped file from the log directory?

(In reply to Sidney Markowitz from comment #20)
> Giovanni, the error message you showed and what you said about running spamd
> as root and it then doing a setuid to a non-root uid implies to me that you
> start out as root in /root/rpmbuild/BUILD/Mail-SpamAssassin-4.0.0 which can
> only be accessed by root, and you run the make test as root. When you do
> that spamd is started as root, and it later does a setuid nobody.
> 
Some relevant info about directories (created by rpmbuild):
# ls -ld /root/rpmbuild/BUILD/Mail-SpamAssassin-4.0.0
drwxr-xr-x. 14 centos centos 4096 Aug 29 07:28 /root/rpmbuild/BUILD/Mail-SpamAssassin-4.0.0

# ls -ld /usr/local/cpanel/
drwx--x--x. 39 root wheel 4096 Aug 29 05:27 /usr/local/cpanel/
# ls -l /usr/local/cpanel/3rdparty/perl/532/lib/perl5/532/x86_64-linux-64int/Sys/Hostname.pm
-r--r--r--. 1 root root 3689 May  3 16:02 /usr/local/cpanel/3rdparty/perl/532/lib/perl5/532/x86_64-linux-64int/Sys/Hostname.pm

> However I added the following diagnostic line to Util.pm before the require
> that is failing
> 
> dbg("util: DEBUG require hostname now running as: ruid=$< euid=$> rgid=$(
> egid=$)");
> 
> When I use the same directories as above and su to a root shell, cd to the
> directory and run the make test, I don't get an error. If I edit the
> sa_check_spamd.t to change an ok pattern to force it to fail so the spamd
> error log file is not deleted, it shows that the dbg line I added says that
> it was still running as root when the require is run, and the setuid nobody
> shows up in the log later.
> 
On my laptop as root I have:
Mon Aug 29 09:46:34 2022 [39733] warn: spamd: still running as root: user not specified with -u, not found, or set to root, falling back to nobody
Mon Aug 29 09:46:34 2022 [39733] dbg: util: get_user_groups: uid is 65534
Mon Aug 29 09:46:34 2022 [39733] dbg: info: user has changed
[...]
Aug 29 09:36:47.224 [36136] dbg: util: DEBUG require hostname now running as: ruid=0 euid=0 rgid=0 0egid=0 0

while on cPanel I have:
Mon Aug 29 07:28:29 2022 [25528] warn: spamd: still running as root: user not specified with -u, not found, or set to root, falling back to nobo
dy
Mon Aug 29 07:28:29 2022 [25528] dbg: util: get_user_groups: uid is 99
Mon Aug 29 07:28:29 2022 [25528] dbg: util: get_user_groups: added 99 (nobody) to group list which is now: 99 99 
Mon Aug 29 07:28:29 2022 [25528] dbg: info: user has changed
[...]
Mon Aug 29 07:17:11 2022 [19184] dbg: util: DEBUG require hostname now running as: ruid=0 euid=99 rgid=0 99egid=99 99

> Are you running the test differently? Could you add that dbg line and run
> the test and attach the spamd.err.0.timestamped file from the log directory?

Created attachment 5804 [details]
sa_check_spamd log file

Very strange. I get the require hostname happening much earlier, before the setuid nobody.

I'll investigate to see if I can find the circumstances in which it would do one or the other.

I have not got as far as getting the exact same error as Giovanni, but I have got to a point where I'm going to close it again as WONTFIX.

I can summarize the problem as this:

When spamd is run as root, it will setuid to switch to either the user in the -u option or to user nobody. If the uid it switches to does not have read access to the modules in the perl lib directories and the SpamAssassin modules, then of course any subsequent require of the modules will fail. Whether or not that is sufficient to cause a specific test to fail depends on the test. But it is still wrong. In my logs, even when the test got a pass, the logs still had permission denied warnings that just didn't happen to cause a die that failed the test. Changing that one require that you see did not really fix anything, just allowed a PASS result.

The bottom line is that it is not valid to run spamd as root and have perl lib or SpamAssassin modules in directories that are not accessible to the user that spamd will run as. That will not work when run in production and so doesn't have to work when running tests.

Put SpamAssassin in a world-accessible directory tree and the test should run even if run as root. But better, only run the root tests as root.

+1 to the wontfix.  Environment issue.