Expose minimum key size in the milter, add tests

Cleans up a couple of FIXMEs

Author: flowerysong

CHANGELOG.md

@@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file.
 - libopenarc - `arc_chain_oldest_pass()`
 - milter - `AuthResIP` configuration option.
 - milter - `RequireSafeKeys` configuration option.
+- milter - `MinimumKeySizeRSA` configuration option.
 
 ### Changed
 - Custom OpenSSL locations must be configured using `OPENSSL_CFLAGS`

libopenarc/arc.c

@@ -2160,15 +2160,12 @@ arc_verify_hash(ARC_MESSAGE *msg, char *b64sig, void *h, size_t hlen)
         goto error;
     }
 
-    keysize = EVP_PKEY_size(pkey);
-    /* FIXME: what is this actually measuring? Should this be
-     * EVP_PKEY_bits() or EVP_PKEY_securitybits()?
-     */
-    if (keysize * 8 < msg->arc_library->arcl_minkeysize)
+    keysize = EVP_PKEY_bits(pkey);
+    if (keysize < msg->arc_library->arcl_minkeysize)
     {
         arc_error(msg, "key size (%u) below minimum (%u)", keysize,
                   msg->arc_library->arcl_minkeysize);
-        status = ARC_STAT_CANTVRFY;
+        status = ARC_STAT_BADSIG;
         goto error;
     }
 
@@ -3340,7 +3337,6 @@ arc_getseal(ARC_MESSAGE         *msg,
     size_t              siglen;
     ARC_STAT            status = ARC_STAT_INTERNAL;
     size_t              diglen;
-    size_t              keysize;
     size_t              len;
     size_t              b64siglen;
     char               *sighdr = NULL;
@@ -3465,18 +3461,6 @@ arc_getseal(ARC_MESSAGE         *msg,
         goto error;
     }
 
-    /* FIXME: what is this actually measuring? Should this be
-     * EVP_PKEY_bits() or EVP_PKEY_securitybits()?
-     */
-    keysize = EVP_PKEY_size(pkey);
-    if (keysize * 8 < msg->arc_library->arcl_minkeysize)
-    {
-        arc_error(msg, "key size (%u) below minimum (%u)", keysize,
-                  msg->arc_library->arcl_minkeysize);
-        status = ARC_STAT_CANTVRFY;
-        goto error;
-    }
-
     dstr = arc_dstring_new(ARC_MAXHEADER, 0, msg, &arc_error_cb);
 
     /*

openarc/openarc-config.h

@@ -32,6 +32,7 @@ struct configdef arcf_config[] = {
     {"KeyFile",                       CONFIG_TYPE_STRING,  false},
     {"MaximumHeaders",                CONFIG_TYPE_INTEGER, false},
     {"MilterDebug",                   CONFIG_TYPE_INTEGER, false},
+    {"MinimumKeySizeRSA",             CONFIG_TYPE_INTEGER, false},
     {"Mode",                          CONFIG_TYPE_STRING,  false},
     {"PeerList",                      CONFIG_TYPE_STRING,  false},
     {"PermitAuthenticationOverrides", CONFIG_TYPE_BOOLEAN, false},

openarc/openarc.c

@@ -133,6 +133,7 @@ struct arcf_config
     unsigned char  *conf_keydata;           /* binary key data */
     size_t          conf_keylen;            /* key length */
     int             conf_maxhdrsz;          /* max. header size */
+    int             conf_minkeysz;          /* min. key size */
     struct config  *conf_data;              /* configuration data */
     ARC_LIB        *conf_libopenarc;        /* shared library instance */
     struct conflist conf_peers;             /* peers hosts */
@@ -1545,6 +1546,9 @@ arcf_config_load(struct config      *data,
         (void) config_get(data, "MaximumHeaders", &conf->conf_maxhdrsz,
                           sizeof conf->conf_maxhdrsz);
 
+        config_get(data, "MinimumKeySizeRSA", &conf->conf_minkeysz,
+                   sizeof conf->conf_minkeysz);
+
         (void) config_get(data, "SignHeaders", &conf->conf_signhdrs_raw,
                           sizeof conf->conf_signhdrs_raw);
 
@@ -1919,6 +1923,22 @@ arcf_config_setlib(struct arcf_config *conf, char **err)
         return false;
     }
 
+    if (conf->conf_minkeysz > 0)
+    {
+        status = arc_options(conf->conf_libopenarc, ARC_OP_SETOPT,
+                             ARC_OPTS_MINKEYSIZE, &conf->conf_minkeysz,
+                             sizeof conf->conf_minkeysz);
+    }
+
+    if (status != ARC_STAT_OK)
+    {
+        if (err != NULL)
+        {
+            *err = "failed to set ARC library options";
+        }
+        return false;
+    }
+
     if (conf->conf_testkeys)
     {
         status = arc_options(conf->conf_libopenarc, ARC_OP_SETOPT,

openarc/openarc.conf.5.in

@@ -130,6 +130,12 @@ provided, "127.0.0.1" is added to the list by default.
 Sets the debug level to be requested from the milter library.  The
 default is 0.
 
+.TP
+.I MinimumKeySizeRSA (integer)
+Disallows signatures whose keys are smaller than the specified size,
+regardless of whether they would otherwise be valid. If this is not
+set the library's default (which is currently 1024) will be used.
+
 .TP
 .I Mode (string)
 Selects the operating mode(s) for this filter.  If the string contains

openarc/openarc.conf.sample

@@ -152,6 +152,15 @@ KeyFile			/var/db/dkim/example.private
 
 # MilterDebug		0
 
+##  MinimumKeySizeRSA n
+##      default (none)
+##
+##  Disallows signatures whose keys are smaller than the specified size,
+##  regardless of whether they would otherwise be valid. If this is not set
+##  the library's default (which is currently 1024) will be used.
+
+# MinimumKeySizeRSA 2048
+
 ##  Mode modes
 ##  	default (none)
 ##

test/files/test_milter_minimum_key_bits.conf

@@ -0,0 +1,8 @@
+Domain          example.com
+AuthservID      example.com
+KeyFile         private.key
+TestKeys        public.key
+Selector        elpmaxe
+Mode            s
+FixedTimestamp  1234567890
+MinimumKeySizeRSA  2048

test/files/test_milter_minimum_key_bits_fail.conf

@@ -0,0 +1,8 @@
+Domain          example.com
+AuthservID      example.com
+KeyFile         private.key
+TestKeys        public.key
+Selector        elpmaxe
+Mode            s
+FixedTimestamp  1234567890
+MinimumKeySizeRSA  2049

test/test_milter.py

@@ -561,6 +561,20 @@ def test_milter_finalreceiver(run_miltertest):
     assert res['headers'][0][1] == ' example.com; arc=pass header.oldest-pass=0 smtp.remote-ip=127.0.0.1 arc.chain="example.com:example.com"'
 
 
+def test_milter_minimum_key_bits(run_miltertest):
+    """A 2048-bit key passes when that is the minimum"""
+    res = run_miltertest()
+    res = run_miltertest(res['headers'])
+    assert 'cv=pass' in res['headers'][0][1]
+
+
+def test_milter_minimum_key_bits_fail(run_miltertest):
+    """A 2048-bit key fails when the minimum is 2049"""
+    res = run_miltertest()
+    res = run_miltertest(res['headers'])
+    assert 'cv=fail' in res['headers'][0][1]
+
+
 def test_milter_peerlist(run_miltertest):
     """Connections from peers just get `accept` back immediately"""
     with pytest.raises(miltertest.MilterError, match='unexpected response: a'):