Can I merge this for jessie? On Nov 11, christian mock <c...@tahina.priv.at> wrote:
> 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 > -- ciao, Marco
pgpenX_g1md7N.pgp
Description: PGP signature
