ansible-collections / community.crypto

The community.crypto collection for Ansible.
https://galaxy.ansible.com/ui/repo/published/community/crypto/
Other
97 stars 86 forks source link

Lack of documentation in new recommended way of generating openssl certificates #76

Closed Akasurde closed 2 years ago

Akasurde commented 4 years ago

From @alexcernat on Jun 20, 2020 10:23

SUMMARY

I am using this code to generate self signed certificates:

- name: Verify SSL certificate
  openssl_certificate:
    provider: assertonly
    path: "{{ path_to_crt }}"
    state: present
    issuer:
      CN: "common name"
      O: "organization"
    subject:
      CN: "{{ ansible_fqdn }}"
    valid_in: "2592000" # 30 days
  delegate_to: localhost
  ignore_errors: True
  register: verify_cert

- name: Generate SSL certificate
  my_ownca_generate:
    common_name: "{{ ansible_fqdn }}"
    key_path: "{{ path_to_key }}"
    cert_path: "{{ path_to_crt }}"
  delegate_to: localhost
  when: verify_cert.failed

First step is checking the actual certificate, and if it doesn't exists or is not valid (CA checks, CN checks, validity etc.), then the certificate is generated in step 2. The fact that I am using my own module to generate the certificate is irrelevant, the fact is that I need to run step 2 (generation) only if step 1 (checking) fails (or maybe if you have a better approach ...?)

From ansible 2.9 using openssl_certificate to check the validity of a certificate is deprecated (and set to be removed in 2.13 IIRC), the documentation suggest that I should use openssl_certificate_info to check different certificate parameters, but only an "assert" method is presented in the docs.

How can I translate that "when: verify_cert.failed" from step 2 in order to work with the new openssl_certificate_info module ? I think that such an example should be provided in the docs.

Also, as a feature request, I believe that a "serial" parameter should be included in the ownca module, or at least increase somehow the "randomness" of that serial number. Last time I've check the code, IIRC the serial was between 1 and 65535, which is not quite so "random".

ISSUE TYPE
COMPONENT NAME
ANSIBLE VERSION
ansible 2.9.9
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/dist-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.16 (default, Oct 10 2019, 22:02:15) [GCC 8.3.0]
CONFIGURATION
OS / ENVIRONMENT
ADDITIONAL INFORMATION

Copied from original issue: ansible/ansible#70193

Akasurde commented 4 years ago

cc @alexcernat

felixfontein commented 4 years ago

assert simply tests whether all conditions are true (check its documentation). So you can simply combine all the conditions you have with and.

About serial numbers: if you happen to use the deprecated pyOpenSSL backend, the serial numbers are between 1000 and 99999. If you use the (default) cryptography backend, you get ~20 bytes of randomness. Also, the random number generator used for the cryptography backend (os.urandom) has a much higher quality than the one used in the pyOpenSSL backend.

alexcernat commented 4 years ago

indeed, the new default cryptography backend generates real random enough serial numbers, just tested it, thanks for the hint; I can forget about my signing bash script, as long as the generated certificate seems to be the same

but I still don't get used to the new approach, it seems to me counter-intuitive (using the deprecated way to check a certificate you just define all required fields in a declarative way and execute later tasks only if the checking task failed); usually I try to change asap any deprecated issue into the new recommended way, but this time ... nope

another idea for ownca: sometimes you want to select only a few of fields in CSR (when the CSR is generated elsewhere); for example: almost any CA will use only the CN from you CSR (for a standard DV certificate) ... no country, no organization, no locality as long as you cannot change an "external" CSR (because it's signed with a private key), the only "ugly" know hack is to extract the public key from CSR and to create new fake key / fake csr and then sign with openssl -force_pubkey; ugly but it works

I looked into the code (openssl_certificate module) and it is very possible to force the public key or maybe to select only some information from CSR ... any plans to implement this in the near or distant future ?

MarkusTeufelberger commented 4 years ago

If I understand you correctly, you want the ownca provider to have a lot more fields available to potentially override things in the CSR that's used as input?

That's one way to do it, I have worked around this issue by generating a test certificate from a given CSR, asserting on its contents and only then generating the real one. There are already a lot of parameters to x509_certificate, but that shouldn't stop us from implementing useful features.

alexcernat commented 4 years ago

this is a snippet from an ansible module written in bash which does the job:

"$OPENSSL" x509 -req -sha256 -days 730 \ -in <("$OPENSSL" req -newkey rsa:2048 -nodes -sha256 -keyout /dev/null -subj "$subj" 2>/dev/null) \ -out "$cert_path" -force_pubkey <("$OPENSSL" req -in "$csr_path" -noout -pubkey) \ -CA "$CA_certificate" -CAkey "$CA_key" -set_serial "0x$($OPENSSL rand -hex 16)" \ -extfile <(cat "$CA_conf" <(printf "\n[SAN]\nDNS = $common_name\nIP = $ip_address\n")) -extensions server_cert

I know, many subshells and also the last line could be improved for openssl 1.1.x; some say it's a "ugly hack", but it works when you want to filter what data to put in the certificate and you don't have the private key to generate a new CSR (i.e. for a HP iLO system)

like I said, the idea is to extract the public key from the CSR and to force generating a new independent certificate but with the extracted key as its public key

of course it can be written as a command or shell module invocation, but IIRC the ansible philosophy is to use commands only as a last resort, if there isn't any other solution

felixfontein commented 4 years ago

@MarkusTeufelberger I sometimes think we should split the openssl_certificate module into smaller modules (one for each provider). The number of options is already very high (even though it will get better when assertonly is gone), and adding a lot more... will make it mainly harder to maintain and to use. What do you think?

@MarkusTeufelberger @alexcernat I see that using _info modules + assert is definitely more annoying in some use-cases than the assertonly provider. The reason we deprecated it was a) it would be better suited as a different module, and b) the Ansible core team didn't want "validate" modules. Now that we moved to a collection, nobody would hinder us to add a validation-only module.

felixfontein commented 4 years ago

@MarkusTeufelberger ping :)

MarkusTeufelberger commented 4 years ago

I'm semi-comfortable with the [foo]_info + assert pattern by now. I think it's more interesting to see where stuff is headed once assertonly is gone and a few more features like the one requested here (more control over every field that a CA has control over instead of trusting/requiring the CSR) have made it in instead. One module per provider might be nice, I'm just not too sure about the actual benefit. after all, it would break plays yet again. OTOH there might be a tangible benefit of only having to read the documentation of the things you're actually able to influence. I'd hold out for a split for roughly 5 providers though as a gut feeling.

felixfontein commented 2 years ago

The assertonly provider has been removed from main in 5f1efb6f7e9e86212868d82a0cb9e735ffd649b1. There's still an example in the x509_certificate docs which shows how to convert an assertonly usage: https://github.com/ansible-collections/community.crypto/blob/5f1efb6f7e9e86212868d82a0cb9e735ffd649b1/plugins/modules/x509_certificate.py#L147-L207

felixfontein commented 2 years ago

As mentioned in #128, I think we need to come to a final conclusion here (or semi-final, as we can still add a new module later on).

I personally never used the assertonly provider controlling when to re-create certificates, and I don't think it's really needed. Both the ownca and selfsigned providers are mostly idempotent, so if you provide them with a CSR which doesn't match the certificate, they will recreate it. Triggering that with a "set force to true if the certificate expires in less then X time units", which is easy to do with the x509_certificate_info module, should be all that's needed. (Also since - for this situation - conveniently, the providers ignore the validity timestamps for idempotency... see #295.)

So basically to me, using assertonly for renewal looks more like a workaround than leveraging the (almost-)idempotency of the module - with only needing the _info module for handling expiry.

What do you all think?

felixfontein commented 2 years ago

Ok, it looks like nobody is interested enough in this anymore. I guess we'll continue without a replacement then...

MarkusTeufelberger commented 2 years ago

Well, my view hasn't changed, I'm still fine with assertonly gone and using [foo]_info + assert for such use cases (e.g. when you want to be more cautious about creating new certificates that could be potentially overwriting ones you still need or when using some custom modules).

The other issue (being able to ignore/overwrite values in a CSR) might still be a useful feature if you expect to handle CSRs coming from users with all kinds of garbage inside. I'm unsure if there's a way to kinda re-package a CSR (e.g. you get a weird one: create one with proper settings using Ansible instead, plop in the public key of the weird one and just don't sign it, then create a certificate from it ignoring the missing/invalid signature) that might be a bit more feasible than adding nearly all CSR fields to ownca as well...

felixfontein commented 2 years ago

The other issue (being able to ignore/overwrite values in a CSR) might still be a useful feature if you expect to handle CSRs coming from users with all kinds of garbage inside. I'm unsure if there's a way to kinda re-package a CSR (e.g. you get a weird one: create one with proper settings using Ansible instead, plop in the public key of the weird one and just don't sign it, then create a certificate from it ignoring the missing/invalid signature) that might be a bit more feasible than adding nearly all CSR fields to ownca as well...

I think we should move this to a separate issue. Re-packing CSRs doesn't work, since you need the private key to sign them. I guess what you could do is create a new CSR with the fields you want (and another private key / signature), and then 'merge' these in the ownca process so that you copy the public key from the original CSR, and everything else from the new CSR. That's kind of a hack though...

felixfontein commented 2 years ago

I created #320 for this aspect of this issue.

felixfontein commented 2 years ago

Since 2.0.0 has been released without assertonly, and we have a new issue for the remaining content, I'm closing this. Thanks everyone!