From f16399777776966fb27e9c46e269f8e43a07b40a Mon Sep 17 00:00:00 2001 From: Alex Smith <44322503+MadAlexUK@users.noreply.github.com> Date: Sat, 20 Oct 2018 18:03:59 +0100 Subject: [PATCH 1/7] Added a mailstream_ssl_set_server_name() function This allows clients to enable Server Name Indication on a TLS session by supplying the server name to be used, via a call within the session open callback. --- src/data-types/mailstream_ssl.c | 21 +++++++++++++++++++++ src/data-types/mailstream_ssl.h | 4 ++++ 2 files changed, 25 insertions(+) diff --git a/src/data-types/mailstream_ssl.c b/src/data-types/mailstream_ssl.c index cb0a17eb..ef87d063 100644 --- a/src/data-types/mailstream_ssl.c +++ b/src/data-types/mailstream_ssl.c @@ -1361,6 +1361,27 @@ int mailstream_ssl_set_server_certicate(struct mailstream_ssl_context * ssl_cont #endif /* USE_SSL */ } +LIBETPAN_EXPORT +int mailstream_ssl_set_server_name(struct mailstream_ssl_context * ssl_context, + char * hostname) +{ + int r = -1; + +#ifdef USE_SSL +# ifdef USE_GNUTLS + r = gnutls_server_name_set(ssl_context->session, GNUTLS_NAME_DNS, hostname, strlen(hostname)); +# else /* !USE_GNUTLS */ +# if (OPENSSL_VERSION_NUMBER >= 0x10000000L) + if (SSL_set_tlsext_host_name(ssl_context->openssl_ssl_ctx, hostname)) { + r = 0; + } +# endif /* (OPENSSL_VERSION_NUMBER >= 0x10000000L) */ +# endif /* !USE_GNUTLS */ +#endif /* USE_SSL */ + + return r; +} + #ifdef USE_SSL #ifndef USE_GNUTLS static struct mailstream_ssl_context * mailstream_ssl_context_new(SSL_CTX * open_ssl_ctx, int fd) diff --git a/src/data-types/mailstream_ssl.h b/src/data-types/mailstream_ssl.h index 885dc7e7..fbf2642a 100644 --- a/src/data-types/mailstream_ssl.h +++ b/src/data-types/mailstream_ssl.h @@ -123,6 +123,10 @@ LIBETPAN_EXPORT int mailstream_ssl_set_server_certicate(struct mailstream_ssl_context * ssl_context, char * CAfile, char * CApath); +LIBETPAN_EXPORT +int mailstream_ssl_set_server_name(struct mailstream_ssl_context * ssl_context, + char * hostname); + LIBETPAN_EXPORT void * mailstream_ssl_get_openssl_ssl_ctx(struct mailstream_ssl_context * ssl_context); From abc0f2bce68913f8bffe1da0351c491e8d10c067 Mon Sep 17 00:00:00 2001 From: Alex Smith <44322503+MadAlexUK@users.noreply.github.com> Date: Sat, 20 Oct 2018 18:51:36 +0100 Subject: [PATCH 2/7] Documented mailstream_ssl_set_server_name() --- doc/API.sgml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/doc/API.sgml b/doc/API.sgml index d5c37a09..0f68d908 100644 --- a/doc/API.sgml +++ b/doc/API.sgml @@ -1213,6 +1213,22 @@ mailstream * mailstream_ssl_open(int fd); mailstream_ssl_open() will open a TLS/SSL socket. + + +int mailstream_ssl_set_server_name(struct mailstream_ssl_context * ssl_context, + char * hostname) + + + + mailstream_ssl_set_server_name() allows the client + to enable the use of the Server Name Indication TLS extension upon + opening a TLS stream, by providing the domain name to be indicated + to server as the desired destination. ssl_context + is the context pointer passed to the client-supplied callback + function by mailstream_ssl_open_with_callback() + etc. Note that hostname must be a domain name, not + a string representation of an IP address. + From 96bc195bbd00ae270cc8f1838dd1a238022b7d03 Mon Sep 17 00:00:00 2001 From: Alex Smith <44322503+MadAlexUK@users.noreply.github.com> Date: Sat, 20 Oct 2018 21:12:11 +0100 Subject: [PATCH 3/7] Corrected the setting of SNI with openssl Made mailstream_ssl_set_server_name() just store the server name in the context structure when we're using openssl, since the openssl function to set the server name to send needs to be called on the full SSL session, which we don't open until after the callback returns, not the SSL context. --- src/data-types/mailstream_ssl.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/data-types/mailstream_ssl.c b/src/data-types/mailstream_ssl.c index ef87d063..9f846389 100644 --- a/src/data-types/mailstream_ssl.c +++ b/src/data-types/mailstream_ssl.c @@ -113,6 +113,7 @@ struct mailstream_ssl_context SSL_CTX * openssl_ssl_ctx; X509* client_x509; EVP_PKEY *client_pkey; + char * server_name; #else gnutls_session session; gnutls_x509_crt client_x509; @@ -463,7 +464,13 @@ static struct mailstream_ssl_data * ssl_data_new_full(int fd, time_t timeout, if (ssl_conn == NULL) goto free_ctx; - + + if (ssl_context->server_name != NULL) { + SSL_set_tlsext_host_name(ssl_conn, ssl_context->server_name); + free(ssl_context->server_name); + ssl_context->server_name = NULL; + } + if (SSL_set_fd(ssl_conn, fd) == 0) goto free_ssl_conn; @@ -1372,9 +1379,13 @@ int mailstream_ssl_set_server_name(struct mailstream_ssl_context * ssl_context, r = gnutls_server_name_set(ssl_context->session, GNUTLS_NAME_DNS, hostname, strlen(hostname)); # else /* !USE_GNUTLS */ # if (OPENSSL_VERSION_NUMBER >= 0x10000000L) - if (SSL_set_tlsext_host_name(ssl_context->openssl_ssl_ctx, hostname)) { - r = 0; + if (hostname != NULL) { + ssl_context->server_name = strdup(hostname); } + else { + ssl_context->server_name[0] = '\0'; + } + r = 0; # endif /* (OPENSSL_VERSION_NUMBER >= 0x10000000L) */ # endif /* !USE_GNUTLS */ #endif /* USE_SSL */ @@ -1395,6 +1406,7 @@ static struct mailstream_ssl_context * mailstream_ssl_context_new(SSL_CTX * open ssl_ctx->openssl_ssl_ctx = open_ssl_ctx; ssl_ctx->client_x509 = NULL; ssl_ctx->client_pkey = NULL; + ssl_ctx->server_name = NULL; ssl_ctx->fd = fd; return ssl_ctx; @@ -1402,8 +1414,12 @@ static struct mailstream_ssl_context * mailstream_ssl_context_new(SSL_CTX * open static void mailstream_ssl_context_free(struct mailstream_ssl_context * ssl_ctx) { - if (ssl_ctx) + if (ssl_ctx != NULL) { + if (ssl_ctx->server_name != NULL) { + free(ssl_ctx->server_name); + } free(ssl_ctx); + } } #else static struct mailstream_ssl_context * mailstream_ssl_context_new(gnutls_session session, int fd) From 073e83af0adf7d7edd4c4b058aee38dcc24aa4bc Mon Sep 17 00:00:00 2001 From: Alex Smith <44322503+MadAlexUK@users.noreply.github.com> Date: Sat, 20 Oct 2018 21:18:43 +0100 Subject: [PATCH 4/7] Minor mailstream_ssl_set_server_name() safety improvement Made mailstream_ssl_set_server_name() clear any previously set server name safely when given a NULL name pointer in the case that we're built against gnutls. --- src/data-types/mailstream_ssl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/data-types/mailstream_ssl.c b/src/data-types/mailstream_ssl.c index 9f846389..96f0ddc6 100644 --- a/src/data-types/mailstream_ssl.c +++ b/src/data-types/mailstream_ssl.c @@ -1376,7 +1376,12 @@ int mailstream_ssl_set_server_name(struct mailstream_ssl_context * ssl_context, #ifdef USE_SSL # ifdef USE_GNUTLS - r = gnutls_server_name_set(ssl_context->session, GNUTLS_NAME_DNS, hostname, strlen(hostname)); + if (hostname != NULL) { + r = gnutls_server_name_set(ssl_context->session, GNUTLS_NAME_DNS, hostname, strlen(hostname)); + } + else { + r = gnutls_server_name_set(ssl_context->session, GNUTLS_NAME_DNS, "", 0U); + } # else /* !USE_GNUTLS */ # if (OPENSSL_VERSION_NUMBER >= 0x10000000L) if (hostname != NULL) { From ee715ddb12e1b953b8ea6e5cbbdc3cf64cc8eed4 Mon Sep 17 00:00:00 2001 From: Alex Smith <44322503+MadAlexUK@users.noreply.github.com> Date: Sat, 20 Oct 2018 23:07:28 +0100 Subject: [PATCH 5/7] Fixed the NULL host name case when openssl is used Corrected the handling of a NULL host name pointer being handed to mailstream_ssl_set_server_name() when we're using openssl. --- src/data-types/mailstream_ssl.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/data-types/mailstream_ssl.c b/src/data-types/mailstream_ssl.c index 96f0ddc6..5a95692c 100644 --- a/src/data-types/mailstream_ssl.c +++ b/src/data-types/mailstream_ssl.c @@ -1388,7 +1388,10 @@ int mailstream_ssl_set_server_name(struct mailstream_ssl_context * ssl_context, ssl_context->server_name = strdup(hostname); } else { - ssl_context->server_name[0] = '\0'; + if (ssl_context->server_name != NULL) { + free(ssl_context->server_name); + } + ssl_context->server_name = NULL; } r = 0; # endif /* (OPENSSL_VERSION_NUMBER >= 0x10000000L) */ From 11183d5cf2749b7d2c6cfca03d038b20348d7452 Mon Sep 17 00:00:00 2001 From: Alex Smith <44322503+MadAlexUK@users.noreply.github.com> Date: Sat, 20 Oct 2018 23:15:26 +0100 Subject: [PATCH 6/7] Commented the copy of the host name in the openssl case Added a comment explaining why mailstream_ssl_set_server_name() takes a copy of the pass host name when we are using openssl, holding on to it in our context structure, and noting where it is passed to openssl and our copy freed. --- src/data-types/mailstream_ssl.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/data-types/mailstream_ssl.c b/src/data-types/mailstream_ssl.c index 5a95692c..561e8aa9 100644 --- a/src/data-types/mailstream_ssl.c +++ b/src/data-types/mailstream_ssl.c @@ -1385,6 +1385,14 @@ int mailstream_ssl_set_server_name(struct mailstream_ssl_context * ssl_context, # else /* !USE_GNUTLS */ # if (OPENSSL_VERSION_NUMBER >= 0x10000000L) if (hostname != NULL) { + /* Unfortunately we can't set this in the openssl session yet since it + * hasn't been created yet; we only have the openssl context at this point. + * We will set it in the openssl session when we create it, soon after the + * client callback that we expect to be calling us (since it is the way the + * client gets our mailstream_ssl_context) returns (see + * ssl_data_new_full()) but we cannot rely on the client persisting it. We + * must therefore take a temporary copy here, which we free once we've set + * it in the openssl session. */ ssl_context->server_name = strdup(hostname); } else { From e3071d43c1db7b21c5fc53eaa7004b434156a518 Mon Sep 17 00:00:00 2001 From: Alex Smith <44322503+MadAlexUK@users.noreply.github.com> Date: Sun, 21 Oct 2018 13:02:56 +0100 Subject: [PATCH 7/7] Guarded all the openssl SNI code with the necessary version check --- src/data-types/mailstream_ssl.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/data-types/mailstream_ssl.c b/src/data-types/mailstream_ssl.c index 561e8aa9..2c1a7441 100644 --- a/src/data-types/mailstream_ssl.c +++ b/src/data-types/mailstream_ssl.c @@ -113,7 +113,9 @@ struct mailstream_ssl_context SSL_CTX * openssl_ssl_ctx; X509* client_x509; EVP_PKEY *client_pkey; +# if (OPENSSL_VERSION_NUMBER >= 0x10000000L) char * server_name; +# endif /* (OPENSSL_VERSION_NUMBER >= 0x10000000L) */ #else gnutls_session session; gnutls_x509_crt client_x509; @@ -465,11 +467,13 @@ static struct mailstream_ssl_data * ssl_data_new_full(int fd, time_t timeout, if (ssl_conn == NULL) goto free_ctx; +#if (OPENSSL_VERSION_NUMBER >= 0x10000000L) if (ssl_context->server_name != NULL) { SSL_set_tlsext_host_name(ssl_conn, ssl_context->server_name); free(ssl_context->server_name); ssl_context->server_name = NULL; } +#endif /* (OPENSSL_VERSION_NUMBER >= 0x10000000L) */ if (SSL_set_fd(ssl_conn, fd) == 0) goto free_ssl_conn; @@ -1422,7 +1426,9 @@ static struct mailstream_ssl_context * mailstream_ssl_context_new(SSL_CTX * open ssl_ctx->openssl_ssl_ctx = open_ssl_ctx; ssl_ctx->client_x509 = NULL; ssl_ctx->client_pkey = NULL; +#if (OPENSSL_VERSION_NUMBER >= 0x10000000L) ssl_ctx->server_name = NULL; +#endif /* (OPENSSL_VERSION_NUMBER >= 0x10000000L) */ ssl_ctx->fd = fd; return ssl_ctx; @@ -1431,9 +1437,11 @@ static struct mailstream_ssl_context * mailstream_ssl_context_new(SSL_CTX * open static void mailstream_ssl_context_free(struct mailstream_ssl_context * ssl_ctx) { if (ssl_ctx != NULL) { +#if (OPENSSL_VERSION_NUMBER >= 0x10000000L) if (ssl_ctx->server_name != NULL) { free(ssl_ctx->server_name); } +#endif /* (OPENSSL_VERSION_NUMBER >= 0x10000000L) */ free(ssl_ctx); } }