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 PEM identification #628

Merged
merged 1 commit into from
Jun 27, 2023
Merged

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Fixes #627.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/module_utils/crypto/pem.py

@felixfontein
Copy link
Contributor Author

@MarkusTeufelberger @Ajpantuso if you have time to review this, a review would be appreciated :)

Copy link
Collaborator

@Ajpantuso Ajpantuso left a comment

Choose a reason for hiding this comment

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

LGTM

@MarkusTeufelberger
Copy link
Contributor

Seems weird that cryptography does not really have code for this. :-/

The "extract_first_pem()" code seems to be in https://docs.rs/pem/latest/pem/ and maybe also a little bit exposed in https://github.com/pyca/cryptography/blob/35219339d937b35435813d0995b5573a1d665cf2/src/rust/src/x509/common.rs#L16, but I'm not sure if that's much better than just rolling our own for now.

LGTM

@felixfontein
Copy link
Contributor Author

@MarkusTeufelberger it would be nice if it would expose that... Unfortunately even if they changed that now, it would take years until we could fully rely on it and stop having our own code for it :)

@felixfontein felixfontein merged commit 83af72a into ansible-collections:main Jun 27, 2023
131 checks passed
@felixfontein felixfontein deleted the pem branch June 27, 2023 15:35
@felixfontein
Copy link
Contributor Author

@Ajpantuso @MarkusTeufelberger thanks a lot for reviewing this!

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.

x509_certificate_info detects a PEM certificate as a DER certificate
3 participants