From 07304b5193b5840abc37207f22a54ca101361bde Mon Sep 17 00:00:00 2001 From: Marvin Scholz Date: Sat, 20 Apr 2019 19:37:24 +0200 Subject: [PATCH 1/6] Update: Do not init OpenSSL since 1.1.0 Explicitly initializing the library is not longer needed since OpenSSL 1.1.0 and the SSL_library_init function is deprecated. Citing the manual: > As of version 1.1.0 OpenSSL will automatically allocate all resources > that it needs so no explicit initialisation is required. Similarly it > will also automatically deinitialise as required. Fix #2318 --- src/tls.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tls.c b/src/tls.c index be858f05..ef22e272 100644 --- a/src/tls.c +++ b/src/tls.c @@ -56,8 +56,10 @@ struct tls_tag { void tls_initialize(void) { +#if OPENSSL_VERSION_NUMBER < 0x10100000L SSL_load_error_strings(); /* readable error messages */ SSL_library_init(); /* initialize library */ +#endif } void tls_shutdown(void) { From ed9a4e658c915203098c2419da62f18891f9c472 Mon Sep 17 00:00:00 2001 From: Marvin Scholz Date: Sat, 20 Apr 2019 19:39:23 +0200 Subject: [PATCH 2/6] Cleanup: Simplify OpenSSL context initialisation Assigning the return value of SSLv23_server_method to a variable is not necessary here and not doing it can get us rid of a lot of condition code given that the type of the return value changed at some point. --- src/tls.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/tls.c b/src/tls.c index ef22e272..da4fa8f7 100644 --- a/src/tls.c +++ b/src/tls.c @@ -68,11 +68,6 @@ void tls_shutdown(void) tls_ctx_t *tls_ctx_new(const char *cert_file, const char *key_file, const char *cipher_list) { tls_ctx_t *ctx; -#if OPENSSL_VERSION_NUMBER < 0x1000114fL - SSL_METHOD *method; -#else - const SSL_METHOD *method; -#endif long ssl_opts; if (!cert_file || !key_file || !cipher_list) @@ -82,10 +77,8 @@ tls_ctx_t *tls_ctx_new(const char *cert_file, const char *key_file, const char * if (!ctx) return NULL; - method = SSLv23_server_method(); - ctx->refc = 1; - ctx->ctx = SSL_CTX_new(method); + ctx->ctx = SSL_CTX_new(SSLv23_server_method()); ssl_opts = SSL_CTX_get_options(ctx->ctx); #ifdef SSL_OP_NO_COMPRESSION From 14ba90fc936b836133b91355b3a0e38b5a4df621 Mon Sep 17 00:00:00 2001 From: Marvin Scholz Date: Sat, 20 Apr 2019 19:51:02 +0200 Subject: [PATCH 3/6] Cleanup: Simplify adding SSL_OP_NO_COMPRESSION --- src/tls.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tls.c b/src/tls.c index da4fa8f7..b2a6f364 100644 --- a/src/tls.c +++ b/src/tls.c @@ -81,12 +81,12 @@ tls_ctx_t *tls_ctx_new(const char *cert_file, const char *key_file, const char * ctx->ctx = SSL_CTX_new(SSLv23_server_method()); ssl_opts = SSL_CTX_get_options(ctx->ctx); + ssl_opts |= SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; // Disable SSLv2 and SSLv3 #ifdef SSL_OP_NO_COMPRESSION - SSL_CTX_set_options(ctx->ctx, ssl_opts|SSL_OP_NO_SSLv2|SSL_OP_NO_SSLv3|SSL_OP_NO_COMPRESSION); -#else - SSL_CTX_set_options(ctx->ctx, ssl_opts|SSL_OP_NO_SSLv2|SSL_OP_NO_SSLv3); + ssl_opts |= SSL_OP_NO_COMPRESSION; // Never use compression #endif + SSL_CTX_set_options(ctx->ctx, ssl_opts); do { if (SSL_CTX_use_certificate_chain_file(ctx->ctx, cert_file) <= 0) { ICECAST_LOG_WARN("Invalid cert file %s", cert_file); From e824e48fdf188b02a4d8f3cb2b6b54f44394f9d3 Mon Sep 17 00:00:00 2001 From: Marvin Scholz Date: Sat, 20 Apr 2019 19:53:49 +0200 Subject: [PATCH 4/6] Cleanup: Remove unnecessary SSL_CTX_get_options According to the documentation the current option state is not cleared but the options are added to the current options, so gettin the current options seems redundant to the behavior of SSL_CTX_set_options: > SSL_CTX_set_options() adds the options set via bitmask in options > to ctx. Options already set before are not cleared! --- src/tls.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/tls.c b/src/tls.c index b2a6f364..729564e8 100644 --- a/src/tls.c +++ b/src/tls.c @@ -80,12 +80,16 @@ tls_ctx_t *tls_ctx_new(const char *cert_file, const char *key_file, const char * ctx->refc = 1; ctx->ctx = SSL_CTX_new(SSLv23_server_method()); - ssl_opts = SSL_CTX_get_options(ctx->ctx); - ssl_opts |= SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; // Disable SSLv2 and SSLv3 + ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; // Disable SSLv2 and SSLv3 #ifdef SSL_OP_NO_COMPRESSION ssl_opts |= SSL_OP_NO_COMPRESSION; // Never use compression #endif + /* Even though this function is called set, it adds the + * flags to the already existing flags (possibly default + * flags already set by OpenSSL)! + * Calling SSL_CTX_get_options is not needed here, therefore. + */ SSL_CTX_set_options(ctx->ctx, ssl_opts); do { if (SSL_CTX_use_certificate_chain_file(ctx->ctx, cert_file) <= 0) { From e09f48a0342ed433a7ed35a5c0b2f59b298ebfd6 Mon Sep 17 00:00:00 2001 From: Marvin Scholz Date: Sat, 20 Apr 2019 20:02:25 +0200 Subject: [PATCH 5/6] Update: Do not use SSLv23_server_method ...with OpenSSL 1.1.0 or newer Instead use TLS_server_method and SSL_CTX_set_min_proto_version to limit the used protocol versions. --- src/tls.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tls.c b/src/tls.c index 729564e8..b268cbae 100644 --- a/src/tls.c +++ b/src/tls.c @@ -68,7 +68,7 @@ void tls_shutdown(void) tls_ctx_t *tls_ctx_new(const char *cert_file, const char *key_file, const char *cipher_list) { tls_ctx_t *ctx; - long ssl_opts; + long ssl_opts = 0; if (!cert_file || !key_file || !cipher_list) return NULL; @@ -78,9 +78,15 @@ tls_ctx_t *tls_ctx_new(const char *cert_file, const char *key_file, const char * return NULL; ctx->refc = 1; - ctx->ctx = SSL_CTX_new(SSLv23_server_method()); +#if OPENSSL_VERSION_NUMBER < 0x10100000L + ctx->ctx = SSL_CTX_new(SSLv23_server_method()); ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; // Disable SSLv2 and SSLv3 +#else + ctx->ctx = SSL_CTX_new(TLS_server_method()); + SSL_CTX_set_min_proto_version(ctx->ctx, TLS1_VERSION); +#endif + #ifdef SSL_OP_NO_COMPRESSION ssl_opts |= SSL_OP_NO_COMPRESSION; // Never use compression #endif From 8b68c462c47f091fec3729344e4fcec68dbd8bf3 Mon Sep 17 00:00:00 2001 From: Marvin Scholz Date: Sat, 20 Apr 2019 21:37:59 +0200 Subject: [PATCH 6/6] Fix: Free strings with older OpenSSL versions --- src/tls.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tls.c b/src/tls.c index b268cbae..379000d9 100644 --- a/src/tls.c +++ b/src/tls.c @@ -61,8 +61,12 @@ void tls_initialize(void) SSL_library_init(); /* initialize library */ #endif } + void tls_shutdown(void) { +#if OPENSSL_VERSION_NUMBER < 0x10100000L + ERR_free_strings(); +#endif } tls_ctx_t *tls_ctx_new(const char *cert_file, const char *key_file, const char *cipher_list)