Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError when calling asdict() on a dataclass containing Certificates #5129

Closed
nabla-c0d3 opened this issue Mar 1, 2020 · 3 comments · Fixed by #5318
Closed

TypeError when calling asdict() on a dataclass containing Certificates #5129

nabla-c0d3 opened this issue Mar 1, 2020 · 3 comments · Fixed by #5318

Comments

@nabla-c0d3
Copy link

  • Versions: Python 3.7.4 with cryptography 2.8.
  • How you installed cryptography: via a Pipfile in my project

I am using Python 3.7's dataclasses to pass around data, including certificates parsed using cryptography.x509.load_pem_x509_certificate(). The dataclass module has a utility function called asdict() which turns a dataclass into a Python dictionary with the same fields (https://docs.python.org/3/library/dataclasses.html#dataclasses.asdict).

When calling asdict() on a dataclass instance that contains a Certificate, it crashes. The fix I found was to add a __deepcopy__() method to the Certificate class (specifically cryptography.hazmat.backends.openssl.x509._Certificate).

I don't know if the lack of support for __deepcopy__() is by design or not. If not I'd be willing to create a pull request with the fix (but I don't really understand all the implications of adding the __deepcopy__() method to this class).

Steps to reproduce:

The following file contains a test case that will trigger the crash and a test case with the fix:

from dataclasses import dataclass, asdict
from pathlib import Path

from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.backends.openssl.x509 import _Certificate
from cryptography.hazmat.primitives.serialization import Encoding
from cryptography.x509 import Certificate, load_pem_x509_certificate


# Any PEM certificate will work
path_to_a_cert = Path(__file__).absolute().parent / ".." / ".." / "certificates" / "github.com.pem"
certificate = load_pem_x509_certificate(path_to_a_cert.read_bytes(), default_backend())


@dataclass
class ClassThatContainsCertificates:
    certificate: Certificate


class TestDataclassWithCertificate:

    def test_crashes(self):
        value = ClassThatContainsCertificates(certificate=certificate)

        # It crashes here
        value_as_dict = asdict(value)

    def test_succeeds(self):
        # Monkeypatch to add __deepcopy__() to cryptography.hazmat.backends.openssl.x509._Certificate
        def _deepcopy_method_for_x509_certificate(inner_self, memo: str) -> Certificate:
            return load_pem_x509_certificate(inner_self.public_bytes(Encoding.PEM), backend=default_backend())

        _Certificate.__deepcopy__ = _deepcopy_method_for_x509_certificate

        value = ClassThatContainsCertificates(certificate=certificate)

        # No crash
        value_as_dict = asdict(value)
        

The stack trace for the crash is the following:

tests\test_crash.py:21 (TestDataclassWithCertificate.test_crashes)
self = <tests.plugins_tests.certificate_info.test_crash.TestDataclassWithCertificate object at 0x00000211DBD8F348>

    def test_crashes(self):
        value = ClassThatContainsCertificates(certificate=certificate)
    
        # It crashes here
>       value_as_dict = asdict(value)

tests\test_crash.py:26: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
..\..\..\AppData\Local\Programs\Python\Python37\Lib\dataclasses.py:1044: in asdict
    return _asdict_inner(obj, dict_factory)
..\..\..\AppData\Local\Programs\Python\Python37\Lib\dataclasses.py:1051: in _asdict_inner
    value = _asdict_inner(getattr(obj, f.name), dict_factory)
..\..\..\AppData\Local\Programs\Python\Python37\Lib\dataclasses.py:1085: in _asdict_inner
    return copy.deepcopy(obj)
..\..\..\.virtualenvs\sslyze-VjIqHxGI\lib\copy.py:180: in deepcopy
    y = _reconstruct(x, memo, *rv)
..\..\..\.virtualenvs\sslyze-VjIqHxGI\lib\copy.py:280: in _reconstruct
    state = deepcopy(state, memo)
..\..\..\.virtualenvs\sslyze-VjIqHxGI\lib\copy.py:150: in deepcopy
    y = copier(x, memo)
..\..\..\.virtualenvs\sslyze-VjIqHxGI\lib\copy.py:240: in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
..\..\..\.virtualenvs\sslyze-VjIqHxGI\lib\copy.py:180: in deepcopy
    y = _reconstruct(x, memo, *rv)
..\..\..\.virtualenvs\sslyze-VjIqHxGI\lib\copy.py:280: in _reconstruct
    state = deepcopy(state, memo)
..\..\..\.virtualenvs\sslyze-VjIqHxGI\lib\copy.py:150: in deepcopy
    y = copier(x, memo)
..\..\..\.virtualenvs\sslyze-VjIqHxGI\lib\copy.py:240: in _deepcopy_dict
    y[deepcopy(key, memo)] = deepcopy(value, memo)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

x = <CompiledFFI object at 0x00000211DAC9D318>
memo = {2275709052744: <cryptography.hazmat.bindings.openssl.binding.Binding object at 0x00000211DBD8F7C8>, 2275709053768: <c...binding': <cryptography.hazmat.bindings.openssl.binding.Binding object at 0x00000211DBD8F7C8>}, 2275725107912: {}, ...}
_nil = []

    def deepcopy(x, memo=None, _nil=[]):
        """Deep copy operation on arbitrary Python objects.
    
        See the module's __doc__ string for more info.
        """
    
        if memo is None:
            memo = {}
    
        d = id(x)
        y = memo.get(d, _nil)
        if y is not _nil:
            return y
    
        cls = type(x)
    
        copier = _deepcopy_dispatch.get(cls)
        if copier:
            y = copier(x, memo)
        else:
            try:
                issc = issubclass(cls, type)
            except TypeError: # cls is not a class (old Boost; see SF #502085)
                issc = 0
            if issc:
                y = _deepcopy_atomic(x, memo)
            else:
                copier = getattr(x, "__deepcopy__", None)
                if copier:
                    y = copier(memo)
                else:
                    reductor = dispatch_table.get(cls)
                    if reductor:
                        rv = reductor(x)
                    else:
                        reductor = getattr(x, "__reduce_ex__", None)
                        if reductor:
>                           rv = reductor(4)
E                           TypeError: can't pickle CompiledFFI objects
@reaperhulk
Copy link
Member

Serializing and re-parsing is the right path for generating a new Certificate object like this. However, to be safe you should use the cert._backend property rather than default_backend() since you want to load it using the same backend that originally parsed it.

I'm reluctant to land code like this in cryptography because it feels like it's the wrong layer to fix it (and to be consistent we'd need to do it to every object + add tests for it, which is a lot of objects!) but I'd like to hear what @alex thinks as well.

@alex
Copy link
Member

alex commented Jul 19, 2020

Defining an __deepcopy__ on certificate that's just return self seems reasonable to me. Certificates are immutable, so their deepcopy can just be self.

@nabla-c0d3
Copy link
Author

Thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants