acme_certificate: fix crash when using fullchain_dest

[felixfontein]

SUMMARY

I noticed that acme_certificate crashes when using the cryptography backend when renewing certificates if the fullchain_dest option is used, since it passes the content of the fullchain file to cryptography's load_pem_x509_certificate function. Since that one no longer calls libssl but uses its own parser (since cryptography 35.0.0), this leads to an error raised.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

acme_certificate

[felixfontein]

I'm currently wondering why CI didn't fail before, The crash happened here for me:

community.crypto/plugins/modules/acme_certificate.py

Line 779 in c0272ea

self.cert_days = self.client.backend.get_cert_days(self.fullchain_dest)

. Interestingly, I cannot reproduce this crash without my local cryptography installation, which is from a more recent main brancht version of pyca/cryptography. So maybe this isn't 35.0.0 related, but 36.0.0?

[felixfontein]

Indeed, in cryptography 35.0.0 the code is different: https://github.com/pyca/cryptography/blob/35.0.0/src/rust/src/x509.rs#L393 It's actually due to a change in the main branch which probably will show up in 36.0.0. I created pyca/cryptography#6550 to track this breaking change in cryptography.

[felixfontein]

I think we should still change this, since it's better to just pass one certificate on to load_pem_x509_certificate if we know there will be multiple (as in fullchain files) and we know we are only interested in the first certificate.

This will probably also be a problem with other modules which read PEM files, like x509_certificate_info. While cryptography's new behavior is definitely better for modules such as x509_certificate, some users might depend on the old behavior with x509_certificate_info.

[felixfontein]

(The Zuul error is unrelated.)

[Ajpantuso]

Regarding the case where concatenated certs should be processed would it make sense to decouple from cryptography's behavior by splitting and iterating that list ourselves and just call load_pem_x509_certificate uniformly (i.e. only use it with the assumption that the target is not a chain)?

[felixfontein]

Regarding the case where concatenated certs should be processed would it make sense to decouple from cryptography's behavior by splitting and iterating that list ourselves and just call load_pem_x509_certificate uniformly (i.e. only use it with the assumption that the target is not a chain)?

Yes, but we already do that (in the cases where we actually want all the certs). The problem are the places where we are only interested in the first cert, but didn't make that explicit.

[felixfontein]

(plugins/module_utils/crypto/pem.py's split_pem_list is used for doing that in openssl_pkcs12, certificate_complete_chain, and for processing chains returned by the ACME CA.)

[Ajpantuso]

LGTM

[felixfontein]

recheck

[felixfontein]

@Ajpantuso thanks for reviewing this!

[patchback]

Backport to stable-1: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-1/51b6bb210de397cb5a3d37f9da976125a3f102f1/pr-324

Backported as #325

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.