mod_ssl OCSP tuning (Re: TR of 2.3.10)
On 11.12.2010 20:27, Jim Jagielski wrote:
I've heard no objections, so on monday (12/13) I'll start
the TR.
Is there any chance that the attached patch might make it into
2.3.10? It includes two OCSP related changes for mod_ssl:
- addresses https://issues.apache.org/bugzilla/show_bug.cgi?id=49784
by adding two config directives (SSLOCSPResponseTimeSkew and
SSLOCSPResponseMaxAge) and defining new default values
- prevents mod_ssl from doing unnecessary OCSP checks
(valid self-issued certs, i.e. trust anchors configured through
SSLCACertificate{File,Path})
Note that mod_ssl's current hardcoded OCSP defaults for the time skew
(60 seconds) and the max age (360 seconds) are quite aggressive -
especially the latter one. As PR 49784 illustrates, real-world OCSP
responses often have a validity of one or more days, and are not updated
at 5-minute intervals. I therefore suggest to default to -1 for the max
age, and to 300 seconds for the time skew - this also matches the
defaults which are currently applied in mod_ssl's OCSP stapling code.
Kaspar
Index: modules/ssl/ssl_private.h
===
--- modules/ssl/ssl_private.h (revision 1044771)
+++ modules/ssl/ssl_private.h (working copy)
@@ -179,6 +179,11 @@
#define DEFAULT_RENEG_BUFFER_SIZE (128 * 1024)
#endif
+/* Default for OCSP response validity */
+#ifndef OCSP_MAX_SKEW
+#define OCSP_MAX_SKEW (60 * 5)
+#endif
+
/**
* Support for MM library
*/
@@ -516,6 +521,8 @@
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;
} modssl_ctx_t;
@@ -620,6 +627,8 @@
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_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,10 @@
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)
#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)
@@ -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,8 @@
mctx-ocsp_enabled= FALSE;
mctx-ocsp_force_default = FALSE;
mctx-ocsp_responder = NULL;
+
Re: mod_ssl OCSP tuning (Re: TR of 2.3.10)
On 12/12/2010 09:28, Kaspar Brand wrote:
On 11.12.2010 20:27, Jim Jagielski wrote:
I've heard no objections, so on monday (12/13) I'll start
the TR.
Is there any chance that the attached patch might make it into
2.3.10? It includes two OCSP related changes for mod_ssl:
- addresses https://issues.apache.org/bugzilla/show_bug.cgi?id=49784
by adding two config directives (SSLOCSPResponseTimeSkew and
SSLOCSPResponseMaxAge) and defining new default values
- prevents mod_ssl from doing unnecessary OCSP checks
(valid self-issued certs, i.e. trust anchors configured through
SSLCACertificate{File,Path})
Note that mod_ssl's current hardcoded OCSP defaults for the time skew
(60 seconds) and the max age (360 seconds) are quite aggressive -
especially the latter one. As PR 49784 illustrates, real-world OCSP
responses often have a validity of one or more days, and are not updated
at 5-minute intervals. I therefore suggest to default to -1 for the max
age, and to 300 seconds for the time skew - this also matches the
defaults which are currently applied in mod_ssl's OCSP stapling code.
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.
Steve.
--
Dr Stephen N. Henson. Senior Technical/Cryptography Advisor,
Open Source Software Institute: www.oss-institute.org
OpenSSL Core team: www.openssl.org
Bug report for Apache httpd-1.3 [2010/12/12]
+---+ | Bugzilla Bug ID | | +-+ | | Status: UNC=Unconfirmed NEW=New ASS=Assigned| | | OPN=ReopenedVER=Verified(Skipped Closed/Resolved) | | | +-+ | | | Severity: BLK=Blocker CRI=Critical REG=Regression MAJ=Major | | | | MIN=Minor NOR=NormalENH=Enhancement TRV=Trivial | | | | +-+ | | | | Date Posted | | | | | +--+ | | | | | Description | | | | | | | |10744|New|Nor|2002-07-12|suexec might fail to open log file| |10747|New|Maj|2002-07-12|ftp SIZE command and 'smart' ftp servers results i| |10760|New|Maj|2002-07-12|empty ftp directory listings from cached ftp direc| |14518|Opn|Reg|2002-11-13|QUERY_STRING parts not incorporated by mod_rewrite| |16013|Opn|Nor|2003-01-13|Fooling mod_autoindex + IndexIgnore | |16631|Inf|Min|2003-01-31|.htaccess errors logged outside the virtual host l| |17318|Inf|Cri|2003-02-23|Abend on deleting a temporary cache file if proxy | |19279|Inf|Min|2003-04-24|Invalid chmod options in solaris build| |21637|Inf|Nor|2003-07-16|Timeout causes a status code of 200 to be logged | |21777|Inf|Min|2003-07-21|mod_mime_magic doesn't handle little gif files| |21975|Opn|Nor|2003-07-29|mod_rewrite RewriteMap from external program gets | |22618|New|Maj|2003-08-21|MultiViews invalidates PATH_TRANSLATED if cgi-wrap| |25057|Inf|Maj|2003-11-27|Empty PUT access control in .htaccess overrides co| |26126|New|Nor|2004-01-14|mod_include hangs with request body | |26152|Ass|Nor|2004-01-15|Apache 1.3.29 and below directory traversal vulner| |26790|New|Maj|2004-02-09|error deleting old cache file | |29257|Opn|Nor|2004-05-27|Problem with apache-1.3.31 and mod_frontpage (dso,| |29498|New|Maj|2004-06-10|non-anonymous ftp broken in mod_proxy | |30207|New|Nor|2004-07-20|Piped logs don't close read end of pipe | |30877|New|Nor|2004-08-26|htpasswd clears passwd file on Sun when /var/tmp i| |30909|New|Cri|2004-08-28|sporadic segfault resulting in broken connections | |31975|New|Nor|2004-10-29|httpd-1.3.33: buffer overflow in htpasswd if calle| |32078|New|Enh|2004-11-05|clean up some compiler warnings | |32539|New|Trv|2004-12-06|[PATCH] configure --enable-shared= brocken on SuSE| |32974|Inf|Maj|2005-01-06|Client IP not set | |33086|New|Nor|2005-01-13|unconsistency betwen 404 displayed path and server| |33495|Inf|Cri|2005-02-10|Apache crashes with WSADuplicateSocket failed for| |33772|New|Nor|2005-02-28|inconsistency in manual and error reporting by sue| |33875|New|Enh|2005-03-07|Apache processes consuming CPU| |34108|New|Nor|2005-03-21|mod_negotiation changes mtime to mtime of Document| |34114|New|Nor|2005-03-21|Apache could interleave log entries when writing t| |34404|Inf|Blk|2005-04-11|RewriteMap prg can not handle fpout | |34571|Inf|Maj|2005-04-22|Apache 1.3.33 stops logging vhost| |34573|Inf|Maj|2005-04-22|.htaccess not working / mod_auth_mysql| |35424|New|Nor|2005-06-20|httpd disconnect in Timeout on CGI| |35439|New|Nor|2005-06-21|Problem with remove /../ in util.c and mod_rewri| |35547|Inf|Maj|2005-06-29|Problems with libapreq 1.2 and Apache::Cookie | |3|New|Nor|2005-06-30|Can't find DBM on Debian Sarge| |36375|Opn|Nor|2005-08-26|Cannot include http_config.h from C++ file| |37166|New|Nor|2005-10-19|Under certain conditions, mod_cgi delivers an empt| |37252|New|Reg|2005-10-26|gen_test_char reject NLS string | |38989|New|Nor|2006-03-15|restart + piped logs stalls httpd for 24 minutes (| |39104|New|Enh|2006-03-25|[FR] fix build with -Wl,--as-needed | |39287|New|Nor|2006-04-12|Incorrect If-Modified-Since validation (due to syn| |39937|New|Nor|2006-06-30|Garbage output if README.html is gzipped or compre| |40224|Ver|Nor|2006-08-10|System time crashes Apache @year 2038 (win32 only?| |41279|New|Nor|2007-01-02|Apache 1.3.37 htpasswd is vulnerable to buffer ove| |42355|New|Maj|2007-05-08|Apache 1.3 permits non-rfc HTTP error code = 600 | |43626|New|Maj|2007-10-15|r-path_info returning invalid value | |44768|New|Blk|2008-04-07|Server suddenly reverted to showing test page only| |44926|New|Nor|2008-05-02|1.3.41 binary downloads are faulty MSIs |
Re: Flip default of Header directive to always from onsuccess
On Friday 29 October 2010, Eric Covener wrote: http://httpd.apache.org/docs/2.3/mod/mod_headers.html#header doc says: By default, this directive only affects successful responses (responses in the 2xx range). The optional condition can be either onsuccess (default) or always (all status codes, including successful responses). A value of always may be needed to influence headers set by some internal modules even for successful responses, and is always needed to affect non-2xx responses such as redirects or client errors. Any thoughts on flipping the default in 2.3 to always? I think this is a holdover from older releases where some of these directives had Error* equivalents. +1 'always' looks like the more reasonable default to me
Re: SetVirtualDocumentRoot / per request document root / context root?
On 13 Dec 2010, at 12:23 AM, Stefan Fritsch wrote: I have looked at the patch and it looks reasonable. The fact that two known modules (mod_vhost_ldap and mod_ftp) copy the whole server_rec just to change the document root means that this feature is needed. An idea from left field. Is there a reason that DocumentRoot is a virtual host wide setting? Does it not make sense to make it per-Location instead? Regards, Graham --
Re: SetVirtualDocumentRoot / per request document root / context root?
On 12/12/2010 5:56 PM, Graham Leggett wrote: On 13 Dec 2010, at 12:23 AM, Stefan Fritsch wrote: I have looked at the patch and it looks reasonable. The fact that two known modules (mod_vhost_ldap and mod_ftp) copy the whole server_rec just to change the document root means that this feature is needed. An idea from left field. Is there a reason that DocumentRoot is a virtual host wide setting? Yes, root describes /, there is only one Location /. Does it not make sense to make it per-Location instead? No, but MountFilePath /path/to/foo in a Location basis makes sense. You might think I just said the same thing, but I had brought this up before back around the beginnings of 2.0. DocumentRoot is an awfully ancient and limiting construct. Some suggest Alias is the same thing (which it is, in one sense), but it seems like we could fold both into a single concept. The reason you are still stuck with legacy docroot constructs is CGI, etc.
Re: SetVirtualDocumentRoot / per request document root / context root?
On 12/12/2010 4:23 PM, Stefan Fritsch wrote: On Wednesday 10 November 2010, Stefan Fritsch wrote: The frequency with which this gets asked seems to make it worthwhile doing something, even if this patch isn't the right thing to do. Maybe the larger solution of having a document_root field in the request_rec would be better. Has anyone had a chance to look at Ondrej's patch, already? I didn't get around to doing it, yet. https://issues.apache.org/bugzilla/show_bug.cgi?id=49705 I have looked at the patch and it looks reasonable. The fact that two known modules (mod_vhost_ldap and mod_ftp) copy the whole server_rec just to change the document root means that this feature is needed. Well, at least for mod_ftp it is... I am wondering if the people who want this for mod_vhost_* actually want something else, namely an easy way to get the root dir of a web application in php/cgi/whatever... What about this idea: Add two new cgi env variables: CONTEXT_ROOT and CONTEXT_ROOT_PATH (or APP_ROOT/APP_ROOT_PATH or ...). And add a new config directive Feh... DOCUMENT_ROOT was bad enough, do we have to persist the concept of sharing such things with the app? For that matter, what are the vars that are used in app doc models today, why would we have to invent this again?
Re: mod_ssl OCSP tuning (Re: TR of 2.3.10)
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) {
