X-Git-Url: https://git.exim.org/exim.git/blobdiff_plain/f5730918ef684baafbd9e606a1d4eb06914563cc..4687a69c269ee3f2a7f0625e0147a503fd9d3d0b:/src/src/tls-openssl.c diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c index eabe34f31..64e8c711a 100644 --- a/src/src/tls-openssl.c +++ b/src/src/tls-openssl.c @@ -2,7 +2,7 @@ * Exim - an Internet mail transport agent * *************************************************/ -/* Copyright (c) The Exim Maintainers 2020 - 2022 */ +/* Copyright (c) The Exim Maintainers 2020 - 2023 */ /* Copyright (c) University of Cambridge 1995 - 2019 */ /* See the file NOTICE for conditions of use and distribution. */ /* SPDX-License-Identifier: GPL-2.0-or-later */ @@ -77,9 +77,9 @@ change this guard and punt the issue for a while longer. */ # define EXIM_HAVE_OPENSSL_KEYLOG # define EXIM_HAVE_OPENSSL_CIPHER_GET_ID # define EXIM_HAVE_SESSION_TICKET -# define EXIM_HAVE_OPESSL_TRACE -# define EXIM_HAVE_OPESSL_GET0_SERIAL -# define EXIM_HAVE_OPESSL_OCSP_RESP_GET0_CERTS +# define EXIM_HAVE_OPENSSL_TRACE +# define EXIM_HAVE_OPENSSL_GET0_SERIAL +# define EXIM_HAVE_OPENSSL_OCSP_RESP_GET0_CERTS # define EXIM_HAVE_SSL_GET0_VERIFIED_CHAIN # ifndef DISABLE_OCSP # define EXIM_HAVE_OCSP @@ -97,6 +97,9 @@ change this guard and punt the issue for a while longer. */ #if LIBRESSL_VERSION_NUMBER >= 0x3040000fL # define EXIM_HAVE_OPENSSL_CIPHER_GET_ID #endif +#if LIBRESSL_VERSION_NUMBER >= 0x3050000fL +# define EXIM_HAVE_OPENSSL_OCSP_RESP_GET0_CERTS +#endif #if !defined(LIBRESSL_VERSION_NUMBER) && (OPENSSL_VERSION_NUMBER >= 0x030000000L) # define EXIM_HAVE_EXPORT_CHNL_BNGNG @@ -122,6 +125,7 @@ change this guard and punt the issue for a while longer. */ # define EXIM_HAVE_OPENSSL_CIPHER_STD_NAME # define EXIM_HAVE_EXP_CHNL_BNGNG # define EXIM_HAVE_OPENSSL_OCSP_RESP_GET0_SIGNER +# define EXIM_HAVE_OPENSSL_SET1_GROUPS # else # define OPENSSL_BAD_SRVR_OURCERT # endif @@ -155,6 +159,7 @@ change this guard and punt the issue for a while longer. */ # endif #endif +#define TESTSUITE_TICKET_LIFE 10 /* seconds */ /************************************************* * OpenSSL option parse * *************************************************/ @@ -675,12 +680,12 @@ if (dh_bitsize <= tls_dh_max_bits) } else DEBUG(D_tls) - debug_printf("Diffie-Hellman initialized from %s with %d-bit prime\n", + debug_printf(" Diffie-Hellman initialized from %s with %d-bit prime\n", dhexpanded ? dhexpanded : US"default", dh_bitsize); } else DEBUG(D_tls) - debug_printf("dhparams '%s' %d bits, is > tls_dh_max_bits limit of %d\n", + debug_printf(" dhparams '%s' %d bits, is > tls_dh_max_bits limit of %d\n", dhexpanded ? dhexpanded : US"default", dh_bitsize, tls_dh_max_bits); #if OPENSSL_VERSION_NUMBER < 0x30000000L @@ -699,6 +704,41 @@ return TRUE; * Initialize for ECDH * *************************************************/ +/* "auto" needs to be handled carefully. +OpenSSL < 1.0.2: we do not select anything, but fallback to prime256v1 +OpenSSL < 1.1.0: we have to call SSL_CTX_set_ecdh_auto + (openssl/ssl.h defines SSL_CTRL_SET_ECDH_AUTO) +OpenSSL >= 1.1.0: we do not set anything, the libray does autoselection + https://github.com/openssl/openssl/commit/fe6ef2472db933f01b59cad82aa925736935984b + +*/ + +static uschar * +init_ecdh_auto(SSL_CTX * sctx) +{ +#if OPENSSL_VERSION_NUMBER < 0x10002000L +DEBUG(D_tls) debug_printf( + " ECDH OpenSSL < 1.0.2: temp key parameter settings: overriding \"auto\" with \"prime256v1\"\n"); +return US"prime256v1"; + +#else +# if defined SSL_CTRL_SET_ECDH_AUTO + +DEBUG(D_tls) debug_printf( + " ECDH OpenSSL 1.0.2+: temp key parameter settings: autoselection\n"); +SSL_CTX_set_ecdh_auto(sctx, 1); +return NULL; + +# else + +DEBUG(D_tls) debug_printf( + " ECDH OpenSSL 1.1.0+: temp key parameter settings: library default selection\n"); +return NULL; + +# endif +#endif +} + /* Load parameters for ECDH encryption. Server only. For now, we stick to NIST P-256 because: it's simple and easy to configure; @@ -729,64 +769,76 @@ init_ecdh(SSL_CTX * sctx, uschar ** errstr) return TRUE; #else -uschar * exp_curve; -int nid; -BOOL rv; - # ifndef EXIM_HAVE_ECDH DEBUG(D_tls) - debug_printf("No OpenSSL API to define ECDH parameters, skipping\n"); + debug_printf(" No OpenSSL API to define ECDH parameters, skipping\n"); return TRUE; # else +uschar * exp_curve; +int ngroups, rc, sep; +const uschar * curves_list, * curve; +# ifdef EXIM_HAVE_OPENSSL_SET1_GROUPS +int nids[16]; +# else +int nids[1]; +# endif + if (!expand_check(tls_eccurve, US"tls_eccurve", &exp_curve, errstr)) return FALSE; + +/* Is the option deliberately empty? */ + if (!exp_curve || !*exp_curve) return TRUE; -/* "auto" needs to be handled carefully. - * OpenSSL < 1.0.2: we do not select anything, but fallback to prime256v1 - * OpenSSL < 1.1.0: we have to call SSL_CTX_set_ecdh_auto - * (openssl/ssl.h defines SSL_CTRL_SET_ECDH_AUTO) - * OpenSSL >= 1.1.0: we do not set anything, the libray does autoselection - * https://github.com/openssl/openssl/commit/fe6ef2472db933f01b59cad82aa925736935984b - */ -if (Ustrcmp(exp_curve, "auto") == 0) - { -#if OPENSSL_VERSION_NUMBER < 0x10002000L - DEBUG(D_tls) debug_printf( - "ECDH OpenSSL < 1.0.2: temp key parameter settings: overriding \"auto\" with \"prime256v1\"\n"); - exp_curve = US"prime256v1"; -#else -# if defined SSL_CTRL_SET_ECDH_AUTO - DEBUG(D_tls) debug_printf( - "ECDH OpenSSL 1.0.2+: temp key parameter settings: autoselection\n"); - SSL_CTX_set_ecdh_auto(sctx, 1); - return TRUE; -# else - DEBUG(D_tls) debug_printf( - "ECDH OpenSSL 1.1.0+: temp key parameter settings: default selection\n"); - return TRUE; -# endif -#endif - } +/* Limit the list to hardwired array size. Drop out if any element is "suto". */ -DEBUG(D_tls) debug_printf("ECDH: curve '%s'\n", exp_curve); -if ( (nid = OBJ_sn2nid (CCS exp_curve)) == NID_undef -# ifdef EXIM_HAVE_OPENSSL_EC_NIST2NID - && (nid = EC_curve_nist2nid(CCS exp_curve)) == NID_undef -# endif - ) - { - tls_error(string_sprintf("Unknown curve name tls_eccurve '%s'", exp_curve), - NULL, NULL, errstr); - return FALSE; - } +curves_list = exp_curve; +sep = 0; +for (ngroups = 0; + ngroups < nelem(nids) + && (curve = string_nextinlist(&curves_list, &sep, NULL, 0)); + ) + if (Ustrcmp(curve, "auto") == 0) + { + DEBUG(D_tls) if (ngroups > 0) + debug_printf(" tls_eccurve 'auto' item takes precedence\n"); + if ((exp_curve = init_ecdh_auto(sctx))) break; /* have a curve name to set */ + return TRUE; /* all done */ + } + else + ngroups++; -# if OPENSSL_VERSION_NUMBER < 0x30000000L +/* Translate to NIDs */ + +curves_list = exp_curve; +for (ngroups = 0; curve = string_nextinlist(&curves_list, &sep, NULL, 0); + ngroups++) + if ( (nids[ngroups] = OBJ_sn2nid (CCS curve)) == NID_undef +# ifdef EXIM_HAVE_OPENSSL_EC_NIST2NID + && (nids[ngroups] = EC_curve_nist2nid(CCS curve)) == NID_undef +# endif + ) + { + uschar * s = string_sprintf("Unknown curve name in tls_eccurve '%s'", curve); + DEBUG(D_tls) debug_printf("TLS error: %s\n", s); + if (errstr) *errstr = s; + return FALSE; + } + +# ifdef EXIM_HAVE_OPENSSL_SET1_GROUPS +/* Set the groups */ + +if ((rc = SSL_CTX_set1_groups(sctx, nids, ngroups)) == 0) + tls_error(string_sprintf("Error enabling '%s' group(s)", exp_curve), NULL, NULL, errstr); +else + DEBUG(D_tls) debug_printf(" ECDH: enabled '%s' group(s)\n", exp_curve); + +# else /* Cannot handle a list; only 1 element nids array */ { EC_KEY * ecdh; - if (!(ecdh = EC_KEY_new_by_curve_name(nid))) + if (!(ecdh = EC_KEY_new_by_curve_name(nids[0]))) { tls_error(US"Unable to create ec curve", NULL, NULL, errstr); return FALSE; @@ -795,23 +847,15 @@ if ( (nid = OBJ_sn2nid (CCS exp_curve)) == NID_undef /* The "tmp" in the name here refers to setting a temporary key not to the stability of the interface. */ - if ((rv = SSL_CTX_set_tmp_ecdh(sctx, ecdh) == 0)) + if ((rc = SSL_CTX_set_tmp_ecdh(sctx, ecdh)) == 0) tls_error(string_sprintf("Error enabling '%s' curve", exp_curve), NULL, NULL, errstr); else - DEBUG(D_tls) debug_printf("ECDH: enabled '%s' curve\n", exp_curve); + DEBUG(D_tls) debug_printf(" ECDH: enabled '%s' curve\n", exp_curve); EC_KEY_free(ecdh); } +# endif /*!EXIM_HAVE_OPENSSL_SET1_GROUPS*/ -#else /* v 3.0.0 + */ - -if ((rv = SSL_CTX_set1_groups(sctx, &nid, 1)) == 0) - tls_error(string_sprintf("Error enabling '%s' group", exp_curve), NULL, NULL, errstr); -else - DEBUG(D_tls) debug_printf("ECDH: enabled '%s' group\n", exp_curve); - -#endif - -return !rv; +return !!rc; # endif /*EXIM_HAVE_ECDH*/ #endif /*OPENSSL_NO_ECDH*/ @@ -960,7 +1004,7 @@ Returns: nothing */ static void -info_callback(SSL * s, int where, int ret) +info_callback(const SSL * s, int where, int ret) { DEBUG(D_tls) { @@ -1151,6 +1195,8 @@ else uschar * name; int rc; while ((name = string_nextinlist(&list, &sep, NULL, 0))) + { + DEBUG(D_tls|D_lookup) debug_printf_indent("%s suitable for cert, per OpenSSL?", name); if ((rc = X509_check_host(cert, CCS name, 0, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS | X509_CHECK_FLAG_SINGLE_LABEL_SUBDOMAINS, @@ -1162,8 +1208,11 @@ else tlsp == &tls_out ? deliver_host_address : sender_host_address); name = NULL; } + DEBUG(D_tls|D_lookup) debug_printf_indent(" yes\n"); break; } + else DEBUG(D_tls|D_lookup) debug_printf_indent(" no\n"); + } if (!name) #else if (!tls_is_name_for_cert(verify_cert_hostnames, cert)) @@ -1392,7 +1441,7 @@ SNI handling. Separately we might try to replace using OCSP_basic_verify() - which seems to not be a public interface into the OpenSSL library (there's no manual entry) - -(in 3.0.0 + is is public) +(in 3.0.0 + it is public) But what with? We also use OCSP_basic_verify in the client stapling callback. And there we NEED it; we must verify that status... unless the library does it for us anyway? */ @@ -1553,8 +1602,13 @@ else ) ) reexpand_tls_files_for_sni = TRUE; - if (!expand_check(state->certificate, US"tls_certificate", &expanded, errstr)) + if ( !expand_check(state->certificate, US"tls_certificate", &expanded, errstr) + || f.expand_string_forcedfail) + { + if (f.expand_string_forcedfail) + *errstr = US"expansion of tls_certificate failed"; return DEFER; + } if (expanded) if (state->is_server) @@ -1622,9 +1676,14 @@ else if ((err = tls_add_certfile(sctx, state, expanded, errstr))) return err; - if ( state->privatekey - && !expand_check(state->privatekey, US"tls_privatekey", &expanded, errstr)) + if ( state->privatekey + && !expand_check(state->privatekey, US"tls_privatekey", &expanded, errstr) + || f.expand_string_forcedfail) + { + if (f.expand_string_forcedfail) + *errstr = US"expansion of tls_privatekey failed"; return DEFER; + } /* If expansion was forced to fail, key_expanded will be NULL. If the result of the expansion is an empty string, ignore it also, and assume the private @@ -1699,13 +1758,13 @@ level. */ DEBUG(D_tls) { - SSL_CTX_set_info_callback(ctx, (void (*)())info_callback); -#if defined(EXIM_HAVE_OPESSL_TRACE) && !defined(OPENSSL_NO_SSL_TRACE) + SSL_CTX_set_info_callback(ctx, info_callback); +#if defined(EXIM_HAVE_OPENSSL_TRACE) && !defined(OPENSSL_NO_SSL_TRACE) /* this needs a debug build of OpenSSL */ - SSL_CTX_set_msg_callback(ctx, (void (*)())SSL_trace); + SSL_CTX_set_msg_callback(ctx, SSL_trace); #endif #ifdef OPENSSL_HAVE_KEYLOG_CB - SSL_CTX_set_keylog_callback(ctx, (void (*)())keylog_callback); + SSL_CTX_set_keylog_callback(ctx, keylog_callback); #endif } @@ -1735,7 +1794,7 @@ state_server.lib_state.lib_ctx = ctx; if (opt_unset_or_noexpand(tls_dhparam)) { - DEBUG(D_tls) debug_printf("TLS: preloading DH params for server\n"); + DEBUG(D_tls) debug_printf("TLS: preloading DH params '%s' for server\n", tls_dhparam); if (init_dh(ctx, tls_dhparam, &dummy_errstr)) state_server.lib_state.dh = TRUE; } @@ -1743,7 +1802,7 @@ else DEBUG(D_tls) debug_printf("TLS: not preloading DH params for server\n"); if (opt_unset_or_noexpand(tls_eccurve)) { - DEBUG(D_tls) debug_printf("TLS: preloading ECDH curve for server\n"); + DEBUG(D_tls) debug_printf("TLS: preloading ECDH curve '%s' for server\n", tls_eccurve); if (init_ecdh(ctx, &dummy_errstr)) state_server.lib_state.ecdh = TRUE; } @@ -2034,7 +2093,7 @@ if (exim_tk.name[0]) exim_tk_old = exim_tk; } -if (f.running_in_test_harness) ssl_session_timeout = 6; +if (f.running_in_test_harness) ssl_session_timeout = TESTSUITE_TICKET_LIFE; DEBUG(D_tls) debug_printf("OpenSSL: %s STEK\n", exim_tk.name[0] ? "rotating" : "creating"); if (RAND_bytes(exim_tk.aes_key, sizeof(exim_tk.aes_key)) <= 0) return; @@ -2201,13 +2260,13 @@ per https://www.openssl.org/docs/manmaster/man3/SSL_client_hello_cb_fn.html #ifdef EXIM_HAVE_OPENSSL_TLSEXT static int -tls_servername_cb(SSL *s, int *ad ARG_UNUSED, void *arg) +tls_servername_cb(SSL * s, int * ad ARG_UNUSED, void * arg) { -const char *servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name); -exim_openssl_state_st *state = (exim_openssl_state_st *) arg; +const char * servername = SSL_get_servername(s, TLSEXT_NAMETYPE_host_name); +exim_openssl_state_st * state = (exim_openssl_state_st *) arg; int rc; int old_pool = store_pool; -uschar * dummy_errstr; +uschar * errstr; if (!servername) return SSL_TLSEXT_ERR_OK; @@ -2227,7 +2286,7 @@ if (!reexpand_tls_files_for_sni) not confident that memcpy wouldn't break some internal reference counting. Especially since there's a references struct member, which would be off. */ -if (lib_ctx_new(&server_sni, NULL, &dummy_errstr) != OK) +if (lib_ctx_new(&server_sni, NULL, &errstr) != OK) goto bad; /* Not sure how many of these are actually needed, since SSL object @@ -2247,8 +2306,8 @@ already exists. Might even need this selfsame callback, for reneg? */ SSL_CTX_set_tlsext_servername_arg(server_sni, state); } -if ( !init_dh(server_sni, state->dhparam, &dummy_errstr) - || !init_ecdh(server_sni, &dummy_errstr) +if ( !init_dh(server_sni, state->dhparam, &errstr) + || !init_ecdh(server_sni, &errstr) ) goto bad; @@ -2267,7 +2326,7 @@ if (state->u_ocsp.server.file) { uschar * v_certs = tls_verify_certificates; if ((rc = setup_certs(server_sni, &v_certs, tls_crl, NULL, - &dummy_errstr)) != OK) + &errstr)) != OK) goto bad; if (v_certs && *v_certs) @@ -2276,14 +2335,16 @@ if (state->u_ocsp.server.file) /* do this after setup_certs, because this can require the certs for verifying OCSP information. */ -if ((rc = tls_expand_session_files(server_sni, state, &dummy_errstr)) != OK) +if ((rc = tls_expand_session_files(server_sni, state, &errstr)) != OK) goto bad; DEBUG(D_tls) debug_printf("Switching SSL context.\n"); SSL_set_SSL_CTX(s, server_sni); return SSL_TLSEXT_ERR_OK; -bad: return SSL_TLSEXT_ERR_ALERT_FATAL; +bad: + log_write(0, LOG_MAIN|LOG_PANIC, "%s", errstr); + return SSL_TLSEXT_ERR_ALERT_FATAL; } #endif /* EXIM_HAVE_OPENSSL_TLSEXT */ @@ -2303,6 +2364,8 @@ static int tls_server_alpn_cb(SSL *ssl, const uschar ** out, uschar * outlen, const uschar * in, unsigned int inlen, void * arg) { +gstring * g = NULL; + server_seen_alpn = TRUE; DEBUG(D_tls) { @@ -2333,12 +2396,19 @@ if ( inlen > 1 /* at least one name */ } } -/* More than one name from clilent, or name did not match our list. */ +/* More than one name from client, or name did not match our list. */ /* This will be fatal to the TLS conn; would be nice to kill TCP also. Maybe as an option in future; for now leave control to the config (must-tls). */ -DEBUG(D_tls) debug_printf("TLS ALPN rejected\n"); +for (int pos = 0, siz; pos < inlen; pos += siz+1) + { + siz = in[pos]; + if (pos + 1 + siz > inlen) siz = inlen - pos - 1; + g = string_append_listele_n(g, ':', in + pos + 1, siz); + } +log_write(0, LOG_MAIN, "TLS ALPN (%Y) rejected", g); +gstring_release_unused(g); return SSL_TLSEXT_ERR_ALERT_FATAL; } #endif /* EXIM_HAVE_ALPN */ @@ -2375,7 +2445,7 @@ tls_in.ocsp = OCSP_NOT_RESP; if (!olist) return SSL_TLSEXT_ERR_NOACK; -#ifdef EXIM_HAVE_OPESSL_GET0_SERIAL +#ifdef EXIM_HAVE_OPENSSL_GET0_SERIAL { const X509 * cert_sent = SSL_get_certificate(s); const ASN1_INTEGER * cert_serial = X509_get0_serialNumber(cert_sent); @@ -2538,7 +2608,7 @@ if (!(bs = OCSP_response_get1_basic(rsp))) asking for certificate-status under DANE, so this callback won't run for that combination. It still will for non-DANE. */ -#ifdef EXIM_HAVE_OPENSSL_OCSP_RESP_GET0_SIGNER +#if defined(EXIM_HAVE_OPENSSL_OCSP_RESP_GET0_SIGNER) && defined(SUPPORT_DANE) X509 * signer; if ( tls_out.dane_verified @@ -2579,7 +2649,7 @@ if (!(bs = OCSP_response_get1_basic(rsp))) debug_printf("certs contained in basicresp:\n"); x509_stack_dump_cert_s_names( -#ifdef EXIM_HAVE_OPESSL_OCSP_RESP_GET0_CERTS +#ifdef EXIM_HAVE_OPENSSL_OCSP_RESP_GET0_CERTS OCSP_resp_get0_certs(bs) #else bs->certs @@ -3437,7 +3507,7 @@ static uschar peerdn[256]; if (tls_in.active.sock >= 0) { tls_error(US"STARTTLS received after TLS started", NULL, US"", errstr); - smtp_printf("554 Already in TLS\r\n", FALSE); + smtp_printf("554 Already in TLS\r\n", SP_NO_MORE); return FAIL; } @@ -3557,7 +3627,7 @@ mode, the fflush() happens when smtp_getc() is called. */ SSL_set_session_id_context(ssl, sid_ctx, Ustrlen(sid_ctx)); if (!tls_in.on_connect) { - smtp_printf("220 TLS go ahead\r\n", FALSE); + smtp_printf("220 TLS go ahead\r\n", SP_NO_MORE); fflush(smtp_out); } @@ -3896,16 +3966,17 @@ if (tlsp->host_resumable) #ifdef EXIM_HAVE_SESSION_TICKET SSL_SESSION_get_ticket_lifetime_hint(ss); #else /* Use, fairly arbitrilarily, what we as server would */ - f.running_in_test_harness ? 6 : ssl_session_timeout; + f.running_in_test_harness ? TESTSUITE_TICKET_LIFE : ssl_session_timeout; #endif - if (lifetime + dt->time_stamp < time(NULL)) + time_t now = time(NULL), expires = lifetime + dt->time_stamp; + if (expires < now) { - DEBUG(D_tls) debug_printf("session expired\n"); + DEBUG(D_tls) debug_printf("session expired (by " TIME_T_FMT "s from %lus)\n", now - expires, lifetime); dbfn_delete(dbm_file, tlsp->resume_index); } else if (SSL_set_session(ssl, ss)) { - DEBUG(D_tls) debug_printf("good session\n"); + DEBUG(D_tls) debug_printf("good session (" TIME_T_FMT "s left of %lus)\n", expires - now, lifetime); tlsp->resumption |= RESUME_CLIENT_SUGGESTED; tlsp->verify_override = dt->verify_override; tlsp->ocsp = dt->ocsp; @@ -3999,7 +4070,7 @@ if (tlsp->host_resumable) tls_error(US"set ex_data", host, NULL, errstr); return FALSE; } - debug_printf("tls_exdata_idx %d cbinfo %p\n", tls_exdata_idx, client_static_state); + /* debug_printf("tls_exdata_idx %d cbinfo %p\n", tls_exdata_idx, client_static_state); */ } tlsp->resumption = RESUME_SUPPORTED; @@ -4464,10 +4535,15 @@ switch(error) /* Handle genuine errors */ case SSL_ERROR_SSL: + { + uschar * conn_info = smtp_get_connection_info(); + if (Ustrncmp(conn_info, US"SMTP ", 5) == 0) conn_info += 5; + /* I'd like to get separated H= here, but too hard for now */ ERR_error_string_n(ERR_get_error(), ssl_errstring, sizeof(ssl_errstring)); - log_write(0, LOG_MAIN, "TLS error (SSL_read): %s", ssl_errstring); + log_write(0, LOG_MAIN, "TLS error (SSL_read): on %s %s", conn_info, ssl_errstring); ssl_xfer_error = TRUE; return FALSE; + } default: DEBUG(D_tls) debug_printf("Got SSL error %d\n", error); @@ -5094,8 +5170,7 @@ if (!expand_check(option_spec, US"openssl_options", &exp, &end)) for (uschar * s = exp; *s; /**/) { - while (isspace(*s)) ++s; - if (*s == '\0') + if (!Uskip_whitespace(&s)) break; if (*s != '+' && *s != '-') {