From c18e2c3b059f6bfd1c6e9a65ffc8243a4d8034fe Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sat, 16 Feb 2019 12:59:23 +0000 Subject: [PATCH] GnuTLS: Fix client detection of server reject of client cert under TLS1.3 (cherry picked from commit fc243e944ec00b59b75f41d07494116f925d58b4) (cherry picked from commit c15523829ba17cce5829e2976aa1ff928965d948) --- doc/doc-txt/ChangeLog | 7 +++ src/src/deliver.c | 2 +- src/src/smtp_out.c | 10 +++-- src/src/tls-gnu.c | 23 +++------- src/src/transports/lmtp.c | 3 +- src/src/transports/smtp.c | 81 +++++++++++++++++++++++++++-------- test/confs/2027 | 8 ++-- test/confs/5652 | 1 + test/confs/5821 | 2 +- test/log/2027 | 2 +- test/runtest | 14 ++++++ test/scripts/2000-GnuTLS/2027 | 2 + 12 files changed, 111 insertions(+), 44 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 1e4322491..363dc934e 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -17,6 +17,13 @@ Since version 4.92 JH/06 Fix buggy handling of autoreply bounce_return_size_limit, and a possible buffer overrun for (non-chunking) other transports. +JH/07 GnuTLS: Our use of late (post-handshake) certificate verification, under + TLS1.3, means that a server rejecting a client certificate is not visible + to the client until the first read of encrypted data (typically the + response to EHLO). Add detection for that case and treat it as a failed + TLS connection attempt, so that the normal retry-in-clear can work (if + suitably configured). + Exim version 4.92 ----------------- diff --git a/src/src/deliver.c b/src/src/deliver.c index 664d00452..e17994117 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -7433,7 +7433,7 @@ if (addr_senddsn) tctx.u.fd = fd; tctx.options = topt_add_return_path | topt_no_body; - /*XXX hmm, retval ignored. + /*XXX hmm, FALSE(fail) retval ignored. Could error for any number of reasons, and they are not handled. */ transport_write_message(&tctx, 0); fflush(f); diff --git a/src/src/smtp_out.c b/src/src/smtp_out.c index 9bd90c77a..b194e8043 100644 --- a/src/src/smtp_out.c +++ b/src/src/smtp_out.c @@ -688,20 +688,22 @@ Returns: TRUE if a valid, non-error response was received; else FALSE /*XXX could move to smtp transport; no other users */ BOOL -smtp_read_response(void * sx0, uschar *buffer, int size, int okdigit, +smtp_read_response(void * sx0, uschar * buffer, int size, int okdigit, int timeout) { smtp_context * sx = sx0; -uschar *ptr = buffer; -int count = 0; +uschar * ptr = buffer; +int count = 0, rc; errno = 0; /* Ensure errno starts out zero */ #ifdef EXPERIMENTAL_PIPE_CONNECT if (sx->pending_BANNER || sx->pending_EHLO) - if (smtp_reap_early_pipe(sx, &count) != OK) + if ((rc = smtp_reap_early_pipe(sx, &count)) != OK) { DEBUG(D_transport) debug_printf("failed reaping pipelined cmd responsess\n"); + buffer[0] = '\0'; + if (rc == DEFER) errno = ERRNO_TLSFAILURE; return FALSE; } #endif diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c index c404dc29a..de2d70c09 100644 --- a/src/src/tls-gnu.c +++ b/src/src/tls-gnu.c @@ -229,7 +229,7 @@ static gnutls_dh_params_t dh_server_params = NULL; static const int ssl_session_timeout = 200; -static const char * const exim_default_gnutls_priority = "NORMAL"; +static const uschar * const exim_default_gnutls_priority = US"NORMAL"; /* Guard library core initialisation */ @@ -1278,7 +1278,6 @@ int rc; size_t sz; const char *errpos; uschar *p; -BOOL want_default_priorities; if (!exim_gnutls_base_init_done) { @@ -1387,32 +1386,24 @@ and replaces gnutls_require_kx, gnutls_require_mac & gnutls_require_protocols. This was backwards incompatible, but means Exim no longer needs to track all algorithms and provide string forms for them. */ -want_default_priorities = TRUE; - +p = NULL; if (state->tls_require_ciphers && *state->tls_require_ciphers) { if (!expand_check_tlsvar(tls_require_ciphers, errstr)) return DEFER; if (state->exp_tls_require_ciphers && *state->exp_tls_require_ciphers) { - DEBUG(D_tls) debug_printf("GnuTLS session cipher/priority \"%s\"\n", - state->exp_tls_require_ciphers); - - rc = gnutls_priority_init(&state->priority_cache, - CS state->exp_tls_require_ciphers, &errpos); - want_default_priorities = FALSE; p = state->exp_tls_require_ciphers; + DEBUG(D_tls) debug_printf("GnuTLS session cipher/priority \"%s\"\n", p); } } -if (want_default_priorities) +if (!p) { + p = exim_default_gnutls_priority; DEBUG(D_tls) - debug_printf("GnuTLS using default session cipher/priority \"%s\"\n", - exim_default_gnutls_priority); - rc = gnutls_priority_init(&state->priority_cache, - exim_default_gnutls_priority, &errpos); - p = US exim_default_gnutls_priority; + debug_printf("GnuTLS using default session cipher/priority \"%s\"\n", p); } +rc = gnutls_priority_init(&state->priority_cache, CCS p, &errpos); exim_gnutls_err_check(rc, string_sprintf( "gnutls_priority_init(%s) failed at offset %ld, \"%.6s..\"", diff --git a/src/src/transports/lmtp.c b/src/src/transports/lmtp.c index 240d78b21..57b346d49 100644 --- a/src/src/transports/lmtp.c +++ b/src/src/transports/lmtp.c @@ -122,7 +122,8 @@ Arguments: Returns: TRUE if a "QUIT" command should be sent, else FALSE */ -static BOOL check_response(int *errno_value, int more_errno, uschar *buffer, +static BOOL +check_response(int *errno_value, int more_errno, uschar *buffer, int *yield, uschar **message) { *yield = '4'; /* Default setting is to give a temporary error */ diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index a351da847..bfd6018dc 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -594,6 +594,11 @@ switch(*errno_value) pl, smtp_command, s); return FALSE; + case ERRNO_TLSFAILURE: /* Handle bad first read; can happen with + GnuTLS and TLS1.3 */ + *message = US"bad first read from TLS conn"; + return TRUE; + case ERRNO_FILTER_FAIL: /* Handle a failed filter process error; can't send QUIT as we mustn't end the DATA. */ *message = string_sprintf("transport filter process failed (%d)%s", @@ -942,6 +947,7 @@ Arguments: Return: OK all well + DEFER error on first read of TLS'd conn FAIL SMTP error in response */ int @@ -949,6 +955,7 @@ smtp_reap_early_pipe(smtp_context * sx, int * countp) { BOOL pending_BANNER = sx->pending_BANNER; BOOL pending_EHLO = sx->pending_EHLO; +int rc = FAIL; sx->pending_BANNER = FALSE; /* clear early to avoid recursion */ sx->pending_EHLO = FALSE; @@ -960,6 +967,7 @@ if (pending_BANNER) if (!smtp_reap_banner(sx)) { DEBUG(D_transport) debug_printf("bad banner\n"); + if (tls_out.active.sock >= 0) rc = DEFER; goto fail; } } @@ -974,6 +982,7 @@ if (pending_EHLO) if (!smtp_reap_ehlo(sx)) { DEBUG(D_transport) debug_printf("bad response for EHLO\n"); + if (tls_out.active.sock >= 0) rc = DEFER; goto fail; } @@ -1011,7 +1020,7 @@ return OK; fail: invalidate_ehlo_cache_entry(sx); (void) smtp_discard_responses(sx, sx->conn_args.ob, *countp); - return FAIL; + return rc; } #endif @@ -1056,6 +1065,7 @@ Returns: 3 if at least one address had 2xx and one had 5xx -2 I/O or other non-response error for RCPT -3 DATA or MAIL failed - errno and buffer set -4 banner or EHLO failed (early-pipelining) + -5 banner or EHLO failed (early-pipelining, TLS) */ static int @@ -1064,10 +1074,11 @@ sync_responses(smtp_context * sx, int count, int pending_DATA) address_item * addr = sx->sync_addr; smtp_transport_options_block * ob = sx->conn_args.ob; int yield = 0; +int rc; #ifdef EXPERIMENTAL_PIPE_CONNECT -if (smtp_reap_early_pipe(sx, &count) != OK) - return -4; +if ((rc = smtp_reap_early_pipe(sx, &count)) != OK) + return rc == FAIL ? -4 : -5; #endif /* Handle the response for a MAIL command. On error, reinstate the original @@ -1083,6 +1094,8 @@ if (sx->pending_MAIL) { DEBUG(D_transport) debug_printf("bad response for MAIL\n"); Ustrcpy(big_buffer, mail_command); /* Fits, because it came from there! */ + if (errno == ERRNO_TLSFAILURE) + return -5; if (errno == 0 && sx->buffer[0] != 0) { int save_errno = 0; @@ -1141,6 +1154,11 @@ while (count-- > 0) } } + /* Error on first TLS read */ + + else if (errno == ERRNO_TLSFAILURE) + return -5; + /* Timeout while reading the response */ else if (errno == ETIMEDOUT) @@ -1253,6 +1271,10 @@ if (pending_DATA != 0) int code; uschar *msg; BOOL pass_message; + + if (errno == ERRNO_TLSFAILURE) /* Error on first TLS read */ + return -5; + if (pending_DATA > 0 || (yield & 1) != 0) { if (errno == 0 && sx->buffer[0] == '4') @@ -1802,7 +1824,9 @@ Args: tc_chunk_last add LAST option to SMTP BDAT command tc_reap_prev reap response to previous SMTP commands -Returns: OK or ERROR +Returns: + OK or ERROR + DEFER TLS error on first read (EHLO-resp); errno set */ static int @@ -1859,10 +1883,12 @@ if (flags & tc_reap_prev && prev_cmd_count > 0) case 2: sx->completed_addr = TRUE; /* 5xx (only) => progress made */ case 0: break; /* No 2xx or 5xx, but no probs */ - case -1: /* Timeout on RCPT */ + case -5: errno = ERRNO_TLSFAILURE; + return DEFER; #ifdef EXPERIMENTAL_PIPE_CONNECT case -4: /* non-2xx for pipelined banner or EHLO */ #endif + case -1: /* Timeout on RCPT */ default: return ERROR; /* I/O error, or any MAIL/DATA error */ } cmd_count = 1; @@ -1933,6 +1959,9 @@ BOOL pass_message = FALSE; uschar * message = NULL; int yield = OK; int rc; +#ifdef SUPPORT_TLS +uschar * tls_errstr; +#endif sx->conn_args.ob = ob; @@ -2474,27 +2503,27 @@ if ( smtp_peer_options & OPTION_TLS TLS_NEGOTIATE: { address_item * addr; - uschar * errstr; 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, &errstr); + &tls_out, &tls_errstr); if (!sx->cctx.tls_ctx) { /* 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 it for this host. */ - DEBUG(D_tls) debug_printf("TLS session fail: %s\n", errstr); +GNUTLS_CONN_FAILED: + DEBUG(D_tls) debug_printf("TLS session fail: %s\n", tls_errstr); # ifdef SUPPORT_DANE if (sx->dane) { log_write(0, LOG_MAIN, "DANE attempt failed; TLS connection to %s [%s]: %s", - sx->conn_args.host->name, sx->conn_args.host->address, errstr); + sx->conn_args.host->name, sx->conn_args.host->address, tls_errstr); # ifndef DISABLE_EVENT (void) event_raise(sx->conn_args.tblock->event_action, US"dane:fail", US"validation-failure"); /* could do with better detail */ @@ -2503,7 +2532,7 @@ if ( smtp_peer_options & OPTION_TLS # endif errno = ERRNO_TLSFAILURE; - message = string_sprintf("TLS session: %s", errstr); + message = string_sprintf("TLS session: %s", tls_errstr); sx->send_quit = FALSE; goto TLS_FAILED; } @@ -2601,7 +2630,22 @@ if (tls_out.active.sock >= 0) #endif { if (!smtp_reap_ehlo(sx)) +#ifdef USE_GNUTLS + { + /* The GnuTLS layer in Exim only spots a server-rejection of a client + cert late, under TLS1.3 - which means here; the first time we try to + receive crypted data. Treat it as if it was a connect-time failure. + See also the early-pipe equivalent... which will be hard; every call + to sync_responses will need to check the result. + It would be nicer to have GnuTLS check the cert during the handshake. + Can it do that, with all the flexibility we need? */ + + tls_errstr = US"error on first read"; + goto GNUTLS_CONN_FAILED; + } +#else goto RESPONSE_FAILED; +#endif smtp_peer_options = 0; } } @@ -3261,6 +3305,7 @@ for (addr = sx->first_addr, address_count = 0; #ifdef EXPERIMENTAL_PIPE_CONNECT case -4: return -1; /* non-2xx for pipelined banner or EHLO */ + case -5: return -1; /* TLS first-read error */ #endif } sx->pending_MAIL = FALSE; /* Dealt with MAIL */ @@ -3589,11 +3634,12 @@ if ( !(sx.peer_offered & OPTION_CHUNKING) case 1: sx.ok = TRUE; /* 2xx (only) => OK, but if LMTP, */ if (!sx.lmtp) sx.completed_addr = TRUE; /* can't tell about progress yet */ - case 0: break; /* No 2xx or 5xx, but no probs */ + case 0: break; /* No 2xx or 5xx, but no probs */ - case -1: goto END_OFF; /* Timeout on RCPT */ + case -1: goto END_OFF; /* Timeout on RCPT */ #ifdef EXPERIMENTAL_PIPE_CONNECT + case -5: /* TLS first-read error */ case -4: HDEBUG(D_transport) debug_printf("failed reaping pipelined cmd responses\n"); #endif @@ -3730,19 +3776,20 @@ else { case 3: sx.ok = TRUE; /* 2xx & 5xx => OK & progress made */ case 2: sx.completed_addr = TRUE; /* 5xx (only) => progress made */ - break; + break; - case 1: sx.ok = TRUE; /* 2xx (only) => OK, but if LMTP, */ + case 1: sx.ok = TRUE; /* 2xx (only) => OK, but if LMTP, */ if (!sx.lmtp) sx.completed_addr = TRUE; /* can't tell about progress yet */ - case 0: break; /* No 2xx or 5xx, but no probs */ + case 0: break; /* No 2xx or 5xx, but no probs */ - case -1: goto END_OFF; /* Timeout on RCPT */ + case -1: goto END_OFF; /* Timeout on RCPT */ #ifdef EXPERIMENTAL_PIPE_CONNECT + case -5: /* TLS first-read error */ case -4: HDEBUG(D_transport) debug_printf("failed reaping pipelined cmd responses\n"); #endif - default: goto RESPONSE_FAILED; /* I/O error, or any MAIL/DATA error */ + default: goto RESPONSE_FAILED; /* I/O error, or any MAIL/DATA error */ } } diff --git a/test/confs/2027 b/test/confs/2027 index cc47218fb..c1b93a2ce 100644 --- a/test/confs/2027 +++ b/test/confs/2027 @@ -47,9 +47,11 @@ local_delivery: user = CALLER send_to_server: - driver = smtp + driver = smtp allow_localhost - hosts = ${if eq{$local_part}{userx}{127.0.0.1}{HOSTIPV4}} - port = PORT_D + hosts = ${if eq{$local_part}{userx}{127.0.0.1}{HOSTIPV4}} + port = PORT_D + tls_verify_certificates = DIR/aux-fixed/cert1 + tls_verify_cert_hostnames = : # End diff --git a/test/confs/5652 b/test/confs/5652 index 13c8d8617..28d3a95bb 100644 --- a/test/confs/5652 +++ b/test/confs/5652 @@ -29,6 +29,7 @@ tls_ocsp_file = DRSA/server1.example.com/server1.example.com.ocsp.good.resp \ : DECDSA/server1.example_ec.com/server1.example_ec.com.ocsp.good.resp +tls_require_ciphers = NORMAL:!VERS-TLS1.3 # ------ ACL ------ diff --git a/test/confs/5821 b/test/confs/5821 index 86ddbdedd..8a2d6459e 100644 --- a/test/confs/5821 +++ b/test/confs/5821 @@ -23,7 +23,7 @@ tls_certificate = ${if eq {SERVER}{server} {CDIR2/fullchain.pem}fail} tls_privatekey = ${if eq {SERVER}{server} {CDIR2/server1.example.com.unlocked.key}fail} # Permit two specific ciphers -tls_require_ciphers = NORMAL:-KX-ALL:+RSA:-CIPHER-ALL:+AES-128-CBC:+CAMELLIA-256-GCM +tls_require_ciphers = NORMAL:-VERS-TLS1.3:-KX-ALL:+RSA:-CIPHER-ALL:+AES-128-CBC:+CAMELLIA-256-GCM # ----- Routers ----- begin routers diff --git a/test/log/2027 b/test/log/2027 index 0bdd86a57..499351cd0 100644 --- a/test/log/2027 +++ b/test/log/2027 @@ -1,7 +1,7 @@ 1999-03-02 09:44:33 10HmaX-0005vi-00 <= CALLER@myhost.test.ex U=CALLER P=local S=sss 1999-03-02 09:44:33 10HmaY-0005vi-00 <= CALLER@myhost.test.ex U=CALLER P=local S=sss 1999-03-02 09:44:33 Start queue run: pid=pppp -qf -1999-03-02 09:44:33 10HmaX-0005vi-00 => userx@test.ex R=client T=send_to_server H=127.0.0.1 [127.0.0.1] X=TLS1.x:ke-RSA-AES256-SHAnnn:xxx CV=no C="250 OK id=10HmaZ-0005vi-00" +1999-03-02 09:44:33 10HmaX-0005vi-00 => userx@test.ex R=client T=send_to_server H=127.0.0.1 [127.0.0.1] X=TLS1.x:ke-RSA-AES256-SHAnnn:xxx CV=yes C="250 OK id=10HmaZ-0005vi-00" 1999-03-02 09:44:33 10HmaX-0005vi-00 Completed 1999-03-02 09:44:33 10HmaY-0005vi-00 TLS session: (gnutls_handshake): A TLS fatal alert has been received.: delivering unencrypted to H=ip4.ip4.ip4.ip4 [ip4.ip4.ip4.ip4] (not in hosts_require_tls) 1999-03-02 09:44:33 10HmaY-0005vi-00 => usery@test.ex R=client T=send_to_server H=ip4.ip4.ip4.ip4 [ip4.ip4.ip4.ip4] C="250 OK id=10HmbA-0005vi-00" diff --git a/test/runtest b/test/runtest index 10ae833ff..dafca16ec 100755 --- a/test/runtest +++ b/test/runtest @@ -1253,6 +1253,20 @@ RESET_AFTER_EXTRA_LINE_READ: s/(DANE attempt failed.*error:)[0-9A-F]{8}(:SSL routines:)(ssl3_get_server_certificate|tls_process_server_certificate|CONNECT_CR_CERT)(?=:certificate verify failed$)/$1xxxxxxxx$2ssl3_get_server_certificate/; s/(DKIM: validation error: )error:[0-9A-F]{8}:rsa routines:(?:(?i)int_rsa_verify|CRYPTO_internal):(?:bad signature|algorithm mismatch)$/$1Public key signature verification has failed./; + # gnutls version variances + if (/TLS error on connection \(recv\): .* Decode error/) + { + my $prev = $_; + $_ = ; + if (/error on first read/) + { + s/TLS session: \Kerror on first read:/(gnutls_handshake): A TLS fatal alert has been received.:/; + goto RESET_AFTER_EXTRA_LINE_READ; + } + else + { $_ = $prev; } + } + # DKIM timestamps if ( /(DKIM: d=.*) t=([0-9]*) x=([0-9]*) / ) { diff --git a/test/scripts/2000-GnuTLS/2027 b/test/scripts/2000-GnuTLS/2027 index 0d94ac4cd..3e071b665 100644 --- a/test/scripts/2000-GnuTLS/2027 +++ b/test/scripts/2000-GnuTLS/2027 @@ -3,9 +3,11 @@ gnutls munge gnutls_handshake exim -DSERVER=server -bd -oX PORT_D **** +# will send to 127.0.0.1 and the server requests a client-cert exim userx@test.ex Test message **** +# will send to HOSTIPV4 and the server requests&requires exim usery@test.ex Test message **** -- 2.30.2