Source: inn2
Severity: wishlist
Tags: patch
Dear Maintainer,
INN, at the moment, supports TLS connections to nnrpd, but does not
allow any configuration besides the certificate and key.
This means that Wheezy's nnrpd is currently susceptible to the CRIME
(because TLS compression is on) and POODLE (because SSLv3 is
supported) attacks, should those be exploitable with NNTP. In
addition, it supports weak symmetrical ciphers (40 and 56 bit key
length).
I've patched nnrpd to allow for detailed TLS configuration: protocol
versions, cipher suites, compression and whether the client or server
choses the cipher can now be configured. With the default
configuration, TLS behaviour is unchanged, as to not break existing
setups.
This patch is to be integrated upstream[0], but ideally I'd like it
to be in the next Wheezy point release because I consider the current
TLS config to be insecure.
The patch, as attached, is against a clean 2.5.4 upstream source, but
I'd be happy to provide a patch for quilt if you tell me which package
version I should target.
regards,
cm.
[0] https://lists.isc.org/pipermail/inn-workers/2014-November/018339.html
diff --git a/doc/pod/inn.conf.pod b/doc/pod/inn.conf.pod
index f8f5f79..98ebd6e 100644
--- a/doc/pod/inn.conf.pod
+++ b/doc/pod/inn.conf.pod
@@ -1054,6 +1054,28 @@ I<pathetc>/key.pem.
This file must only be readable by the news user or B<nnrpd> will refuse to
use it.
+=item I<tlsprotocols>
+
+The list of TLS protocol versions to support. Valid protocols are
+B<SSLv2>, B<SSLv3>, B<TLSv1>, B<TLSv1.1> and B<TLSv1.2>. The default
+value is B<[ SSLv3 TLSv1 TLSv1.1 TLSv1.2 ]>.
+
+=item I<tlsciphers>
+
+The string describing the cipher suites OpenSSL will support. See
+OpenSSL's B<cipher> command documentation for details. The default is
+unset, which uses OpenSSL's default cipher suite list.
+
+=item I<tlsprefer_server_ciphers>
+
+Whether to let the client or the server decide the preferred cipher.
+This is a boolean and the default is false.
+
+=item I<tlscompression>
+
+Whether to enable or disable TLS compression support (boolean). The
+default is true.
+
=back
=head2 Monitoring
diff --git a/doc/pod/news.pod b/doc/pod/news.pod
index 4315b3f..64cd93b 100644
--- a/doc/pod/news.pod
+++ b/doc/pod/news.pod
@@ -1,3 +1,17 @@
+=head1 Changes in TLS configuration
+
+=over 2
+
+=item *
+
+New parameters used by B<nnrpd> to fine-tune the TLS configuration:
+I<tlsprotocols>, I<tlsciphers>, I<tlsprefer_server_ciphers> and
+I<tls_compression>. If you've been using TLS with B<nnrpd> before, be
+aware that the defaults of those parameters may differ from the
+previous defaults (which depended on your OpenSSL version).
+
+=back
+
=head1 Changes in 2.5.4
=over 2
diff --git a/doc/pod/nnrpd.pod b/doc/pod/nnrpd.pod
index 9c13821..32698ae 100644
--- a/doc/pod/nnrpd.pod
+++ b/doc/pod/nnrpd.pod
@@ -224,6 +224,12 @@ run B<nnrpd>. (Change the path to B<nnrpd> to match your installation.)
You may need to replace C<nntps> with C<563> if C<nntps> isn't
defined in F</etc/services> on your system.
+Optionally, you may set the I<tlsprotocols>, I<tlsciphers>,
+I<tlsprefer_server_ciphers> and I<tlscompression> parameters in
+F<inn.conf> to fine-tune the behaviour of the TLS negotiation whenever
+a new attack on the TLS protocol or some supported cipher suite is
+discovered.
+
=head1 PROTOCOL DIFFERENCES
B<nnrpd> implements the NNTP commands defined in S<RFC 3977> (NNTP),
diff --git a/include/inn/innconf.h b/include/inn/innconf.h
index ee16620..669255c 100644
--- a/include/inn/innconf.h
+++ b/include/inn/innconf.h
@@ -127,6 +127,10 @@ struct innconf {
char *tlscapath; /* Path to a directory of CA certificates */
char *tlscertfile; /* Path to the SSL certificate to use */
char *tlskeyfile; /* Path to the key for the certificate */
+ bool tlsprefer_server_ciphers; /* Make server select the cipher */
+ bool tlscompression; /* Turn TLS compression on/off */
+ struct vector *tlsprotocols; /* List of supported TLS versions */
+ char *tlsciphers; /* openssl-style cipher string */
#endif /* HAVE_SSL */
/* Monitoring */
diff --git a/lib/innconf.c b/lib/innconf.c
index ded674c..9e6183d 100644
--- a/lib/innconf.c
+++ b/lib/innconf.c
@@ -231,6 +231,10 @@ const struct config config_table[] = {
{ K(tlscapath), STRING (NULL) },
{ K(tlscertfile), STRING (NULL) },
{ K(tlskeyfile), STRING (NULL) },
+ { K(tlsprefer_server_ciphers), BOOL (false) },
+ { K(tlscompression), BOOL (true) },
+ { K(tlsprotocols), LIST (NULL) },
+ { K(tlsciphers), STRING (NULL) },
#endif /* HAVE_SSL */
/* The following settings are used by nnrpd and rnews. */
diff --git a/nnrpd/tls.c b/nnrpd/tls.c
index 62b1a51..22a00c7 100644
--- a/nnrpd/tls.c
+++ b/nnrpd/tls.c
@@ -425,7 +425,9 @@ set_cert_stuff(SSL_CTX * ctx, char *cert_file, char *key_file)
int
tls_init_serverengine(int verifydepth, int askcert, int requirecert,
char *tls_CAfile, char *tls_CApath, char *tls_cert_file,
- char *tls_key_file)
+ char *tls_key_file, int prefer_server_ciphers,
+ int tls_compression, struct vector *tls_proto_vect,
+ char *tls_ciphers)
{
int off = 0;
int verify_flags = SSL_VERIFY_NONE;
@@ -434,6 +436,8 @@ tls_init_serverengine(int verifydepth, int askcert, int requirecert,
char *s_cert_file;
char *s_key_file;
struct stat buf;
+ int tls_protos = 0;
+ int i;
if (tls_serverengine)
return (0); /* Already running. */
@@ -493,6 +497,70 @@ tls_init_serverengine(int verifydepth, int askcert, int requirecert,
SSL_CTX_set_tmp_dh_callback(CTX, tmp_dh_cb);
SSL_CTX_set_options(CTX, SSL_OP_SINGLE_DH_USE);
+#ifdef SSL_OP_CIPHER_SERVER_PREFERENCE
+ if(prefer_server_ciphers) {
+ SSL_CTX_set_options(CTX, SSL_OP_CIPHER_SERVER_PREFERENCE);
+ }
+#endif
+
+ if((tls_proto_vect != NULL) && (tls_proto_vect->count > 0)) {
+ for(i = 0; i < tls_proto_vect->count; i++) {
+ if(tls_proto_vect->strings[i] != NULL) {
+ if(strcmp(tls_proto_vect->strings[i], "SSLv2") == 0)
+ tls_protos |= INN_TLS_SSLv2;
+ else if(strcmp(tls_proto_vect->strings[i], "SSLv3") == 0)
+ tls_protos |= INN_TLS_SSLv3;
+ else if(strcmp(tls_proto_vect->strings[i], "TLSv1") == 0)
+ tls_protos |= INN_TLS_TLSv1;
+ else if(strcmp(tls_proto_vect->strings[i], "TLSv1.1") == 0)
+ tls_protos |= INN_TLS_TLSv1_1;
+ else if(strcmp(tls_proto_vect->strings[i], "TLSv1.2") == 0)
+ tls_protos |= INN_TLS_TLSv1_2;
+ else
+ syslog(L_ERROR, "TLS engine: unknown protocol '%s' in tlsprotocols",
+ tls_proto_vect->strings[i]);
+ }
+ }
+ } else {
+ tls_protos = (INN_TLS_SSLv3 | INN_TLS_TLSv1 | INN_TLS_TLSv1_1 | INN_TLS_TLSv1_2);
+ }
+
+ if(!(tls_protos & INN_TLS_SSLv2)) {
+ SSL_CTX_set_options(CTX, SSL_OP_NO_SSLv2);
+ }
+
+ if(!(tls_protos & INN_TLS_SSLv3)) {
+ SSL_CTX_set_options(CTX, SSL_OP_NO_SSLv3);
+ }
+
+ if(!(tls_protos & INN_TLS_TLSv1)) {
+ SSL_CTX_set_options(CTX, SSL_OP_NO_TLSv1);
+ }
+
+#ifdef SSL_OP_NO_TLSv1_1
+ if(!(tls_protos & INN_TLS_TLSv1_1)) {
+ SSL_CTX_set_options(CTX, SSL_OP_NO_TLSv1_1);
+ }
+#endif
+
+#ifdef SSL_OP_NO_TLSv1_2
+ if(!(tls_protos & INN_TLS_TLSv1_2)) {
+ SSL_CTX_set_options(CTX, SSL_OP_NO_TLSv1_2);
+ }
+#endif
+
+ if(tls_ciphers)
+ if(SSL_CTX_set_cipher_list(CTX, tls_ciphers) == 0) {
+ syslog(L_ERROR, "TLS engine: cannot set cipher list");
+ return (-1);
+ }
+
+#ifdef SSL_OP_NO_COMPRESSION
+ if(!tls_compression) {
+ SSL_CTX_set_options(CTX, SSL_OP_NO_COMPRESSION);
+ }
+#endif
+
verify_depth = verifydepth;
if (askcert!=0)
verify_flags |= SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE;
@@ -529,7 +597,11 @@ tls_init(void)
innconf->tlscafile,
innconf->tlscapath,
innconf->tlscertfile,
- innconf->tlskeyfile);
+ innconf->tlskeyfile,
+ innconf->tlsprefer_server_ciphers,
+ innconf->tlscompression,
+ innconf->tlsprotocols,
+ innconf->tlsciphers);
if (ssl_result == -1) {
Reply("%d Error initializing TLS\r\n",
initialSSL ? NNTP_FAIL_TERMINATING : NNTP_ERR_STARTTLS);
diff --git a/nnrpd/tls.h b/nnrpd/tls.h
index 90c8edb..012434d 100644
--- a/nnrpd/tls.h
+++ b/nnrpd/tls.h
@@ -27,6 +27,13 @@
#include <openssl/x509.h>
#include <openssl/ssl.h>
+/* protocol support */
+#define INN_TLS_SSLv2 1
+#define INN_TLS_SSLv3 2
+#define INN_TLS_TLSv1 4
+#define INN_TLS_TLSv1_1 8
+#define INN_TLS_TLSv1_2 16
+
/* Init TLS engine. */
int tls_init_serverengine(int verifydepth, /* Depth to verify. */
int askcert, /* 1 = Verify client. */
@@ -34,7 +41,11 @@ int tls_init_serverengine(int verifydepth, /* Depth to verify. */
char *tls_CAfile,
char *tls_CApath,
char *tls_cert_file,
- char *tls_key_file);
+ char *tls_key_file,
+ int prefer_server_ciphers,
+ int tls_compression,
+ struct vector *tls_protocols,
+ char *tls_ciphers);
/* Init TLS. */
int tls_init(void);
diff --git a/samples/inn.conf.in b/samples/inn.conf.in
index d92423e..b8d4115 100644
--- a/samples/inn.conf.in
+++ b/samples/inn.conf.in
@@ -137,6 +137,10 @@ backofftrigger: 10000
#tlscapath: @sysconfdir@
#tlscertfile: @sysconfdir@/cert.pem
#tlskeyfile: @sysconfdir@/key.pem
+#tlsprotocols: [ SSLv3 TLSv1 TLSv1.1 TLSv1.2 ]
+#tlsciphers:
+#tlsprefer_server_ciphers: false
+#tlscompression: true
# Monitoring
