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

Migration of Public API in support of sigstore-go migration #3879

Open
codysoyland opened this issue Sep 12, 2024 · 2 comments
Open

Migration of Public API in support of sigstore-go migration #3879

codysoyland opened this issue Sep 12, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@codysoyland
Copy link
Member

Migration of Public API in support of sigstore-go migration

As we work toward support of the TrustedRoot in the cosign verifier, I would like to take a moment to consider our plans for the cosign public API. Beginning with verification, there are several functions that take a CheckOpts struct as an argument. This struct is used to configure the verification process for a series of functions:

  • func VerifyBlobAttestation(ctx context.Context, att oci.Signature, h v1.Hash, co *CheckOpts) (bool, error)
  • func VerifyBlobSignature(ctx context.Context, sig oci.Signature, co *CheckOpts) (bundleVerified bool, err error)
  • func VerifyImageAttestation(ctx context.Context, atts oci.Signatures, h v1.Hash, co *CheckOpts) (checkedAttestations []oci.Signature, bundleVerified bool, err error)
  • func VerifyImageAttestations(ctx context.Context, signedImgRef name.Reference, co *CheckOpts) (checkedAttestations []oci.Signature, bundleVerified bool, err error)
  • func VerifyImageSignature(ctx context.Context, sig oci.Signature, h v1.Hash, co *CheckOpts) (bundleVerified bool, err error)
  • func VerifyImageSignatures(ctx context.Context, signedImgRef name.Reference, co *CheckOpts) (checkedSignatures []oci.Signature, bundleVerified bool, err error)
  • func VerifyLocalImageAttestations(ctx context.Context, path string, co *CheckOpts) (checkedAttestations []oci.Signature, bundleVerified bool, err error)
  • func VerifyLocalImageSignatures(ctx context.Context, path string, co *CheckOpts) (checkedSignatures []oci.Signature, bundleVerified bool, err error)
  • func VerifyRFC3161Timestamp(sig oci.Signature, co *CheckOpts) (*timestamp.Timestamp, error)

CheckOpts is largely replaced by a few data structures in sigstore-go:

  • TrustedMaterial contains the trusted root (CA certs, TSA certs, and transparency log keys), as well as a method of providing a custom public key verifier
  • CertificateIdentities contains the set of identities that a certificate must match in order to be considered valid
  • VerifierOptions contains a set of options that can be used to configure the verification process, such as online verification and policy options like number of required TSA signatures

There are a few paths we can take with the cosign API:

  1. We can deprecate all the existing API functions and the use of CheckOpts altogether, requiring users to migrate to sigstore-go for general verification, and we would offer only a handful of container image verification utilities in cosign's public API. This could be very disruptful for existing users of cosign's public API, but ease development on cosign.
  2. We can continue supporting cosign's public API as is, and add new functions that use the new sigstore-go types. This would be less disruptive to existing users, but would result in a more complex API surface.
  3. We can deprecate most of the fields within CheckOpts, replacing them with equivalent sigstore-go types. This would be less disruptive to existing users, giving them a clear migration path, but may result in more complexity cosign's development. In this option, we would support both verifiers, and only use the deprecated fields if a TrustedMaterial is not provided. After a deprecation period, we can delete the deprecated fields and all associated verification code paths. This option would ensure API continuity for existing users.

In option 3, I would propose we modify CheckOpts like so:

Field Strategy
RegistryClientOpts Keep
Annotations Keep
ClaimVerifier Keep
RekorClient Deprecate, use TrustedMaterial
RekorPubKeys Deprecate, use TrustedMaterial
SigVerifier Keep?
PKOpts Keep?
RootCerts Deprecate, use TrustedMaterial
IntermediateCerts Deprecate, use TrustedMaterial
CertGithubWorkflowTrigger Deprecate, use CertificateIdentities
CertGithubWorkflowSha Deprecate, use CertificateIdentities
CertGithubWorkflowName Deprecate, use CertificateIdentities
CertGithubWorkflowRepository Deprecate, use CertificateIdentities
CertGithubWorkflowRef Deprecate, use CertificateIdentities
IgnoreSCT Deprecate, use VerifierOptions
SCT Deprecate, stop supporting detached SCTs
CTLogPubKeys Deprecate, use TrustedMaterial
SignatureRef Deprecate, get from Bundle
PayloadRef Deprecate, get from Bundle
Identities Deprecate, use CertificateIdentities
Offline Deprecate, use VerifierOptions
TSACertificate Deprecate, use TrustedMaterial
TSARootCertificates Deprecate, use TrustedMaterial
TSAIntermediateCertificates Deprecate, use TrustedMaterial
IgnoreTlog Deprecate, use VerifierOptions
MaxWorkers Currently unsupported by sigstore-go
ExperimentalOCI11 Deprecate, replace with Cosign Bundle Spec
TrustedMaterial New field, root.TrustedMaterial
CertificateIdentities New field, verify.CertificateIdentities
VerifierOptions New field, []verify.VerifierOption

Also note that several existing API functions receive a oci.Signature argument. We could do a similar thing to this struct, adding a field for the SigstoreBundle and deprecating other fields.

I have been somewhat in favor of option 3: migration of CheckOpts and continuity of API surface, however, I can see how this would complicate development on cosign, and outside of container image verification, it would be good to encourage users to switch to sigstore-go, so perhaps option 1 is preferable. I don't have a good sense of how much work this would put on existing users. I will say that Sigstore Policy Controller is somewhat tightly coupled with the CheckOpts and RegistryClientOpts structs. I would love to get feedback from the greater cosign community on which option makes the most sense.

@codysoyland
Copy link
Member Author

As a small follow-on, I'd like to demonstrate how a proposed additional API surface might look with one example. If we deprecate the CheckOpts field, then the function to verify an image attestation would look like this:

old version:

func VerifyImageAttestation(ctx context.Context, atts oci.Signatures, h v1.Hash, co *CheckOpts) (checkedAttestations []oci.Signature, bundleVerified bool, err error)

new version:

func VerifyImageAttestationBundle(ctx context.Context, atts oci.Signatures, h v1.Hash, trustedMaterial *root.TrustedMaterial, certificateIdentities verify.CertificateIdentities, opts []verify.VerifierOption, registryClientOpts []ociremote.Option, annotations map[string]interface{}) (checkedAttestations []oci.Signature, bundleVerified bool, err error)

I dislike long function signatures especially in public APIs, so I would probably want to encapsulate image verification options, much like CheckOpts currently does. Perhaps an ImageVerificationOptions struct could be created for this.

@steiza
Copy link
Member

steiza commented Sep 23, 2024

I updated #3854 to make use of CheckOpts as described above, with some small tweaks.

Instead of CertificateIdentities I added IdentityPolicies []verify.PolicyOption, because the identity policy could be that you're verifying with a key instead of a certificate.

Otherwise, it seems like everything else worked pretty well! Definitely interested in feedback on the pull request.

steiza added a commit to steiza/cosign that referenced this issue Sep 23, 2024
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

2 participants