Merge pull request #1188 from meejah/3828.key-length

Remove ability to configure RSA key size

Fixes: ticket:3828

newsfragments/3828.feature

@@ -0,0 +1,8 @@
+The implementation of SDMF and MDMF (mutables) now requires RSA keys to be exactly 2048 bits, aligning them with the specification.
+
+Some code existed to allow tests to shorten this and it's
+conceptually possible a modified client produced mutables
+with different key-sizes. However, the spec says that they
+must be 2048 bits. If you happen to have a capability with
+a key-size different from 2048 you may use 1.17.1 or earlier
+to read the content.

src/allmydata/client.py

@@ -168,29 +168,12 @@ class SecretHolder(object):
 
 class KeyGenerator(object):
     """I create RSA keys for mutable files. Each call to generate() returns a
-    single keypair. The keysize is specified first by the keysize= argument
+    single keypair."""
-    to generate(), then with a default set by set_default_keysize(), then
-    with a built-in default of 2048 bits."""
-    def __init__(self):
-        self.default_keysize = 2048
 
-    def set_default_keysize(self, keysize):
+    def generate(self):
-        """Call this to override the size of the RSA keys created for new
-        mutable files which don't otherwise specify a size. This will affect
-        all subsequent calls to generate() without a keysize= argument. The
-        default size is 2048 bits. Test cases should call this method once
-        during setup, to cause me to create smaller keys, so the unit tests
-        run faster."""
-        self.default_keysize = keysize
-
-    def generate(self, keysize=None):
         """I return a Deferred that fires with a (verifyingkey, signingkey)
-        pair. I accept a keysize in bits (2048 bit keys are standard, smaller
+        pair. The returned key will be 2048 bit"""
-        keys are used for testing). If you do not provide a keysize, I will
+        keysize = 2048
-        use my default, which is set by a call to set_default_keysize(). If
-        set_default_keysize() has never been called, I will create 2048 bit
-        keys."""
-        keysize = keysize or self.default_keysize
         # RSA key generation for a 2048 bit key takes between 0.8 and 3.2
         # secs
         signer, verifier = rsa.create_signing_keypair(keysize)
@@ -993,9 +976,6 @@ class _Client(node.Node, pollmixin.PollMixin):
         helper_furlfile = self.config.get_private_path("helper.furl").encode(get_filesystem_encoding())
         self.tub.registerReference(self.helper, furlFile=helper_furlfile)
 
-    def set_default_mutable_keysize(self, keysize):
-        self._key_generator.set_default_keysize(keysize)
-
     def _get_tempdir(self):
         """
         Determine the path to the directory where temporary files for this node
@@ -1096,8 +1076,8 @@ class _Client(node.Node, pollmixin.PollMixin):
     def create_immutable_dirnode(self, children, convergence=None):
         return self.nodemaker.create_immutable_directory(children, convergence)
 
-    def create_mutable_file(self, contents=None, keysize=None, version=None):
+    def create_mutable_file(self, contents=None, version=None):
-        return self.nodemaker.create_mutable_file(contents, keysize,
+        return self.nodemaker.create_mutable_file(contents,
                                                   version=version)
 
     def upload(self, uploadable, reactor=None):

src/allmydata/crypto/rsa.py

@@ -77,6 +77,14 @@ def create_signing_keypair_from_string(private_key_der):
         password=None,
         backend=default_backend(),
     )
+    if not isinstance(priv_key, rsa.RSAPrivateKey):
+        raise ValueError(
+            "Private Key did not decode to an RSA key"
+        )
+    if priv_key.key_size != 2048:
+        raise ValueError(
+            "Private Key must be 2048 bits"
+        )
     return priv_key, priv_key.public_key()
 
 

src/allmydata/nodemaker.py

@@ -126,12 +126,12 @@ class NodeMaker(object):
             return self._create_dirnode(filenode)
         return None
 
-    def create_mutable_file(self, contents=None, keysize=None, version=None):
+    def create_mutable_file(self, contents=None, version=None):
         if version is None:
             version = self.mutable_file_default
         n = MutableFileNode(self.storage_broker, self.secret_holder,
                             self.default_encoding_parameters, self.history)
-        d = self.key_generator.generate(keysize)
+        d = self.key_generator.generate()
         d.addCallback(n.create_with_keys, contents, version=version)
         d.addCallback(lambda res: n)
         return d

src/allmydata/test/common.py

@@ -133,8 +133,6 @@ from subprocess import (
     PIPE,
 )
 
-TEST_RSA_KEY_SIZE = 522
-
 EMPTY_CLIENT_CONFIG = config_from_string(
     "/dev/null",
     "tub.port",

src/allmydata/test/common_system.py

@@ -34,7 +34,6 @@ from twisted.python.filepath import (
 )
 
 from .common import (
-    TEST_RSA_KEY_SIZE,
     SameProcessStreamEndpointAssigner,
 )
 
@@ -736,7 +735,6 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
         c = yield client.create_client(basedirs[0])
         c.setServiceParent(self.sparent)
         self.clients.append(c)
-        c.set_default_mutable_keysize(TEST_RSA_KEY_SIZE)
 
         with open(os.path.join(basedirs[0],"private","helper.furl"), "r") as f:
             helper_furl = f.read()
@@ -754,7 +752,6 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
             c = yield client.create_client(basedirs[i])
             c.setServiceParent(self.sparent)
             self.clients.append(c)
-            c.set_default_mutable_keysize(TEST_RSA_KEY_SIZE)
         log.msg("STARTING")
         yield self.wait_for_connections()
         log.msg("CONNECTED")
@@ -838,7 +835,6 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
         def _stopped(res):
             new_c = yield client.create_client(self.getdir("client%d" % num))
             self.clients[num] = new_c
-            new_c.set_default_mutable_keysize(TEST_RSA_KEY_SIZE)
             new_c.setServiceParent(self.sparent)
         d.addCallback(_stopped)
         d.addCallback(lambda res: self.wait_for_connections())
@@ -877,7 +873,6 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
 
         c = yield client.create_client(basedir.path)
         self.clients.append(c)
-        c.set_default_mutable_keysize(TEST_RSA_KEY_SIZE)
         self.numclients += 1
         if add_to_sparent:
             c.setServiceParent(self.sparent)

src/allmydata/test/data/pycryptopp-rsa-1024-priv.txt

@@ -0,0 +1 @@
+MIICdQIBADANBgkqhkiG9w0BAQEFAASCAl8wggJbAgEAAoGBAJLEAfZueLuT4vUQ1+c8ZM9dJ/LA29CYgA5toaMklQjbVQ2Skywvw1wEkRjhMpjQAx5+lpLTE2xCtqtfkHooMRNnquOxoh0o1Xya60jUHze7VB5QMV7BMKeUTff1hQqpIgw/GLvJRtar53cVY+SYf4SXx2/slDbVr8BI3DPwdeNtAgERAoGABzHD3GTJrteQJRxu+cQ3I0NPwx2IQ/Nlplq1GZDaIQ/FbJY+bhZrdXOswnl4cOcPNjNhu+c1qHGznv0ntayjCGgJ9dDySGqknDau+ezZcBO1JrIpPOABS7MVMst79mn47vB2+t8w5krrBYahAVp/L5kY8k+Pr9AU+L9mbevFW9MCQQDA+bAeMRNBfGc4gvoVV8ecovE1KRksFDlkaDVEOc76zNW6JZazHhQF/zIoMkV81rrg5UBntw3WR3R8A3l9osgDAkEAwrLQICJ3zjsJBt0xEkCBv9tK6IvSIc7MUQIc4J2Y1hiSjqsnTRACRy3UMsODfx/Lg7ITlDbABCLfv3v4D39jzwJBAKpFuYQNLxuqALlkgk8RN6hTiYlCYYE/BXa2TR4U4848RBy3wTSiEarwO1Ck0+afWZlCwFuDZo/kshMSH+dTZS8CQQC3PuIAIHDCGXHoV7W200zwzmSeoba2aEfTxcDTZyZvJi+VVcqi4eQGwbioP4rR/86aEQNeUaWpijv/g7xK0j/RAkBbt2U9bFFcja10KIpgw2bBxDU/c67h4+38lkrBUnM9XVBZxjbtQbnkkeAfOgQDiq3oBDBrHF3/Q8XM0CzZJBWS

src/allmydata/test/mutable/test_problems.py

@@ -26,7 +26,6 @@ from allmydata.mutable.common import \
      NotEnoughServersError
 from allmydata.mutable.publish import MutableData
 from allmydata.storage.common import storage_index_to_dir
-from ..common import TEST_RSA_KEY_SIZE
 from ..no_network import GridTestMixin
 from .. import common_util as testutil
 from ..common_util import DevNullDictionary
@@ -219,7 +218,7 @@ class Problems(GridTestMixin, AsyncTestCase, testutil.ShouldFailMixin):
         # use #467 static-server-selection to disable permutation and force
         # the choice of server for share[0].
 
-        d = nm.key_generator.generate(TEST_RSA_KEY_SIZE)
+        d = nm.key_generator.generate()
         def _got_key(keypair):
             (pubkey, privkey) = keypair
             nm.key_generator = SameKeyGenerator(pubkey, privkey)

src/allmydata/test/mutable/util.py

@@ -25,7 +25,6 @@ from allmydata.storage_client import StorageFarmBroker
 from allmydata.mutable.layout import MDMFSlotReadProxy
 from allmydata.mutable.publish import MutableData
 from ..common import (
-    TEST_RSA_KEY_SIZE,
     EMPTY_CLIENT_CONFIG,
 )
 
@@ -287,7 +286,7 @@ def make_storagebroker_with_peers(peers):
     return storage_broker
 
 
-def make_nodemaker(s=None, num_peers=10, keysize=TEST_RSA_KEY_SIZE):
+def make_nodemaker(s=None, num_peers=10):
     """
     Make a ``NodeMaker`` connected to some number of fake storage servers.
 
@@ -298,20 +297,20 @@ def make_nodemaker(s=None, num_peers=10, keysize=TEST_RSA_KEY_SIZE):
         the node maker.
     """
     storage_broker = make_storagebroker(s, num_peers)
-    return make_nodemaker_with_storage_broker(storage_broker, keysize)
+    return make_nodemaker_with_storage_broker(storage_broker)
 
 
-def make_nodemaker_with_peers(peers, keysize=TEST_RSA_KEY_SIZE):
+def make_nodemaker_with_peers(peers):
     """
     Make a ``NodeMaker`` connected to the given storage servers.
 
     :param list peers: The storage servers to associate with the node maker.
     """
     storage_broker = make_storagebroker_with_peers(peers)
-    return make_nodemaker_with_storage_broker(storage_broker, keysize)
+    return make_nodemaker_with_storage_broker(storage_broker)
 
 
-def make_nodemaker_with_storage_broker(storage_broker, keysize):
+def make_nodemaker_with_storage_broker(storage_broker):
     """
     Make a ``NodeMaker`` using the given storage broker.
 
@@ -319,8 +318,6 @@ def make_nodemaker_with_storage_broker(storage_broker, keysize):
     """
     sh = client.SecretHolder(b"lease secret", b"convergence secret")
     keygen = client.KeyGenerator()
-    if keysize:
-        keygen.set_default_keysize(keysize)
     nodemaker = NodeMaker(storage_broker, sh, None,
                           None, None,
                           {"k": 3, "n": 10}, SDMF_VERSION, keygen)

src/allmydata/test/no_network.py

@@ -61,7 +61,6 @@ from allmydata.storage_client import (
     _StorageServer,
 )
 from .common import (
-    TEST_RSA_KEY_SIZE,
     SameProcessStreamEndpointAssigner,
 )
 
@@ -393,7 +392,6 @@ class NoNetworkGrid(service.MultiService):
 
         if not c:
             c = yield create_no_network_client(clientdir)
-            c.set_default_mutable_keysize(TEST_RSA_KEY_SIZE)
 
         c.nodeid = clientid
         c.short_nodeid = b32encode(clientid).lower()[:8]

src/allmydata/test/test_crypto.py

@@ -60,6 +60,28 @@ class TestRegression(unittest.TestCase):
         # The public key corresponding to `RSA_2048_PRIV_KEY`.
         RSA_2048_PUB_KEY = b64decode(f.read().strip())
 
+    with RESOURCE_DIR.child('pycryptopp-rsa-1024-priv.txt').open('r') as f:
+        # Created using `pycryptopp`:
+        #
+        #     from base64 import b64encode
+        #     from pycryptopp.publickey import rsa
+        #     priv = rsa.generate(1024)
+        #     priv_str = b64encode(priv.serialize())
+        #     pub_str = b64encode(priv.get_verifying_key().serialize())
+        RSA_TINY_PRIV_KEY = b64decode(f.read().strip())
+        assert isinstance(RSA_TINY_PRIV_KEY, native_bytes)
+
+    with RESOURCE_DIR.child('pycryptopp-rsa-32768-priv.txt').open('r') as f:
+        # Created using `pycryptopp`:
+        #
+        #     from base64 import b64encode
+        #     from pycryptopp.publickey import rsa
+        #     priv = rsa.generate(32768)
+        #     priv_str = b64encode(priv.serialize())
+        #     pub_str = b64encode(priv.get_verifying_key().serialize())
+        RSA_HUGE_PRIV_KEY = b64decode(f.read().strip())
+        assert isinstance(RSA_HUGE_PRIV_KEY, native_bytes)
+
     def test_old_start_up_test(self):
         """
         This was the old startup test run at import time in `pycryptopp.cipher.aes`.
@@ -232,6 +254,22 @@ class TestRegression(unittest.TestCase):
         priv_key, pub_key = rsa.create_signing_keypair_from_string(self.RSA_2048_PRIV_KEY)
         rsa.verify_signature(pub_key, self.RSA_2048_SIG, b'test')
 
+    def test_decode_tiny_rsa_keypair(self):
+        '''
+        An unreasonably small RSA key is rejected ("unreasonably small"
+        means less that 2048 bits)
+        '''
+        with self.assertRaises(ValueError):
+            rsa.create_signing_keypair_from_string(self.RSA_TINY_PRIV_KEY)
+
+    def test_decode_huge_rsa_keypair(self):
+        '''
+        An unreasonably _large_ RSA key is rejected ("unreasonably large"
+        means 32768 or more bits)
+        '''
+        with self.assertRaises(ValueError):
+            rsa.create_signing_keypair_from_string(self.RSA_HUGE_PRIV_KEY)
+
     def test_encrypt_data_not_bytes(self):
         '''
         only bytes can be encrypted