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/pkg] High-level, pluggable, composable, cosigning and verification flows #844

Open
dekkagaijin opened this issue Oct 4, 2021 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@dekkagaijin
Copy link
Member

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

  • Registry-resident OCI containers as the signable object
  • Generating fulcio certs
  • Uploading keys to rekor

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 dekkagaijin added the enhancement New feature or request label Oct 4, 2021
@dekkagaijin
Copy link
Member Author

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
Copy link
Member

mattmoor commented Oct 5, 2021

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
Copy link
Contributor

luhring commented Oct 11, 2021

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
Copy link
Contributor

luhring commented Nov 7, 2021

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
Copy link
Contributor

luhring commented Nov 7, 2021

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
Copy link
Member Author

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
Copy link
Contributor

luhring commented Nov 8, 2021

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!

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

3 participants