refactor away from pycryptopp "helper" classes

src/allmydata/client.py

@@ -12,7 +12,7 @@ from twisted.python.failure import Failure
 
 import allmydata
 from allmydata.crypto.ed25519 import SigningKey
-from allmydata.crypto.rsa import PrivateKey
+from allmydata.crypto import rsa
 from allmydata.storage.server import StorageServer
 from allmydata import storage_client
 from allmydata.immutable.upload import Uploader
@@ -156,8 +156,7 @@ class KeyGenerator(object):
         keysize = keysize or self.default_keysize
         # RSA key generation for a 2048 bit key takes between 0.8 and 3.2
         # secs
-        signer = PrivateKey.generate(keysize)
+        signer, verifier = rsa.create_signing_keypair(keysize)
-        verifier = signer.public_key()
         return defer.succeed( (verifier, signer) )
 
 class Terminator(service.Service):

src/allmydata/crypto/rsa.py

@@ -8,110 +8,153 @@ from cryptography.hazmat.primitives.serialization import load_der_private_key, l
 from allmydata.crypto import BadSignature
 
 
-class RsaMixin(object):
+"""
+This is the value that was used by `pycryptopp`, and we must continue to use it for
+both backwards compatibility and interoperability.
 
-    '''
+The docs for `cryptography` suggest to use the constant defined at
-    This is the value that was used by `pycryptopp`, and we must continue to use it for
+`cryptography.hazmat.primitives.asymmetric.padding.PSS.MAX_LENGTH`, but this causes old
-    both backwards compatibility and interoperability.
+signatures to fail to validate.
-
+"""
-    The docs for `cryptography` suggest to use the constant defined at
+RSA_PSS_SALT_LENGTH = 32
-    `cryptography.hazmat.primitives.asymmetric.padding.PSS.MAX_LENGTH`, but this causes old
-    signatures to fail to validate.
-    '''
-    RSA_PSS_SALT_LENGTH = 32
 
 
-class PrivateKey(RsaMixin):
+def create_verifying_key_from_string(public_key_der):
+    """
+    Create an RSA verifying key from a previously serialized public key.
 
-    def __init__(self, priv_key):
+    :returns: public key
-        self._priv_key = priv_key
+    """
+    pub_key = load_der_public_key(
+        public_key_der,
+        backend=default_backend(),
+    )
+    return pub_key
 
-    @classmethod
-    def generate(cls, key_size):
-        priv_key = rsa.generate_private_key(
-            public_exponent=65537,  # serisously don't change this value
-            key_size=key_size,
-            backend=default_backend()
-        )
-        return cls(priv_key)
 
-    @classmethod
+def create_signing_keypair(key_size):
-    def parse_string(cls, priv_key_str):
+    """
-        priv_key = load_der_private_key(
+    Create a new RSA signing keypair from scratch. Can be used with
-            priv_key_str,
+    `sign_data` function.
-            password=None,
-            backend=default_backend(),
-        )
-        return cls(priv_key)
 
-    def public_key(self):
+    :param int key_size: length of key in bits
-        return PublicKey(self._priv_key.public_key())
 
-    def serialize(self):
+    :returns: 2-tuple of (private_key, public_key)
-        return self._priv_key.private_bytes(
+    """
-            encoding=Encoding.DER,
+    priv_key = rsa.generate_private_key(
-            format=PrivateFormat.PKCS8,
+        public_exponent=65537,  # serisously don't change this value
-            encryption_algorithm=NoEncryption(),
+        key_size=key_size,
-        )
+        backend=default_backend()
+    )
+    return priv_key, priv_key.public_key()
 
-    def sign(self, data):
+
-        return self._priv_key.sign(
+def create_signing_keypair_from_string(private_key_der):
+    """
+    Create an RSA signing key from a previously serialized private key.
+
+    :returns: 2-tuple of (private_key, public_key)
+    """
+    priv_key = load_der_private_key(
+        private_key_der,
+        password=None,
+        backend=default_backend(),
+    )
+    return priv_key, priv_key.public_key()
+
+
+def der_string_from_signing_key(private_key):
+    """
+    Serializes a given RSA private key to a DER string
+    """
+    _validate_private_key(private_key)
+    return private_key.private_bytes(
+        encoding=Encoding.DER,
+        format=PrivateFormat.PKCS8,
+        encryption_algorithm=NoEncryption(),
+    )
+
+
+def der_string_from_verifying_key(public_key):
+    """
+    Serializes a given RSA private key to a DER string
+    """
+    _validate_public_key(public_key)
+    return public_key.public_bytes(
+        encoding=Encoding.DER,
+        format=PublicFormat.SubjectPublicKeyInfo,
+    )
+
+
+def create_verifying_key_from_string(public_key_der):
+    """
+    Create an RSA signing key from a previously serialized public key
+    """
+    pub_key = load_der_public_key(
+        public_key_der,
+        backend=default_backend(),
+    )
+    return pub_key
+
+
+def sign_data(private_key, data):
+    """
+    :param private_key: the private part of a keypair returned from
+        `create_signing_keypair_from_string` or `create_signing_keypair`
+
+    :param bytes data: the bytes to sign
+
+    :returns: bytes which are a signature of the bytes given as `data`.
+    """
+    _validate_private_key(private_key)
+    return private_key.sign(
+        data,
+        padding.PSS(
+            mgf=padding.MGF1(hashes.SHA256()),
+            salt_length=RSA_PSS_SALT_LENGTH,
+        ),
+        hashes.SHA256(),
+    )
+
+def verify_signature(public_key, alleged_signature, data):
+    """
+    :param public_key: a verifying key, returned from `create_verifying_key_from_string` or `create_verifying_key_from_private_key`
+
+    :param bytes alleged_signature: the bytes of the alleged signature
+
+    :param bytes data: the data which was allegedly signed
+    """
+    try:
+        public_key.verify(
+            alleged_signature,
             data,
             padding.PSS(
                 mgf=padding.MGF1(hashes.SHA256()),
-                salt_length=self.RSA_PSS_SALT_LENGTH,
+                salt_length=RSA_PSS_SALT_LENGTH,
             ),
             hashes.SHA256(),
         )
-
+    except InvalidSignature:
-    def __eq__(self, other):
+        raise BadSignature
-        if isinstance(other, type(self)):
-            return self.serialize() == other.serialize()
-        else:
-            return False
-
-    def __ne__(self, other):
-        return not self.__eq__(other)
 
 
-class PublicKey(RsaMixin):
+def _validate_public_key(public_key):
-
+    """
-    def __init__(self, pub_key):
+    Internal helper. Checks that `public_key` is a valid cryptography
-        self._pub_key = pub_key
+    object
-
+    """
-    @classmethod
+    if not isinstance(public_key, rsa.RSAPublicKey):
-    def parse_string(cls, pub_key_str):
+        raise ValueError(
-        pub_key = load_der_public_key(
+            "public_key not an RSAPublicKey"
-            pub_key_str,
-            backend=default_backend(),
-        )
-        return cls(pub_key)
-
-    def serialize(self):
-        return self._pub_key.public_bytes(
-            encoding=Encoding.DER,
-            format=PublicFormat.SubjectPublicKeyInfo,
         )
 
-    def verify(self, signature, data):
-        try:
-            self._pub_key.verify(
-                signature,
-                data,
-                padding.PSS(
-                    mgf=padding.MGF1(hashes.SHA256()),
-                    salt_length=self.RSA_PSS_SALT_LENGTH,
-                ),
-                hashes.SHA256(),
-            )
-        except InvalidSignature:
-            raise BadSignature
 
-    def __eq__(self, other):
+def _validate_private_key(private_key):
-        if isinstance(other, type(self)):
+    """
-            return self.serialize() == other.serialize()
+    Internal helper. Checks that `public_key` is a valid cryptography
-        else:
+    object
-            return False
+    """
-
+    if not isinstance(private_key, rsa.RSAPrivateKey):
-    def __ne__(self, other):
+        raise ValueError(
-        return not self.__eq__(other)
+            "private_key not an RSAPrivateKey"
+        )

src/allmydata/mutable/filenode.py

@@ -5,6 +5,7 @@ from twisted.internet import defer, reactor
 from foolscap.api import eventually
 
 from allmydata.crypto.aes import AES
+from allmydata.crypto import rsa
 from allmydata.interfaces import IMutableFileNode, ICheckable, ICheckResults, \
      NotEnoughSharesError, MDMF_VERSION, SDMF_VERSION, IMutableUploadable, \
      IMutableFileVersion, IWriteable
@@ -128,8 +129,8 @@ class MutableFileNode(object):
         """
         (pubkey, privkey) = keypair
         self._pubkey, self._privkey = pubkey, privkey
-        pubkey_s = self._pubkey.serialize()
+        pubkey_s = rsa.der_string_from_verifying_key(self._pubkey)
-        privkey_s = self._privkey.serialize()
+        privkey_s = rsa.der_string_from_signing_key(self._privkey)
         self._writekey = hashutil.ssk_writekey_hash(privkey_s)
         self._encprivkey = self._encrypt_privkey(self._writekey, privkey_s)
         self._fingerprint = hashutil.ssk_pubkey_fingerprint_hash(pubkey_s)

src/allmydata/mutable/publish.py

@@ -6,6 +6,7 @@ from twisted.internet import defer
 from twisted.python import failure
 
 from allmydata.crypto.aes import AES
+from allmydata.crypto import rsa
 from allmydata.interfaces import IPublishStatus, SDMF_VERSION, MDMF_VERSION, \
                                  IMutableUploadable
 from allmydata.util import base32, hashutil, mathutil, log
@@ -849,7 +850,7 @@ class Publish(object):
         started = time.time()
         self._status.set_status("Signing prefix")
         signable = self._get_some_writer().get_signable()
-        self.signature = self._privkey.sign(signable)
+        self.signature = rsa.sign_data(self._privkey, signable)
 
         for (shnum, writers) in self.writers.iteritems():
             for writer in writers:
@@ -864,7 +865,7 @@ class Publish(object):
         self._status.set_status("Pushing shares")
         self._started_pushing = started
         ds = []
-        verification_key = self._pubkey.serialize()
+        verification_key = rsa.der_string_from_verifying_key(self._pubkey)
 
         for (shnum, writers) in self.writers.copy().iteritems():
             for writer in writers:

src/allmydata/mutable/retrieve.py

@@ -9,7 +9,7 @@ from foolscap.api import eventually, fireEventually, DeadReferenceError, \
      RemoteException
 
 from allmydata.crypto.aes import AES
-from allmydata.crypto.rsa import PrivateKey
+from allmydata.crypto import rsa
 from allmydata.interfaces import IRetrieveStatus, NotEnoughSharesError, \
      DownloadStopped, MDMF_VERSION, SDMF_VERSION
 from allmydata.util.assertutil import _assert, precondition
@@ -935,7 +935,7 @@ class Retrieve(object):
         # it's good
         self.log("got valid privkey from shnum %d on reader %s" %
                  (reader.shnum, reader))
-        privkey = PrivateKey.parse_string(alleged_privkey_s)
+        privkey, _ = rsa.create_signing_keypair_from_string(alleged_privkey_s)
         self._node._populate_encprivkey(enc_privkey)
         self._node._populate_privkey(privkey)
         self._need_privkey = False

src/allmydata/mutable/servermap.py

@@ -9,7 +9,7 @@ from twisted.python import failure
 from foolscap.api import DeadReferenceError, RemoteException, eventually, \
                          fireEventually
 from allmydata.crypto import BadSignature
-from allmydata.crypto.rsa import PublicKey, PrivateKey
+from allmydata.crypto import rsa
 from allmydata.util import base32, hashutil, log, deferredutil
 from allmydata.util.dictutil import DictOfSets
 from allmydata.storage.server import si_b2a
@@ -845,7 +845,7 @@ class ServermapUpdater(object):
             # against the public key before keeping track of it.
             assert self._node.get_pubkey()
             try:
-                self._node.get_pubkey().verify(signature[1], prefix)
+                rsa.verify_signature(self._node.get_pubkey(), signature[1], prefix)
             except BadSignature:
                 raise CorruptShareError(server, shnum,
                                         "signature is invalid")
@@ -916,7 +916,7 @@ class ServermapUpdater(object):
                                                               update_data)
 
     def _deserialize_pubkey(self, pubkey_s):
-        verifier = PublicKey.parse_string(pubkey_s)
+        verifier = rsa.create_verifying_key_from_string(pubkey_s)
         return verifier
 
     def _try_to_validate_privkey(self, enc_privkey, server, shnum, lp):
@@ -937,7 +937,7 @@ class ServermapUpdater(object):
         self.log("got valid privkey from shnum %d on serverid %s" %
                  (shnum, server.get_name()),
                  parent=lp)
-        privkey = PrivateKey.parse_string(alleged_privkey_s)
+        privkey, _ = rsa.create_signing_keypair_from_string(alleged_privkey_s)
         self._node._populate_encprivkey(enc_privkey)
         self._node._populate_privkey(privkey)
         self._need_privkey = False

src/allmydata/test/mutable/test_problems.py

@@ -5,6 +5,7 @@ from twisted.trial import unittest
 from twisted.internet import defer
 from foolscap.logging import log
 from allmydata import uri
+from allmydata.crypto import rsa
 from allmydata.interfaces import NotEnoughSharesError, SDMF_VERSION, MDMF_VERSION
 from allmydata.util import fileutil
 from allmydata.util.hashutil import ssk_writekey_hash, ssk_pubkey_fingerprint_hash
@@ -211,8 +212,8 @@ class Problems(GridTestMixin, unittest.TestCase, testutil.ShouldFailMixin):
         def _got_key(keypair):
             (pubkey, privkey) = keypair
             nm.key_generator = SameKeyGenerator(pubkey, privkey)
-            pubkey_s = pubkey.serialize()
+            pubkey_s = rsa.der_string_from_verifying_key(pubkey)
-            privkey_s = privkey.serialize()
+            privkey_s = rsa.der_string_from_signing_key(privkey)
             u = uri.WriteableSSKFileURI(ssk_writekey_hash(privkey_s),
                                         ssk_pubkey_fingerprint_hash(pubkey_s))
             self._storage_index = u.get_storage_index()

src/allmydata/test/test_crypto.py

@@ -209,13 +209,8 @@ class TestRegression(unittest.TestCase):
         This simply checks that keys and signatures generated using the old code are still valid
         using the new code.
         '''
-        priv_key = rsa.PrivateKey.parse_string(self.RSA_2048_PRIV_KEY)
+        priv_key, pub_key = rsa.create_signing_keypair_from_string(self.RSA_2048_PRIV_KEY)
-        pub_key = priv_key.public_key()
+        rsa.verify_signature(pub_key, self.RSA_2048_SIG, b'test')
-
-        parsed_pub_key = rsa.PublicKey.parse_string(self.RSA_2048_PUB_KEY)
-        self.failUnlessEqual(pub_key, parsed_pub_key)
-
-        pub_key.verify(self.RSA_2048_SIG, b'test')
 
 
 class TestEd25519(unittest.TestCase):
@@ -249,24 +244,26 @@ class TestEd25519(unittest.TestCase):
 class TestRsa(unittest.TestCase):
 
     def test_keys(self):
-        priv_key = rsa.PrivateKey.generate(2048)
+        priv_key, pub_key = rsa.create_signing_keypair(2048)
-        priv_key_str = priv_key.serialize()
+        priv_key_str = rsa.der_string_from_signing_key(priv_key)
 
         self.assertIsInstance(priv_key_str, six.string_types)
 
-        priv_key2 = rsa.PrivateKey.parse_string(priv_key_str)
+        priv_key2, pub_key2 = rsa.create_signing_keypair_from_string(priv_key_str)
 
-        self.failUnlessEqual(priv_key, priv_key2)
+        # instead of asking "are these two keys equal", we can instead
+        # test their function: can the second key verify a signature
+        # produced by the first (and FAIL a signature with different
+        # data)
 
-        pub_key = priv_key.public_key()
+        data_to_sign = b"test data"
-        pub_key2 = priv_key2.public_key()
+        sig0 = rsa.sign_data(priv_key, data_to_sign)
+        rsa.verify_signature(pub_key2, sig0, data_to_sign)
 
-        self.failUnlessEqual(pub_key, pub_key2)
+        # ..and the other way
+        sig1 = rsa.sign_data(priv_key2, data_to_sign)
+        rsa.verify_signature(pub_key, sig1, data_to_sign)
 
-        pub_key_str = pub_key.serialize()
+        # ..and a failed way
-
+        with self.assertRaises(rsa.BadSignature):
-        self.assertIsInstance(pub_key_str, six.string_types)
+            rsa.verify_signature(pub_key, sig1, data_to_sign + b"more")
-
-        pub_key2 = rsa.PublicKey.parse_string(pub_key_str)
-
-        self.failUnlessEqual(pub_key, pub_key2)