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

Leaf certificate expiration reported by API does not account for CA expiration date #377

Open
1 of 3 tasks
ae-govau opened this issue Sep 12, 2022 · 4 comments · May be fixed by #378
Open
1 of 3 tasks

Leaf certificate expiration reported by API does not account for CA expiration date #377

ae-govau opened this issue Sep 12, 2022 · 4 comments · May be fixed by #378

Comments

@ae-govau
Copy link

What version of the credhub server you are using?
2.12.8

What version of the credhub cli you are using?
2.6.2

If you were attempting to accomplish a task, what was it you were attempting to do?

As part of our CI/CD, we run credhub curl -p /api/v1/certificates, parse the output, and verify that the most recent version of all certificates has an expiry_date at least 30 days out. This helps give us plenty of time to initiate rotation procedures before expiration.

What did you expect to happen?

The expiry_date should be the value when the certificate ceases to function.

What was the actual behavior?

The expiry_date is parsed from the certificate data itself, without regard to the CA that signed it, which has a much closer in expiration date.

For example:

curl -p /api/v1/certificates

Output:

{
  "certificates": [
    {
      "id": "xxx",
      "name": "/bosh/cf/diego_instance_identity_ca",
      "signed_by": "/bosh/cf/application_ca",
      "signs": [],
      "versions": [
        {
          "certificate_authority": true,
          "expiry_date": "2023-03-30T00:59:21Z",
          "generated": true,
          "id": "yyy",
          "self_signed": false,
          "transitional": false
        },

However a subsequent call to fetch more data about that version of the certificate:

credhub curl -p /api/v1/data/yyy | jq -r .value.ca | openssl x509 -noout -enddate

Reveals that the CA expires 6 months earlier (a few days ago):

notAfter=Sep  9 21:13:40 2022 GMT

We believe this is likely due to a previous rotation where update-mode converge may not have been correctly set, however none-the-less we think it is unhelpful for credhub to report an expiration date that is longer than the CA that it knows has signed it. Separately it would helpful to simply not issue a certificate longer than the parent CA as well.

Please confirm where necessary:

  • I have included a log output
  • My log includes an error message
  • I have included steps for reproduction
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

ae-govau added a commit to ae-govau/credhub that referenced this issue Sep 12, 2022
date

This prevents one from issuing a certificate that appears to be valid,
but in reality typically won't validate as being so.

Partially fixes cloudfoundry#377
@peterhaochen47
Copy link
Member

peterhaochen47 commented Sep 20, 2022

Hi @ae-govau, thank you for reporting the issue.

Question about your workflow:
You mentioned that:

As part of our CI/CD, we run credhub curl -p /api/v1/certificates, parse the output, and verify that the most recent version of all certificates has an expiry_date at least 30 days out... Reveals that the CA expires 6 months earlier (a few days ago):

Shouldn't credhub curl -p /api/v1/certificates also return the CA in question? So when the CA is expiring within 30 days, you should be able to catch it? Why would it have expired a few days ago?

For whether a leaf certificate's notAfter can be longer than the notAfter of the CA, I haven't found an official RFC about this question. And it seems that the implementations of our peers vary and people are warned that they should not assume that this requirement is enforced.

That said, your argument makes sense. However, I think the change should be that CredHub returns an error when you request to generate a leaf certificate with a longer validity duration than the CA (as opposed to silently lowering the duration for the user). This way, the user gets a clear feedback on whether their exact request has been fulfilled. But in any case, this would be a breaking change, which warrants a major version bump. We could consider it when we are ready for our next major version.

@ae-govau
Copy link
Author

Shouldn't credhub curl -p /api/v1/certificates also return the CA in question? So when the CA is expiring within 30 days, you should be able to catch it? Why would it have expired a few days ago?

The call to /api/v1/certificates returned:

      "name": "/bosh/cf/diego_instance_identity_ca",
      "signed_by": "/bosh/cf/application_ca",

And you're partially correct - it also does return a record for the signing CA (/bosh/cf/application_ca).

However what we found is that the current certificate for the signing CA is newer - but the actual CA used to sign the child certificate was an older version of that signing CA.

We have automation built to apply the process outlined here:
https://github.com/pivotal/credhub-release/blob/main/docs/ca-rotation.md

But for this particular certificate (which you can see is a CA cert itself), we were left in this situation where after following that process (and perhaps we have an error in our scripting) we are left with this CA cert in a state where it's been signed by an older version of the parent CA.

My gut feeling is that having 2 levels of CAs may simply not work with the 4 step process for rotating CAs listed above, and that we may be better to try to weed out why that's actually needed. I'll see if I can take another look at cf-deployment and find a good way to remove that requirement, as I think that'll make this process more robust. I find it quite curious that the CF deployment has been running quite happily for the past 6 months in this state - suggesting it's configured to trust the intermediate rather than the root anyway, thus negating the purpose of the additional complexity.

We've since added an extra step to our regular validation to make an extra call to the API to fetch the CA for each cert and validate that it's also in the list returned by /api/v1/certificates, and this enables us to verify that we've checked that too.

For whether a leaf certificate's notAfter can be longer than the notAfter of the CA, I haven't found an official RFC about this question. And it seems that the implementations of our peers vary and people are warned that they should not assume that this requirement is enforced.

I'm pretty certain most clients will enforce it at validation time, as they'll be validating the parent cert which won't be valid if it's expired. I also wasn't able to find a definitive answer on issuance. In theory you could issue a child cert valid longer than the parent, and the re-issue the parent to the same key for a longer validity, so in theory it might be ok, but in practice it's not.

However, I think the change should be that CredHub returns an error when you request to generate a leaf certificate with a longer validity duration than the CA (as opposed to silently lowering the duration for the user). This way, the user gets a clear feedback on whether their exact request has been fulfilled.

Re raising an error if requesting a longer date, wouldn't that nearly always occur? ie initially request a year for the CA, then the next request will be for that CA to sign a child cert for the same period?

Would you be open to adding an additional field to each version object returned by /api/v1/certificates? e.g. adding signed_by_id?

@peterhaochen47
Copy link
Member

Me: For whether a leaf certificate's notAfter can be longer than the notAfter of the CA, I haven't found an official RFC about this question. And it seems that the implementations of our peers vary and people are warned that they should not assume that this requirement is enforced.

You: I'm pretty certain most clients will enforce it at validation time, as they'll be validating the parent cert which won't be valid if it's expired. I also wasn't able to find a definitive answer on issuance. In theory you could issue a child cert valid longer than the parent, and the re-issue the parent to the same key for a longer validity, so in theory it might be ok, but in practice it's not.

Sorry for the confusion. My original statement was intended to be in agreement with you. Let me rephrase:
For whether an issuer can issue a leaf certificate whose notAfter is longer than the notAfter of the CA, I haven't found an official RFC about this question. And it seems that the implementations of our peers (other cert issuers) vary and clients/validators are warned that they should not assume that issuers conform to this requirement (hence the clients/validators should always the validity of all certificates in the trust chain).

I'll respond to your other points separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

3 participants