From 09df4e2d5dfea4715f3376da77a7e5dd7b206cf8 Mon Sep 17 00:00:00 2001 From: Sara Dickinson <sara@sinodun.com> Date: Wed, 22 Feb 2017 13:00:27 +0000 Subject: [PATCH 1/7] Fix spacing error in stubby help output --- src/tools/getdns_query.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/getdns_query.c b/src/tools/getdns_query.c index 61825f1b..bb452929 100644 --- a/src/tools/getdns_query.c +++ b/src/tools/getdns_query.c @@ -215,7 +215,7 @@ print_usage(FILE *out, const char *progname) fprintf(out, "\t\tThe file must be in json dict format.\n"); if (i_am_stubby) { fprintf(out, "\t\tBy default, configuration is first read from"); - fprintf(out, "\t\t\"/etc/stubby.conf\" and then from \"$HOME/.stubby.conf\""); + fprintf(out, "\n\t\t\"/etc/stubby.conf\" and then from \"$HOME/.stubby.conf\"\n"); } fprintf(out, "\t-D\tSet edns0 do bit\n"); fprintf(out, "\t-d\tclear edns0 do bit\n"); From 356408955db7ea8caaac56c017e627ebb882e785 Mon Sep 17 00:00:00 2001 From: Sara Dickinson <sara@sinodun.com> Date: Wed, 22 Feb 2017 13:08:32 +0000 Subject: [PATCH 2/7] Update the SPKI pin in the stubby.conf file for the Sinodun/Surfnet servers. --- src/tools/stubby.conf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/stubby.conf b/src/tools/stubby.conf index 5deb03db..e562e49d 100644 --- a/src/tools/stubby.conf +++ b/src/tools/stubby.conf @@ -5,14 +5,14 @@ , tls_auth_name: "dnsovertls.sinodun.com" , tls_pubkey_pinset: [ { digest: "sha256" - , value: 0xA132D34D34C181765337C70B83E3697B9524DDDB05A7118B43C0284033D5A0CC + , value: 0xEB694ABBD1EC0D56F288F7A70299DCE2C7E64984C73957C580BDE9C81F9C04BE } ] }, { address_data: 145.100.185.16 , tls_auth_name: "dnsovertls1.sinodun.com" , tls_pubkey_pinset: [ { digest: "sha256" - , value: 0x659B41EB08DCC70EE9D624E6219C76EE31954DA1548B0C8519EAE5228CB24150 + , value: 0x704D9E7002DE13907EBAB2610EB26554599FDFC7092C0BEA7A438DBE3BE9A940 } ] }, { address_data: 185.49.141.38 From 1b7aef5a882b39f04ca12e325d66a7d03d580519 Mon Sep 17 00:00:00 2001 From: Sara Dickinson <sara@sinodun.com> Date: Thu, 23 Feb 2017 14:49:17 +0000 Subject: [PATCH 3/7] Add a new GETDNS_RETURN code for the case where no upstream is considered valid and hence a query cannot even be scheduled. Only applies when using purely stateful transports. This can happen when using Stubby if there are problems with connections to upstreams. --- src/const-info.c | 2 ++ src/getdns/getdns_extra.h.in | 2 ++ src/stub.c | 5 ++--- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/const-info.c b/src/const-info.c index a71f928a..0022a768 100644 --- a/src/const-info.c +++ b/src/const-info.c @@ -24,6 +24,7 @@ static struct const_info consts_info[] = { { 310, "GETDNS_RETURN_MEMORY_ERROR", GETDNS_RETURN_MEMORY_ERROR_TEXT }, { 311, "GETDNS_RETURN_INVALID_PARAMETER", GETDNS_RETURN_INVALID_PARAMETER_TEXT }, { 312, "GETDNS_RETURN_NOT_IMPLEMENTED", GETDNS_RETURN_NOT_IMPLEMENTED_TEXT }, + { 398, "GETDNS_RETURN_NO_UPSTREAM_AVAILABLE", GETDNS_RETURN_NO_UPSTREAM_AVAILABLE_TEXT }, { 399, "GETDNS_RETURN_NEED_MORE_SPACE", GETDNS_RETURN_NEED_MORE_SPACE_TEXT }, { 400, "GETDNS_DNSSEC_SECURE", GETDNS_DNSSEC_SECURE_TEXT }, { 401, "GETDNS_DNSSEC_BOGUS", GETDNS_DNSSEC_BOGUS_TEXT }, @@ -218,6 +219,7 @@ static struct const_name_info consts_name_info[] = { { "GETDNS_RETURN_GOOD", 0 }, { "GETDNS_RETURN_INVALID_PARAMETER", 311 }, { "GETDNS_RETURN_MEMORY_ERROR", 310 }, + { "GETDNS_RETURN_NO_UPSTREAM_AVAILABLE", 398}, { "GETDNS_RETURN_NEED_MORE_SPACE", 399 }, { "GETDNS_RETURN_NOT_IMPLEMENTED", 312 }, { "GETDNS_RETURN_NO_SUCH_DICT_NAME", 305 }, diff --git a/src/getdns/getdns_extra.h.in b/src/getdns/getdns_extra.h.in index 5abf39e7..a76564eb 100644 --- a/src/getdns/getdns_extra.h.in +++ b/src/getdns/getdns_extra.h.in @@ -56,6 +56,8 @@ extern "C" { * \defgroup Ureturnvaluesandtext Additional return values and texts * @{ */ +#define GETDNS_RETURN_NO_UPSTREAM_AVAILABLE ((getdns_return_t) 398 ) +#define GETDNS_RETURN_NO_UPSTREAM_AVAILABLE_TEXT "None of the configured upstreams could be used to send queries on the specified transports" #define GETDNS_RETURN_NEED_MORE_SPACE ((getdns_return_t) 399 ) #define GETDNS_RETURN_NEED_MORE_SPACE_TEXT "The buffer was too small" /** @} diff --git a/src/stub.c b/src/stub.c index 37c025a8..369c0d38 100644 --- a/src/stub.c +++ b/src/stub.c @@ -520,7 +520,7 @@ stub_cleanup(getdns_network_req *netreq) static void upstream_failed(getdns_upstream *upstream, int during_setup) { - DEBUG_STUB("%s %-35s: FD: %d During setup = %d\n", + DEBUG_STUB("%s %-35s: FD: %d Failure during connection setup = %d\n", STUB_DEBUG_CLEANUP, __FUNC__, upstream->fd, during_setup); /* Fallback code should take care of queue queries and then close conn when idle.*/ @@ -2023,8 +2023,7 @@ _getdns_submit_stub_request(getdns_network_req *netreq) * All other set up is done async*/ fd = upstream_find_for_netreq(netreq); if (fd == -1) - /* Handle better, will give unhelpful error is some cases */ - return GETDNS_RETURN_GENERIC_ERROR; + return GETDNS_RETURN_NO_UPSTREAM_AVAILABLE; getdns_transport_list_t transport = netreq->transports[netreq->transport_current]; From ff4ecd5b39997fb1e2650243aa0c66aa5765e9dc Mon Sep 17 00:00:00 2001 From: Sara Dickinson <sara@sinodun.com> Date: Thu, 23 Feb 2017 15:38:45 +0000 Subject: [PATCH 4/7] Couple of extra output messages so Stubby users in strict mode know why the authentication failed --- src/stub.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/stub.c b/src/stub.c index 369c0d38..c40d518e 100644 --- a/src/stub.c +++ b/src/stub.c @@ -872,9 +872,16 @@ tls_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) /* 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_fallback_ok) - DEBUG_STUB("%s %-35s: FD: %d WARNING: Proceeding even though hostname validation failed!\n", - STUB_DEBUG_SETUP_TLS, __FUNC__, upstream->fd); + if (err == X509_V_ERR_HOSTNAME_MISMATCH) { + if (upstream->tls_fallback_ok) + DEBUG_STUB("%s %-35s: FD: %d WARNING: Proceeding even though hostname validation failed!\n", + STUB_DEBUG_SETUP_TLS, __FUNC__, upstream->fd); +#if defined(DAEMON_DEBUG) && DAEMON_DEBUG + else + DEBUG_DAEMON("%s %s : Conn failed : Transport=TLS - *Failure* - Hostname mismatch\n", + STUB_DEBUG_DAEMON, upstream->addr_str); +#endif + } #else /* if we weren't built against OpenSSL with hostname matching we * could not have matched the hostname, so this would be an automatic @@ -897,9 +904,15 @@ 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, __FUNC__, upstream->fd); +#if defined(DAEMON_DEBUG) && DAEMON_DEBUG + else + DEBUG_DAEMON("%s %s : Conn failed : Transport=TLS - *Failure* - Pinset validation failure\n", + STUB_DEBUG_DAEMON, upstream->addr_str); +#endif } else { /* If we _only_ had a pinset and it is good then force succesful - authentication when the cert self-signed */ + authentication when the cert self-signed + TODO: We need to check for other error cases here, not blindly accept the cert!! */ 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)) { @@ -915,6 +928,7 @@ tls_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) 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) */ return (upstream->tls_fallback_ok) ? 1 : preverify_ok; From ebdf657fd784bc6a7e6f53f0bb8937653eef659c Mon Sep 17 00:00:00 2001 From: Sara Dickinson <sara@sinodun.com> Date: Thu, 23 Feb 2017 16:48:16 +0000 Subject: [PATCH 5/7] Change pins for IPv6 addresses for Sinodun privacy servers! Improve logging of auth failure --- src/stub.c | 15 +++++++-------- src/tools/stubby.conf | 4 ++-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/stub.c b/src/stub.c index c40d518e..148c1a2b 100644 --- a/src/stub.c +++ b/src/stub.c @@ -868,20 +868,19 @@ tls_verify_callback(int preverify_ok, X509_STORE_CTX *ctx) STUB_DEBUG_SETUP_TLS, __FUNC__, upstream->fd, err, X509_verify_cert_error_string(err)); #endif +#if defined(DAEMON_DEBUG) && DAEMON_DEBUG + if (!preverify_ok && !upstream->tls_fallback_ok) + DEBUG_DAEMON("%s %s : Conn failed : Transport=TLS - *Failure* - (%d) \"%s\"\n", + STUB_DEBUG_DAEMON, upstream->addr_str, 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) { - if (upstream->tls_fallback_ok) + 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, __FUNC__, upstream->fd); -#if defined(DAEMON_DEBUG) && DAEMON_DEBUG - else - DEBUG_DAEMON("%s %s : Conn failed : Transport=TLS - *Failure* - Hostname mismatch\n", - STUB_DEBUG_DAEMON, upstream->addr_str); -#endif - } #else /* if we weren't built against OpenSSL with hostname matching we * could not have matched the hostname, so this would be an automatic diff --git a/src/tools/stubby.conf b/src/tools/stubby.conf index e562e49d..054a9256 100644 --- a/src/tools/stubby.conf +++ b/src/tools/stubby.conf @@ -26,14 +26,14 @@ , tls_auth_name: "dnsovertls.sinodun.com" , tls_pubkey_pinset: [ { digest: "sha256" - , value: 0xA132D34D34C181765337C70B83E3697B9524DDDB05A7118B43C0284033D5A0CC + , value: 0xEB694ABBD1EC0D56F288F7A70299DCE2C7E64984C73957C580BDE9C81F9C04BE } ] }, { address_data: 2001:610:1:40ba:145:100:185:16 , tls_auth_name: "dnsovertls1.sinodun.com" , tls_pubkey_pinset: [ { digest: "sha256" - , value: 0x659B41EB08DCC70EE9D624E6219C76EE31954DA1548B0C8519EAE5228CB24150 + , value: 0x704D9E7002DE13907EBAB2610EB26554599FDFC7092C0BEA7A438DBE3BE9A940 } ] }, { address_data: 2a04:b900:0:100::38 From f71dd2bf7121070720950897959aaa1ad183dd66 Mon Sep 17 00:00:00 2001 From: Sara Dickinson <sara@sinodun.com> Date: Thu, 23 Feb 2017 16:50:29 +0000 Subject: [PATCH 6/7] Re-order so checks pass! --- src/const-info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/const-info.c b/src/const-info.c index 0022a768..473f7a45 100644 --- a/src/const-info.c +++ b/src/const-info.c @@ -219,12 +219,12 @@ static struct const_name_info consts_name_info[] = { { "GETDNS_RETURN_GOOD", 0 }, { "GETDNS_RETURN_INVALID_PARAMETER", 311 }, { "GETDNS_RETURN_MEMORY_ERROR", 310 }, - { "GETDNS_RETURN_NO_UPSTREAM_AVAILABLE", 398}, { "GETDNS_RETURN_NEED_MORE_SPACE", 399 }, { "GETDNS_RETURN_NOT_IMPLEMENTED", 312 }, { "GETDNS_RETURN_NO_SUCH_DICT_NAME", 305 }, { "GETDNS_RETURN_NO_SUCH_EXTENSION", 307 }, { "GETDNS_RETURN_NO_SUCH_LIST_ITEM", 304 }, + { "GETDNS_RETURN_NO_UPSTREAM_AVAILABLE", 398}, { "GETDNS_RETURN_UNKNOWN_TRANSACTION", 303 }, { "GETDNS_RETURN_WRONG_TYPE_REQUESTED", 306 }, { "GETDNS_RRCLASS_ANY", 255 }, From 7c8605c3b133d6739045b5e0cae834c76c21c183 Mon Sep 17 00:00:00 2001 From: Sara Dickinson <sara@sinodun.com> Date: Thu, 23 Feb 2017 17:03:00 +0000 Subject: [PATCH 7/7] And fix the whitespace... --- src/const-info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/const-info.c b/src/const-info.c index 473f7a45..4556634a 100644 --- a/src/const-info.c +++ b/src/const-info.c @@ -224,7 +224,7 @@ static struct const_name_info consts_name_info[] = { { "GETDNS_RETURN_NO_SUCH_DICT_NAME", 305 }, { "GETDNS_RETURN_NO_SUCH_EXTENSION", 307 }, { "GETDNS_RETURN_NO_SUCH_LIST_ITEM", 304 }, - { "GETDNS_RETURN_NO_UPSTREAM_AVAILABLE", 398}, + { "GETDNS_RETURN_NO_UPSTREAM_AVAILABLE", 398 }, { "GETDNS_RETURN_UNKNOWN_TRANSACTION", 303 }, { "GETDNS_RETURN_WRONG_TYPE_REQUESTED", 306 }, { "GETDNS_RRCLASS_ANY", 255 },