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

cosign as a library #666

Closed
mattmoor opened this issue Sep 14, 2021 · 20 comments · Fixed by #770
Closed

cosign as a library #666

mattmoor opened this issue Sep 14, 2021 · 20 comments · Fixed by #770

Comments

@mattmoor
Copy link
Member

Description

tl;dr I propose the creation of a well-curated set of libraries for signing, that compose extremely well with and complement github.com/google/go-containerregistry for signing OCI images and indices.

cosign has gained considerable traction as a mechanism for signing OCI artifacts as a standalone executable. I believe that to kick this into an even higher-gear, we want to encourage ecosystem tooling to natively integrate cosign as a library to naturally extend image production to include signing.

cosign (along with a growing list of other tooling) builds around github.com/google/go-containerregistry (aka "GGCR") for interacting with OCI images and indices, so this isn't a huge reach, it is mostly refactoring the code we have today to better align with the way we do things in GGCR so that it is trivially simple for folks to adopt signing, if they've adopted GGCR.


One of the directions I'd like to see us push the library portion of this is to align with the "philosophy" of GGCR, in part to enable a wider variety of use cases.

For example, today cosign is very oriented around registry-based images. Despite it's name GGCR actually allows interacting with images in a wide variety of formats, including things like docker save-tarballs, OCI-layouts, and more.

If cosign were to adhere to some of the principles of GGCR, we might see signing take the form of (v1.Image, Options) -> v1.Image, where the input image is what we want signed and the output image is the signature payload we want recorded somewhere.

With this separation of the signing from the storage, consumers can then leverage a variety of mechanisms to publish the resulting image in different ways. They can choose to remote.Write that image to a registry following cosign's naming convention, or they can tarball.Write.


GGCR is all about enabling cool new things thru composition, and so this is very much about making sure cosign becomes a tool compatible with that ecosystem of composition!

I am still wrapping my head around everything that cosign does, so this picture is starting to form in my head, but if folks are receptive to this (or better willing to help) then I'd love to discuss this direction.

cc @jonjohnsonjr @imjasonh @dekkagaijin @dlorenc

@dlorenc
Copy link
Member

dlorenc commented Sep 14, 2021

@jonjohnsonjr who could have seen this coming?

@mattmoor
Copy link
Member Author

Only someone with vision 😏

@dekkagaijin
Copy link
Member

Gonna add another +1 here to the idea of composition :p

@sambhav
Copy link
Contributor

sambhav commented Sep 15, 2021

+1 This would help a lot with integrations into the Buildpacks ecosystem. Our lifecycle and various platforms almost all use/heavily rely on GGCR. cc: @natalieparellano @matthewmcnew @jromero

@cpanato
Copy link
Member

cpanato commented Sep 15, 2021

+1

mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 16, 2021
Cosign has it's own flavor of GGCR's `empty` package, which lets users toggle the media type we use with an environment variable.  This creates our own package to mirror this.

This also start to set up an `internal/` package structure that we can use to develop the new package structure without (yet) worrying about breaking folks downstream when we change things.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
@mattmoor
Copy link
Member Author

I'm gonna start small. I am staging a PR to create ./internal/oci/empty to host our own flavor of empty.Image().

I think we should use internal/ to keep folks from adopting this prematurely, and let us move quickly if we want to change things (possibly dramatically). Once we're happy, we can move this out and tread more carefully.

dlorenc pushed a commit that referenced this issue Sep 16, 2021
Cosign has it's own flavor of GGCR's `empty` package, which lets users toggle the media type we use with an environment variable.  This creates our own package to mirror this.

This also start to set up an `internal/` package structure that we can use to develop the new package structure without (yet) worrying about breaking folks downstream when we change things.

Related: #666
Signed-off-by: Matt Moore <[email protected]>
@mattmoor
Copy link
Member Author

I think the next step (for tomorrow) will be to set up a ./internal/oci/signed.Image, which constructs the signature artifact itself.

The way I'm thinking about it now, it should take functional options for a few things, like:

  • Signer - maybe this would default to the keyless signing flow (or be required for now?)
  • Existing signature image - this would default to the new empty.Image() if unspecified.

There may be other things as well, but the less variable/optional things would likely just be passed directly.

I'm hoping this will let us reduce UploadSignature to setting up for signed.Image and then feeding that to remote.Write 🤞

@imjasonh
Copy link
Member

One naming question: a signed.Image sounds like it would be the original image, plus a Signature() (v1.Image, error) method to get the signed image's signature. Is that what you're intending?

Since signatures are actually just images, you could have a type Signature v1.Image* and a func Sign(v1.Image, Config) (Signature, error), to make it clearer when passing around the signature that it's the signature and not the signed-thing. This would let you have methods that take a Signature and not a v1.Image, for example.

*type Signature v1.Image could also be a struct or interface that embeds v1.Image to add more signature-specific functionality.

@mattmoor
Copy link
Member Author

Yeah, totally +1. I had the same thought as I walked up to bed and made a mental note to change this to ‘signature.Image’, since it is-a signature, where ‘signed.Image’ sounds like it has-a signature.

@mattmoor
Copy link
Member Author

I do like the idea though of extending the Image interface 🤔

Perhaps we have: signature.Image extend v1.Image with accessors for signature related fields and abstract the layer walking etc. Same for attestation.Image.

I also wonder about defining an actual signed.Image that wraps v1.Image and allows folks to associate signature and attestation images. We could have a mutate package to facilitate this.

Then we start to think about things like remote.Get and remote.Write, which could be used to publish a fully signed image with attestations 🤩

I think all of these would be relatively thin shims around ggcr.

@imjasonh
Copy link
Member

signature.Image feels like a misnomer to me -- sure, it's technically stored as an image, but consumers of the package shouldn't have to care about that detail. To them, it's a "signature", and maybe there's some escape hatch to get at its image bits (layers, etc.), but mostly we hope they don't have to.

I also wonder about defining an actual signed.Image that wraps v1.Image and allows folks to associate signature and attestation images. We could have a mutate package to facilitate this.

signed.Image could embed v1.Image and add Signature() (cosign.Signature, error), but then you have to figure out how to handle that if the wrapped v1.Image isn't a remote.Image. You'd need some generalized way to associate things, regardless of remote-ness.

That being said: whatever you do, as long as it's in internal, go nuts. 😆

@mattmoor
Copy link
Member Author

@imjasonh I think it'd be fair for the Signature stuff to be an alias of v1.Image, but not be called Image, this would also let us iterate with some of these highly correlated things within a single package 😅

That being said: whatever you do, as long as it's in internal, go nuts

Yeah, I wanted the flexibility to put something together, let it marinate and then hate it and tear it up for something better 🤣

@mattmoor
Copy link
Member Author

Ok, so I spent some time thinking about this on my whiteboard, and I'm going to try and articulate things here, and then start breaking off pieces.

I think that we want an aggregation type for reasoning about images, their signatures and attestations that extends v1.Image:

// SignedImage represents an OCI Image, complemented with accessors
// for retrieving signed metadata associated with that image.
type SignedImage interface{
   v1.Image

   // Signatures returns the set of signatures currently associated with this
   // image, or the empty equivalent if none are found.
   Signatures() (Signatures, error)

   // Attestations returns the set of attestations currently associated with this
   // image, or the empty equivalent if none are found.
   Attestations() (Attestations, error)
}

// Signatures represents a set of signatures that are associated with a particular
// v1.Image.
type Signatures interface{
   v1.Image // The low-level representation of the signatures

  // TODO: Accessors that build on `v1.Image` to provide higher-level accessors
  // for the signature data that is embedded in the wrapped `v1.Image`
}

// Attestations represents a set of attestations that are associated with a particular
// v1.Image.
type Attestations interface{
   v1.Image // The low-level representation of the attestations

  // TODO: Accessors that build on `v1.Image` to provide higher-level accessors
  // for the attestation data that is embedded in the wrapped `v1.Image`
}

// SignedIndex represents an OCI ImageIndex, complemented with accessors
// for retrieving signed metadata associated with that ImageIndex.
type SignedImageIndex interface{
   v1.ImageIndex

   // SignedImage is the same as Image, but provides accessors for the nested
   // image's signed metadata.
   SignedImage(v1.Hash) (SignedImage, error)

   // SignedImageIndex is the same as ImageIndex, but provides accessors for
   // the nested image index's signed metadata.
   SignedImageIndex(v1.Hash) (SignedImageIndex, error)

   // Signatures returns the set of signatures currently associated with this
   // image, or the empty equivalent if none are found.
   Signatures() (Signatures, error)

   // Attestations returns the set of attestations currently associated with this
   // image, or the empty equivalent if none are found.
   Attestations() (Attestations, error)
}

@mattmoor
Copy link
Member Author

I think my inclination here is to land this as an interface under internal/oci and then we can start to create an implementation for accessing all of this information in a high-level way (moving the logic we have now behind the above interface) under internal/oci/remote

mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 16, 2021
This starts to layout our augmented interfaces for reasoning about signed images and indices.  The next step will be to try and shoe-horn some of the implementation of these that we currently use into a new `internal/oci/remote` package.

A couple (seemingly) superficial things about how this is set up:
1. The layout of this reflects the layout of GGCR.  The interfaces there are in `pkg/v1`, and implementations of them in `pkg/v1/<name>`.
2. By embedding `v1.Image` instead of *just* surfacing the higher-level things, we enable folks to `remote.Write` and `tarball.Write` the signatures, which feels useful.

None of this stuff is carved in stone, so we'll see how far off we are as we start to use this first-stab for realz.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
dekkagaijin pushed a commit that referenced this issue Sep 16, 2021
This starts to layout our augmented interfaces for reasoning about signed images and indices.  The next step will be to try and shoe-horn some of the implementation of these that we currently use into a new `internal/oci/remote` package.

A couple (seemingly) superficial things about how this is set up:
1. The layout of this reflects the layout of GGCR.  The interfaces there are in `pkg/v1`, and implementations of them in `pkg/v1/<name>`.
2. By embedding `v1.Image` instead of *just* surfacing the higher-level things, we enable folks to `remote.Write` and `tarball.Write` the signatures, which feels useful.

None of this stuff is carved in stone, so we'll see how far off we are as we start to use this first-stab for realz.

Related: #666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 17, 2021
1. Move `AttachedImageTag` calls closer to `UploadSignature` (and friends),
1. Make `sbom.go` use `remote.Signatures`,
1. Make `Signature` embed `v1.Layer`.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
dekkagaijin pushed a commit that referenced this issue Sep 17, 2021
1. Move `AttachedImageTag` calls closer to `UploadSignature` (and friends),
2. Make `sbom.go` use `remote.Signatures`,
3. Make `Signature` embed `v1.Layer`.

Related: #666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 18, 2021
These files were starting to feel large in my previous change, but I didn't want to combine a bunch of functionality with breaking everything apart, so I left it large.  This change is just moving logic into separate files.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
@mattmoor
Copy link
Member Author

I'm going to drop the Attestations path for now, which is wholly NYI everywhere, since everyone is just treating these as a different form of oci.Signatures and accessing them by overriding the signature tag suffix.

#769

mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 23, 2021
These are getting complete enough that it's time to move them into `pkg` 🎉

This is essentially three things:
```
git mv internal/oci pkg/
sed -i 's@cosign/internal/oci@cosign/pkg/oci@g' ...
gofmt -w -s ...
```

Fixes: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
dekkagaijin pushed a commit that referenced this issue Sep 23, 2021
These are getting complete enough that it's time to move them into `pkg` 🎉

This is essentially three things:
```
git mv internal/oci pkg/
sed -i 's@cosign/internal/oci@cosign/pkg/oci@g' ...
gofmt -w -s ...
```

Fixes: #666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 23, 2021
This is very similar to the change to `sign`, but for `attest`.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 23, 2021
This is very similar to the change to `sign`, but for `attest`.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 23, 2021
This is very similar to the change to `sign`, but for `attest`.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit that referenced this issue Sep 23, 2021
This is very similar to the change to `sign`, but for `attest`.

Related: #666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 24, 2021
This change retires `cremote.UploadSignature`, as this is the last caller of the old API, and it now uses the new API.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit that referenced this issue Sep 24, 2021
This change retires `cremote.UploadSignature`, as this is the last caller of the old API, and it now uses the new API.

Related: #666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 24, 2021
I know I just killed this off for being NYI, but I don't like the fact that we need `WithSignatureSuffix` to access things.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit that referenced this issue Sep 24, 2021
I know I just killed this off for being NYI, but I don't like the fact that we need `WithSignatureSuffix` to access things.

Related: #666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 24, 2021
Previously, this was always `Signatures()`, but which sense of "signature" was nebulous (e.g. `.sig`, `.att`).

With this change, the interface here changes to take and accessor so folks explicitly specify which one they want, and I exposed `Verify{Signatures,Attestations}` methods with identical function signatures to what we exposed previously.

After this change, the only usage of `ociremote.WithSignatureSuffix` is some logic that deals with SBOMs, which it abusing things a bit, and the whole "attachment" thing is one of the next things I'm going to look at adding some abstraction for.

Related: sigstore#666
Signed-off-by: Matt Moore <[email protected]>
dekkagaijin pushed a commit that referenced this issue Sep 24, 2021
Previously, this was always `Signatures()`, but which sense of "signature" was nebulous (e.g. `.sig`, `.att`).

With this change, the interface here changes to take and accessor so folks explicitly specify which one they want, and I exposed `Verify{Signatures,Attestations}` methods with identical function signatures to what we exposed previously.

After this change, the only usage of `ociremote.WithSignatureSuffix` is some logic that deals with SBOMs, which it abusing things a bit, and the whole "attachment" thing is one of the next things I'm going to look at adding some abstraction for.

Related: #666
Signed-off-by: Matt Moore <[email protected]>
mattmoor added a commit to mattmoor/cosign that referenced this issue Sep 24, 2021
This has been addressed as part of the march towards sigstore#666, but never got cleaned up.

Signed-off-by: Matt Moore <[email protected]>
dlorenc pushed a commit that referenced this issue Sep 24, 2021
This has been addressed as part of the march towards #666, but never got cleaned up.

Signed-off-by: Matt Moore <[email protected]>
This was referenced Sep 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants