synch with github

ChangeLog

@@ -4,6 +4,18 @@
  * Fixed a theoretical uninitialized read when testing for JSON error responses from the
    ACME CA. Bugreported at <https://bz.apache.org/bugzilla/show_bug.cgi?id=64297>.
    (Ported from maintenance-2.2.x branch)
+ * Adapted test suite to run against a current letsencrypt boulder version.
+
+v2.3.next
+----------------------------------------------------------------------------------------------------
+ * ACME problem reports from CAs that include parameters in the Content-Type header are handled correctly.
+   (Previously, the problem text would not be reported and retries could exist CA limits.)
+ * Account Update transactions to V2 CAs now use the correct POST-AS-GET method.  Previously, an
+   empty JSON object was sent - which apparently LE accepted, but others reject.
+ * If a CA directory includes both V1 and V2 endpoints, mod_md now will use the V2 endpoint.  Previously,
+   it would prefer V1 in this unusual configuration. V2 is standard; V1 is deprecated.
+ * Synchronized with Apache trunk changes, added test case for issue #218.
+>>>>>>> dbad16be376d67785ac1bbad256c349c90c3d5f6
 
 v2.3.3 (BETA)
 ----------------------------------------------------------------------------------------------------

event_interface_notes.txt

@@ -13,10 +13,98 @@ https://github.com/AnalogJ/lexicon/tree/master/lexicon/providers
 is another possible source of updaters for which a wrapper might
 be desirable.
 
+Rev 2.
 T. Litt
 
 Background/boundary conditions
 
+mod_md's terminology is somewhat confusing, specifically
+in terms of how it uses the word 'domain'.  In this document,
+we consider that mod_md interacts with a certificate
+authority (usually Let's Encrypt) in units of "certificates".
+A certificate has one or more "subjects".  Currently, Let's
+Encrypt only allows DNS host names (including wildcards) to
+be the subject(s) of a certificate.  But IP addresses are
+coming to LE, and I fully expect that e-mail addresses will
+be offered by some other CA, if not LE.
+
+In mod_md, a certificate is named by the MDomain directive.
+E.g. MDomain example.net
+If it has more than one subject, the secondary subjects (or
+in X.509 terms, "subject alternate names"  are listed after
+the certificate name.  E.g. MDomain example.net www.example.net.
+
+An alternate syntax is a configuration block, where the
+secondary subjects are specified within a <MDomain> block
+with MDmember directives.  E.g. <MDomain example.net>
+MDmember example.org
+MDmember www.example.net
+</MDomain>
+In this syntax, certain attributes of the certificate
+can be assigned on a per-certificate basis, including
+which Certificate Authority is to issue the certificate,
+agreement to terms of service, verification (challenge)
+method(s) to be used, and so forth.
+.
+In either case, for each grouping a single certificate
+is requested that will identify all the subjects listed.
+It is important to note that a single certificate can
+have MANY subjects - LE supports up to 100, but other
+CAs could support more.  There is no practical upper
+bound, though the size of a certificate with that
+many ibkects could become an issue.
+
+Mod_md documentation and internals use "domain" for both
+the certificate and the subject.  I'll try not to do that
+here.
+
+Multiple certificates can be requested for distinct
+groups of subjects.  (Technically, there could be
+overlap - a subject could be included in more than one
+certificate, but mod_md doesn't handle this gracefully.
+
+For the purposes of this document, an event is a state change
+that occurs to, or within a certificate and/or its subjects.
+
+For this document, Interesting events are those that interact
+with people or software other than mod_md.  They are listed
+below, but include things like certificate issuance,
+expiration, renewal, and revocation.
+
+There are several ways to categorize events.  One is
+to consider events that apply to the certificate as a
+whole (e.g. issuance) vs. those that apply to the subjects
+of a certificate (e.g. a validation challenge).  In
+either case, responding to an event notification will
+require identification of both the certificate name
+and the subject(s).
+
+Another categorization notes that while mod_md manges
+the issuance, renewal, and revocation of certificates,
+all of the certificates that it manges are not necessarily
+installed in the httpd instance that runs mod_md.  Some
+(or all) subjects may be for other services (e.g. imap,
+ldap,...) and some may be installed on other machines.  I
+refer to these as "remote" subjects.
+
+Remote subject have unique issues with respect to HTTP
+(and even DNS) validation, certificate installation, and
+server restart.
+
+A third categorization is what action or response is required
+to an event.  This varies - from the sample script previously
+distributed with mod_md tht e-mail and administrator, to 
+responses that enable the issuance/renewal and/or installation
+processes.  For example, pushing a validation token or
+issued certificate and key to a remote host, and/or
+restarting a remote HTTPD server.
+
+There are common aspects to all these actions:
+The certificate needs to be identified.  Configuration
+data needs to be maintained.  And logging is desirable.
+For events that require action by (or for) each subject,
+the subject(s) need to be identified.
+
 mod_md creates and interacts with several external programs,
 currently on an ad-hoc basis.  This is an attempt to unify
 these interfaces and expand them to cover more use cases.
@@ -31,6 +119,7 @@ it is a goal that this is possible.  This implies a common
 calling sequence, unique/disjoint event names, and standard return
 codes.
 
+
 The events in question are low frequency, low volume events.  Thus
 this interface serves one event per activation.  It does not define
 or support a persistent event server.
@@ -60,7 +149,19 @@ For add, called with 'setup' '<domain>' 'challenge-data'
 For remove, called with 'teardown' '<domain>'
 
 '<domain>' here is not necessarily the MDomain name.  It will be
-each of the MDMembers.
+each of a certificate's subjects - one event per subject per
+state change.  E.g. renewing a certificate may require 100
+DNS updates to install the validation tokens, and 100 more
+to remove them.  (The CA's validation agent may query
+the token multiple times from multiple viewpoints.)
+
+A DNS update to install a token may take considerable
+time to propagate through the DNS,  Thus, the
+delay time should be configurable and the delay should
+not block the service thread in HTTPD.
+
+DNS update is registrar-specific, and usually done via
+a custom script.
 
 Changes:
  - document that '<optional-args>' can follow executable,
@@ -80,7 +181,7 @@ MDNotifyCmd <path-to-executable>
 Called to notify of renewals.
 
 Called with '<domain>'
-Environment variables: MD_STORE, MD_VERSION
+Additional data required: MD_STORE, MD_VERSION, subjects.
 
 Changes:
  - document that '<optional-args>' can follow executable,
@@ -95,7 +196,7 @@ Intended for messages, but close to an event interface.
 Called with one of 'renewed', 'installed', 'expiring',
                    'errored', 'ocsp-renewed', 'ocsp-errored'
 
-Environment variables: MD_STORE, MD_VERSION
+Additional data required: MD_STORE, MD_VERSION
 
 'installed' is called with privileges.
 rate-limited: 'renewal' once; errors once/hour; expiring once/day.
@@ -171,7 +272,7 @@ Syntax:
 Program called with:
         <optional-args> 'event-name' 'mdomain-name' <intrinsic data>
 
-Environment variables: MD_STORE, MD_VERSION
+Additional data required: MD_STORE, MD_VERSION
 
 Events:
      'renewed', 'installed', 'expiring',
@@ -195,11 +296,11 @@ plus:  (not ideal names, but backward compatible)
         'teardown-http-01' 'mdomain' 'test-domain' 'token'
                         Removes a setup-http-01 resource.
 
-Program can use mdomain-name to get per-domain configuration.
+Program can use mdomain-name to get per certificate configuration.
 
 For compatibility with MDMessageCmd, it might be better to put mdomain-name
-into an environment variable (MD_MDOMAIN?) and leave the setup/teardown
-arguments the same as now.
+into a named variable (MD_MDOMAIN?) after the current arguments, and
+leave the setup/teardown arguments the same as now.
 
 For DNS, this isn't the same as the place the acme-challenge TXT record is stored.
 For HTTP, this isn't the same as the place where the HTTP resource is created.
@@ -284,3 +385,21 @@ so a coarse solution should suffice.  OTOH, it doesn't make sense for
 internal responses to wait 10s of minutes for getting the first certificate.
 
 Delays between job steps should not block httpd (or even mod_md's threads).
+
+Stefan's suggestion of named variables instead of environment variables
+is attractive, at least for data that isn't intrinsic to an event.  It
+would be nice to have the 
+
+See issues:
+  #198, where the envar issues were most recently discussed.
+  #189, where we agree that "a more general event interface"
+ is the apparent direction;
+  #200, that drove a mod_ssl change that needs to go upstream
+  #187, excluding status pages from MdRequireHttps
+  #194, where I also mentioned why a certificate ("mdomain" name)
+        differs from the subject name/test domain probed by the
+        CA's validator for DNS01.  Note that one of my commits
+        in yesterday's merge brings the certificate name to what
+        was to be an envvar, now a trailing named parameter.  It
+        should not be lost...
+

src/md_acme.c

@@ -121,6 +121,7 @@ static md_acme_req_t *md_acme_req_create(md_acme_t *acme, const char *method, co
     if (rv != APR_SUCCESS) {
         return NULL;
     }
+    apr_pool_tag(pool, "md_acme_req");
 
     req = apr_pcalloc(pool, sizeof(*req));
     if (!req) {
@@ -162,13 +163,15 @@ apr_status_t md_acme_init(apr_pool_t *p, const char *base,  int init_ssl)
 static apr_status_t inspect_problem(md_acme_req_t *req, const md_http_response_t *res)
 {
     const char *ctype;
-    md_json_t *problem;
-
+    md_json_t *problem = NULL;
+    apr_status_t rv;
+
     ctype = apr_table_get(req->resp_hdrs, "content-type");
+    ctype = md_util_parse_ct(res->req->pool, ctype);
     if (ctype && !strcmp(ctype, "application/problem+json")) {
         /* RFC 7807 */
-        md_json_read_http(&problem, req->p, res);
-        if (problem) {
+        rv = md_json_read_http(&problem, req->p, res);
+        if (rv == APR_SUCCESS && problem) {
             const char *ptype, *pdetail;
 
             req->resp_json = problem;
@@ -572,7 +575,7 @@ apr_status_t md_acme_use_acct(md_acme_t *acme, md_store_t *store,
             rv = md_acme_acct_validate(acme, store, p);
         }
         else {
-            /* account is from a nother server or, more likely, from another
+            /* account is from another server or, more likely, from another
              * protocol endpoint on the same server */
             rv = APR_ENOENT;
         }
@@ -705,28 +708,21 @@ static apr_status_t update_directory(const md_http_response_t *res, void *data)
     }
 
     /* What have we got? */
-    if ((s = md_json_dups(acme->p, json, "new-authz", NULL))) {
-        acme->api.v1.new_authz = s;
-        acme->api.v1.new_cert = md_json_dups(acme->p, json, "new-cert", NULL);
-        acme->api.v1.new_reg = md_json_dups(acme->p, json, "new-reg", NULL);
-        acme->api.v1.revoke_cert = md_json_dups(acme->p, json, "revoke-cert", NULL);
-        if (acme->api.v1.new_authz && acme->api.v1.new_cert 
-            && acme->api.v1.new_reg && acme->api.v1.revoke_cert) {
-            acme->version = MD_ACME_VERSION_1;
-        }
-        acme->ca_agreement = md_json_dups(acme->p, json, "meta", "terms-of-service", NULL);
-        acme->new_nonce_fn = acmev1_new_nonce;
-        acme->req_init_fn = acmev1_req_init;
-        acme->post_new_account_fn = acmev1_POST_new_account;
-    }
-    else if ((s = md_json_dups(acme->p, json, "newAccount", NULL))) {
+    if ((s = md_json_dups(acme->p, json, "newAccount", NULL))) {
         acme->api.v2.new_account = s;
         acme->api.v2.new_order = md_json_dups(acme->p, json, "newOrder", NULL);
         acme->api.v2.revoke_cert = md_json_dups(acme->p, json, "revokeCert", NULL);
         acme->api.v2.key_change = md_json_dups(acme->p, json, "keyChange", NULL);
         acme->api.v2.new_nonce = md_json_dups(acme->p, json, "newNonce", NULL);
-        if (acme->api.v2.new_account && acme->api.v2.new_order 
-            && acme->api.v2.revoke_cert && acme->api.v2.key_change
+        /* RFC 8555 only requires "directory" and "newNonce" resources.
+         * mod_md uses "newAccount" and "newOrder" so check for them.
+         * But mod_md does not use the "revokeCert" or "keyChange"
+         * resources, so tolerate the absense of those keys.  In the
+         * future if mod_md implements revocation or key rollover then
+         * the use of those features should be predicated on the
+         * server's advertised capabilities. */
+        if (acme->api.v2.new_account
+            && acme->api.v2.new_order
             && acme->api.v2.new_nonce) {
             acme->version = MD_ACME_VERSION_2;
         }
@@ -735,6 +731,20 @@ static apr_status_t update_directory(const md_http_response_t *res, void *data)
         acme->req_init_fn = acmev2_req_init;
         acme->post_new_account_fn = acmev2_POST_new_account;
     }
+    else if ((s = md_json_dups(acme->p, json, "new-authz", NULL))) {
+        acme->api.v1.new_authz = s;
+        acme->api.v1.new_cert = md_json_dups(acme->p, json, "new-cert", NULL);
+        acme->api.v1.new_reg = md_json_dups(acme->p, json, "new-reg", NULL);
+        acme->api.v1.revoke_cert = md_json_dups(acme->p, json, "revoke-cert", NULL);
+        if (acme->api.v1.new_authz && acme->api.v1.new_cert
+            && acme->api.v1.new_reg && acme->api.v1.revoke_cert) {
+            acme->version = MD_ACME_VERSION_1;
+        }
+        acme->ca_agreement = md_json_dups(acme->p, json, "meta", "terms-of-service", NULL);
+        acme->new_nonce_fn = acmev1_new_nonce;
+        acme->req_init_fn = acmev1_req_init;
+        acme->post_new_account_fn = acmev1_POST_new_account;
+    }
 
     if (MD_ACME_VERSION_UNKNOWN == acme->version) {
         md_result_printf(result, APR_EINVAL,

src/md_acme_acct.c

@@ -406,12 +406,12 @@ typedef struct {
 
 static apr_status_t on_init_acct_upd(md_acme_req_t *req, void *baton)
 {
-    md_json_t *jpayload;
+    md_json_t *jpayload = NULL;
 
     (void)baton;
-    jpayload = md_json_create(req->p);
     switch (MD_ACME_VERSION_MAJOR(req->acme->version)) {
         case 1:
+            jpayload = md_json_create(req->p);
             md_json_sets("reg", jpayload, MD_KEY_RESOURCE, NULL);
             break;
         default:

src/md_acme_authz.c

@@ -250,11 +250,11 @@ static apr_status_t on_init_authz_resp(md_acme_req_t *req, void *baton)
     jpayload = md_json_create(req->p);
     if (MD_ACME_VERSION_MAJOR(req->acme->version) <= 1) {
         md_json_sets(MD_KEY_CHALLENGE, jpayload, MD_KEY_RESOURCE, NULL);
+        if (ctx->challenge->key_authz) {
+            md_json_sets(ctx->challenge->key_authz, jpayload, MD_KEY_KEYAUTHZ, NULL);
+        }
     }
-    if (ctx->challenge->key_authz) {
-        md_json_sets(ctx->challenge->key_authz, jpayload, MD_KEY_KEYAUTHZ, NULL);
-    }
-
+
     return md_acme_req_body_init(req, jpayload);
 } 
 

src/md_acme_drive.c

@@ -201,6 +201,7 @@ static apr_status_t add_http_certs(apr_array_header_t *chain, apr_pool_t *p,
     const char *ct;
 
     ct = apr_table_get(res->headers, "Content-Type");
+    ct = md_util_parse_ct(res->req->pool, ct);
     if (ct && !strcmp("application/x-pkcs7-mime", ct)) {
         /* this looks like a root cert and we do not want those in our chain */
         goto out; 
@@ -397,6 +398,7 @@ static apr_status_t on_add_chain(md_acme_t *acme, const md_http_response_t *res,
 
     (void)acme;
     ct = apr_table_get(res->headers, "Content-Type");
+    ct = md_util_parse_ct(res->req->pool, ct);
     if (ct && !strcmp("application/x-pkcs7-mime", ct)) {
         /* root cert most likely, end it here */
         return APR_SUCCESS;
@@ -588,7 +590,7 @@ static apr_status_t acme_driver_init(md_proto_driver_t *d, md_result_t *result)
                              dis_http? " The http: challenge 'http-01' is disabled because the server seems not reachable on public port 80." : "",
                              dis_https? " The https: challenge 'tls-alpn-01' is disabled because the server seems not reachable on public port 443." : "",
                              dis_alpn_acme? " The https: challenge 'tls-alpn-01' is disabled because the Protocols configuration does not include the 'acme-tls/1' protocol." : "",
-                             dis_dns? "The DNS challenge 'dns-01' is disabled because the directive 'MDChallengeDns01' is not configured." : ""
+                             dis_dns? " The DNS challenge 'dns-01' is disabled because the directive 'MDChallengeDns01' is not configured." : ""
                              );
             goto leave;
         }

src/md_cmd_main.c

@@ -41,7 +41,6 @@
 #include "md_cmd_store.h"
 #include "md_curl.h"
 
-
 /**************************************************************************************************/
 /* command infrastructure */
 

src/md_crypt.c

@@ -1380,6 +1380,7 @@ apr_status_t md_cert_read_http(md_cert_t **pcert, apr_pool_t *p,
     apr_status_t rv;
 
     ct = apr_table_get(res->headers, "Content-Type");
+    ct = md_util_parse_ct(res->req->pool, ct);
     if (!res->body || !ct || strcmp("application/pkix-cert", ct)) {
         rv = APR_ENOENT;
         goto out;
@@ -1431,7 +1432,8 @@ apr_status_t md_cert_chain_read_http(struct apr_array_header_t *chain,
         rv = APR_ENOENT;
         goto out;
     }
-    else if (!strcmp("application/pem-certificate-chain", ct)) {
+    ct = md_util_parse_ct(res->req->pool, ct);
+    if (!strcmp("application/pem-certificate-chain", ct)) {
         if (APR_SUCCESS == (rv = apr_brigade_pflatten(res->body, &data, &data_len, res->req->pool))) {
             int added = 0;
             md_cert_t *cert;

src/md_http.c

@@ -169,6 +169,7 @@ static apr_status_t req_create(md_http_request_t **preq, md_http_t *http,
     if (rv != APR_SUCCESS) {
         return rv;
     }
+    apr_pool_tag(pool, "md_http_req");
 
     req = apr_pcalloc(pool, sizeof(*req));
     req->pool = pool;