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

Adding module which allows to complete certificate chains #44169

Merged
merged 4 commits into from
Aug 23, 2018

Conversation

felixfontein
Copy link
Contributor

SUMMARY

This module allows to complete certificate chains. Given an incomplete chain (leaf certificate, maybe also some intermediates), pointers to potential intermediate certificates, and pointers to root certificates, the module tries to form a complete chain from leaf until a root certificate.

This can for example be used to find the root certificate for certificates issued with the acme_certificate module (which allows to retrieve the intermediate certificate(s), but not the root certificate since the ACME protocol doesn't allow for that); this is sometimes needed for configuration (f.ex. for Amazon AWS ELB load balancers), or to simply verify the chain of trust with openssl verify ....

CC @resmo

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

certificate_complete_chain

ANSIBLE VERSION
2.7.0

@ansibot
Copy link
Contributor

ansibot commented Aug 15, 2018

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 community_review In order to be merged, this PR must follow the community review workflow. 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 Aug 15, 2018
@ansibot
Copy link
Contributor

ansibot commented Aug 15, 2018

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Aug 15, 2018
description:
- A concatenated set of certificates in PEM format forming a chain.
- The module will try to complete this chain.
input_chain_src:
Copy link
Contributor

Choose a reason for hiding this comment

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

not a blocker (IMHO) but having 2 params for input chain: input_chain_src and input_chain is not necessary (I know acme_certificate has also 2 but this has historical reasons). I would use only input_chain, because reading from a file could be done with a lookup plugin: `input_chain: "{{ lookup('file', '/path/filename' )}". Also remember, lookup plugins can also "read" from all kinds of things, like DBs, Vaults, kv stores.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's a good point. I'll change the module accordingly, since here the use-case "input data is on remote system" isn't very important (and the input files are small).

One question though: what happens if the contents of a file on the remote system are needed? With input_chain_src, this is easily doable, but without it? Do I have to do a fetch: ... first in that case, before I can use this module?

Copy link
Contributor

Choose a reason for hiding this comment

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

it would require to have the content into a var e.g like with the slurp module.

Copy link
Contributor

Choose a reason for hiding this comment

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

how would this work with input_chain_src?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah sorry, I meant slurp: ..., not fetch: .... And yes, just having ..._src makes this complicated.

But: for acme_certificate (and friends), I think it is different, and both parameters should be provided. The reason is that depending on how you use the module, and how your security model is set up, you do not want the private account key to leave the remote host where the module is executed. If you have to slurp it first, this condition is violated. So there, having both account_key_content and account_key_src is a Very Good Thing(tm).

But that doesn't apply to this module, since certificates are usually public enough. Also, the input size isn't huge (another potential reason for having both parameters).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@remove I've removed input_chain_src in 681a468.

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Aug 15, 2018
@felixfontein
Copy link
Contributor Author

@Spredzy Did you have a chance to look at this module?

@Spredzy
Copy link
Contributor

Spredzy commented Aug 22, 2018

@felixfontein I haven't yet. I'll try to go throught it between this afternoon and tomorrow. Sorry for that.

@felixfontein
Copy link
Contributor Author

@Spredzy No problem! Thanks a lot! :)

Copy link
Contributor

@resmo resmo left a comment

Choose a reason for hiding this comment

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

shipit

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed core_review In order to be merged, this PR must follow the core review workflow. labels Aug 22, 2018
Copy link
Contributor

@Spredzy Spredzy left a comment

Choose a reason for hiding this comment

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

shipit

@felixfontein
Copy link
Contributor Author

@Spredzy @resmo Thanks for your reviews!

@resmo resmo merged commit 0e6234a into ansible:devel Aug 23, 2018
@felixfontein felixfontein deleted the crypto-complete-chain branch August 24, 2018 05:52
@felixfontein
Copy link
Contributor Author

and thanks for merging, @resmo :)

@dagwieers dagwieers added the crypto Crypto community (ACME, openssl, letsencrypt) label Feb 7, 2019
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 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. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants