[Bug]: Exception when attempting to revoke expired deleted certificate

[darkphoenix]

Is this a critical security issue?

• This is not a security issue.

Describe the Bug

When attempting to revoke a certificate that has previously been deleted, but is still present in the inventory, but has also since expired, the Puppetserver CA throws a FileNotFoundException.

Expected Behavior

Like in other cases where revoke is called for an already revoked or nonexistent certificate, it fails cleanly and returns a 404 to the API.

Steps to Reproduce

1. Create and revoke a certificate with very short lifetime

puppetserver ca generate --certname manually-deleted.test.invalid --ttl 60s
puppetserver ca clean --certname=manually-deleted.test.invalid

2. wait for the 60s to run out if they haven't already
3. attempt to revoke the certificate again

$ puppetserver ca revoke --certname=manually-deleted.test.invalid
Error:
    When attempting to revoke certificate 'manually-deleted.test.invalid', received:
      code: 500
      body: Internal Server Error: java.io.FileNotFoundException: /etc/puppetlabs/puppetserver/ca/signed/manually-deleted.test.invalid.pem (No such file or directory)

Environment

• Version: 8.7.0
• Platform: Debian 11

Additional Context

This appears to be because the Puppetserver CA code first attempts to find the certificate serial number in the inventory, but skips expired entries in that case, and then if it finds nothing falls through to searching the filesystem. For some reason, this error is caught cleanly when the certificate has never existed, and obviously the certificate is just revoked again if it is correctly found in the inventory. The issue only occurs when an expired entry exists in the inventory, but the file has been removed.

The Foreman plugin for Puppet always attempts to revoke and delete any previous certificates with the same hostname, which is usually a safe operation to perform on a nonexistent certificate. In this case, the exception causes the VM creation to fail.

This bug was previously reported as puppetlabs/puppetserver#2901, however we have since switched to OpenVox, which obviously has the same issue.

Relevant log output

[ffrank]

Off the top of my head, the intention behind this behavior could be that the server should avoid matching old entries when looking for the certificate to revoke, because FQDNs will be reused many times in long term installations.

It should be safe to change this behavior to make it so that all unrevoked certificates with the given CN are revoked.

[luitzifa]

We have the exact same problem. Unable to reuse hostnames in foreman because foreman always tries to revoke and delete certificates first.

Did you find any workaround?

[ffrank]

This will be hard to work around, as it looks like intended behavior.

Looks like it was introduced in a4337a8 without much elaboration. The commit refers to ticket PE-36859 and I'm not sure this one is publicly available.

I agree that hard failing in this scenario is wrong, and I'm willing to look for a way to just disable this particular exception, and instead just ignore the missing certificate when trying the revocation.

[darkphoenix]

We have the exact same problem. Unable to reuse hostnames in foreman because foreman always tries to revoke and delete certificates first.

Did you find any workaround?

We did; we are resolving this issue by pruning the relevant lines from the inventory, which does not appear to have other ill effects.
So, we remove individual hosts upon incurring this issue like this:
sed -i "/CN=${HOST}/d" /etc/puppetlabs/puppetserver/ca/inventory.txt

We also performed a one time cleanup as a preemptive measure, like this:
cat /etc/puppetlabs/puppetserver/ca/inventory.txt | awk -F= '($2 ~ /Puppet CA/) || (system("test -f /etc/puppetlabs/puppetserver/ca/signed/" $2 ".pem") == 0) {print $0}' > inventory.txt.new
This removes all entries from the inventory which do not have a corresponding file in the filesystem, indicating they were previously revoked and removed. Run this at your own risk, obviously, but it solved this issue for us for the most part, and we have not seen any issues with the CA because of it.