From: Jim Jagielski <jim@apache.org>
Date: Tue, 16 Dec 2014 13:06:19 +0000 (+0000)
Subject: Merge r1641077, r1641095 from trunk:
X-Git-Tag: 2.4.11~85
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1910951711da2a1e7059391306b70f6744585d26;p=apache

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 7039840da1..b0d5a699cd 100644
--- 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 81e95b41ca..0e83baf3ea 100644
--- 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!");