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

Fail to sign artifacts when ExtendedKeyUsage claim is absent from issuer certificate #658

Closed
mayaCostantini opened this issue May 25, 2023 · 7 comments · Fixed by #674
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@mayaCostantini
Copy link
Contributor

Description

Signing an artifact against a private instance of Fulcio deployed with a password-protected private key backend raises an ExtensionNotFound error. After investigation, it seems like when verifying the SCT, the _is_preissuer internal method fails if it cannot find the ExtendedKeyUsage extension in the issuer certificate.

However I did not observe this behavior when signing with Cosign against the same instances.
The issue is also absent when signing with sigstore-python against another private instance of Sigstore deployed with a Google CA backend, which seems to provide the correct extensions.

Version

sigstore-python v1.1.1

@mayaCostantini mayaCostantini added the bug Something isn't working label May 25, 2023
@woodruffw
Copy link
Member

Hmm, could you say more about the expected behavior here? From this:

Signing an artifact against a private instance of Fulcio deployed with a password-protected private key backend raises an ExtensionNotFound error. After investigation, it seems like when verifying the SCT, the _is_preissuer internal method fails if it cannot find the ExtendedKeyUsage extension in the issuer certificate.

-- it sounds like your backing CA isn't using a Precertificate Signing Certificate per RFC 6962.

My read of the RFC is that the CA is required to do one of two things:

  • Either use a Precertificate Signing Certificate (which MUST have an EKU extension);
  • Or sign the TBSCert with the CA certificate that will sign the final certificate.

We haven't seen the latter in practice with Sigstore, but maybe that's what's happening here 🙂

@ccordoui
Copy link
Contributor

Yes indeed, we are hitting the second case with the TBS that need to be signed by the same Authority than the final one.

As such the CA doesn't need to have the CT EKU, in fact it does not need to have an EKU at all.
That being said, the first line of _is_preissuer:

ext_key_usage = issuer.extensions.get_extension_for_class(ExtendedKeyUsage)

Assumes that we have at least one EKU and only checks whether or not CT is one of them, wich doesn't comply with the RFC which only require EKU for the Precertificate Signing Certificate.

One last thing, I might be wrong but the Precertificate Signing Certificate and the certificate signing the final certificate should both be trusted CA, I couldn't found the part of the code that is verify the CA:true extension and I think that we are also missing the verification of the chain, I can only see a comment here that is stating that Cryptography do not do it (yet).

@woodruffw
Copy link
Member

Cool, thanks for confirming.

One last thing, I might be wrong but the Precertificate Signing Certificate and the certificate signing the final certificate should both be trusted CA, I couldn't found the part of the code that is verify the CA:true extension and I think that we are also missing the verification of the chain, I can only see a comment here that is stating that Cryptography do not do it (yet).

I believe that's right, concerningly -- I don't remember why we don't do the full chain verification here. I'll have some more time to look into this later today.

CC @asraa and @haydentherapper -- I figure one or both of you might have some more context here, including what other clients do w/r/t SCT chain validation 🙂

@woodruffw
Copy link
Member

I believe that's right, concerningly -- I don't remember why we don't do the full chain verification here. I'll have some more time to look into this later today.

Answering my question: we don't do chain verification in the precertificate case because we're doing plain-old public key cryptography here, with the ctfe.pub (or equivalent) that's embedded in our root of trust (via TUF).

@haydentherapper
Copy link
Contributor

+1 to @woodruffw. Agreed that we should ideally support all of RFC6962, but given that we'll have to do a bit of work to support that (it's going to differ by language, golang has good SCT support but other languages don't), are you open to signing with a separate key rather than the CA?

@ccordoui
Copy link
Contributor

I think I see the reasoning behind not verifying the chain (we blindly trust that TUF is providing us the right CA cert for the CT), even though it would be better to do it just to prevent edge cases in a perfect world.

If I've read correctly, the RFC 6962 states that in the precertificate case, we should have the CA:true extension and the CT EKU, if one is verified, it would be logical to verify the other, wouldn't it?

Anyway the problem of @mayaCostantini isn't with the precertificate case but in the case where the CT CA is the same for both, the TBS and the final certificate.
In that case there is no need for any EKU, and _is_preissuer shouln't assume that an EKU is present here:

ext_key_usage = issuer.extensions.get_extension_for_class(ExtendedKeyUsage)

It should be at least catch cryptography.x509.ExtensionNotFound exception and return False.

@woodruffw
Copy link
Member

If I've read correctly, the RFC 6962 states that in the precertificate case, we should have the CA:true extension and the CT EKU, if one is verified, it would be logical to verify the other, wouldn't it?

Agreed; I'll add that check 🙂

It should be at least catch cryptography.x509.ExtensionNotFound exception and return False.

Makes sense to me, I'll add that as well.

@woodruffw woodruffw self-assigned this Jun 2, 2023
@woodruffw woodruffw added this to the 2.0 milestone Jun 2, 2023
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 7, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

Resolves: sigstore#658
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 7, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 11, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

In both case, the certificate must be a valid CA

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 11, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

In both case, the certificate must be a valid CA

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 11, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

In both case, the certificate must be a valid CA

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 12, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

In both case, the certificate must be a valid CA

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 20, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

In both case, the certificate must be a valid CA

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 20, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

In both case, the certificate must be a valid CA

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 20, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

In both case, the certificate must be a valid CA

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 28, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

In both case, the certificate must be a valid CA

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 28, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

In both case, the certificate must be a valid CA

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>

Update sigstore/_utils.py

Signed-off-by: William Woodruff <william@yossarian.net>

Update CHANGELOG.md

Signed-off-by: William Woodruff <william@yossarian.net>

Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
ccordoui added a commit to ccordoui/sigstore-python that referenced this issue Jun 28, 2023
RFC 6962 stipulate that the TBSCertificate can be either signed by:
- A special purpose CA Precertificate Signing Certificate with an EKU
- The final CA Certificate with no mandatory EKU
In _is_preissuer was failing on the later if no EKU was present

In both case, the certificate must be a valid CA

Resolves: sigstore#658
Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>

Update sigstore/_utils.py

Signed-off-by: William Woodruff <william@yossarian.net>

Update CHANGELOG.md

Signed-off-by: William Woodruff <william@yossarian.net>

Signed-off-by: Cyril Cordoui <ccordoui@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants