V2.3.1 (BETA)

---------------------------------------------------------------------------------------------------- * The keyname in key and certificate files is now forced lower case. * Formatting in 'server-status' page has been improved when showing >1 certificates * Fallback certificates (when the domain has none yet) are now generated for all key types requested in MDPrivateKeys of that domain. * Update /.httpd/certificate-status to correctly handle multiple keys.
icing · Mar 7, 2020 · 78388e1 · 78388e1
1 parent 406c13c
commit 78388e1
Show file tree

Hide file tree

Showing 11 changed files with 46 additions and 70 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,9 +1,10 @@
-V2.3.next
+V2.3.1 (BETA)
 ----------------------------------------------------------------------------------------------------
- * Fix lowercasing of filenames
- * Improved format of Managed Certificate Status
- * Provide fallback certificates for all key types requested in MDPrivateKeys
- * Update /.httpd/certificate-status to correctly handle multiple keys
+ * The keyname in key and certificate files is now forced lower case.
+ * Formatting in 'server-status' page has been improved when showing >1 certificates
+ * Fallback certificates (when the domain has none yet) are now generated for all 
+   key types requested in MDPrivateKeys of that domain.
+ * Update /.httpd/certificate-status to correctly handle multiple keys.
 
 v2.3.0 (BETA)
 ----------------------------------------------------------------------------------------------------

diff --git a/configure.ac b/configure.ac
@@ -14,7 +14,7 @@
 #
 
 AC_PREREQ([2.69])
-AC_INIT([mod_md], [2.3.0], [[email protected]])
+AC_INIT([mod_md], [2.3.1], [[email protected]])
 
 LT_PREREQ([2.2.6])
 LT_INIT()

diff --git a/src/md_store.c b/src/md_store.c
@@ -264,7 +264,7 @@ static const char *pk_filename(const char *keyname, const char *base, apr_pool_t
         apr_pstrcat(p, base, ".", keyname, ".pem", NULL)
         : apr_pstrcat(p, base, ".pem", NULL);
     for (t = s; *t; t++ )
-        *t = apr_tolower(*t);
+        *t = (char)apr_tolower(*t);
     return s;
 }
 

diff --git a/src/md_version.h b/src/md_version.h
@@ -27,15 +27,15 @@
  * @macro
  * Version number of the md module as c string
  */
-#define MOD_MD_VERSION "2.3.0-git"
+#define MOD_MD_VERSION "2.3.1-git"
 
 /**
  * @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 0x020300
+#define MOD_MD_VERSION_NUM 0x020301
 
 #define MD_ACME_DEF_URL    "https://acme-v02.api.letsencrypt.org/directory"
 

diff --git a/src/mod_md.c b/src/mod_md.c
@@ -1135,53 +1135,27 @@ static apr_status_t get_certificates(server_rec *s, apr_pool_t *p, int fallback,
             /* Provide temporary, self-signed certificate as fallback, so that
              * clients do not get obscure TLS handshake errors or will see a fallback
              * virtual host that is not intended to be served here. */
-            int n;
             char *kfn, *cfn;
 
             store = md_reg_store_get(reg);
             assert(store);
 
-            if( (n = md_pkeys_spec_count(md->pks)) <= 0 ) {
-                md_pkey_spec_t dspec;
-
-                dspec.type = MD_PKEY_TYPE_RSA;
-                dspec.params.rsa.bits = MD_PKEY_RSA_BITS_DEF;
-                fallback_fnames(p, &dspec, &kfn, &cfn);
+            for (i = 0; i < md_pkeys_spec_count(md->pks); ++i) {
+                spec = md_pkeys_spec_get(md->pks, i);
+                fallback_fnames(p, spec, &kfn, &cfn);
 
                 md_store_get_fname(&keyfile, store, MD_SG_DOMAINS, md->name, kfn, p);
                 md_store_get_fname(&chainfile, store, MD_SG_DOMAINS, md->name, cfn, p);
                 if (!md_file_exists(keyfile, p) || !md_file_exists(chainfile, p)) {
-                    if (APR_SUCCESS != (rv = make_fallback_cert(store, md, &dspec, s, p, kfn, cfn))) {
+                    if (APR_SUCCESS != (rv = make_fallback_cert(store, md, spec, s, p, kfn, cfn))) {
                         return rv;
                     }
                 }
                 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(10116)
-                         "%s: providing %s fallback certificate for server %s",
-                             md->name, md_pkey_spec_name(&dspec), s->server_hostname);
+                             "%s: providing %s fallback certificate for server %s",
+                             md->name, md_pkey_spec_name(spec), s->server_hostname);
                 APR_ARRAY_PUSH(key_files, const char*) = keyfile;
                 APR_ARRAY_PUSH(chain_files, const char*) = chainfile;
-            } else {
-                int i;
-
-                for (i = 0; i < n; ++i) {
-                    md_pkey_spec_t *spec;
-
-                    spec = md_pkeys_spec_get(md->pks, i);
-                    fallback_fnames(p, spec, &kfn, &cfn);
-
-                    md_store_get_fname(&keyfile, store, MD_SG_DOMAINS, md->name, kfn, p);
-                    md_store_get_fname(&chainfile, store, MD_SG_DOMAINS, md->name, cfn, p);
-                    if (!md_file_exists(keyfile, p) || !md_file_exists(chainfile, p)) {
-                        if (APR_SUCCESS != (rv = make_fallback_cert(store, md, spec, s, p, kfn, cfn))) {
-                            return rv;
-                        }
-                    }
-                    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(10116)
-                                 "%s: providing %s fallback certificate for server %s",
-                                 md->name, md_pkey_spec_name(spec), s->server_hostname);
-                    APR_ARRAY_PUSH(key_files, const char*) = keyfile;
-                    APR_ARRAY_PUSH(chain_files, const char*) = chainfile;
-                }
             }
             rv = APR_EAGAIN;
             goto leave;

diff --git a/src/mod_md_status.c b/src/mod_md_status.c
@@ -59,7 +59,7 @@
 int md_http_cert_status(request_rec *r)
 {
     int i;
-    md_json_t *resp, *mdj;
+    md_json_t *resp, *mdj, *cj;
     const md_srv_conf_t *sc;
     const md_t *md;
     md_pkey_spec_t *spec;
@@ -100,9 +100,11 @@ int md_http_cert_status(request_rec *r)
 
     resp = md_json_create(r->pool);
 
-    for (i = 0; i < md_pkeys_spec_count(md->pks); ++i) {
-        md_json_t *cj;
+    if (md_json_has_key(mdj, MD_KEY_CERT, MD_KEY_VALID, NULL)) {
+         md_json_setj(md_json_getj(mdj, MD_KEY_CERT, MD_KEY_VALID, NULL), resp, MD_KEY_VALID, NULL);
+     }
 
+    for (i = 0; i < md_pkeys_spec_count(md->pks); ++i) {
         spec = md_pkeys_spec_get(md->pks, i);
         keyname = md_pkey_spec_name(spec);
         cj = md_json_create(r->pool);
@@ -120,19 +122,18 @@ int md_http_cert_status(request_rec *r)
             md_json_sets(md_json_gets(mdj, MD_KEY_CERT, keyname, MD_KEY_SHA256_FINGERPRINT, NULL),
                          cj, MD_KEY_SHA256_FINGERPRINT, NULL);
         }
-
-        if (md_json_has_key(mdj, MD_KEY_CERT, keyname, MD_KEY_RENEWAL, NULL)) {
-            md_json_t *certj, *j;
-            /* copy over the information we want to make public about this:
-             *  - when not finished, add an empty object to indicate something is going on
-             *  - when a certificate is staged, add the information from that */
-            certj = md_json_getj(mdj, MD_KEY_CERT, keyname, MD_KEY_RENEWAL, NULL);
-            j = certj? certj : md_json_create(r->pool);
-            md_json_setj(j, cj, MD_KEY_RENEWAL, NULL);
-        }
         md_json_setj(cj, resp, keyname, NULL );
     }
-
+
+    if (md_json_has_key(mdj, MD_KEY_RENEWAL, NULL)) {
+           /* copy over the information we want to make public about this:
+            *  - when not finished, add an empty object to indicate something is going on
+            *  - when a certificate is staged, add the information from that */
+           cj = md_json_getj(mdj, MD_KEY_RENEWAL, MD_KEY_CERT, NULL);
+           cj = cj? cj : md_json_create(r->pool);; 
+           md_json_setj(cj, resp, MD_KEY_RENEWAL, MD_KEY_CERT, NULL);
+     }
+
     ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r, "md[%s]: sending status", md->name);
     apr_table_set(r->headers_out, "Content-Type", "application/json"); 
     bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);

diff --git a/test/TestEnv.py b/test/TestEnv.py
@@ -347,7 +347,7 @@ def store_staged_file( cls, domain, filename ) :
 
     @classmethod
     def path_fallback_cert( cls, domain ) :
-        return os.path.join(TestEnv.STORE_DIR, 'domains', domain, 'fallback-cert.pem')
+        return os.path.join(TestEnv.STORE_DIR, 'domains', domain, 'fallback-pubcert.pem')
 
     @classmethod
     def path_job( cls, domain ) :

diff --git a/test/test_0700_auto.py b/test/test_0700_auto.py
@@ -292,20 +292,20 @@ def test_700_009(self):
         cert1 = CertUtil( TestEnv.store_domain_file(domain, 'pubcert.pem') )
         # compare with what md reports as status
         stat = TestEnv.get_certificate_status(domain);
-        assert stat['serial'] == cert1.get_serial()
+        assert stat['rsa']['serial'] == cert1.get_serial()
         #
         # create self-signed cert, with critical remaining valid duration -> drive again
         TestEnv.create_self_signed_cert( [domain], { "notBefore": -120, "notAfter": 2  }, serial=7009)
         cert3 = CertUtil( TestEnv.store_domain_file(domain, 'pubcert.pem') )
         assert cert3.get_serial() == '1B61'
         assert TestEnv.apache_restart() == 0
         stat = TestEnv.get_certificate_status(domain);
-        assert stat['serial'] == cert3.get_serial()
+        assert stat['rsa']['serial'] == cert3.get_serial()
         #
         # cert should renew and be different afterwards
         assert TestEnv.await_completion( [ domain ], must_renew=True )
         stat = TestEnv.get_certificate_status(domain);
-        assert stat['serial'] != cert3.get_serial()
+        assert stat['rsa']['serial'] != cert3.get_serial()
 
     # test case: drive with an unsupported challenge due to port availability 
     def test_700_010(self):
@@ -412,7 +412,7 @@ def test_700_030(self):
         assert TestEnv.apache_restart() == 0
         TestEnv.check_md( new_list )
         status = TestEnv.get_certificate_status( nameA )
-        assert status['serial'] == certA.get_serial() 
+        assert status['rsa']['serial'] == certA.get_serial() 
 
     # test case: Same as 7030, but remove *and* add another at the same time.
     # restart. should find and keep the existing MD and renew for additional name.
@@ -458,7 +458,7 @@ def test_700_031(self):
         assert TestEnv.apache_restart() == 0
         TestEnv.check_md( new_list )
         status = TestEnv.get_certificate_status( nameA )
-        assert status['serial'] == certA.get_serial() 
+        assert status['rsa']['serial'] == certA.get_serial() 
 
     # test case: create two MDs, move them into one
     # see: <https://bz.apache.org/bugzilla/show_bug.cgi?id=62572>

diff --git a/test/test_0702_auto.py b/test/test_0702_auto.py
@@ -302,20 +302,20 @@ def test_702_009(self):
         cert1 = CertUtil( TestEnv.store_domain_file(domain, 'pubcert.pem') )
         # compare with what md reports as status
         stat = TestEnv.get_certificate_status(domain);
-        assert stat['serial'] == cert1.get_serial()
+        assert stat['rsa']['serial'] == cert1.get_serial()
         #
         # create self-signed cert, with critical remaining valid duration -> drive again
         TestEnv.create_self_signed_cert( [domain], { "notBefore": -120, "notAfter": 2  }, serial=7029)
         cert3 = CertUtil( TestEnv.store_domain_file(domain, 'pubcert.pem') )
         assert cert3.get_serial() == '1B75'
         assert TestEnv.apache_restart() == 0
         stat = TestEnv.get_certificate_status(domain);
-        assert stat['serial'] == cert3.get_serial()
+        assert stat['rsa']['serial'] == cert3.get_serial()
         #
         # cert should renew and be different afterwards
         assert TestEnv.await_completion( [ domain ], must_renew=True)
         stat = TestEnv.get_certificate_status(domain);
-        assert stat['serial'] != cert3.get_serial()
+        assert stat['rsa']['serial'] != cert3.get_serial()
 
     # test case: drive with an unsupported challenge due to port availability 
     def test_702_010(self):
@@ -423,7 +423,7 @@ def test_702_030(self):
         assert TestEnv.apache_restart() == 0
         TestEnv.check_md( new_list )
         status = TestEnv.get_certificate_status( nameA )
-        assert status['serial'] == certA.get_serial() 
+        assert status['rsa']['serial'] == certA.get_serial() 
 
     # test case: Same as 7030, but remove *and* add another at the same time.
     # restart. should find and keep the existing MD and renew for additional name.

diff --git a/test/test_0710_migration.py b/test/test_0710_migration.py
@@ -75,7 +75,7 @@ def test_710_001(self):
         # restart, gets cert, should still be the same cert as it remains valid
         assert TestEnv.apache_restart() == 0
         status = TestEnv.get_certificate_status( domain )
-        assert status['serial'] == cert1.get_serial() 
+        assert status['rsa']['serial'] == cert1.get_serial() 
 
         # change the MD so that we need a new cert
         domains = [ domain, "www." + domain, "another."  + domain ]
@@ -88,7 +88,7 @@ def test_710_001(self):
         assert TestEnv.await_completion([ domain ] )
         # should no longer the same cert
         status = TestEnv.get_certificate_status( domain )
-        assert status['serial'] != cert1.get_serial() 
+        assert status['rsa']['serial'] != cert1.get_serial() 
         TestEnv.check_md_complete(domain)
         # should have a 2 accounts now
         assert 2 == len(TestEnv.list_accounts())

diff --git a/test/test_0920_status.py b/test/test_0920_status.py
@@ -63,9 +63,9 @@ def test_920_001(self):
         assert TestEnv.apache_restart() == 0
         status = TestEnv.get_certificate_status( domain )
         assert not 'renewal' in status
-        assert 'sha256-fingerprint' in status
-        assert 'valid' in status
-        assert 'from' in status['valid']
+        assert 'sha256-fingerprint' in status['rsa']
+        assert 'valid' in status['rsa']
+        assert 'from' in status['rsa']['valid']
 
     # simple MD, drive it, manipulate staged credentials and check status
     def test_920_002(self):