Use community.crypto for the certificates role by stejskalleos · Pull Request #441 · theforeman/foremanctl

stejskalleos · 2026-04-01T13:16:44Z

Use the community.crypto module
Fix re-generating certs
--reset-certificate-cname

stejskalleos · 2026-04-01T13:19:12Z

@ehelms review please.

This needs more testing on a prod-like setup. It worked on my dev setup, but I'd like to do more testing once you confirm it's a good direction.

ehelms · 2026-04-01T14:21:02Z

As there are other properties that could also change, I wonder if we would be better served by switching to the community.crypto collection and using the built-ins which I believe have idempotency designed into them.

https://docs.ansible.com/projects/ansible/latest/collections/community/crypto/

For example:

- name: Generate Private Key
  community.crypto.openssl_privatekey:
    path: /etc/ssl/private/ansible.key

- name: Generate CSR with SANs
  community.crypto.openssl_csr:
    path: /etc/ssl/csr/ansible.csr
    privatekey_path: /etc/ssl/private/ansible.key
    common_name: example.com
    subject_alt_name:
      - "DNS:example.com"
      - "DNS:://example.com"
      - "DNS:://example.com" # Adding this triggers the update

- name: Generate Certificate
  community.crypto.x509_certificate:
    path: /etc/ssl/certs/ansible.crt
    csr_path: /etc/ssl/csr/ansible.csr
    privatekey_path: /etc/ssl/private/ansible.key
    provider: selfsigned # Or 'ownca' / 'acme'

ehelms · 2026-04-07T15:23:38Z

First, thanks for doing a version of this with the community collection @stejskalleos, this helps see and think about the differences. I see the community module can generate EC certificates which is good. I am thinking ahead to PQC and this will require us to rely on cryptography to support that.

The trade-off on approaches as I see them:

Using OpenSSL directly

Control our own destiny
No additional dependencies other than openssl
Requires us to implement already solved problems like regeneration / idempotency when properties change

Using community collection

Benefits from community updates
Solves idempotency already
Dependency on python-cryptogaphy

I'd like to get more opinion on this, especially form @evgeni

evgeni · 2026-04-10T09:53:33Z

I think depending on python3-cryptography is an OK thing to do.
I wish Ansible would offer this functionality closer to the core (and not in a community collection), but that ship sails somewhere else.

stejskalleos · 2026-04-13T10:55:32Z

Task Import Manifest for organization '6d7f65b1-ae16-4ff7-bb0e-097c2353e0f7'(1e05a625-617e-43e2-a66e-2c6481fbc71a) did not succeed. Task information: ['org.candlepin.async.JobExecutionException: Failed to verify signature!'

Candlepin can't verify the signature. Should we re-regenerate manifest?

evgeni · 2026-04-13T11:02:37Z

re-generate with what? that's a static manifest that should "just work".

stejskalleos · 2026-04-13T11:20:18Z

Well, according to CI, it doesn't.

evgeni · 2026-04-13T12:16:20Z

The Candlepin logs (which are sadly not collected by sos) say:

2026-04-13 12:15:00,201 [thread=Thread-9 (ActiveMQ-client-global-threads)] [job=4028fcf39d86a075019d86c43a1e000c, job_key=ImportJob, org=30c9d1d7-a9a9-4ce2-934f-f8e7445197a0, csid=6226c86c-dcbb-47b0-9b8c-34121405d295] ERROR org.can
dlepin.sync.Importer - Recording import failure
org.candlepin.pki.SignatureFailedException: Failed to verify signature!
        at org.candlepin.pki.impl.Signer.verifySHA256WithRSAHash(Signer.java:111)
        at org.candlepin.pki.impl.Signer.verifySignature(Signer.java:73)
        at org.candlepin.sync.Importer.doExport(Importer.java:376)
        at org.candlepin.sync.Importer.loadStoredExport(Importer.java:193)
        at org.candlepin.controller.ManifestManager.importStoredManifest(ManifestManager.java:222)
        at com.google.inject.persist.jpa.JpaLocalTxnInterceptor.invoke(JpaLocalTxnInterceptor.java:66)
        at org.candlepin.async.tasks.ImportJob.execute(ImportJob.java:234)
        at org.candlepin.async.JobManager.executeJob(JobManager.java:1280)
        at org.candlepin.async.JobMessageReceiver$MessageListener.handleMessage(JobMessageReceiver.java:350)
        at org.candlepin.messaging.impl.artemis.ArtemisConsumer$ArtemisMessageForwarder.onMessage(ArtemisConsumer.java:59)
        at org.apache.activemq.artemis.core.client.impl.ClientConsumerImpl.callOnMessage(ClientConsumerImpl.java:982)
        at org.apache.activemq.artemis.core.client.impl.ClientConsumerImpl$Runner.run(ClientConsumerImpl.java:1135)
        at org.apache.activemq.artemis.utils.actors.OrderedExecutor.doTask(OrderedExecutor.java:59)
        at org.apache.activemq.artemis.utils.actors.OrderedExecutor.doTask(OrderedExecutor.java:32)
        at org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:69)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at org.apache.activemq.artemis.utils.ActiveMQThreadFactory$1.run(ActiveMQThreadFactory.java:120)
Caused by: java.security.InvalidKeyException: Wrong key usage
        at java.base/java.security.Signature.getPublicKeyFromCert(Signature.java:555)
        at java.base/java.security.Signature.initVerify(Signature.java:581)
        at org.candlepin.pki.impl.Signer.verifySHA256WithRSAHash(Signer.java:102)
        ... 17 common frames omitted

evgeni

I thought that'd fix it (that aligns with the old config) but it did not

evgeni · 2026-04-13T12:54:02Z

+    key_usage:
+      - keyCertSign
+      - cRLSign
+      - digitalSignature


adding this helped, but now I am confused, the old OpenSSL config doesn't add that to the CA cert…

I checked the original openssl.cnf, the digitalSignature is mentioned in ssl_server and ssl_client sections:

[ ssl_server ] basicConstraints = CA:FALSE nsCertType = server keyUsage = digitalSignature, keyEncipherment extendedKeyUsage = serverAuth, nsSGC, msSGC nsComment = "OpenSSL Certificate for SSL Web Server" [ ssl_client ] basicConstraints = CA:FALSE nsCertType = client keyUsage = digitalSignature, keyEncipherment extendedKeyUsage = clientAuth nsComment = "OpenSSL Certificate for SSL Client"

Sure. But this is the CA.

for comparison, this is what gets generated by the various implementations we have

old openssl code:

X509v3 extensions: X509v3 Basic Constraints: CA:TRUE Netscape Cert Type: SSL CA X509v3 Key Usage: Certificate Sign, CRL Sign X509v3 Extended Key Usage: TLS Web Server Authentication, TLS Web Client Authentication

your ansible code:

X509v3 extensions: X509v3 Key Usage: critical Digital Signature, Certificate Sign, CRL Sign X509v3 Extended Key Usage: TLS Web Server Authentication, TLS Web Client Authentication X509v3 Basic Constraints: critical CA:TRUE

old katello-certs-tools:

X509v3 Basic Constraints: CA:TRUE X509v3 Key Usage: Digital Signature, Key Encipherment, Certificate Sign, CRL Sign X509v3 Extended Key Usage: TLS Web Server Authentication, TLS Web Client Authentication Netscape Cert Type: SSL Server, SSL CA

why is key usage marked critical for your code?

aha, I think that's what fooling us here. we need digital signature, but without the usage being marked as critical, this is actually not enforced by the consumer.

yepp, just tried it out, dropping key_usage_critical: true "fixes" it too.

now, I'd argue that setting key_usage_critical: true as you did is more correct here, but not sure about the implications.

@ehelms opinions? should we aim at bug-for-bug compatibility with katello-certs-tools or rather not? (I'd prefer not)

I think aiming to be "correct" for the new certificate generation code is the way to go. What I worry about is, what happens to installer-based certificates that are inherited and continue to be managed (see #421). Will this try and trigger a regeneration of the CA?

The modules might, yes. We talked about a related issue yesterday that people should not get the CA regenerated when e.g. the expiration date changes (it is currently relative to the date the execution happens) and ended up saying that for CA-changing operations we'd require an explicit flag (something like "allow CA changes") so that the CA won't change once generated unless explicitly requested.

Oh, docs say "not valid after" is "not used to determine whether an existing certificate should be regenerated. ", cool.
(The rest still applies, ofc)

ShimShtein

I see that we have a basic test that loads the certs after the install (which is good), but I am missing tests for the regeneration use cases.
@ehelms, @evgeni What do you think would be the best place to add those?

evgeni · 2026-04-17T06:29:29Z

I see that we have a basic test that loads the certs after the install (which is good), but I am missing tests for the regeneration use cases. @ehelms, @evgeni What do you think would be the best place to add those?

We don't have a good way (today) to execute that without basically doing a "do a full deployment, ensure things work, regen certs, ensure things still work", which is rather expensive (IMHO).
On the other hand, this PR does not ("officially") introduce any regeneration features, just transforms our custom OpenSSL usage to the community.crypto collection which will allow us to have proper regeneration workflows.

Therefor I'd leave the tests a bit for the future. We probably can build something similar to https://github.com/theforeman/foremanctl/blob/master/tests/unit/foreman_ceritificate_check_test.py by writing dedicated tests for the certificates role and run that outside the regular "deploy a full foreman" testing pipeline.

* Use the community.crypto module * Fix re-generating certs * --reset-certificate-cname

ShimShtein · 2026-04-19T14:20:13Z

Therefor I'd leave the tests a bit for the future. We probably can build something similar to https://github.com/theforeman/foremanctl/blob/master/tests/unit/foreman_ceritificate_check_test.py by writing dedicated tests for the certificates role and run that outside the regular "deploy a full foreman" testing pipeline.

I would expect a test that will assert regeneration of certificates when critical properties are changed (like the SANs). We may also test that the certificates generated by the CA and issue roles are passing the certificate_check assertions. As you have said - it's for the future, but really needed.

pr-processor Bot added the Not yet reviewed label Apr 1, 2026

stejskalleos mentioned this pull request Apr 1, 2026

Certificates should be regenerated if their properties change #401

Open

stejskalleos requested a review from ehelms April 1, 2026 13:17

stejskalleos force-pushed the ls/regenerate_certs branch from 905fb90 to e8b4ff3 Compare April 1, 2026 13:23

ehelms reviewed Apr 1, 2026

View reviewed changes

Comment thread docs/certificates.md Outdated

pr-processor Bot removed the Not yet reviewed label Apr 1, 2026

stejskalleos force-pushed the ls/regenerate_certs branch from e8b4ff3 to 62bcef4 Compare April 7, 2026 12:26

stejskalleos changed the title ~~Regenerate certs when --certificate-cname is present~~ Use community.crypto for the certificates role Apr 7, 2026

stejskalleos force-pushed the ls/regenerate_certs branch from 62bcef4 to 30ea6d6 Compare April 7, 2026 12:28

stejskalleos marked this pull request as ready for review April 7, 2026 12:28

ehelms reviewed Apr 7, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/ca.yml

ehelms reviewed Apr 7, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/ca.yml Outdated

ehelms reviewed Apr 7, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/issue.yml Outdated

stejskalleos marked this pull request as draft April 10, 2026 06:41

evgeni reviewed Apr 10, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/ca.yml Outdated

evgeni reviewed Apr 10, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/ca.yml Outdated

stejskalleos force-pushed the ls/regenerate_certs branch from 30ea6d6 to 0ffe0c1 Compare April 13, 2026 06:25

stejskalleos marked this pull request as ready for review April 13, 2026 06:25

stejskalleos force-pushed the ls/regenerate_certs branch from 0ffe0c1 to cdb5977 Compare April 13, 2026 06:32

evgeni reviewed Apr 13, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/ca.yml

Comment thread src/roles/certificates/tasks/issue.yml

evgeni reviewed Apr 13, 2026

View reviewed changes

stejskalleos mentioned this pull request Apr 14, 2026

foremanctl deploy --certificate-cname SatelliteQE/robottelo#21309

Merged

evgeni reviewed Apr 14, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/ca.yml Outdated

ShimShtein reviewed Apr 15, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/issue.yml Outdated

Comment thread src/roles/certificates/tasks/issue.yml Outdated

stejskalleos force-pushed the ls/regenerate_certs branch from 8f50c35 to f1f7159 Compare April 15, 2026 11:29

evgeni reviewed Apr 15, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/issue.yml Outdated

evgeni approved these changes Apr 15, 2026

View reviewed changes

Comment thread src/roles/httpd/tasks/main.yml

ehelms reviewed Apr 15, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/ca.yml Outdated

stejskalleos force-pushed the ls/regenerate_certs branch from 1a89995 to f167c76 Compare April 16, 2026 08:48

ehelms approved these changes Apr 16, 2026

View reviewed changes

ShimShtein reviewed Apr 16, 2026

View reviewed changes

Comment thread src/roles/certificates/tasks/issue.yml Outdated

Use community.crypto for the certificates role

c2f74a3

* Use the community.crypto module * Fix re-generating certs * --reset-certificate-cname

stejskalleos force-pushed the ls/regenerate_certs branch from b8b377b to c2f74a3 Compare April 17, 2026 07:11

evgeni approved these changes Apr 17, 2026

View reviewed changes

evgeni enabled auto-merge (rebase) April 17, 2026 07:14

evgeni merged commit 4e45414 into theforeman:master Apr 17, 2026
11 checks passed

evgeni mentioned this pull request Apr 29, 2026

don't install foremanctl from copr but from the regular repository SatelliteQE/robottelo#21440

Open

Conversation

stejskalleos commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stejskalleos commented Apr 1, 2026

Uh oh!

Uh oh!

ehelms commented Apr 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ehelms commented Apr 7, 2026

Using OpenSSL directly

Using community collection

Uh oh!

Uh oh!

Uh oh!

evgeni commented Apr 10, 2026

Uh oh!

stejskalleos commented Apr 13, 2026

Uh oh!

evgeni commented Apr 13, 2026

Uh oh!

stejskalleos commented Apr 13, 2026

Uh oh!

evgeni commented Apr 13, 2026

Uh oh!

evgeni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

evgeni Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

stejskalleos Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

evgeni Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

evgeni Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

evgeni Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

evgeni Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

ehelms Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

evgeni Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

evgeni Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShimShtein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

evgeni commented Apr 17, 2026

Uh oh!

Uh oh!

ShimShtein commented Apr 19, 2026

Uh oh!

Reviewers

Assignees

Labels

stejskalleos commented Apr 1, 2026 •

edited

Loading

stejskalleos Apr 14, 2026 •

edited

Loading

evgeni Apr 14, 2026 •

edited

Loading