From 849f85a05c17828c592bed26bd99707f211fad1c Mon Sep 17 00:00:00 2001 From: Alexander Barton Date: Sun, 15 Sep 2013 14:09:31 +0200 Subject: ConnSSL_InitLibrary(): Code cleanup --- src/ngircd/conn-ssl.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/ngircd/conn-ssl.c b/src/ngircd/conn-ssl.c index 096ff951..595cb615 100644 --- a/src/ngircd/conn-ssl.c +++ b/src/ngircd/conn-ssl.c @@ -285,8 +285,10 @@ ConnSSL_InitLibrary( void ) if (!RAND_status()) { Log(LOG_ERR, "OpenSSL PRNG not seeded: /dev/urandom missing?"); /* - * it is probably best to fail and let the user install EGD or a similar program if no kernel random device is available. - * According to OpenSSL RAND_egd(3): "The automatic query of /var/run/egd-pool et al was added in OpenSSL 0.9.7"; + * it is probably best to fail and let the user install EGD or + * a similar program if no kernel random device is available. + * According to OpenSSL RAND_egd(3): "The automatic query of + * /var/run/egd-pool et al was added in OpenSSL 0.9.7"; * so it makes little sense to deal with PRNGD seeding ourselves. */ array_free(&Conf_SSLOptions.ListenPorts); @@ -305,7 +307,8 @@ ConnSSL_InitLibrary( void ) SSL_CTX_set_options(newctx, SSL_OP_SINGLE_DH_USE|SSL_OP_NO_SSLv2); SSL_CTX_set_mode(newctx, SSL_MODE_ENABLE_PARTIAL_WRITE); - SSL_CTX_set_verify(newctx, SSL_VERIFY_PEER|SSL_VERIFY_CLIENT_ONCE, Verify_openssl); + SSL_CTX_set_verify(newctx, SSL_VERIFY_PEER|SSL_VERIFY_CLIENT_ONCE, + Verify_openssl); SSL_CTX_free(ssl_ctx); ssl_ctx = newctx; Log(LOG_INFO, "%s initialized.", SSLeay_version(SSLEAY_VERSION)); @@ -318,12 +321,17 @@ out: #ifdef HAVE_LIBGNUTLS int err; static bool initialized; - if (initialized) /* TODO: cannot reload gnutls keys: can't simply free x509 context -- it may still be in use */ + + if (initialized) { + /* TODO: cannot reload gnutls keys: can't simply free x509 + * context -- it may still be in use */ return false; + } err = gnutls_global_init(); if (err) { - Log(LOG_ERR, "Failed to initialize GnuTLS: %s", gnutls_strerror(err)); + Log(LOG_ERR, "Failed to initialize GnuTLS: %s", + gnutls_strerror(err)); array_free(&Conf_SSLOptions.ListenPorts); return false; } -- cgit 1.4.1 From 84ed46d4c1caaa4ec79a6223c35785afcf1c9d53 Mon Sep 17 00:00:00 2001 From: Alexander Barton Date: Sun, 15 Sep 2013 15:09:36 +0200 Subject: Cipher list selection for OpenSSL This patch introduces the possibility to arbitrarily select ciphers which should be promoted resp. declined when establishing a SSL connection with a client by implementing the new configuration option "CipherList". By default, OpenSSL would accept low and medium strength and RC-4 ciphers, which nowadays are known to be broken. This patch only implements the feature for OpenSSL. A GnuTLS counterpart has to be implemented in another patch ... Original patch by Bastian . Closes bug #162. --- doc/sample-ngircd.conf.tmpl | 7 +++++++ man/ngircd.conf.5.tmpl | 7 +++++++ src/ngircd/conf.c | 10 ++++++++++ src/ngircd/conf.h | 1 + src/ngircd/conn-ssl.c | 24 +++++++++++++++++++++++- 5 files changed, 48 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/doc/sample-ngircd.conf.tmpl b/doc/sample-ngircd.conf.tmpl index ae1b2139..a4dbf869 100644 --- a/doc/sample-ngircd.conf.tmpl +++ b/doc/sample-ngircd.conf.tmpl @@ -248,6 +248,13 @@ # SSL Server Key Certificate ;CertFile = :ETCDIR:/ssl/server-cert.pem + # Select cipher suites allowed for SSL/TLS connections (OpenSSL only). + # This defaults to the empty string, so all supported ciphers are + # allowed. Please see 'man 1ssl ciphers' for details. + # The example below only allows "high strength" cipher suites, disables + # the ones without authentication, and sorts by strength: + ;CipherList = HIGH:!aNULL:@STRENGTH + # Diffie-Hellman parameters ;DHFile = :ETCDIR:/ssl/dhparams.pem diff --git a/man/ngircd.conf.5.tmpl b/man/ngircd.conf.5.tmpl index cf926f9a..263dec04 100644 --- a/man/ngircd.conf.5.tmpl +++ b/man/ngircd.conf.5.tmpl @@ -366,6 +366,13 @@ when it is compiled with support for SSL using OpenSSL or GnuTLS! \fBCertFile\fR (string) SSL Certificate file of the private server key. .TP +\fBCipherList\fR (string) +OpenSSL only: Select cipher suites allowed for SSL/TLS connections. This +defaults to the empty string, so all supported ciphers are allowed. Please see +'man 1ssl ciphers' for details. This setting allows only "high strength" cipher +suites, disables the ones without authentication, and sorts by strength, for +example: "HIGH:!aNULL:@STRENGTH". +.TP \fBDHFile\fR (string) Name of the Diffie-Hellman Parameter file. Can be created with GnuTLS "certtool \-\-generate-dh-params" or "openssl dhparam". If this file is not diff --git a/src/ngircd/conf.c b/src/ngircd/conf.c index b10f4905..9ab66e54 100644 --- a/src/ngircd/conf.c +++ b/src/ngircd/conf.c @@ -117,6 +117,9 @@ ConfSSL_Init(void) array_free_wipe(&Conf_SSLOptions.KeyFilePassword); array_free(&Conf_SSLOptions.ListenPorts); + + free(Conf_SSLOptions.CipherList); + Conf_SSLOptions.CipherList = NULL; } /** @@ -432,6 +435,8 @@ Conf_Test( void ) puts("[SSL]"); printf(" CertFile = %s\n", Conf_SSLOptions.CertFile ? Conf_SSLOptions.CertFile : ""); + printf(" CipherList = %s\n", Conf_SSLOptions.CipherList + ? Conf_SSLOptions.CipherList : ""); printf(" DHFile = %s\n", Conf_SSLOptions.DHFile ? Conf_SSLOptions.DHFile : ""); printf(" KeyFile = %s\n", Conf_SSLOptions.KeyFile @@ -1869,6 +1874,11 @@ Handle_SSL(const char *File, int Line, char *Var, char *Arg) ports_parse(&Conf_SSLOptions.ListenPorts, Line, Arg); return; } + if (strcasecmp(Var, "CipherList") == 0) { + assert(Conf_SSLOptions.CipherList == NULL); + Conf_SSLOptions.CipherList = strdup_warn(Arg); + return; + } Config_Error_Section(File, Line, Var, "SSL"); } diff --git a/src/ngircd/conf.h b/src/ngircd/conf.h index 948749de..02d23315 100644 --- a/src/ngircd/conf.h +++ b/src/ngircd/conf.h @@ -75,6 +75,7 @@ struct SSLOptions { char *DHFile; /**< File containing DH parameters */ array ListenPorts; /**< Array of listening SSL ports */ array KeyFilePassword; /**< Key file password */ + char *CipherList; /**< Set SSL cipher list to use */ }; #endif diff --git a/src/ngircd/conn-ssl.c b/src/ngircd/conn-ssl.c index 595cb615..059e871d 100644 --- a/src/ngircd/conn-ssl.c +++ b/src/ngircd/conn-ssl.c @@ -305,6 +305,19 @@ ConnSSL_InitLibrary( void ) if (!ConnSSL_LoadServerKey_openssl(newctx)) goto out; + if(Conf_SSLOptions.CipherList && *Conf_SSLOptions.CipherList) { + if(SSL_CTX_set_cipher_list(newctx, Conf_SSLOptions.CipherList) == 0 ) { + Log(LOG_ERR, + "Failed to apply SSL cipher list \"%s\"!", + Conf_SSLOptions.CipherList); + goto out; + } else { + Log(LOG_INFO, + "Successfully applied SSL cipher list: \"%s\".", + Conf_SSLOptions.CipherList); + } + } + SSL_CTX_set_options(newctx, SSL_OP_SINGLE_DH_USE|SSL_OP_NO_SSLv2); SSL_CTX_set_mode(newctx, SSL_MODE_ENABLE_PARTIAL_WRITE); SSL_CTX_set_verify(newctx, SSL_VERIFY_PEER|SSL_VERIFY_CLIENT_ONCE, @@ -328,6 +341,14 @@ out: return false; } + if(Conf_SSLOptions.CipherList != NULL) { + Log(LOG_ERR, + "Failed to apply SSL cipher list \"%s\": Not implemented for GnuTLS!", + Conf_SSLOptions.CipherList); + array_free(&Conf_SSLOptions.ListenPorts); + return false; + } + err = gnutls_global_init(); if (err) { Log(LOG_ERR, "Failed to initialize GnuTLS: %s", @@ -339,6 +360,7 @@ out: array_free(&Conf_SSLOptions.ListenPorts); return false; } + Log(LOG_INFO, "GnuTLS %s initialized.", gnutls_check_version(NULL)); initialized = true; return true; @@ -368,7 +390,7 @@ ConnSSL_LoadServerKey_gnutls(void) if (array_bytes(&Conf_SSLOptions.KeyFilePassword)) Log(LOG_WARNING, - "Ignoring KeyFilePassword: Not supported by GnuTLS."); + "Ignoring SSL \"KeyFilePassword\": Not supported by GnuTLS."); if (!Load_DH_params()) return false; -- cgit 1.4.1 From 51231ac8d45bf329f4724a145e6bc7a3ea118570 Mon Sep 17 00:00:00 2001 From: Alexander Barton Date: Sun, 15 Sep 2013 17:35:52 +0200 Subject: ConnSSL_Init_SSL(): correctly set CONN_SSL flag The CONN_SSL flag must be set before any calls to ConnSSL_Free()! --- src/ngircd/conn-ssl.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/ngircd/conn-ssl.c b/src/ngircd/conn-ssl.c index 059e871d..a9789675 100644 --- a/src/ngircd/conn-ssl.c +++ b/src/ngircd/conn-ssl.c @@ -460,7 +460,10 @@ static bool ConnSSL_Init_SSL(CONNECTION *c) { int ret; + + LogDebug("Initializing SSL ..."); assert(c != NULL); + #ifdef HAVE_LIBSSL if (!ssl_ctx) { Log(LOG_ERR, @@ -475,6 +478,7 @@ ConnSSL_Init_SSL(CONNECTION *c) LogOpenSSLError("Failed to create SSL structure", NULL); return false; } + Conn_OPTION_ADD(c, CONN_SSL); ret = SSL_set_fd(c->ssl_state.ssl, c->sock); if (ret != 1) { @@ -484,8 +488,9 @@ ConnSSL_Init_SSL(CONNECTION *c) } #endif #ifdef HAVE_LIBGNUTLS + Conn_OPTION_ADD(c, CONN_SSL); ret = gnutls_set_default_priority(c->ssl_state.gnutls_session); - if (ret < 0) { + if (ret != 0) { Log(LOG_ERR, "Failed to set GnuTLS default priority: %s", gnutls_strerror(ret)); ConnSSL_Free(c); @@ -497,17 +502,20 @@ ConnSSL_Init_SSL(CONNECTION *c) * There doesn't seem to be an alternate GNUTLS API we could use instead, see e.g. * http://www.mail-archive.com/help-gnutls@gnu.org/msg00286.html */ - gnutls_transport_set_ptr(c->ssl_state.gnutls_session, (gnutls_transport_ptr_t) (long) c->sock); - gnutls_certificate_server_set_request(c->ssl_state.gnutls_session, GNUTLS_CERT_REQUEST); - ret = gnutls_credentials_set(c->ssl_state.gnutls_session, GNUTLS_CRD_CERTIFICATE, x509_cred); - if (ret < 0) { - Log(LOG_ERR, "Failed to set SSL credentials: %s", gnutls_strerror(ret)); + gnutls_transport_set_ptr(c->ssl_state.gnutls_session, + (gnutls_transport_ptr_t) (long) c->sock); + gnutls_certificate_server_set_request(c->ssl_state.gnutls_session, + GNUTLS_CERT_REQUEST); + ret = gnutls_credentials_set(c->ssl_state.gnutls_session, + GNUTLS_CRD_CERTIFICATE, x509_cred); + if (ret != 0) { + Log(LOG_ERR, "Failed to set SSL credentials: %s", + gnutls_strerror(ret)); ConnSSL_Free(c); return false; } gnutls_dh_set_prime_bits(c->ssl_state.gnutls_session, DH_BITS_MIN); #endif - Conn_OPTION_ADD(c, CONN_SSL); return true; } @@ -675,7 +683,6 @@ ConnSSL_Accept( CONNECTION *c ) return false; } #endif - LogDebug("Initializing SSL data ..."); if (!ConnSSL_Init_SSL(c)) return -1; } -- cgit 1.4.1 From b9006acee3649600226652a8361f13c859726cf2 Mon Sep 17 00:00:00 2001 From: Alexander Barton Date: Sun, 15 Sep 2013 17:57:41 +0200 Subject: Cipher list selection for GnuTLS This patch implements the missing functionality for cipher list selection using GnuTLS (our OpenSSL code has this already). --- doc/sample-ngircd.conf.tmpl | 14 ++++++++----- man/ngircd.conf.5.tmpl | 12 ++++++----- src/ngircd/conn-ssl.c | 49 +++++++++++++++++++++++++++++---------------- 3 files changed, 48 insertions(+), 27 deletions(-) (limited to 'src') diff --git a/doc/sample-ngircd.conf.tmpl b/doc/sample-ngircd.conf.tmpl index a4dbf869..1bdf01ee 100644 --- a/doc/sample-ngircd.conf.tmpl +++ b/doc/sample-ngircd.conf.tmpl @@ -248,12 +248,16 @@ # SSL Server Key Certificate ;CertFile = :ETCDIR:/ssl/server-cert.pem - # Select cipher suites allowed for SSL/TLS connections (OpenSSL only). - # This defaults to the empty string, so all supported ciphers are - # allowed. Please see 'man 1ssl ciphers' for details. - # The example below only allows "high strength" cipher suites, disables - # the ones without authentication, and sorts by strength: + # Select cipher suites allowed for SSL/TLS connections. This defaults + # to the empty string, so all supported ciphers are allowed. Please + # see 'man 1ssl ciphers' (OpenSSL) and 'man 3 gnutls_priority_init' + # (GnuTLS) for details. + # For example, this setting allows only "high strength" cipher suites, + # disables the ones without authentication, and sorts by strength: + # For OpenSSL: ;CipherList = HIGH:!aNULL:@STRENGTH + # For GnuTLS: + ;CipherList = SECURE128 # Diffie-Hellman parameters ;DHFile = :ETCDIR:/ssl/dhparams.pem diff --git a/man/ngircd.conf.5.tmpl b/man/ngircd.conf.5.tmpl index 263dec04..862c1424 100644 --- a/man/ngircd.conf.5.tmpl +++ b/man/ngircd.conf.5.tmpl @@ -367,11 +367,13 @@ when it is compiled with support for SSL using OpenSSL or GnuTLS! SSL Certificate file of the private server key. .TP \fBCipherList\fR (string) -OpenSSL only: Select cipher suites allowed for SSL/TLS connections. This -defaults to the empty string, so all supported ciphers are allowed. Please see -'man 1ssl ciphers' for details. This setting allows only "high strength" cipher -suites, disables the ones without authentication, and sorts by strength, for -example: "HIGH:!aNULL:@STRENGTH". +Select cipher suites allowed for SSL/TLS connections. This defaults to the +empty string, so all supported ciphers are allowed. +Please see 'man 1ssl ciphers' (OpenSSL) and 'man 3 gnutls_priority_init' +(GnuTLS) for details. +For example, this setting allows only "high strength" cipher suites, disables +the ones without authentication, and sorts by strength: +"HIGH:!aNULL:@STRENGTH" (OpenSSL), "SECURE128" (GnuTLS). .TP \fBDHFile\fR (string) Name of the Diffie-Hellman Parameter file. Can be created with GnuTLS diff --git a/src/ngircd/conn-ssl.c b/src/ngircd/conn-ssl.c index a9789675..b16c6b94 100644 --- a/src/ngircd/conn-ssl.c +++ b/src/ngircd/conn-ssl.c @@ -58,6 +58,7 @@ static bool ConnSSL_LoadServerKey_openssl PARAMS(( SSL_CTX *c )); static gnutls_certificate_credentials_t x509_cred; static gnutls_dh_params_t dh_params; +static gnutls_priority_t priorities_cache; static bool ConnSSL_LoadServerKey_gnutls PARAMS(( void )); #endif @@ -308,12 +309,12 @@ ConnSSL_InitLibrary( void ) if(Conf_SSLOptions.CipherList && *Conf_SSLOptions.CipherList) { if(SSL_CTX_set_cipher_list(newctx, Conf_SSLOptions.CipherList) == 0 ) { Log(LOG_ERR, - "Failed to apply SSL cipher list \"%s\"!", + "Failed to apply OpenSSL cipher list \"%s\"!", Conf_SSLOptions.CipherList); goto out; } else { Log(LOG_INFO, - "Successfully applied SSL cipher list: \"%s\".", + "Successfully applied OpenSSL cipher list \"%s\".", Conf_SSLOptions.CipherList); } } @@ -341,29 +342,43 @@ out: return false; } - if(Conf_SSLOptions.CipherList != NULL) { - Log(LOG_ERR, - "Failed to apply SSL cipher list \"%s\": Not implemented for GnuTLS!", - Conf_SSLOptions.CipherList); - array_free(&Conf_SSLOptions.ListenPorts); - return false; - } - err = gnutls_global_init(); if (err) { Log(LOG_ERR, "Failed to initialize GnuTLS: %s", gnutls_strerror(err)); - array_free(&Conf_SSLOptions.ListenPorts); - return false; + goto out; } - if (!ConnSSL_LoadServerKey_gnutls()) { - array_free(&Conf_SSLOptions.ListenPorts); - return false; + + if (!ConnSSL_LoadServerKey_gnutls()) + goto out; + + if(Conf_SSLOptions.CipherList && *Conf_SSLOptions.CipherList) { + err = gnutls_priority_init(&priorities_cache, + Conf_SSLOptions.CipherList, NULL); + if (err != GNUTLS_E_SUCCESS) { + Log(LOG_ERR, + "Failed to apply GnuTLS cipher list \"%s\"!", + Conf_SSLOptions.CipherList); + goto out; + } + Log(LOG_INFO, + "Successfully applied GnuTLS cipher list \"%s\".", + Conf_SSLOptions.CipherList); + } else { + err = gnutls_priority_init(&priorities_cache, "NORMAL", NULL); + if (err != GNUTLS_E_SUCCESS) { + Log(LOG_ERR, + "Failed to apply GnuTLS cipher list \"NORMAL\"!"); + goto out; + } } Log(LOG_INFO, "GnuTLS %s initialized.", gnutls_check_version(NULL)); initialized = true; return true; +out: + array_free(&Conf_SSLOptions.ListenPorts); + return false; #endif } @@ -489,9 +504,9 @@ ConnSSL_Init_SSL(CONNECTION *c) #endif #ifdef HAVE_LIBGNUTLS Conn_OPTION_ADD(c, CONN_SSL); - ret = gnutls_set_default_priority(c->ssl_state.gnutls_session); + ret = gnutls_priority_set(c->ssl_state.gnutls_session, priorities_cache); if (ret != 0) { - Log(LOG_ERR, "Failed to set GnuTLS default priority: %s", + Log(LOG_ERR, "Failed to set GnuTLS session priorities: %s", gnutls_strerror(ret)); ConnSSL_Free(c); return false; -- cgit 1.4.1