>From 1d785fb7d138a2a5c5542ec76aaa33a13122e150 Mon Sep 17 00:00:00 2001 From: Ander Juaristi Date: Sat, 28 Apr 2018 14:06:34 +0200 Subject: [PATCH 1/3] Enhance SSL/TLS security This commit hardens SSL/TLS a bit more in the following ways: * Explicitly exclude NULL authentication and the 'MEDIUM' cipher list category. Ciphers in the 'HIGH' level are only considered - this includes all symmetric ciphers with key lengths larger than 128 bits, and some ('modern') 128-bit ciphers, such as AES in GCM mode. * Allow RSA key exchange by default, but exclude it when Perfect Forward Secrecy is desired (with --secure-protocol=PFS). * Introduce new option --ciphers to set the cipher list that the SSL/TLS engine will favor. This string is fed directly to the underlying TLS library (GnuTLS or OpenSSL) without further processing, and hence its format and syntax are directly dependent on the specific library. Reported-by: Jeffrey Walton --- src/gnutls.c | 78 ++++++++++++++++++++++++++++++++++++++--------------------- src/init.c | 1 + src/main.c | 5 ++++ src/openssl.c | 26 +++++++++++++++++--- src/options.h | 2 ++ 5 files changed, 81 insertions(+), 31 deletions(-) diff --git a/src/gnutls.c b/src/gnutls.c index 0fd8da8c..0368b4a4 100644 --- a/src/gnutls.c +++ b/src/gnutls.c @@ -535,35 +535,10 @@ _sni_hostname(const char *hostname) return sni_hostname; } -bool -ssl_connect_wget (int fd, const char *hostname, int *continue_session) +static int +set_prio_default (gnutls_session_t session) { - struct wgnutls_transport_context *ctx; - gnutls_session_t session; - int err; - - gnutls_init (&session, GNUTLS_CLIENT); - - /* We set the server name but only if it's not an IP address. */ - if (! is_valid_ip_address (hostname)) - { - /* GnuTLS 3.4.x (x<=10) disrespects the length parameter, we have to construct a new string */ - /* see https://gitlab.com/gnutls/gnutls/issues/78 */ - const char *sni_hostname = _sni_hostname(hostname); - - gnutls_server_name_set (session, GNUTLS_NAME_DNS, sni_hostname, strlen(sni_hostname)); - xfree(sni_hostname); - } - - gnutls_credentials_set (session, GNUTLS_CRD_CERTIFICATE, credentials); -#ifndef FD_TO_SOCKET -# define FD_TO_SOCKET(X) (X) -#endif -#ifdef HAVE_INTPTR_T - gnutls_transport_set_ptr (session, (gnutls_transport_ptr_t) (intptr_t) FD_TO_SOCKET (fd)); -#else - gnutls_transport_set_ptr (session, (gnutls_transport_ptr_t) FD_TO_SOCKET (fd)); -#endif + int err = -1; #if HAVE_GNUTLS_PRIORITY_SET_DIRECT switch (opt.secure_protocol) @@ -642,6 +617,53 @@ ssl_connect_wget (int fd, const char *hostname, int *continue_session) } #endif + return err; +} + +bool +ssl_connect_wget (int fd, const char *hostname, int *continue_session) +{ + struct wgnutls_transport_context *ctx; + gnutls_session_t session; + int err; + + gnutls_init (&session, GNUTLS_CLIENT); + + /* We set the server name but only if it's not an IP address. */ + if (! is_valid_ip_address (hostname)) + { + /* GnuTLS 3.4.x (x<=10) disrespects the length parameter, we have to construct a new string */ + /* see https://gitlab.com/gnutls/gnutls/issues/78 */ + const char *sni_hostname = _sni_hostname(hostname); + + gnutls_server_name_set (session, GNUTLS_NAME_DNS, sni_hostname, strlen(sni_hostname)); + xfree(sni_hostname); + } + + gnutls_credentials_set (session, GNUTLS_CRD_CERTIFICATE, credentials); +#ifndef FD_TO_SOCKET +# define FD_TO_SOCKET(X) (X) +#endif +#ifdef HAVE_INTPTR_T + gnutls_transport_set_ptr (session, (gnutls_transport_ptr_t) (intptr_t) FD_TO_SOCKET (fd)); +#else + gnutls_transport_set_ptr (session, (gnutls_transport_ptr_t) FD_TO_SOCKET (fd)); +#endif + + if (!opt.tls_ciphers_string) + { + err = set_prio_default (session); + } + else + { +#if HAVE_GNUTLS_PRIORITY_SET_DIRECT + err = gnutls_priority_set_direct (session, opt.tls_ciphers_string, NULL); +#else + logprintf (LOG_NOTQUIET, _("GnuTLS: Cannot set prio string directly. Falling back to default priority.\n")); + err = gnutls_set_default_priority (); +#endif + } + if (err < 0) { logprintf (LOG_NOTQUIET, "GnuTLS: %s\n", gnutls_strerror (err)); diff --git a/src/init.c b/src/init.c index e4186abe..d7e6a723 100644 --- a/src/init.c +++ b/src/init.c @@ -280,6 +280,7 @@ static const struct { #endif { "preservepermissions", &opt.preserve_perm, cmd_boolean }, #ifdef HAVE_SSL + { "ciphers", &opt.tls_ciphers_string, cmd_string }, { "privatekey", &opt.private_key, cmd_file }, { "privatekeytype", &opt.private_key_type, cmd_cert_type }, #endif diff --git a/src/main.c b/src/main.c index 46824efd..635f0c1a 100644 --- a/src/main.c +++ b/src/main.c @@ -390,6 +390,7 @@ static struct cmdline_option option_data[] = { "preferred-location", 0, OPT_VALUE, "preferredlocation", -1 }, #endif { "preserve-permissions", 0, OPT_BOOLEAN, "preservepermissions", -1 }, + { IF_SSL ("ciphers"), 0, OPT_VALUE, "ciphers", -1 }, { IF_SSL ("private-key"), 0, OPT_VALUE, "privatekey", -1 }, { IF_SSL ("private-key-type"), 0, OPT_VALUE, "privatekeytype", -1 }, { "progress", 0, OPT_VALUE, "progress", -1 }, @@ -862,6 +863,10 @@ HTTPS (SSL/TLS) options:\n"), --egd-file=FILE file naming the EGD socket with random data\n"), #endif "\n", + N_("\ + --ciphers=STR Set the priority string (GnuTLS) or cipher list string (OpenSSL) directly.\n\ + Use with care. This option overrides --secure-protocol.\n\ + The format and syntax of this string depend on the specific SSL/TLS engine.\n"), #endif /* HAVE_SSL */ #ifdef HAVE_HSTS diff --git a/src/openssl.c b/src/openssl.c index aab55d98..3f155b05 100644 --- a/src/openssl.c +++ b/src/openssl.c @@ -173,6 +173,7 @@ ssl_init (void) { SSL_METHOD const *meth; long ssl_options = 0; + char *ciphers_string = NULL; #if !defined(LIBRESSL_VERSION_NUMBER) && (OPENSSL_VERSION_NUMBER >= 0x10100000L) int ssl_proto_version = 0; #endif @@ -305,10 +306,29 @@ ssl_init (void) #endif /* OpenSSL ciphers: https://www.openssl.org/docs/apps/ciphers.html - * Since we want a good protection, we also use HIGH (that excludes MD4 ciphers and some more) + * + * Rules: + * 1. --ciphers overrides everything + * 2. We allow RSA key exchange by default (secure_protocol_auto) + * 3. We disallow RSA key exchange if PFS was requested (secure_protocol_pfs) */ - if (opt.secure_protocol == secure_protocol_pfs) - SSL_CTX_set_cipher_list (ssl_ctx, "HIGH:MEDIUM:!RC4:!SRP:!PSK:!RSA:address@hidden"); + if (!opt.tls_ciphers_string) + { + if (opt.secure_protocol == secure_protocol_auto) + ciphers_string = "HIGH:!aNULL:!RC4:!MD5:!SRP:!PSK"; + else if (opt.secure_protocol == secure_protocol_pfs) + ciphers_string = "HIGH:!aNULL:!RC4:!MD5:!SRP:!PSK:!kRSA"; + } + else + { + ciphers_string = opt.tls_ciphers_string; + } + + if (ciphers_string && !SSL_CTX_set_cipher_list(ssl_ctx, ciphers_string)) + { + logprintf(LOG_NOTQUIET, _("OpenSSL: Invalid cipher list: %s\n"), ciphers_string); + goto error; + } SSL_CTX_set_default_verify_paths (ssl_ctx); SSL_CTX_load_verify_locations (ssl_ctx, opt.ca_cert, opt.ca_directory); diff --git a/src/options.h b/src/options.h index 30845a1b..2697a1d2 100644 --- a/src/options.h +++ b/src/options.h @@ -257,6 +257,8 @@ struct options bool ftps_fallback_to_ftp; bool ftps_implicit; bool ftps_clear_data_connection; + + char *tls_ciphers_string; #endif /* HAVE_SSL */ bool cookies; /* whether cookies are used. */ -- 2.14.1