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

Implement support for HSM for CA private key storage #7238

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

rcritten
Copy link
Contributor

Implement the design for HSM support.

This PR set adds options for HSM token, library path and token password to the CA/KRA-related installers.

The intention is to test this in CI using a SoftHSM2 token. This is not recommended for production since it lacks the Hardware part of HSM.

It has additionally been tested using an nCipher and Thales Luna HSM with CA installation.

The HSM code requires PKI 11.5.0+.

Non-HSM installations should not be affected.

@rcritten rcritten added needs review Pull Request is waiting for a review ipa-4-11 Mark for backport to ipa 4.11 re-run Trigger a new run of PR-CI labels Feb 16, 2024
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Feb 16, 2024
@flo-renaud
Copy link
Contributor

@rcritten PKI hasn't released 11.5.0 yet in fedora but there are builds in the copr repo @pki/master. Testing in progress at freeipa-pr-ci2#3415

@rcritten
Copy link
Contributor Author

I suppose I can drop the pki requires temporarily. pki-master copr is enabled for the testing. This will get it past the Azure failure.

@rcritten rcritten added the re-run Trigger a new run of PR-CI label Feb 21, 2024
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Feb 21, 2024
@rcritten rcritten added the re-run Trigger a new run of PR-CI label Apr 24, 2024
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Apr 24, 2024
Copy link
Contributor

@flo-renaud flo-renaud left a comment

Choose a reason for hiding this comment

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

Hi @rcritten
please find my inline comments.
I am not completely done yet with the review (I would like to play a bit with the replica install and renewal scenarios, and check the tests) but I don't want to loose my comments in the mean time...

ipatests/test_integration/test_hsm.py Show resolved Hide resolved
freeipa.spec.in Show resolved Hide resolved
install/share/60certificate-profiles.ldif Show resolved Hide resolved
ipatests/prci_definitions/temp_commit.yaml Outdated Show resolved Hide resolved
ipatests/pytest_ipa/integration/tasks.py Outdated Show resolved Hide resolved
ipaserver/install/certs.py Show resolved Hide resolved
ipaserver/install/ipa_kra_install.py Outdated Show resolved Hide resolved
ipaserver/install/ca.py Outdated Show resolved Hide resolved
ipaserver/install/ca.py Outdated Show resolved Hide resolved
ipaserver/install/ca.py Outdated Show resolved Hide resolved
@flo-renaud
Copy link
Contributor

Additional comment: the token_password is written in clear in /var/log/ipaserver-install.log and ipaserver-kra-install.log

topology = 'star'

@classmethod
def install(cls, mh):
Copy link
Contributor

Choose a reason for hiding this comment

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

@mrizwan93
The install and uninstall methods are almost identical for all the test classes. It is possible to define a class BaseHSMTest(IntegrationTest) and define in this class install/uninstall, and inherit in the other test classes.

@rcritten
Copy link
Contributor Author

I added a chunk of code for replica installs which should make all the CA and/or KRA certificates visible in the softoken and fixes the different nickname for the IPA CA cert.

@rcritten
Copy link
Contributor Author

I added a temporary patch which will fix the CI testing. But I don't think it should have failed since it should be using a softhsm2 token. I'll check the logs on the other side.

rcritten and others added 26 commits May 7, 2024 18:00
The bulk of the installer effort to enable HSM support without
having to provide an override file.

This pulls the HSM configuration from a remote server when installing
a replica so that the token name and library don't need to be
passed with every installation.

Fixes: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
This will allow the HSM stored configuration to be read.

Fixes: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Needed so the helper renew_ca_cert can read password.conf in order
to get the token password. These files are already readable with
FS permissions.

Fixes: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
The certificates live on the token so need to be retrieved
from there with the token name. The certificates are visible
in NSS softoken but operations need to be done on the HSM
version. The right password is necessary so retrieve it from
the PKI password store.

Fixes: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
A token can only be set in an HSM installation so this is implicit:
if a token exists then HSM is enabled, if not then it isn't.

Fixes: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
This script deletes all CA certificates so a new chain
can be loaded. It identified CA certs by those that did
not have private keys. This change adds the  ca_flags test
in as well. It is probably sufficient on its own but it
is left for compatibility.

An HSM-based NSS database when not accessing it with the
token will not contain the private keys so removing all
certificates without a private key will remove certificates
that it shouldn't. The NSS softoken stores the certifcate
trust so the certificates will be visible but they lack
private keys because those reside in the HSM. Therefore
deleting any certificate without a private key removed
nearly everything.

Preserve the nickname 'caSigningCert cert-pki-ca'. The
certstore uses the nickame format '{REALM} IPA CA' and
will replace the PKI-named key if we don't act to
preserve it.

Fixes: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
The PKI audit certificates require that trusted peer (P) be
set on the certificate. This is done already for the CA audit
certificate. Also set this on the KRA audit certificate on
renewal.

https://pagure.io/freeipa/issue/9353

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Simple function that takes a list of file names and copies
them from one host to another.

It isn't the most efficient but for a small number of files it
should be sufficient.

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Use SoftHSM2 to install an IPA CA to store the keys in an HSM.

Whenenver new keys are generated either in the initial install
or if a KRA is installed then the token needs to be synced
between all servers prior to installing a new CA or KRA.

Fixes: https://pagure.io/freeipa/issue/9273

Signed-off-by: Mohammad Rizwan <myusuf@redhat.com>
This can be eventually squashed into the main "test" patch but
keeping it separate to make it easier to see what has happened.

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
It would fail eventually with the output in the CA logs but it
wasn't always very obvious and you had to wait a while to find
out about a typo.

Scraping modutil output is a bit ugly but it is guaranteed
to be installed and this should work both with p11-kit and
without.

Related: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
This certificate should not be renewed this way.
ipa-cacert-manage renew should be used.

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
A number of files that need to be managed by certmonger
have unconfined_u:object_r:pki_common_t:s0.

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
This is simple, a port needs to be available to certmonger
to communicate during renewals of CA subsystem certificats.

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Use SoftHSM2 to install an IPA CA to store the keys in an HSM.

Whenenver new keys are generated either in the initial install
or if a KRA is installed then the token needs to be synced
between all servers prior to installing a new CA or KRA.

Fixes: https://pagure.io/freeipa/issue/9273

Signed-off-by: Mohammad Rizwan <myusuf@redhat.com>
Arguments were added to the configuration file to allow specifying
the token option values. These needed to be included into the
defaults as well.

This should be merged into the tests prior to pushing.

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
On a non-HSM, non-renewal-server replica we look in LDAP for
an updated certificate. If the certificates don't match then we
have a new one and write it out. If they match the assumption is
that it hasn't been renewed yet so go into CA_WORKING.

The problem is that for networked HSMs the cert will already be
visible in the database so certmonger will always be in CA_WORKING.
In this case we can assume that if the certs are the same then
that's just fine.

Related: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
If the password wasn't provided by --token-password then an empty
value would be passed into the CA installer which promptly failed.

Related: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Not all HSMs support PKCS#1 v1.5. The nShield nFast is one we know
of so force the KRA to use OAEP in this case..

This can be seen in HSMs where the device doesn't support the
PKCS#1 v1.5 mechanism. It will error out with either "invalid
algorithm" or CKR_FUNCTION_FAILED.

There is currently no good way to test for this capability in
advance of configuration. Testing for mechanisms alone is
insufficient. The only real way to test would be to attempt a
wrap/unwrap but it is very complex.

If the list of affected HSMs increases we can use a table
instead based on "best guess" of some sort of property but
looking for a unique string inside the library path is a
pretty straigthforward way.

Note that this doesn't preclude someone from wanting to require
OAEP directly by modifying the KRA CS.cfg and it won't impact
FIPs mode which requires OAEP.

Related: https://pagure.io/freeipa/issue/9191

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
If a certificate on a token does not have NSS trust set then
it won't be visible in the softoken. This can be disconcerting
for those used to seeing all the certificates.

Loop through the possibilities and set no trust (or Peer) for
all the certificates on the token.

Also ensure that the CA certificate has the correct nickname.

Related: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
* Switch to CA user when saving NSS certificates
* Add new certs to internal token, try harder to remove on renewal
* Don't restrict tokens to CKM_RSA_X_509

Related: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Related: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
hsm_validator was validating that the token was available but
not that the provided password worked. Add that capability.

Also call it early in the CA and KRA installation cycle so that
it errors out early. This is particularly important for the KRA
because there is no uninstaller.

Bump the minimum PKI release to 11.5.0 as that contains important
fixes for the HSM.

Remove an unused arguments to hsm_version and hsm_validator.

Related: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Don't blow up if the expected module is not installed but warn
about it. Hopefully users will actually read the output and/or the
installation log.

This is done by looking for strings in the path. Not great but
it's at least something.

Related: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Additional SELinux rules are necessary for the HSM to be
managed by IPA and certmonger. Given the infinite possible
naming combinations of library paths and modules this is
a best effort. A message is logged if a missing module
is detected.

Related: https://pagure.io/freeipa/issue/9273

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
@rcritten
Copy link
Contributor Author

rcritten commented May 7, 2024

The base class changes passed CI. I sqaushed a bunch of WIP commits and hopefully didn't introduce any breaking changes in the process. I believe that if this passes then all comments have been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ipa-next Mark as master (4.12) only needs review Pull Request is waiting for a review
Projects
None yet
4 participants