From 65dbf72cf511b23509f58392498ff5a3bf0d21be Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Tue, 30 Oct 2012 22:18:51 -0700 Subject: [PATCH 1/4] Add support for SNI with wildcard certs --- stud.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/stud.c b/stud.c index 1e83617..151002a 100644 --- a/stud.c +++ b/stud.c @@ -555,6 +555,22 @@ RSA *load_rsa_privatekey(SSL_CTX *ctx, const char *file) { } #ifndef OPENSSL_NO_TLSEXT + +int is_servername_match(const char *servername, char *certname) { + if (strcasecmp(servername, certname) == 0) { + return 1; + } else { + if (strlen(certname) > 2 && strstr(certname, "*.") == certname) { + char *dot = strstr(servername, "."); + char *after_subdomain = strcasestr(servername, &certname[1]); + if (dot == after_subdomain && strlen(after_subdomain) == strlen(&certname[1])) { + return 1; + } + } + } + return 0; +} + /* * Switch the context of the current SSL object to the most appropriate one * based on the SNI header @@ -571,7 +587,7 @@ int sni_switch_ctx(SSL *ssl, int *al, void *data) { // For now, just compare servernames as case insensitive strings. Someday, // it might be nice to Do The Right Thing around star certs. for (cl = sni_ctxs; cl != NULL; cl = cl->next) { - if (strcasecmp(servername, cl->servername) == 0) { + if (is_servername_match(servername, cl->servername)) { SSL_set_SSL_CTX(ssl, cl->ctx); return SSL_TLSEXT_ERR_NOACK; } From 0e16cf8f9a4a16942fb1f18e241ba96b93985535 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Tue, 30 Oct 2012 22:22:43 -0700 Subject: [PATCH 2/4] Add stronger check for wildcard cert matching --- stud.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stud.c b/stud.c index 151002a..96a4f16 100644 --- a/stud.c +++ b/stud.c @@ -563,7 +563,7 @@ int is_servername_match(const char *servername, char *certname) { if (strlen(certname) > 2 && strstr(certname, "*.") == certname) { char *dot = strstr(servername, "."); char *after_subdomain = strcasestr(servername, &certname[1]); - if (dot == after_subdomain && strlen(after_subdomain) == strlen(&certname[1])) { + if (dot && dot == after_subdomain && strlen(after_subdomain) == strlen(&certname[1])) { return 1; } } From 198ec599d07be03ee65cf3566e16ac8526731d14 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Wed, 31 Oct 2012 09:06:01 -0700 Subject: [PATCH 3/4] Add some notes to the configuration file re: SNI --- configuration.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/configuration.c b/configuration.c index 25fead7..b5a176e 100644 --- a/configuration.c +++ b/configuration.c @@ -957,8 +957,11 @@ void config_print_default (FILE *fd, stud_config *cfg) { fprintf(fd, "\n"); fprintf(fd, "# SSL x509 certificate file. REQUIRED.\n"); - fprintf(fd, "# List multiple certs to use SNI. Certs are used in the order they\n"); - fprintf(fd, "# are listed; the last cert listed will be used if none of the others match\n"); + fprintf(fd, "# List multiple certs to use SNI like so:\n"); + fprintf(fd, "#pem-file=\"/my/pem/file1.pem\"\n"); + fprintf(fd, "#pem-file=\"/my/pem/file2.pem\"\n"); + fprintf(fd, "# Certs are used in the order they are listed; \n"); + fprintf(fd, "# the last cert listed will be used if none of the others match\n"); fprintf(fd, "#\n"); fprintf(fd, "# type: string\n"); fprintf(fd, FMT_QSTR, CFG_PEM_FILE, ""); From 10a7ff19aae2c6f568928e256da9117ed9bb35a3 Mon Sep 17 00:00:00 2001 From: Christian Wyglendowski Date: Fri, 21 Dec 2012 17:52:37 -0500 Subject: [PATCH 4/4] Properly handle the SSL error queue Errors were not getting cleared from the queue between various OpenSSL function calls. This was causing the error queue to contain misleading information on subsequent calls to SSL_get_error. One degenerate case was incorrect dispatching of errors after a failed SSL_read. Conditions that simply should have lead to another call to SSL_read were being treated as fatal. This was verified while stracing a stud process and seeing read() return with EAGAIN and stud immediately closing the socket fd and logging an SSL_read error. --- stud.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/stud.c b/stud.c index 4b76cba..40d9965 100644 --- a/stud.c +++ b/stud.c @@ -446,6 +446,13 @@ static void shutdown_proxy(proxystate *ps, SHUTDOWN_REQUESTOR req) { close(ps->fd_up); close(ps->fd_down); + // Clear the SSL error queue - it might contain details + // of errors that we haven't consumed for whatever reason. + // If we don't, future calls to SSL_get_error will lead to + // weird/confusing results that can throw off the handling + // of normal conditions like SSL_ERROR_WANT_READ. + ERR_clear_error(); + SSL_set_shutdown(ps->ssl, SSL_SENT_SHUTDOWN); SSL_free(ps->ssl); @@ -672,7 +679,15 @@ static void client_handshake(struct ev_loop *loop, ev_io *w, int revents) { shutdown_proxy(ps, SHUTDOWN_UP); } else { - LOG("{client} Unexpected SSL error (in handshake): %d\n", err); + + // Try and get more detail on the error from the SSL + // error queue. ERR_error_string requires a char buffer + // of 120 bytes. + unsigned long err_detail = ERR_get_error(); + char err_msg[120]; + ERR_error_string(err_detail, err_msg); + + LOG("{client} Unexpected SSL error (in handshake): %d, %s\n", err, err_msg); shutdown_proxy(ps, SHUTDOWN_UP); } }