Merge of /httpd/httpd/trunk:r1893359

  *) mod_md: fixed a bug in handling multiple parallel OCSP requests. These could
     run into an assertion which terminated (and restarted) the child process where
     the task was running. Eventually, all OCSP responses were collected, but not
     in the way that things are supposed to work.
     See also <https://bz.apache.org/bugzilla/show_bug.cgi?id=65567>.
     The bug was possibly triggered when more than one OCSP status needed updating
     at the same time. For example for several renewed certificates after a server
     reload.

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

diff --git a/changes-entries/md_ocsp_multi_update.txt b/changes-entries/md_ocsp_multi_update.txt
new file mode 100644 (file)
index 0000000..e927c79
--- /dev/null
+++ b/changes-entries/md_ocsp_multi_update.txt
@@ -0,0 +1,8 @@
+  *) mod_md: fixed a bug in handling multiple parallel OCSP requests. These could
+     run into an assertion which terminated (and restarted) the child process where
+     the task was running. Eventually, all OCSP responses were collected, but not
+     in the way that things are supposed to work.
+     See also <https://bz.apache.org/bugzilla/show_bug.cgi?id=65567>.
+     The bug was possibly triggered when more than one OCSP status needed updating
+     at the same time. For example for several renewed certificates after a server
+     reload.

diff --git a/modules/md/md_curl.c b/modules/md/md_curl.c
index 59d83e52e8846105002340855fd5d9b665779427..e05f37b8997b41a865d3e86a8a02c7feb85e5496 100644 (file)
--- a/modules/md/md_curl.c
+++ b/modules/md/md_curl.c
@@ -491,7 +491,7 @@ static apr_status_t md_curl_multi_perform(md_http_t *http, apr_pool_t *p,
             else if (APR_STATUS_IS_ENOENT(rv)) {
                 md_log_perror(MD_LOG_MARK, MD_LOG_TRACE3, 0, p, 
                               "multi_perform[%d reqs]: no more requests", requests->nelts);
-                if (!running) {
+                if (!requests->nelts) {
                     goto leave;
                 }
                 break;
@@ -524,13 +524,13 @@ static apr_status_t md_curl_multi_perform(md_http_t *http, apr_pool_t *p,
         }
 
         /* process status messages, e.g. that a request is done */
-        while (1) {
+        while (running < requests->nelts) {
             curlmsg = curl_multi_info_read(curlm, &msgcount);
             if (!curlmsg) break;
             if (curlmsg->msg == CURLMSG_DONE) {
                 req = find_curl_request(requests, curlmsg->easy_handle);
                 if (req) {
-                    md_log_perror(MD_LOG_MARK, MD_LOG_TRACE3, 0, p, 
+                    md_log_perror(MD_LOG_MARK, MD_LOG_TRACE2, 0, p,
                                   "multi_perform[%d reqs]: req[%d] done", 
                                   requests->nelts, req->id);
                     update_status(req);
@@ -546,7 +546,6 @@ static apr_status_t md_curl_multi_perform(md_http_t *http, apr_pool_t *p,
                 }
             }
         }
-        assert(running == requests->nelts);
     };
 
 leave:

diff --git a/modules/md/md_ocsp.c b/modules/md/md_ocsp.c
index 95ecaa3a68964b11856970eb924b4d7174133e53..01e601f765918b52d8de787520409f00f507900a 100644 (file)
--- a/modules/md/md_ocsp.c
+++ b/modules/md/md_ocsp.c
@@ -339,7 +339,7 @@ apr_status_t md_ocsp_prime(md_ocsp_reg_t *reg, const char *ext_id, apr_size_t ex
     rv = md_cert_get_ocsp_responder_url(&ostat->responder_url, reg->p, cert);
     if (APR_SUCCESS != rv) {
         md_log_perror(MD_LOG_MARK, MD_LOG_ERR, rv, reg->p,
-                      "md[%s]: certificate with serial %s has not OCSP responder URL",
+                      "md[%s]: certificate with serial %s has no OCSP responder URL",
                       name, md_cert_get_serial_number(cert, reg->p));
         goto cleanup;
     }
@@ -609,7 +609,11 @@ static apr_status_t ostat_on_resp(const md_http_response_t *resp, void *baton)
     if (NULL == (ocsp_resp = d2i_OCSP_RESPONSE(NULL, (const unsigned char**)&der.data, 
                                                (long)der.len))) {
         rv = APR_EINVAL;
-        md_result_set(update->result, rv, "response body does not parse as OCSP response");
+
+        md_result_set(update->result, rv,
+                      apr_psprintf(req->pool, "req[%d] response body does not parse as "
+                                   "OCSP response, status=%d, body brigade length=%ld",
+                                   resp->req->id, resp->status, (long)der.len));
         md_result_log(update->result, MD_LOG_DEBUG);
         goto cleanup;
     }
@@ -635,7 +639,7 @@ static apr_status_t ostat_on_resp(const md_http_response_t *resp, void *baton)
      * to accept it. */
     switch ((n = OCSP_check_nonce(ostat->ocsp_req, basic_resp))) {
         case 1:
-            md_log_perror(MD_LOG_MARK, MD_LOG_DEBUG, 0, req->pool, 
+            md_log_perror(MD_LOG_MARK, MD_LOG_TRACE3, 0, req->pool,
                           "req[%d]: OCSP respoonse nonce does match", req->id);
             break;
         case 0:
@@ -645,7 +649,7 @@ static apr_status_t ostat_on_resp(const md_http_response_t *resp, void *baton)
             goto cleanup;
             
         case -1:
-            md_log_perror(MD_LOG_MARK, MD_LOG_TRACE1, 0, req->pool, 
+            md_log_perror(MD_LOG_MARK, MD_LOG_TRACE3, 0, req->pool,
                           "req[%d]: OCSP respoonse did not return the nonce", req->id);
             break;
         default:
@@ -832,6 +836,9 @@ static apr_status_t next_todo(md_http_request_t **preq, void *baton,
             md_http_set_on_status_cb(req, ostat_on_req_status, update);
             md_http_set_on_response_cb(req, ostat_on_resp, update);
             rv = APR_SUCCESS;
+            md_log_perror(MD_LOG_MARK, MD_LOG_TRACE2, 0, req->pool,
+                          "scheduling OCSP request for %s, %d request in flight",
+                          ostat->md_name, in_flight);
         }
     }
 cleanup:

diff --git a/modules/md/md_reg.c b/modules/md/md_reg.c
index 7d61f0c5d457ae8865e2acf1434facb7d7b2b5cf..b500ddc601a2de4a92f1f02699733ade430ff8e4 100644 (file)
--- a/modules/md/md_reg.c
+++ b/modules/md/md_reg.c
@@ -549,7 +549,11 @@ static apr_status_t pubcert_load(void *baton, apr_pool_t *p, apr_pool_t *ptemp,
         rv = md_pubcert_load(reg->store, group, md->name, spec, &certs, p);
     }
     if (APR_SUCCESS != rv) goto leave;
-            
+    if (certs->nelts == 0) {
+        rv = APR_ENOENT;
+        goto leave;
+    }
+
     pubcert = apr_pcalloc(p, sizeof(*pubcert));
     pubcert->certs = certs;
     cert = APR_ARRAY_IDX(certs, 0, const md_cert_t *);

diff --git a/modules/md/md_store_fs.c b/modules/md/md_store_fs.c
index 038bcc50c42ddc991a1e4d272d715f244b5b22d0..959bafee80f587b94752d5476a40112613172b3b 100644 (file)
--- a/modules/md/md_store_fs.c
+++ b/modules/md/md_store_fs.c
@@ -508,19 +508,21 @@ static apr_status_t mk_group_dir(const char **pdir, md_store_fs_t *s_fs,
 
     rv = md_util_is_dir(*pdir, p);
     if (APR_STATUS_IS_ENOENT(rv)) {
-        md_log_perror(MD_LOG_MARK, MD_LOG_DEBUG, rv, p, "not a directory, creating %s", *pdir);
+        md_log_perror(MD_LOG_MARK, MD_LOG_TRACE3, rv, p, "not a directory, creating %s", *pdir);
         rv = apr_dir_make_recursive(*pdir, perms->dir, p);
         if (APR_SUCCESS != rv) goto cleanup;
         dispatch(s_fs, MD_S_FS_EV_CREATED, group, *pdir, APR_DIR, p);
     }
 
     rv = apr_file_perms_set(*pdir, perms->dir);
-    md_log_perror(MD_LOG_MARK, MD_LOG_DEBUG, rv, p, "mk_group_dir %s perm set", *pdir);
+    md_log_perror(MD_LOG_MARK, MD_LOG_TRACE3, rv, p, "mk_group_dir %s perm set", *pdir);
     if (APR_STATUS_IS_ENOTIMPL(rv)) {
         rv = APR_SUCCESS;
     }
 cleanup:
-    md_log_perror(MD_LOG_MARK, MD_LOG_DEBUG, rv, p, "mk_group_dir %d %s", group, name);
+    if (APR_SUCCESS != rv) {
+        md_log_perror(MD_LOG_MARK, MD_LOG_ERR, rv, p, "mk_group_dir %d %s", group, name);
+    }
     return rv;
 }
 

diff --git a/modules/md/md_version.h b/modules/md/md_version.h
index f76ed708c2489e9a64221ac52e72dde034d8bd4b..e52c2fce8465158a509326c6c539f6a5272c8a74 100644 (file)
--- a/modules/md/md_version.h
+++ b/modules/md/md_version.h
@@ -27,7 +27,7 @@
  * @macro
  * Version number of the md module as c string
  */
-#define MOD_MD_VERSION "2.4.5"
+#define MOD_MD_VERSION "2.4.6"
 
 /**
  * @macro
@@ -35,7 +35,7 @@
  * release. This is a 24 bit number with 8 bits for major number, 8 bits
  * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
  */
-#define MOD_MD_VERSION_NUM 0x020404
+#define MOD_MD_VERSION_NUM 0x020406
 
 #define MD_ACME_DEF_URL    "https://acme-v02.api.letsencrypt.org/directory"