Merge pull request #1607 from cjwatson/10208-conch-ed25519-pynacl

Author: cjwatson Reviewer: adiroiban Fixes: ticket:10208 Add a PyNaCl-based fallback path for Ed25519 Some commonly-used platforms that Twisted still supports, such as Ubuntu 18.04 LTS, lack a version of OpenSSL new enough to support Ed25519 via cryptography. Add a fallback path using PyNaCl to support these.
twisted · Feb 11, 2022 · 0011ddd · 0011ddd
2 parents 125b2d6 + 8a463f4
commit 0011ddd
Show file tree

Hide file tree

Showing 8 changed files with 294 additions and 8 deletions.
diff --git a/docs/installation/howto/optional.rst b/docs/installation/howto/optional.rst
@@ -36,6 +36,8 @@ The following optional dependencies are supported:
   * `pyasn1`_
   * `cryptography`_
 
+* **conch_nacl** - **conch** options and `PyNaCl`_ to support Ed25519 keys on systems with OpenSSL < 1.1.1b.
+
 * **soap** - the `SOAPpy`_ package to work with SOAP.
 
 * **serial** - the `pyserial`_ package to work with serial data.
@@ -65,6 +67,7 @@ The following optional dependencies are supported:
 .. _service_identity: https://pypi.python.org/pypi/service_identity
 .. _pyasn1: https://pypi.python.org/pypi/pyasn1
 .. _cryptography: https://pypi.python.org/pypi/cryptography
+.. _PyNaCl: https://pypi.python.org/pypi/PyNaCl
 .. _SOAPpy: https://pypi.python.org/pypi/SOAPpy
 .. _pyserial: https://pypi.python.org/pypi/pyserial
 .. _pyobjc: https://pypi.python.org/pypi/pyobjc

diff --git a/setup.cfg b/setup.cfg
@@ -84,6 +84,11 @@ conch =
     appdirs >= 1.4.0
     bcrypt >= 3.0.0
 
+conch_nacl =
+    %(conch)s
+    # Used to support Ed25519 keys on systems with OpenSSL < 1.1.1b
+    PyNaCl
+
 serial =
     pyserial >= 3.0
     pywin32 != 226; platform_system == "Windows"
@@ -123,6 +128,7 @@ mypy =
     types-pyOpenSSL
     %(dev)s
     %(all_non_platform)s
+    %(conch_nacl)s
 
 [options.packages.find]
 where = src

diff --git a/src/twisted/conch/newsfragments/10208.feature b/src/twisted/conch/newsfragments/10208.feature
@@ -0,0 +1 @@
+twisted.conch.ssh now has an alternative Ed25519 implementation using PyNaCl, in order to support platforms that lack OpenSSL >= 1.1.1b.  The new "conch_nacl" extra has the necessary dependency.
diff --git a/src/twisted/conch/scripts/ckeygen.py b/src/twisted/conch/scripts/ckeygen.py
@@ -179,9 +179,7 @@ def generateECDSAkey(options):
 
 @_keyGenerator("ed25519")
 def generateEd25519key(options):
-    from cryptography.hazmat.primitives.asymmetric import ed25519
-
-    keyPrimitive = ed25519.Ed25519PrivateKey.generate()
+    keyPrimitive = keys.Ed25519PrivateKey.generate()
     key = keys.Key(keyPrimitive)
     _saveKey(key, options)
 

diff --git a/src/twisted/conch/ssh/_keys_pynacl.py b/src/twisted/conch/ssh/_keys_pynacl.py
@@ -0,0 +1,104 @@
+# -*- test-case-name: twisted.conch.test.test_keys -*-
+# Copyright (c) Twisted Matrix Laboratories.
+# See LICENSE for details.
+
+"""
+Optional PyNaCl fallback code for Ed25519 keys.
+"""
+
+from cryptography.exceptions import InvalidSignature
+from cryptography.hazmat.primitives import serialization
+from cryptography.hazmat.primitives.asymmetric import ed25519
+from nacl.exceptions import BadSignatureError
+from nacl.signing import SigningKey, VerifyKey
+
+
+class Ed25519PublicKey(ed25519.Ed25519PublicKey):
+    def __init__(self, data: bytes):
+        self._key = VerifyKey(data)
+
+    def __bytes__(self) -> bytes:
+        return bytes(self._key)
+
+    def __hash__(self) -> int:
+        return hash(bytes(self))
+
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, self.__class__):
+            return False
+        return self._key == other._key
+
+    def __ne__(self, other: object) -> bool:
+        return not (self == other)
+
+    @classmethod
+    def from_public_bytes(cls, data: bytes) -> ed25519.Ed25519PublicKey:
+        return cls(data)
+
+    def public_bytes(
+        self,
+        encoding: serialization.Encoding,
+        format: serialization.PublicFormat,
+    ) -> bytes:
+        if (
+            encoding is not serialization.Encoding.Raw
+            or format is not serialization.PublicFormat.Raw
+        ):
+            raise ValueError("Both encoding and format must be Raw")
+        return bytes(self)
+
+    def verify(self, signature: bytes, data: bytes) -> None:
+        try:
+            self._key.verify(data, signature)
+        except BadSignatureError as e:
+            raise InvalidSignature(str(e))
+
+
+class Ed25519PrivateKey(ed25519.Ed25519PrivateKey):
+    def __init__(self, data: bytes):
+        self._key = SigningKey(data)
+
+    def __bytes__(self) -> bytes:
+        return bytes(self._key)
+
+    def __hash__(self) -> int:
+        return hash(bytes(self))
+
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, self.__class__):
+            return False
+        return self._key == other._key
+
+    def __ne__(self, other: object) -> bool:
+        return not (self == other)
+
+    @classmethod
+    def generate(cls) -> ed25519.Ed25519PrivateKey:
+        return cls(bytes(SigningKey.generate()))
+
+    @classmethod
+    def from_private_bytes(cls, data: bytes) -> ed25519.Ed25519PrivateKey:
+        return cls(data)
+
+    def public_key(self) -> ed25519.Ed25519PublicKey:
+        return Ed25519PublicKey(bytes(self._key.verify_key))
+
+    def private_bytes(
+        self,
+        encoding: serialization.Encoding,
+        format: serialization.PrivateFormat,
+        encryption_algorithm: serialization.KeySerializationEncryption,
+    ) -> bytes:
+        if (
+            encoding is not serialization.Encoding.Raw
+            or format is not serialization.PrivateFormat.Raw
+            or not isinstance(encryption_algorithm, serialization.NoEncryption)
+        ):
+            raise ValueError(
+                "Encoding and format must be Raw and "
+                "encryption_algorithm must be NoEncryption"
+            )
+        return bytes(self)
+
+    def sign(self, data: bytes) -> bytes:
+        return self._key.sign(data).signature
diff --git a/src/twisted/conch/ssh/keys.py b/src/twisted/conch/ssh/keys.py
@@ -14,6 +14,7 @@
 import warnings
 from base64 import b64encode, decodebytes, encodebytes
 from hashlib import md5, sha256
+from typing import Optional, Type
 
 import bcrypt
 from cryptography import utils
@@ -67,6 +68,20 @@
 }
 
 
+Ed25519PublicKey: Optional[Type[ed25519.Ed25519PublicKey]]
+Ed25519PrivateKey: Optional[Type[ed25519.Ed25519PrivateKey]]
+
+if default_backend().ed25519_supported():
+    Ed25519PublicKey = ed25519.Ed25519PublicKey
+    Ed25519PrivateKey = ed25519.Ed25519PrivateKey
+else:  # pragma: no cover
+    try:
+        from twisted.conch.ssh._keys_pynacl import Ed25519PrivateKey, Ed25519PublicKey
+    except ImportError:
+        Ed25519PublicKey = None
+        Ed25519PrivateKey = None
+
+
 class BadKeyError(Exception):
     """
     Raised when a key isn't what we expected from it.
@@ -921,10 +936,13 @@ def _fromEd25519Components(cls, a, k=None):
         @type k: L{bytes}
         """
 
+        if Ed25519PublicKey is None or Ed25519PrivateKey is None:
+            raise BadKeyError("Ed25519 keys not supported on this system")
+
         if k is None:
-            keyObject = ed25519.Ed25519PublicKey.from_public_bytes(a)
+            keyObject = Ed25519PublicKey.from_public_bytes(a)
         else:
-            keyObject = ed25519.Ed25519PrivateKey.from_private_bytes(k)
+            keyObject = Ed25519PrivateKey.from_private_bytes(k)
 
         return cls(keyObject)
 

diff --git a/src/twisted/conch/test/test_keys.py b/src/twisted/conch/test/test_keys.py
@@ -21,16 +21,20 @@
     skipCryptography = "Cannot run without cryptography."
 
 pyasn1 = requireModule("pyasn1")
+_keys_pynacl = requireModule("twisted.conch.ssh._keys_pynacl")
 
 
 if cryptography and pyasn1:
+    from cryptography.exceptions import InvalidSignature
     from cryptography.hazmat.backends import default_backend
     from cryptography.hazmat.primitives import hashes
     from cryptography.hazmat.primitives.asymmetric import padding
 
     from twisted.conch.ssh import common, keys, sexpy
 
-    ED25519_SUPPORTED = default_backend().ed25519_supported()
+    ED25519_SUPPORTED = (
+        default_backend().ed25519_supported() or _keys_pynacl is not None
+    )
 else:
     ED25519_SUPPORTED = False
 
@@ -1669,6 +1673,156 @@ def test_reprPrivateEd25519(self):
         )
 
 
+class PyNaClKeyTests(KeyTests):
+    """
+    Key tests, but forcing the use of C{PyNaCl}.
+    """
+
+    if cryptography is None:
+        skip = skipCryptography
+    if _keys_pynacl is None:
+        skip = "Cannot run without PyNaCl"
+
+    def setUp(self):
+        super().setUp()
+        self.patch(keys, "Ed25519PublicKey", _keys_pynacl.Ed25519PublicKey)
+        self.patch(keys, "Ed25519PrivateKey", _keys_pynacl.Ed25519PrivateKey)
+
+    def test_naclPrivateBytes(self):
+        """
+        L{_keys_pynacl.Ed25519PrivateKey.private_bytes} and
+        L{_keys_pynacl.Ed25519PrivateKey.from_private_bytes} round-trip.
+        """
+        from cryptography.hazmat.primitives import serialization
+
+        key = _keys_pynacl.Ed25519PrivateKey.generate()
+        key_bytes = key.private_bytes(
+            serialization.Encoding.Raw,
+            serialization.PrivateFormat.Raw,
+            serialization.NoEncryption(),
+        )
+        self.assertIsInstance(key_bytes, bytes)
+        self.assertEqual(
+            key, _keys_pynacl.Ed25519PrivateKey.from_private_bytes(key_bytes)
+        )
+
+    def test_naclPrivateBytesInvalidParameters(self):
+        """
+        L{_keys_pynacl.Ed25519PrivateKey.private_bytes} only accepts certain parameters.
+        """
+        from cryptography.hazmat.primitives import serialization
+
+        key = _keys_pynacl.Ed25519PrivateKey.generate()
+        self.assertRaises(
+            ValueError,
+            key.private_bytes,
+            serialization.Encoding.PEM,
+            serialization.PrivateFormat.Raw,
+            serialization.NoEncryption(),
+        )
+        self.assertRaises(
+            ValueError,
+            key.private_bytes,
+            serialization.Encoding.Raw,
+            serialization.PrivateFormat.PKCS8,
+            serialization.NoEncryption(),
+        )
+        self.assertRaises(
+            ValueError,
+            key.private_bytes,
+            serialization.Encoding.Raw,
+            serialization.PrivateFormat.Raw,
+            serialization.BestAvailableEncryption(b"password"),
+        )
+
+    def test_naclPrivateHash(self):
+        """
+        L{_keys_pynacl.Ed25519PrivateKey.__hash__} allows instances to be hashed.
+        """
+        key = _keys_pynacl.Ed25519PrivateKey.generate()
+        d = {key: True}
+        self.assertTrue(d[key])
+
+    def test_naclPrivateEquality(self):
+        """
+        L{_keys_pynacl.Ed25519PrivateKey} implements equality test methods.
+        """
+        key1 = _keys_pynacl.Ed25519PrivateKey.generate()
+        key2 = _keys_pynacl.Ed25519PrivateKey.generate()
+        self.assertEqual(key1, key1)
+        self.assertNotEqual(key1, key2)
+        self.assertNotEqual(key1, bytes(key1))
+
+    def test_naclPublicBytes(self):
+        """
+        L{_keys_pynacl.Ed25519PublicKey.public_bytes} and
+        L{_keys_pynacl.Ed25519PublicKey.from_public_bytes} round-trip.
+        """
+        from cryptography.hazmat.primitives import serialization
+
+        key = _keys_pynacl.Ed25519PrivateKey.generate().public_key()
+        key_bytes = key.public_bytes(
+            serialization.Encoding.Raw, serialization.PublicFormat.Raw
+        )
+        self.assertIsInstance(key_bytes, bytes)
+        self.assertEqual(
+            key, _keys_pynacl.Ed25519PublicKey.from_public_bytes(key_bytes)
+        )
+
+    def test_naclPublicBytesInvalidParameters(self):
+        """
+        L{_keys_pynacl.Ed25519PublicKey.public_bytes} only accepts certain parameters.
+        """
+        from cryptography.hazmat.primitives import serialization
+
+        key = _keys_pynacl.Ed25519PrivateKey.generate().public_key()
+        self.assertRaises(
+            ValueError,
+            key.public_bytes,
+            serialization.Encoding.PEM,
+            serialization.PublicFormat.Raw,
+        )
+        self.assertRaises(
+            ValueError,
+            key.public_bytes,
+            serialization.Encoding.Raw,
+            serialization.PublicFormat.PKCS1,
+        )
+
+    def test_naclPublicHash(self):
+        """
+        L{_keys_pynacl.Ed25519PublicKey.__hash__} allows instances to be hashed.
+        """
+        key = _keys_pynacl.Ed25519PrivateKey.generate().public_key()
+        d = {key: True}
+        self.assertTrue(d[key])
+
+    def test_naclPublicEquality(self):
+        """
+        L{_keys_pynacl.Ed25519PublicKey} implements equality test methods.
+        """
+        key1 = _keys_pynacl.Ed25519PrivateKey.generate().public_key()
+        key2 = _keys_pynacl.Ed25519PrivateKey.generate().public_key()
+        self.assertEqual(key1, key1)
+        self.assertNotEqual(key1, key2)
+        self.assertNotEqual(key1, bytes(key1))
+
+    def test_naclVerify(self):
+        """
+        L{_keys_pynacl.Ed25519PublicKey.verify} raises appropriate exceptions.
+        """
+        key = _keys_pynacl.Ed25519PrivateKey.generate()
+        self.assertIsInstance(key, keys.Ed25519PrivateKey)
+        signature = key.sign(b"test data")
+        self.assertIsNone(key.public_key().verify(signature, b"test data"))
+        self.assertRaises(
+            InvalidSignature, key.public_key().verify, signature, b"wrong data"
+        )
+        self.assertRaises(
+            InvalidSignature, key.public_key().verify, b"0" * 64, b"test data"
+        )
+
+
 class PersistentRSAKeyTests(unittest.TestCase):
     """
     Tests for L{keys._getPersistentRSAKey}.

diff --git a/tox.ini b/tox.ini
@@ -46,8 +46,10 @@ extras =
     nodeps: test
 
     ; We also need to include `dev_release` so that we can test our
-    ; release helpers or documentation generation.
-    alldeps: all_non_platform, dev_release
+    ; release helpers or documentation generation, and `conch_nacl` so that
+    ; we can test conch fallbacks to PyNaCl without requiring most users to
+    ; install it.
+    alldeps: all_non_platform, dev_release, conch_nacl
 
     windows: windows_platform
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		twisted.conch.ssh now has an alternative Ed25519 implementation using PyNaCl, in order to support platforms that lack OpenSSL >= 1.1.1b. The new "conch_nacl" extra has the necessary dependency.