sigstore / sigstore-conformance

Conformance testing for Sigstore clients
https://sigstore.dev
7 stars 10 forks source link

CoSign Verify.go func TrustedCert's required Extended Key Usage (EKU) #148

Open JeffreyFBarry opened 2 months ago

JeffreyFBarry commented 2 months ago

Description Referencing func TrustedCert: https://github.com/sigstore/cosign/blob/main/pkg/cosign/verify.go#L1331 Main Branch func TrustedCert currently implements a code section requested for removal, approximately:

KeyUsages: []x509.ExtKeyUsage{ x509.ExtKeyUsageCodeSigning,}

Lockheed Martin (LM) suggests in regard to this TrustedCert function to modify it by, removal of the KeyUsage requirement for func TrustCert. This Main Branch change is requested for CoSign in order to support LMs implementation of Sigstore using RFC5280 conformant Issuing CAs/Certificate Holder certificates.

The importance of conformance to this RFC to Lockheed Martin is from a Security Engineering perspective, Microsoft and other platforms following RFC 5280 interpretations will trust at "run-time execution" a code signature made with a trusted certificate chain and an a code signer with an EKU consistent with "Code Signing." Lockheed Martin limits issuance of this category of certificate to specific restrictions .. note **both a certificate with "anyEKU" or that contains "no EKU extension" are valid certificate 'wildcard' EKU entries that are also consistent with Code Signing,

As such,

To achieve better conformance, the Fulcio CA Main Branch could also be updated, LM would like to identify a solution for Sigstore's ecosystem to conform with RFC 5280 related to short-lived certificates issued for rekor ledger signing, this would also meet our current use case following security engineering's request the implementation limit the issuance of certificates types with a "Code Signing" EKU to only those meant for signing consistent with RFC 5280's description of executable code,

"id-kp-codeSigning OBJECT IDENTIFIER ::= { id-kp 3 } -- Signing of downloadable executable code -- Key usage bits that may be consistent: digitalSignature"

https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.12

Creating a new-purposed Sigstore OID under existing ARC to implement w/ Fulcio CA, http://oid-info.com/get/1.3.6.1.4.1.57264, for an EKU consistent for "Code Integrity" would meet conformance and security requirements, but would ultimately be less permissive of various PKI implementations or customizations made to Fulcio CA if CoSign's func TrustedCert stays hard-coded to any specific EKU.

If a new EKU is pursued, Lockheed Martin would request the change to CoSign's func Trustedcert is still simultaneously pursued to be merged into the Main Branch in order to support our use case of our RFC compliant certificates from LM's "Code Integrity" Root we have instantiated and tested with a CoSign fork of the requested modification.

@bobcallaway

haydentherapper commented 2 months ago

Hey @JeffreyFBarry, I think I understand the issue, though please correct me if not. To summarize, the issue is that RFC5280 has a very precise definition for what code signing is. With Sigstore, you can digitally sign anything, not just executable code, which means that the issued certificates are not RFC5280 compliant since RFC5280 is not generalizable. In researching this, I came upon a related issue - https://www.digicert.com/blog/creating-a-dedicated-eku-for-document-signing - where Digicert was proposing a new EKU for "document signing" for services that digitally sign documents like Adobe or Docusign. Of course this EKU is also too precise for Sigstore, but it seems like this is a known limitation of RFC5280 compliance.

With that said, we do not aim to be RFC5280 compliant, as we are private and not public PKI. We try to be as close as possible so that existing PKI tooling can verify certificates.

Additionally, the CAB Forum now has a WG for code signing. Again, we aren't trying to be compliant with CAB's requirements since that's more for platform (eg Windows) code signing, but we do take inspiration from them. In the baseline requirements, they define "code" as "A contiguous set of bits that has been or can be digitally signed with a Private Key that corresponds to a Code Signing Certificate", which I think is a reasonable definition and more generalizable than RFC5280's definition of "code". By that definition, the codeSigning EKU is the proper EKU to use, which they require as part of their baseline requirements (7.1.2.3).

We will not be changing the EKU set by Fulcio, that would be a breaking change and have significant ramifications across the Sigstore ecosystem and clients.

From the client's perspective, I would very much like to see a well-defined UX around private PKI where we can relax all of these various constraints, which is a part of Sigstore's client roadmap. This would require implementing a set of scoped CLI options (e.g. cosign private verify-blob) which could be configured based on the PKI needs. If there is a succient proposal for how to relax this constraint in the current version of Cosign without impacting users, I would be happy to review that as well.

JeffreyFBarry commented 2 months ago

Thanks here Hayden for the look into this, just to restate, the base requested change would be to remove the Code Signing EKU requirement from the CoSign verify function main branch, not a requested change to Fulcio CA necessarily at all.

Modifying func TrustedCert as proposed here would ultimately make CoSign less restrictive to RFC compliant Fulcio implementations and in terms of validation I would not expect that change to have a functional impact to Sigstore ecosystem and clients.

I would separate here that the baseline of the proposal being sought is that line of code be removed from Cosign, and I would be here promoting action toward this first as I hope a reasonable implementable solution.


Regarding a Sigstore EKU for consideration, you are correct that would be an ecosystem-wide impact, how large could be determined, but just to give you some additional clarity on if that is a good idea or not:

I think you have some of the concept, noted that compliance is not a first driver here,

to help sort out any confusion about the baseline requirements purpose, its misrepresenting the issue to tie to that in this case - the CA Browser forum baseline requirements are related to issuance of valid code signing certificates, commercially, and that starts with requirements for RFC 5280 compliance, defining a "Valid Certificate" as one that "passes the validation procedure specified in RFC 5280."

those baseline requirements cited are for issuers/Certification Authorities participating in Public Trust of their Code signing Root Certificates, e.g., running under most likely a formal Certificate Policy. Adherence and audit to those requirements qualifies those CAs to issue RFC 5280 compliant certificate holder certificates with this "somewhat protected" id-kp-codeSigning EKU,

id-kp-codeSigning EKU per definition is still probably not appropriate for document/ledger record signatures, even if its a record of code.

Signing contiguous sets of bits for an integrity of origin is "signing code" for software build of materials perhaps, but it is not the same as usage intended for the defined "codeSigning" EKU, which is reserved per RFC 5280 to be consistent with signing downloadable executable code, e.g., for a time of execution trust ultimately - this is how Microsoft and others have historically interpreted valid use.

signing rekor entries would not meet this usage definition in my view unless there is another functional part to the "code signatures" that fits the strict usage definition of codeSigning: for download/execution, typically applied to compiled dependencies like .dll files or .exe runnables

This suggestion is also made for general awareness including the codeSigning EKU where its inappropriate to introduces an unintended potentially exploitable vulnerability in the event of a CA compromise, or in a case where trusted certificates were mis-used to sign code targeted to at least Windows systems that have installed trust for a Fulcio CA instance issuing with the codeSigning EKU.

In the event of a CA compromise or certificate mis-use of certificates intended for say ledger signing, could be mis-used to sign malicious code would be executable in a trusted way ...

In terms of alternative EKUs, id-kp-documentSigning is a generic document signing EKU has already been in wide-use, https://www.ietf.org/archive/id/draft-ito-documentsigning-eku-00.pdf - this I believe is the initiative you are referring to.

just to Note, Security Considerations from the IETF draft are consistent with this enhancement suggestion.

"5. Security Considerations The Use of id-kp-documentSigning EKU can prevents the usage of id-kp- emailProtection for none-email purposes and id-kp-codeSigning for signing objects other than binary codes. An id-kp-documentSigning EKU value does not introduce any new security or privacy concerns."

To satisfy our Security Engineering request, relaxing the constraint in CoSign will allow Lockheed Martin to rely on the main branch of CoSign while maintaining our private compliant PKI per our operating practices / certificate constraints as we issue.

Thank you again on the consideration put into assessment on the write-up, the EKU proposal is separate-but-related to bringing forward the Verify.go update requested here to the main branch code ..

@haydentherapper @bobcallaway

kommendorkapten commented 2 months ago

This is a really good discussion ❤

Would it be possible to share some more details around you use cases to get a better insight into the issue here? I understand the problem in general, but there are still some details that not clear.

id-kp-codeSigning EKU per definition is still probably not appropriate for document/ledger record signatures, even if its a record of code.

Just a minor comment here. It is true that certificates from Fulcio is often used to sign build provenance attestations (documents), but these certificates are not used for the ledger records. The ledger (Rekor) has it's own private key (typically not bound to a certificate) to sign the ledger where the records are stored.

JeffreyFBarry commented 2 months ago

Fredrik,

-In this brought case the usage is build provenance, noted on the distinction of the attestation (document) vs. the ledger record signing action happening with a different certificate, that was my slight conflation there for descriptions sake,

-Change requested is to CoSign, func TrustedCert https://github.com/sigstore/cosign/blob/main/pkg/cosign/verify.go#L1331

-Line 1339 - 1340 specifically for removal,

-LM would be in either instance running our CAs with security conscious applied constraints on forked code, the basic request here is the main branch to update the Verify.go function identified to remove id-kp-codeSigning EKU requirement from the validation function,

-In our view that EKU is otherwise applied some-what improperly per the compliance discussion brought for our use case of provenance specifically if there are other valid use cases here - again the requested change of just this functions code would be an ideal compromise for the projects interoperability with our use case in question,

Simply the requested code change to CoSign will allow stock reliance on main branch of CoSign rather than in our usage LM also running a fork of that - that is the use case justified benefit in this case for us, this requested change of func TrustedCert would also allow other custom implementations to "just work" without any management or update to a safe or compliant EKU is our assessment -

@kommendorkapten @bobcallaway

haydentherapper commented 2 months ago

Is this concern one of semantics or because you're deploying on a platform like Windows that has a specific definition for codeSigning? If semantics, I'm not convinced that we need to comply with RFC5280's definition for the reasons I stated earlier.

If you're thinking either about deployment on Windows or the risk of CA compromise leading to issued malicious certificates being trusted, I don't think this is a concern. Sigstore certificates do not meet Windows' code signing and EV requirements and Sigstore's CA root won't meet the requirements to be distributed as the trust root. Even if the root CA certificate is manually loaded into the trust store by the user (which would not be recommended as we dynamically distribute roots via TUF, so a user may be left trusting a root that has been rotated), there would be no verification of the inclusion proofs, so a Sigstore client or non-platform-native tool would be needed to do complete verification of the artifact. If the CA is compromised, the Sigstore certificates would not be trusted by Windows for the same reasons.

For Cosign, I can review a proposal to make the check configurable. This would mean that your certificates would not be verifiable on other Sigstore clients though as they are no longer conforming to the expected certificate profile. As I mentioned earlier, bring-your-own-PKI is an area of active interest, but I wouldn't expect uniformity across clients.

JeffreyFBarry commented 2 months ago

Hayden, for a CoSign proposal, just to confirm that update would be fulfilling supporting Lockheed Martin (LM) in our use case of our Private PKI here if promoted to main, I would be grateful where that is the ideal outcome here for initiating a review of this,

something consistent with modifying func TrustedCert to 1. to have the EKU constraint default removed/or 2. make EKU a configurable variable or passable array/or 3. a boolean to turn off EKU check ..

LMs requirement is to adhere to certain security practices and recommendations related to our PKIs, those in this instance are aligned with limiting issuance of Code Signing capable certificates to only usage consistent with security recommendations per IETF draft cited and Microsoft platform behavior, these are the unavoidable concerns we are working with.

the update to CoSign would directly be enabling our stock reliance on main branch code for our provenance use case.

@haydentherapper @bobcallaway

idunbarh commented 2 months ago

(I'm not a pki/cert person so what I say here is probably wrong. I'll rely on @JeffreyFBarry to help correct my comments)

We're deploying an internal sigstore stack deployment. The fulcio deployment is signing short lived certificates from a corporate issued root certificate ~thats trusted on all of systems~.

We don't want EKU (codeSigning) allowed since code signing issued fulcio certificates allow code execution on windows machines. We don't want to allow users of our internal fulcio deployment the ability sign arbitrary code for execution.

To solve this issue, we created the corporate issued root without EKU (codeSigning). Since cosign validates EKU (codeSigning) is set, verification of all fulcio certs fail.

We want to do document signing, and I think the terminology around code signing not matching is confusing.

jku commented 2 months ago

I think I understand the use case but it seems that the core issue is this:

code signing issued fulcio certificates allow code execution on windows machines.

This seems to not be required by or a part of Sigstore design but a decision made by whoever manages the system certificate store... Is the Fulcio certificate trusted by Windows just because that was the easy way to deploy the certificate to devices or is that somehow useful?

I'm not criticizing btw: practical deployment reasons are valid, I'm just trying to understand how the situation appears

idunbarh commented 2 months ago

This seems to not be required by or a part of Sigstore design but a decision made by whoever manages the system certificate store... Is the Fulcio certificate trusted by Windows just because that was the easy way to deploy the certificate to devices or is that somehow useful?

I think we're in total agreement with the statement "[should be] a decision made by whoever manages the system certificate store".

This concern is setting the EKU in issued certificates is required in the cosign implementation. Its not a choice available to "whoever manages the system certificate store". If you choose to not set the EKU, cosign will not validate certificates.

This might be my naive opinion (I'm not a certificate or PKI knowledgable person), but i think the default behavior should be based on the "lease privilege" principles. I view the primary usecase of sigstore is signing blobs (document signing) not code execution signing. We shouldn't prohibit the code execution signing usecase, but it shouldn't be required either.

I also agree that allowing the certificates to be trusted for code execution signing on windows machines is one avenue to addressing the concern, we're just trying to mitigate future issues.

woodruffw commented 2 months ago

(Catching up now, apologies if I miss any context.)

I'm (unwillingly) a bit of a PKI person, and I concur with @haydentherapper's opinion here: in practice, the codeSigning EKU conveys semantics that are broader than the comment in RFC 5280 (which isn't normative) suggests, as reflected by how CA/B's WGs have interpreted it (this is reinforced by CA/B's policy of remaining a subset of RFC 5280's requirements, suggesting that they believe their interpretation of codeSigning is compatible with the RFC.)

With the above however, I'm a little bit confused by why the codeSigning EKU is causing problems for LM: the Sigstore X.509 profile says that the root certificate MUST NOT contain any EKUs, meaning that it's a "wildcard" usage certificate like you say. Only the intermediate and end-entity certificates are required to have codeSigning, but these shouldn't appear in your certificate store at all; only the root should.

I confirmed that this is true for the Sigstore public good instance:

Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            b6:4d:00:f1:5d:c4:73:f0:8d:e0:e5:a0:3c:32:60:28:40:3b:fe
        Signature Algorithm: ecdsa-with-SHA384
        Issuer: O=sigstore.dev, CN=sigstore
        Validity
            Not Before: Oct  7 13:56:59 2021 GMT
            Not After : Oct  5 13:56:58 2031 GMT
        Subject: O=sigstore.dev, CN=sigstore
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (384 bit)
                pub:
                    04:fb:5d:e1:53:e2:b6:f7:3d:01:b0:4b:82:1a:8e:
                    d2:e4:df:f3:a5:9e:98:1a:9e:06:81:72:56:29:b1:
                    80:6b:e6:2f:b8:ca:70:74:ed:c7:9b:dc:b3:f4:38:
                    83:99:77:17:b1:5f:af:5c:e6:25:6e:c8:94:50:f8:
                    7c:f4:e7:28:be:50:5d:ee:05:60:25:1e:98:92:e6:
                    c8:74:f8:7d:86:1c:4e:d2:5e:b9:35:10:2e:66:d5:
                    3a:f5:f4:bf:60:83:dd
                ASN1 OID: secp384r1
                NIST CURVE: P-384
        X509v3 extensions:
            X509v3 Key Usage: critical
                Certificate Sign, CRL Sign
            X509v3 Basic Constraints: critical
                CA:TRUE
            X509v3 Subject Key Identifier: 
                58:C0:1E:5F:91:45:A5:66:A9:7A:CC:90:A1:93:22:D0:2A:C5:C5:FA
            X509v3 Authority Key Identifier: 
                58:C0:1E:5F:91:45:A5:66:A9:7A:CC:90:A1:93:22:D0:2A:C5:C5:FA
    Signature Algorithm: ecdsa-with-SHA384
    Signature Value:
        30:66:02:31:00:8f:59:c7:79:76:69:fb:5d:cd:58:13:5a:f8:
        40:ec:0c:ff:06:d5:65:a0:d6:d0:8c:58:ff:d6:1c:fa:a9:69:
        5a:34:8e:1b:30:78:d1:59:81:2b:34:78:4e:f0:60:8e:2a:02:
        31:00:d9:60:7d:a2:df:7c:b0:89:28:17:7b:d9:61:d7:77:fd:
        5b:56:07:96:fd:4c:d3:1e:6b:b2:31:fe:cb:49:e5:37:dc:2c:
        b7:80:04:b1:38:04:d2:4e:b1:0e:2f:9c:11:c9
-----BEGIN CERTIFICATE-----
MIIB9zCCAXygAwIBAgIUALZNAPFdxHPwjeDloDwyYChAO/4wCgYIKoZIzj0EAwMw
KjEVMBMGA1UEChMMc2lnc3RvcmUuZGV2MREwDwYDVQQDEwhzaWdzdG9yZTAeFw0y
MTEwMDcxMzU2NTlaFw0zMTEwMDUxMzU2NThaMCoxFTATBgNVBAoTDHNpZ3N0b3Jl
LmRldjERMA8GA1UEAxMIc2lnc3RvcmUwdjAQBgcqhkjOPQIBBgUrgQQAIgNiAAT7
XeFT4rb3PQGwS4IajtLk3/OlnpgangaBclYpsYBr5i+4ynB07ceb3LP0OIOZdxex
X69c5iVuyJRQ+Hz05yi+UF3uBWAlHpiS5sh0+H2GHE7SXrk1EC5m1Tr19L9gg92j
YzBhMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBRY
wB5fkUWlZql6zJChkyLQKsXF+jAfBgNVHSMEGDAWgBRYwB5fkUWlZql6zJChkyLQ
KsXF+jAKBggqhkjOPQQDAwNpADBmAjEAj1nHeXZp+13NWBNa+EDsDP8G1WWg1tCM
WP/WHPqpaVo0jhsweNFZgSs0eE7wYI4qAjEA2WB9ot98sIkoF3vZYdd3/VtWB5b9
TNMea7Ix/stJ5TfcLLeABLE4BNJOsQ4vnBHJ
-----END CERTIFICATE-----
JeffreyFBarry commented 2 months ago

@woodruffw William, you are correct here, and that is our in-practice implementation where EKU is deployed to just the intermediates / end certificates in our case, we are fully RFC 5280 compliant, apologies if this was misconstrued or left out in detail, Lockheed Martin's use case example here is our separate root to support a whole certificate path, the root in that chain does not assert an EKU extension, solely Key usage, similar to the certificate you provided info for,

@jku simply we are following our standards of practices in managing our PKI, the chain/root in question for us is limited in its deployment, the risk identified by security engineering assessment is consistent with RFC compliance of our PKI based on those practices,

Ian gave the best explanation here to focus review

Per @idunbarh

This concern is setting the EKU in issued certificates is required in the cosign implementation. Its not a choice available to "whoever manages the system certificate store". If you choose to not set the Code-signing EKU, cosign will not validate certificates.

Right now id-kp-codeSigning EKU is required by func TrustedCert, - proposed change here is for CoSign's func TrustedCert to be un-restricted, or at least more permissive by checked or configurable EKUs per @haydentherapper's possible review that portion of this base proposal ..

each would be consistent with CoSign main branch supporting use case(s) for management of certificate stores both as described in Lockheed Martin's case for provenance or other uses, as well as for PKIs to be run as default here today without the need to also operate on forked CoSign code to permit other EKUs -

@bobcallaway