You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@bugzilla.spamassassin.org on 2015/11/17 00:33:43 UTC
[Bug 7267] New: no way to set SSL_VERIFY_PEER in spamd
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Bug ID: 7267
Summary: no way to set SSL_VERIFY_PEER in spamd
Product: Spamassassin
Version: 3.4.1
Hardware: All
OS: All
Status: NEW
Severity: enhancement
Priority: P2
Component: spamc/spamd
Assignee: dev@spamassassin.apache.org
Reporter: curtis@ipv6.occnc.com
There is no server side SSL verification in spamd.
The fix on the spamd side is simple. Not so simple in spamc.
On spamd there needs to be three options, "--ssl-ca-file=file",
"-ssl-ca-path=path", and "--ssl-verify".
Setting any one of them sets "$sockopt->SSL_verify_mode =
SSL_VERIFY_PEER;" on about line 1077 in spamd. If either ssl-ca-file or
ssl-ca-path are set, then set sockopt->SSL_ca_file or sockopt->SSL_ca_path.
That's it for spamd. I haven't looked at spamc, but it should not be much more
difficult to add options for key and cert. Just C rather than perl.
Diffs to follow.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
--- Comment #4 from Curtis Villamizar <cu...@ipv6.occnc.com> ---
Created attachment 5350
--> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5350&action=edit
patch to spamc/spamc.pod
Changes to documentation adding new options, updating the interpretation of the
B<--ssl>=I<sslversion> handling, and saying bad things about using SSLv3 which
need to be said.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
--- Comment #5 from Curtis Villamizar <cu...@ipv6.occnc.com> ---
Created attachment 5351
--> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5351&action=edit
patch for spamc/spamc,c
Nothing at all non-obvious. Just added code to handle new options.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Kevin A. McGrail <km...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|3.4.2 |3.4.3
--- Comment #16 from Kevin A. McGrail <km...@apache.org> ---
Pushing to 3.4.3. Too big a change for this late in the game.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Henrik Krohns <ap...@hege.li> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #5653|0 |1
is obsolete| |
--- Comment #20 from Henrik Krohns <ap...@hege.li> ---
Created attachment 5767
--> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5767&action=edit
Further cleaned up version with debug (nonworking)
Here's a further cleaned up version, which still doesn't work.. I'm
flabbergasted why certificate handshake goes fine and everything looks fine,
but things crash because $client->peerhost is missing when
peer_info_from_socket() is called.
DEBUG: .../IO/Socket/SSL.pm:1099: handshake done, socket ready
DEBUG: .../IO/Socket/SSL.pm:991: accept_SSL ok
Apr 12 16:34:49.155 [1446667] warn: spamd: error: failed to obtain port and ip
from socket at /usr/local/perl-sa/bin/spamd line 1658.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
--- Comment #6 from Curtis Villamizar <cu...@ipv6.occnc.com> ---
Created attachment 5352
--> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5352&action=edit
patch to spamc/libspamc.h
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
--- Comment #7 from Curtis Villamizar <cu...@ipv6.occnc.com> ---
Created attachment 5353
--> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5353&action=edit
patch to spamc/libspamc.c
All changes are inside #ifdef SPAMC_SSL / #endif pairs.
Two functions are added. Each function is called in two places where a few
lines are replaced.
It helps to look at the replaced code first. The SSLv3_client_method() and
TLSv1_client_method() functions require exact match to SSLv3 or TLSv1, rather
than "or better". Since "or better" is assumed to have been the intent,
SSLv23_client_method() is used in the new code, with SSL_CTX_set_options used
to restrict the version negotiation (SSL_OP_NO_SSLv2, SSL_OP_NO_SSLv3).
The first new function replaced code did some init and "ctx =
SSL_CTX_new(meth);". The new code just replaces the code restricting which
SSL/TLS versions are allowed.
The second function supports the new options. First the CA certs are loaded if
provided on the command line. Then the client cert and key are loaded if
provided on the command line and then checked. The rest is like the original
code but with a bit of error checking.
The "ssl = SSL_new(ctx);" is run and "ssl" is copied back to the caller. Then
"SSL_set_fd(ssl, sock)" is done, only this code checks the error return and
logs. Then "SSL_connect(ssl)" is run, again with error checking (which in this
case is disabled). The return value of SSL_connect(ssl) was ignored in the
original code so it is ignored here.
Maybe that should change.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Kevin A. McGrail <km...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|Undefined |3.4.3
--- Comment #11 from Kevin A. McGrail <km...@apache.org> ---
Thank you for responding but sorry, it's really not. Can you take a pass at
https://www.apache.org/licenses/icla.pdf?
If you can give some clarity about which patches to look at, which order. which
are tested, etc. that would be helpful. KAM
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
--- Comment #10 from Curtis Villamizar <cu...@ipv6.occnc.com> ---
Please consider the patches in this bug report to be the property of Apache
Foundation to be made available under the Apache Licence Agreement and any
other agreement spammassassin is made available under.
I hope that is sufficient for a CLA. If not please let me know.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Henrik Krohns <ap...@hege.li> changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|blocker |enhancement
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
--- Comment #2 from Curtis Villamizar <cu...@ipv6.occnc.com> ---
Now that this is all debugged (took longer than I thought due to an oversight
on my part in looking over the spamc code) I will be attaching five patch files
and explaining the changes in each one. The five files are:
patch-spamc__libspamc.c
patch-spamc__libspamc.h
patch-spamc__spamc.c
patch-spamc__spamc.pod
patch-spamd__spamd.raw
This is an overview of the patches. Details will come with the files.
Spamd has three options added: B<--ssl-verify>, B<--ssl-ca-file>=I<cafile>, and
B<--ssl-ca-path>=I<capath>. The first just turns on client certificate
verification and without the other two would use the built in CA certs. The
other two allow a CA file or CA path to be added as in IO::Socket::SSL.
The handling of ssl-version has changed. The default is now tlsv12 (you can
change back if you want). The code now makes it so sslv3 allows sslv3 or
better, tlsv1 allows tlsv1 or better, etc. The options to specific tlsv11 and
tlsv12 are added (with tlsv11 allowing "or better").
If SSL is requested, fallback to plain text socket is not allowed.
If any one of the three new options are specified, then the client will be
required to provide a certificate and it has to be signed by a CA (which only
really makes sense if a CA cert is specified and its your own CA cert (which
could be self signed, or a domain signing certificate).
Since spamc didn't support sending a client cert, it had to be changed.
The pod file just has changed docs. The .h file has a few lines to support the
new options. The spamc.c file has a small amount of change to support the help
and the setting the options. The new options for spamc are
B<--ssl-cert>=I<cert-file> and B<--ssl-key>=I<key-file> to specify a client
cert and client key. Here also the default is changed to tlsv1. The code is
changed so sslv3 allows sslv3 or better and tlsv1 allows tlsv1 or better.
The real work in spamc is all in libspamc.c. Two new functions were created.
Each takes a few identical lines formerly pasted into two places and expands on
that code (rather than expanding in four places all together). Now fallback to
plain text is not allowed. If client cert and key are provided, then spamc can
provide a client cert if asked for one by spamd.
It seems like a real bad idea to have a server cert verification fail and fall
back to doing the same thing in the clear. I'm assuming that was a bug and not
intentional. It also seems best to change the default from the known flawed
sslv3 to at least tlsv1.0 and even better to tlsv1.2. A few words of code can
change the defaults back. Since spamc and spamd are distributed together for
most users thats not an issue. If a transition is needed either end can get
upgraded first specifying sslv3 to support the transition and changing configs
to tlsv1 after transition is done.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Curtis Villamizar <cu...@ipv6.occnc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #5347|0 |1
is obsolete| |
--- Comment #3 from Curtis Villamizar <cu...@ipv6.occnc.com> ---
Created attachment 5349
--> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5349&action=edit
patch to spamd/spamd.raw
The $ssl_version_mapping allows simple sslv3, tlsv1, tlsv[12] options to be
specified by the user but having the effect of that version or better. The
full syntax in IO::Socket::SSL is more flexible but this is easier on the user.
The $have_ssl_module block is earlier in the code so things like 0x01 be
replaced with Net::SSLeay::VERIFY_PEER(). Also
Net::SSLeay::VERIFY_FAIL_IF_NO_PEER_CERT() is or'd in so fallback to plain text
is not allowed.
Setting SSL_verifycn_scheme to 'none' is OK if the CA cert is your own and you
only sign for things you trust. Otherwise an rDNS lookup and CN check against
it would be needed and this would have to be done in a SSL_verifycn_scheme
callback. I turned off CRL check though supporting a CRL file would be a
possible further enhancement.
Nothing else at all non-obvious in the changes.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Curtis Villamizar <cu...@ipv6.occnc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #5350|0 |1
is patch| |
Attachment #5350|application/x-perl |text/plain
mime type| |
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Stingertough <wo...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |wolfsplat@gmail.com
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Giovanni Bechis <gi...@paclan.it> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #5588|0 |1
is obsolete| |
CC| |giovanni@paclan.it
--- Comment #17 from Giovanni Bechis <gi...@paclan.it> ---
Created attachment 5653
--> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5653&action=edit
Let spamc compile without ssl support, check return values
Polished version of the diff, it seems not to work correctly anyway for me,
"openssl s_client" is able to connect to spamd but when spamc connects gives a
"bad protocol: header error: (closed before headers)" warning message.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Kevin A. McGrail <km...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|enhancement |blocker
Target Milestone|3.4.3 |3.4.2
--- Comment #13 from Kevin A. McGrail <km...@apache.org> ---
(In reply to Curtis Villamizar from comment #12)
> I just sent in the ICLA. Second time was a charm. First time files was too
> big.
>
> As so which patches, all of them except the one that is marked "preliminary"
> and "not tested":
>
> patch-spamc__libspamc.c
> patch-spamc__libspamc.h
> patch-spamc__spamc.c
> patch-spamc__spamc.pod
> patch-spamd__spamd.raw
>
> The comment trail describes the changes to each file.
>
> These have not changed since Nov 2015 and have been used as local patches
> since then. I'm using the FreeBSD ports collections and have built most
> recently on July 25 of this year. So they have been tested for three years
> now.
>
> If you prefer I can just concatonate the individual patch files into one
> file.
If you can review the patches and made a consolidated patch and take a good
pass at the documentation, I'll add this for 3.4.2.
Upgrading to blocker.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
--- Comment #12 from Curtis Villamizar <cu...@ipv6.occnc.com> ---
I just sent in the ICLA. Second time was a charm. First time files was too
big.
As so which patches, all of them except the one that is marked "preliminary"
and "not tested":
patch-spamc__libspamc.c
patch-spamc__libspamc.h
patch-spamc__spamc.c
patch-spamc__spamc.pod
patch-spamd__spamd.raw
The comment trail describes the changes to each file.
These have not changed since Nov 2015 and have been used as local patches since
then. I'm using the FreeBSD ports collections and have built most recently on
July 25 of this year. So they have been tested for three years now.
If you prefer I can just concatonate the individual patch files into one file.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Kevin A. McGrail <km...@pccc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |kmcgrail@pccc.com
--- Comment #8 from Kevin A. McGrail <km...@pccc.com> ---
Curtis,
These are no trivial patches that will require a license agreement. Can you
please take a look at http://apache.org/licenses/#clas ?
Regards,
KAM
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
--- Comment #19 from Henrik Krohns <ap...@hege.li> ---
If someone can confirm it applies and works properly on current trunk/4.0.0, I
can have a look at committing. Don't have any spare time for this.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Henrik Krohns <ap...@hege.li> changed:
What |Removed |Added
----------------------------------------------------------------------------
Blocks| |7185
Referenced Bugs:
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7185
[Bug 7185] Spamc/Spamd with SSL Improve documentation and Process
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Curtis Villamizar <cu...@ipv6.occnc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #5349|0 |1
is obsolete| |
Attachment #5350|0 |1
is obsolete| |
Attachment #5351|0 |1
is obsolete| |
Attachment #5352|0 |1
is obsolete| |
Attachment #5353|0 |1
is obsolete| |
--- Comment #14 from Curtis Villamizar <cu...@ipv6.occnc.com> ---
Created attachment 5588
--> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5588&action=edit
consolidated patches to five files into one patch file
This is a consolidated patch file that is just a concatonation of the five
patch files.
Documentation is in spamc.pod and the pod comments in spamd.raw. All new
command options are described in the help as well as the man pages from pod.
I've tried to match coding style.
I'm going to bring up a test spamd server and test spamc client and further
test to make sure the TLS version options all work.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Henrik Krohns <ap...@hege.li> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #21 from Henrik Krohns <ap...@hege.li> ---
(In reply to Henrik Krohns from comment #20)
> things crash because $client->peerhost is missing when peer_info_from_socket() is called.
And this was because spamc already closed the connection due to completely
faulty _try_ssl_connect return code handling..
Removed --ssl-version, since I don't see the point, it can be added later if
needed.
Should be good enough for committing:
Sending trunk/UPGRADE
Sending trunk/spamc/libspamc.c
Sending trunk/spamc/libspamc.h
Sending trunk/spamc/spamc.c
Sending trunk/spamc/spamc.pod
Sending trunk/spamd/spamd.raw
Transmitting file data ......done
Committing transaction...
Committed revision 1899803.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Curtis Villamizar <cu...@ipv6.occnc.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |curtis@ipv6.occnc.com
--- Comment #1 from Curtis Villamizar <cu...@ipv6.occnc.com> ---
Created attachment 5347
--> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5347&action=edit
preliminary patch for spamd - not tested
Looking into spamc shortly. Will test after that.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
--- Comment #9 from Kevin A. McGrail <km...@apache.org> ---
Checking if we have an ICLA from Curtis Villamizar otherwise very little can be
done with these patches.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@bugzilla.spamassassin.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Bill Cole <sa...@billmail.scconsult.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jerryzh168@gmail.com
--- Comment #15 from Bill Cole <sa...@billmail.scconsult.com> ---
*** Bug 7092 has been marked as a duplicate of this bug. ***
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 7267] no way to set SSL_VERIFY_PEER in spamd
Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7267
Henrik Krohns <he...@hege.li> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|3.4.3 |4.0.0
CC| |hege@hege.li
--- Comment #18 from Henrik Krohns <he...@hege.li> ---
Targeting 4.0.0 which seems much more appropriate
--
You are receiving this mail because:
You are the assignee for the bug.