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

certificate_complete_chain: add ability to identify ed25519 complete chains #777

Merged
merged 6 commits into from
Jul 11, 2024

Conversation

gderber
Copy link
Contributor

@gderber gderber commented Jul 5, 2024

SUMMARY

This adds the ability to use the certificate_complete_chain module on ed25519 certificates.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

certificate_complete_chain

ADDITIONAL INFORMATION

Without this change, using this module with an ed25519 cert provides an error:
[WARNING]: Unknown public key type "<cryptography.hazmat.backends.openssl.ed25519._Ed25519PublicKey object at 0x7f5263b8bd90>"

Upon making the change, it works. However I do not like it. The reason I don't like it is because the "verify" method for class Ed25519PublicKey in the cryptography library is an abstractmethod with undefined functionality. I could never find where the actual functionality is defined. I don't really understand the cryptography library well, nor how this ansible module uses it. Which means more work may be needed but I am having trouble identifying what that might be.

References:

I am quite open to critics and pointers.

@felixfontein
Copy link
Contributor

Copy link
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Can you please add a changelog fragment? Thanks.

plugins/modules/certificate_complete_chain.py Outdated Show resolved Hide resolved
@felixfontein felixfontein changed the title Add ability to identify ed25519 complete chains. certificate_complete_chain: add ability to identify ed25519 complete chains Jul 5, 2024
@gderber
Copy link
Contributor Author

gderber commented Jul 10, 2024

I made the requested changes. I did a force push to my repo because I wanted to change the author / email associated with the first commit.

I am not sure I did the fragment correctly.

Also, wasn't sure if it'd go under minor change or bug fix. I elected minor change since for this specific module it's a new feature. However as the collection as a whole supports ed25519 and ed448, and this module exits with an error without this change.

Copy link

github-actions bot commented Jul 11, 2024

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and the docs are now incorporated into main:
https://ansible-collections.github.io/community.crypto/branch/main

@felixfontein felixfontein merged commit b02fb8e into ansible-collections:main Jul 11, 2024
143 checks passed
@felixfontein
Copy link
Contributor

@gderber thanks for your contribution!

@gderber
Copy link
Contributor Author

gderber commented Jul 11, 2024

You're welcome! Thank you for your patience and guidance on this.

@gderber gderber deleted the feature/chain_ed25519 branch September 17, 2024 01:59
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