* various fixes in MDRenewWindow handling when specifying percent. S…

…erialization changed. If someone already used percent configurations, it is advised to change these to a new value, reload and change back to the wanted ones. * various fixes in handling of MDPrivateKeys when specifying 2048 bits (the default) explicitly. * mod_md version removed from top level md_store.json file. The store has its own format version to facilitate upgrades.
icing · Sep 5, 2017 · 4fc0fcf · 4fc0fcf
1 parent 9188ec5
commit 4fc0fcf
Show file tree

Hide file tree

Showing 15 changed files with 81 additions and 23 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,12 @@
+v0.9.1
+----------------------------------------------------------------------------------------------------
+ * various fixes in MDRenewWindow handling when specifying percent. Serialization changed. If 
+   someone already used percent configurations, it is advised to change these to a new value,
+   reload and change back to the wanted ones.
+ * various fixes in handling of MDPrivateKeys when specifying 2048 bits (the default) explicitly.
+ * mod_md version removed from top level md_store.json file. The store has its own format version
+   to facilitate upgrades.
+
 v0.9.0
 ----------------------------------------------------------------------------------------------------
  * Improved interface to mod_ssl for fallback handling. Backward compatible to previous mod_ssl

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

diff --git a/src/md.h b/src/md.h
@@ -28,7 +28,9 @@ struct md_srv_conf_t;
 struct md_pkey_spec_t;
 
 #define MD_TLSSNI01_DNS_SUFFIX     ".acme.invalid"
-#define MD_PKEY_RSA_BITS_DEF       2048U
+
+#define MD_PKEY_RSA_BITS_MIN       2048
+#define MD_PKEY_RSA_BITS_DEF       2048
 
 typedef enum {
     MD_S_UNKNOWN,                   /* MD has not been analysed yet */

diff --git a/src/md_crypt.c b/src/md_crypt.c
@@ -191,7 +191,7 @@ md_json_t *md_pkey_spec_to_json(const md_pkey_spec_t *spec, apr_pool_t *p)
                 break;
             case MD_PKEY_TYPE_RSA:
                 md_json_sets("RSA", json, MD_KEY_TYPE, NULL);
-                if (spec->params.rsa.bits > 2048) {
+                if (spec->params.rsa.bits >= MD_PKEY_RSA_BITS_MIN) {
                     md_json_setl(spec->params.rsa.bits, json, MD_KEY_BITS, NULL);
                 }
                 break;
@@ -217,14 +217,36 @@ md_pkey_spec_t *md_pkey_spec_from_json(struct md_json_t *json, apr_pool_t *p)
         else if (!apr_strnatcasecmp("RSA", s)) {
             spec->type = MD_PKEY_TYPE_RSA;
             l = md_json_getl(json, MD_KEY_BITS, NULL);
-            if (l > 2048) {
+            if (l >= MD_PKEY_RSA_BITS_MIN) {
                 spec->params.rsa.bits = (unsigned int)l;
             }
+            else {
+                spec->params.rsa.bits = MD_PKEY_RSA_BITS_DEF;
+            }
         }
     }
     return spec;
 }
 
+int md_pkey_spec_eq(md_pkey_spec_t *spec1, md_pkey_spec_t *spec2)
+{
+    if (spec1 == spec2) {
+        return 1;
+    }
+    if (spec1 && spec2 && spec1->type == spec2->type) {
+        switch (spec1->type) {
+            case MD_PKEY_TYPE_DEFAULT:
+                return 1;
+            case MD_PKEY_TYPE_RSA:
+                if (spec1->params.rsa.bits == spec2->params.rsa.bits) {
+                    return 1;
+                }
+                break;
+        }
+    }
+    return 0;
+}
+
 static md_pkey_t *make_pkey(apr_pool_t *p) 
 {
     md_pkey_t *pkey = apr_pcalloc(p, sizeof(*pkey));
@@ -363,7 +385,7 @@ static apr_status_t gen_rsa(md_pkey_t **ppkey, apr_pool_t *p, unsigned int bits)
         rv = APR_SUCCESS;
     }
     else {
-        md_log_perror(MD_LOG_MARK, MD_LOG_WARNING, 0, p, "unable to generate new key"); 
+        md_log_perror(MD_LOG_MARK, MD_LOG_WARNING, 0, p, "error generate pkey RSA %d", bits); 
         *ppkey = NULL;
         rv = APR_EGENERAL;
     }

diff --git a/src/md_crypt.h b/src/md_crypt.h
@@ -80,6 +80,7 @@ void *md_pkey_get_EVP_PKEY(struct md_pkey_t *pkey);
 
 struct md_json_t *md_pkey_spec_to_json(const md_pkey_spec_t *spec, apr_pool_t *p);
 md_pkey_spec_t *md_pkey_spec_from_json(struct md_json_t *json, apr_pool_t *p);
+int md_pkey_spec_eq(md_pkey_spec_t *spec1, md_pkey_spec_t *spec2);
 
 /**************************************************************************************************/
 /* X509 certificates */

diff --git a/src/md_reg.c b/src/md_reg.c
@@ -497,6 +497,13 @@ static apr_status_t p_md_update(void *baton, apr_pool_t *p, apr_pool_t *ptemp, v
         nmd->ca_challenges = (updates->ca_challenges? 
                               apr_array_copy(p, updates->ca_challenges) : NULL);
     }
+    if (MD_UPD_PKEY_SPEC & fields) {
+        md_log_perror(MD_LOG_MARK, MD_LOG_TRACE1, 0, ptemp, "update pkey spec: %s", name);
+        nmd->pkey_spec = NULL;
+        if (updates->pkey_spec) {
+            nmd->pkey_spec = apr_pmemdup(p, updates->pkey_spec, sizeof(md_pkey_spec_t));
+        }
+    }
 
     if (fields && APR_SUCCESS == (rv = md_save(reg->store, p, MD_SG_DOMAINS, nmd, 0))) {
         rv = state_init(reg, ptemp, nmd, 0);
@@ -764,6 +771,13 @@ apr_status_t md_reg_sync(md_reg_t *reg, apr_pool_t *p, apr_pool_t *ptemp,
                     smd->ca_challenges = NULL;
                     fields |= MD_UPD_CA_CHALLENGES;
                 }
+                if (!md_pkey_spec_eq(md->pkey_spec, smd->pkey_spec)) {
+                    fields |= MD_UPD_PKEY_SPEC;
+                    smd->pkey_spec = NULL;
+                    if (md->pkey_spec) {
+                        smd->pkey_spec = apr_pmemdup(p, md->pkey_spec, sizeof(md_pkey_spec_t));
+                    }
+                }
 
                 if (fields) {
                     rv = md_reg_update(reg, ptemp, smd->name, smd, fields);

diff --git a/src/md_reg.h b/src/md_reg.h
@@ -91,7 +91,8 @@ int md_reg_do(md_reg_do_cb *cb, void *baton, md_reg_t *reg, apr_pool_t *p);
 #define MD_UPD_DRIVE_MODE   0x0080
 #define MD_UPD_RENEW_WINDOW 0x0100
 #define MD_UPD_CA_CHALLENGES 0x0200
-#define MD_UPD_ALL          0x7FFF
+#define MD_UPD_PKEY_SPEC    0x0400
+#define MD_UPD_ALL          0x7FFFFFFF
 
 /**
  * Update the given fields for the managed domain. Take the new

diff --git a/src/md_store_fs.c b/src/md_store_fs.c
@@ -37,7 +37,7 @@
 /**************************************************************************************************/
 /* file system based implementation of md_store_t */
 
-#define MD_STORE_VERSION        2
+#define MD_STORE_VERSION        3
 
 typedef struct {
     apr_fileperms_t dir;
@@ -99,7 +99,6 @@ static apr_status_t init_store_file(md_store_fs_t *s_fs, const char *fname,
     unsigned char *key;
     apr_status_t rv;
 
-    md_json_sets(MOD_MD_VERSION, json, MD_KEY_VERSION, NULL);
     md_json_setn(MD_STORE_VERSION, json, MD_KEY_STORE, MD_KEY_VERSION, NULL);
 
     s_fs->key_len = FS_STORE_KLEN;
@@ -217,9 +216,13 @@ static apr_status_t read_store_file(md_store_fs_t *s_fs, const char *fname,
         /* Need to migrate format? */
         if (store_version < MD_STORE_VERSION) {
             if (store_version <= 1.0) {
-                md_log_perror(MD_LOG_MARK, MD_LOG_DEBUG, 0, p, "migrating store v1.0 -> v1.1");
+                md_log_perror(MD_LOG_MARK, MD_LOG_DEBUG, 0, p, "migrating store v1 -> v2");
                 rv = upgrade_from_1_0(s_fs, p, ptemp);
             }
+            if (store_version <= 2.0) {
+                md_log_perror(MD_LOG_MARK, MD_LOG_DEBUG, 0, p, "migrating store v2 -> v3");
+                md_json_del(json, MD_KEY_VERSION, NULL);
+            }
 
             if (APR_SUCCESS == rv) {
                 md_json_setn(MD_STORE_VERSION, json, MD_KEY_STORE, MD_KEY_VERSION, NULL);

diff --git a/src/md_version.h b/src/md_version.h
@@ -26,15 +26,15 @@
  * @macro
  * Version number of the md module as c string
  */
-#define MOD_MD_VERSION "0.9.0-git"
+#define MOD_MD_VERSION "0.9.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 0x000900
+#define MOD_MD_VERSION_NUM 0x000901
 
 #define MD_EXPERIMENTAL 0
 #define MD_ACME_DEF_URL    "https://acme-v01.api.letsencrypt.org/directory"

diff --git a/src/mod_md_config.c b/src/mod_md_config.c
@@ -630,10 +630,11 @@ static const char *md_config_set_pkeys(cmd_parms *cmd, void *arg,
         }
         else if (argc == 2) {
             bits = (int)apr_atoi64(argv[1]);
-            if (bits < 2048 || bits >= INT_MAX) {
-                return "must be a 2048 or higher in order to be considered safe. "
-                "Too large a value will slow down everything. Larger then 4096 probably does "
-                "not make sense unless quantum cryptography really changes spin.";
+            if (bits < MD_PKEY_RSA_BITS_MIN || bits >= INT_MAX) {
+                return apr_psprintf(cmd->pool, "must be %d or higher in order to be considered "
+                "safe. Too large a value will slow down everything. Larger then 4096 probably does "
+                "not make sense unless quantum cryptography really changes spin.", 
+                MD_PKEY_RSA_BITS_MIN);
             }
         }
         else {

diff --git a/test/test_0300_conf_validate.py b/test/test_0300_conf_validate.py
@@ -145,7 +145,7 @@ def test_300_015(self):
     @pytest.mark.parametrize("confFile,expErrMsg", [ 
         ("test_016a", "unsupported private key type"), 
         ("test_016b", "needs to specify the private key type"), 
-        ("test_016c", "must be a 2048 or higher"), 
+        ("test_016c", "must be 2048 or higher"), 
         ("test_016d", "key type 'RSA' has only one optinal parameter") ])
     def test_300_016(self, confFile, expErrMsg):
         # invalid pkey specification

diff --git a/test/test_0310_conf_store.py b/test/test_0310_conf_store.py
@@ -39,6 +39,7 @@ class TestConf:
 
     @classmethod
     def setup_class(cls):
+        time.sleep(1)
         cls.dns_uniq = "%d.org" % time.time()
 
     def setup_method(self, method):
@@ -215,7 +216,8 @@ def test_310_119(self):
         TestEnv.install_test_conf("key_rsa_2048");
         assert TestEnv.apache_restart() == 0
         assert TestEnv.a2md(["list"])['jout']['output'][0]['privkey'] == {
-            "type": "RSA"
+            "type": "RSA",
+            "bits": 2048
         }
 
     def test_310_120(self):
@@ -336,7 +338,6 @@ def test_310_208(self):
         assert TestEnv.apache_restart() == 0
         assert 'challenges' not in TestEnv.a2md(["list"])['jout']['output'][0]['ca']
 
-    @pytest.mark.skip(reason="fallback to default, if directive is removed from config?")
     @pytest.mark.parametrize("confFile", [ 
         ("key_rsa_2048"), ("key_rsa_4096")
     ])
@@ -447,7 +448,6 @@ def test_310_306(self):
         assert TestEnv.apache_restart() == 0
         assert TestEnv.a2md(["list"])['jout']['output'][0]['ca']['challenges'] == [ 'http-01', 'tls-sni-01' ]
 
-    @pytest.mark.skip(reason="store value not resetted when switching back to default value")
     def test_310_307(self):
         # test case:  RSA key length: 4096 -> 2048 -> 4096
         TestEnv.install_test_conf("key_rsa_4096");
@@ -460,7 +460,8 @@ def test_310_307(self):
         TestEnv.install_test_conf("key_rsa_2048");
         assert TestEnv.apache_restart() == 0
         assert TestEnv.a2md(["list"])['jout']['output'][0]['privkey'] == {
-            "type": "RSA"
+            "type": "RSA",
+            "bits": 2048
         }
 
         TestEnv.install_test_conf("key_rsa_4096");

diff --git a/test/test_0500_drive.py b/test/test_0500_drive.py
@@ -32,6 +32,7 @@ class TestDrive :
 
     @classmethod
     def setup_class(cls):
+        time.sleep(1)
         cls.dns_uniq = "%d.org" % time.time()
         cls.TMP_CONF = os.path.join(TestEnv.GEN_DIR, "auto.conf")
 
@@ -307,7 +308,6 @@ def test_500_201(self, renewWindow, testDataList):
             assert md["renew"] == tc["renew"], \
                 "Expected renew == {} indicator in {}, test case {}".format(tc["renew"], md, tc)
 
-    @pytest.mark.skip(reason="key generation fails with directive: MDPrivateKeys RSA 2048")
     @pytest.mark.parametrize("keyType,keyParams,expKeyLength", [
         ( "RSA", [ 2048 ], 2048 ),
         ( "RSA", [ 3072 ], 3072),
@@ -328,13 +328,14 @@ def test_500_202(self, keyType, keyParams, expKeyLength):
         assert TestEnv.apache_restart() == 0
         assert TestEnv.a2md([ "list", name])['jout']['output'][0]['state'] == TestEnv.MD_S_INCOMPLETE
         # setup: drive it
-        assert TestEnv.a2md( [ "-vv", "drive", name ] )['rv'] == 0
+        assert TestEnv.a2md( [ "-vv", "drive", name ] )['rv'] == 0, \
+            "Expected drive to succeeed for MDPrivateKeys {} {}".format(keyType, keyParams)
         assert TestEnv.a2md([ "list", name ])['jout']['output'][0]['state'] == TestEnv.MD_S_COMPLETE
         # check cert key length
         cert = CertUtil(TestEnv.path_domain_pubcert(name))
         assert cert.get_key_length() == expKeyLength
 
-    @pytest.mark.skip(reason="wrong TOS url in sandbox not replaced after config change")
+    #@pytest.mark.skip(reason="wrong TOS url in sandbox not replaced after config change")
     def test_500_203(self):
         # test case: reproduce issue with initially wrong agreement URL
         domain = "test500-203-" + TestDrive.dns_uniq
@@ -356,6 +357,7 @@ def test_500_203(self):
         conf.add_drive_mode( "manual" )
         conf.add_md( [name] )
         conf.install()
+        time.sleep(1)
         assert TestEnv.apache_restart() == 0
         assert TestEnv.a2md([ "list", name])['jout']['output'][0]['state'] == TestEnv.MD_S_INCOMPLETE
         # drive it -> runs OK

diff --git a/test/test_0600_roundtrip.py b/test/test_0600_roundtrip.py
@@ -29,6 +29,7 @@ class TestRoundtrip:
 
     @classmethod
     def setup_class(cls):
+        time.sleep(1)
         cls.dns_uniq = "%d.org" % time.time()
         cls.TMP_CONF = os.path.join(TestEnv.GEN_DIR, "roundtrip.conf")
 

diff --git a/test/test_0700_auto.py b/test/test_0700_auto.py
@@ -36,6 +36,7 @@ class TestAuto:
 
     @classmethod
     def setup_class(cls):
+        time.sleep(1)
         cls.dns_uniq = "%d.org" % time.time()
         cls.TMP_CONF = os.path.join(TestEnv.GEN_DIR, "auto.conf")