diff options
| -rw-r--r-- | doc/doc-txt/ChangeLog | 4 | ||||
| -rw-r--r-- | src/src/tls-openssl.c | 89 | ||||
| -rw-r--r-- | test/scripts/5600-OCSP-OpenSSL/5610 | 8 | ||||
| -rw-r--r-- | test/scripts/5600-OCSP-OpenSSL/5611 | 6 | ||||
| -rw-r--r-- | test/src/client.c | 40 | ||||
| -rw-r--r-- | test/stderr/5610 | 76 |
6 files changed, 120 insertions, 103 deletions
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 3a6684f25..e9b0705f4 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -125,6 +125,10 @@ JH/31 Fix longstanding bug with aborted TLS server connection handling. Under Exim did stdio operations after fclose. This was exposed by a recent change which nulled out the file handle after the fclose. +JH/32 Bug 1909: Fix OCSP proof verification for cases where the proof is + signed directly by the cert-signing cert, rather than an intermediate + OCSP-signing cert. This is the model used by LetsEncrypt. + Exim version 4.87 ----------------- diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c index 64dcab600..39c5e1f56 100644 --- a/src/src/tls-openssl.c +++ b/src/src/tls-openssl.c @@ -159,6 +159,7 @@ typedef struct tls_ext_ctx_cb { } server; struct { X509_STORE *verify_store; /* non-null if status requested */ + STACK_OF(X509) *verify_stack; BOOL verify_required; } client; } u_ocsp; @@ -426,6 +427,7 @@ else if (depth != 0) if (!X509_STORE_add_cert(client_static_cbinfo->u_ocsp.client.verify_store, cert)) ERR_clear_error(); + sk_X509_push(client_static_cbinfo->u_ocsp.client.verify_stack, cert); } #endif #ifndef DISABLE_EVENT @@ -780,6 +782,34 @@ return !rv; * Load OCSP information into state * *************************************************/ +static STACK_OF(X509) * +cert_stack_from_store(X509_STORE * store) +{ +STACK_OF(X509_OBJECT) * roots= store->objs; +STACK_OF(X509) * sk = sk_X509_new_null(); +int i; + +for(i = sk_X509_OBJECT_num(roots) - 1; i >= 0; i--) + { + X509_OBJECT * tmp_obj= sk_X509_OBJECT_value(roots, i); + if(tmp_obj->type == X509_LU_X509) + { + X509 * x = tmp_obj->data.x509; + sk_X509_push(sk, x); + } + } +return sk; +} + +static void +cert_stack_free(STACK_OF(X509) * sk) +{ +while (sk_X509_num(sk) > 0) (void) sk_X509_pop(sk); +sk_X509_free(sk); +} + + + /* Called to load the server OCSP response from the given file into memory, once caller has determined this is needed. Checks validity. Debugs a message if invalid. @@ -796,12 +826,13 @@ Arguments: static void ocsp_load_response(SSL_CTX *sctx, tls_ext_ctx_cb *cbinfo, const uschar *expanded) { -BIO *bio; -OCSP_RESPONSE *resp; -OCSP_BASICRESP *basic_response; -OCSP_SINGLERESP *single_response; -ASN1_GENERALIZEDTIME *rev, *thisupd, *nextupd; -X509_STORE *store; +BIO * bio; +OCSP_RESPONSE * resp; +OCSP_BASICRESP * basic_response; +OCSP_SINGLERESP * single_response; +ASN1_GENERALIZEDTIME * rev, * thisupd, * nextupd; +X509_STORE * store; +STACK_OF(X509) * sk; unsigned long verify_flags; int status, reason, i; @@ -812,8 +843,7 @@ if (cbinfo->u_ocsp.server.response) cbinfo->u_ocsp.server.response = NULL; } -bio = BIO_new_file(CS cbinfo->u_ocsp.server.file_expanded, "rb"); -if (!bio) +if (!(bio = BIO_new_file(CS cbinfo->u_ocsp.server.file_expanded, "rb"))) { DEBUG(D_tls) debug_printf("Failed to open OCSP response file \"%s\"\n", cbinfo->u_ocsp.server.file_expanded); @@ -828,16 +858,14 @@ if (!resp) return; } -status = OCSP_response_status(resp); -if (status != OCSP_RESPONSE_STATUS_SUCCESSFUL) +if ((status = OCSP_response_status(resp)) != OCSP_RESPONSE_STATUS_SUCCESSFUL) { DEBUG(D_tls) debug_printf("OCSP response not valid: %s (%d)\n", OCSP_response_status_str(status), status); goto bad; } -basic_response = OCSP_response_get1_basic(resp); -if (!basic_response) +if (!(basic_response = OCSP_response_get1_basic(resp))) { DEBUG(D_tls) debug_printf("OCSP response parse error: unable to extract basic response.\n"); @@ -845,21 +873,38 @@ if (!basic_response) } store = SSL_CTX_get_cert_store(sctx); +sk = cert_stack_from_store(store); verify_flags = OCSP_NOVERIFY; /* check sigs, but not purpose */ /* May need to expose ability to adjust those flags? OCSP_NOSIGS OCSP_NOVERIFY OCSP_NOCHAIN OCSP_NOCHECKS OCSP_NOEXPLICIT OCSP_TRUSTOTHER OCSP_NOINTERN */ -i = OCSP_basic_verify(basic_response, NULL, store, verify_flags); -if (i <= 0) +/* This does a full verify on the OCSP proof before we load it for serviing +up; possibly overkill - just date-checks might be nice enough. + +OCSP_basic_verify takes a "store" arg, but does not +use it for the chain verification, which is all we do +when OCSP_NOVERIFY is set. The content from the wire +"basic_response" and a cert-stack "sk" are all that is used. + +Seperately 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) - +But what with? We also use OCSP_basic_verify in the client stapling callback. +And there we NEED it; we miust verify that status... unless the +library does it for us anyway? */ + +if ((i = OCSP_basic_verify(basic_response, sk, NULL, verify_flags)) < 0) { - DEBUG(D_tls) { + DEBUG(D_tls) + { ERR_error_string(ERR_get_error(), ssl_errstring); debug_printf("OCSP response verify failure: %s\n", US ssl_errstring); } + cert_stack_free(sk); goto bad; } +cert_stack_free(sk); /* Here's the simplifying assumption: there's only one response, for the one certificate we use, and nothing for anything else in a chain. If this @@ -868,8 +913,8 @@ proves false, we need to extract a cert id from our issued cert right cert in the stack and then calls OCSP_single_get0_status()). I'm hoping to avoid reworking a bunch more of how we handle state here. */ -single_response = OCSP_resp_get0(basic_response, 0); -if (!single_response) + +if (!(single_response = OCSP_resp_get0(basic_response, 0))) { DEBUG(D_tls) debug_printf("Unable to get first response from OCSP basic response.\n"); @@ -1279,7 +1324,7 @@ if(!(bs = OCSP_response_get1_basic(rsp))) /* Use the chain that verified the server cert to verify the stapled info */ /* DEBUG(D_tls) x509_store_dump_cert_s_names(cbinfo->u_ocsp.client.verify_store); */ - if ((i = OCSP_basic_verify(bs, NULL, + if ((i = OCSP_basic_verify(bs, cbinfo->u_ocsp.client.verify_stack, cbinfo->u_ocsp.client.verify_store, 0)) <= 0) { tls_out.ocsp = OCSP_FAILED; @@ -1409,7 +1454,10 @@ if ((cbinfo->is_server = host==NULL)) cbinfo->u_ocsp.server.response = NULL; } else + { cbinfo->u_ocsp.client.verify_store = NULL; + cbinfo->u_ocsp.client.verify_stack = NULL; + } #endif cbinfo->dhparam = dhparam; cbinfo->server_cipher_list = NULL; @@ -1535,6 +1583,11 @@ else /* client */ DEBUG(D_tls) debug_printf("failed to create store for stapling verify\n"); return FAIL; } + if (!(cbinfo->u_ocsp.client.verify_stack = sk_X509_new_null())) + { + DEBUG(D_tls) debug_printf("failed to create stack for stapling verify\n"); + return FAIL; + } SSL_CTX_set_tlsext_status_cb(*ctxp, tls_client_stapling_cb); SSL_CTX_set_tlsext_status_arg(*ctxp, cbinfo); } diff --git a/test/scripts/5600-OCSP-OpenSSL/5610 b/test/scripts/5600-OCSP-OpenSSL/5610 index c8668ea38..fccd94486 100644 --- a/test/scripts/5600-OCSP-OpenSSL/5610 +++ b/test/scripts/5600-OCSP-OpenSSL/5610 @@ -5,7 +5,7 @@ # '1: Server sends good staple on request' # exim -bd -oX PORT_D -DSERVER=server \ - -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.good.resp + -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.signernocert.good.resp **** client-ssl \ -ocsp aux-fixed/exim-ca/example.com/server1.example.com/ca_chain.pem \ @@ -34,7 +34,7 @@ killdaemon # '2: Server does not staple an outdated response' # exim -bd -oX PORT_D -DSERVER=server \ - -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.dated.resp + -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.signernocert.dated.resp **** # XXX test sequence might not be quite right; this is for a server refusal # and we're expecting a client refusal. @@ -59,7 +59,7 @@ killdaemon # '3: Server does not staple a response for a revoked cert' # exim -bd -oX PORT_D -DSERVER=server \ - -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.revoked.resp + -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.signernocert.revoked.resp **** client-ssl \ -ocsp aux-fixed/exim-ca/example.com/server1.example.com/ca_chain.pem \ @@ -84,7 +84,7 @@ killdaemon # '4: Connection functions when server is prepared to staple but client does not request it' # exim -bd -oX PORT_D -DSERVER=server \ - -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.good.resp + -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.signernocert.good.resp **** # client-ssl \ diff --git a/test/scripts/5600-OCSP-OpenSSL/5611 b/test/scripts/5600-OCSP-OpenSSL/5611 index 248c44219..cb8f44fe1 100644 --- a/test/scripts/5600-OCSP-OpenSSL/5611 +++ b/test/scripts/5600-OCSP-OpenSSL/5611 @@ -15,7 +15,7 @@ killdaemon # # Client works when we don't request OCSP stapling exim -bd -oX PORT_D -DSERVER=server \ - -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.good.resp + -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.signernocert.good.resp **** exim nostaple@test.ex test message. @@ -48,7 +48,7 @@ sudo rm spool/db/retry # # Client fails on revoked stapled info EXIM_TESTHARNESS_DISABLE_OCSPVALIDITYCHECK=y exim -bd -oX PORT_D -DSERVER=server \ - -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.revoked.resp + -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.signernocert.revoked.resp **** exim CALLER@test.ex test message. @@ -63,7 +63,7 @@ sudo rm spool/db/retry # # Client fails on expired stapled info EXIM_TESTHARNESS_DISABLE_OCSPVALIDITYCHECK=y exim -bd -oX PORT_D -DSERVER=server \ - -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.dated.resp + -DRETURN=DIR/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.signernocert.dated.resp **** exim CALLER@test.ex test message. diff --git a/test/src/client.c b/test/src/client.c index fe646d64f..5e6b6472a 100644 --- a/test/src/client.c +++ b/test/src/client.c @@ -199,6 +199,33 @@ setup_verify(BIO *bp, char *CAfile, char *CApath) #ifndef DISABLE_OCSP +static STACK_OF(X509) * +cert_stack_from_store(X509_STORE * store) +{ +STACK_OF(X509_OBJECT) * roots= store->objs; +STACK_OF(X509) * sk = sk_X509_new_null(); +int i; + +for(i = sk_X509_OBJECT_num(roots) - 1; i >= 0; i--) + { + X509_OBJECT * tmp_obj= sk_X509_OBJECT_value(roots, i); + if(tmp_obj->type == X509_LU_X509) + { + X509 * x = tmp_obj->data.x509; + sk_X509_push(sk, x); + } + } +return sk; +} + +static void +cert_stack_free(STACK_OF(X509) * sk) +{ +while (sk_X509_num(sk) > 0) (void) sk_X509_pop(sk); +sk_X509_free(sk); +} + + static int tls_client_stapling_cb(SSL *s, void *arg) { @@ -208,6 +235,7 @@ OCSP_RESPONSE *rsp; OCSP_BASICRESP *bs; char *CAfile = NULL; X509_STORE *store = NULL; +STACK_OF(X509) * sk; int ret = 1; len = SSL_get_tlsext_status_ocsp_resp(s, &p); @@ -229,6 +257,7 @@ if(!(bs = OCSP_response_get1_basic(rsp))) return 0; } + CAfile = ocsp_stapling; if(!(store = setup_verify(arg, CAfile, NULL))) { @@ -236,8 +265,14 @@ if(!(store = setup_verify(arg, CAfile, NULL))) return 0; } -/* No file of alternate certs, no options */ -if(OCSP_basic_verify(bs, NULL, store, 0) <= 0) +sk = cert_stack_from_store(store); + +/* OCSP_basic_verify takes a "store" arg, but does not +use it for the chain verification, which is all we do +when OCSP_NOVERIFY is set. The content from the wire +(in "bs") and a cert-stack "sk" are all that is used. */ + +if(OCSP_basic_verify(bs, sk, NULL, OCSP_NOVERIFY) <= 0) { BIO_printf(arg, "Response Verify Failure\n"); ERR_print_errors(arg); @@ -246,6 +281,7 @@ if(OCSP_basic_verify(bs, NULL, store, 0) <= 0) else BIO_printf(arg, "Response verify OK\n"); +cert_stack_free(sk); X509_STORE_free(store); return ret; } diff --git a/test/stderr/5610 b/test/stderr/5610 index 9282f1ea0..045fadc9b 100644 --- a/test/stderr/5610 +++ b/test/stderr/5610 @@ -1,78 +1,2 @@ ******** SERVER ******** -Exim version x.yz .... -configuration file is TESTSUITE/test-config -admin user -daemon_smtp_port overridden by -oX: - <: 1225 -listening on all interfaces (IPv6) port 1225 -listening on all interfaces (IPv4) port 1225 -pid written to TESTSUITE/spool/exim-daemon.pid -LOG: MAIN - exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225 -daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID -Listening... -Connection request from ip4.ip4.ip4.ip4 port 51808 -1 SMTP accept process running -Listening... -11402 Process 11402 is handling incoming connection from [ip4.ip4.ip4.ip4] -LOG: MAIN - acl_conn: ocsp in status: 0 (notreq) -Process 11402 is ready for new message -setting SSL CTX options: 0x1100000 -Diffie-Hellman initialized from default with 2048-bit prime -ECDH: curve 'prime256v1' -ECDH: enabled 'prime256v1' curve -tls_certificate file TESTSUITE/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.pem -tls_privatekey file TESTSUITE/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.unlocked.key -tls_ocsp_file TESTSUITE/aux-fixed/exim-ca/example.com/server1.example.com/server1.example.com.ocsp.good.resp -Initialized TLS -Added 1 certificate authorities. -Calling SSL_accept -SSL info: before/accept initialization -SSL info: before/accept initialization -Received TLS status request (OCSP stapling); have response -SSL info: SSLv3 read client hello A -SSL info: SSLv3 write server hello A -SSL info: SSLv3 write certificate A -SSL info: unknown state -SSL info: SSLv3 write key exchange A -SSL info: SSLv3 write certificate request A -SSL info: SSLv3 flush data -SSL authenticated verify ok: depth=0 SN=/C=UK/O=The Exim Maintainers/OU=Test Suite/CN=Phil Pennock -SSL info: SSLv3 read client certificate A -SSL info: SSLv3 read client key exchange A -SSL info: SSLv3 read certificate verify A -SSL info: SSLv3 read finished A -SSL info: SSLv3 write session ticket A -SSL info: SSLv3 write change cipher spec A -SSL info: SSLv3 write finished A -SSL info: SSLv3 flush data -SSL info: SSL negotiation finished successfully -SSL info: SSL negotiation finished successfully -SSL_accept was successful -Cipher: TLSv1:AES256-SHA:256 -Shared ciphers: AES256-SHA:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA384:AES256-SHA:ECDHE-ECDSA-AES256-SHA:AES256-SHA:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES128-SHA:AES256-SHA:AES256-SHA256:AES256-SHA:CAMELLIA256-SHA:AES256-SHA:AES128-SHA256:AES128-SHA:CAMELLIA128-SHA:DHE-DSS-AES256-SHA:AES256-SHA:AES256-SHA:DHE-DSS-AES256-SHA256:AES256-SHA:DHE-DSS-AES256-SHA:DHE-RSA-CAMELLIA256-SHA:DHE-DSS-CAMELLIA256-SHA:DHE-DSS-AES256-SHA:AES256-SHA:DHE-RSA-AES128-SHA256:DHE-DSS-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-DSS-AES128-SHA:DHE-RSA-CAMELLIA128-SHA:DHE-DSS-CAMELLIA128-SHA:ECDHE-RSA-DES-CBC3-SHA:ECDHE-ECDSA-DES-CBC3-SHA:DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:EDH-DSS-DES-CBC3-SHA -TLS active -Calling SSL_read(0x970690, 0x981230, 4096) -LOG: MAIN - acl_mail: ocsp in status: 4 (verified) -tls_do_write(0x83cc40, 8) -SSL_write(SSL, 0x83cc40, 8) -outbytes=8 error=0 -Calling SSL_read(0x970690, 0x981230, 4096) -tls_do_write(0x83cc40, 14) -SSL_write(SSL, 0x83cc40, 14) -outbytes=14 error=0 -Calling SSL_read(0x970690, 0x981230, 4096) -tls_do_write(0x83cc40, 44) -SSL_write(SSL, 0x83cc40, 44) -outbytes=44 error=0 -tls_close(): shutting down SSL -SSL info: SSL negotiation finished successfully -LOG: smtp_connection MAIN - SMTP connection from [ip4.ip4.ip4.ip4] closed by QUIT -11398 child 11402 ended: status=0x0 -11398 normal exit, 0 -11398 0 SMTP accept processes now running -11398 Listening... |