Hi Jean-Frédéric, On Fri, Feb 3, 2017 at 5:19 PM, <jfcl...@apache.org> wrote: > Author: jfclere > Date: Fri Feb 3 16:19:17 2017 > New Revision: 1781575 > > URL: http://svn.apache.org/viewvc?rev=1781575&view=rev > Log: > Add Configuration for trusted OCSP responder certificates > Fix for PR 46037 > > Modified: > httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
The merge of SSLOCSPNoverify looks incorrect. First you want it to be Off by default (i.e. verify the OCSP's responder certificate when not configured), right? Couldn't that break existing configurations since we currently (until 2.4.25) do not verify it? > > Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=1781575&r1=1781574&r2=1781575&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original) > +++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Fri Feb 3 16:19:17 2017 > @@ -146,6 +146,13 @@ static void modssl_ctx_init(modssl_ctx_t > mctx->ocsp_use_request_nonce = UNSET; > mctx->proxy_uri = NULL; > > +/* Set OCSP Responder Certificate Verification variable */ > + mctx->ocsp_noverify = FALSE; We possibly want UNSET here for merges to work... > +/* Set OCSP Responder File variables */ > + mctx->ocsp_verify_flags = 0; > + mctx->ocsp_certs_file = NULL; > + mctx->ocsp_certs = NULL; > + [...] > > Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_ocsp.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_ocsp.c?rev=1781575&r1=1781574&r2=1781575&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/ssl/ssl_engine_ocsp.c (original) > +++ httpd/httpd/trunk/modules/ssl/ssl_engine_ocsp.c Fri Feb 3 16:19:17 2017 > @@ -183,12 +183,16 @@ static int verify_ocsp_status(X509 *cert > } > > if (rc == V_OCSP_CERTSTATUS_GOOD) { > - /* TODO: allow flags configuration. */ > - if (OCSP_basic_verify(basicResponse, NULL, > X509_STORE_CTX_get0_store(ctx), 0) != 1) { > - ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01925) > - "failed to verify the OCSP response"); > - ssl_log_ssl_error(SSLLOG_MARK, APLOG_ERR, s); > - rc = V_OCSP_CERTSTATUS_UNKNOWN; > + /* Check if OCSP certificate verification required */ > + if (!sc->server->ocsp_noverify) { ... and: + if (sc->server->ocsp_noverify != TRUE) { above. > + /* Modify OCSP response verification to include OCSP Responder > cert */ > + if (OCSP_basic_verify(basicResponse, sc->server->ocsp_certs, > X509_STORE_CTX_get0_store(ctx), > + sc->server->ocsp_verify_flags) != 1) { > + ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01925) > + "failed to verify the OCSP response"); > + ssl_log_ssl_error(SSLLOG_MARK, APLOG_ERR, s); > + rc = V_OCSP_CERTSTATUS_UNKNOWN; > + } > } > } Regards, Yann.
