You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Kaspar Brand <ht...@velox.ch> on 2014/04/02 08:21:18 UTC
Re: svn commit: r1583191 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c
ssl_engine_config.c ssl_engine_ocsp.c ssl_private.h
On 30.03.2014 21:25, ylavic@apache.org wrote:
> Author: ylavic
> Date: Sun Mar 30 19:25:20 2014
> New Revision: 1583191
>
> URL: http://svn.apache.org/r1583191
> Log:
> mod_ssl: send OCSP request's nonce according to SSLOCSPUseRequestNonce on/off. PR 56233.
> @@ -171,7 +174,8 @@ static int verify_ocsp_status(X509 *cert
> }
> }
>
> - if (rc == V_OCSP_CERTSTATUS_GOOD) {
> + if (rc == V_OCSP_CERTSTATUS_GOOD &&
> + sc->server->ocsp_use_request_nonce != FALSE) {
> if (OCSP_check_nonce(request, basicResponse) != 1) {
> ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01924)
> "Bad OCSP responder answer (bad nonce)");
Perhaps rewrite this as
if (rc == V_OCSP_CERTSTATUS_GOOD &&
sc->server->ocsp_use_request_nonce != FALSE &&
OCSP_check_nonce(request, basicResponse) != 1) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01924)
"Bad OCSP responder answer (bad nonce)");
rc = V_OCSP_CERTSTATUS_UNKNOWN;
}
}
?
> Modified: httpd/httpd/trunk/modules/ssl/ssl_private.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_private.h?rev=1583191&r1=1583190&r2=1583191&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ssl/ssl_private.h (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_private.h Sun Mar 30 19:25:20 2014
> @@ -614,6 +614,7 @@ typedef struct {
> SSL_CONF_CTX *ssl_ctx_config; /* Configuration context */
> apr_array_header_t *ssl_ctx_param; /* parameters to pass to SSL_CTX */
> #endif
> + int ocsp_use_request_nonce;
> } modssl_ctx_t;
modssl_ctx_t isn't a public struct, so I think it's preferrable to
insert this definition four lines earlier (after ocsp_responder_timeout,
see r1059917 for a similar case).
And last but not least, can you add docs for this new directive (and an
entry in CHANGES)?
Kaspar
Re: svn commit: r1583191 - in /httpd/httpd/trunk/modules/ssl:
mod_ssl.c ssl_engine_config.c ssl_engine_ocsp.c ssl_private.h
Posted by Yann Ylavic <yl...@gmail.com>.
Done in r1584098.
On Wed, Apr 2, 2014 at 8:21 AM, Kaspar Brand <ht...@velox.ch> wrote:
> On 30.03.2014 21:25, ylavic@apache.org wrote:
>> Author: ylavic
>> Date: Sun Mar 30 19:25:20 2014
>> New Revision: 1583191
>>
>> URL: http://svn.apache.org/r1583191
>> Log:
>> mod_ssl: send OCSP request's nonce according to SSLOCSPUseRequestNonce on/off. PR 56233.
>
>> @@ -171,7 +174,8 @@ static int verify_ocsp_status(X509 *cert
>> }
>> }
>>
>> - if (rc == V_OCSP_CERTSTATUS_GOOD) {
>> + if (rc == V_OCSP_CERTSTATUS_GOOD &&
>> + sc->server->ocsp_use_request_nonce != FALSE) {
>> if (OCSP_check_nonce(request, basicResponse) != 1) {
>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01924)
>> "Bad OCSP responder answer (bad nonce)");
>
> Perhaps rewrite this as
>
> if (rc == V_OCSP_CERTSTATUS_GOOD &&
> sc->server->ocsp_use_request_nonce != FALSE &&
> OCSP_check_nonce(request, basicResponse) != 1) {
> ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01924)
> "Bad OCSP responder answer (bad nonce)");
> rc = V_OCSP_CERTSTATUS_UNKNOWN;
> }
> }
>
> ?
>
>> Modified: httpd/httpd/trunk/modules/ssl/ssl_private.h
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_private.h?rev=1583191&r1=1583190&r2=1583191&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/ssl/ssl_private.h (original)
>> +++ httpd/httpd/trunk/modules/ssl/ssl_private.h Sun Mar 30 19:25:20 2014
>> @@ -614,6 +614,7 @@ typedef struct {
>> SSL_CONF_CTX *ssl_ctx_config; /* Configuration context */
>> apr_array_header_t *ssl_ctx_param; /* parameters to pass to SSL_CTX */
>> #endif
>> + int ocsp_use_request_nonce;
>> } modssl_ctx_t;
>
> modssl_ctx_t isn't a public struct, so I think it's preferrable to
> insert this definition four lines earlier (after ocsp_responder_timeout,
> see r1059917 for a similar case).
>
> And last but not least, can you add docs for this new directive (and an
> entry in CHANGES)?
>
> Kaspar