Refactor certificates role to normalize server and client certificate…

[ehelms]

… creation

[ekohl]

Again, this also makes me think about how we could implement an ACME client to get what we used to call custom certificates

[ehelms]

Again, this also makes me think about how we could implement an ACME client to get what we used to call custom certificates

I am more focused on the "input", the interface for providing certificates and so think of this certificate role as a reference implementation.

[ekohl]

I often feel Ansible with all of its variables doesn't make a clear distinction between (user) input and (intermediate) output. It's late so I'm not going to thoroughly review it now, but perhaps it's already something to think about

[ehelms]

I often feel Ansible with all of its variables doesn't make a clear distinction between (user) input and (intermediate) output. It's late so I'm not going to thoroughly review it now, but perhaps it's already something to think about

Oh I have :) But baby steps.. first this re-factor, then my take on that part as this helps pave the way.

[ekohl]

This may be taking it a step too far in this PR, but I'd be tempted to work towards getting rid of the separate server and client certs. In vanilla Foreman we (by default) always used the same certificates to be the server and the client.

In other words, stop generating separate server certs and make server_certificate and server_key default to client_certificate and client_key. If a user provides server certs, they are expected to already be signed by some CA.

This will be an issue for services that we run on localhost today because no respected CA should ever sign a certificate for localhost, but that's the general direction I'd like to move to.

[ekohl]

I have a recollection that by default the extension isn't copied over when calling openssl ca. https://stackoverflow.com/questions/33989190/subject-alternative-name-is-not-copied-to-signed-certificate mentions you should use -copy_extensions copyall when signing

[ehelms]

This may be taking it a step too far in this PR, but I'd be tempted to work towards getting rid of the separate server and client certs. In vanilla Foreman we (by default) always used the same certificates to be the server and the client.

I think this is a fair design goal that we can work to support. The current certificates implementation for upgrades will be the puppet-certs implementation that does split server and client certificates and indicates their purpose. That's one implementation. This reference implementation attempts to not stray far from that basic design principle. I think, at the start, this also helps make sure we consider that there are places where client certificates are expected and places where server certificates are expected. Early on, I don't want to lose site of that and have a gap.

The place I'd like to end up with is to have the following:

1. A reference implementation for certificate generation, that is used by default and by developers.
2. Documentation that lays out what the required certificates are and any requirements.
3. Eliminate the server vs. default concept that we created - either use the defaults or bring them all.

Then for example, we can document it was a client certificate is needed here and a server certificate need there. You the user can choose to have a single certificate for both, that has both client and server purpose. Or you can choose to split them into dedicated purposes if you want that level of separation of concern.

[ekohl]

I was wondering how to verify this. Could we somehow run katello-certs-check against the result of this in CI? Do we have any molecule tests already that we could leverage?

[ehelms]

I was wondering how to verify this. Could we somehow run katello-certs-check against the result of this in CI? Do we have any molecule tests already that we could leverage?

I think adding some tests to ensure this role does what we expect and vets the generated certificates against katello-certs-check is a good idea.

This started as a re-factor to better be in service of https://github.com/theforeman/foreman-quadlet/pull/90 to help out with our installation research.

I'll look at adding some tests and verification.

[ehelms]

I added a role to run certificate check using katello-certs-check. And, for this iteration, pulled out the certificate variables generated into a vars fie. This draws the line between user input and "internal" input parameters.

@evgeni I am thinking then for the installer split work, instead of defining them directly (f425856) you can provide a vars file that defines the certificates coming from puppet-certs and then just include that file conditionally rather than the one from the certificates role.

[evgeni]

Under which circumstances would this be false?

[ehelms]

When generating a certificate bundle for use on proxies - https://github.com/theforeman/foreman-quadlet/pull/90/files#diff-b1491adbed08c8b00b570245e695c525e8dbf2a9ef94181d292272ffae52b5a5R6

[evgeni]

Ah, so you ran this once with certificates_ca: true to create a CA, and then you run it with false (on the same machine) to create the bundle (with a new hostname -- for a different machine) without regenerating the CA?

Does that imply ca.yml is not idempotent? Or is that pure speed optimization?
(Reading the playbook I think it's idempotent?)

[ehelms]

Speed and safety optimization I suppose. I was also not sure how the design would fall out where something else provided the certificates yet (e.g. puppet-certs). I am still not 100% and this will get re-factored as we iterate.

[evgeni]

I like this, especially as it makes my "use puppet-certs certs with quadlet" frankenstein easier. And tests are also passing, so 🎉
So unless @ekohl has objections, I'd love to merge this, as it's an improvement and we can layer on further ones on top.

[ekohl]

I like the change where a lot of the fields on the cert become optional. It always bothered me we had those weird defaults that probably nobody changed and served no purpose.