On 12.12.2010 13:05, Dr Stephen Henson wrote:
> It also makes sense to add a directive to make the OCSP timeout configurable.
> This can be done in the OCSP stapling code but not the OCSP code itself. The
> current default is (I think) the same as the http request timeout which is way
> too long in practice: if an OCSP responder doesn't respond in a few seconds it
> isn't likely to respond at all.
Agreed, attached is v2 of the patch. It adds an SSLOCSPResponderTimeout
directive, which defaults to 10 seconds. I also added the cfgMergeInt
statements in ssl_engine_config.c, which I forgot in v1 by mistake.
There are actually additional improvements I would like to see with the
OCSP (clientauth) checking - in particular, having a cache (possibly
reusing code from the stapling code)... but I was hoping that we could
get the proposed fixes in for 2.3.10, at least. Reviews and/or commits
are much appreciated - thanks!
Kaspar
Index: modules/ssl/ssl_private.h
===================================================================
--- modules/ssl/ssl_private.h (revision 1044771)
+++ modules/ssl/ssl_private.h (working copy)
@@ -179,6 +179,16 @@
#define DEFAULT_RENEG_BUFFER_SIZE (128 * 1024)
#endif
+/* Default for OCSP response validity */
+#ifndef OCSP_MAX_SKEW
+#define OCSP_MAX_SKEW (60 * 5)
+#endif
+
+/* Default timeout for OCSP queries */
+#ifndef OCSP_DEFAULT_TIMEOUT
+#define OCSP_DEFAULT_TIMEOUT 10
+#endif
+
/**
* Support for MM library
*/
@@ -516,6 +526,9 @@
BOOL ocsp_force_default; /* true if the default responder URL is
* used regardless of per-cert URL */
const char *ocsp_responder; /* default responder URL */
+ long ocsp_resptime_skew;
+ long ocsp_resp_maxage;
+ apr_interval_time_t ocsp_responder_timeout;
} modssl_ctx_t;
@@ -620,6 +633,9 @@
const char *ssl_cmd_SSLOCSPOverrideResponder(cmd_parms *cmd, void *dcfg, int
flag);
const char *ssl_cmd_SSLOCSPDefaultResponder(cmd_parms *cmd, void *dcfg, const
char *arg);
+const char *ssl_cmd_SSLOCSPResponseTimeSkew(cmd_parms *cmd, void *dcfg, const
char *arg);
+const char *ssl_cmd_SSLOCSPResponseMaxAge(cmd_parms *cmd, void *dcfg, const
char *arg);
+const char *ssl_cmd_SSLOCSPResponderTimeout(cmd_parms *cmd, void *dcfg, const
char *arg);
const char *ssl_cmd_SSLOCSPEnable(cmd_parms *cmd, void *dcfg, int flag);
const char *ssl_cmd_SSLFIPS(cmd_parms *cmd, void *dcfg, int flag);
Index: modules/ssl/mod_ssl.c
===================================================================
--- modules/ssl/mod_ssl.c (revision 1044771)
+++ modules/ssl/mod_ssl.c (working copy)
@@ -197,6 +197,12 @@
"URL of the default OCSP Responder")
SSL_CMD_SRV(OCSPOverrideResponder, FLAG,
"Force use of the default responder URL ('on', 'off')")
+ SSL_CMD_SRV(OCSPResponseTimeSkew, TAKE1,
+ "Maximum time difference in OCSP responses")
+ SSL_CMD_SRV(OCSPResponseMaxAge, TAKE1,
+ "Maximum age of OCSP responses")
+ SSL_CMD_SRV(OCSPResponderTimeout, TAKE1,
+ "OCSP responder query timeout")
#ifdef HAVE_OCSP_STAPLING
/*
Index: modules/ssl/ssl_engine_ocsp.c
===================================================================
--- modules/ssl/ssl_engine_ocsp.c (revision 1044771)
+++ modules/ssl/ssl_engine_ocsp.c (working copy)
@@ -141,10 +141,10 @@
request = create_request(ctx, cert, &certID, s, pool);
if (request) {
- /* Use default I/O timeout for the server. */
- response = modssl_dispatch_ocsp_request(ruri,
- mySrvFromConn(c)->timeout,
- request, c, pool);
+ apr_interval_time_t to = sc->server->ocsp_responder_timeout == UNSET ?
+ OCSP_DEFAULT_TIMEOUT :
+ sc->server->ocsp_responder_timeout;
+ response = modssl_dispatch_ocsp_request(ruri, to, request, c, pool);
}
if (!request || !response) {
@@ -205,15 +205,16 @@
rc = status;
}
- /* TODO: make these configurable. */
-#define MAX_SKEW (60)
-#define MAX_AGE (360)
-
/* Check whether the response is inside the defined validity
* period; otherwise fail. */
if (rc != V_OCSP_CERTSTATUS_UNKNOWN) {
- int vrc = OCSP_check_validity(thisup, nextup, MAX_SKEW, MAX_AGE);
-
+ long resptime_skew = sc->server->ocsp_resptime_skew == UNSET ?
+ OCSP_MAX_SKEW :
sc->server->ocsp_resptime_skew;
+ /* oscp_resp_maxage can be passed verbatim - UNSET (-1) means
+ * that responses can be of any age as long as nextup is in the
+ * future. */
+ int vrc = OCSP_check_validity(thisup, nextup, resptime_skew,
+ sc->server->ocsp_resp_maxage);
if (vrc != 1) {
ssl_log_ssl_error(SSLLOG_MARK, APLOG_ERR, s);
ssl_log_cxerror(SSLLOG_MARK, APLOG_ERR, 0, c, cert,
@@ -251,6 +252,12 @@
apr_pool_t *vpool;
int rv;
+ /* don't do OCSP checking for valid self-issued certs */
+ if (cert->valid && X509_check_issued(cert,cert) == X509_V_OK) {
+ X509_STORE_CTX_set_error(ctx, X509_V_OK);
+ return 1;
+ }
+
/* Create a temporary pool to constrain memory use (the passed-in
* pool may be e.g. a connection pool). */
apr_pool_create(&vpool, pool);
Index: modules/ssl/ssl_engine_config.c
===================================================================
--- modules/ssl/ssl_engine_config.c (revision 1044771)
+++ modules/ssl/ssl_engine_config.c (working copy)
@@ -130,6 +130,9 @@
mctx->ocsp_enabled = FALSE;
mctx->ocsp_force_default = FALSE;
mctx->ocsp_responder = NULL;
+ mctx->ocsp_resptime_skew = UNSET;
+ mctx->ocsp_resp_maxage = UNSET;
+ mctx->ocsp_responder_timeout = UNSET;
#ifdef HAVE_OCSP_STAPLING
mctx->stapling_enabled = UNSET;
@@ -243,6 +246,9 @@
cfgMergeBool(ocsp_enabled);
cfgMergeBool(ocsp_force_default);
cfgMerge(ocsp_responder, NULL);
+ cfgMergeInt(ocsp_resptime_skew);
+ cfgMergeInt(ocsp_resp_maxage);
+ cfgMergeInt(ocsp_responder_timeout);
#ifdef HAVE_OCSP_STAPLING
cfgMergeBool(stapling_enabled);
cfgMergeInt(stapling_resptime_skew);
@@ -1442,6 +1448,37 @@
return NULL;
}
+const char *ssl_cmd_SSLOCSPResponseTimeSkew(cmd_parms *cmd, void *dcfg, const
char *arg)
+{
+ SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
+ sc->server->ocsp_resptime_skew = atoi(arg);
+ if (sc->server->ocsp_resptime_skew < 0) {
+ return "SSLOCSPResponseTimeSkew: invalid argument";
+ }
+ return NULL;
+}
+
+const char *ssl_cmd_SSLOCSPResponseMaxAge(cmd_parms *cmd, void *dcfg, const
char *arg)
+{
+ SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
+ sc->server->ocsp_resp_maxage = atoi(arg);
+ if (sc->server->ocsp_resp_maxage < 0) {
+ return "SSLOCSPResponseMaxAge: invalid argument";
+ }
+ return NULL;
+}
+
+const char *ssl_cmd_SSLOCSPResponderTimeout(cmd_parms *cmd, void *dcfg, const
char *arg)
+{
+ SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
+ sc->server->ocsp_responder_timeout = atoi(arg);
+ sc->server->ocsp_responder_timeout *= APR_USEC_PER_SEC;
+ if (sc->server->ocsp_responder_timeout < 0) {
+ return "SSLOCSPResponderTimeout: invalid argument";
+ }
+ return NULL;
+}
+
const char *ssl_cmd_SSLProxyCheckPeerExpire(cmd_parms *cmd, void *dcfg, int
flag)
{
SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
Index: docs/manual/mod/mod_ssl.xml
===================================================================
--- docs/manual/mod/mod_ssl.xml (revision 1044771)
+++ docs/manual/mod/mod_ssl.xml (working copy)
@@ -1837,6 +1837,53 @@
</directivesynopsis>
<directivesynopsis>
+<name>SSLOCSPResponseTimeSkew</name>
+<description>Maximum allowable time skew for OCSP response
validation</description>
+<syntax>SSLOCSPResponseTimeSkew <em>seconds</em></syntax>
+<default>SSLOCSPResponseTimeSkew 300</default>
+<contextlist><context>server config</context>
+<context>virtual host</context></contextlist>
+<compatibility>Available in httpd 2.3 and later, if using OpenSSL 0.9.7 or
later</compatibility>
+
+<usage>
+<p>This option sets the maximum allowable time skew for OCSP responses
+(when checking their <code>thisUpdate</code> and <code>nextUpdate</code>
fields).</p>
+</usage>
+</directivesynopsis>
+
+<directivesynopsis>
+<name>SSLOCSPResponseMaxAge</name>
+<description>Maximum allowable age for OCSP responses</description>
+<syntax>SSLOCSPResponseMaxAge <em>seconds</em></syntax>
+<default>SSLOCSPResponseMaxAge -1</default>
+<contextlist><context>server config</context>
+<context>virtual host</context></contextlist>
+<compatibility>Available in httpd 2.3 and later, if using OpenSSL 0.9.7 or
later</compatibility>
+
+<usage>
+<p>This option sets the maximum allowable age ("freshness") for OCSP responses.
+The default value (<code>-1</code>) does not enforce a maximum age,
+which means that OCSP responses are considered valid as long as their
+<code>nextUpdate</code> field is in the future.</p>
+</usage>
+</directivesynopsis>
+
+<directivesynopsis>
+<name>SSLOCSPResponderTimeout</name>
+<description>Timeout for OCSP queries</description>
+<syntax>SSLOCSPResponderTimeout <em>seconds</em></syntax>
+<default>SSLOCSPResponderTimeout 10</default>
+<contextlist><context>server config</context>
+<context>virtual host</context></contextlist>
+<compatibility>Available in httpd 2.3 and later, if using OpenSSL 0.9.7 or
later</compatibility>
+
+<usage>
+<p>This option sets the timeout for queries to OCSP responders, when
+<directive module="mod_ssl">SSLOCSPEnable</directive> is turned on.</p>
+</usage>
+</directivesynopsis>
+
+<directivesynopsis>
<name>SSLInsecureRenegotiation</name>
<description>Option to enable support for insecure renegotiation</description>
<syntax>SSLInsecureRenegotiation <em>flag</em></syntax>
