| 1 | From c15523829ba17cce5829e2976aa1ff928965d948 Mon Sep 17 00:00:00 2001 |
| 2 | From: Jeremy Harris <jgh146exb@wizmail.org> |
| 3 | Date: Sat, 16 Feb 2019 12:59:23 +0000 |
| 4 | Subject: [PATCH 7/7] GnuTLS: Fix client detection of server reject of client |
| 5 | cert under TLS1.3 |
| 6 | |
| 7 | (cherry picked from commit fc243e944ec00b59b75f41d07494116f925d58b4) |
| 8 | --- |
| 9 | doc/ChangeLog | 7 +++ |
| 10 | src/deliver.c | 2 +- |
| 11 | src/smtp_out.c | 10 +++-- |
| 12 | src/tls-gnu.c | 23 +++------- |
| 13 | src/transports/lmtp.c | 3 +- |
| 14 | src/transports/smtp.c | 81 +++++++++++++++++++++++++++-------- |
| 15 | test/confs/2027 | 8 ++-- |
| 16 | test/confs/5652 | 1 + |
| 17 | test/confs/5821 | 2 +- |
| 18 | test/log/2027 | 2 +- |
| 19 | test/runtest | 14 ++++++ |
| 20 | test/scripts/2000-GnuTLS/2027 | 2 + |
| 21 | 12 files changed, 111 insertions(+), 44 deletions(-) |
| 22 | |
| 23 | diff --git a/doc/ChangeLog b/doc/ChangeLog |
| 24 | index 66c8a7a1..867a1d8a 100644 |
| 25 | --- a/doc/ChangeLog |
| 26 | +++ b/doc/ChangeLog |
| 27 | @@ -11,6 +11,13 @@ Since version 4.92 |
| 28 | JH/06 Fix buggy handling of autoreply bounce_return_size_limit, and a possible |
| 29 | buffer overrun for (non-chunking) other transports. |
| 30 | |
| 31 | +JH/07 GnuTLS: Our use of late (post-handshake) certificate verification, under |
| 32 | + TLS1.3, means that a server rejecting a client certificate is not visible |
| 33 | + to the client until the first read of encrypted data (typically the |
| 34 | + response to EHLO). Add detection for that case and treat it as a failed |
| 35 | + TLS connection attempt, so that the normal retry-in-clear can work (if |
| 36 | + suitably configured). |
| 37 | + |
| 38 | |
| 39 | Exim version 4.92 |
| 40 | ----------------- |
| 41 | diff --git a/src/deliver.c b/src/deliver.c |
| 42 | index 664d0045..e1799411 100644 |
| 43 | --- a/src/deliver.c |
| 44 | +++ b/src/deliver.c |
| 45 | @@ -7433,7 +7433,7 @@ if (addr_senddsn) |
| 46 | |
| 47 | tctx.u.fd = fd; |
| 48 | tctx.options = topt_add_return_path | topt_no_body; |
| 49 | - /*XXX hmm, retval ignored. |
| 50 | + /*XXX hmm, FALSE(fail) retval ignored. |
| 51 | Could error for any number of reasons, and they are not handled. */ |
| 52 | transport_write_message(&tctx, 0); |
| 53 | fflush(f); |
| 54 | diff --git a/src/smtp_out.c b/src/smtp_out.c |
| 55 | index 9bd90c77..b194e804 100644 |
| 56 | --- a/src/smtp_out.c |
| 57 | +++ b/src/smtp_out.c |
| 58 | @@ -688,20 +688,22 @@ Returns: TRUE if a valid, non-error response was received; else FALSE |
| 59 | /*XXX could move to smtp transport; no other users */ |
| 60 | |
| 61 | BOOL |
| 62 | -smtp_read_response(void * sx0, uschar *buffer, int size, int okdigit, |
| 63 | +smtp_read_response(void * sx0, uschar * buffer, int size, int okdigit, |
| 64 | int timeout) |
| 65 | { |
| 66 | smtp_context * sx = sx0; |
| 67 | -uschar *ptr = buffer; |
| 68 | -int count = 0; |
| 69 | +uschar * ptr = buffer; |
| 70 | +int count = 0, rc; |
| 71 | |
| 72 | errno = 0; /* Ensure errno starts out zero */ |
| 73 | |
| 74 | #ifdef EXPERIMENTAL_PIPE_CONNECT |
| 75 | if (sx->pending_BANNER || sx->pending_EHLO) |
| 76 | - if (smtp_reap_early_pipe(sx, &count) != OK) |
| 77 | + if ((rc = smtp_reap_early_pipe(sx, &count)) != OK) |
| 78 | { |
| 79 | DEBUG(D_transport) debug_printf("failed reaping pipelined cmd responsess\n"); |
| 80 | + buffer[0] = '\0'; |
| 81 | + if (rc == DEFER) errno = ERRNO_TLSFAILURE; |
| 82 | return FALSE; |
| 83 | } |
| 84 | #endif |
| 85 | diff --git a/src/tls-gnu.c b/src/tls-gnu.c |
| 86 | index c404dc29..de2d70c0 100644 |
| 87 | --- a/src/tls-gnu.c |
| 88 | +++ b/src/tls-gnu.c |
| 89 | @@ -229,7 +229,7 @@ static gnutls_dh_params_t dh_server_params = NULL; |
| 90 | |
| 91 | static const int ssl_session_timeout = 200; |
| 92 | |
| 93 | -static const char * const exim_default_gnutls_priority = "NORMAL"; |
| 94 | +static const uschar * const exim_default_gnutls_priority = US"NORMAL"; |
| 95 | |
| 96 | /* Guard library core initialisation */ |
| 97 | |
| 98 | @@ -1278,7 +1278,6 @@ int rc; |
| 99 | size_t sz; |
| 100 | const char *errpos; |
| 101 | uschar *p; |
| 102 | -BOOL want_default_priorities; |
| 103 | |
| 104 | if (!exim_gnutls_base_init_done) |
| 105 | { |
| 106 | @@ -1387,32 +1386,24 @@ and replaces gnutls_require_kx, gnutls_require_mac & gnutls_require_protocols. |
| 107 | This was backwards incompatible, but means Exim no longer needs to track |
| 108 | all algorithms and provide string forms for them. */ |
| 109 | |
| 110 | -want_default_priorities = TRUE; |
| 111 | - |
| 112 | +p = NULL; |
| 113 | if (state->tls_require_ciphers && *state->tls_require_ciphers) |
| 114 | { |
| 115 | if (!expand_check_tlsvar(tls_require_ciphers, errstr)) |
| 116 | return DEFER; |
| 117 | if (state->exp_tls_require_ciphers && *state->exp_tls_require_ciphers) |
| 118 | { |
| 119 | - DEBUG(D_tls) debug_printf("GnuTLS session cipher/priority \"%s\"\n", |
| 120 | - state->exp_tls_require_ciphers); |
| 121 | - |
| 122 | - rc = gnutls_priority_init(&state->priority_cache, |
| 123 | - CS state->exp_tls_require_ciphers, &errpos); |
| 124 | - want_default_priorities = FALSE; |
| 125 | p = state->exp_tls_require_ciphers; |
| 126 | + DEBUG(D_tls) debug_printf("GnuTLS session cipher/priority \"%s\"\n", p); |
| 127 | } |
| 128 | } |
| 129 | -if (want_default_priorities) |
| 130 | +if (!p) |
| 131 | { |
| 132 | + p = exim_default_gnutls_priority; |
| 133 | DEBUG(D_tls) |
| 134 | - debug_printf("GnuTLS using default session cipher/priority \"%s\"\n", |
| 135 | - exim_default_gnutls_priority); |
| 136 | - rc = gnutls_priority_init(&state->priority_cache, |
| 137 | - exim_default_gnutls_priority, &errpos); |
| 138 | - p = US exim_default_gnutls_priority; |
| 139 | + debug_printf("GnuTLS using default session cipher/priority \"%s\"\n", p); |
| 140 | } |
| 141 | +rc = gnutls_priority_init(&state->priority_cache, CCS p, &errpos); |
| 142 | |
| 143 | exim_gnutls_err_check(rc, string_sprintf( |
| 144 | "gnutls_priority_init(%s) failed at offset %ld, \"%.6s..\"", |
| 145 | diff --git a/src/transports/lmtp.c b/src/transports/lmtp.c |
| 146 | index 240d78b2..57b346d4 100644 |
| 147 | --- a/src/transports/lmtp.c |
| 148 | +++ b/src/transports/lmtp.c |
| 149 | @@ -122,7 +122,8 @@ Arguments: |
| 150 | Returns: TRUE if a "QUIT" command should be sent, else FALSE |
| 151 | */ |
| 152 | |
| 153 | -static BOOL check_response(int *errno_value, int more_errno, uschar *buffer, |
| 154 | +static BOOL |
| 155 | +check_response(int *errno_value, int more_errno, uschar *buffer, |
| 156 | int *yield, uschar **message) |
| 157 | { |
| 158 | *yield = '4'; /* Default setting is to give a temporary error */ |
| 159 | diff --git a/src/transports/smtp.c b/src/transports/smtp.c |
| 160 | index a351da84..bfd6018d 100644 |
| 161 | --- a/src/transports/smtp.c |
| 162 | +++ b/src/transports/smtp.c |
| 163 | @@ -594,6 +594,11 @@ switch(*errno_value) |
| 164 | pl, smtp_command, s); |
| 165 | return FALSE; |
| 166 | |
| 167 | + case ERRNO_TLSFAILURE: /* Handle bad first read; can happen with |
| 168 | + GnuTLS and TLS1.3 */ |
| 169 | + *message = US"bad first read from TLS conn"; |
| 170 | + return TRUE; |
| 171 | + |
| 172 | case ERRNO_FILTER_FAIL: /* Handle a failed filter process error; |
| 173 | can't send QUIT as we mustn't end the DATA. */ |
| 174 | *message = string_sprintf("transport filter process failed (%d)%s", |
| 175 | @@ -942,6 +947,7 @@ Arguments: |
| 176 | |
| 177 | Return: |
| 178 | OK all well |
| 179 | + DEFER error on first read of TLS'd conn |
| 180 | FAIL SMTP error in response |
| 181 | */ |
| 182 | int |
| 183 | @@ -949,6 +955,7 @@ smtp_reap_early_pipe(smtp_context * sx, int * countp) |
| 184 | { |
| 185 | BOOL pending_BANNER = sx->pending_BANNER; |
| 186 | BOOL pending_EHLO = sx->pending_EHLO; |
| 187 | +int rc = FAIL; |
| 188 | |
| 189 | sx->pending_BANNER = FALSE; /* clear early to avoid recursion */ |
| 190 | sx->pending_EHLO = FALSE; |
| 191 | @@ -960,6 +967,7 @@ if (pending_BANNER) |
| 192 | if (!smtp_reap_banner(sx)) |
| 193 | { |
| 194 | DEBUG(D_transport) debug_printf("bad banner\n"); |
| 195 | + if (tls_out.active.sock >= 0) rc = DEFER; |
| 196 | goto fail; |
| 197 | } |
| 198 | } |
| 199 | @@ -974,6 +982,7 @@ if (pending_EHLO) |
| 200 | if (!smtp_reap_ehlo(sx)) |
| 201 | { |
| 202 | DEBUG(D_transport) debug_printf("bad response for EHLO\n"); |
| 203 | + if (tls_out.active.sock >= 0) rc = DEFER; |
| 204 | goto fail; |
| 205 | } |
| 206 | |
| 207 | @@ -1011,7 +1020,7 @@ return OK; |
| 208 | fail: |
| 209 | invalidate_ehlo_cache_entry(sx); |
| 210 | (void) smtp_discard_responses(sx, sx->conn_args.ob, *countp); |
| 211 | - return FAIL; |
| 212 | + return rc; |
| 213 | } |
| 214 | #endif |
| 215 | |
| 216 | @@ -1056,6 +1065,7 @@ Returns: 3 if at least one address had 2xx and one had 5xx |
| 217 | -2 I/O or other non-response error for RCPT |
| 218 | -3 DATA or MAIL failed - errno and buffer set |
| 219 | -4 banner or EHLO failed (early-pipelining) |
| 220 | + -5 banner or EHLO failed (early-pipelining, TLS) |
| 221 | */ |
| 222 | |
| 223 | static int |
| 224 | @@ -1064,10 +1074,11 @@ sync_responses(smtp_context * sx, int count, int pending_DATA) |
| 225 | address_item * addr = sx->sync_addr; |
| 226 | smtp_transport_options_block * ob = sx->conn_args.ob; |
| 227 | int yield = 0; |
| 228 | +int rc; |
| 229 | |
| 230 | #ifdef EXPERIMENTAL_PIPE_CONNECT |
| 231 | -if (smtp_reap_early_pipe(sx, &count) != OK) |
| 232 | - return -4; |
| 233 | +if ((rc = smtp_reap_early_pipe(sx, &count)) != OK) |
| 234 | + return rc == FAIL ? -4 : -5; |
| 235 | #endif |
| 236 | |
| 237 | /* Handle the response for a MAIL command. On error, reinstate the original |
| 238 | @@ -1083,6 +1094,8 @@ if (sx->pending_MAIL) |
| 239 | { |
| 240 | DEBUG(D_transport) debug_printf("bad response for MAIL\n"); |
| 241 | Ustrcpy(big_buffer, mail_command); /* Fits, because it came from there! */ |
| 242 | + if (errno == ERRNO_TLSFAILURE) |
| 243 | + return -5; |
| 244 | if (errno == 0 && sx->buffer[0] != 0) |
| 245 | { |
| 246 | int save_errno = 0; |
| 247 | @@ -1141,6 +1154,11 @@ while (count-- > 0) |
| 248 | } |
| 249 | } |
| 250 | |
| 251 | + /* Error on first TLS read */ |
| 252 | + |
| 253 | + else if (errno == ERRNO_TLSFAILURE) |
| 254 | + return -5; |
| 255 | + |
| 256 | /* Timeout while reading the response */ |
| 257 | |
| 258 | else if (errno == ETIMEDOUT) |
| 259 | @@ -1253,6 +1271,10 @@ if (pending_DATA != 0) |
| 260 | int code; |
| 261 | uschar *msg; |
| 262 | BOOL pass_message; |
| 263 | + |
| 264 | + if (errno == ERRNO_TLSFAILURE) /* Error on first TLS read */ |
| 265 | + return -5; |
| 266 | + |
| 267 | if (pending_DATA > 0 || (yield & 1) != 0) |
| 268 | { |
| 269 | if (errno == 0 && sx->buffer[0] == '4') |
| 270 | @@ -1802,7 +1824,9 @@ Args: |
| 271 | tc_chunk_last add LAST option to SMTP BDAT command |
| 272 | tc_reap_prev reap response to previous SMTP commands |
| 273 | |
| 274 | -Returns: OK or ERROR |
| 275 | +Returns: |
| 276 | + OK or ERROR |
| 277 | + DEFER TLS error on first read (EHLO-resp); errno set |
| 278 | */ |
| 279 | |
| 280 | static int |
| 281 | @@ -1859,10 +1883,12 @@ if (flags & tc_reap_prev && prev_cmd_count > 0) |
| 282 | case 2: sx->completed_addr = TRUE; /* 5xx (only) => progress made */ |
| 283 | case 0: break; /* No 2xx or 5xx, but no probs */ |
| 284 | |
| 285 | - case -1: /* Timeout on RCPT */ |
| 286 | + case -5: errno = ERRNO_TLSFAILURE; |
| 287 | + return DEFER; |
| 288 | #ifdef EXPERIMENTAL_PIPE_CONNECT |
| 289 | case -4: /* non-2xx for pipelined banner or EHLO */ |
| 290 | #endif |
| 291 | + case -1: /* Timeout on RCPT */ |
| 292 | default: return ERROR; /* I/O error, or any MAIL/DATA error */ |
| 293 | } |
| 294 | cmd_count = 1; |
| 295 | @@ -1933,6 +1959,9 @@ BOOL pass_message = FALSE; |
| 296 | uschar * message = NULL; |
| 297 | int yield = OK; |
| 298 | int rc; |
| 299 | +#ifdef SUPPORT_TLS |
| 300 | +uschar * tls_errstr; |
| 301 | +#endif |
| 302 | |
| 303 | sx->conn_args.ob = ob; |
| 304 | |
| 305 | @@ -2474,27 +2503,27 @@ if ( smtp_peer_options & OPTION_TLS |
| 306 | TLS_NEGOTIATE: |
| 307 | { |
| 308 | address_item * addr; |
| 309 | - uschar * errstr; |
| 310 | sx->cctx.tls_ctx = tls_client_start(sx->cctx.sock, sx->conn_args.host, |
| 311 | sx->addrlist, sx->conn_args.tblock, |
| 312 | # ifdef SUPPORT_DANE |
| 313 | sx->dane ? &tlsa_dnsa : NULL, |
| 314 | # endif |
| 315 | - &tls_out, &errstr); |
| 316 | + &tls_out, &tls_errstr); |
| 317 | |
| 318 | if (!sx->cctx.tls_ctx) |
| 319 | { |
| 320 | /* TLS negotiation failed; give an error. From outside, this function may |
| 321 | be called again to try in clear on a new connection, if the options permit |
| 322 | it for this host. */ |
| 323 | - DEBUG(D_tls) debug_printf("TLS session fail: %s\n", errstr); |
| 324 | +GNUTLS_CONN_FAILED: |
| 325 | + DEBUG(D_tls) debug_printf("TLS session fail: %s\n", tls_errstr); |
| 326 | |
| 327 | # ifdef SUPPORT_DANE |
| 328 | if (sx->dane) |
| 329 | { |
| 330 | log_write(0, LOG_MAIN, |
| 331 | "DANE attempt failed; TLS connection to %s [%s]: %s", |
| 332 | - sx->conn_args.host->name, sx->conn_args.host->address, errstr); |
| 333 | + sx->conn_args.host->name, sx->conn_args.host->address, tls_errstr); |
| 334 | # ifndef DISABLE_EVENT |
| 335 | (void) event_raise(sx->conn_args.tblock->event_action, |
| 336 | US"dane:fail", US"validation-failure"); /* could do with better detail */ |
| 337 | @@ -2503,7 +2532,7 @@ if ( smtp_peer_options & OPTION_TLS |
| 338 | # endif |
| 339 | |
| 340 | errno = ERRNO_TLSFAILURE; |
| 341 | - message = string_sprintf("TLS session: %s", errstr); |
| 342 | + message = string_sprintf("TLS session: %s", tls_errstr); |
| 343 | sx->send_quit = FALSE; |
| 344 | goto TLS_FAILED; |
| 345 | } |
| 346 | @@ -2601,7 +2630,22 @@ if (tls_out.active.sock >= 0) |
| 347 | #endif |
| 348 | { |
| 349 | if (!smtp_reap_ehlo(sx)) |
| 350 | +#ifdef USE_GNUTLS |
| 351 | + { |
| 352 | + /* The GnuTLS layer in Exim only spots a server-rejection of a client |
| 353 | + cert late, under TLS1.3 - which means here; the first time we try to |
| 354 | + receive crypted data. Treat it as if it was a connect-time failure. |
| 355 | + See also the early-pipe equivalent... which will be hard; every call |
| 356 | + to sync_responses will need to check the result. |
| 357 | + It would be nicer to have GnuTLS check the cert during the handshake. |
| 358 | + Can it do that, with all the flexibility we need? */ |
| 359 | + |
| 360 | + tls_errstr = US"error on first read"; |
| 361 | + goto GNUTLS_CONN_FAILED; |
| 362 | + } |
| 363 | +#else |
| 364 | goto RESPONSE_FAILED; |
| 365 | +#endif |
| 366 | smtp_peer_options = 0; |
| 367 | } |
| 368 | } |
| 369 | @@ -3261,6 +3305,7 @@ for (addr = sx->first_addr, address_count = 0; |
| 370 | |
| 371 | #ifdef EXPERIMENTAL_PIPE_CONNECT |
| 372 | case -4: return -1; /* non-2xx for pipelined banner or EHLO */ |
| 373 | + case -5: return -1; /* TLS first-read error */ |
| 374 | #endif |
| 375 | } |
| 376 | sx->pending_MAIL = FALSE; /* Dealt with MAIL */ |
| 377 | @@ -3589,11 +3634,12 @@ if ( !(sx.peer_offered & OPTION_CHUNKING) |
| 378 | |
| 379 | case 1: sx.ok = TRUE; /* 2xx (only) => OK, but if LMTP, */ |
| 380 | if (!sx.lmtp) sx.completed_addr = TRUE; /* can't tell about progress yet */ |
| 381 | - case 0: break; /* No 2xx or 5xx, but no probs */ |
| 382 | + case 0: break; /* No 2xx or 5xx, but no probs */ |
| 383 | |
| 384 | - case -1: goto END_OFF; /* Timeout on RCPT */ |
| 385 | + case -1: goto END_OFF; /* Timeout on RCPT */ |
| 386 | |
| 387 | #ifdef EXPERIMENTAL_PIPE_CONNECT |
| 388 | + case -5: /* TLS first-read error */ |
| 389 | case -4: HDEBUG(D_transport) |
| 390 | debug_printf("failed reaping pipelined cmd responses\n"); |
| 391 | #endif |
| 392 | @@ -3730,19 +3776,20 @@ else |
| 393 | { |
| 394 | case 3: sx.ok = TRUE; /* 2xx & 5xx => OK & progress made */ |
| 395 | case 2: sx.completed_addr = TRUE; /* 5xx (only) => progress made */ |
| 396 | - break; |
| 397 | + break; |
| 398 | |
| 399 | - case 1: sx.ok = TRUE; /* 2xx (only) => OK, but if LMTP, */ |
| 400 | + case 1: sx.ok = TRUE; /* 2xx (only) => OK, but if LMTP, */ |
| 401 | if (!sx.lmtp) sx.completed_addr = TRUE; /* can't tell about progress yet */ |
| 402 | - case 0: break; /* No 2xx or 5xx, but no probs */ |
| 403 | + case 0: break; /* No 2xx or 5xx, but no probs */ |
| 404 | |
| 405 | - case -1: goto END_OFF; /* Timeout on RCPT */ |
| 406 | + case -1: goto END_OFF; /* Timeout on RCPT */ |
| 407 | |
| 408 | #ifdef EXPERIMENTAL_PIPE_CONNECT |
| 409 | + case -5: /* TLS first-read error */ |
| 410 | case -4: HDEBUG(D_transport) |
| 411 | debug_printf("failed reaping pipelined cmd responses\n"); |
| 412 | #endif |
| 413 | - default: goto RESPONSE_FAILED; /* I/O error, or any MAIL/DATA error */ |
| 414 | + default: goto RESPONSE_FAILED; /* I/O error, or any MAIL/DATA error */ |
| 415 | } |
| 416 | } |
| 417 | |
| 418 | -- |
| 419 | 2.20.1 |
| 420 | |