sigstore / cosign

Code signing and transparency for containers and binaries
Apache License 2.0
4.54k stars 547 forks source link

[cosign/pkg] High-level, pluggable, composable, `cosign`ing and verification flows #844

Open dekkagaijin opened 3 years ago

dekkagaijin commented 3 years ago

Right now cosign as a package is tightly coupled with its signing and verification implementation:

This leads to significant amounts of control flow being required to properly implement even canonical signing/verifying operations.

As a follow-up to @mattmoor's work to bury OCI registry-specific implementation under a more generic API, we should also consider how we could present individual operations (e.g. fulcio, rekor upload/verification) as generic, pluggable parts of a high-level control flow.

dekkagaijin commented 3 years ago

The imagined high-level API would be something like:

type SigningClient struct {
  steps []SigningOp
}

func (s *SigningClient) Sign(Signable) (Sig, error) {
  ...
}

func (s *SigningClient) Attest(Signable, Attestation) (Sig, error) {
  ...
}

type VerificationClient struct {
  steps []VerificationOp
}

func (v *VerificationClient) VerifySig(Verifiable) (error) {
  ...
}

func (v *VerificationClient) VerifyAttestation(Verifiable, Att) (error) {
  ...
}

or possibly, for the more functional people in the audience:

func  Sign(Signable, ops SignOp...) (Sig, error) {
  ...
}

func Attest(Signable, Attestation, ops SignOp...) (Sig, error) {
  ...
}

The inputs would be presented to steps in turn and results passed to subsequent steps. This way we can fully abstract away interactions with storage, eliminate complex control flow related to upload/verification with fulcio and rekor, and allow for consumers to easily insert their own logic (e.g. publish to PubSub)

mattmoor commented 3 years ago

I think the key bit here is that we should decompose the CLI spaghetti monster around a few key interfaces:

type Signer interface {
   Sign(Signable) (Sig, error)
   Attest(Signable, Attestation) (Sig, error)
}

type Verifier interface {
   ...
}

We could split out implementations of these interfaces into separate packages for:

  1. Local public/private key
  2. KMS's (including K8s secrets as faux-KMS)
  3. Fulcio
  4. Rekor (composed with one of the above)

The over-arching goal would be for the CLI to effectively be a glorified orchestrator that composes these packages based on options, but that downstream library consumers with more focused intentions could compose specific elements more concretely (e.g. if I know I want Fulcio, I don't need to vendor every KMS system's SDK).

luhring commented 3 years ago

I'm late in adding thoughts, but here's a first pass of questions after trying to use the existing functions (under cli) and looking at this issue again...

  1. For signing/attesting, can the data to be signed be an io.Reader instead of a file path? This is useful when the consumer wants to find the input bytes on their own, and specifically (like in Syft's case) when the input bytes have never touched the filesystem.
  2. What might Signable look like? (or is this already defined today somewhere?)
  3. It looks like both Attest and Sign return the same thing —Sig — but how will consumers get access to the attestation data structure (e.g. envelope) for their own use?

Let me know if there's a place to find the latest state of this work, and if it'd be helpful for me to grab some tasks on this refactor as well. ✌️

luhring commented 3 years ago

I'm wondering if perhaps the Signer interface shouldn't have an Attest method... 🤔 It's not clear to me yet that consumers of the Signer interface have a need for a single object to perform both Sign and Attest. And it could be that implementers of the Signer interface don't have a need to implement their own attestation functionality.

IIUC, implementations of Signer have no logic that's unique to themselves for the Attest operation, aside from their implementation of the Sign operation. Attest could conceivably be a static function, that takes a Signer as a parameter.

A related question is: should both Sign and Attest return a result of the same type Sig (or Signature) as proposed above? Is this a reference to the existing oci.Signature type?

My suspicion is that callers of Attest would like to use the return type in some ways that are unique to attestations (and not signatures), such as getting the predicate type, or even type asserting a more specific attestation type on the payload. If we stick with relying on a Payload() ([]byte, error) signature (taken from the oci.Signature type), I believe that means that callers need to either get a strongly typed attestation from the []byte in a type-unsafe manner, or they need to type assert the entire Signature into an attestation, which means all attestations need to implement all of the behaviors specified in the Signature interface, neither of which seem great. I may be overlooking something...

I'd love to hear others' latest thinking. ❤️

In the meantime, I plan to open a draft PR soon with some of my ideas thus far, intended for open-season critique!

luhring commented 3 years ago

We could split out implementations of these interfaces into separate packages for [...]

Question: Should we consider moving some part of this to sigstore/sigstore? A lot of the logic that's getting changed seems to be pure signature logic, as opposed to logic that's coupled closely with container images and registries.

dekkagaijin commented 3 years ago

I'm wondering if perhaps the Signer interface shouldn't have an Attest method

Splitting Signature and Attestation is one avenue to disentangle to the current spaghetti. It's one we'll probably go down. However, 'signature' and 'signed thing' are the fundamental, atomic units we care about.

Should we consider moving some part of this to sigstore/sigstore?

Yes and no. We are dealing with signatures on arbitrary inputs, which just so happen to be living in an OCI world. I'd argue that OCI related functionality should stay within cosign unless there's a very good reason to declare it a core component of the overall strategy

luhring commented 3 years ago

I'm wondering if perhaps the Signer interface shouldn't have an Attest method

Splitting Signature and Attestation is one avenue to disentangle to the current spaghetti. It's one we'll probably go down. However, 'signature' and 'signed thing' are the fundamental, atomic units we care about.

This makes sense. I think this is a separate question from "which methods does the Signer interface specify?". Regardless of where Sign and Attest functions appear, we can have them both return a Signature.

Should we consider moving some part of this to sigstore/sigstore?

Yes and no. We are dealing with signatures on arbitrary inputs, which just so happen to be living in an OCI world. I'd argue that OCI related functionality should stay within cosign unless there's a very good reason to declare it a core component of the overall strategy

This seems like the right distinction. So maybe "yes" to "some part", but only for logic that's truly decoupled from the OCI world.

Thanks for the response, @dekkagaijin!