cosign download attestation cannot filter by attestation predicate type

[lcarva]

Description

Given a test image:

$ IMAGE='quay.io/lucarval/festoji@sha256:61ac53fe9316734ed8a8bc6418b2c371730a2c122b5df1d7f76b960d381f5457'

I can download all of its attestations:

$ cosign download attestation $IMAGE | jq '.payload | @base64d | fromjson | .predicateType'
"https://slsa.dev/provenance/v0.2"
"https://spdx.dev/Document"

However, I cannot use the --predicate-type parameter to select the SLSA provenance attestation:

$ cosign download attestation $IMAGE --predicate-type 'https://slsa.dev/provenance/v0.2'
Error: no attestations with predicate type 'https://slsa.dev/provenance/v0.2' found
main.go:74: error during command execution: no attestations with predicate type 'https://slsa.dev/provenance/v0.2' found

However, it does work for the SBOM attestation:

$ cosign download attestation $IMAGE --predicate-type 'https://spdx.dev/Document'
{"payloadType":"application/vnd.in-toto+json","payload":"...

Version

🐚 cosign version
  ______   ______        _______. __    _______ .__   __.
 /      | /  __  \      /       ||  |  /  _____||  \ |  |
|  ,----'|  |  |  |    |   (----`|  | |  |  __  |   \|  |
|  |     |  |  |  |     \   \    |  | |  | |_ | |  . `  |
|  `----.|  `--'  | .----)   |   |  | |  |__| | |  |\   |
 \______| \______/  |_______/    |__|  \______| |__| \__|
cosign: A tool for Container Signing, Verification and Storage in an OCI registry.

GitVersion:    v2.0.2
GitCommit:     871448050b924a7946ebe47678f23aae09ef432d
GitTreeState:  clean
BuildDate:     2023-04-24T17:31:42Z
GoVersion:     go1.20.3
Compiler:      gc
Platform:      linux/amd64

I can also reproduce the issue on the current HEAD of main (83c08ce).

[lcarva]

It looks like the cosign download attestation command requires the predicateType annotation when filtering:

cosign/pkg/cosign/fetch.go

Lines 153 to 163 in 83c08ce

	if predicateType != "" {
	anns, err := att.Annotations()
	if err != nil {
	return nil, err
	}
	pt, ok := anns["predicateType"]
	// Skip attestation if predicateType annotation is not present or predicateType annotation does not match supplied predicate type
	if !ok || pt != predicateType {
	continue
	}
	}

Sure enough, the SLSA Provenance attestation doesn't have that annotation, but the SBOM one does.

There's disparity in how the download attestation and the verify-attestation commands work. I can filter by predicate type on the second:

cosign verify-attestation $IMAGE --certificate-identity-regexp '.*'  --certificate-oidc-issuer-regexp '.*' --type 'https://slsa.dev/provenance/v0.2'

Verification for quay.io/lucarval/festoji@sha256:61ac53fe9316734ed8a8bc6418b2c371730a2c122b5df1d7f76b960d381f5457 --
The following checks were performed on each of these signatures:
  - The cosign claims were validated
  - Existence of the claims in the transparency log was verified offline
  - The code-signing certificate was verified using trusted certificate authority certificates
Certificate subject: https://github.com/slsa-framework/slsa-github-generator/.github/workflows/generator_container_slsa3.yml@refs/tags/v1.4.0
Certificate issuer URL: https://token.actions.githubusercontent.com
GitHub Workflow Trigger: push
GitHub Workflow SHA: d9f2616186b454700a4c196e696b9a60fdcecfdb
GitHub Workflow Name: Package
GitHub Workflow Repository: lcarva/festoji
GitHub Workflow Ref: refs/heads/master
{"payloadType":"application/vnd.in-toto+json","payload":"

That's because the verify-attestation command parses each attestation and does a comparison of the predicate type based on the payload data:

cosign/pkg/policy/attestation.go

Lines 84 to 87 in 83c08ce

	if statement.PredicateType != predicateURI {
	// This is not the predicate we're looking for, so skip it.
	return nil, statement.PredicateType, nil
	}

[lcarva]

The problem seems to be that older cosign versions do not add the predicateType annotation. In fact, it was added simultaneously with support for filtering by predicate type: dd747ee

I don't see anything in the in-toto spec about using annotations to specify the predicate type: https://github.com/in-toto/attestation/blob/main/spec/README.md#statement

It does seem like the approach taken by verify-attestation is the more correct one.

Proposal: stop caring about the predicateType annotation. Instead, inspect the statement's payload to determine the predicate type. Thoughts?

[znewman01]

That sounds right to me—it does the right thing in more circumstances (including better backwards compat).

CC @chaospuppy would love your thoughts here!

[chaospuppy]

Thanks for the ping @znewman01! So the motivation for adding the annotation to the attestation layer of the manifest (which is not part of the DSSE envelope or a diversion from the in-toto spec) was two-fold:

1. Doing this meant that tools outside of the cosign ecosystem could find and retrieve the desired attestation blob using more-readily available manifest/manifest-layer metadata
2. You wouldn't need to perform the more resource-hungry processes of decoding each payload before inspecting it

I do think it's confusing to have started adding the annotations at the same time they became usable, but that's why the default behavior matches what it was previously, where all the attestations are returned if a --predicateType isn't specified.

Maybe it would be best to update the fallback behavior if a --predcateType is specified, but no matching annotations are found, to use the verify-attestation method instead of failing?

[lcarva]

@chaospuppy, thanks for the prompt reply! See inline comments below.

1. Doing this meant that tools outside of the cosign ecosystem could find and retrieve the desired attestation blob using more-readily available manifest/manifest-layer metadata

I'd be overly cautious of using a field that is not part of the spec.

2. You wouldn't need to perform the more resource-hungry processes of decoding each payload before inspecting it

Good point. I do think we should favor correctness over performance. IMO the performance hit here is negligible.

I do think it's confusing to have started adding the annotations at the same time they became usable, but that's why the default behavior matches what it was previously, where all the attestations are returned if a --predicateType isn't specified.

The issue is that the predicateType annotation is not part of any standard. If it were, then there would've been no issue with adding support for reading and writing at the same time.

Maybe it would be best to update the fallback behavior if a --predcateType is specified, but no matching annotations are found, to use the verify-attestation method instead of failing?

I think this is an improvement over the current approach. However, I take issue in it being different than what verify-attestation does. As a user, it is very confusing when those commands don't behave the same way.

I made a PR which makes the logic the same for both commands: #3031

Happy to hear what you think!

[chaospuppy]

I'd be overly cautious of using a field that is not part of the spec.

Just to make sure we're thinking about this in the same way, this is what the manifest for an attestation looks like, where the annotations are being added in a standard location:

╰─ crane manifest 192.168.106.3:5000/test/muliarch:sha256-413a41ca3491c2ab9f9428c2419ac3ae2b9b986a2bcbe64981bf9970f79e19d6.att | jq                                                       ─╯
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "config": {
    "mediaType": "application/vnd.oci.image.config.v1+json",
    "size": 245,
    "digest": "sha256:b12b687d8664c216877edc85b075fd71f55b3e136f2b6740dd021f29f85c48c7"
  },
  "layers": [
    {
      "mediaType": "application/vnd.dsse.envelope.v1+json",
      "size": 8216,
      "digest": "sha256:90e4d184b4a6635ec6f3b88e244de0b7fe0a078569bbec710ea9109db29d8712",
      "annotations": {
        "dev.cosignproject.cosign/signature": "...",
        "dev.sigstore.cosign/bundle": "...",
        "dev.sigstore.cosign/certificate": "....",
        "dev.sigstore.cosign/chain": "....",
        "predicateType": "http://example.mytype" 
      }
    }
  ]
}

The issue is that the predicateType annotation is not part of any standard. If it were, then there would've been no issue with adding support for reading and writing at the same time.

We are definitely in agreement about this, there is no standard that will mention predicateType. However, annotations set on OCI manifests are arbitrary by nature to enable this kind of workflow.

I have no issue with making verify-attestation and download attestation behave the same way, but I do still see value in keeping the annotation and trying to use it before falling back to inspecting each payload. My main sticking point on this is it's preferable from an external application perspective to be able to hit a registry REST API to find the blob you want and retrieve it without too much fuss.

[chaospuppy]

Ultimately I'm good with the PR being made because the annotations will still be getting created, but there's probably some followup work on my end to get the predicateType (or more likely something like dev.sigstore.cosign/predicatetype) into the layer so it's a bit more standardized. When that happens, perhaps this short cut (no payload processing up front) could be re-added in both verify-attestation and download attestation? Thoughts @znewman01 ?

[znewman01]

At the risk of complicating things even further, how does this play with OCI 1.1 reference types which support a filter by artifactType? Is the artifactType always going to be just application/vnd.dsse.envelope.v1+json (in which case this is moot) or would we ever encode the predicate type into the artifactType somehow?

CC @sudo-bmitch

---

Overall I defer to you all. My priorities are consistency between the different commands and correctness (no surprise missing data), and performance is a distant third. So I think I'd be okay with either:

1. Fallback workflow: use the annotation if it exists, and if it doesn't fall back to parsing the JSON blob.
2. Just parse the JSON.

IIUC #3031 does (2).

Please let me know if I'm misunderstanding anything here, the OCI side of Cosign is not my strength :)

[lcarva]

I'm not too familiar with artifactType, but it looks like, if anything, this would be set to a type that indicates that the artifact is an in-toto statement. The predicate type is one level deeper.

Fallback workflow: use the annotation if it exists, and if it doesn't fall back to parsing the JSON blob.

If we do this in download attestation, we should probably also do it in verify-attestations to provide consistency. I can think of a couple of use cases where even with a fallback option, the filtering would be different.

I think we should go with option 2, which is what the PR does, at least for now. There's a longer term feature request to push the predicate type annotation into the in-toto standard(?) to facilitate querying. Having cosign continuing adding those annotation makes sense to me and brings us closer to this goal.

[sudo-bmitch]

For an artifact, you do not want to use "application/vnd.oci.image.config.v1+json" for the config. That's a sign that this is a container image (though runtimes will throw errors when they see the layer media type). Instead you either need a dedicated config blob with your own type and content, or if you have no separate config, you would use the artifactType field. We are finishing up what the config descriptor would contain in opencontainers/image-spec#1068, but most likely an artifact that is just a application/vnd.dsse.envelope.v1+json blob would have something like the following manifest:

{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "artifactType": "application/vnd.dsse.envelope.v1+json",
  "config": {
    "mediaType": "application/vnd.oci.empty.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
  "layers": [
    {
      "mediaType": "application/vnd.dsse.envelope.v1+json",
      "size": 8216,
      "digest": "sha256:90e4d184b4a6635ec6f3b88e244de0b7fe0a078569bbec710ea9109db29d8712",
      "annotations": {
        "dev.cosignproject.cosign/signature": "...",
        "dev.sigstore.cosign/bundle": "...",
        "dev.sigstore.cosign/certificate": "....",
        "dev.sigstore.cosign/chain": "....",
        "predicateType": "http://example.mytype" 
      }
    }
  ]
}

You'll likely want to feature gate this behind the oci-1-1 options in cosign since we haven't GA'd this yet. The one registry that I know you'll want to work with is GitLab who has an allow list on their config mediaType and may do the same for the artifactType.

[chaospuppy]

So sounds like you're on the money @lcarva, we will still need to set the annotation if we wish to identify which layer contains the desired predicate type, even after reference types are introduced.

[sudo-bmitch]

Assuming you only have a single layer, you would want to define any annotations useful for filtering on the manifest itself, rather than the layer. Annotations on the manifest are pulled up in the referrers response, allowing you to pick the dsse envelope artifact from a list of artifacts all created for the same image.

This is a bit different from the current cosign design where there's one attestation manifest with multiple layers. The goal of the OCI design is to make it easier to add and remove entries, especially when the registry is managing the referrers response.