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

Add support for cryptography CRLs to X509Store #1252

Merged
merged 2 commits into from Oct 23, 2023

Conversation

facutuesca
Copy link
Contributor

As a follow up to deprecating CRL APIs (#1249), there is one remaining function that won't be deprecated but accepts the CRL type: X509Store.add_crl(crl: CRL). In order to give users an alternative once CRL is deprecated, this PR adds support to add_crl() so that it also accepts cryptography's X509.CertificateRevocationList.

This PR also adds tests for the new code path, by parametrizing existing tests that use add_crl so that the CRL being passed as a parameter is either PyOpenSSL's or cryptography's. For example:

    @pytest.mark.parametrize(
        "create_crl",
        [
            pytest.param(
                _make_test_crl,
                id="pyOpenSSL CRL",
            ),
            pytest.param(
                _make_test_crl_cryptography,
                id="cryptography CRL",
            ),
        ],
    )
    def test_verify_with_revoked(self, create_crl):
            #...
            root_crl = create_crl(self.root_cert, self.root_key, certs=[self.intermediate_cert])
            #...

@@ -2428,7 +2448,7 @@ def get_revoked(self) -> Optional[Tuple[_RevokedInternal, ...]]:
return tuple(results)
return None

def add_revoked(self, revoked: Revoked) -> None:
def add_revoked(self, revoked: _RevokedInternal) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this fail mypy or are we avoiding deprecation warnings?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for the docs, see the current type signature for this function:

add_revoked(revoked: <cryptography.utils._DeprecatedValue object at 0x7f66c71ab1d0>) → None
Add a revoked (by value not reference) to the CRL structure

@reaperhulk
Copy link
Member

This still needs a changelog entry 😄

@facutuesca
Copy link
Contributor Author

This still needs a changelog entry 😄

@reaperhulk My bad, fixed!

@reaperhulk reaperhulk merged commit bbcee8d into pyca:main Oct 23, 2023
34 checks passed
@facutuesca facutuesca deleted the deprecate-crl-apis-2 branch October 23, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants