From 7b58dc25a6324227a2795845e178dd5d2150cad3 Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Wed, 7 Dec 2016 17:01:03 +0000 Subject: [PATCH] - Fix bug where a self signed cert + only a pinset would not authenticate - Add OARC servers with pinset only to stubby.conf - Move Authentication strings to types_internal for use in call_debugging - Add connection counts to call_debugging - --- src/context.c | 9 --------- src/stub.c | 43 +++++++++++++++++++++++++++++-------------- src/tools/stubby.conf | 12 ++++++++++++ src/types-internal.h | 7 +++++++ src/util-internal.c | 38 +++++++++++++++++++++++++++++++++++--- 5 files changed, 83 insertions(+), 26 deletions(-) diff --git a/src/context.c b/src/context.c index 20dea912..275faaca 100644 --- a/src/context.c +++ b/src/context.c @@ -699,15 +699,6 @@ _getdns_upstreams_dereference(getdns_upstreams *upstreams) GETDNS_FREE(upstreams->mf, upstreams); } -#if defined(DAEMON_DEBUG) && DAEMON_DEBUG -static char* -getdns_auth_str_array[] = { - GETDNS_STR_AUTH_NONE, - GETDNS_STR_AUTH_FAILED, - GETDNS_STR_AUTH_OK -}; -#endif - void _getdns_upstream_shutdown(getdns_upstream *upstream) { diff --git a/src/stub.c b/src/stub.c index 30915709..4f3cff73 100644 --- a/src/stub.c +++ b/src/stub.c @@ -862,29 +862,30 @@ tls_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) if (!upstream) return 0; -#if defined(STUB_DEBUG) && STUB_DEBUG || defined(X509_V_ERR_HOSTNAME_MISMATCH) - int err = X509_STORE_CTX_get_error(ctx); - + int err = X509_STORE_CTX_get_error(ctx); +#if defined(STUB_DEBUG) && STUB_DEBUG DEBUG_STUB("%s %-35s: FD: %d Verify result: (%d) \"%s\"\n", STUB_DEBUG_SETUP_TLS, __FUNCTION__, upstream->fd, err, X509_verify_cert_error_string(err)); #endif + /* First deal with the hostname authentication done by OpenSSL. */ #ifdef X509_V_ERR_HOSTNAME_MISMATCH /*Report if error is hostname mismatch*/ - if (err == X509_V_ERR_HOSTNAME_MISMATCH) { - upstream->tls_auth_state = GETDNS_AUTH_FAILED; - if (upstream->tls_fallback_ok) - DEBUG_STUB("%s %-35s: FD: %d WARNING: Proceeding even though hostname validation failed!\n", - STUB_DEBUG_SETUP_TLS, __FUNCTION__, upstream->fd); - } + if (err == X509_V_ERR_HOSTNAME_MISMATCH && upstream->tls_fallback_ok) + DEBUG_STUB("%s %-35s: FD: %d WARNING: Proceeding even though hostname validation failed!\n", + STUB_DEBUG_SETUP_TLS, __FUNCTION__, upstream->fd); #else /* if we weren't built against OpenSSL with hostname matching we * could not have matched the hostname, so this would be an automatic * tls_auth_fail if there is a hostname provided*/ - if (upstream->tls_auth_name[0]) + if (upstream->tls_auth_name[0]) { upstream->tls_auth_state = GETDNS_AUTH_FAILED; + preverify_ok == 0; + } #endif + + /* Now deal with the pinset validation*/ if (upstream->tls_pubkey_pinset) pinset_ret = _getdns_verify_pinset_match(upstream->tls_pubkey_pinset, ctx); @@ -896,10 +897,23 @@ tls_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) if (upstream->tls_fallback_ok) DEBUG_STUB("%s %-35s: FD: %d, WARNING: Proceeding even though pinset validation failed!\n", STUB_DEBUG_SETUP_TLS, __FUNCTION__, upstream->fd); + } else { + /* If we _only_ had a pinset and it is good then force succesful + authentication when the cert self-signed */ + if ((upstream->tls_pubkey_pinset && upstream->tls_auth_name[0] == '\0') && + (err == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN || + err == X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT)) { + preverify_ok = 1; + DEBUG_STUB("%s %-35s: FD: %d, Allowing self-signed (%d) cert since pins match\n", + STUB_DEBUG_SETUP_TLS, __FUNCTION__, upstream->fd, err); + } } + /* If nothing has failed yet and we had credentials, we have succesfully authenticated*/ - if (upstream->tls_auth_state == GETDNS_AUTH_NONE && - (upstream->tls_pubkey_pinset || upstream->tls_auth_name[0])) + if (preverify_ok == 0) + upstream->tls_auth_state = GETDNS_AUTH_FAILED; + else if (upstream->tls_auth_state == GETDNS_AUTH_NONE && + (upstream->tls_pubkey_pinset || upstream->tls_auth_name[0])) upstream->tls_auth_state = GETDNS_AUTH_OK; /* If fallback is allowed, proceed regardless of what the auth error is (might not be hostname or pinset related) */ @@ -1044,8 +1058,9 @@ tls_do_handshake(getdns_upstream *upstream) /* A re-used session is not verified so need to fix up state in that case */ if (SSL_session_reused(upstream->tls_obj)) upstream->tls_auth_state = upstream->last_tls_auth_state; - DEBUG_STUB("%s %-35s: FD: %d Handshake succeeded with auth state %d. Session is %s.\n", - STUB_DEBUG_SETUP_TLS, __FUNCTION__, upstream->fd, upstream->tls_auth_state, + DEBUG_STUB("%s %-35s: FD: %d Handshake succeeded with auth state %s. Session is %s.\n", + STUB_DEBUG_SETUP_TLS, __FUNCTION__, upstream->fd, + getdns_auth_str_array[upstream->tls_auth_state], SSL_session_reused(upstream->tls_obj) ?"re-used":"new"); if (upstream->tls_session != NULL) SSL_SESSION_free(upstream->tls_session); diff --git a/src/tools/stubby.conf b/src/tools/stubby.conf index 3d3fd30e..5deb03db 100644 --- a/src/tools/stubby.conf +++ b/src/tools/stubby.conf @@ -42,6 +42,18 @@ [ { digest: "sha256" , value: 0x7e8c59467221f606695a797ecc488a6b4109dab7421aba0c5a6d3681ac5273d4 } ] + }, + { address_data: 184.105.193.78 + , tls_pubkey_pinset: + [ { digest: "sha256" + , value: 0xA4E5EBA54B7D9203E06D6C411457014DB447DA17A8DB01F05E9D5F7780045572 + } ] + }, + { address_data: 2620:ff:c000:0:1::64:25 + , tls_pubkey_pinset: + [ { digest: "sha256" + , value: 0xA4E5EBA54B7D9203E06D6C411457014DB447DA17A8DB01F05E9D5F7780045572 + } ] } ] , tls_authentication: GETDNS_AUTHENTICATION_REQUIRED diff --git a/src/types-internal.h b/src/types-internal.h index 06eea0da..068cf282 100644 --- a/src/types-internal.h +++ b/src/types-internal.h @@ -67,6 +67,13 @@ typedef enum getdns_auth_state { #define GETDNS_STR_AUTH_FAILED "Failed" #define GETDNS_STR_AUTH_OK "Success" +static char* +getdns_auth_str_array[] = { + GETDNS_STR_AUTH_NONE, + GETDNS_STR_AUTH_FAILED, + GETDNS_STR_AUTH_OK +}; + struct getdns_context; struct getdns_upstreams; struct getdns_upstream; diff --git a/src/util-internal.c b/src/util-internal.c index 1fbd9632..a7c588ba 100644 --- a/src/util-internal.c +++ b/src/util-internal.c @@ -838,6 +838,16 @@ _getdns_create_call_reporting_dict( was actually used for the last successful query.*/ if (transport == GETDNS_TRANSPORT_TCP && netreq->debug_udp == 1) { transport = GETDNS_TRANSPORT_UDP; + if (getdns_dict_set_int( netreq_debug, "udp_responses_for_this_upstream", + netreq->upstream->udp_responses)) { + getdns_dict_destroy(netreq_debug); + return NULL; + } + if (getdns_dict_set_int( netreq_debug, "udp_timeouts_for_this_upstream", + netreq->upstream->udp_timeouts)) { + getdns_dict_destroy(netreq_debug); + return NULL; + } } if (getdns_dict_set_int( netreq_debug, "transport", transport)) { getdns_dict_destroy(netreq_debug); @@ -858,16 +868,38 @@ _getdns_create_call_reporting_dict( return NULL; } } + /* The running totals are only updated when a connection is closed. + Since it is open as we have just used it, calcualte the value on the fly */ + if (getdns_dict_set_int( netreq_debug, "responses_on_this_connection", + netreq->upstream->responses_received)) { + getdns_dict_destroy(netreq_debug); + return NULL; + } + if (getdns_dict_set_int( netreq_debug, "timeouts_on_this_connection", + netreq->upstream->responses_timeouts)) { + getdns_dict_destroy(netreq_debug); + return NULL; + } + if (getdns_dict_set_int( netreq_debug, "responses_for_this_upstream", + netreq->upstream->responses_received + + netreq->upstream->total_responses)) { + getdns_dict_destroy(netreq_debug); + return NULL; + } + if (getdns_dict_set_int( netreq_debug, "timeouts_for_this_upstream", + netreq->upstream->responses_timeouts + + netreq->upstream->total_timeouts)) { + getdns_dict_destroy(netreq_debug); + return NULL; + } } if (netreq->upstream->transport != GETDNS_TRANSPORT_TLS) return netreq_debug; /* Only include the auth status if TLS was used */ - /* TODO: output all 3 options */ if (getdns_dict_util_set_string(netreq_debug, "tls_auth_status", - netreq->debug_tls_auth_status == GETDNS_AUTH_OK ? - "OK: Server authenticated":"FAILED or NOT TRIED: Server not authenticated")){ + getdns_auth_str_array[netreq->debug_tls_auth_status])){ getdns_dict_destroy(netreq_debug); return NULL;