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

Improve error handling with different OpenSSL versions #672

Conversation

vzhestkov
Copy link
Contributor

What does this PR do?

Backports saltstack/salt#66818

With the most recent versions of cryptography module the exception value which is checked here https://github.com/saltstack/salt/blob/246d0664577ef72da8bd1f0c4dff0d18b4428b23/salt/utils/x509.py#L704 is different.
The latest version of cryptography is returning https://github.com/pyca/cryptography/blob/932b8a3f67810140a6e178f7b676e1cb9c3585b1/src/rust/src/backend/utils.rs#L463

It could also be returned with the lower version of cryptography depending on the combination with the OpenSSL version it's used with.

What issues does this PR fix or reference?

Tracks: https://github.com/SUSE/spacewalk/issues/24859

Previous Behavior

x509.private_key_managed state function could fail with the comment Could not load PEM-encoded private key
The following tests could fail as well:

tests/pytests/functional/states/test_x509_v2.py::test_private_key_managed_passphrase_changed_overwrite
tests/pytests/functional/states/test_x509_v2.py::test_private_key_managed_passphrase_changed_not_overwrite

New Behavior

No test fails and x509.private_key_managed state with most recent cryptography or some other OpenSSL versions which can produce different errors on such cases.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes/No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@vzhestkov vzhestkov force-pushed the openSUSE/fix/3006.6/better-error-handling-x509 branch from 666019a to 1e89217 Compare August 27, 2024 09:35
@vzhestkov
Copy link
Contributor Author

Commit f0a69e9 is different from the upstream PR, but it only makes sense for outdated versions of OpenSSL, like the ones used in CentOS7 and SLE12, so it doesn't make any sense to try to push it upstream.

Copy link
Contributor

@m-czernek m-czernek left a comment

Choose a reason for hiding this comment

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

+1, LGTM

@vzhestkov vzhestkov changed the title Make error checking of x509 compatible with some combinations of cryptography and OpenSSL Improve error handling on different OpenSSL versions Aug 30, 2024
@vzhestkov vzhestkov changed the title Improve error handling on different OpenSSL versions Improve error handling with different OpenSSL versions Aug 30, 2024
@vzhestkov vzhestkov merged commit 4e22642 into openSUSE/release/3006.0 Aug 30, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants