Support Verification in sigstore/cosign with X.509 Certificate Chain

[yangkenneth]

Summary

• Within sigstore/cosign we had opened a thread with @Hayden-IO around supporting OCI Image validation within the v0.3 bundle using certificate chains.

So tl;dr - it is fine to include the certificate chain in a v0.3 bundle. If there's any constraints in sigstore-go around parsing v0.3 bundles with chains, we can relax those constraints. Implementing support in Cosign is the intended design - see #132 and #298 (Edit: also #253 for more discussion) for some context, where we discussed an interface that allows us to create verifiers for alternative (aka not Sigstore-spec compliant) signing/verification paths for private deployments.

• For the change in Supporting OCI Signing with X.509 Certificate Chain cosign#4614 if we attempt to create a v0.3 bundle which includes a certificate chain and verify the OCI Image we get the following error. By removing the validation check the signature verification for the OCI Image passes as expected.

cosign verify \
    --insecure-ignore-tlog \
    --insecure-ignore-sct \
    --check-claims=true \
    --certificate-identity example.com \
    --certificate-oidc-issuer-regexp ".*" \
    --trusted-root trusted-roots.json \
    --new-bundle-format \
    --use-signed-timestamps \
    --experimental-oci11 \
    {OCI IMAGE}
WARNING: Skipping tlog verification is an insecure practice that lacks transparency and auditability verification for the signature.
Error: no signatures found
error during command execution: no signatures found

Release Note

• NONE

Documentation

• https://docs.sigstore.dev/cosign/signing/signing_with_containers

[kommendorkapten]

@Hayden-IO Just removing the check for the certificate chain here seems a bit too permissive. This could potential have effects for clients using a PGI-like deployment (such as GitHub) which operates under the same semantics.

However, I still don't think this would work as IIRC chain building ignores what's in the bundle, it only relies on the intermediates from the bundle:

sigstore-go/pkg/root/certificate_authority.go

Lines 47 to 50 in 3f2ee9e

	intermediateCertPool := x509.NewCertPool()
	for _, cert := range ca.Intermediates {
	intermediateCertPool.AddCert(cert)
	}

[Hayden-IO]

+1 to @kommendorkapten, I agree this is too permissive. I'd be open to some sort of a policy flag that allows this check to be bypassed when set explicitly by the calling library, though off the top of my head I'm not sure the best way to cleanly do this.

[yangkenneth]

+1 to @kommendorkapten, I agree this is too permissive. I'd be open to some sort of a policy flag that allows this check to be bypassed when set explicitly by the calling library, though off the top of my head I'm not sure the best way to cleanly do this.

@Hayden-IO and @kommendorkapten pushed an update to support passing verification options through the bundle. If we want this to be opt-in, we’d need a corresponding change in cosign to add a flag through cosign for verification (e.g. --allow-certificate-chain). That way the default behavior stays as-is, and users can explicitly enable validation using certificate chains when using the new bundle format.

cosign verify \
    --insecure-ignore-tlog \
    --insecure-ignore-sct \
    --check-claims=true \
    --certificate-identity example.com \
    --certificate-oidc-issuer-regexp ".*" \
    --trusted-root trusted-roots.json \
    --new-bundle-format \
    --use-signed-timestamps \
    --experimental-oci11 \
    --allow-certificate-chain \
    {OCI IMAGE}

[kommendorkapten]

Sorry for a late response but having an explicit parameter --allow-certificate-chain seems good to me, as that could be implemented in sigstore-go as a WithBundleIntermediate option to be passed to the verifier.

[Hayden-IO]

LGTM to this approach as well. I've asked @yangkenneth to file an issue on sigstore/sigstore-conformance to start the discussion with other clients about whether or not we want this standardized across clients.

[Hayden-IO]

Can you add a test as well?

[yangkenneth]

@Hayden-IO added tests to validation the options as well as creating an issue within sigstore/sigstore-conformance#342. Let me know if you need anything else in the meantime 🙏

[woodruffw]

I'm not a maintainer of sigstore-go so my opinion isn't binding here, but IMO this would be a problematic addition: it effectively punches a hole in the v2/v3 bundle distinction for a use case that is best solved by using the v2 bundle format directly (since it isn't deprecated or obsolete, and is intended for this kind of X.509 topology).

(xref sigstore/sigstore-conformance#342 (comment) for related)

[yangkenneth]

I'm not a maintainer of sigstore-go so my opinion isn't binding here, but IMO this would be a problematic addition: it effectively punches a hole in the v2/v3 bundle distinction for a use case that is best solved by using the v2 bundle format directly (since it isn't deprecated or obsolete, and is intended for this kind of X.509 topology).

(xref sigstore/sigstore-conformance#342 (comment) for related)

Appreciate the insight @woodruffw; the blocker that I originally ran into was because the current release for cosign does not allow bundle versions under v0.3.

This seems to leave us with a few potential paths forward:

1. Modify sigstore/cosign to accept v0.2 bundles; however this raises the question about long-term trajectory. If there are protobuf changes for the Sigstore bundle this becomes a breaking change where implementations using certificate chains would not be able to leverage the latest spec.
2. Allow v0.3 bundles to optionally contain certificate chains through an explicit opt-in flag. An comment would need to be made within the protobuf spec to allow certificate chains in v0.3.

Would appreciate hearing both your and @Hayden-IO's opinions here on what the best path forward is.

[woodruffw]

Yeah, I think option 1 probably makes the most sense.

Allow v0.3 bundles to optionally contain certificate chains through an explicit opt-in flag. An comment would need to be made within the protobuf spec to allow certificate chains in v0.3.

This would effectively be a significant semantic change, which IMO would mean it would need to be v4 instead of v3. The protobuf-specs themselves also don't proscribe things like opt-in flags, so I think this would be a bit of a mismatch.

[Hayden-IO]

I'm going to move the conversation back over to conformance to make sure we can get some of the other clients to chime in as well, we'll hold off on making changes to sigstore-go until we have some consensus.