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 openssl_privatekey_info module #54845

Merged
merged 28 commits into from
Apr 8, 2019

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Adds a new module openssl_privatekey_info module, similar to openssl_certificate_info in #54709.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

openssl_privatekey_info

@ansibot
Copy link
Contributor

ansibot commented Apr 4, 2019

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.8 This issue/PR affects Ansible v2.8 crypto Crypto community (ACME, openssl, letsencrypt) module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_module This PR includes a new module. new_plugin This PR includes a new plugin. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests. labels Apr 4, 2019
@ansibot
Copy link
Contributor

ansibot commented Apr 4, 2019

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with 1 error:

lib/ansible/modules/crypto/openssl_privatekey_info.py:0:0: missing documentation (or could not parse documentation): expected string or buffer

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/crypto/openssl_privatekey_info.py:0:0: missing documentation (or could not parse documentation): expected string or buffer

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/crypto/openssl_privatekey_info.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/crypto/openssl_privatekey_info.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/crypto/openssl_privatekey_info.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test ansible-doc --python 3.8 [explain] failed with 1 error:

lib/ansible/modules/crypto/openssl_privatekey_info.py:0:0: missing documentation (or could not parse documentation): expected string or bytes-like object

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/crypto/openssl_privatekey_info.py:0:0: E305 DOCUMENTATION.options.return_private_key_data.description.2: expected str @ data['options']['return_private_key_data']['description'][2]. Got {'WARNING': "you have to make sure that private key data isn't accidentally logged!"}
lib/ansible/modules/crypto/openssl_privatekey_info.py:304:0: E403 Type comparison using type() found. Use isinstance() instead
lib/ansible/modules/crypto/openssl_privatekey_info.py:307:0: E403 Type comparison using type() found. Use isinstance() instead

click here for bot help

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Apr 4, 2019
@felixfontein felixfontein reopened this Apr 4, 2019
@felixfontein felixfontein reopened this Apr 4, 2019
@felixfontein felixfontein reopened this Apr 4, 2019
@felixfontein felixfontein reopened this Apr 4, 2019
@felixfontein felixfontein changed the title [WIP] Add openssl_privatekey_info module Add openssl_privatekey_info module Apr 4, 2019
@felixfontein
Copy link
Contributor Author

I had to access OpenSSL directly for getting information on pyOpenSSL private keys (newer versions allow to get hold of a cryptography private key, which makes extracting information easier). But I think that now it works well enough, and apparently also for older OpenSSL versions (Ubuntu 16.04 and FreeBSD 11.1 have 1.0.x, OSX has 0.9.8zg).

ready_for_review

@ansibot
Copy link
Contributor

ansibot commented Apr 4, 2019

@MarkusTeufelberger @Xyon @gdelpierre @japokorn @john-westcott-iv @lolcube @mgruener @thomwiggers

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. labels Apr 4, 2019
@felixfontein
Copy link
Contributor Author

@MarkusTeufelberger I've added that check. I found out that cryptography does not offer such checks at all (it also doesn't call the OpenSSL RSA_check_key() function), so I implemented that by directly calling the OpenSSL function. That won't work if cryptography ever supports a non-OpenSSL backend, but that will also break some other stuff (like for openssl_certificate_info). I'll probably try to get cryptography extended once all the major _info modules are done and I know what else is missing :)

@ansibot
Copy link
Contributor

ansibot commented Apr 6, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/crypto/openssl_privatekey_info.py:265:47: undefined-variable Undefined variable 'text_data'

click here for bot help

@felixfontein
Copy link
Contributor Author

ready_for_review

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Apr 7, 2019
return True


def _is_cryptography_key_consistent(key, key_public_data, key_private_data):
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe these functions should go into module_utils/crypto? Seem useful for CSRs and certificates (maybe even SSH keys?) too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean the ones above this? Makes sense. I've added a commit for this; I've kept _check_dsa_consistency and _is_cryptography_key_consistent, though, since they are (at the moment) private key specific, and the latter specific to cryptography. We can still move them when we actually need them in other modules as well (and do some necessary generalizations then).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the ones that verify properties of private/public keys.

I'm not 100% sure what would happen in the selfsigned and ownca providers if used with a keypair that violates these specs, but I agreee, that can be fixed later.

@MarkusTeufelberger
Copy link
Contributor

Looks great.
Let's

shipit

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Apr 7, 2019
@mkrizek mkrizek merged commit 7a16703 into ansible:devel Apr 8, 2019
@felixfontein felixfontein deleted the openssl_privatekey_info branch April 8, 2019 08:12
@felixfontein
Copy link
Contributor Author

@thomwiggers @MarkusTeufelberger thanks for reviewing!
@mkrizek thanks for merging!

@ansible ansible locked and limited conversation to collaborators Jul 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 crypto Crypto community (ACME, openssl, letsencrypt) module This issue/PR relates to a module. new_module This PR includes a new module. new_plugin This PR includes a new plugin. shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants