Merge of /httpd/httpd/trunk:r1913837,1916861,1916907

 * mod_md:
   - Using OCSP stapling information to trigger certificate renewals. Proposed
     by @frasertweedale.
   - Added directive `MDCheckInterval` to control how often the server checks
     for detected revocations. Added proposals for configurations in the
     README.md chapter "Revocations".
   - OCSP stapling: accept OCSP responses without a `nextUpdate` entry which is
     allowed in RFC 6960. Treat those as having an update interval of 12 hours.
     Added by @frasertweedale.
   - Adapt OpenSSL usage to changes in their API. By Yann Ylavic.



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

changes-entries/md_2.4.26.txt

@@ -0,0 +1,10 @@
+ * mod_md:
+   - Using OCSP stapling information to trigger certificate renewals. Proposed
+     by @frasertweedale.
+   - Added directive `MDCheckInterval` to control how often the server checks
+     for detected revocations. Added proposals for configurations in the
+     README.md chapter "Revocations".
+   - OCSP stapling: accept OCSP responses without a `nextUpdate` entry which is
+     allowed in RFC 6960. Treat those as having an update interval of 12 hours.
+     Added by @frasertweedale.
+   - Adapt OpenSSL usage to changes in their API. By Yann Ylavic.

modules/md/md_crypt.c

@@ -57,21 +57,11 @@
 #include <process.h>
 #endif
 
-#if defined(LIBRESSL_VERSION_NUMBER)
-/* Missing from LibreSSL */
-#define MD_USE_OPENSSL_PRE_1_1_API (LIBRESSL_VERSION_NUMBER < 0x2070000f)
-#else /* defined(LIBRESSL_VERSION_NUMBER) */
-#define MD_USE_OPENSSL_PRE_1_1_API (OPENSSL_VERSION_NUMBER < 0x10100000L)
-#endif
-
-#if (defined(LIBRESSL_VERSION_NUMBER) && (LIBRESSL_VERSION_NUMBER < 0x3050000fL)) || (OPENSSL_VERSION_NUMBER < 0x10100000L) 
+#if !defined(OPENSSL_NO_CT) \
+    && OPENSSL_VERSION_NUMBER >= 0x10100000L \
+    && (!defined(LIBRESSL_VERSION_NUMBER) \
+        || LIBRESSL_VERSION_NUMBER >= 0x3050000fL)
 /* Missing from LibreSSL < 3.5.0 and only available since OpenSSL v1.1.x */
-#ifndef OPENSSL_NO_CT
-#define OPENSSL_NO_CT
-#endif
-#endif
-
-#ifndef OPENSSL_NO_CT
 #include <openssl/ct.h>
 #endif
 
@@ -955,12 +945,9 @@ apr_status_t md_pkey_gen(md_pkey_t **ppkey, apr_pool_t *p, md_pkey_spec_t *spec)
     }
 }
 
-#if MD_USE_OPENSSL_PRE_1_1_API || (defined(LIBRESSL_VERSION_NUMBER) && \
-                                   LIBRESSL_VERSION_NUMBER < 0x2070000f)
-
-#ifndef NID_tlsfeature
-#define NID_tlsfeature          1020
-#endif
+#if OPENSSL_VERSION_NUMBER < 0x10100000L \
+    || (defined(LIBRESSL_VERSION_NUMBER) \
+        && LIBRESSL_VERSION_NUMBER < 0x2070000f)
 
 static void RSA_get0_key(const RSA *r,
                          const BIGNUM **n, const BIGNUM **e, const BIGNUM **d)

modules/md/md_ocsp.c

@@ -32,13 +32,6 @@
 #include <openssl/pem.h>
 #include <openssl/x509v3.h>
 
-#if defined(LIBRESSL_VERSION_NUMBER)
-/* Missing from LibreSSL */
-#define MD_USE_OPENSSL_PRE_1_1_API (LIBRESSL_VERSION_NUMBER < 0x2070000f)
-#else /* defined(LIBRESSL_VERSION_NUMBER) */
-#define MD_USE_OPENSSL_PRE_1_1_API (OPENSSL_VERSION_NUMBER < 0x10100000L)
-#endif
-
 #include "md.h"
 #include "md_crypt.h"
 #include "md_event.h"
@@ -563,7 +556,9 @@ static const char *single_resp_summary(OCSP_SINGLERESP* resp, apr_pool_t *p)
     ASN1_GENERALIZEDTIME *bup = NULL, *bnextup = NULL;
     md_timeperiod_t valid;
 
-#if MD_USE_OPENSSL_PRE_1_1_API
+#if OPENSSL_VERSION_NUMBER < 0x10100000L \
+    || (defined(LIBRESSL_VERSION_NUMBER) \
+        && LIBRESSL_VERSION_NUMBER < 0x2070000f)
     certid = resp->certId;
 #else
     certid = OCSP_SINGLERESP_get0_id(resp);
@@ -683,12 +678,6 @@ static apr_status_t ostat_on_resp(const md_http_response_t *resp, void *baton)
         md_result_log(update->result, MD_LOG_DEBUG);
         goto cleanup;
     }
-    if (!bnextup) {
-        rv = APR_EINVAL;
-        md_result_set(update->result, rv, "OCSP basicresponse reports not valid dates");
-        md_result_log(update->result, MD_LOG_DEBUG);
-        goto cleanup;
-    }
 
     /* Coming here, we have a response for our certid and it is either GOOD
      * or REVOKED. Both cases we want to remember and use in stapling. */
@@ -703,7 +692,14 @@ static apr_status_t ostat_on_resp(const md_http_response_t *resp, void *baton)
     new_der.free_data = md_openssl_free;
     nstat = (bstatus == V_OCSP_CERTSTATUS_GOOD)? MD_OCSP_CERT_ST_GOOD : MD_OCSP_CERT_ST_REVOKED;
     valid.start = bup? md_asn1_generalized_time_get(bup) : apr_time_now();
-    valid.end = md_asn1_generalized_time_get(bnextup);
+    if (bnextup) {
+        valid.end = md_asn1_generalized_time_get(bnextup);
+    }
+    else {
+        /* nextUpdate not set; default to 12 hours.
+         * Refresh attempts will be started some time earlier. */
+        valid.end = valid.start + apr_time_from_sec(MD_SECS_PER_DAY / 2);
+    }
 
     /* First, update the instance with a copy */
     apr_thread_mutex_lock(ostat->reg->mutex);

modules/md/md_reg.c

@@ -31,6 +31,7 @@
 #include "md_json.h"
 #include "md_result.h"
 #include "md_reg.h"
+#include "md_ocsp.h"
 #include "md_store.h"
 #include "md_status.h"
 #include "md_tailscale.h"
@@ -1321,3 +1322,38 @@ md_job_t *md_reg_job_make(md_reg_t *reg, const char *mdomain, apr_pool_t *p)
 {
     return md_job_make(p, reg->store, MD_SG_STAGING, mdomain, reg->min_delay);
 }
+
+static int get_cert_count(const md_t *md)
+{
+    if (md->cert_files && md->cert_files->nelts) {
+        return md->cert_files->nelts;
+    }
+    return md_pkeys_spec_count(md->pks);
+}
+
+int md_reg_has_revoked_certs(md_reg_t *reg, struct md_ocsp_reg_t *ocsp,
+                             const md_t *md, apr_pool_t *p)
+{
+    const md_pubcert_t *pubcert;
+    const md_cert_t *cert;
+    md_timeperiod_t ocsp_valid;
+    md_ocsp_cert_stat_t cert_stat;
+    apr_status_t rv = APR_SUCCESS;
+    int i;
+
+    if (!md->stapling || !ocsp)
+        return 0;
+
+    for (i = 0; i < get_cert_count(md); ++i) {
+        if (APR_SUCCESS != md_reg_get_pubcert(&pubcert, reg, md, i, p))
+            continue;
+        cert = APR_ARRAY_IDX(pubcert->certs, 0, const md_cert_t*);
+        if(!cert)
+            continue;
+        rv = md_ocsp_get_meta(&cert_stat, &ocsp_valid, ocsp, cert, p, md);
+        if (APR_SUCCESS == rv && cert_stat == MD_OCSP_CERT_ST_REVOKED) {
+            return 1;
+        }
+    }
+    return 0;
+}

modules/md/md_reg.h

@@ -23,6 +23,7 @@ struct md_pkey_t;
 struct md_cert_t;
 struct md_result_t;
 struct md_pkey_spec_t;
+struct md_ocsp_reg_t;
 
 #include "md_store.h"
 
@@ -310,4 +311,10 @@ apr_status_t md_reg_lock_global(md_reg_t *reg, apr_pool_t *p);
  */
 void md_reg_unlock_global(md_reg_t *reg, apr_pool_t *p);
 
+/**
+ * @return != 0 iff `md` has any certificates known to be REVOKED.
+ */
+int md_reg_has_revoked_certs(md_reg_t *reg, struct md_ocsp_reg_t *ocsp,
+                             const md_t *md, apr_pool_t *p);
+
 #endif /* mod_md_md_reg_h */

modules/md/md_version.h

@@ -27,15 +27,15 @@
  * @macro
  * Version number of the md module as c string
  */
-#define MOD_MD_VERSION "2.4.25"
+#define MOD_MD_VERSION "2.4.26"
 
 /**
  * @macro
  * Numerical representation of the version number of the md module
  * 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 0x020419
+#define MOD_MD_VERSION_NUM 0x02041a
 
 #define MD_ACME_DEF_URL         "https://acme-v02.api.letsencrypt.org/directory"
 #define MD_TAILSCALE_DEF_URL    "file://localhost/var/run/tailscale/tailscaled.sock"

modules/md/mod_md_config.c

@@ -84,6 +84,7 @@ static md_mod_conf_t defmc = {
     "crt.sh",                  /* default cert checker site name */
     "https://crt.sh?q=",       /* default cert checker site url */
     NULL,                      /* CA cert file to use */
+    apr_time_from_sec(MD_SECS_PER_DAY/2), /* default time between cert checks */
     apr_time_from_sec(5),      /* minimum delay for retries */
     13,                        /* retry_failover after 14 errors, with 5s delay ~ half a day */
     0,                         /* store locks, disabled by default */
@@ -624,6 +625,24 @@ static const char *md_config_set_base_server(cmd_parms *cmd, void *dc, const cha
     return set_on_off(&config->mc->manage_base_server, value, cmd->pool);
 }
 
+static const char *md_config_set_check_interval(cmd_parms *cmd, void *dc, const char *value)
+{
+    md_srv_conf_t *config = md_config_get(cmd->server);
+    const char *err = md_conf_check_location(cmd, MD_LOC_NOT_MD);
+    apr_time_t interval;
+
+    (void)dc;
+    if (err) return err;
+    if (md_duration_parse(&interval, value, "s") != APR_SUCCESS) {
+        return "unrecognized duration format";
+    }
+    if (interval < apr_time_from_sec(1)) {
+        return "check interval cannot be less than one second";
+    }
+    config->mc->check_interval = interval;
+    return NULL;
+}
+
 static const char *md_config_set_min_delay(cmd_parms *cmd, void *dc, const char *value)
 {
     md_srv_conf_t *config = md_config_get(cmd->server);
@@ -1304,7 +1323,8 @@ const command_rec md_cmds[] = {
                   "Configure locking of store for updates."),
     AP_INIT_TAKE1("MDMatchNames", md_config_set_match_mode, NULL, RSRC_CONF,
                   "Determines how DNS names are matched to vhosts."),
-
+    AP_INIT_TAKE1("MDCheckInterval", md_config_set_check_interval, NULL, RSRC_CONF,
+                  "Time between certificate checks."),
     AP_INIT_TAKE1(NULL, NULL, NULL, RSRC_CONF, NULL)
 };
 

modules/md/mod_md_config.h

@@ -75,6 +75,7 @@ struct md_mod_conf_t {
     const char *cert_check_name;       /* name of the linked certificate check site */
     const char *cert_check_url;        /* url "template for" checking a certificate */
     const char *ca_certs;              /* root certificates to use for connections */
+    apr_time_t check_interval;         /* duration between cert renewal checks */
     apr_time_t min_delay;              /* minimum delay for retries */
     int retry_failover;                /* number of errors to trigger CA failover */
     int use_store_locks;               /* use locks when updating store */

modules/md/mod_md_drive.c

@@ -100,15 +100,20 @@ static void process_drive_job(md_renew_ctx_t *dctx, md_job_t *job, apr_pool_t *p
     }
 
     if (md_will_renew_cert(md)) {
-        /* Renew the MDs credentials in a STAGING area. Might be invoked repeatedly 
+        /* Renew the MDs credentials in a STAGING area. Might be invoked repeatedly
          * without discarding previous/intermediate results.
          * Only returns SUCCESS when the renewal is complete, e.g. STAGING has a
          * complete set of new credentials.
          */
         ap_log_error( APLOG_MARK, APLOG_DEBUG, 0, dctx->s, APLOGNO(10052) 
                      "md(%s): state=%d, driving", job->mdomain, md->state);
 
-        if (!md_reg_should_renew(dctx->mc->reg, md, dctx->p)) {
+        if (md->stapling && dctx->mc->ocsp &&
+            md_reg_has_revoked_certs(dctx->mc->reg, dctx->mc->ocsp, md, dctx->p)) {
+            ap_log_error( APLOG_MARK, APLOG_DEBUG, 0, dctx->s, APLOGNO(10500)
+                         "md(%s): has revoked certificates", job->mdomain);
+        }
+        else if (!md_reg_should_renew(dctx->mc->reg, md, dctx->p)) {
             ap_log_error( APLOG_MARK, APLOG_DEBUG, 0, dctx->s, APLOGNO(10053) 
                          "md(%s): no need to renew", job->mdomain);
             goto expiry;
@@ -180,10 +185,13 @@ int md_will_renew_cert(const md_t *md)
     return 1;
 }
 
-static apr_time_t next_run_default(void)
+static apr_time_t next_run_default(md_renew_ctx_t *dctx)
 {
-    /* we'd like to run at least twice a day by default */
-    return apr_time_now() + apr_time_from_sec(MD_SECS_PER_DAY / 2);
+    unsigned char c;
+    apr_time_t delay = dctx->mc->check_interval;
+
+    md_rand_bytes(&c, sizeof(c), dctx->p);
+    return apr_time_now() + delay + (delay * (c - 128) / 256);
 }
 
 static apr_status_t run_watchdog(int state, void *baton, apr_pool_t *ptemp)
@@ -211,7 +219,7 @@ static apr_status_t run_watchdog(int state, void *baton, apr_pool_t *ptemp)
              * and we schedule ourself at the earliest of all. A job may specify 0
              * as next_run to indicate that it wants to participate in the normal
              * regular runs. */
-            next_run = next_run_default();
+            next_run = next_run_default(dctx);
             for (i = 0; i < dctx->jobs->nelts; ++i) {
                 job = APR_ARRAY_IDX(dctx->jobs, i, md_job_t *);
 

test/modules/md/dns01_v2.py

@@ -0,0 +1,62 @@
+#!/usr/bin/env python3
+
+import subprocess
+import sys
+
+curl = "curl"
+challtestsrv = "localhost:8055"
+
+
+def run(args):
+    sys.stderr.write(f"run: {' '.join(args)}\n")
+    p = subprocess.Popen(args, stdin=None, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    output, errput = p.communicate(None)
+    rv = p.wait()
+    if rv != 0:
+        sys.stderr.write(errput.decode())
+    sys.stdout.write(output.decode())
+    return rv
+
+
+def teardown(domain):
+    rv = run([curl, '-s', '-d', f'{{"host":"_acme-challenge.{domain}"}}',
+              f'{challtestsrv}/clear-txt'])
+    if rv == 0:
+        rv = run([curl, '-s', '-d', f'{{"host":"{domain}"}}',
+                  f'{challtestsrv}/set-txt'])
+    return rv
+
+
+def setup(domain, challenge):
+    teardown(domain)
+    rv = run([curl, '-s', '-d', f'{{"host":"{domain}", "addresses":["127.0.0.1"]}}',
+              f'{challtestsrv}/set-txt'])
+    if rv == 0:
+        rv = run([curl, '-s', '-d', f'{{"host":"_acme-challenge.{domain}.", "value":"{challenge}"}}',
+                  f'{challtestsrv}/set-txt'])
+    return rv
+
+
+def main(argv):
+    if len(argv) > 1:
+        if argv[1] == 'setup':
+            if len(argv) != 4:
+                sys.stderr.write("wrong number of arguments: dns01.py setup <domain> <challenge>\n")
+                sys.exit(2)
+            rv = setup(argv[2], argv[3])
+        elif argv[1] == 'teardown':
+            if len(argv) != 4:
+                sys.stderr.write("wrong number of arguments: dns01.py teardown <domain> <challenge>\n")
+                sys.exit(1)
+            rv = teardown(argv[2])
+        else:
+            sys.stderr.write(f"unknown option {argv[1]}\n")
+            rv = 2
+    else:
+        sys.stderr.write("dns01.py wrong number of arguments\n")
+        rv = 2
+    sys.exit(rv)
+
+
+if __name__ == "__main__":
+    main(sys.argv)

test/modules/md/md_cert_util.py

@@ -166,10 +166,10 @@ def get_key_length(self):
 
     def get_san_list(self):
         text = OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_TEXT, self.cert).decode("utf-8")
-        m = re.search(r"X509v3 Subject Alternative Name:\s*(.*)", text)
+        m = re.search(r"X509v3 Subject Alternative Name:(\s+critical)?\s*(.*)", text)
         sans_list = []
         if m:
-            sans_list = m.group(1).split(",")
+            sans_list = m.group(2).split(",")
 
         def _strip_prefix(s):
             return s.split(":")[1] if s.strip().startswith("DNS:") else s.strip()

test/modules/md/md_env.py

@@ -73,7 +73,11 @@ def has_acme_server(cls):
 
     @classmethod
     def has_acme_eab(cls):
-        return cls.get_acme_server() == 'pebble'
+        return False
+        # Pebble, since v2.5.0 no longer supports HS256 for EAB, which
+        # is the only thing mod_md supports. Issue opened at pebble:
+        # https://github.com/letsencrypt/pebble/issues/455
+        # return cls.get_acme_server() == 'pebble'
 
     @classmethod
     def is_pebble(cls) -> bool:
@@ -356,13 +360,14 @@ def check_md_credentials(self, domain):
         MDCertUtil.validate_privkey(self.store_domain_file(domain, 'privkey.pem'))
         cert = MDCertUtil(self.store_domain_file(domain, 'pubcert.pem'))
         cert.validate_cert_matches_priv_key(self.store_domain_file(domain, 'privkey.pem'))
-        # check SANs and CN
-        assert cert.get_cn() == domain
+        # No longer check CN, it may not be set or is not trusted anyway
+        # assert cert.get_cn() == domain, f'CN: expected "{domain}", got {cert.get_cn()}'
+        # check SANs
         # compare lists twice in opposite directions: SAN may not respect ordering
         san_list = list(cert.get_san_list())
         assert len(san_list) == len(domains)
-        assert set(san_list).issubset(domains)
-        assert set(domains).issubset(san_list)
+        assert set(san_list).issubset(domains), f'{san_list} not subset of {domains}'
+        assert set(domains).issubset(san_list), f'{domains} not subset of {san_list}'
         # check valid dates interval
         not_before = cert.get_not_before()
         not_after = cert.get_not_after()