From fee864c25cc8537803796daeb14b9b36a9ebf59b Mon Sep 17 00:00:00 2001 From: Jim Hague Date: Fri, 7 Dec 2018 12:38:17 +0000 Subject: [PATCH] Implement setting cipher/curve lists. Set the priority string to a concatenation of the connection cipher and curve strings, falling back to the context ones if the connection value isn't specified. Also get context.c to specify NULL for default context list and the opportunistic list for the connection, moving these library-specific quantities into the specific implementation. --- src/context.c | 10 +--- src/gnutls/tls-internal.h | 6 ++ src/gnutls/tls.c | 116 ++++++++++++++++++++++++++++++++++---- src/openssl/tls.c | 16 ++++++ src/stub.c | 2 +- src/tls.h | 13 +++-- 6 files changed, 139 insertions(+), 24 deletions(-) diff --git a/src/context.c b/src/context.c index ddda54de..6e919114 100644 --- a/src/context.c +++ b/src/context.c @@ -1343,10 +1343,6 @@ static char const * const _getdns_default_trust_anchors_verify_CA = static char const * const _getdns_default_trust_anchors_verify_email = "dnssec@iana.org"; -static char const * const _getdns_default_tls_cipher_list = - "TLS13-AES-256-GCM-SHA384:TLS13-AES-128-GCM-SHA256:" - "TLS13-CHACHA20-POLY1305-SHA256:EECDH+AESGCM:EECDH+CHACHA20"; - /* * getdns_context_create * @@ -3556,9 +3552,7 @@ _getdns_context_prepare_for_resolution(getdns_context *context) } /* Be strict and only use the cipher suites recommended in RFC7525 Unless we later fallback to opportunistic. */ - if (_getdns_tls_context_set_cipher_list(context->tls_ctx, - context->tls_cipher_list ? context->tls_cipher_list - : _getdns_default_tls_cipher_list)) + if (_getdns_tls_context_set_cipher_list(context->tls_ctx, context->tls_cipher_list)) return GETDNS_RETURN_BAD_CONTEXT; if (context->tls_curves_list && @@ -5277,7 +5271,7 @@ getdns_context_get_tls_cipher_list( *tls_cipher_list = context->tls_cipher_list ? context->tls_cipher_list - : _getdns_default_tls_cipher_list; + : _getdns_tls_context_default_cipher_list; return GETDNS_RETURN_GOOD; } diff --git a/src/gnutls/tls-internal.h b/src/gnutls/tls-internal.h index 15115b4d..d9ac1974 100644 --- a/src/gnutls/tls-internal.h +++ b/src/gnutls/tls-internal.h @@ -54,6 +54,9 @@ typedef struct _getdns_tls_context { + struct mem_funcs* mfs; + char* cipher_list; + char* curve_list; bool min_proto_1_2; } _getdns_tls_context; @@ -62,6 +65,9 @@ typedef struct _getdns_tls_connection { gnutls_certificate_credentials_t cred; int shutdown; _getdns_tls_context* ctx; + struct mem_funcs* mfs; + char* cipher_list; + char* curve_list; } _getdns_tls_connection; typedef struct _getdns_tls_session { diff --git a/src/gnutls/tls.c b/src/gnutls/tls.c index 5a2b6c94..a60dcddf 100644 --- a/src/gnutls/tls.c +++ b/src/gnutls/tls.c @@ -40,6 +40,75 @@ #include "tls.h" +/* + * Cipher suites recommended in RFC7525. + * + * The GnuTLS 3.5.19 being used for this proof of concept doesn't have + * TLS 1.3 support, as in the OpenSSL equivalent. Fall back for now to + * a known working priority string. + */ +char const * const _getdns_tls_context_default_cipher_list = + "SECURE192:-VERS-ALL:+VERS-TLS1.2"; + +static char const * const _getdns_tls_connection_opportunistic_cipher_list = + "NORMAL"; + +static char* getdns_strdup(struct mem_funcs* mfs, const char* s) +{ + char* res; + + if (!s) + return NULL; + + res = GETDNS_XMALLOC(*mfs, char, strlen(s) + 1); + if (!res) + return NULL; + strcpy(res, s); + return res; +} + +static char* getdns_priappend(struct mem_funcs* mfs, char* s1, const char* s2) +{ + char* res; + + if (!s1) + return getdns_strdup(mfs, s2); + if (!s2) + return s1; + + res = GETDNS_XMALLOC(*mfs, char, strlen(s1) + strlen(s2) + 2); + if (!res) + return NULL; + strcpy(res, s1); + strcat(res, ":"); + strcat(res, s2); + GETDNS_FREE(*mfs, s1); + return res; +} + +static int set_connection_ciphers(_getdns_tls_connection* conn) +{ + char* pri = NULL; + int res; + + if (conn->cipher_list) + pri = getdns_priappend(conn->mfs, pri, conn->cipher_list); + else if (conn->ctx->cipher_list) + pri = getdns_priappend(conn->mfs, pri, conn->ctx->cipher_list); + + if (conn->curve_list) + pri = getdns_priappend(conn->mfs, pri, conn->curve_list); + else if (conn->ctx->curve_list) + pri = getdns_priappend(conn->mfs, pri, conn->ctx->curve_list); + + if (pri) + res = gnutls_priority_set_direct(conn->tls, pri, NULL); + else + res = gnutls_set_default_priority(conn->tls); + GETDNS_FREE(*conn->mfs, pri); + return res; +} + static getdns_return_t error_may_want_read_write(_getdns_tls_connection* conn, int err) { switch (err) { @@ -95,7 +164,9 @@ _getdns_tls_context* _getdns_tls_context_new(struct mem_funcs* mfs) if (!(res = GETDNS_MALLOC(*mfs, struct _getdns_tls_context))) return NULL; + res->mfs = mfs; res->min_proto_1_2 = false; + res->cipher_list = res->curve_list = NULL; return res; } @@ -103,6 +174,8 @@ getdns_return_t _getdns_tls_context_free(struct mem_funcs* mfs, _getdns_tls_cont { if (!ctx) return GETDNS_RETURN_INVALID_PARAMETER; + GETDNS_FREE(*mfs, ctx->curve_list); + GETDNS_FREE(*mfs, ctx->cipher_list); GETDNS_FREE(*mfs, ctx); return GETDNS_RETURN_GOOD; } @@ -122,19 +195,24 @@ getdns_return_t _getdns_tls_context_set_min_proto_1_2(_getdns_tls_context* ctx) getdns_return_t _getdns_tls_context_set_cipher_list(_getdns_tls_context* ctx, const char* list) { - (void) list; - if (!ctx) return GETDNS_RETURN_INVALID_PARAMETER; + + if (!list) + list = _getdns_tls_context_default_cipher_list; + + GETDNS_FREE(*ctx->mfs, ctx->cipher_list); + ctx->cipher_list = getdns_strdup(ctx->mfs, list); return GETDNS_RETURN_GOOD; } getdns_return_t _getdns_tls_context_set_curves_list(_getdns_tls_context* ctx, const char* list) { - (void) list; - if (!ctx) return GETDNS_RETURN_INVALID_PARAMETER; + + GETDNS_FREE(*ctx->mfs, ctx->curve_list); + ctx->curve_list = getdns_strdup(ctx->mfs, list); return GETDNS_RETURN_GOOD; } @@ -161,6 +239,9 @@ _getdns_tls_connection* _getdns_tls_connection_new(struct mem_funcs* mfs, _getdn res->shutdown = 0; res->ctx = ctx; + res->mfs = mfs; + res->cipher_list = NULL; + res->curve_list = NULL; r = gnutls_certificate_allocate_credentials(&res->cred); if (r == GNUTLS_E_SUCCESS) @@ -168,7 +249,7 @@ _getdns_tls_connection* _getdns_tls_connection_new(struct mem_funcs* mfs, _getdn if (r == GNUTLS_E_SUCCESS) r = gnutls_init(&res->tls, GNUTLS_CLIENT | GNUTLS_NONBLOCK); if (r == GNUTLS_E_SUCCESS) - r = gnutls_set_default_priority(res->tls); + r = set_connection_ciphers(res); if (r == GNUTLS_E_SUCCESS) r = gnutls_credentials_set(res->tls, GNUTLS_CRD_CERTIFICATE, res->cred); if (r != GNUTLS_E_SUCCESS) { @@ -187,6 +268,8 @@ getdns_return_t _getdns_tls_connection_free(struct mem_funcs* mfs, _getdns_tls_c gnutls_deinit(conn->tls); gnutls_certificate_free_credentials(conn->cred); + GETDNS_FREE(*mfs, conn->curve_list); + GETDNS_FREE(*mfs, conn->cipher_list); GETDNS_FREE(*mfs, conn); return GETDNS_RETURN_GOOD; } @@ -209,20 +292,31 @@ getdns_return_t _getdns_tls_connection_shutdown(_getdns_tls_connection* conn) getdns_return_t _getdns_tls_connection_set_cipher_list(_getdns_tls_connection* conn, const char* list) { - (void) list; - if (!conn || !conn->tls) return GETDNS_RETURN_INVALID_PARAMETER; - return GETDNS_RETURN_GOOD; + + if (!list) + list = _getdns_tls_connection_opportunistic_cipher_list; + + GETDNS_FREE(*conn->mfs, conn->cipher_list); + conn->cipher_list = getdns_strdup(conn->mfs, list); + if (set_connection_ciphers(conn) == GNUTLS_E_SUCCESS) + return GETDNS_RETURN_GOOD; + else + return GETDNS_RETURN_GENERIC_ERROR; } getdns_return_t _getdns_tls_connection_set_curves_list(_getdns_tls_connection* conn, const char* list) { - (void) list; - if (!conn || !conn->tls) return GETDNS_RETURN_INVALID_PARAMETER; - return GETDNS_RETURN_GOOD; + + GETDNS_FREE(*conn->mfs, conn->curve_list); + conn->curve_list = getdns_strdup(conn->mfs, list); + if (set_connection_ciphers(conn) == GNUTLS_E_SUCCESS) + return GETDNS_RETURN_GOOD; + else + return GETDNS_RETURN_GENERIC_ERROR; } getdns_return_t _getdns_tls_connection_set_session(_getdns_tls_connection* conn, _getdns_tls_session* s) diff --git a/src/openssl/tls.c b/src/openssl/tls.c index dcc68494..82fa0870 100644 --- a/src/openssl/tls.c +++ b/src/openssl/tls.c @@ -49,6 +49,14 @@ #include "tls.h" +/* Cipher suites recommended in RFC7525. */ +char const * const _getdns_tls_context_default_cipher_list = + "TLS13-AES-256-GCM-SHA384:TLS13-AES-128-GCM-SHA256:" + "TLS13-CHACHA20-POLY1305-SHA256:EECDH+AESGCM:EECDH+CHACHA20"; + +static char const * const _getdns_tls_connection_opportunistic_cipher_list = + "DEFAULT"; + static int _getdns_tls_verify_always_ok(int ok, X509_STORE_CTX *ctx) { # if defined(STUB_DEBUG) && STUB_DEBUG @@ -275,6 +283,10 @@ getdns_return_t _getdns_tls_context_set_cipher_list(_getdns_tls_context* ctx, co { if (!ctx || !ctx->ssl) return GETDNS_RETURN_INVALID_PARAMETER; + + if (!list) + list = _getdns_tls_context_default_cipher_list; + if (!SSL_CTX_set_cipher_list(ctx->ssl, list)) return GETDNS_RETURN_BAD_CONTEXT; return GETDNS_RETURN_GOOD; @@ -366,6 +378,10 @@ getdns_return_t _getdns_tls_connection_set_cipher_list(_getdns_tls_connection* c { if (!conn || !conn->ssl) return GETDNS_RETURN_INVALID_PARAMETER; + + if (!list) + list = _getdns_tls_connection_opportunistic_cipher_list; + if (!SSL_set_cipher_list(conn->ssl, list)) return GETDNS_RETURN_BAD_CONTEXT; return GETDNS_RETURN_GOOD; diff --git a/src/stub.c b/src/stub.c index b6a9cdf1..7cbfe9a2 100644 --- a/src/stub.c +++ b/src/stub.c @@ -875,7 +875,7 @@ tls_create_object(getdns_dns_req *dnsreq, int fd, getdns_upstream *upstream) } } if (upstream->tls_fallback_ok) { - _getdns_tls_connection_set_cipher_list(tls, "DEFAULT"); + _getdns_tls_connection_set_cipher_list(tls, NULL); DEBUG_STUB("%s %-35s: WARNING: Using Oppotunistic TLS (fallback allowed)!\n", STUB_DEBUG_SETUP_TLS, __FUNC__); } else { diff --git a/src/tls.h b/src/tls.h index fe05dc52..e58d4ce3 100644 --- a/src/tls.h +++ b/src/tls.h @@ -94,7 +94,7 @@ getdns_return_t _getdns_tls_context_set_min_proto_1_2(_getdns_tls_context* ctx); * Set list of allowed ciphers. * * @param ctx the context. - * @param list the list of cipher identifiers. + * @param list the list of cipher identifiers. NULL for default setting. * @return GETDNS_RETURN_GOOD on success. * @return GETDNS_RETURN_INVALID_PARAMETER on bad context pointer. * @return GETDNS_RETURN_BAD_CONTEXT on failure. @@ -105,7 +105,7 @@ getdns_return_t _getdns_tls_context_set_cipher_list(_getdns_tls_context* ctx, co * Set list of allowed curves. * * @param ctx the context. - * @param list the list of curve identifiers. + * @param list the list of curve identifiers. NULL for default setting. * @return GETDNS_RETURN_GOOD on success. * @return GETDNS_RETURN_INVALID_PARAMETER on bad context pointer. * @return GETDNS_RETURN_BAD_CONTEXT on failure. @@ -164,7 +164,7 @@ getdns_return_t _getdns_tls_connection_shutdown(_getdns_tls_connection* conn); * Set list of allowed ciphers on this connection. * * @param conn the connection. - * @param list the list of cipher identifiers. + * @param list the list of cipher identifiers. NULL for opportunistic setting. * @return GETDNS_RETURN_GOOD on success. * @return GETDNS_RETURN_INVALID_PARAMETER on bad connection pointer. * @return GETDNS_RETURN_BAD_CONTEXT on failure. @@ -175,7 +175,7 @@ getdns_return_t _getdns_tls_connection_set_cipher_list(_getdns_tls_connection* c * Set list of allowed curves on this connection. * * @param conn the connection. - * @param list the list of curve identifiers. + * @param list the list of curve identifiers. NULL for default setting. * @return GETDNS_RETURN_GOOD on success. * @return GETDNS_RETURN_INVALID_PARAMETER on bad connection pointer. * @return GETDNS_RETURN_BAD_CONTEXT on failure. @@ -407,4 +407,9 @@ void _getdns_tls_sha1(const void* data, size_t data_size, unsigned char* buf); */ void _getdns_tls_cookie_sha256(uint32_t secret, void* addr, size_t addrlen, unsigned char* buf, size_t* buflen); +/** + * Default context cipher list. + */ +const char* const _getdns_tls_context_default_cipher_list; + #endif /* _GETDNS_TLS_H */