From: Stefan Eissing Date: Wed, 15 Sep 2021 13:41:35 +0000 (+0000) Subject: Merge of /httpd/httpd/trunk:r1893359 X-Git-Tag: candidate-2.4.50-rc1~50 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dedd251e93fce01b0f7dd9cf5c7d057f7af4e908;p=thirdparty%2Fapache%2Fhttpd.git 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 . 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 index 00000000000..e927c79927f --- /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 . + 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 59d83e52e88..e05f37b8997 100644 --- 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 95ecaa3a689..01e601f7659 100644 --- 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 7d61f0c5d45..b500ddc601a 100644 --- 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 038bcc50c42..959bafee80f 100644 --- 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 f76ed708c24..e52c2fce846 100644 --- 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"