Bug 5574 - [review] new setuid code broken under perl 5.6.x and 5.8.x
[review] new setuid code broken under perl 5.6.x and 5.8.x
Status: RESOLVED FIXED
Product: Spamassassin
Classification: Unclassified
Component: spamc/spamd
3.2.2
Other other
: P1 blocker
: 3.1.10
Assigned To: SpamAssassin Developer Mailing List
:
: 5578 (view as bug list)
Depends on: 5518
Blocks:
  Show dependency tree
 
Reported: 2007-07-25 11:08 UTC by Justin Mason
Modified: 2007-08-11 11:57 UTC (History)
4 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status
test case patch None Justin Mason [HasCLA]
possible fix, as checked into trunk patch None Justin Mason [HasCLA]
second half of the fix patch None Justin Mason [HasCLA]
fixes, with test case and higher timeout patch None Justin Mason [HasCLA]
redone with 30-second timeout patch None Justin Mason [HasCLA]
quick-and-dirty for installed system patch None Ben Lentz [NoCLA]
Full patch against 3.1 branch patch None Sidney Markowitz [HasCLA]

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Mason 2007-07-25 11:08:49 UTC
Argh.  it appears that we didn't sufficiently test bug 5518 with Perl 5.6.1;
in particular, when used with the "-u" switch, the spamd children fail
to setuid, and immediately abort with:

  spamd: setuid to uid 65534 failed
  [3544] info: spamd: server successfully spawned child process, pid 3548
  [3544] dbg: prefork: child 3548: entering state 0
  [3544] dbg: prefork: new lowest idle kid: none
  [3544] dbg: prefork: child 3548: just exited

the spamd server will immediately fork another child, and another, etc.
until killed.  Obviously it cannot process requests, although it
will appear to be up to scripts that check "ps" or look at the
spamd log file.

I've written a test case for this.
Comment 1 Justin Mason 2007-07-25 11:10:38 UTC
Created attachment 4062 [details]
test case

this is in SVN trunk:

: jm 63...; svn commit -m "add test case for bug 5574, t/root_spamd_u.t"
Sending        MANIFEST
Sending        build/README
Adding	       t/root_spamd_u.t
Transmitting file data ...
Committed revision 559545.
Comment 2 Justin Mason 2007-07-25 11:47:19 UTC
to clarify, some test results:

Solaris 5.10:
: jm 118...; sudo /local/perl586/bin/prove t/root_spamd_u.t
t/root_spamd_u....ok

: jm 121...; sudo /local/perl561/bin/prove t/root_spamd_u.t
t/root_spamd_u....ok 1/11timed out at t/root_spamd_u.t line 36.
[CTRL-C]


linux x86:
: jm 67...; sudo /usr/local/perl561/bin/prove t/root_spamd_u.t
t/root_spamd_u....ok 1/11timed out at t/root_spamd_u.t line 36.
[CTRL-C]

: jm 69...; sudo prove t/root_spamd_u.t   (perl 5.8.7)
t/root_spamd_u....ok

so it's perl 5.6.1, on any platform...
Comment 3 Justin Mason 2007-07-25 12:16:10 UTC
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5518#c13 is the key; we
missed a thing that amavisd does:

  POSIX::setuid($uid) or die "Can't setuid to $uid: $!";
  $> = $uid; $< = $uid;  # just in case

this "just in case" line appears to fix it. checked into trunk, please test:

: jm 103...; svn commit -m "bug 5574: fix new setuid code to work with perl 5.6.1"
Sending        spamd/spamd.raw
Transmitting file data .
Committed revision 559563.
Comment 4 Justin Mason 2007-07-25 12:16:44 UTC
Created attachment 4063 [details]
possible fix, as checked into trunk

here's that checkin, for reference
Comment 5 Justin Mason 2007-07-25 12:18:03 UTC
t/root_spamd_u.t now passes on 5.6.1, 5.8.x on both Linux and Solaris for me.
Comment 6 Sidney Markowitz 2007-07-25 12:31:05 UTC
So 3.2.2 was released broken for Perl 5.6.1?
Comment 7 Justin Mason 2007-07-25 12:42:24 UTC
yep :(
Comment 8 Sidney Markowitz 2007-07-25 14:33:03 UTC
This looks like the perl 5.6.1 bug that is involved:

Bug in 5.6.0:
http://www.nntp.perl.org/group/perl.perl5.porters/2000/09/msg17830.html

That says that the correct woraround is what you are proposing in the patch

"After some experimentation, it seems that the way to drop privileges
on this system is to do the following in Perl: 

	$> = $id;
	$< = $id;
"

Fix in 5.7.0, statement that it was intended to be backported to 5.6.1, but I
guess they never did:

http://www.nntp.perl.org/group/perl.perl5.porters/2000/11/msg20255.html

So after this analysis, I am +1 on the suggested patch for 3.1 and 3.2 branches

Comment 9 Mark Martinec 2007-07-25 15:33:11 UTC
> "After some experimentation, it seems that the way to drop privileges
> on this system is to do the following in Perl:   $> = $id;  $< = $id;
> So after this analysis, I am +1 on the suggested patch for 3.1 and 3.2
> branches

We already know there isn't a single 'right' order of assignments
to $> and $< that fully drops privileges on all systems.
Linux wants $> = $id; $< = $id, BSD wants $< = $id; $> = $id
(probably also Mac OS X). Don't know about Solaris and other
Unix systems.
Comment 10 Sidney Markowitz 2007-07-25 15:56:24 UTC
> We already know there isn't a single 'right' order of assignments
> to $> and $< that fully drops privileges on all systems.

Right, I feel like I'm going around in circles. Of course that's why we ended up
going with the POSIX::setuid code, because somebody already went through this
mess and made sure that it all works correctly on all platforms. Except that
they didn't do that until after perl 5.6.1. So if we want to work correctly on
perl 5.6.1 we have to do the equivalent of backporting the POSIX::setuid from
the newer versions of perl. Sigh.
Comment 11 Duncan Findlay 2007-07-25 22:17:37 UTC
Perl 5.8.0 was released 5 years ago (July 18, 2002). The last two Debian
releases have perl 5.8 (and we all know Debian releases are about as frequent as
ice ages). Perhaps it is about time to start dropping 5.6 support? (I'm not
saying we gratuitously dump our perl 5.6 compatibility, we just umm... stop
fixing perl 5.6 bugs?)
Comment 12 Justin Mason 2007-07-26 02:20:42 UTC
a mail on the users list has something that may be related:

'Am trying to upgrade to the new 3.2.2 and everything looked OK during
make/test/install but during operation, I'm getting these errors:

Jul 25 13:46:53 boxen spamd[25710]: dcc: check failed: util: setuid 0 to 501
failed! at /usr/lib/perl5/site_perl/5.8.5/Mail/SpamAssassin/Util.pm line
1343.
[some normal spamassassin status messages]
Jul 25 13:46:53 boxen spamd[25710]: spamd: accidental fork: 25710 != 21988
at /usr/bin/spamd line 1628. 
Jul 25 13:46:53 boxen spamd[21988]: dcc: check failed: failed to read header 
[some normal spamassassin status messages]
Jul 25 13:47:00 boxen spamc[25709]: failed sanity check, 1654 bytes claimed,
3385 bytes seen

The resulting messages do not have any X-Spam headers.

I reverted back to 3.2.1 and all of these error messages went away.

This is on a CentOS 4.5 machine running sendmail-8.13.1-3.2.el4, perl
v5.8.5, and dcc-1.3.58.'


(In reply to comment #11)
> Perl 5.8.0 was released 5 years ago (July 18, 2002). The last two Debian
> releases have perl 5.8 (and we all know Debian releases are about as frequent as
> ice ages). Perhaps it is about time to start dropping 5.6 support? 

I'd prefer not to do this -- there are still BSD platforms running 5.6.1, iirc,
and it's not that much trouble to support.  (I maintain a perl561 install on my
laptop, and there's a buildbot too.)

Also, there have been reports of setuid-related problems on perl 5.8.1 and 5.8.5
(see above), so it may not be limited to 5.6.1 anyway...

> (I'm not
> saying we gratuitously dump our perl 5.6 compatibility, we just umm... stop
> fixing perl 5.6 bugs?)

then we become perl 5.6.1 incompatible, we can no longer state that we're
compatible with 5.6.1, therefore we're no longer supporting it. ;)
Comment 13 Matt Kettler 2007-07-27 05:54:51 UTC
Sidney asked for me to comment here.

I'm using:
This is perl, v5.8.8 built for x86_64-linux-thread-multi

The failures I'm having are with t/spamd_plugin,
t/spamd_plugin..................        Not found: called1 =  test: called
myTestPlugin, round 1
# Failed test 2 in t/SATest.pm at line 607
t/spamd_plugin..................NOK 2   Not found: called2 =  called
myTestPlugin, round 2
# Failed test 4 in t/SATest.pm at line 607 fail #2
t/spamd_plugin..................NOK 4   Not found: called3 =  called
myTestPlugin, round 3
# Failed test 6 in t/SATest.pm at line 607 fail #3

Doesn't seem to be related to any setuid issues, as far as I can tell. Unless
the myTestPlugin test does a setuid and nothing else does.

Comment 14 Justin Mason 2007-07-27 09:59:55 UTC
actually, I don't think Matt's bug is setuid-related... probably better to
take it elsewhere (another bug).

however I have been able to reproduce the bug from comment 12 with
perl 5.8.x.  I'll check in a test case: t/root_spamd_u_dcc.t

here's what I get when I run it on linux x86 with perl 5.8.7:

: jm 48...; sudo ./root_spamd_u_dcc.t
1..9
# Running under perl version 5.008007 for linux
# Current time local: Fri Jul 27 17:56:13 2007
# Current time GMT:   Fri Jul 27 16:56:13 2007
# Using Test.pm version 1.25
        /usr/bin/perl -T -w ../spamassassin.raw -C log/test_rules_copy 
--siteconfigpath log/localrules.tmp -p log/test_default.cf  -t -D info -r <
data/spam/gtubedcc.eml
ok 1
        /usr/bin/perl SATest.pl -Mredirect -Olog/d.root_spamd_u_dcc/spamd.err.2
-olog/d.root_spamd_u_dcc/spamd.out.2 -- /usr/bin/perl -T -w ../spamd/spamd.raw
-D -x -s stderr -C log/test_rules_copy  --siteconfigpath log/localrules.tmp -p
36735 -A 127.0.0.1 -c -H -s log/d.root_spamd_u_dcc/spamd.err.2.timestamped &
ok 2
        sudo -u nobody ../spamc/spamc -F data/spamc_blank.cf -d 127.0.0.1 -p
36735 < data/spam/gtubedcc.eml
timed out at ./root_spamd_u_dcc.t line 50.

if you look at 'log/d.root_spamd_u_dcc/spamd.err.2.timestamped' you see this:


Fri Jul 27 17:56:37 2007 [3985] dbg: info: entering helper-app run mode
Fri Jul 27 17:56:37 2007 [3985] dbg: dcc: opening pipe: /usr/local/bin/dccproc
-H -x 0 -a 255.255.255.255 < /tmp/.spamassassin39853rohNatmp
Fri Jul 27 17:56:37 2007 [3992] dbg: util: changing real uid from 0 to match
effective uid 65534
Fri Jul 27 17:56:37 2007 [3992] dbg: info: leaving helper-app run mode
Fri Jul 27 17:56:37 2007 [3992] warn: dcc: check failed: util: setuid 0 to 65534
failed! at ../blib/lib/Mail/SpamAssassin/Util.pm line 1343.


Comment 15 Justin Mason 2007-07-27 10:08:57 UTC
this is unfortunately a very big bug :(

The comment-12 variant affects both perl 5.6.x and 5.8.x, when spamd is run as
root, and pyzor or dcc (dccproc) are active. that's a lot of sites.  When it
happens, the spamd children get hung during the dcc check phase of scanning, the
spamcs eventually time out and pass the messages through unscanned.
Comment 16 Justin Mason 2007-07-27 10:21:39 UTC
Created attachment 4066 [details]
second half of the fix

Here's the fix for the comment 12 part of the bug.

(btw, interesting factoid: with this issue, the symptoms are *the other way
around*; perl 5.6.x works ok without this fix, but perl 5.8.x doesn't!)

Please test these two patches (or svn trunk) on as many platforms and perl
versions as you possibly can; I think we have a little bit of "out of the
frying pan into the fire" with our new setuid code, so it needs lots of
testing.  Run both t/root_spamd_u.t and t/root_spamd_u_dcc.t, as root, ensuring
that "t/config" has 'run_dcc_tests=y' and 'run_root_tests=y' beforehand.

my results:

linux x86 5.6.1: both pass
linux x86 5.8.7: both pass
Comment 17 Justin Mason 2007-07-27 10:36:33 UTC
(In reply to comment #16)
> Please test these two patches (or svn trunk) on as many platforms and perl
> versions as you possibly can; I think we have a little bit of "out of the
> frying pan into the fire" with our new setuid code, so it needs lots of
> testing.  Run both t/root_spamd_u.t and t/root_spamd_u_dcc.t, as root, ensuring
> that "t/config" has 'run_dcc_tests=y' and 'run_root_tests=y' beforehand.

in particular, we need perl 5.6.1 and 5.8.x tested on BSD and MacOS X, btw.

Solaris 5.10 is all good as far as I can tell, with both 5.6.1 and 5.8.6.
Comment 18 snowcrash+apache 2007-07-27 11:49:51 UTC
fyi, osx 10.4.10 + perl 5.8.8 + sa 32-branch (r560339) + 2 patches

perl -V | grep -A3 Summary
Summary of my perl5 (revision 5 version 8 subversion 8) configuration:
  Platform:
    osname=darwin, osvers=8.8.0, archname=darwin-thread-multi-2level
    uname='darwin sc 8.10.0 Darwin Kernel Version 8.10.0: Wed May 23 16:50:59
PDT 2007; root:xnu-792.21.3~1/RELEASE_PPC Power Macintosh powerpc '

svn info 3.2 | grep Revision
	Revision: 560339

make test TEST_VERBOSE=1 TEST_FILES="t/root_spamd_u.t"
	All tests successful.
	Files=1, Tests=11, 46 wallclock secs ( 1.68 cusr +  0.52 csys =  2.20 CPU)

make test TEST_VERBOSE=1 TEST_FILES="t/root_spamd_u_dcc.t"
	All tests successful.
	Files=1, Tests=23, 107 wallclock secs ( 7.05 cusr +  1.63 csys =  8.68 CPU)
Comment 19 Sidney Markowitz 2007-07-27 14:19:36 UTC
> in particular, we need perl 5.6.1 and 5.8.x tested on BSD and MacOS X

I haven't figured out how to get perl 5.6.1 on MacOS X. I could try to build
perl from sources but if it is that difficult, I think it means that we don't
have to worry about anyone running that version of perl on MacOS X.

If someone does know of any prebuilt binary or even a fink or darwinports source
package, let me know and I'll try it. I will draw the line at downloading a
source tarball from perl.org, as I can see no reason for someone to go that far
to put together a SpamAssassin installation using an old perl on their MacOS X
system.
Comment 20 snowcrash+apache 2007-07-27 15:02:08 UTC
fyi, perl 561 @ fink -
http://pdb.finkproject.org/pdb/package.php/perl561
Comment 21 Sidney Markowitz 2007-07-27 15:16:32 UTC
Yup, the fink package list shows no perl 5.6.1 except for MacOS 10.2.

The current version of MacOS is 10.4, some people still run 10.3, but I see no
reason to try to support SpamAssassin under MacOS 10.2.
Comment 22 Sidney Markowitz 2007-07-27 16:09:42 UTC
Not too good results with svn trunk under mac OS 10.4.10, perl 5.8.6

All tests passed as non root. Running as root,

Failed Test                    Stat Wstat Total Fail  List of Failed
-------------------------------------------------------------------------------
t/root_spamd.t                               14   13  2-14
t/root_spamd_tell.t                           6    5  2-6
t/root_spamd_tell_paranoid.t                  6    5  2-6
t/root_spamd_tell_x.t                         6    5  2-6
t/root_spamd_tell_x_paranoid.t                6    5  2-6
t/root_spamd_u.t                             11   10  2-11
t/root_spamd_u_dcc.t                         23   21  3-23
t/root_spamd_virtual.t                       14   13  2-14
t/root_spamd_x.t                             14   13  2-14
t/root_spamd_x_paranoid.t                    14   13  2-14
t/root_spamd_x_u.t                           14   13  2-14
t/spamc_optC.t                                9    4  2 4 6 8
t/spamc_optL.t                               16   16  1-16
t/spamd_allow_user_rules.t                    5    1  4
Comment 23 snowcrash+apache 2007-07-27 16:59:12 UTC
is that with osx's stock perl 586, or your own build?
Comment 24 Sidney Markowitz 2007-07-27 17:03:32 UTC
Stock perl 5.8.6

I backed out the two fixes here and got the same results. I'll have to track
this down some more. It is possible that I messed up something on my machine.
Comment 25 snowcrash+apache 2007-07-27 17:19:19 UTC
re: your earlier comment about most users not likely to build perl from src ...
note that my perl588 _is_ my own config/build/install from stable tarball @
perl, not a pre-packaged bin, or from fink/darwinports.

may want to try one of those 'more likely' sources.
Comment 26 Sidney Markowitz 2007-07-27 18:04:54 UTC
> my perl588 _is_ my own config/build/install from stable tarball @ perl

Doing that I can understand, to get the very latest stable version of perl. But
I don't understand going through that to get a very old version of perl unless
you have some reason to test that specific version. The point is whether there
is a reason to test whether SpamAssassin works running as root under perl 5.6.1
under MacOS X. If not testing that configuration results in a problem for
someone who takes their MacOS X 10.3 or 10.4 and downloads the tarball for perl
5.6.1 and builds from source and then runs SpamAssassin under that old perl as
root (which is not required for someone running it as a personal machine as
opposed to a server), then I'm comfortable with not supporting running under
perl 5.6.1 on the Mac.

Update: Testing as root unpatched svn 3.2 branch all tests passed
Comment 27 snowcrash+apache 2007-07-27 18:17:42 UTC
i was _just_ talking about the possibility of testing other sources of v588
perl; namely those more likely to get installed than a
built-from-src-with-your-own-dependency-mgmt setup.

personally, i'm not convinced there's any sense in expending resources
supporting anything earlier than the perl version shipped in fully-updated osx
(perl v586 on 10.4.14, iirc).

different strokes.
Comment 28 Sidney Markowitz 2007-07-27 18:42:23 UTC
> personally, i'm not convinced there's any sense in expending resources
> supporting anything earlier than the perl version shipped in fully-updated osx

Then we agree :-) That's what I was trying to say. Although I don't mind testing
under MacOS 10.3 and the perl 5.8.x that ships with it because it is easy to
find a machine to test on and it's easy to run a test.

Update: MacOS 10.4.10 perl 5.8.6, testing 3.2 branch with the two patches from
here applied, sudo make test all tests passed.
Comment 29 snowcrash+apache 2007-07-27 20:52:49 UTC
> Then we agree :-)

cool :-)

> I don't mind testing under MacOS 10.3 and the perl 5.8.x that ships with it

Testing is one thing.  Supporting is another.

Not clear to me what the 'project policy' is (tho', i'm sure it's somewhere in
the list/wiki/docs/whatever/etc that we're all supposed to exhaustively search
each/every time ...) re: which perl versions _are_.

My limited experience around here has shown that there's an 'attitude' (not
necessarily a good one) about non-OS perl instances, but, at the same time, a
historical willingness to support versions from the 'dark ages', as lone as
they're "os releases".

"Poppycock" is a nice int'l version of my sentiment abt that.

I think supporting too many dependency versions on any given os is a waste of
time/resources.  But, to avoid it, there needs to be clear policy.

Yeah, yeah.  Blah blah. ;-) Just my $0.02

What I have works well, regardless of 'supported' or not. :-)  if my results
help, then gr8!

cheers.
Comment 30 Justin Mason 2007-07-28 04:21:23 UTC
(In reply to comment #29)
> My limited experience around here has shown that there's an 'attitude' (not
> necessarily a good one) about non-OS perl instances, but, at the same time, a
> historical willingness to support versions from the 'dark ages', as lone as
> they're "os releases".

uh, you're imagining that.  We support perl from 5.6.1 onwards, on as many
platforms as we can.  Sadly, the core devs don't have access to many operating
systems, so that support sometimes runs into OS-specific bugs (such as this crap
with setuid).  But it's not due to any "attitude", believe me.
Comment 31 Justin Mason 2007-07-30 05:10:33 UTC
anyone able to test the patch on *BSD?
Comment 32 Nico Prenzel 2007-07-30 08:46:20 UTC
*** Bug 5578 has been marked as a duplicate of this bug. ***
Comment 33 Nico Prenzel 2007-07-30 08:50:28 UTC
see my comment #2 in http://issues.apache.org/SpamAssassin/
show_bug.cgi?id=5578#c2

applied both patches on debian 3.1 with perl version
perl -v

This is perl, v5.8.4 built for i386-linux-thread-multi
Comment 34 Nico Prenzel 2007-07-30 08:52:59 UTC
uupps,

sorry for my terrible post:

I meant this comment at:
http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5578#c2

fix works for me!
Comment 35 Sidney Markowitz 2007-07-30 14:11:47 UTC
This is very strange. I used svn -r {date} update in trunk to roll back all the
way to the first checkin of the root tests and the failure is still there. I
don't remember if I ever tested sudo make test in trunk or only in the 3.2
branch when the issue first came up. I'm trying to identify the difference
between trunk and 3.2 branch that makes a difference here.
Comment 36 Sidney Markowitz 2007-07-30 21:16:05 UTC
svn co trunk into a fresh directory passes sudo make test with no problem.

Whatever I messed up in my trunk directory that survived my attempt to get the
directory in sync without blowing away the whole thing, I don't think it is a
SpamAssassin bug.

Now I'll see if I can find my FreeBSD Parallels image and free up some disk
space to put it on to test this with.
Comment 37 Sidney Markowitz 2007-07-31 18:07:33 UTC
Is t/root_spamd_u_dcc.t supposed to be a test case for this bug? It isn't in any
of the attachments here.

On my high-latency satellite connection I fail root_spamd_u_dcc.t unless the
timeout in the last loop of tests is increased from 10 seconds. Setting it to 30
seconds works fine for me. The test only takes as long as is necessary when
there is not a timeout, so increasing the limit will not slow down make test on
systems that don't have a problem.

Running under FreeBSD 6.2 with perl 5.8.8, using svn trunk and branches/3.2 all
tests passed both with and without the patches. So the patches don't break
anything, but I did not reproduce the problem that they are supposed to fix. I
did have DCC installed and DCC tests enabled, but not Pyzor.

To reproduce the problem is DCC supposed to be installed to run under some user
other than root? I just did a default install, which runs DCC as root.

If everyone who has seen this problem can say that the patch fixes it for them
I'll vote +1 for it, with t/root_spamd_u_dcc.t added in and with it's timeout
bumped up from 10 seconds to 30.

Comment 38 Justin Mason 2007-08-01 02:05:45 UTC
(In reply to comment #37)
> Is t/root_spamd_u_dcc.t supposed to be a test case for this bug? It isn't in any
> of the attachments here.

Yes -- I didn't include the test cases in the backported code; I didn't think it
would be a requirement to fix the bug and close the test.  simply copying the 2
test scripts from trunk's "t" directory will work fine.

> On my high-latency satellite connection I fail root_spamd_u_dcc.t unless the
> timeout in the last loop of tests is increased from 10 seconds. Setting it to 30
> seconds works fine for me. The test only takes as long as is necessary when
> there is not a timeout, so increasing the limit will not slow down make test on
> systems that don't have a problem.

so, adding "dcc_timeout 30" ?  ok, I'll add that to the test case.

Note that it's irrelevant if DCC fires or not.  The bug happens as soon as the
DCC helper process is forked -- before the dccproc process is exec()'d.  so if
the code gets to exec dccproc, and collects *any* result from that process, even
a timeout, the bug is fixed.

> Running under FreeBSD 6.2 with perl 5.8.8, using svn trunk and branches/3.2 all
> tests passed both with and without the patches. So the patches don't break
> anything, but I did not reproduce the problem that they are supposed to fix. I
> did have DCC installed and DCC tests enabled, but not Pyzor.
> 
> To reproduce the problem is DCC supposed to be installed to run under some user
> other than root? I just did a default install, which runs DCC as root.

You need to have dcc installed (as root is fine).  Pyzor is not required.  You
also need to edit or create t/config and change "run_dcc_tests" to "y".

> If everyone who has seen this problem can say that the patch fixes it for them
> I'll vote +1 for it, with t/root_spamd_u_dcc.t added in and with it's timeout
> bumped up from 10 seconds to 30.

ok, I'll redo the patches with the test scripts included, and increased timeout.
Comment 39 Justin Mason 2007-08-01 02:06:42 UTC
Created attachment 4075 [details]
fixes, with test case and higher timeout

this supercedes all previous patches.
Comment 40 Justin Mason 2007-08-01 02:07:56 UTC
(In reply to comment #39)
> Created an attachment (id=4075) [edit]
> fixes, with test case and higher timeout
> 
> this supercedes all previous patches.

uh, I should be clearer -- the code changes (patch 4063 and 4066) are unchanged
in this patch, it's just the stuff Sidney requested: addition of the test
scripts, and a MANIFEST change to include them, and that higher timeout.
Comment 41 Sidney Markowitz 2007-08-01 03:22:30 UTC
> adding "dcc_timeout 30" ?  ok, I'll add that to the test case.

Oh, that isn't what I meant, but that might explain why I often get a failure in
t/dcc.t and then it works when I run it again.

I was talking about the alarm 10 in

 for my $try (1 .. 5) {
  $SIG{ALRM} = sub { stop_spamd(); die "timed out"; };
  alarm 10;

If the test is supposed to pass if that alarm times out, that doesn't seem to
work. A time out there appears to result in the test failing. 

I'll try again with the new patch and see if the increase in DCC timeout is good
enough.
Comment 42 Justin Mason 2007-08-01 03:30:10 UTC
(In reply to comment #41)
> I was talking about the alarm 10 in
> 
>  for my $try (1 .. 5) {
>   $SIG{ALRM} = sub { stop_spamd(); die "timed out"; };
>   alarm 10;
> 
> If the test is supposed to pass if that alarm times out, that doesn't seem to
> work. A time out there appears to result in the test failing. 

oh, right, I understand.  yes, I think that would need to be increased *as well
as* the dcc_timeout to deal with that satellite connection of yours :(
Comment 43 Sidney Markowitz 2007-08-01 05:28:34 UTC
The new patch with the alarm 10 changed to alarm 30 works fine on my FreeBSD
6.2, perl 5.8.8 test machine.

I also found that adding dcc_timeout 30 to t/dcc.t allowed that test to work
consistently, which it never had before.

However, when I backed out the changes to Util.pm and spamd.raw it still worked. 
Comment 44 Justin Mason 2007-08-02 09:28:00 UTC
Created attachment 4077 [details]
redone with 30-second timeout

ok, here's 4075 with that 30-second timeout, as suggested by Sidney; code is
otherwise unchanged
Comment 45 Warren Togami 2007-08-02 12:39:42 UTC
To clarify, the patch in Comment #44 is exactly what is needed?

How far away is 3.2.3?  Should I bother patching our 3.2.2 for our users?
Comment 46 Justin Mason 2007-08-02 12:50:20 UTC
(In reply to comment #45)
> To clarify, the patch in Comment #44 is exactly what is needed?

yep!

> How far away is 3.2.3?  Should I bother patching our 3.2.2 for our users?

I would say yes.  we haven't even got around to discussing a release schedule
for 3.2.3 yet, and in the meantime, SA is unusable if pyzor and dcc are enabled.
Comment 47 Troy Brown 2007-08-02 17:33:17 UTC
Can you please let me know the actual patch command to use for this and if I 
need to run it in a particular directory.

Thanks!
Comment 48 snowcrash+apache 2007-08-02 17:38:01 UTC
(In reply to comment #47)
> Can you please let me know the actual patch command to use for this and if I 
> need to run it in a particular directory.

http://mail-archives.apache.org/mod_mbox/spamassassin-users/200708.mbox/browser
Comment 49 snowcrash+apache 2007-08-02 17:43:31 UTC
(In reply to comment #47)
> Can you please let me know the actual patch command to use for this and if I 
> need to run it in a particular directory.

ok, so that didn't work ...
http://article.gmane.org/gmane.mail.spam.spamassassin.general/101910
Comment 50 Justin Mason 2007-08-03 08:36:14 UTC
committers: more votes are needed.  this is a critical bug, rendering spamd
unusable for lots of sites. we need to fix it in SVN and release a 3.2.3 ASAP...
Comment 51 Ben Lentz 2007-08-03 09:38:11 UTC
Created attachment 4079 [details]
quick-and-dirty for installed system
Comment 52 Ben Lentz 2007-08-03 09:38:33 UTC
Weird, DCC worked fine for me after upgrading from 3.2.1 to 3.2.3 (CPAN/root
installation test failure prevented 3.2.2) and all tests passed (Perl 5.8.5)...
Pyzor was the only thing broken on my system after upgrading. Applying these
changes to spamd and Mail/SpamAssassin/Util.pm fixed Pyzor right up.
Comment 53 Justin Mason 2007-08-03 09:46:42 UTC
(In reply to comment #52)
> Weird, DCC worked fine for me after upgrading from 3.2.1 to 3.2.3

um, 3.2.3 hasn't been released yet ;)
Comment 54 Ben Lentz 2007-08-03 09:56:41 UTC
Right... Subtract 0.0.1 from everything I just said. D'Oh!

Weird, DCC worked fine for me after upgrading from 3.2.0 to 3.2.2 (CPAN/root
installation test failure prevented 3.2.1) and all tests passed (Perl 5.8.5)...
Pyzor was the only thing broken on my system after upgrading. Applying these
changes to spamd and Mail/SpamAssassin/Util.pm fixed Pyzor right up.
Comment 55 Justin Mason 2007-08-03 10:07:36 UTC
you could be using dccifd, which doesn't require forking a helper process; that
wouldn't display this bug, then.
Comment 56 Ben Lentz 2007-08-03 10:13:25 UTC
Bingo... I am. Good catch!
Comment 57 Stein Magne 2007-08-06 01:59:14 UTC
(In reply to comment #50)
> committers: more votes are needed.  this is a critical bug, rendering spamd
> unusable for lots of sites. we need to fix it in SVN and release a 3.2.3 ASAP...

Are you done testing and ready to release a official fix for this or should we
try using the quick and dirty fix?
Comment 58 Sidney Markowitz 2007-08-06 13:24:27 UTC
> Are you done testing and ready to release a official fix for this
> or should we try using the quick and dirty fix?

Nobody has objected to this fix, but we need one more vote from a committer
before we can check it in and release 3.2.3.

Come on, vote anyone?

The "quick and dirty" patch that Ben attached here is the same as the official
fix but without the additional test cases. If you apply the "quick and dirty"
patch you will have the same fix that we are voting on, you just won't also have
the new tests in "make test" that would have caught this bug. The quick and
dirty patch has the advantage that you can apply it with a small one line edit
to each of two files if that is easier for you than using "patch" to apply a
patch file. Otherwise you might as well apply attachment 4077 [details].

Either way, if you try it please let us know if you find any problems.

Comment 59 Doc Schneider 2007-08-06 17:48:06 UTC
+1 built on CentOS 5 with perl 5.8.8 non-root:
All tests successful, 34 tests skipped.
Files=142, Tests=1980, 4277 wallclock secs (459.19 cusr + 31.14 csys = 490.33 CPU)
as root (run_root_tests=y):
All tests successful, 27 tests skipped.
Files=142, Tests=2054, 1016 wallclock secs (461.21 cusr + 32.33 csys = 493.54 CPU)
Comment 60 Justin Mason 2007-08-07 08:28:23 UTC
ok, applied patch 4077 to 3.2.x.

: jm 355...; svn commit -m "bug 5574: fix new setuid code to work with perl
5.6.1, and to support DCC and Pyzor in all releases of perl"
Sending        MANIFEST
Sending        lib/Mail/SpamAssassin/Util.pm
Sending        spamd/spamd.raw
Adding         t/root_spamd_u.t
Adding         t/root_spamd_u_dcc.t
Transmitting file data .....
Committed revision 563523.
Comment 61 Sidney Markowitz 2007-08-07 12:00:06 UTC
We need this in for 3.1 too, right?

If we do, here's my +1 for committing to the 3.1 branch.

By the way, how did this get closed with no indication that it has a dependency
on open bug 5518? Bugzilla doesn't flag something like that?

Comment 62 Justin Mason 2007-08-07 13:49:43 UTC
(In reply to comment #61)
> We need this in for 3.1 too, right?
> 
> If we do, here's my +1 for committing to the 3.1 branch.

oh yeah.  +1 here too (assuming that patch fits)

> By the way, how did this get closed with no indication that it has a dependency
> on open bug 5518? Bugzilla doesn't flag something like that?

yeah, odd.  It didn't even warn me about that!

Comment 63 Sidney Markowitz 2007-08-09 18:00:34 UTC
It was pointed out to me that we can't vote without having a patch against 3.1.
I'm testing it now before uploading it.
Comment 64 Sidney Markowitz 2007-08-09 18:07:40 UTC
I didn't mean to click the assign radio button when I submitted that last
comment, reassigning back to the dev list, not that it makes much difference.
Comment 65 Sidney Markowitz 2007-08-09 18:35:18 UTC
Created attachment 4080 [details]
Full patch against 3.1 branch

Please vote on this patch for 3.1 branch
Comment 66 Doc Schneider 2007-08-10 06:26:45 UTC
+1 
Comment 67 Tom Schulz 2007-08-10 06:33:03 UTC
I am not sure how important this is, but the 3.1 branch (at least 3.1.6)
does not have any of the root* series of tests and therefore does not have
the option in config.dist to enable/disable these tests. You might want to
put the following in config.dist (lifted from the 3.2 config.dist)

# ---------------------------------------------------------------------

# The "root_*.t" tests require root privileges, and may create files in
# the filesystem as part of the test.  Disabled by default.
run_root_tests=n
Comment 68 Sidney Markowitz 2007-08-10 06:52:33 UTC
run_root_tests was backported to the 3.1 branch for version 3.1.9
Comment 69 Matt Kettler 2007-08-10 18:10:17 UTC
+1 for inclusion of attachment 4080 [details] in 3.1.10

after grabbing a fresh copy of the 3.1 SVN branch and applying this patch, even
as root make test goes nicely.

------------
All tests successful, 24 tests skipped.
------------

Beautiful!
Comment 70 Sidney Markowitz 2007-08-10 22:35:32 UTC
Committed to 3.1 branch revision 564834.
Comment 71 Warren Togami 2007-08-11 10:46:01 UTC
So 3.1.9 was affected by this as well?
Comment 72 Sidney Markowitz 2007-08-11 11:57:05 UTC
> So 3.1.9 was affected by this as well?

This bug is about a problem introduced by the fix for bug 5518 which was checked
into the 3.1 branch after 3.1.9 was released. Bug 5518 was a problem only on
some platforms, I think MacOS and BSD, but not Linux. The problem with the fix
for bug 5518 affected certain other combinations of platforms and perl versions,
and I think would affect some Linux systems.

So, 3.1.9 has bug 5518 in it, which is not a problem on Linux, and does not have
this bug in it. 3.2.2 has the fix for bug 5518 in it, which introduced this bug,
which can be a problem on Linux.