Merge r1641077, r1641095 from trunk:

mod_ssl: Fix recognition of OCSP stapling responses that are encoded
         improperly or too large.

The one byte "ok" flag stored with the response was accounted for in
the wrong condition.

follow up to r1641077:

one bug was traded for another in r1641077; track the response
length and the cached object length separately to avoid such
confusion

Submitted by: trawick
Reviewed/backported by: jim

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1645935 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 7039840da14ad3a9a5a8b796d16607528c314d3f..b0d5a699cdffd3daf35cd7d4b789e57dc02fa597 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -22,6 +22,9 @@ Changes with Apache 2.4.11
      request headers earlier.  Adds "MergeTrailers" directive to restore
      legacy behavior.  [Edward Lu, Yann Ylavic, Joe Orton, Eric Covener]
 
+  *) mod_ssl: Fix recognition of OCSP stapling responses that are encoded
+     improperly or too large.  [Jeff Trawick]
+
   *) mod_proxy_fcgi, mod_authnz_fcgi: stop reading the response and issue an
      error when parsing or forwarding the response fails. [Yann Ylavic]
 

diff --git a/modules/ssl/ssl_util_stapling.c b/modules/ssl/ssl_util_stapling.c
index 81e95b41cad7ee7e653e977f1b13045985fef801..0e83baf3ea9061b92044cea4fe4c4301d4f57c26 100644 (file)
--- a/modules/ssl/ssl_util_stapling.c
+++ b/modules/ssl/ssl_util_stapling.c
@@ -210,13 +210,13 @@ static BOOL stapling_cache_response(server_rec *s, modssl_ctx_t *mctx,
                                     BOOL ok, apr_pool_t *pool)
 {
     SSLModConfigRec *mc = myModConfig(s);
-    unsigned char resp_der[MAX_STAPLING_DER];
+    unsigned char resp_der[MAX_STAPLING_DER]; /* includes one-byte flag + response */
     unsigned char *p;
-    int resp_derlen;
+    int resp_derlen, stored_len;
     BOOL rv;
     apr_time_t expiry;
 
-    resp_derlen = i2d_OCSP_RESPONSE(rsp, NULL) + 1;
+    resp_derlen = i2d_OCSP_RESPONSE(rsp, NULL);
 
     if (resp_derlen <= 0) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01927)
@@ -224,7 +224,8 @@ static BOOL stapling_cache_response(server_rec *s, modssl_ctx_t *mctx,
         return FALSE;
     }
 
-    if (resp_derlen > sizeof resp_der) {
+    stored_len = resp_derlen + 1; /* response + ok flag */
+    if (stored_len > sizeof resp_der) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01928)
                      "OCSP stapling response too big (%u bytes)", resp_derlen);
         return FALSE;
@@ -248,7 +249,7 @@ static BOOL stapling_cache_response(server_rec *s, modssl_ctx_t *mctx,
 
     rv = mc->stapling_cache->store(mc->stapling_cache_context, s,
                                    cinf->idx, sizeof(cinf->idx),
-                                   expiry, resp_der, resp_derlen, pool);
+                                   expiry, resp_der, stored_len, pool);
     if (rv != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, APLOGNO(01929)
                      "stapling_cache_response: OCSP response session store error!");