Repository: pyca/pyopenssl
Pull request #1348 (2024-08-06 12:28:13)

Deprecate passing `PKey` instances to `use_privatekey`
Allow passing cryptography keys instead.

Refs #1321

alex: Not on older pythons

On Tue, Aug 6, 2024, 9:30 AM Maximilian Hils ***@***.***>
wrote:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/OpenSSL/crypto.py
> <https://github.com/pyca/pyopenssl/pull/1348#discussion_r1705544665>:
>
> > @@ -84,6 +84,14 @@
>  ]
>
>
> +_PrivateKey = Union[
> +    dsa.DSAPrivateKey,
> +    ec.EllipticCurvePrivateKey,
> +    ed25519.Ed25519PrivateKey,
> +    ed448.Ed448PrivateKey,
> +    rsa.RSAPrivateKey,
> +]
> +# TODO: Turn this into `PublicKey | PrivateKey` once we use `|`.
>
> Can't we do that right away?
>
> >>> a = Union[1,2]>>> b = Union[3,4]>>> Union[a,b]typing.Union[1, 2, 3, 4]
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/pyca/pyopenssl/pull/1348#pullrequestreview-2221317782>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAAGBBY3S7BWLDZBWCFBQ3ZQDFYRAVCNFSM6AAAAABMCHLMESVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMRRGMYTONZYGI>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>

mhils: ```python
> docker run --rm -it python:3.7.0
Python 3.7.0 (default, Oct 16 2018, 07:10:55)
[GCC 6.3.0 20170516] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from typing import Union
>>> a = Union[str, int]
>>> b = Union[bool, float]
>>> Union[a,b]
typing.Union[str, int, bool, float]
```

This looks ok to me? (I've also tested 3.8-3.10)
alex: Ohhh, yeah, i guess I didn't try that. Will fix this evening

On Tue, Aug 6, 2024, 9:59 AM Maximilian Hils ***@***.***>
wrote:

> > docker run --rm -it python:3.7.0Python 3.7.0 (default, Oct 16 2018, 07:10:55)
> [GCC 6.3.0 20170516] on linuxType "help", "copyright", "credits" or "license" for more information.>>> from typing import Union>>> a = Union[str, int]>>> b = Union[bool, float]>>> Union[a,b]typing.Union[str, int, bool, float]
>
> This looks ok to me? (I've also tested 3.8-3.10)
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/pyca/pyopenssl/pull/1348#issuecomment-2271369616>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAAGBGRHDB6NUZTWY26CB3ZQDJFJAVCNFSM6AAAAABMCHLMESVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZRGM3DSNRRGY>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>


Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index e14baa0..e10c956 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -16,6 +16,7 @@ Deprecations:
 - Deprecated ``OpenSSL.rand`` - callers should use ``os.urandom()`` instead.
 - Deprecated ``OpenSSL.crypto.get_elliptic_curves`` and ``OpenSSL.crypto.get_elliptic_curve``, as well as passing the reult of them to ``OpenSSL.SSL.Context.set_tmp_ecdh``, users should instead pass curves from ``cryptography``.
 - Deprecated passing ``X509`` objects to ``OpenSSL.SSL.Context.use_certificate``, ``OpenSSL.SSL.Connection.use_certificate``, ``OpenSSL.SSL.Context.add_extra_chain_cert``, and ``OpenSSL.SSL.Context.add_client_ca``, users should instead pass ``cryptography.x509.Certificate`` instances. This is in preparation for deprecating pyOpenSSL's ``X509`` entirely.
+- Deprecated passing ``PKey`` objects to ``OpenSSL.SSL.Context.use_privatekey`` and ``OpenSSL.SSL.Connection.use_privatekey``, users should instead pass ``cryptography`` priate key instances. This is in preparation for deprecating pyOpenSSL's ``PKey`` entirely.
 
 Changes:
 ^^^^^^^^
diff --git a/src/OpenSSL/SSL.py b/src/OpenSSL/SSL.py
index 4d17a9d..e4d9ce1 100644
--- a/src/OpenSSL/SSL.py
+++ b/src/OpenSSL/SSL.py
@@ -46,6 +46,7 @@ from OpenSSL.crypto import (
     X509Store,
     _EllipticCurve,
     _PassphraseHelper,
+    _PrivateKey,
 )
 
 __all__ = [
@@ -1204,7 +1205,7 @@ class Context:
         if not use_result:
             self._raise_passphrase_exception()
 
-    def use_privatekey(self, pkey: PKey) -> None:
+    def use_privatekey(self, pkey: _PrivateKey | PKey) -> None:
         """
         Load a private key from a PKey object
 
@@ -1213,7 +1214,16 @@ class Context:
         """
         # Mirrored at Connection.use_privatekey
         if not isinstance(pkey, PKey):
-            raise TypeError("pkey must be a PKey instance")
+            pkey = PKey.from_cryptography_key(pkey)
+        else:
+            warnings.warn(
+                (
+                    "Passing pyOpenSSL PKey objects is deprecated. You "
+                    "should use a cryptography private key instead."
+                ),
+                DeprecationWarning,
+                stacklevel=2,
+            )
 
         use_result = _lib.SSL_CTX_use_PrivateKey(self._context, pkey._pkey)
         if not use_result:
@@ -2070,7 +2080,7 @@ class Connection:
         if not use_result:
             _raise_current_error()
 
-    def use_privatekey(self, pkey: PKey) -> None:
+    def use_privatekey(self, pkey: _PrivateKey | PKey) -> None:
         """
         Load a private key from a PKey object
 
@@ -2079,7 +2089,16 @@ class Connection:
         """
         # Mirrored from Context.use_privatekey
         if not isinstance(pkey, PKey):
-            raise TypeError("pkey must be a PKey instance")
+            pkey = PKey.from_cryptography_key(pkey)
+        else:
+            warnings.warn(
+                (
+                    "Passing pyOpenSSL PKey objects is deprecated. You "
+                    "should use a cryptography private key instead."
+                ),
+                DeprecationWarning,
+                stacklevel=2,
+            )
 
         use_result = _lib.SSL_use_PrivateKey(self._ssl, pkey._pkey)
         if not use_result:
diff --git a/src/OpenSSL/crypto.py b/src/OpenSSL/crypto.py
index 2cfb617..c511842 100644
--- a/src/OpenSSL/crypto.py
+++ b/src/OpenSSL/crypto.py
@@ -84,18 +84,21 @@ __all__ = [
 ]
 
 
-_Key = Union[
+_PrivateKey = Union[
     dsa.DSAPrivateKey,
-    dsa.DSAPublicKey,
     ec.EllipticCurvePrivateKey,
-    ec.EllipticCurvePublicKey,
     ed25519.Ed25519PrivateKey,
-    ed25519.Ed25519PublicKey,
     ed448.Ed448PrivateKey,
-    ed448.Ed448PublicKey,
     rsa.RSAPrivateKey,
+]
+_PublicKey = Union[
+    dsa.DSAPublicKey,
+    ec.EllipticCurvePublicKey,
+    ed25519.Ed25519PublicKey,
+    ed448.Ed448PublicKey,
     rsa.RSAPublicKey,
 ]
+_Key = Union[_PrivateKey, _PublicKey]
 StrOrBytesPath = Union[str, bytes, PathLike]
 PassphraseCallableT = Union[bytes, Callable[..., bytes]]
 


Test patch:
diff --git a/tests/test_ssl.py b/tests/test_ssl.py
index c3618fa..fa6f40a 100644
--- a/tests/test_ssl.py
+++ b/tests/test_ssl.py
@@ -727,6 +727,11 @@ class TestContext:
         context.use_certificate(cert)
         assert None is context.check_privatekey()
 
+        context = Context(SSLv23_METHOD)
+        context.use_privatekey(key.to_cryptography_key())
+        context.use_certificate(cert)
+        assert None is context.check_privatekey()
+
     def test_check_privatekey_invalid(self):
         """
         `Context.check_privatekey` raises `Error` if the `Context` instance
@@ -741,6 +746,12 @@ class TestContext:
         with pytest.raises(Error):
             context.check_privatekey()
 
+        context = Context(SSLv23_METHOD)
+        context.use_privatekey(key.to_cryptography_key())
+        context.use_certificate(cert)
+        with pytest.raises(Error):
+            context.check_privatekey()
+
     def test_app_data(self):
         """
         `Context.set_app_data` stores an object for later retrieval
@@ -2163,6 +2174,8 @@ class TestContextConnection:
         with pytest.raises(TypeError):
             ctx_or_conn.use_privatekey("")
 
+        ctx_or_conn.use_privatekey(key.to_cryptography_key())
+
     def test_use_privatekey_wrong_key(self, ctx_or_conn):
         """
         `use_privatekey` raises `OpenSSL.SSL.Error` when passed a