From d09675539ee23d9af85bd7e275cf4e7561897361 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Mon, 21 Dec 2015 19:27:40 -0500 Subject: [PATCH] Provide access to the pinsets during the TLS verification callback We do this by associating a getdns_upstream object with the SSL object handled by that upstream. This allows us to collapse the verification callback code to a single function. Note that if we've agreed that fallback is ok, we are now willing to accept *any* cert verification error, not just HOSTNAME_MISMATCH. This is fine, because the alternative is falling back to cleartext, which would be worse. We also always set SSL_VERIFY_PEER, since we might as well try to do so; we'll drop the verification error ourselves if we know we're OK with falling back. --- src/context.h | 1 + src/pubkey-pinning.c | 49 +++++++++++++++++++++++++++++++++++++ src/pubkey-pinning.h | 12 ++++++++++ src/stub.c | 57 ++++++++++++++++---------------------------- 4 files changed, 83 insertions(+), 36 deletions(-) diff --git a/src/context.h b/src/context.h index 729962ac..d814219c 100644 --- a/src/context.h +++ b/src/context.h @@ -149,6 +149,7 @@ typedef struct getdns_upstream { unsigned has_prev_client_cookie : 1; unsigned has_server_cookie : 1; unsigned server_cookie_len : 5; + unsigned tls_fallback_ok : 1; /* TSIG */ uint8_t tsig_dname[256]; diff --git a/src/pubkey-pinning.c b/src/pubkey-pinning.c index 96d37fe1..f2935dbb 100644 --- a/src/pubkey-pinning.c +++ b/src/pubkey-pinning.c @@ -308,5 +308,54 @@ _getdns_get_pubkey_pinset_list(getdns_context *ctx, return r; } +/* this should only happen once ever in the life of the library. it's + used to associate a getdns_context_t with an SSL_CTX, to be able to + do custom verification. + + see doc/HOWTO/proxy_certificates.txt as an example +*/ +static int +_get_ssl_getdns_upstream_idx() +{ + static volatile int idx = -1; + if (idx < 0) { + CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE); + if (idx < 0) + idx = SSL_get_ex_new_index(0, "associated getdns upstream", + NULL,NULL,NULL); + CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE); + } + return idx; +} + +getdns_upstream* +_getdns_upstream_from_x509_store(X509_STORE_CTX *store) +{ + int uidx = _get_ssl_getdns_upstream_idx(); + int sslidx = SSL_get_ex_data_X509_STORE_CTX_idx(); + const SSL *ssl; + + /* all *_get_ex_data() should return NULL on failure anyway */ + ssl = X509_STORE_CTX_get_ex_data(store, sslidx); + if (ssl) + return (getdns_upstream*) SSL_get_ex_data(ssl, uidx); + else + return NULL; + /* TODO: if we want more details about errors somehow, we + * might call ERR_get_error (see CRYPTO_set_ex_data(3ssl))*/ +} + +getdns_return_t +_getdns_associate_upstream_with_SSL(SSL *ssl, + getdns_upstream *upstream) +{ + int uidx = _get_ssl_getdns_upstream_idx(); + if (SSL_set_ex_data(ssl, uidx, upstream)) + return GETDNS_RETURN_GOOD; + else + return GETDNS_RETURN_GENERIC_ERROR; + /* TODO: if we want more details about errors somehow, we + * might call ERR_get_error (see CRYPTO_set_ex_data(3ssl))*/ +} /* pubkey-pinning.c */ diff --git a/src/pubkey-pinning.h b/src/pubkey-pinning.h index 3b5a5983..c5585567 100644 --- a/src/pubkey-pinning.h +++ b/src/pubkey-pinning.h @@ -48,6 +48,18 @@ _getdns_get_pubkey_pinset_list(getdns_context *ctx, const sha256_pin_t *pinset_in, getdns_list **pinset_list); + +/* internal functions for associating X.509 verification processes in + * OpenSSL with getdns_upstream objects. */ + +getdns_upstream* +_getdns_upstream_from_x509_store(X509_STORE_CTX *store); + + +getdns_return_t +_getdns_associate_upstream_with_SSL(SSL *ssl, + getdns_upstream *upstream); + #endif /* pubkey-pinning.h */ diff --git a/src/stub.c b/src/stub.c index 4576851e..44c0e58e 100644 --- a/src/stub.c +++ b/src/stub.c @@ -47,6 +47,7 @@ #include "context.h" #include "util-internal.h" #include "general.h" +#include "pubkey-pinning.h" #define STUB_OUT_OF_OPTIONS -5 /* upstream options exceeded MAXIMUM_UPSTREAM_OPTION_SPACE */ #define STUB_TLS_SETUP_ERROR -4 @@ -841,40 +842,20 @@ tls_auth_status_ok(getdns_upstream *upstream, getdns_network_req *netreq) { int tls_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) { -#if defined(STUB_DEBUG) && STUB_DEBUG int err; - const char * err_str; - - err = X509_STORE_CTX_get_error(ctx); - err_str = X509_verify_cert_error_string(err); - DEBUG_STUB("--- %s, VERIFY RESULT: %s\n", __FUNCTION__, err_str); -#endif - /*Always proceed without changing result*/ - return preverify_ok; -} - -int -tls_verify_callback_with_fallback(int preverify_ok, X509_STORE_CTX *ctx) -{ -#ifdef X509_V_ERR_HOSTNAME_MISMATCH - int err; -# if defined(STUB_DEBUG) && STUB_DEBUG - const char * err_str; -# endif + getdns_upstream *upstream; err = X509_STORE_CTX_get_error(ctx); -# if defined(STUB_DEBUG) && STUB_DEBUG - err_str = X509_verify_cert_error_string(err); - DEBUG_STUB("--- %s, VERIFY RESULT: (%d) \"%s\"\n", __FUNCTION__, err, err_str); -# endif + upstream = _getdns_upstream_from_x509_store(ctx); + DEBUG_STUB("--- %s, VERIFY RESULT: (%d) \"%s\"\n", __FUNCTION__, + err, X509_verify_cert_error_string(err)); + +#ifdef X509_V_ERR_HOSTNAME_MISMATCH /*Proceed if error is hostname mismatch*/ - if (err == X509_V_ERR_HOSTNAME_MISMATCH) { + if (upstream && upstream->tls_fallback_ok && err == X509_V_ERR_HOSTNAME_MISMATCH) DEBUG_STUB("--- %s, PROCEEDING WITHOUT HOSTNAME VALIDATION!!\n", __FUNCTION__); - return 1; - } - else #endif - return preverify_ok; + return (upstream && upstream->tls_fallback_ok) ? 1 : preverify_ok; } static SSL* @@ -892,11 +873,17 @@ tls_create_object(getdns_dns_req *dnsreq, int fd, getdns_upstream *upstream) SSL_free(ssl); return NULL; } + /* make sure we'll be able to find the context again when we need it */ + if (_getdns_associate_upstream_with_SSL(ssl, upstream) != GETDNS_RETURN_GOOD) { + SSL_free(ssl); + return NULL; + } /* NOTE: this code will fallback on a given upstream, without trying authentication on other upstreams first. This is non-optimal and but avoids multiple TLS handshakes before getting a usable connection. */ + upstream->tls_fallback_ok = 0; /* If we have a hostname, always use it */ if (upstream->tls_auth_name[0] != '\0') { /*Request certificate for the auth_name*/ @@ -920,12 +907,8 @@ tls_create_object(getdns_dns_req *dnsreq, int fd, getdns_upstream *upstream) } #endif /* Allow fallback to opportunistic if settings permit it*/ - if (dnsreq->netreqs[0]->tls_auth_min == GETDNS_AUTHENTICATION_HOSTNAME) - SSL_set_verify(ssl, SSL_VERIFY_PEER, tls_verify_callback); - else { - SSL_set_verify(ssl, SSL_VERIFY_NONE, tls_verify_callback_with_fallback); - SSL_set_cipher_list(ssl, "DEFAULT"); - } + if (dnsreq->netreqs[0]->tls_auth_min != GETDNS_AUTHENTICATION_HOSTNAME) + upstream->tls_fallback_ok = 1; } else { /* Lack of host name is OK unless only authenticated TLS is specified*/ if (dnsreq->netreqs[0]->tls_auth_min == GETDNS_AUTHENTICATION_HOSTNAME) { @@ -937,10 +920,12 @@ tls_create_object(getdns_dns_req *dnsreq, int fd, getdns_upstream *upstream) /* no hostname verification, so we will make opportunistic connections */ DEBUG_STUB("--- %s, PROCEEDING WITHOUT HOSTNAME VALIDATION!!\n", __FUNCTION__); upstream->tls_auth_failed = 1; - SSL_set_verify(ssl, SSL_VERIFY_NONE, tls_verify_callback_with_fallback); - SSL_set_cipher_list(ssl, "DEFAULT"); + upstream->tls_fallback_ok = 1; } } + if (upstream->tls_fallback_ok) + SSL_set_cipher_list(ssl, "DEFAULT"); + SSL_set_verify(ssl, SSL_VERIFY_PEER, tls_verify_callback); SSL_set_connect_state(ssl); (void) SSL_set_mode(ssl, SSL_MODE_AUTO_RETRY);