I can reproduce this in Linux by running in a source directory in which I've built spamassassin, where user1 and user2 are two different users on the system so that the group user1 specified in the command is not a group that user2 is in:

sudo runuser -g user1 -u user2 -- ./spamassassin -D util -t < /dev/null > /dev/null

Note that the output only has warnings, no errors, but if this happens under some circumstances that can be considered normal, then the warnings would be a bug.

Is it a normal circumstance for the service to be set to run with gid set to a group that the uid it is run at does not belong to?

(In reply to Sidney Markowitz from comment #1)
> sudo runuser -g user1 -u user2 -- ./spamassassin -D util -t < /dev/null >
> /dev/null

Correction, reproducing the error only requires that -g specify any group that is not the primary group of the user specified in the -u option.

I think that if the current ruid and euid are the same and non-zero, there is no reason to try to change egid to the primary group of the uid, especially since it is impossible to do it anyway.

The warnings can just go away, and in fact the code there can be skipped if not running as root instead of fruitlessly trying to set group and user ids that can't be set.

Henrik, this looks like you code from revision 1889937 can you take a look and see if warnings could be dropped in this circumstance?

Also, I wondered if it is possible to get such an error by giving more than 5 bytes of binary string to a binary(5) column. When I looked at it I see that bayes tokens are made from the lower 40 bits of a SHA1 hash, so always exactly 5 bytes.

But that's yet another indication that the UTF-8 change cannot be the correct fix, there is no way for tokens to be any character set, they are arbitrary 40-bit binary strings.

(In reply to Sidney Markowitz from comment #4)

Whoops, sorry, posted in wrong browser tab, sorry.

(In reply to Sidney Markowitz from comment #3)
> Henrik, this looks like you code from revision 1889937 can you take a look
> and see if warnings could be dropped in this circumstance?

Honestly I'm a bit swamped right now, so if anyone else can look deeper into these.. too hard to wrap my head around the all the "circumstances" and how to handle all.

I believe I added the warnings to catch these kinds of things that may need improving. :-)

(In reply to Henrik Krohns from comment #6)
> Honestly I'm a bit swamped right now, so if anyone else can look deeper into
> these..

No problem, I can take it on, I'll just go for minimum necessary changes.

So we don't lose historical context, some of the original reasoning around this in in bug 3097 comment 19

One thing that brings out is that there is no reason to be concerned about setuid unless RUID == 0 which is a good thing because if it isn't zero then setuid is impossible. Issuing meaningless warnings when it isn't zero may be what this bug is.

And to add a bit more important historical context, to emphasize that we should not try to "simplify" this mess of code too much without knowing what we are doing, see bug 3586 for an explanation of much of the strange complexity in this code.

Created attachment 5818 [details]
patch to handle spamd launched with gid of supplemental group instead of primary group

In deciding whether the real gid needs to be changed, setuid_to_euid checks whether it currently equals the primary group of the user specified by the effective uid. The idea is that we are about to set real uid to the value of the effective uid, so the real gid should be the group corresponding to that uid. It is necessary to look at that instead of the effective gid because it is possible to set the euid to some user value while leaving the effective gid set to root.

In the use case of this bug, spamd is being launched as a service with a gid that the uid is a member of, but is not the primary group of that uid. The fix for this is instead of testing whether real gid is equal to the primary group id, test if it is found in the list of group ids for that uid.

In this use case, spamd never has privileges because it wasn't launched as root, so it would be ok to simply not run setuid_to_euid. However, in addition to real uid/gid and effective uid/gid, there is a third pair called saved uid/gid that makes it possible for a process to look like it is not privileged based on the values of the real and effective uids, but still be able to escalate back to root privileges. And perl does not have a way of accessing the saved uid/gid to find out if that is the case. So there is no workaround to running setuid_to euid just in case it is needed. Perl does have a way of ensuring that privileges have been fully dropped and saved uid/gid are non-root, and that is in the setuid_to euid code already.

Please check this and vote on committing it for 4.0.0

Sidney I've confirmed that the patch fixes the warnings I was seeing when restarting Amavis with SpamAssassin 4. Thanks!

Can't test but +1 for good explanation and changes look minor

+1 (non-voting) from me as the changes are minor. Hopefully this makes it into SpamAssassin 4 before the official release.

+1 (can't test it, code looks reasonable)

Note, the commit contains a fix to a misspelled word in a comment that is in the patch that was voted on.

trunk % svn ci -m "Bug 8043 - Don't try and fail to setgid to drop privs when spamd started with a supplemental group without privs"
Sending        lib/Mail/SpamAssassin/Util.pm
Transmitting file data .done
Committing transaction...
Committed revision 1904201.