Add a missing step for SSL cert renewal

[rh-max]

What changes are you introducing?

Adding the missing service restart step.

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

https://issues.redhat.com/browse/SAT-19673

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

Checklists

• I am okay with my commits getting squashed when you merge this PR.
• I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

• Foreman 3.15/Katello 4.17
• Foreman 3.14/Katello 4.16 (Satellite 6.17)
• Foreman 3.13/Katello 4.15 (EL9 only)
• Foreman 3.12/Katello 4.14 (Satellite 6.16; orcharhino 7.2 on EL9 only)
• Foreman 3.11/Katello 4.13 (orcharhino 6.11 on EL8 only; orcharhino 7.0 on EL8+EL9; orcharhino 7.1 with Leapp)
• Foreman 3.10/Katello 4.12
• Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
• We do not accept PRs for Foreman older than 3.9.

[github-actions]

The PR preview for cbf2908 is available at theforeman-foreman-documentation-preview-pr-3941.surge.sh

The following output files are affected by this PR:

• Administering_Project/index-katello.html
• Administering_Project/index-orcharhino.html
• Administering_Project/index-satellite.html

show diff

show diff as HTML

[ekohl]

I wouldn't expect this to be needed since services should restart after changing certificates. That IMHO is a bug and we shouldn't only document steps for users to work around bugs.

@adamruzicka I get the impression we only support the system trust store for TLS LDAP connections. Is that correct? If so, then that we support that at all is a complete side effect that in the bootstrap RPM building we add it to the trusted CA store:
https://github.com/theforeman/puppet-foreman_proxy_content/blob/11cadd50b6cf19baa2edaea44bd6f85e87282fce/manifests/bootstrap_rpm.pp#L20-L23

AFAIK we never intended that to work and once we drop the bootstrap RPM and/or containerize we'll likely break that functionality. Do we have any tests that verify this behavior?

[adamruzicka]

I get the impression we only support the system trust store for TLS LDAP connections. Is that correct?

That seems to be true.

Do we have any tests that verify this behavior?

If I'm reading things right (cc @lhellebr to keep me on the right track), the openldap/ad/idm instances we have available use their own (possibly self-signed) certs. In tests, the ca cert is pulled from the openldap/ad/idm instance and placed into /etc/pki/ca-trust/source/anchors/ on the foreman machine. I don't think we have a test that would explicitly check that foreman trusts certs signed by a ca that signed foreman's cert when connecting to ldap.

[Lennonka]

I think this needs to be cherry picked to older versions as well -> updating PR description.

[ekohl]

If I'm reading things right (cc @lhellebr to keep me on the right track), the openldap/ad/idm instances we have available use their own (possibly self-signed) certs. In tests, the ca cert is pulled from the openldap/ad/idm instance and placed into /etc/pki/ca-trust/source/anchors/ on the foreman machine. I don't think we have a test that would explicitly check that foreman trusts certs signed by a ca that signed foreman's cert when connecting to ldap.

This made me look for what we have today and at least found #3954.

Then I also wondered about what OpenSSL does. I see that if no ca_file, ca_path or cert_store is provided that it uses the default store: https://github.com/ruby/openssl/blob/328d0dc96b4f1d91cb166c0c01826ab8075897e9/lib/openssl/ssl.rb#L148-L150

While I don't want to dive so deep in the code verify it, I do think it's very likely that it caches all the trusted certificates in the store and the report is valid. I just don't like that it restarts services that may have already been restarted. Worse, it may be incomplete in other places.

Back to my original goal: which documentation do we have for this? The answer is https://docs.theforeman.org/nightly/Configuring_User_Authentication/index-katello.html#Configuring_TLS_for_Secure_LDAP_authentication. There is no restart there.

@adamruzicka @lhellebr can we verify the original report is a problem and then decide on a proper solution? I'm inclined to treat this as a bug report rather than a docs bug. We may choose to work around it in docs, but preferably we solve it in code.

[lhellebr]

@adamruzicka our external ldap sources use certs signed by RH CA

[maximiliankolb]

two minor suggestions; one is more or less mandatory.

[maximiliankolb]

line 12: please change "*" to "." to make it an ordered list.

[maximiliankolb]

triage: @rh-max Please apply the suggestions.

[aneta-petrova]

Moving to draft while we look for a new owner.