Improve API of cosign module.

[vembacher]

Introduction

I think the cosign API could be improved at a number of places.

I already gave some feedback on the cosign API in #274:

Toggle for previous feedback.

• I feel like some parts of the Cosign API are too "low level" and unclear about what is being verified.
• trusted_signature_layers: I think this function could be removed from the public API, because it is somewhat confusing and it is not obvious what it does.
• verify_constraints: I think this function could be moved into the the CosignCapabilities trait and replace trusted_signature_layers as part of the public API. The parameters could be a mixture of both. The name could be changed to something similar to the verify_blob* methods.
• Is it the user's/OCI client's task to verify that image data and signature are consistent? The way I understand it, I can only assume the actual image data is authentic, if I'm fetching it with a reference that contains the digest and the OCI client ensures image data is consistent with the manifest.
• Blob verification
  • I think the verify_blob* methods could use a variant that supports Fulcio based certificates + verification constraints.
  • Does it make sense to have verify_blob* methods for both bundle and raw signatures?
  • Does it make to use an enum for the 'verification key source' for public key, certs and Fulcio, instead of having functions for each?

Changes

I hope the following can provide more details on what I had in mind.

Change 1: consolidate trusted_signature_layers and verify_constraints into a single method that is part of the CosignCapabilities trait. I think this current setup is somewhat confusing.

Currently we have the following signature:

// sigstore::cosign::CosignCapabilities
pub trait CosignCapabilities {
// ...
    async fn trusted_signature_layers(
        &mut self,
        auth: &Auth,
        source_image_digest: &str,
        cosign_image: &OciReference,
    ) -> Result<Vec<SignatureLayer>>;
}

// sigstore::cosign::verify_constraints
pub fn verify_constraints<'a, 'b, I>(
    signature_layers: &'a [SignatureLayer],
    constraints: I,
) -> std::result::Result<(), SigstoreVerifyConstraintsError<'b>>
where
    I: Iterator<Item = &'b Box<dyn VerificationConstraint>>

I personally think having an API similar to this would be better:

// sigstore::cosign::CosignCapabilities
pub trait CosignCapabilities {
// ...
    async fn verify_image<I>(
        &mut self,
        auth: &Auth,
        source_image_digest: &str,
        cosign_image: &OciReference,
        constraints: I,
    ) -> Result<_> // unsure about the return type
    where
      I: Iterator<Item = &'b Box<dyn VerificationConstraint>>;
}

Notes:

• I'm not sure what the return type should be.
• I think this change can be made without breaking changes if the previous methods are only deprecated for now.

Change 2: standardize container image and blob verification APIs

Methods for blob verification verify_blob and verify_blob_with_public_key have a very different API from how container images are verified. I think both should be more similar to improve the DX of users. So my idea is to have functions that are part of the CosignCapabilities trait that are similar to this:

• verify_image(): see above
• verify_blob(): this already exists along with verify_blob_with_public_key()
• I personally would also add constraint verification to the blob verification method.

Change 3: improve verify_blob API

I think verify_blob and verify_blob_with_public_key have an inconsistent naming scheme.
I personally would either change verify_blob to verify_blob_with_certificate or unify them into a single function named verify_blob.

One option would be to use an enum for the key type:

enum VerificationKeySource {
  Sigstore, // analogous to how container images are verified
  Certificate(...),
  PublicKey(...),
}

pub trait CosignCapabilities {
  // ...
  fn verify_blob<I>(
      &self, // add this to get access to keys the client already stores
      key_source: &VerificationKeySource,
      bundle: &CosignBundle,  // use a bundle instead of a raw signature
      blob: &[u8],
      constraints: I, // add constraints to enforce signer subjects, etc.
   ) -> Result<()>
   where
      I: Iterator<Item = &'b Box<dyn VerificationConstraint>>;
}

Notes:

• This approach makes it possible to verify blobs with Fulcio-issued certificates.
• This requires users to build a cosign client first.
• We do not use raw signatures anymore, is it necessary to support them?

Summary

I think these changes could help to unify the cosign API and make it easier to use. I most definitely did not consider all implications these changes would have, so I think this likely requires some changes.