Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration free certs #4584

Merged
merged 4 commits into from
Oct 9, 2023
Merged

Conversation

fmarco76
Copy link
Member

@fmarco76 fmarco76 commented Oct 9, 2023

Subsystem certificates were stored in the CS.cfg configuration file and NSSDB storage. This is useless and could create problem is they are not synced.

Therefore the certificates will not be stored in CS.cfg during the installation and all the requests will read the certificate from the NSSDB.

This partially implemented https://issues.redhat.com/browse/RHCS-3451, the certreq has the same problem and need to be removed from configuration files.

Fix #2157

Certificate are stored in nssdb of the instance so the copy in the file
is redundant and will be removed.
When working with HSM the Lightweight CA access the root CA certificate
from the CS.cfg. This has been modified reading the certificate from
NSSDB to allow the removal of such field from the configuration.

NOTE: Currently, Lightweight CA does not work with HSM so this change
cannot be really tested until the Github issue dogtagpki#2412 is fixed.
@fmarco76 fmarco76 requested review from edewata and ladycfu October 9, 2023 10:22
Copy link
Contributor

@edewata edewata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some minor comments, but overall it looks good. Feel free to update/merge.

@edewata
Copy link
Contributor

edewata commented Oct 9, 2023

@rcritten FYI, with this change PKI will no longer store cert data in the CS.cfg in new installations, so I suppose IPA should also stop checking/updating the <subsystem>.<tag>.cert params (e.g. ca.signing.cert).

@fmarco76 Once IPA is no longer checking/updating the params, we can add an upgrade script to remove the params from existing installations.

@fmarco76
Copy link
Member Author

fmarco76 commented Oct 9, 2023

we can add an upgrade script to remove the params from existing installations.

For the script I would complete with the elimination of the CSR. For that I am not sure yet if it can be just removed or we need to keep a copy. After I will see for the upgrade script.

@rcritten
Copy link
Contributor

rcritten commented Oct 9, 2023

There is a pki healthcheck that compares the values. That needs to be removed as well. It may well be all of base/server/healthcheck/pki/server/healthcheck/meta/csconfig.py but definitely including CADogtagCertsConfigCheck, OCSPDogtagCertsConfigCheck, KRADogtagCertsConfigCheck, TKSDogtagCertsConfigCheck, TPSDogtagCertsConfigCheck,

@fmarco76
Copy link
Member Author

fmarco76 commented Oct 9, 2023

There is a pki healthcheck that compares the values

@rcritten this has been already disabled with PR #4579.

@fmarco76 fmarco76 force-pushed the ConfigurationFreeCerts branch from dda04c8 to c89c3c2 Compare October 9, 2023 16:08
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@fmarco76
Copy link
Member Author

fmarco76 commented Oct 9, 2023

@edewata Thanks!

@fmarco76 fmarco76 merged commit 369c19f into dogtagpki:master Oct 9, 2023
151 of 152 checks passed
@fmarco76 fmarco76 deleted the ConfigurationFreeCerts branch October 9, 2023 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redundant cert data in CS.cfg
3 participants