From c05bdbd6fcc573e071652f88b468091f57a0430d Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Fri, 28 Dec 2018 11:52:43 +0000 Subject: [PATCH] TLS: refactor client-start interface --- doc/doc-txt/ChangeLog | 2 +- src/src/expand.c | 36 +++++++++---------- src/src/functions.h | 9 ++--- src/src/structs.h | 5 +++ src/src/tls-gnu.c | 73 +++++++++++++++++++------------------ src/src/tls-openssl.c | 76 +++++++++++++++++++-------------------- src/src/transports/smtp.c | 24 ++++--------- src/src/transports/smtp.h | 1 - 8 files changed, 105 insertions(+), 121 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 6b60cf073..5a42f0e7c 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -32,7 +32,7 @@ JH/07 GnuTLS: Our use of late (post-handshake) certificate verification, under TLS connection attempt, so that the normal retry-in-clear can work (if suitably configured). -JB/01 Big 2375: fix expansions of 822 addresses having comments in local-part +JB/01 Bug 2375: fix expansions of 822 addresses having comments in local-part and/or domain. Found and fixed by Jason Betts. diff --git a/src/src/expand.c b/src/src/expand.c index 6e0ba93d6..14d7ce451 100644 --- a/src/src/expand.c +++ b/src/src/expand.c @@ -4934,7 +4934,7 @@ while (*s != 0) case EITEM_READSOCK: { - int fd; + client_conn_ctx cctx; int timeout = 5; int save_ptr = yield->ptr; FILE * fp; @@ -4944,7 +4944,6 @@ while (*s != 0) host_item host; BOOL do_shutdown = TRUE; BOOL do_tls = FALSE; /* Only set under SUPPORT_TLS */ - void * tls_ctx = NULL; /* ditto */ blob reqstr; if (expand_forbid & RDO_READSOCK) @@ -5045,11 +5044,11 @@ while (*s != 0) } /*XXX we trust that the request is idempotent. Hmm. */ - fd = ip_connectedsocket(SOCK_STREAM, server_name, port, port, + cctx.sock = ip_connectedsocket(SOCK_STREAM, server_name, port, port, timeout, &host, &expand_string_message, do_tls ? NULL : &reqstr); callout_address = NULL; - if (fd < 0) + if (cctx.sock < 0) goto SOCK_FAIL; if (!do_tls) reqstr.len = 0; @@ -5062,7 +5061,7 @@ while (*s != 0) struct sockaddr_un sockun; /* don't call this "sun" ! */ int rc; - if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) == -1) + if ((cctx.sock = socket(PF_UNIX, SOCK_STREAM, 0)) == -1) { expand_string_message = string_sprintf("failed to create socket: %s", strerror(errno)); @@ -5076,7 +5075,7 @@ while (*s != 0) sigalrm_seen = FALSE; ALARM(timeout); - rc = connect(fd, (struct sockaddr *)(&sockun), sizeof(sockun)); + rc = connect(cctx.sock, (struct sockaddr *)(&sockun), sizeof(sockun)); ALARM_CLR(0); if (sigalrm_seen) { @@ -5098,14 +5097,11 @@ while (*s != 0) #ifdef SUPPORT_TLS if (do_tls) { + smtp_connect_args conn_args = {.host = &host }; tls_support tls_dummy = {.sni=NULL}; uschar * errstr; - if (!(tls_ctx = tls_client_start(fd, &host, NULL, NULL, -# ifdef SUPPORT_DANE - NULL, -# endif - &tls_dummy, &errstr))) + if (!tls_client_start(&cctx, &conn_args, NULL, &tls_dummy, &errstr)) { expand_string_message = string_sprintf("TLS connect failed: %s", errstr); goto SOCK_FAIL; @@ -5124,9 +5120,9 @@ while (*s != 0) reqstr.data); if ( ( #ifdef SUPPORT_TLS - tls_ctx ? tls_write(tls_ctx, reqstr.data, reqstr.len, FALSE) : + cctx.tls_ctx ? tls_write(cctx.tls_ctx, reqstr.data, reqstr.len, FALSE) : #endif - write(fd, reqstr.data, reqstr.len)) != reqstr.len) + write(cctx.sock, reqstr.data, reqstr.len)) != reqstr.len) { expand_string_message = string_sprintf("request write to socket " "failed: %s", strerror(errno)); @@ -5139,7 +5135,7 @@ while (*s != 0) system doesn't have this function, make it conditional. */ #ifdef SHUT_WR - if (!tls_ctx && do_shutdown) shutdown(fd, SHUT_WR); + if (!cctx.tls_ctx && do_shutdown) shutdown(cctx.sock, SHUT_WR); #endif if (f.running_in_test_harness) millisleep(100); @@ -5147,22 +5143,22 @@ while (*s != 0) /* Now we need to read from the socket, under a timeout. The function that reads a file can be used. */ - if (!tls_ctx) - fp = fdopen(fd, "rb"); + if (!cctx.tls_ctx) + fp = fdopen(cctx.sock, "rb"); sigalrm_seen = FALSE; ALARM(timeout); yield = #ifdef SUPPORT_TLS - tls_ctx ? cat_file_tls(tls_ctx, yield, sub_arg[3]) : + cctx.tls_ctx ? cat_file_tls(cctx.tls_ctx, yield, sub_arg[3]) : #endif cat_file(fp, yield, sub_arg[3]); ALARM_CLR(0); #ifdef SUPPORT_TLS - if (tls_ctx) + if (cctx.tls_ctx) { - tls_close(tls_ctx, TRUE); - close(fd); + tls_close(cctx.tls_ctx, TRUE); + close(cctx.sock); } else #endif diff --git a/src/src/functions.h b/src/src/functions.h index 27981db10..f8c0bbf8a 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -44,12 +44,9 @@ extern uschar * tls_cert_fprt_md5(void *); extern uschar * tls_cert_fprt_sha1(void *); extern uschar * tls_cert_fprt_sha256(void *); -extern void * tls_client_start(int, host_item *, address_item *, - transport_instance *, -# ifdef SUPPORT_DANE - dns_answer *, -# endif - tls_support *, uschar **); +extern BOOL tls_client_start(client_conn_ctx *, smtp_connect_args *, + void *, tls_support *, uschar **); + extern void tls_close(void *, int); extern BOOL tls_could_read(void); extern int tls_export_cert(uschar *, size_t, void *); diff --git a/src/src/structs.h b/src/src/structs.h index ac3efa587..8c229236c 100644 --- a/src/src/structs.h +++ b/src/src/structs.h @@ -800,6 +800,11 @@ typedef struct { host_item * host; int host_af; uschar * interface; + +#if defined(SUPPORT_TLS) && defined(SUPPORT_DANE) + BOOL dane:1; /* connection must do dane */ + dns_answer tlsa_dnsa; +#endif } smtp_connect_args; /* A client-initiated connection. If TLS, the second element is non-NULL */ diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c index 1752bf3d8..44a20adf8 100644 --- a/src/src/tls-gnu.c +++ b/src/src/tls-gnu.c @@ -2324,36 +2324,29 @@ return TRUE; /* Called from the smtp transport after STARTTLS has been accepted. Arguments: - fd the fd of the connection - host connected host (for messages and option-tests) - addr the first address (not used) - tb transport (always smtp) - tlsa_dnsa non-NULL, either request or require dane for this host, and - a TLSA record found. Therefore, dane verify required. - Which implies cert must be requested and supplied, dane - verify must pass, and cert verify irrelevant (incl. - hostnames), and (caller handled) require_tls - tlsp record details of channel configuration - errstr error string pointer - -Returns: Pointer to TLS session context, or NULL on error + cctx connection context + conn_args connection details + cookie datum for randomness (not used) + tlsp record details of channel configuration here; must be non-NULL + errstr error string pointer + +Returns: TRUE for success with TLS session context set in smtp context, + FALSE on error */ -void * -tls_client_start(int fd, host_item *host, - address_item *addr ARG_UNUSED, - transport_instance * tb, -#ifdef SUPPORT_DANE - dns_answer * tlsa_dnsa, -#endif - tls_support * tlsp, uschar ** errstr) +BOOL +tls_client_start(client_conn_ctx * cctx, smtp_connect_args * conn_args, + void * cookie ARG_UNUSED, + tls_support * tlsp, uschar ** errstr) { -smtp_transport_options_block *ob = tb +host_item * host = conn_args->host; /* for msgs and option-tests */ +transport_instance * tb = conn_args->tblock; /* always smtp or NULL */ +smtp_transport_options_block * ob = tb ? (smtp_transport_options_block *)tb->options_block : &smtp_transport_option_defaults; int rc; exim_gnutls_state_st * state = NULL; -uschar *cipher_list = NULL; +uschar * cipher_list = NULL; #ifndef DISABLE_OCSP BOOL require_ocsp = @@ -2362,15 +2355,20 @@ BOOL request_ocsp = require_ocsp ? TRUE : verify_check_given_host(CUSS &ob->hosts_request_ocsp, host) == OK; #endif -DEBUG(D_tls) debug_printf("initialising GnuTLS as a client on fd %d\n", fd); +DEBUG(D_tls) debug_printf("initialising GnuTLS as a client on fd %d\n", cctx->sock); #ifdef SUPPORT_DANE -if (tlsa_dnsa && ob->dane_require_tls_ciphers) +/* If dane is flagged, have either request or require dane for this host, and +a TLSA record found. Therefore, dane verify required. Which implies cert must +be requested and supplied, dane verify must pass, and cert verify irrelevant +(incl. hostnames), and (caller handled) require_tls */ + +if (conn_args->dane && ob->dane_require_tls_ciphers) { /* not using expand_check_tlsvar because not yet in state */ if (!expand_check(ob->dane_require_tls_ciphers, US"dane_require_tls_ciphers", &cipher_list, errstr)) - return NULL; + return FALSE; cipher_list = cipher_list && *cipher_list ? ob->dane_require_tls_ciphers : ob->tls_require_ciphers; } @@ -2382,7 +2380,7 @@ if (!cipher_list) if (tls_init(host, ob->tls_certificate, ob->tls_privatekey, ob->tls_sni, ob->tls_verify_certificates, ob->tls_crl, cipher_list, &state, tlsp, errstr) != OK) - return NULL; + return FALSE; { int dh_min_bits = ob->tls_dh_min_bits; @@ -2406,7 +2404,7 @@ set but both tls_verify_hosts and tls_try_verify_hosts are unset. Check only the specified host patterns if one of them is defined */ #ifdef SUPPORT_DANE -if (tlsa_dnsa && dane_tlsa_load(state, tlsa_dnsa)) +if (conn_args->dane && dane_tlsa_load(state, &conn_args->tlsa_dnsa)) { DEBUG(D_tls) debug_printf("TLS: server certificate DANE required.\n"); @@ -2453,7 +2451,7 @@ if (request_ocsp) NULL, 0, NULL)) != OK) { tls_error(US"cert-status-req", US gnutls_strerror(rc), state->host, errstr); - return NULL; + return FALSE; } tlsp->ocsp = OCSP_NOT_RESP; } @@ -2468,9 +2466,9 @@ if (tb && tb->event_action) } #endif -gnutls_transport_set_ptr(state->session, (gnutls_transport_ptr_t)(long) fd); -state->fd_in = fd; -state->fd_out = fd; +gnutls_transport_set_ptr(state->session, (gnutls_transport_ptr_t)(long) cctx->sock); +state->fd_in = cctx->sock; +state->fd_out = cctx->sock; DEBUG(D_tls) debug_printf("about to gnutls_handshake\n"); /* There doesn't seem to be a built-in timeout on connection. */ @@ -2491,7 +2489,7 @@ if (rc != GNUTLS_E_SUCCESS) } else tls_error(US"gnutls_handshake", US gnutls_strerror(rc), state->host, errstr); - return NULL; + return FALSE; } DEBUG(D_tls) @@ -2518,7 +2516,7 @@ DEBUG(D_tls) if (!verify_certificate(state, errstr)) { tls_error(US"certificate verification failed", *errstr, state->host, errstr); - return NULL; + return FALSE; } #ifndef DISABLE_OCSP @@ -2546,7 +2544,7 @@ if (require_ocsp) { tlsp->ocsp = OCSP_FAILED; tls_error(US"certificate status check failed", NULL, state->host, errstr); - return NULL; + return FALSE; } DEBUG(D_tls) debug_printf("Passed OCSP checking\n"); tlsp->ocsp = OCSP_VFIED; @@ -2556,13 +2554,14 @@ if (require_ocsp) /* Figure out peer DN, and if authenticated, etc. */ if (peer_status(state, errstr) != OK) - return NULL; + return FALSE; /* Sets various Exim expansion variables; may need to adjust for ACL callouts */ extract_exim_vars_from_tls_state(state); -return state; +cctx->tls_ctx = state; +return TRUE; } diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c index ba9e7da11..306877f22 100644 --- a/src/src/tls-openssl.c +++ b/src/src/tls-openssl.c @@ -2490,33 +2490,30 @@ return DEFER; /* Called from the smtp transport after STARTTLS has been accepted. -Argument: - fd the fd of the connection - host connected host (for messages and option-tests) - addr the first address (for some randomness; can be NULL) - tb transport (always smtp) - tlsa_dnsa tlsa lookup, if DANE, else null - tlsp record details of channel configuration here; must be non-NULL - errstr error string pointer - -Returns: Pointer to TLS session context, or NULL on error +Arguments: + cctx connection context + conn_args connection details + cookie datum for randomness; can be NULL + tlsp record details of TLS channel configuration here; must be non-NULL + errstr error string pointer + +Returns: TRUE for success with TLS session context set in connection context, + FALSE on error */ -void * -tls_client_start(int fd, host_item *host, address_item *addr, - transport_instance * tb, -#ifdef SUPPORT_DANE - dns_answer * tlsa_dnsa, -#endif - tls_support * tlsp, uschar ** errstr) +BOOL +tls_client_start(client_conn_ctx * cctx, smtp_connect_args * conn_args, + void * cookie, tls_support * tlsp, uschar ** errstr) { +host_item * host = conn_args->host; /* for msgs and option-tests */ +transport_instance * tb = conn_args->tblock; /* always smtp or NULL */ smtp_transport_options_block * ob = tb ? (smtp_transport_options_block *)tb->options_block : &smtp_transport_option_defaults; exim_openssl_client_tls_ctx * exim_client_ctx; -static uschar peerdn[256]; uschar * expciphers; int rc; +static uschar peerdn[256]; #ifndef DISABLE_OCSP BOOL request_ocsp = FALSE; @@ -2535,7 +2532,7 @@ tlsp->tlsa_usage = 0; #ifndef DISABLE_OCSP { # ifdef SUPPORT_DANE - if ( tlsa_dnsa + if ( conn_args->dane && ob->hosts_request_ocsp[0] == '*' && ob->hosts_request_ocsp[1] == '\0' ) @@ -2565,22 +2562,22 @@ rc = tls_init(&exim_client_ctx->ctx, host, NULL, #ifndef DISABLE_OCSP (void *)(long)request_ocsp, #endif - addr, &client_static_cbinfo, errstr); -if (rc != OK) return NULL; + cookie, &client_static_cbinfo, errstr); +if (rc != OK) return FALSE; tlsp->certificate_verified = FALSE; client_verify_callback_called = FALSE; expciphers = NULL; #ifdef SUPPORT_DANE -if (tlsa_dnsa) +if (conn_args->dane) { /* We fall back to tls_require_ciphers if unset, empty or forced failure, but other failures should be treated as problems. */ if (ob->dane_require_tls_ciphers && !expand_check(ob->dane_require_tls_ciphers, US"dane_require_tls_ciphers", &expciphers, errstr)) - return NULL; + return FALSE; if (expciphers && *expciphers == '\0') expciphers = NULL; } @@ -2588,7 +2585,7 @@ if (tlsa_dnsa) if (!expciphers && !expand_check(ob->tls_require_ciphers, US"tls_require_ciphers", &expciphers, errstr)) - return NULL; + return FALSE; /* In OpenSSL, cipher components are separated by hyphens. In GnuTLS, they are separated by underscores. So that I can use either form in my tests, and @@ -2602,12 +2599,12 @@ if (expciphers) if (!SSL_CTX_set_cipher_list(exim_client_ctx->ctx, CS expciphers)) { tls_error(US"SSL_CTX_set_cipher_list", host, NULL, errstr); - return NULL; + return FALSE; } } #ifdef SUPPORT_DANE -if (tlsa_dnsa) +if (conn_args->dane) { SSL_CTX_set_verify(exim_client_ctx->ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, @@ -2616,12 +2613,12 @@ if (tlsa_dnsa) if (!DANESSL_library_init()) { tls_error(US"library init", host, NULL, errstr); - return NULL; + return FALSE; } if (DANESSL_CTX_init(exim_client_ctx->ctx) <= 0) { tls_error(US"context init", host, NULL, errstr); - return NULL; + return FALSE; } } else @@ -2630,21 +2627,21 @@ else if (tls_client_basic_ctx_init(exim_client_ctx->ctx, host, ob, client_static_cbinfo, errstr) != OK) - return NULL; + return FALSE; if (!(exim_client_ctx->ssl = SSL_new(exim_client_ctx->ctx))) { tls_error(US"SSL_new", host, NULL, errstr); - return NULL; + return FALSE; } SSL_set_session_id_context(exim_client_ctx->ssl, sid_ctx, Ustrlen(sid_ctx)); -SSL_set_fd(exim_client_ctx->ssl, fd); +SSL_set_fd(exim_client_ctx->ssl, cctx->sock); SSL_set_connect_state(exim_client_ctx->ssl); if (ob->tls_sni) { if (!expand_check(ob->tls_sni, US"tls_sni", &tlsp->sni, errstr)) - return NULL; + return FALSE; if (!tlsp->sni) { DEBUG(D_tls) debug_printf("Setting TLS SNI forced to fail, not sending\n"); @@ -2664,9 +2661,9 @@ if (ob->tls_sni) } #ifdef SUPPORT_DANE -if (tlsa_dnsa) - if (dane_tlsa_load(exim_client_ctx->ssl, host, tlsa_dnsa, errstr) != OK) - return NULL; +if (conn_args->dane) + if (dane_tlsa_load(exim_client_ctx->ssl, host, &conn_args->tlsa_dnsa, errstr) != OK) + return FALSE; #endif #ifndef DISABLE_OCSP @@ -2710,14 +2707,14 @@ rc = SSL_connect(exim_client_ctx->ssl); ALARM_CLR(0); #ifdef SUPPORT_DANE -if (tlsa_dnsa) +if (conn_args->dane) DANESSL_cleanup(exim_client_ctx->ssl); #endif if (rc <= 0) { tls_error(US"SSL_connect", host, sigalrm_seen ? US"timed out" : NULL, errstr); - return NULL; + return FALSE; } DEBUG(D_tls) @@ -2747,9 +2744,10 @@ tlsp->cipher_stdname = cipher_stdname_ssl(exim_client_ctx->ssl); tlsp->ourcert = crt ? X509_dup(crt) : NULL; } -tlsp->active.sock = fd; +tlsp->active.sock = cctx->sock; tlsp->active.tls_ctx = exim_client_ctx; -return exim_client_ctx; +cctx->tls_ctx = exim_client_ctx; +return TRUE; } diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index 6ff993d57..5fd278e1d 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -1949,9 +1949,6 @@ Returns: OK - the connection was made and the delivery attempted; int smtp_setup_conn(smtp_context * sx, BOOL suppress_tls) { -#if defined(SUPPORT_TLS) && defined(SUPPORT_DANE) -dns_answer tlsa_dnsa; -#endif smtp_transport_options_block * ob = sx->conn_args.tblock->options_block; BOOL pass_message = FALSE; uschar * message = NULL; @@ -1976,7 +1973,7 @@ sx->utf8_needed = FALSE; #endif sx->dsn_all_lasthop = TRUE; #if defined(SUPPORT_TLS) && defined(SUPPORT_DANE) -sx->dane = FALSE; +sx->conn_args.dane = FALSE; sx->dane_required = verify_check_given_host(CUSS &ob->hosts_require_dane, sx->conn_args.host) == OK; #endif @@ -2067,9 +2064,9 @@ if (!continue_hostname) if( sx->dane_required || verify_check_given_host(CUSS &ob->hosts_try_dane, sx->conn_args.host) == OK ) - switch (rc = tlsa_lookup(sx->conn_args.host, &tlsa_dnsa, sx->dane_required)) + switch (rc = tlsa_lookup(sx->conn_args.host, &sx->conn_args.tlsa_dnsa, sx->dane_required)) { - case OK: sx->dane = TRUE; + case OK: sx->conn_args.dane = TRUE; ob->tls_tempfail_tryclear = FALSE; break; case FAIL_FORCED: break; @@ -2500,14 +2497,7 @@ if ( smtp_peer_options & OPTION_TLS else TLS_NEGOTIATE: { - sx->cctx.tls_ctx = tls_client_start(sx->cctx.sock, sx->conn_args.host, - sx->addrlist, sx->conn_args.tblock, -# ifdef SUPPORT_DANE - sx->dane ? &tlsa_dnsa : NULL, -# endif - &tls_out, &tls_errstr); - - if (!sx->cctx.tls_ctx) + if (!tls_client_start(&sx->cctx, &sx->conn_args, sx->addrlist, &tls_out, &tls_errstr)) { /* TLS negotiation failed; give an error. From outside, this function may be called again to try in clear on a new connection, if the options permit @@ -2516,7 +2506,7 @@ GNUTLS_CONN_FAILED: DEBUG(D_tls) debug_printf("TLS session fail: %s\n", tls_errstr); # ifdef SUPPORT_DANE - if (sx->dane) + if (sx->conn_args.dane) { log_write(0, LOG_MAIN, "DANE attempt failed; TLS connection to %s [%s]: %s", @@ -2652,7 +2642,7 @@ have one. */ else if ( sx->smtps # ifdef SUPPORT_DANE - || sx->dane + || sx->conn_args.dane # endif # ifdef EXPERIMENTAL_REQUIRETLS || tls_requiretls & REQUIRETLS_MSG @@ -2669,7 +2659,7 @@ else if ( sx->smtps smtp_peer_options & OPTION_TLS ? "an attempt to start TLS failed" : "the server did not offer TLS support"); # if defined(SUPPORT_DANE) && !defined(DISABLE_EVENT) - if (sx->dane) + if (sx->conn_args.dane) (void) event_raise(sx->conn_args.tblock->event_action, US"dane:fail", smtp_peer_options & OPTION_TLS ? US"validation-failure" /* could do with better detail */ diff --git a/src/src/transports/smtp.h b/src/src/transports/smtp.h index 57f87a296..056efbcf7 100644 --- a/src/src/transports/smtp.h +++ b/src/src/transports/smtp.h @@ -133,7 +133,6 @@ typedef struct { #endif BOOL dsn_all_lasthop:1; #if defined(SUPPORT_TLS) && defined(SUPPORT_DANE) - BOOL dane:1; BOOL dane_required:1; #endif #ifdef EXPERIMENTAL_PIPE_CONNECT -- 2.25.1