Sorry, I just realized that description is a little incorrect.

If we use --sql-config or --ldap-config, spamd won't try to use the default
setuid behavior, but it will try to fallback to nobody if it's running as root.
This doesn't really make sense, since we shouldn't be using the setuid behavior
to begin with.

Anyways, I've committed a patch. I'm still unsure whether we should drop the
requirement for '-u' if we're running spamd as a different (non-root) user.

Committed revision 596888.

Created attachment 4188 [details]
Fix for 3.2 branch

+1

+1

'sudo make test TEST_FILES=t/root*' passes.

Now, does anyone have an opinion on whether we should be checking for -u or that
$> != 0?

(In reply to comment #5)
> Now, does anyone have an opinion on whether we should be checking for -u or that
> $> != 0?

can you clarify?

what is the behaviour if run as non-root without -u?  spamd won't be able to
setuid anyway so it should be a moot point...

(In reply to comment #6)
> (In reply to comment #5)
> > Now, does anyone have an opinion on whether we should be checking for -u or that
> > $> != 0?
> 
> can you clarify?

This whole section of code hurts my brain.

I believe under the current code, you can't run
"spamd --vpopmail" or "spamd --virtual-config-dir" as a non-root user. We
specifically check to see if these options are used without the "--username"
option and die.

With my change, this also applies to "spamd --sql-config" and "spamd --ldap-config".

The quick fix (I think) is to wrap this code (and the like)

if ( $opt{'vpopmail'} ) {
  if ( !$opt{'username'} ) {
    die "spamd: cannot use --vpopmail without -u\n";
  }
}

with an "if ($setuid_to_user) {}" or "if ($> == 0) {}" block.

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Now, does anyone have an opinion on whether we should be checking for -u
or that
> > > $> != 0?
> > 
> > can you clarify?
> 
> This whole section of code hurts my brain.
> 
> I believe under the current code, you can't run
> "spamd --vpopmail" or "spamd --virtual-config-dir" as a non-root user. We
> specifically check to see if these options are used without the "--username"
> option and die.
> 
> With my change, this also applies to "spamd --sql-config" and "spamd
--ldap-config".
> 
> The quick fix (I think) is to wrap this code (and the like)
> 
> if ( $opt{'vpopmail'} ) {
>   if ( !$opt{'username'} ) {
>     die "spamd: cannot use --vpopmail without -u\n";
>   }
> }
> 
> with an "if ($setuid_to_user) {}" or "if ($> == 0) {}" block.

I think it's ok, since $setuid_to_user is set to 1 if ($> != 0).

(In reply to comment #8)

> I think it's ok, since $setuid_to_user is set to 1 if ($> != 0).

I'm confused by your comment (what is ok?) -- what do you think is the right
thing to do?

(In reply to comment #9)
> (In reply to comment #8)
> 
> > I think it's ok, since $setuid_to_user is set to 1 if ($> != 0).
> 
> I'm confused by your comment (what is ok?) -- what do you think is the right
> thing to do?

actually, ignore my comment -- I misread both your intentions, and the code ;)
So you said:

'I believe under the current code, you can't run
"spamd --vpopmail" or "spamd --virtual-config-dir" as a non-root user. We
specifically check to see if these options are used without the "--username"
option and die.
With my change, this also applies to "spamd --sql-config" and "spamd
--ldap-config".'

that's almost correct.  Actually, it *IS* possible to run as non-root if the
user uses '-u $currentuser'; e.g. if you look at t/spamd_allow_user_rules.t, it
runs spamd with

  spamd --virtual-config-dir=log/virtualconfig/%u -L -u $spamd_run_as_user

where $spamd_run_as_user is set by the test framework to be whatever the username
of the currently-running uid is.  So the caller has to know what their current
username is, and use that in the invocation, and spamd will then run ok as
non-root with those options.

If you are suggesting that we should *automatically* deal with this case, by
doing something like

  if ($> != 0 && !$opt{'username'}) {
    # we are run as non-root; set username to whatever we're currently running as
    $opt{'username'} = currently_running_username();
  }

I would be in favour of this, as a new feature.  But it'd be a separate bug...

is that what you were talking about?

Committed to 3.2 branch (with fixed typo in comment) as r599208

Updated Summary to reflect description better.

Filed bug 5734 in response to Justin's comments.