Repository: pyca/pyopenssl
Pull request #1255 (2023-09-26 20:00:24)

Deprecate X509Extension
Closes https://github.com/pyca/pyopenssl/issues/1249

reaperhulk: This needs changelog information and some prose in the documentation to give people instructions on how to do this going forward.
facutuesca: > This needs changelog information and some prose in the documentation to give people instructions on how to do this going forward.

@reaperhulk I added an entry in the changelog. I didn't see any mention in the documentation about the other deprecations, so I'm not sure where to put the prose. Do you have an example?
reaperhulk: I don't have an example and the pyOpenSSL docs don't really lend themselves well to this. We already have a note at the top of the crypto module so I think that's okay for now. 

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 88a4037..8788405 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -25,6 +25,7 @@ Deprecations:
 - Deprecated ``OpenSSL.crypto.Revoked``
 - Deprecated ``OpenSSL.crypto.load_crl`` and ``OpenSSL.crypto.dump_crl``
 - Deprecated ``OpenSSL.crypto.sign`` and ``OpenSSL.crypto.verify``
+- Deprecated ``OpenSSL.crypto.X509Extension``
 
 Changes:
 ^^^^^^^^
diff --git a/src/OpenSSL/crypto.py b/src/OpenSSL/crypto.py
index 8038ff1..8ee4b94 100644
--- a/src/OpenSSL/crypto.py
+++ b/src/OpenSSL/crypto.py
@@ -941,6 +941,19 @@ class X509Extension:
         return _ffi.buffer(char_result, result_length)[:]
 
 
+_X509ExtensionInternal = X509Extension
+utils.deprecated(
+    X509Extension,
+    __name__,
+    (
+        "X509Extension support in pyOpenSSL is deprecated. You should use the "
+        "APIs in cryptography."
+    ),
+    DeprecationWarning,
+    name="X509Extension",
+)
+
+
 class X509Req:
     """
     An X.509 certificate signing requests.
@@ -1063,7 +1076,9 @@ class X509Req:
 
         return name
 
-    def add_extensions(self, extensions: Iterable[X509Extension]) -> None:
+    def add_extensions(
+        self, extensions: Iterable[_X509ExtensionInternal]
+    ) -> None:
         """
         Add extensions to the certificate signing request.
 
@@ -1077,7 +1092,7 @@ class X509Req:
         stack = _ffi.gc(stack, _lib.sk_X509_EXTENSION_free)
 
         for ext in extensions:
-            if not isinstance(ext, X509Extension):
+            if not isinstance(ext, _X509ExtensionInternal):
                 raise ValueError("One of the elements is not an X509Extension")
 
             # TODO push can fail (here and elsewhere)
@@ -1086,7 +1101,7 @@ class X509Req:
         add_result = _lib.X509_REQ_add_extensions(self._req, stack)
         _openssl_assert(add_result == 1)
 
-    def get_extensions(self) -> List[X509Extension]:
+    def get_extensions(self) -> List[_X509ExtensionInternal]:
         """
         Get X.509 extensions in the certificate signing request.
 
@@ -1106,7 +1121,7 @@ class X509Req:
         )
 
         for i in range(_lib.sk_X509_EXTENSION_num(native_exts_obj)):
-            ext = X509Extension.__new__(X509Extension)
+            ext = _X509ExtensionInternal.__new__(_X509ExtensionInternal)
             extension = _lib.X509_EXTENSION_dup(
                 _lib.sk_X509_EXTENSION_value(native_exts_obj, i)
             )
@@ -1600,7 +1615,9 @@ class X509:
         """
         return _lib.X509_get_ext_count(self._x509)
 
-    def add_extensions(self, extensions: Iterable[X509Extension]) -> None:
+    def add_extensions(
+        self, extensions: Iterable[_X509ExtensionInternal]
+    ) -> None:
         """
         Add extensions to the certificate.
 
@@ -1609,14 +1626,14 @@ class X509:
         :return: ``None``
         """
         for ext in extensions:
-            if not isinstance(ext, X509Extension):
+            if not isinstance(ext, _X509ExtensionInternal):
                 raise ValueError("One of the elements is not an X509Extension")
 
             add_result = _lib.X509_add_ext(self._x509, ext._extension, -1)
             if not add_result:
                 _raise_current_error()
 
-    def get_extension(self, index: int) -> X509Extension:
+    def get_extension(self, index: int) -> _X509ExtensionInternal:
         """
         Get a specific extension of the certificate by index.
 
@@ -1630,7 +1647,7 @@ class X509:
 
         .. versionadded:: 0.12
         """
-        ext = X509Extension.__new__(X509Extension)
+        ext = _X509ExtensionInternal.__new__(_X509ExtensionInternal)
         ext._extension = _lib.X509_get_ext(self._x509, index)
         if ext._extension == _ffi.NULL:
             raise IndexError("extension index out of bounds")


Test patch:
diff --git a/tests/test_crypto.py b/tests/test_crypto.py
index 4d093e2..ac3d669 100644
--- a/tests/test_crypto.py
+++ b/tests/test_crypto.py
@@ -27,7 +27,6 @@ from OpenSSL.crypto import (
     X509,
     Error,
     PKey,
-    X509Extension,
     X509Name,
     X509Req,
     X509Store,
@@ -54,6 +53,7 @@ with pytest.warns(DeprecationWarning):
         PKCS12,
         NetscapeSPKI,
         Revoked,
+        X509Extension,
         dump_crl,
         load_crl,
     )
diff --git a/tests/test_ssl.py b/tests/test_ssl.py
index a23e162..6cd882e 100644
--- a/tests/test_ssl.py
+++ b/tests/test_ssl.py
@@ -49,7 +49,6 @@ from OpenSSL.crypto import (
     TYPE_RSA,
     X509,
     PKey,
-    X509Extension,
     X509Store,
     dump_certificate,
     dump_privatekey,
@@ -57,6 +56,10 @@ from OpenSSL.crypto import (
     load_certificate,
     load_privatekey,
 )
+
+with pytest.warns(DeprecationWarning):
+    from OpenSSL.crypto import X509Extension
+
 from OpenSSL.SSL import (
     DTLS_METHOD,
     MODE_RELEASE_BUFFERS,