debian bug 387883 is reported as bug 5163

I don't think this is a bug or even an issue unless I'm missing something.

bayes_journal shouldn't be a symlink, but there's no real reason why it can't
be.  The user's spamassassin user directory shouldn't allow others to create
files in there, so it could only ever be the user or root who creates that
symlink.  If you're worried about root trying to screw the user, then you've
already lost.  spamd writes the bayes_journal file as the user, not root, so
this could only really effect things the user can write to anyway.

So what's the issue?

Adding Martin to CC.

What if the user wants to screw over the system and creates a symlink from
bayes_journal to /etc/passwd? Then spamd will overwrite this file and happiness
ensures.

(In reply to comment #4)
> What if the user wants to screw over the system and creates a symlink from
> bayes_journal to /etc/passwd? Then spamd will overwrite this file and happiness
> ensures.

Since the user has no write privs to /etc/passwd, nothing will happen.

(In reply to comment #5)
> Since the user has no write privs to /etc/passwd, nothing will happen.

I haven't tried this yet myself, but doesn't the output shown in Description
show otherwise?

seamus:~/.spamassassin> echo test | sudo tee /etc/mypasswd
test
seamus:~/.spamassassin> ls -l /etc/mypasswd
-rw-r--r-- 1 root root 5 2007-05-22 22:52 /etc/mypasswd

At this point /etc/mypasswd is owned by root and contains "test".

seamus:~/.spamassassin> ls -l /etc/mypasswd
-rw-r--r-- 1 root root 16325 2007-05-22 23:00 /etc/mypasswd
seamus:~/.spamassassin> grep -c test /etc/mypasswd
0

At this point it has been overwritten

(In reply to comment #6)
> I haven't tried this yet myself, but doesn't the output shown in Description
> show otherwise?

Not necessarily, in my opinion.  Also, if no one else can reproduce this then
any problem could just be the host in use ...

> seamus:~/.spamassassin> echo test | sudo tee /etc/mypasswd
> test
> seamus:~/.spamassassin> ls -l /etc/mypasswd
> -rw-r--r-- 1 root root 5 2007-05-22 22:52 /etc/mypasswd

So we don't know what user is running commands here, but if you're running a
bunch of stuff via sudo, then you're using root privileges to monkey around with
things.  As I said before, though in a different way, if you're working with
root to subvert your system, then you'll subvert your system.

> seamus:~/.spamassassin> sudo chown madduck.madduck bayes_journal \
>     && sa-learn --force-expire && ln -s /etc/mypasswd bayes_journal

So the ls output shows bayes_journal is already a symlink, but it doesn't show
where it points to...

lrwxrwxrwx 1 root    root       5261 2007-05-22 22:52 bayes_journal

You, as root, change the owner and group of the file that bayes_journal is
pointing to -- chown/chgrp/chmod follows symlinks unless you use the correct
flag (see "man chown" and things like "-h").  So now /etc/mypasswd is owned by
madduck.madduck, which means spamd can write to it as you.

> Now wait a bit until spamd processes another message, and:
> seamus:~/.spamassassin> ls -l /etc/mypasswd
> -rw-r--r-- 1 root root 16325 2007-05-22 23:00 /etc/mypasswd
> seamus:~/.spamassassin> grep -c test /etc/mypasswd
> 0

Due to the above chown command, I don't believe this output.  Or, something's
gone through and changed ownership back.

If someone can reproduce this, without using sudo and similar commands, then
it's potentially an issue.  Please show exactly how spamd is run, how spamc is
called, and give a concise recipe to reproduce.

Created attachment 3949 [details]
shell session log with comments

This was a paste error while making the bug report.

Please first see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=387883 ---
spamd does not drop root rights in time in this example, and the two are likely
related.

Anyway, I attach an undoctored transcript with plenty of comments. Please take
the time to read it.

needs looking at/fixing for 3.2.1.  bug 5163 is probably the right thing to fix.

Thanks for the session log -- it was very helpful in illustrating the
issue.

Ugh -- verified:

  sudo spamd --create-prefs --max-children 5 --helper-home-dir \
    --allow-tell --paranoid -x --debug --pidfile=/var/run/spamd.pid

Then running "spamc -lx -L ham < sample-nonspam.txt", I get 
Message successfully un/learned

And the spamd debug log says:

[25125] dbg: locker: safe_lock: trying to get lock on /root/.spamassassin/bayes
with 0 retries
[25125] dbg: locker: safe_lock: link to /root/.spamassassin/bayes.lock: link ok
[25125] dbg: bayes: tie-ing to DB file R/W /root/.spamassassin/bayes_toks
[25125] dbg: bayes: tie-ing to DB file R/W /root/.spamassassin/bayes_seen
[25125] dbg: bayes: new db, set db version 3 and 0 tokens
[25125] dbg: bayes: learned
'a1381889b89a4970ed596e4c6b4f0ce310276bb0@sa_generated', atime: 987802486
[25125] dbg: bayes: untie-ing
[25125] dbg: bayes: files locked, now unlocking lock
[25125] dbg: locker: safe_unlock: unlink /root/.spamassassin/bayes.lock
[25125] info: spamd: Tell: Setting local for jm:0 in 0.9 seconds, 6494 bytes


This is indeed a critical security bug in --allow-tell, afaics. :(


However, I'm not yet certain the --virtual-config-dir issue is quite as
critical; it's normal to use -u on the spamd command line to force running as a
single, unprivileged user when --virtual-config-dir is used, since that switch
entirely inhibits setuid operation.  The issue here may be a documentation
and usage one, where we just need to block use of --virtual-config-dir
without -u.

Using 

  sudo rm -rf /tstsrv/vmail
  sudo mkdir -p /tstsrv/vmail
  sudo chown nobody /tstsrv/vmail
  sudo spamd --allow-tell --username=nobody --create-prefs \
   --max-children 5 --helper-home-dir  --paranoid -x --debug \
   --pidfile=/var/run/spamd.pid \
   --virtual-config-dir=/tstsrv/vmail/%d/%l/.spamassassin

then

  ./spamc/spamc -lx -u jm@foo.com -L ham < sample-nonspam.txt

the expected result is that the /tstsrv/vmail/foo.com/jm/.spamassassin/
dir is created as "nobody", and that's indeed what happens:

: jm 34...; sudo ls -la /tstsrv/vmail/foo.com/jm/.spamassassin/
total 40
drwx------ 2 nobody nogroup  4096 May 25 18:28 .
drwx------ 3 nobody nogroup  4096 May 25 18:28 ..
-rw------- 1 nobody nogroup 12288 May 25 18:28 bayes_seen
-rw------- 1 nobody nogroup 24576 May 25 18:28 bayes_toks

I haven't had time to look at fixing this, yet :(

is there a vulnerability that can be exploited by a user _without_ a local
nonroot login on the spamd box?

i don't think this is a remote DoS or root exploit, it's just a symlink
attack/local DoS.

OK. The problem is that setuid doesn't happen at all if "-x" is specified --
this is what's causing the bug here. This limitation is not made clear in the
POD doc at all:

  =item B<-x>, B<--nouser-config>, B<--user-config>

  Turn off(on) reading of per-user configuration files (user_prefs) from the
  user's home directory.  The default behaviour is to read per-user
  configuration from the user's home directory.

  This option does not disable or otherwise influence the SQL, LDAP or
  Virtual Config Dir settings.


I don't think it makes sense to just fix the documentation to note this
behaviour, since the behaviour is nonsensical anyway.   If per-user conf files
are disabled with -x, there's no setuid!?  crazy.  The better fix is to fix -x
behaviour to match the doc.

So here's a patch that does that. It:

  - ignores -x when deciding whether or not to setuid

  - adds debugs noting whether or not it will setuid, and what euid is
    running at check()/dotell() time (for debugging)

  - consolidates some shared code between dotell() and check(), copying
    behaviour that was missing in dotell() from check() in the process --
    this is the key part

  - dies if spamd is running with a root euid in check() or dotell():
    we've documented that this is impossible, so let's make sure!

  - renames "handle_user()" to "handle_user_setuid_basic()", which
    more closely matches the other "handle_user_foo()" functions

  - adds code to handle_user_setuid_basic() to setuid even if -x is active

  - splits out the user-config-specific behaviour in handle_user_setuid_basic()
    into a new subfunction for clarity

  - updates POD docs for --vpopmail and --virtual-config-dir to note
    that they inhibit setuid and require -u

  - add a initialisation check to enforce that
 
With this patch, things are much saner; things setuid and run as non-root, -x
or no -x. This also fixes bug 5163, since  --virtual-config-dir now enforces
its -u requirement correctly.

The second patch then, is a small run-as-root set of tests which I intend to
add to the test suite to test regression on this issue; they only run if
running as root, and if the "run_root_tests" var in t/config is set to "y".
They then can be run like this:

        echo run_root_tests=y >> t/config
        sudo prove t/root*

Created attachment 3959 [details]
patch, fixing the bug

Created attachment 3960 [details]
patch, adding the root* tests

From cursory testing, I think this does in fact solve the two bugs. However, it
also makes it impossible to use spamd in a mixed environment with some real and
some virtual users. C.f.
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=387878. Will I have to start
two spamd master instances to support this scenario?

Oh, and one other thing, with the patch, spamd no longer logs to syslog.
Correction: with the -u switch, before or after the patch, no more syslog
entries happen. I am not sure this is related. I started just now as I was
trying the patch, but I also cannot make it work anymore after removing the patch.

(In reply to comment #16)
> From cursory testing, I think this does in fact solve the two bugs. However, it
> also makes it impossible to use spamd in a mixed environment with some real and
> some virtual users. C.f.
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=387878. Will I have to start
> two spamd master instances to support this scenario?

yes, I'm afraid so... there's probably other bugs that would interfere with it,
too.  we haven't really designed a way to support that.

two spamd instances is probably the easiest way to do it, anyway.

(In reply to comment #17)
> Oh, and one other thing, with the patch, spamd no longer logs to syslog.
> Correction: with the -u switch, before or after the patch, no more syslog
> entries happen. I am not sure this is related. I started just now as I was
> trying the patch, but I also cannot make it work anymore after removing the patch.

hmm. could it be /dev/console permissions?  very odd... I'm pretty sure it's
unrelated though

Created attachment 3962 [details]
patch, fixing t/spamd_allow_user_rules.t

hmm. this patch is also required. it fixes t/spamd_allow_user_rules.t
to use "-u", since it uses "--virtual-config-dir" which now requires
that (even though it can't setuid...)

ok, I propose that once this is reviewed, we release a 3.2.1 immediately with
this fix.  Anything else in the 3.2.x queue should wait, since this is a
critical issue for people using spamd --allow-tell.

We also need to verify if this is an issue with 3.1.x, and if it is, if the fix
needs to be backported there too.

Should we be getting a CVE number?

(I'm in S.F. without a computer, so I can't review the patch right now.)

yep, I think it warrants a CVE.

The root tests patch has a bug consistent across the various test files - It
sets constant RUN_TEST but then tests RUN_TESTS

+1, after the RUN_TEST/RUN_TESTS errors are fixed in the test files.

By the way, I would like to see bug 5422 get the one more vote it needs to be
committed if we are going to push out 3.2.1 so soon. It would be a shame not to
get that in when it just missed getting in the 3.2 release. Also there are a few
bugs that are all voted on and ready for commit in the 3.2 branch.

Forwarded from email from Mark J Cox:

"I don't have permissions to edit that bug, please use CVE-2007-2873 and 
note that in the bug."

agreed, Sidney.

Created attachment 3965 [details]
consolidated patch for 3.1.x

yep, it affects 3.1.x, too :(

here's a version of that patch for 3.1.x.  The only difference is (a) the
RUN_TEST/RUN_TESTS fix and (b) a slight difference in the code that removes the
--vpopmail/-u semantics, since in 3.2.x there's a bug fix to that code in
handle_user() which never made it into 3.1.x.

Created attachment 3966 [details]
consolidated patch for 3.2.x

and here's a redone consolidated patch for 3.2.x, which fixes the
RUN_TEST/RUN_TESTS thing Sidney spotted.  (no other changes.)

We ought to think about the security announcement to go along with the fix for
this bug.

Which options are necessary to demonstrate the problem? It looks like -x is
required I see --allow-tell in a demonstration of the bug in comment #10, but is
--virtual-config required along with that for it to be a vulnerability? Is
-v/--vpopmail also vulnerable, or does that just get involved with the fix?

Here is a first cut at an announcement. It probably needs quite a bit of
editing, but I wanted to get it going before we find we needed it yesterday. In
particular I guessed at the details about exactly which switch combinations do
and do not exhibit the vulnerability:

To be first mailed

To: vendor-sec@lst.de [can be forwarded by people on cc list of this bug]

[CONFIDENTIAL SECURITY ADVISORY - NOT FOR PUBLIC RELEASE]

and when 3.2.1 is released posted at
'http://spamassassin.apache.org/advisories/cve-2007-2873.txt'

  ---------

A local user symlink-attack DoS vulnerability in SpamAssassin has been found,
affecting versions 3.1.x, 3.2, and SVN trunk.  It has been
assigned CVE-2007-2873. Details:

- It only affects systems where spamd is run as root, is used with vpopmail or
virtual users via the "-v"/"--vpopmail" OR "--virtual-config-dir" switch, AND
with the "-x"/"--no-user-config AND WITHOUT the "-u"/"--username" switch AND
with the "-l"/"--allow-tell" switch

  This is not default on any distro package, and is not a common configuration.
  
 - It is a local exploit that requires the attacker to have a local account
whose mail is being processed by spamd.

 - The effect of the exploit is to allow overwriting of arbitrary files that are
accessible by the spamd process with data that is not under the control of the
attacker. It is DoS vulnerability that does not provide remote execution nor
escalation of local privileges.

Workaround: If you are running spamd using a vulnerable combination of switches,
add the "-u" / "--username" switch to specify a non-root user that spamd child
processes will run as. Note that in a mixed real/virtual user environment you
will have to run two separate instances of spamd on different ports, with the
instance that specifies "-v"/"--vpopmail" or "--virtual-config" also specifying
"-u"/"--username".

The vulnerability is fixed in SpamAssassin version 3.2.1 by, among other fixes,
no longer allowing the use of "-v"/"--vpopmail" or "--virtual-config" without
"-u"/"--username". Thus, the configuration change described in the above
workaround is still necessary when upgrading to 3.2.1.

+1 for 3.1 branch

What do you think of checking this in to trunk with a comment referencing bug
5163 and closing that bug?

+1 for 3.2 fix
+1 for 3.1 fix

I just had a thought... We could check this in to trunk and both branches with a
comment referencing bug 5163. That way we don't alert people to the
vulnerability before it is announced, and more importantly we don't make it
clear right away how to exploit the bug. Yes, that's relying on obscurity, but
I'm proposing that we do that to obscure things during the short time period
that we have to have between checking in the fix and releasing 3.2.1.

Now that we have the votes for the patch we should finish the announcement and
get it out to vedor-sec right away so that they get it before the public
announcement.

Does this have anything to do with bug 5163?  Should 5163 be fixed, if its
immediately clear how, for 3.2.1 also?

The summary for bug 5163 says that spamd does a setuid away from root too late.
But the example has it using --virtual-config-dir with -x and not with -u. That,
as we see here, cause it to never setuid non-root, which I guess qualifies as
too late.

This fix disallows running in that configuration and closes that bug.

Someone please correct me if I'm misunderstanding the situation.

Ah, OK, I haven't really read either bug.

Yep, Sidney's right; this fixes bug 5163 too.

I would prefer *not* to check this into trunk just yet, fwiw.  Here's what we
decided was the security policy -- (guess who forgot to put our security policy
up on the wiki, like I said I would after the long-URI DoS issue ;) ...

- Generally figure out which version(s) are impacted by the issue

- Write up a general vulnerability statement explaining the issue

- Request a CVE.  security@apache.org last said to contact
Mark J Cox <mark@awe.com> to get a number. (*)

- Notifications are made in advance to vendor-sec and anyone the
committers feel like informing, as long as it is kept private.
notifications contain the vulnerability statement, CVE info, and patch
(if possible).

- Public releases are made at an agreed upon time, ideally 1-2
business days after the notification to vendor-sec.
patch is not committed to SVN until the public release tarballs are being prepared.


Today's a holiday in Ireland, btw, so I won't be moving on this in a hurry until
tomorrow... if anyone wants to get going on it, feel free.

btw:

'Note that in a mixed real/virtual user environment you
will have to run two separate instances of spamd on different ports, with the
instance that specifies "-v"/"--vpopmail" or "--virtual-config" also specifying
"-u"/"--username".'

I think that can be omitted from the vulnerability description -- that was
always the case anyway, it was more like a bug exploit that a mixed real/virtual
setup might possibly work.

Also, I would say that the vuln description should note that in the described
configuration, the spamd process will be running with root privileges (rather
than with some kind of non-root-but-privileged account).

> it was more like a bug exploit that a mixed real/virtual
> setup might possibly work

I included it because I thought that the set of people who are running a
vulnerable configuration includes people who are running a mixed real/virtual
setup, and they should be informed that the workaround and the bug fix preclude
doing that anymore. If that isn't the case, i.e., if there aren't going to be
people who find with this announcement that they have to figure out how to do
something different, then I agree that there is no need to mention it.

In the announcement, it would be nice to give Martin credit for the discovery.

> In the announcement, it would be nice to give Martin credit for the discovery

+1

Created attachment 3970 [details]
cve-2007-2873.txt

ok, here's that announcement, slightly tidied, arranged into a more "standard"
CVE format with the fields we've used in the past, and with a credit for Martin
;)

gimme a couple of +1s and we'll send it out, along with copies of the patch for
3.1.x and 3.2.0, to vendor-sec, then get building a tarball (for release a
couple of days later).

actually, a couple of days is Friday.  should we leave public notification
until the following Monday, to avoid spoiling sysadmin weekends? ;)

Monday might be a good idea.

Do the new tarball releases really need to come a few days after disclosure?  It
would be preferable if we could get the ball rolling of pushing security updates
with the new upstream version.  It helps a bit on user confusion because they
see "3.1.8 and earlier" is affected but we still are shipping what appears to be
3.1.8.

(In reply to comment #43)
> Monday might be a good idea.
> 
> Do the new tarball releases really need to come a few days after disclosure?  It
> would be preferable if we could get the ball rolling of pushing security updates
> with the new upstream version.  It helps a bit on user confusion because they
> see "3.1.8 and earlier" is affected but we still are shipping what appears to be
> 3.1.8.

well, this would be disclosure to vendor-sec; that's not public disclosure yet.
 The idea would be to embargo it until the proposed tarball release date.  So in
other words, the disclosure and the new tarballs are simultaneous.

+1 on the announcement

I'm agnostic about extending the release of the embargo until Monday. This isn't
a very critical vulnerability. I would bet that any malicious local users who
see the announcement on Friday and want to DoS the system that is delivering
their own mail at an installation that is running vpopmail/virtual-config-dir -x
no -u and --allow-tell as root whose sysadmin waits until Monday to change the
configuration to use -u would probably not figure out how to perform the DoS
until Monday anyway :-)

Two things: this is a vendor-sec announcement, so users won't see it until x
days after we send this. Thus I think we should send it now to get more eyes
looking at this, who can then still wait for a working day.

Second, it would be nice if my email said madduck@debian.org instead of the
@pobox.madduck.net address.

+1 on the immediate announcement to vendor-sec.  +1 on 3.2.1 release and public
announcement on Monday.

I don't have a vote, but I like this plan.

We really ought to make tarballs available to vendor-sec type people before the
public release. (.htaccess protected web dir on people.apache.org?)

That would be highly desirable to us as well.

Once a tarball is available, even if it's not posted publicly (I believe in the
past we've still just put it on people.apache.org), you could always do an svn
checkout of the tag for the version and run make dist.

Here's what I'm saying:

Vendor-sec disclosure:
 - patch and tarballs made available to vendor-sec
 - no commits made to svn

Public disclosure
 - tarballs made public
 - commit made to svn

Daryl, I think you're saying that if you have the patch, it's easy to generate
the tarball by hitting make dist. That's true, but I'd rather have "official"
tarballs with "offical" md5sums, signatures, etc. I don't think there's any
guarantee a tarball done with make dist on different systems has the same
md5sum. (Debian policy, for example, requires the tarball they distribute to be
(bit for bit) identical to the upstream tarball if at all possible.)

That is Fedora and Red Hat policy as well.

ugh, that'll be tricky.  bear in mind that to generate a tarball for the
vendor-sec announcement we'd have to:

- check in the fix to public SVN, 2 days in advance of public notification
- make public SVN tags for the release
- OR change our build procedures to use private SVN and private URLs, instead of
public

my objections:

- I don't like "leaking" the patch to public SVN, 2 days in advance.  and we
don't have a procedure to make a tarball except from public SVN.
- now, when we need to do this quickly, is not the time to change our build
procedures.  nor do we have the infrastructure to operate a private SVN repository.

I'd prefer to *just* send the patches along with the vendor-sec announcement,
and note that we will be releasing a 3.2.1 (and 3.1.9!) tarball containing the
fix (along with others) on Monday.  if we want to come up with a new way to
generate "private" tarballs as project builds, we can take the time to do that
afterwards.

> - I don't like "leaking" the patch to public SVN, 2 days in advance.  and we
> don't have a procedure to make a tarball except from public SVN.

to clarify: it's possible to generate some kind of tarballs without writing to
SVN, currently -- but they're not "official" tarballs built according to our
build procedure, hence we can't release them as-is, we'd have to re-do them and
hope they're still the same.  Not much good.


I went ahead with the announcement, since it was +1'd by Daryl and Sidney, and
Duncan didn't give any kind of vote either way.

Justin, you missed the request in comment #46

Could you correct that in the public announcement, and perhaps in an errata to
vendor-sec?

(In reply to comment #56)
> Justin, you missed the request in comment #46
> 
> Could you correct that in the public announcement, and perhaps in an errata to
> vendor-sec?

oops. sorry about that, will do...

There are no more bugs on the 3.2.1 queue other than this one and bug 5163 which
can be closed when this one is.

Created attachment 3977 [details]
manifest patch for 3.2.x

here's an additional patch to add those t/root* tests to the MANIFEST file;
it's non-urgent and doesn't need to be applied to make tarballs, just tidies up
a loose end.  it'd be nice to apply (no hurry though, I suggest post-release).


In the meantime, here's the tarballs:

  md5sum of archive files:
  7b2fdbcdca5e9a181d4bb1b17663c138  Mail-SpamAssassin-3.2.1.tar.bz2
  a7d51294c565999da01f212e5ad2a031  Mail-SpamAssassin-3.2.1.tar.gz
  e058ed0dfe82ee62f617c12cc02e538b  Mail-SpamAssassin-3.2.1.zip
  sha1sum of archive files:
  3095b38d90d0362c4e47e117fb612778a2ac362b  Mail-SpamAssassin-3.2.1.tar.bz2
  fbb5f538238e188f985c8e6672dad531fa035eea  Mail-SpamAssassin-3.2.1.tar.gz
  d6566975544cd706052d310481d7a100ffce14d1  Mail-SpamAssassin-3.2.1.zip

  md5sum of archive files:
  ad5d812b1a04228f3dc3147ebd649bb3  Mail-SpamAssassin-3.1.9.tar.bz2
  c0a6dc8564e60bf50d1792e4edc18e97  Mail-SpamAssassin-3.1.9.tar.gz
  a1ed25d0878d102c17a91233ee741f87  Mail-SpamAssassin-3.1.9.zip
  sha1sum of archive files:
  bed85f0b7e269253e925831015f11809009080eb  Mail-SpamAssassin-3.1.9.tar.bz2
  181e0ca4e0568bb51e955b8b8e4595313fb7de8b  Mail-SpamAssassin-3.1.9.tar.gz
  c5f87a454ce4562558fd1af9ea71b7b858899f3e  Mail-SpamAssassin-3.1.9.zip

  http://people.apache.org/~jm/private/oops5480kdf/

please vote on these... 

also, please don't share these outside the dev team just yet, until they've
been
voted on.  if/when they get 2 +1's, *then* go ahead and pass them on if
you like; but until then, we don't want these confused with "official"
releases.

Created attachment 3978 [details]
manifest patch for 3.1.x

and the MANIFEST update for 3.1.x, too. (again, low priority)

Created attachment 3979 [details]
3.1.9 release announcement mail

Created attachment 3980 [details]
3.2.1 release announcement mail

+1 on both tarballs
+1 on 3977 (manifest patch for 3.2.x)
+1 on release announcements

3978 toggles $IS_DEVEL_BUILD when it shouldn't, otherwise +1 on it.

Same votes as Daryl:

+1 on both tarballs
+1 on 3977 (manifest patch for 3.2.x)
+1 on release announcements

3978 toggles $IS_DEVEL_BUILD when it shouldn't, otherwise +1 on it.

Created attachment 3982 [details]
fixed manifest patch for 3.1.x

oops. thanks for spotting that.  I also notice that I had 3965 included in
patch 3978, which was superfluous.  here's a redone patch with the superfluous
files removed; shout if your votes no longer apply for some reason ;)

anyway, we're ready to go for Monday...

btw, now that the tarballs are official, I've passed their location on to
vendors via vendor-sec.

http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5336
http://cvs.fedora.redhat.com/viewcvs/rpms/spamassassin/FC-6/spamassassin-3.1.8-sa-learn.patch?root=core&rev=1.1&view=log
Our spamassassin-3.1.8 has been applying this patch.  It seems that we forgot to
apply it to 3.1.9?

Do I need to continue applying that patch to our packaged 3.1.9?

(In reply to comment #67)
> http://issues.apache.org/SpamAssassin/show_bug.cgi?id=5336
>
http://cvs.fedora.redhat.com/viewcvs/rpms/spamassassin/FC-6/spamassassin-3.1.8-sa-learn.patch?root=core&rev=1.1&view=log
> Our spamassassin-3.1.8 has been applying this patch.  It seems that we forgot to
> apply it to 3.1.9?
> 
> Do I need to continue applying that patch to our packaged 3.1.9?

If it wasn't included, it really needs to be.  Apparently no one ever committed
it after the patch was approved. :(

I just did that.

argh.  I suggest we go ahead and release 3.2.1 now at least.

then, do we want to hold up 3.1.9 for that sa-learn patch? (I suggest we forget
it, assuming it's in 3.2.x anyway.)

votes please -- and quickly, since the embargo is over and we need to get a
release out asap :(

Impact of that patch:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228968

(In reply to comment #70)
> Impact of that patch:
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228968

that's pretty minor -- a harmless warning, right?

+1 for releasing the hounds! 

Minor annoyance, doesn't break it.  Please don't delay release of 3.1.9.

Hmm... RHEL-3 ships spamassassin-2.55.  Any idea if it is susceptible to this issue?

(In reply to comment #74)
> Hmm... RHEL-3 ships spamassassin-2.55.  Any idea if it is susceptible to this
issue?

It's ok -- it doesn't support --allow-tell.

(In reply to comment #71)
> (In reply to comment #70)
> > Impact of that patch:
> > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228968
> 
> that's pretty minor -- a harmless warning, right?

It's a warning, but it's annoying when you're trying to do the extremely common
"pipe message to sa-learn", etc.  It was discovered very shortly after 3.1.8
came out, we came up with the patch, and were planning a 3.1.9 release to fix it
and a few other small issues.  Then everyone forgot about doing 3.1 releases and
nothing happened for months.

So at this point, just go ahead and do the release -- it's already been voted
on, and vendors have probably already gotten ahold of it so it's essentially
"official", as far as our version scheme is concerned.

I'm disappointed that this bug has been around for months, with a patch, ready
to commit, and no one (yes I include myself) has done anything about it.  We
always get into this "why worry about 3.x when 3.y is out/coming out RSN" state.  :(

Let's at least prep for a 3.1.10, move the bugs to a new milestone, etc. 
Someday, that bug will be fixed.

I'm a little confused by something -- Theo committed the patch to 3.1 branch
just now, and we haven't yet applied this security fix to the branch. Doesn't
that mean we are ready to go and 3.1.9 will have this patch in it? It does mean
that the pre-release we sent to vendor-sec is missing something that will be in
the official release. Can't we deal with that with an "oops, sorry" email to
vendor-sec? I'm +1 for dealing with it that way and moving on with getting 3.1.9
out the door using what is now checked into the 3.1 branch.

We already built packages for RHEL4 and RHEL5 using 3.1.9 provided in Comment
#59, and they are currently in QA.  We would be OK with whatever upstream
chooses to do for this release.

> So at this point, just go ahead and do the release -- it's already been voted
> on, and vendors have probably already gotten ahold of it so it's essentially
> "official", as far as our version scheme is concerned.

thanks theo!  I've done that now.

> I'm disappointed that this bug has been around for months, with a patch, ready
> to commit, and no one (yes I include myself) has done anything about it.  We
> always get into this "why worry about 3.x when 3.y is out/coming out RSN"
state.  :(

yes.  what *should* have happened there is that the patch was applied to
3.1.x as soon as the votes were in, and definitely before the bug was moved off
the milestone, marked RESOLVED, or any other similar thing etc.

> Let's at least prep for a 3.1.10, move the bugs to a new milestone, etc. 
> Someday, that bug will be fixed.

+1

'I'm a little confused by something -- Theo committed the patch to 3.1 branch
just now, and we haven't yet applied this security fix to the branch. Doesn't
that mean we are ready to go and 3.1.9 will have this patch in it?'

nope, not without cutting new tarballs etc. etc. etc.... it would just be
a bit of hassle I'm afraid, and right now time is of the essence, it's not good
to have a publicly-known security bug with no release that fixes it IMO. :(

anyway, 3.1.9 and 3.2.1 are now out there with this bug (bug 5480) fixed. closing!

> nope, not without cutting new tarballs etc. etc. etc

So this means that you have put together a procedure that allows us to make
official builds containing patches that have not yet been committed to svn, then
commit just those patches and tag the repository to correspond to the build that
had been made? In that case, yes I agree with what has been done since the
tarballs that have already been cut are the official ones whose hashes are in
the announcements.

Is the new build procedure now documented in the wiki?

(In reply to comment #80)
> 
> Is the new build procedure now documented in the wiki?
> 

I believe it is outlined in build/README

(In reply to comment #81)
> (In reply to comment #80)
> > 
> > Is the new build procedure now documented in the wiki?
> > 
> 
> I believe it is outlined in build/README

yep.

opening bug to the public