just to clarify -- this issue happens when the tests are run as root.  the test
script should be ensuring that its temp and output dirs are writable by
"nobody", since now part of the spamd tests will run as "nobody" instead of
"root". (in previous versions, there was a security hole whereby they didn't
drop privs when running as root.)

this breaks the "sudo cpan Mail::SpamAssassin" style of installation, so is
pretty important :(

an easy option may be, btw, to skip certain tests if running as root.

Another solution is to chmod the directories to world writable before dropping
privs.

I think we need to fix this and cut a release soon.  If we aim for Wednesday it
leaves the weekend for anyone who feels like playing whack-a-bug.

I think I may have a fix nearly ready...

Created attachment 3989 [details]
fix

I think this may do it... give it a try.

Works on Mandriva Corporate server 4:
t/spamc_optC................ok                                               
t/spamc_optL................ok                                               
t/spamc_y...................ok                                               
t/spamc_z...................ok                                               
t/spamd.....................ok                                               
t/spamd_allow_user_rules....ok           

It will take another 10 minutes or so to finish compiling, but this is where it
always broke before.

I'd give it +1, but I don't count.

This doesn't work for me in MacOS 10.4.9, but the bug may be something in spamd
that I didn't notice before. This error happens when you try as root to run

  spamd --virtual-config-dir=somepath -u nobody

which is what t/spamd_allow_user_rules.t tries to do

[16061] info: spamd: server started on port 48825/tcp (running version
3.2.2-r547235)
[16061] info: spamd: server pid: 16061
[16061] info: spamd: server successfully spawned child process, pid 16065
[16061] dbg: prefork: child 16065: entering state 0
[16061] dbg: prefork: new lowest idle kid: none
[16061] info: spamd: server successfully spawned child process, pid 16066
[16061] dbg: prefork: child 16066: entering state 0
[16061] dbg: prefork: new lowest idle kid: none
[16065] error: setrgid() not implemented at ../spamd/spamd.raw line 1041.
setrgid() not implemented at ../spamd/spamd.raw line 1041.
[16066] error: setrgid() not implemented at ../spamd/spamd.raw line 1041.
setrgid() not implemented at ../spamd/spamd.raw line 1041.
[16061] dbg: prefork: child closed connection

Doing some digging I fond this mentioned as a long-time problem with trying to
run spamd as nobody on MacOS X. Both the uid and gid of 'nobody' on the Mac are
-2 and that seems to confuse things as the spamd sees $ugid as having a value of
4294967294 and then $( = $ugid produces the misleading error message that
setrgid is not implemented.

  spamd --virtual-config-dir=somepath -u www

for example executes the $( = $ugid; with no error.

getpwnam('nobody') returns the 4294967294 values instead of -2. If at that point
I  force the values of $uuid and $ugid to be -2 then everything seems to work
correctly.

I see some code that appears to recognize the possibility of something like this

      if ( $> != $uuid and $> != ( $uuid - 2**32 ) ) {
        die "spamd: setuid to uid $uuid failed\n";
      }

but I don't really know what is going on with the various tests and workarounds
for BSD-specific behavior so I don't know just how to fix this.

More digging: The underlying variables in the C library are unsigned ints. So
getpwnam returns what is there which looks like 4294967294 even though Apple
puts a -2 in /etc/passwd. Assignments to $< and $> are fine with either
4294967294 or -2, doing the same thing in either case. It is only assignments to
$( and $) that flake out if the number is bigger than the maximum positive 32
bit signed int.

However, POSIX::setgid() works fine to set both the real and effective gid with
one call and it accepts 4294967294.

So I suggest that we replace

   $) = "$ugid $ugid";    # effective gid
   $( = $ugid;            # real gid

with
    setgid($ugid);

I'm not expert in this stuff... Does anyone see any problem with using
POSIX::setgid here on other platforms?

(In reply to comment #12)
> More digging: The underlying variables in the C library are unsigned ints. So
> getpwnam returns what is there which looks like 4294967294 even though Apple
> puts a -2 in /etc/passwd. Assignments to $< and $> are fine with either
> 4294967294 or -2, doing the same thing in either case. It is only assignments to
> $( and $) that flake out if the number is bigger than the maximum positive 32
> bit signed int.

this sounds like a perl bug on MacOS X (another one!! what are they doing over
there?)

> However, POSIX::setgid() works fine to set both the real and effective gid with
> one call and it accepts 4294967294.
> 
> So I suggest that we replace
> 
>    $) = "$ugid $ugid";    # effective gid
>    $( = $ugid;            # real gid
> 
> with
>     setgid($ugid);
> 
> I'm not expert in this stuff... Does anyone see any problem with using
> POSIX::setgid here on other platforms?

it may not be implemented, for one, which would cause a die().  We'd have
to protect it with an eval ' ... ' block.

you'd probably have to do sth like this:

  - run the $) / $( assignments in an eval '...' block
  - capture errors
  - if error =~ /setrgid\(\) not implemented/, then
    - run POSIX::setgid(...)
    - die if that failed too

however would it be simpler to just do (pseudocode)

  if (platform eq MacOS && ($uid = getpwnam($user)) == 4294967294) { $uid = -2; }

if that works, that would be better than attempting to use another syscall
entirely, IMO.

Also I suggest that should be opened as a totally separate bug; it's not caused
by this one, it's just being _exposed_ by the fix.

btw, I've added "t/root_spamd_virtual.t" as a test case for this.  Sidney, could
you verify that fails for you?

doh.  that was for bug 5518!

anyway, I've applied patch 3989 to SVN trunk:

: jm 247...; svn commit -m "bug 5510: fix test suite to support running as root,
now that spamd will automatically fall back to 'nobody' privs"
Sending        t/SATest.pm
Sending        t/spamd_allow_user_rules.t
Sending        t/spamd_plugin.t
Transmitting file data ...
Committed revision 547650.

setting this to review state; please vote for inclusion in 3.2.x.

(In reply to comment #13)
Assignments to $< and $> are fine with either 4294967294 or -2
[...] assignments to $( and $) that flake out if the number
is bigger than the maximum positive 32 bit signed int.
> 
> this sounds like a perl bug on MacOS X

> However, POSIX::setgid() works fine to set both the real
> and effective gid with

> you'd probably have to do sth like this:
>   - run the $) / $( assignments in an eval '...' block
>   - capture errors
>   - if error =~ /setrgid\(\) not implemented/, then
>     - run POSIX::setgid(...)
>     - die if that failed too

If POSIX::setgid and POSIX::setuid are available (and I can't think
why they wouldn't be), by all means, use ONLY these, to avoid
all the $<, $>, $( and $) perl crap, which is causing all kinds
of inconsistencies on various platforms.  The Net::Server (which
is used by amavisd-new) had a series of problem-reports until
it finally switched to POSIX::setgid and POSIX::setuid.

For illustration (not that I claim it is best), this is what
currently stands in Net::Server::Daemonize.pm:

sub set_uid {
  my $uid = get_uid( shift() );
  POSIX::setuid($uid);
  if ($< != $uid || $> != $uid) { # check $> also (rt #21262)
    $< = $> = $uid; # try again-needed by some 5.8.0 linux systems (rt #13450)
    if ($< != $uid) {
      die "Couldn't become uid \"$uid\": $!\n";
    }
  }
  return 1;
}

sub set_gid {
  my $gids = get_gid( @_ );
  my $gid  = (split /\s+/, $gids)[0];
  eval { $) = $gids }; # store all the gids - this is really sort of optional
  POSIX::setgid($gid);
  if (! grep {$gid == $_} split /\s+/, $() { # look for any valid id in list
    die "Couldn't become gid \"$gid\": $!\n";
  }
  return 1;
}

> If POSIX::setgid and POSIX::setuid are available (and I can't think
> why they wouldn't be), by all means, use ONLY these, to avoid

Martin, I'm going to copy your comment over to bug 5518 where we are continuing
this discussion, and I'll respond there.

As for the proposed patch for this bug that is up for review
I vote +1

+1

ok, checked in on 3.2.x:

: jm 142...; svn commit -m "bug 5110: fix test suite to support running as root,
now that spamd will automatically fall back to 'nobody' privs; this was breaking
CPAN installation" t/spamd_allow_user_rules.t t/spamd_plugin.t t/SATest.pm
Sending        t/SATest.pm
Sending        t/spamd_allow_user_rules.t
Sending        t/spamd_plugin.t
Transmitting file data ...
Committed revision 548391.

*** Bug 5525 has been marked as a duplicate of this bug. ***

*** Bug 5550 has been marked as a duplicate of this bug. ***

Created attachment 4050 [details]
same patch against 3.1 branch

Here is the same fix in the form of a patch against the 3.1 branch

Reopen to target to 3.1.10

Here's my +1 to apply it

+1

+1

Committed to branch 3.1 revision 558675.