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

Add interface for certificate validation #298

Open
kommendorkapten opened this issue Sep 27, 2024 · 4 comments
Open

Add interface for certificate validation #298

kommendorkapten opened this issue Sep 27, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@kommendorkapten
Copy link
Member

Description

Originally discussed here: #253 (comment)

The original implementation was made against the operational mode of PGI using ephemeral certificates and the trust root delivered via TUF. This mode of operation allows us to make the following simplifications:

  • End entity (leaf) certificates expires after only ten minutes, so no method for revocation is implemented
  • The Sigstore bundle only contains the end entity certificate
  • All intermediates are provided via the trust root
  • Long lived certificates (root and intermediates) are distributed via TUF, which allows us to perform instant revocation by removing the certificate from the trust root, so no method for verifying certificate revocation is needed

However, for private deployments these preconditions may not always be met, and certain users may want to rely on a more traditional PKI, where untrusted certificates are provided in the Sigstore bundle, and CLRs or OCSP is used to verify validity of the certificate.

To achieve required security using Certificates in Sigstore-go the client has to perform certificate verification themselves, by parsing the Sigstore bundle and then verify the certificate, which is not great UX.

Proposed is to add the support for a callback function:

func ValidateCertificate(c *x509.Certifiicate) (bool, error) {
        // validate certificate
}

That would then called for each certificate found in the bundle, including the end entity certificate.

To not complicate the logic for the default policy (using PGI), if the callback function is not set, it would set it to the a function that always returns true.

Trust root

Should the callback method be called for the certificates found in the trust root? My take is no. The trust root is created and provide by the client, and so the client could perform the certificate verification prior to composing the trust root.

@codysoyland
Copy link
Member

For the API, are you thinking this would be provided by a special verify.VerifierOption passed to verify.NewSignedEntityVerifier?

Something like this?

func WithCustomCertificateValidator(validateCert func(*x509.Certificate) (bool, error)) VerifierOption {
	return func(c *VerifierConfig) error {
		c.validateCert = validateCert
	}
}

This could definitely be done, but I wonder if this is flexible enough, as some clients would need more control over the actual verification process in https://github.com/sigstore/sigstore-go/blob/main/pkg/verify/certificate.go -- such as modification of the VerifyOptions (e.g. KeyUsages) or other custom logic that would require access to the timestamp and trusted material.

I think my other proposal could provide this same functionality, allowing the client complete control over the certificate verification process. A custom CertificateAuthority implementation could also accept a callback similar to that proposed here.

@haydentherapper
Copy link
Contributor

@kommendorkapten Did you have an example in mind for how to handle "other custom logic that would require access to the timestamp and trusted material"? Is your thought to keep it simple, expecting implementers to design their own structs to contain any other verification material necessary for validating the certificate?

@kommendorkapten
Copy link
Member Author

@haydentherapper yes, keeping the interface simple. My thinking was to make scenarios that works more like typical X.509 PKI easy to implement, where the time in question is the current date. We could have an optional parameter being sent too which is one of observed timestamps, by calling this function for each timestamp, and only count those entries that are valid.

@codysoyland wasn't aware of that, will look more into that issue. Is you thinking that the struct would provide a method like AddToCertPool that the client may intercept and use that to provide certificate verification? My primary concern is not for the certificates provided via the trusted material, but any untrusted intermediary found in the bundle.

@kommendorkapten
Copy link
Member Author

An other option could be to further modify @codysoyland's #300 to expose the chains in the VerificationResult. This can then be documented so that post verification, a validity check of each certificate can be made. It could be argued that it's safer to have a hook where a certificate can be rejected before the chain is verified, but can't think of any real threat by first verify the chain, then go over the certificates as no data except certificates is parsed during this stage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants