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

Support validating LWCA certmonger requests #308

Merged
merged 3 commits into from
Nov 7, 2023

Conversation

rcritten
Copy link
Collaborator

The LWCA ids are UUID4 format and are stored in LDAP so we can retrieve the list (ignoring the ipa entry) and construct what the request should look like.

Fixes: #307

@rcritten
Copy link
Collaborator Author

While developing the original patch I noticed that cn=cas was being hammered. It was because the call to cert_find included all=True which was confusing the cache. We in fact don't need all attributes. This increased the cache hits from 7 to 24.

@rcritten
Copy link
Collaborator Author

rcritten commented Nov 2, 2023

Switched from caching only the LWCA requests to all expected requests. This saves a fair bit of effort and a few potential LDAP calls.

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
thanks for the new check, LGTM. I tested the following:

  • fresh install without any LWCA. Prints a warning that there is no LWCA entry:
# ipa-healthcheck --source ipahealthcheck.ipa.certs 
Search for LWCA entries failed: no matching entry found
[]

Maybe the message could be made a bit less negative since it's a completely valid situation, for instance: Skipping LWCA checks because the search did not return any LWCA entry

  • Add a LWCA with ipa ca-add subca --subject cn=subca,O=IPA.TEST but do not run ipa-certupdate. The subca entry is found but there is no tracking yet:
# ipa-healthcheck --source ipahealthcheck.ipa.certs 
[
  {
    "source": "ipahealthcheck.ipa.certs",
    "check": "IPACertTracking",
    "result": "ERROR",
    "uuid": "c0dbcc12-67d0-4f40-a7ee-e9db42168cda",
    "when": "20231106094520Z",
    "duration": "0.406647",
    "kw": {
      "key": "cert-database=/etc/pki/pki-tomcat/alias, cert-nickname=caSigningCert cert-pki-ca f4315412-6c51-4576-ab12-2a3088720869, ca-name=dogtag-ipa-ca-renew-agent, cert-presave-command=/usr/libexec/ipa/certmonger/stop_pkicad, cert-postsave-command=/usr/libexec/ipa/certmonger/renew_ca_cert \"caSigningCert cert-pki-ca f4315412-6c51-4576-ab12-2a3088720869\", template-profile=caCACert",
      "msg": "Expected certmonger tracking is missing for {key}. Automated renewal will not happen for this certificate"
    }
  }
]
  • run ipa-certupdate to create the tracking request. No error reported
# ipa-healthcheck --source ipahealthcheck.ipa.certs 
[]
  • disable and remove the ca (ipa ca-disable subca and ipa ca-del subca, remove the tracking request with getcert stop-tracking -i ID and run ipa-certupdate. No error reported:
# ipa-healthcheck --source ipahealthcheck.ipa.certs 
Search for LWCA entries failed: no matching entry found
[]

@rcritten
Copy link
Collaborator Author

rcritten commented Nov 6, 2023

Huh. I'd have sworn I was logging the LWCA search result as debug and not warn. I'll fix that up and drop the "failed" bit. The search succeeded but returned nothing.

The LWCA ids are UUID4 format and are stored in LDAP so
we can retrieve the list (ignoring the ipa entry) and
construct what the request should look like.

Add a cache for the get_expected_requests() function. The
certificates aren't going to change (or shouldn't) in the
middle of a run and there is no point in duplicating several
LDAP requests for each call.

Fixes: freeipa#307

Signed-off-by: Rob Crittenden <[email protected]>
This was causing a cache miss in the LDAPCache class. The
'*' + all default attributes was confusing the cache. We in fact
do not need all attributes so this is fine. This increases the
cache hits in cert.py from 7 to 24, reducing the number of
duplicate LDAP searches.

Related: freeipa#307

Signed-off-by: Rob Crittenden <[email protected]>
These pass locally for me but fail in the github workflow. Marking
as xfail for now.

A deprecation warning is being spit out now on stderr instead out
stdout which includes the underlying message. Check both stdout
and stderr to be on the safe side.

Note: these tests only run as root.

Related: freeipa#309

Signed-off-by: Rob Crittenden <[email protected]>
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 updated patch, works for me.

@flo-renaud flo-renaud self-assigned this Nov 7, 2023
@flo-renaud flo-renaud added the ack label Nov 7, 2023
@rcritten rcritten merged commit 25bbaab into freeipa:master Nov 7, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support verifying LWCA (SubCA) certmonger requests
2 participants