You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ic...@apache.org on 2021/09/15 13:22:27 UTC

svn commit: r1893359 - in /httpd/httpd/trunk: changes-entries/md_ocsp_multi_update.txt modules/md/md_curl.c modules/md/md_ocsp.c modules/md/md_reg.c modules/md/md_store_fs.c modules/md/md_version.h

Author: icing
Date: Wed Sep 15 13:22:27 2021
New Revision: 1893359

URL: http://svn.apache.org/viewvc?rev=1893359&view=rev
Log:
  *) 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.


Added:
    httpd/httpd/trunk/changes-entries/md_ocsp_multi_update.txt
Modified:
    httpd/httpd/trunk/modules/md/md_curl.c
    httpd/httpd/trunk/modules/md/md_ocsp.c
    httpd/httpd/trunk/modules/md/md_reg.c
    httpd/httpd/trunk/modules/md/md_store_fs.c
    httpd/httpd/trunk/modules/md/md_version.h

Added: httpd/httpd/trunk/changes-entries/md_ocsp_multi_update.txt
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/changes-entries/md_ocsp_multi_update.txt?rev=1893359&view=auto
==============================================================================
--- httpd/httpd/trunk/changes-entries/md_ocsp_multi_update.txt (added)
+++ httpd/httpd/trunk/changes-entries/md_ocsp_multi_update.txt Wed Sep 15 13:22:27 2021
@@ -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.

Modified: httpd/httpd/trunk/modules/md/md_curl.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/md/md_curl.c?rev=1893359&r1=1893358&r2=1893359&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/md/md_curl.c (original)
+++ httpd/httpd/trunk/modules/md/md_curl.c Wed Sep 15 13:22:27 2021
@@ -491,7 +491,7 @@ static apr_status_t md_curl_multi_perfor
             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_perfor
         }
 
         /* 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_perfor
                 }
             }
         }
-        assert(running == requests->nelts);
     };
 
 leave:

Modified: httpd/httpd/trunk/modules/md/md_ocsp.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/md/md_ocsp.c?rev=1893359&r1=1893358&r2=1893359&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/md/md_ocsp.c (original)
+++ httpd/httpd/trunk/modules/md/md_ocsp.c Wed Sep 15 13:22:27 2021
@@ -339,7 +339,7 @@ apr_status_t md_ocsp_prime(md_ocsp_reg_t
     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
     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
      * 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
             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_re
             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:

Modified: httpd/httpd/trunk/modules/md/md_reg.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/md/md_reg.c?rev=1893359&r1=1893358&r2=1893359&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/md/md_reg.c (original)
+++ httpd/httpd/trunk/modules/md/md_reg.c Wed Sep 15 13:22:27 2021
@@ -549,7 +549,11 @@ static apr_status_t pubcert_load(void *b
         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 *);

Modified: httpd/httpd/trunk/modules/md/md_store_fs.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/md/md_store_fs.c?rev=1893359&r1=1893358&r2=1893359&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/md/md_store_fs.c (original)
+++ httpd/httpd/trunk/modules/md/md_store_fs.c Wed Sep 15 13:22:27 2021
@@ -508,19 +508,21 @@ static apr_status_t mk_group_dir(const c
 
     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;
 }
 

Modified: httpd/httpd/trunk/modules/md/md_version.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/md/md_version.h?rev=1893359&r1=1893358&r2=1893359&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/md/md_version.h (original)
+++ httpd/httpd/trunk/modules/md/md_version.h Wed Sep 15 13:22:27 2021
@@ -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"