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

Replace service certificates with ipa-server-certinstall #6920

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

Conversation

rcritten
Copy link
Contributor

Replace service certificates with ipa-server-certinstall

Also revoke existing IPA-issued certificates.

ipa-server-certinstall does not update the HTTP or ldap service
entries when the the new certificates. This is confusing when
looking at the service entries because it looks like the new
certificates are not active, when they are.

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

@rcritten rcritten added ipa-4-9 Mark for backport to ipa 4.9 ipa-4-10 Mark for backport to ipa 4.10 labels Jul 19, 2023
Also revoke existing IPA-issued certificates.

ipa-server-certinstall does not update the HTTP or ldap service
entries when the the new certificates. This is confusing when
looking at the service entries because it looks like the new
certificates are not active, when they are.

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

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
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.

@rcritten
thanks for the PR, the code works fine. Please find inline comments below.

ds.cert = cert
name = f'{ds.service_prefix}/{ds.fqdn}'
oldcerts = api.Command.cert_find(service=name)['result']
ds.add_cert_to_service(append=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

The option append=False introduces a slightly different behavior for IPA-issued certs and 3rd-party certs.
When the LDAP server cert is issued by IPA and renewed by IPA, the new certificate is added to the service entry and the old one is kept: https://github.com/freeipa/freeipa/blob/master/ipaserver/plugins/cert.py#L961-L968
With this code, if the LDAP server cert is a 3rd-party cert, it replaces the old cert in the service entry (the old cert is removed).

IMO the logic should be similar in both cases (either always replace or always add). No strong opinion on the best choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's been a while so I forget my reasoning. I think it may be related to what cert-find returned since it also looks in the service cert values. I'll double-check.

oldcerts = api.Command.cert_find(service=name)['result']
ds.add_cert_to_service(append=False)

revoke_certs(oldcerts)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not comfortable with the revocation of the previous cert. For instance, if the same 3rd-party cert is used initially for both LDAP and HTTP but later on the administrator realizes he should reduce the attack surface and replaces the LDAP cert with another one (in order to have a distinct cert for each service), he doesn't want to revoke the HTTP cert.
I know that 3rd part certs can't be revoked inside IPA but that's just an example for a scenario where replacing a cert is not synonym of revoking a cert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But they should be able to issue a new HTTP, LDAP or PKINIT cert from IPA using certmonger. I'll give that a try but I don't see why it wouldn't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So yes, it's possible. I tested replacing the Apache and LDAP certs with 3rd party certs, then replaced those with IPA-issued certificates.

For Apache you first need to move the cert and key out of the way (or remove them). Then you can have certminger issue a new one:

ipa-getcert request -f /var/lib/ipa/certs/httpd.crt -k /var/lib/ipa/private/httpd.key -p /var/lib/ipa/passwds/ipa.example.test-443-RSA -D ipa.example.test -D ipa-ca.example.test -K HTTP/ipa.example.test@EXAMPLE.TEST -C /usr/libexec/ipa/certmonger/restart_httpd -T caIPAserviceCert -v -w

Restart httpd: systemctl restart httpd

The paths are the same for all HTTP certs, CA-issued or 3rd party

For 389-ds you can request a new cert:

ipa-getcert request -d /etc/dirsrv/slapd-EXAMPLE-TEST -n Server-Cert -p /etc/dirsrv/slapd-EXAMPLE-TEST/pwdfile.txt -D ipa.example.test -K ldap/ipa.example.test@EXAMPLE.TEST -C "/usr/libexec/ipa/certmonger/restart_dirsrv EXAMPLE-TEST" -T caIPAserviceCert -v -w

Stop dirsrv

Edit dse.ldif and replace nsSSLPersonalitySSL with Server-Cert, or whatever nickname was chosen, then restart dirsrv.

Done and the certs are replaced and tracked now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flo-renaud flo-renaud self-assigned this Sep 25, 2023
Copy link

stale bot commented Mar 17, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Stale PR [Bot] label Mar 17, 2024
@abbra
Copy link
Contributor

abbra commented May 22, 2024

can we rebase this one?

@stale stale bot removed the stale Stale PR [Bot] label May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ipa-4-9 Mark for backport to ipa 4.9 ipa-4-10 Mark for backport to ipa 4.10
Projects
None yet
3 participants