From 1b0a09a23f2b306ec37addd3e564b09b8f903b00 Mon Sep 17 00:00:00 2001 From: Jim Hague Date: Tue, 20 Nov 2018 14:53:31 +0000 Subject: [PATCH] Wrap hostname/certificate verification. This removes the last OpenSSL items from stub.c. --- src/openssl/tls.c | 99 ++++++++++++++++++++++++++++++++++++ src/stub.c | 127 +++++++++++++--------------------------------- src/tls.h | 36 +++++++++++++ 3 files changed, 170 insertions(+), 92 deletions(-) diff --git a/src/openssl/tls.c b/src/openssl/tls.c index 134525fd..36a0f9a3 100644 --- a/src/openssl/tls.c +++ b/src/openssl/tls.c @@ -34,7 +34,9 @@ #include "config.h" #include +#include #include +#include #include #include #include @@ -42,8 +44,35 @@ #include #include +#include "debug.h" +#include "context.h" + #include "tls.h" +static int _getdns_tls_verify_always_ok(int ok, X509_STORE_CTX *ctx) +{ +# if defined(STUB_DEBUG) && STUB_DEBUG + char buf[8192]; + X509 *cert; + int err; + int depth; + + cert = X509_STORE_CTX_get_current_cert(ctx); + err = X509_STORE_CTX_get_error(ctx); + depth = X509_STORE_CTX_get_error_depth(ctx); + + if (cert) + X509_NAME_oneline(X509_get_subject_name(cert), buf, sizeof(buf)); + else + strcpy(buf, ""); + DEBUG_STUB("DEBUG Cert verify: depth=%d verify=%d err=%d subject=%s errorstr=%s\n", depth, ok, err, buf, X509_verify_cert_error_string(err)); +# else /* defined(STUB_DEBUG) && STUB_DEBUG */ + (void)ok; + (void)ctx; +# endif /* #else defined(STUB_DEBUG) && STUB_DEBUG */ + return 1; +} + static _getdns_tls_x509* _getdns_tls_x509_new(X509* cert) { _getdns_tls_x509* res; @@ -394,6 +423,76 @@ getdns_return_t _getdns_tls_connection_is_session_reused(_getdns_tls_connection* return GETDNS_RETURN_TLS_CONNECTION_FRESH; } +getdns_return_t _getdns_tls_connection_setup_hostname_auth(_getdns_tls_connection* conn, const char* auth_name) +{ + if (!conn || !conn->ssl || !auth_name) + return GETDNS_RETURN_INVALID_PARAMETER; + + SSL_set_tlsext_host_name(conn->ssl, auth_name); + /* Set up native OpenSSL hostname verification */ + X509_VERIFY_PARAM *param; + param = SSL_get0_param(conn->ssl); + X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); + X509_VERIFY_PARAM_set1_host(param, auth_name, 0); + return GETDNS_RETURN_GOOD; +} + +getdns_return_t _getdns_tls_connection_set_host_pinset(_getdns_tls_connection* conn, const char* auth_name, const sha256_pin_t* pinset) +{ + if (!conn || !conn->ssl || !auth_name) + return GETDNS_RETURN_INVALID_PARAMETER; + + int osr = SSL_dane_enable(conn->ssl, *auth_name ? auth_name : NULL); + (void) osr; + DEBUG_STUB("%s %-35s: DEBUG: SSL_dane_enable(\"%s\") -> %d\n" + , STUB_DEBUG_SETUP_TLS, __FUNC__, upstream->tls_auth_name, osr); + SSL_set_verify(conn->ssl, SSL_VERIFY_PEER, _getdns_tls_verify_always_ok); + const sha256_pin_t *pin_p; + size_t n_pins = 0; + for (pin_p = pinset; pin_p; pin_p = pin_p->next) { + osr = SSL_dane_tlsa_add(conn->ssl, 2, 1, 1, + (unsigned char *)pin_p->pin, SHA256_DIGEST_LENGTH); + DEBUG_STUB("%s %-35s: DEBUG: SSL_dane_tlsa_add() -> %d\n" + , STUB_DEBUG_SETUP_TLS, __FUNC__, osr); + if (osr > 0) + ++n_pins; + osr = SSL_dane_tlsa_add(conn->ssl, 3, 1, 1, + (unsigned char *)pin_p->pin, SHA256_DIGEST_LENGTH); + DEBUG_STUB("%s %-35s: DEBUG: SSL_dane_tlsa_add() -> %d\n" + , STUB_DEBUG_SETUP_TLS, __FUNC__, osr); + if (osr > 0) + ++n_pins; + } + return GETDNS_RETURN_GOOD; +} + +getdns_return_t _getdns_tls_connection_verify(_getdns_tls_connection* conn, long* errnum, const char** errmsg) +{ + if (!conn || !conn->ssl) + return GETDNS_RETURN_INVALID_PARAMETER; + + long verify_result = SSL_get_verify_result(conn->ssl); + switch (verify_result) { + case X509_V_OK: + return GETDNS_RETURN_GOOD; + + case X509_V_ERR_DANE_NO_MATCH: + if (errnum) + *errnum = 0; + if (errmsg) + *errmsg = "Pinset validation failure"; + return GETDNS_RETURN_GENERIC_ERROR; + + default: + if (errnum) + *errnum = verify_result; + if (errmsg) + *errmsg = X509_verify_cert_error_string(verify_result); + return GETDNS_RETURN_GENERIC_ERROR; + } +} + + getdns_return_t _getdns_tls_connection_read(_getdns_tls_connection* conn, uint8_t* buf, size_t to_read, size_t* read) { int sread; diff --git a/src/stub.c b/src/stub.c index 52478dfb..8db8a7fe 100644 --- a/src/stub.c +++ b/src/stub.c @@ -39,9 +39,6 @@ #define INTERCEPT_COM_DS 0 #include "debug.h" -#include -#include -#include #include #include "stub.h" #include "gldns/gbuffer.h" @@ -826,31 +823,6 @@ tls_requested(getdns_network_req *netreq) 1 : 0; } -static int -_getdns_tls_verify_always_ok(int ok, X509_STORE_CTX *ctx) -{ -# if defined(STUB_DEBUG) && STUB_DEBUG - char buf[8192]; - X509 *cert; - int err; - int depth; - - cert = X509_STORE_CTX_get_current_cert(ctx); - err = X509_STORE_CTX_get_error(ctx); - depth = X509_STORE_CTX_get_error_depth(ctx); - - if (cert) - X509_NAME_oneline(X509_get_subject_name(cert), buf, sizeof(buf)); - else - strcpy(buf, ""); - DEBUG_STUB("DEBUG Cert verify: depth=%d verify=%d err=%d subject=%s errorstr=%s\n", depth, ok, err, buf, X509_verify_cert_error_string(err)); -# else /* defined(STUB_DEBUG) && STUB_DEBUG */ - (void)ok; - (void)ctx; -# endif /* #else defined(STUB_DEBUG) && STUB_DEBUG */ - return 1; -} - static _getdns_tls_connection* tls_create_object(getdns_dns_req *dnsreq, int fd, getdns_upstream *upstream) { @@ -881,12 +853,7 @@ tls_create_object(getdns_dns_req *dnsreq, int fd, getdns_upstream *upstream) /*Request certificate for the auth_name*/ DEBUG_STUB("%s %-35s: Hostname verification requested for: %s\n", STUB_DEBUG_SETUP_TLS, __FUNC__, upstream->tls_auth_name); - SSL_set_tlsext_host_name(tls->ssl, upstream->tls_auth_name); - /* Set up native OpenSSL hostname verification */ - X509_VERIFY_PARAM *param; - param = SSL_get0_param(tls->ssl); - X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS); - X509_VERIFY_PARAM_set1_host(param, upstream->tls_auth_name, 0); + _getdns_tls_connection_setup_hostname_auth(tls, upstream->tls_auth_name); /* Allow fallback to opportunistic if settings permit it*/ if (dnsreq->netreqs[0]->tls_auth_min != GETDNS_AUTHENTICATION_REQUIRED) upstream->tls_fallback_ok = 1; @@ -926,32 +893,7 @@ tls_create_object(getdns_dns_req *dnsreq, int fd, getdns_upstream *upstream) __FUNC__); } - int osr; -# if defined(STUB_DEBUG) && STUB_DEBUG - osr = -# else - (void) -# endif - SSL_dane_enable(tls->ssl, *upstream->tls_auth_name ? upstream->tls_auth_name : NULL); - DEBUG_STUB("%s %-35s: DEBUG: SSL_dane_enable(\"%s\") -> %d\n" - , STUB_DEBUG_SETUP_TLS, __FUNC__, upstream->tls_auth_name, osr); - SSL_set_verify(tls->ssl, SSL_VERIFY_PEER, _getdns_tls_verify_always_ok); - sha256_pin_t *pin_p; - size_t n_pins = 0; - for (pin_p = upstream->tls_pubkey_pinset; pin_p; pin_p = pin_p->next) { - osr = SSL_dane_tlsa_add(tls->ssl, 2, 1, 1, - (unsigned char *)pin_p->pin, SHA256_DIGEST_LENGTH); - DEBUG_STUB("%s %-35s: DEBUG: SSL_dane_tlsa_add() -> %d\n" - , STUB_DEBUG_SETUP_TLS, __FUNC__, osr); - if (osr > 0) - ++n_pins; - osr = SSL_dane_tlsa_add(tls->ssl, 3, 1, 1, - (unsigned char *)pin_p->pin, SHA256_DIGEST_LENGTH); - DEBUG_STUB("%s %-35s: DEBUG: SSL_dane_tlsa_add() -> %d\n" - , STUB_DEBUG_SETUP_TLS, __FUNC__, osr); - if (osr > 0) - ++n_pins; - } + _getdns_tls_connection_set_host_pinset(tls, upstream->tls_auth_name, upstream->tls_pubkey_pinset); /* Session resumption. There are trade-offs here. Want to do it when possible only if we have the right type of connection. Note a change @@ -1005,12 +947,9 @@ tls_do_handshake(getdns_upstream *upstream) upstream->tls_auth_state = upstream->last_tls_auth_state; else if (upstream->tls_pubkey_pinset || upstream->tls_auth_name[0]) { - X509 *peer_cert = SSL_get_peer_certificate(upstream->tls_obj->ssl); - long verify_result = SSL_get_verify_result(upstream->tls_obj->ssl); + _getdns_tls_x509* peer_cert = _getdns_tls_connection_get_peer_certificate(upstream->tls_obj); - upstream->tls_auth_state = peer_cert && verify_result == X509_V_OK - ? GETDNS_AUTH_OK : GETDNS_AUTH_FAILED; - if (!peer_cert) + if (!peer_cert) { _getdns_upstream_log(upstream, GETDNS_LOG_UPSTREAM_STATS, ( upstream->tls_fallback_ok @@ -1021,38 +960,42 @@ tls_do_handshake(getdns_upstream *upstream) ( upstream->tls_fallback_ok ? "Tolerated because of Opportunistic profile" : "*Failure*" )); + upstream->tls_auth_state = GETDNS_AUTH_FAILED; + } else { + long verify_errno; + const char* verify_errmsg; - /* Since we don't have DANE validation yet, DANE validation - * failures are always pinset validation failures - */ - else if (verify_result == X509_V_ERR_DANE_NO_MATCH) - _getdns_upstream_log(upstream, - GETDNS_LOG_UPSTREAM_STATS, - ( upstream->tls_fallback_ok - ? GETDNS_LOG_INFO : GETDNS_LOG_ERR), - "%-40s : Verify failed : TLS - %s - " - "Pinset validation failure\n", upstream->addr_str, - ( upstream->tls_fallback_ok - ? "Tolerated because of Opportunistic profile" - : "*Failure*" )); - else if (verify_result != X509_V_OK) - _getdns_upstream_log(upstream, - GETDNS_LOG_UPSTREAM_STATS, - ( upstream->tls_fallback_ok - ? GETDNS_LOG_INFO : GETDNS_LOG_ERR), - "%-40s : Verify failed : TLS - %s - " - "(%d) \"%s\"\n", upstream->addr_str, - ( upstream->tls_fallback_ok - ? "Tolerated because of Opportunistic profile" - : "*Failure*" ), verify_result, - X509_verify_cert_error_string(verify_result)); - else + if (!_getdns_tls_connection_verify(upstream->tls_obj, &verify_errno, &verify_errmsg)) { + upstream->tls_auth_state = GETDNS_AUTH_OK; + if (verify_errno != 0) { + _getdns_upstream_log(upstream, + GETDNS_LOG_UPSTREAM_STATS, + ( upstream->tls_fallback_ok + ? GETDNS_LOG_INFO : GETDNS_LOG_ERR), "%-40s : Verify failed : TLS - %s - " + "(%d) \"%s\"\n", upstream->addr_str, + ( upstream->tls_fallback_ok + ? "Tolerated because of Opportunistic profile" + : "*Failure*" ), + verify_errno, verify_errmsg); + } else { + _getdns_upstream_log(upstream, + GETDNS_LOG_UPSTREAM_STATS, + ( upstream->tls_fallback_ok + ? GETDNS_LOG_INFO : GETDNS_LOG_ERR), "%-40s : Verify failed : TLS - %s - " + "%s\n", upstream->addr_str, + ( upstream->tls_fallback_ok + ? "Tolerated because of Opportunistic profile" + : "*Failure*" ), + verify_errno, verify_errmsg); + } + } else { _getdns_upstream_log(upstream, GETDNS_LOG_UPSTREAM_STATS, GETDNS_LOG_DEBUG, "%-40s : Verify passed : TLS\n", upstream->addr_str); - - X509_free(peer_cert); + } + _getdns_tls_x509_free(peer_cert); + } if (upstream->tls_auth_state == GETDNS_AUTH_FAILED && !upstream->tls_fallback_ok) return STUB_SETUP_ERROR; diff --git a/src/tls.h b/src/tls.h index 9d0b0704..295b649c 100644 --- a/src/tls.h +++ b/src/tls.h @@ -38,6 +38,10 @@ #include "tls-internal.h" +/* Forward declare type. */ +struct sha256_pin; +typedef struct sha256_pin sha256_pin_t; + /* Additional return codes required by TLS abstraction. Internal use only. */ #define GETDNS_RETURN_TLS_WANT_READ ((getdns_return_t) 420) #define GETDNS_RETURN_TLS_WANT_WRITE ((getdns_return_t) 421) @@ -100,6 +104,38 @@ _getdns_tls_x509* _getdns_tls_connection_get_peer_certificate(_getdns_tls_connec */ getdns_return_t _getdns_tls_connection_is_session_reused(_getdns_tls_connection* conn); +/** + * Set up host name verification. + * + * @param conn the connection. + * @param auth_name the hostname. + * @return GETDNS_RETURN_GOOD if all OK. + * @return GETDNS_RETURN_INVALID_PARAMETER if conn is null or has no SSL. + */ +getdns_return_t _getdns_tls_connection_setup_hostname_auth(_getdns_tls_connection* conn, const char* auth_name); + +/** + * Set host pinset. + * + * @param conn the connection. + * @param auth_name the hostname. + * @return GETDNS_RETURN_GOOD if all OK. + * @return GETDNS_RETURN_INVALID_PARAMETER if conn is null or has no SSL. + */ +getdns_return_t _getdns_tls_connection_set_host_pinset(_getdns_tls_connection* conn, const char* auth_name, const sha256_pin_t* pinset); + +/** + * Get result of certificate verification. + * + * @param conn the connection. + * @param errno failure error number. + * @param errmsg failure error message. + * @return GETDNS_RETURN_GOOD if all OK. + * @return GETDNS_RETURN_INVALID_PARAMETER if conn is null or has no SSL. + * @return GETDNS_RETURN_GENERIC_ERROR if verification failed. + */ +getdns_return_t _getdns_tls_connection_verify(_getdns_tls_connection* conn, long* errnum, const char** errmsg); + /** * Read from TLS. *