-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Error "cert created without giving its location header" when using a different CA #265
Comments
Hmm, just setup a domain and got a certificate successfully:
If you set |
I can do that but I have to wait a week because the module hit the 20 certs per week limit. |
urgs. But they seem to have a test API endoint: https://api.test4.buypass.no/acme/directory |
This problem exists on a production system. I can't just switch that over, or everyone is getting SSL errors. |
A minimal test setup, I used here:
that will trigger a renewal, even though that domain is not visible in any VirtualHosts. Hope this helps. |
I found a domain I could use to create yet another virtual host and sent the log to you. In the mean time, is there a way to edit the json files to convince mod_md that the certs are valid and it should leave the CA alone until renewal is up? |
In the logs you provided I see that you have If you configure |
Seems to work so far. |
There it is: https://community.buypass.com/t/60h8l7f/support-for-ocsp-must-staple This is a bug on their side. If a client submits a certificate request with This makes Apache go into an endless loop until it hits their rate limit. |
In that case, adding an appropriate error message like "MDMustStaple is not supported by this CA" would be an acceptable solution. |
I agree, if the code could find out that this is the case and not an error on the CA side. |
it could also just tell "generated certificate does not match expectations: ". |
Yes. What I did already is to tell the reason why a renewal is done. So, in the future, one will see that a new cert is requested because the MustStaple is missing. But you are right that a check on the retrieved cert with a proper error message would be good as well. |
I just got this error - but I don't have any setting for MustStaple.... I'll quickly try it again with that specifically turned off. EDIT: It also seems like BuyPass doesn't support |
I switched to the Test API URL listed above, and still get:
I did eventually get a cert after what looked to be 4 attempts from the testing API..... I went back to
The config I'm using:
|
They counted the previous attempts as successful. If you have just this one domain, you may try stopping your server, wipe the |
Yeah - I tried this by doing:
I think it keeps count of the domain name - cos I'm only testing them with the one system that one a single DNS name... |
I tried this again with buypass today for the same domain.... It created the cert several times and still failed with:
This was with the following config:
|
@icing I did encounter this recently and this seems to stem from outdated code in function Lines 307 to 312 in e5d131b
The https://datatracker.ietf.org/doc/html/rfc8555/#section-7.1
The Now that ACMEv1 support is no more https://datatracker.ietf.org/doc/html/rfc8555/#section-7.1.6
This part of the function also seems outdated if RFC8555 is to be trusted a server will not return the certificate as the response for the finalize order. Lines 321 to 335 in e5d131b
Also note the issue within Lines 161 to 165 in e5d131b
The case described above results in the client attempting to finalize the order again and again, even if it was already finalized because function In short Probably something like this ? (untested+missing error handling, meant as a demonstration of idea more than actual patch) diff --git a/src/md_acme_drive.c b/src/md_acme_drive.c
index 4bb04f3..fd3c0fe 100644
--- a/src/md_acme_drive.c
+++ b/src/md_acme_drive.c
@@ -295,46 +295,19 @@ static apr_status_t on_init_csr_req(md_acme_req_t *req, void *baton)
return md_acme_req_body_init(req, jpayload);
}
-static apr_status_t csr_req(md_acme_t *acme, const md_http_response_t *res, void *baton)
+static apr_status_t csr_req(md_acme_t *acme, apr_pool_t *p, const apr_table_t *hdrs,
+ md_json_t *body, void *baton)
{
md_proto_driver_t *d = baton;
md_acme_driver_t *ad = d->baton;
- const char *location;
- md_cert_t *cert;
- apr_status_t rv = APR_SUCCESS;
-
+
(void)acme;
- location = apr_table_get(res->headers, "location");
- if (!location) {
- md_log_perror(MD_LOG_MARK, MD_LOG_ERR, APR_EINVAL, d->p,
- "cert created without giving its location header");
- return APR_EINVAL;
- }
- ad->order->certificate = apr_pstrdup(d->p, location);
- if (APR_SUCCESS != (rv = md_acme_order_save(d->store, d->p, MD_SG_STAGING,
- d->md->name, ad->order, 0))) {
- md_log_perror(MD_LOG_MARK, MD_LOG_ERR, APR_EINVAL, d->p,
- "%s: saving cert url %s", d->md->name, location);
- return rv;
- }
-
- /* Check if it already was sent with this response */
- ad->chain_up_link = NULL;
- if (APR_SUCCESS == (rv = md_cert_read_http(&cert, d->p, res))) {
- md_log_perror(MD_LOG_MARK, MD_LOG_DEBUG, rv, d->p, "cert parsed");
- apr_array_clear(ad->cred->chain);
- APR_ARRAY_PUSH(ad->cred->chain, md_cert_t*) = cert;
- get_up_link(d, res->headers);
- }
- else if (APR_STATUS_IS_ENOENT(rv)) {
- rv = APR_SUCCESS;
- if (location) {
- md_log_perror(MD_LOG_MARK, MD_LOG_DEBUG, rv, d->p,
- "cert not in response, need to poll %s", location);
- }
- }
-
- return rv;
+ (void)hdrs;
+
+ /* @@@ Not sure about what pool to use */
+ order_update_from_json(ad->order, body, ad->order->p)
+
+ return APR_SUCCESS;
}
/**
@@ -382,7 +355,7 @@ apr_status_t md_acme_drive_setup_cred_chain(md_proto_driver_t *d, md_result_t *r
md_result_activity_printf(result, "Submitting %s CSR to CA", md_pkey_spec_name(spec));
assert(ad->order->finalize);
- rv = md_acme_POST(ad->acme, ad->order->finalize, on_init_csr_req, NULL, csr_req, NULL, d);
+ rv = md_acme_POST(ad->acme, ad->order->finalize, on_init_csr_req, csr_req, NULL, NULL, d);
leave:
md_acme_report_result(ad->acme, rv, result);
diff --git a/src/md_acmev2_drive.c b/src/md_acmev2_drive.c
index 9dfca96..1c08391 100644
--- a/src/md_acmev2_drive.c
+++ b/src/md_acmev2_drive.c
@@ -164,11 +164,13 @@ retry:
md_log_perror(MD_LOG_MARK, MD_LOG_DEBUG, 0, d->p, "%s: finalized order", d->md->name);
}
- rv = md_acme_order_await_valid(ad->order, ad->acme, d->md,
- ad->authz_monitor_timeout, result, d->p);
- if (APR_SUCCESS != rv) goto leave;
-
- if (!ad->order->certificate) {
+ if (MD_ACME_ORDER_ST_PROCESSING == ad->order->status) {
+ rv = md_acme_order_await_valid(ad->order, ad->acme, d->md,
+ ad->authz_monitor_timeout, result, d->p);
+ if (APR_SUCCESS != rv) goto leave;
+ }
+
+ if (MD_ACME_ORDER_ST_VALID == ad->order->status && !ad->order->certificate) {
md_result_set(result, APR_EINVAL, "Order valid, but certificate url is missing.");
goto leave;
} EDIT: the patch is likely missing a save of the updated order json to the store somewhere. |
header returned by the ACME CA. This was the way it was done in ACME before it became an IETF standard. Let's Encrypt still supports this, but other CAs do not. Refs #265.
Thanks @bitscher for pointing out the problem. I just released https://github.com/icing/mod_md/releases/tag/v2.4.27 where the Location header is still used if present, but no longer causes an issue when missing. Hope this works for you all. Let me know if you can verify it. |
Validated for my test case. |
Closing as fixed |
This message is logged for every ACME certificate:
However, the certificate is still issued and activated. The status page lists the status as "incomplete" and the "Activity" as "finished successfully". Every time the server is restarted it will try to obtain a certificate again.
Configuration for different CA:
The text was updated successfully, but these errors were encountered: