cosign icon indicating copy to clipboard operation
cosign copied to clipboard

support verification of the signature with multiple public keys

Open developer-guy opened this issue 4 years ago • 22 comments
trafficstars

Description

The idea is actually came from here^1. We can support verification of the signature with multiple public keys, I couldn't think over the design much yet but, at the end of the day it might look like the following:

$ cosign verify --key awskms:///1234abcd-12ab-34cd-56ef-1234567890ab --key hashivault://testkey --key k8s://default/mysecret  <IMAGE>

cc: @dentrax @dlorenc @JimBugwadia @hectorj2f

developer-guy avatar Oct 25 '21 22:10 developer-guy

Yes, this scenario makes sense and we are planning to donate that implementation to cosigned.

hectorj2f avatar Oct 25 '21 22:10 hectorj2f

As briefly detailed here https://github.com/sigstore/cosign/issues/594#issuecomment-938074341, it will support multiple keys.

hectorj2f avatar Oct 25 '21 22:10 hectorj2f

Regarding AND or OR question, we are today following an OR approach. However I am more inclined towards an AND that follows an airport security checking approach.

hectorj2f avatar Oct 25 '21 22:10 hectorj2f

So, can we start working on the implementation of it with an AND approach, or what are the other concerns should we consider first?

developer-guy avatar Oct 26 '21 10:10 developer-guy

I'd like to hear @dlorenc thoughts about which approach would fit better.

hectorj2f avatar Oct 26 '21 10:10 hectorj2f

I think either one is fine, it worries me that it might not actually be clear though :(

AND is safer since it would "fail closed".

dlorenc avatar Oct 26 '21 12:10 dlorenc

I wonder if it would make sense to move this up to the policy level rather than in flags.

What if you could pass in a simple cue file with public keys, that could contain more complex and/or logic:

  • These two keys, or that one
  • 3 out of these 5

etc.

dlorenc avatar Oct 26 '21 12:10 dlorenc

Also: I think there are at least three places we can handle this (maybe differently!):

  • Cosigned (support for OR makes sense here because of rotation)
  • the CLI
  • the libraries

dlorenc avatar Oct 26 '21 12:10 dlorenc

AND is safer but OR is a valid use case.

"admit the pod if the image was signed by this system/team or that system/team or this vendor" etc.

probably more common than the AND use case.

my position is that the user should have the choice to configure AND or OR, or 3 of 5, or at least 2, or whatever.

adamd-vmw avatar Oct 26 '21 17:10 adamd-vmw

For the CLI, would it be fair to have flags specifying the policy (i.e., --policy-type any-of/all-of) and a threshold number (i.e., --threshold 1)?

stormqueen1990 avatar Oct 26 '21 17:10 stormqueen1990

AND is safer but OR is a valid use case.

Yep... in the CLI where it's implicit I'd feel safer with AND. I'd rather figure out a way to be explicit about it though.

dlorenc avatar Oct 26 '21 17:10 dlorenc

Just asking out of curiosity, sorry if i am off topic. Wouldn't doing something similar for attestations make also sense ? (maybe using envelope and dsse signers that support multi signer flow)

houdini91 avatar Oct 27 '21 13:10 houdini91

Wouldn't doing something similar for attestations make also sense ? (maybe using envelope and dsse signers that support multi signer flow)

Yes probably!

I wrote this up with some other ideas on custom polices: https://gist.github.com/dlorenc/a9681f6c0ed08a7710ba52a7f76887f6

I'd love some early feedback!

dlorenc avatar Oct 29 '21 03:10 dlorenc

Currently cosign.VerifySignatures takes a signature.Verifier and loops through all signatures and invokes the verifier. Can this be refactored to push multiple signatures to a new abstraction that performs the logic to verify the entire set against policies or other declarations?

Something like this:


type Verifier interface {
	 VerifySignatures(signatures []oci.Signature, opts ...signature.VerifyOption) error
}

type MultiKeyVerifier struct {
	// the number of verifiers that must be applied (defaults to 1)
	// - all: min = len(verifiers)
	// - any: min = 1
	// - count (e.g. 2/5): min = count
	min int
	// the list of keys to attempt
	verifiers []Verifier
}

func (mv *MultiKeyVerifier) VerifySignatures(signatures []oci.Signature, opts ...signature.VerifyOption) error {
	// todo - verify signatures based on signature sets
	return nil
}

type CueVerifier struct {
  // todo - declare or construct a Cue policy
}

func (mv *CueVerifier ) VerifySignatures(signatures []oci.Signature, opts ...signature.VerifyOption) error {
	// todo - verify signatures based on Cue policy
        return nil
}

We can then support different verifiers e.g. CueVerifier, MultiKeyVerifier, SingleKeyVerifier, etc. that perform the necessary checks and pass or fail the entire signature set or attestation set.

JimBugwadia avatar Nov 01 '21 20:11 JimBugwadia

Kind ping here 🙏

Dentrax avatar Nov 16 '21 09:11 Dentrax

Issue/PR may be relevant. https://github.com/secure-systems-lab/go-securesystemslib/issues/4 https://github.com/secure-systems-lab/go-securesystemslib/pull/7

houdini91 avatar Nov 17 '21 06:11 houdini91

Hi @houdini91 @Dentrax - is the plan to use the MultiEnvelopeSigner in cosign.VerifySignatures?

JimBugwadia avatar Nov 18 '21 18:11 JimBugwadia

Hi @JimBugwadia, IMHO, there are things that we have to decide what we should do first like:

  • Should it be evaluated AND or OR when we define multiple keys?

  • Should we define a threshold number to make it valid check for defining at least N keys should be evaluated as true?

am I right @dlorenc @hectorj2f ?

developer-guy avatar Nov 19 '21 06:11 developer-guy

Yes, you are right in my opinion.

Should it be evaluated AND or OR when we define multiple keys?

For our use cases, we need to answer this question ☝🏻 which is more important at this moment. It should be configurable because it sounds like a realistic situation at many customers.

Should we define a threshold number to make it valid check for defining at least N keys should be evaluated as true?

I am not sure this is necessary if we decide on using cue to define the AND/OR policy for the keys, as proposed from @dlorenc .

hectorj2f avatar Nov 19 '21 11:11 hectorj2f

@JimBugwadia

Hi @houdini91 @Dentrax - is the plan to use the MultiEnvelopeSigner in cosign.VerifySignatures?

I am down with what ever the community leader think is best. My own view of the support is as following

  1. Add DSSE multi sign/verify support as specified by spec DSSE Multi sign spec PR is in progress 7
  2. Add support to sigstore code base using the MultiEnvelopeSigner and MultiEnvelopeVerifier
  3. Hopefully until this time there will be better answers how cosign multi sign verifying process. I am guessing cosign may decide it wants to pass the verified key list via policy or other logic as well as threshold or maybe they will decide to always use signal envelope signer witch are simply multi signers/verifiers with a default threshold of 1. In any way the basic building block will be (1) and (2).
  • In my opinion Threshold is just a bonus verification option i added it to the PR because it is part of the DSSE spec.
  • Multi sign/verify support should return ALL the accepted keys no matter the threshold value or envelope type.

houdini91 avatar Nov 22 '21 14:11 houdini91

We have a use case for the OR multiple public keys. On testing we can see only AND support currently, has the support for OR based multi-keys stalled over the last 2 years or has it been taken elsewhere?

ls250412 avatar Apr 18 '24 12:04 ls250412

We would also be interested in this feature to verify multiple pub keys. Is there any movement? We have the use case that we are automatically rotating our secrets; currently, we would have to implement the verification of multiple pubkeys with a wrapper around cosign. It would be great if cosing would support multiple pub.keys verification.

Example I have a file example.pub with the following content:

-----BEGIN PUBLIC KEY-----
KEY 1
-----END PUBLIC KEY-----

-----BEGIN PUBLIC KEY-----
KEY 2
-----END PUBLIC KEY-----

If I execute the following command and Key2 is valid I would expect a valid validation:

console cosign verify-attestation --type cyclonedx --key example.pub image:latest

The current behaviour is if Key 1 is valid the the verifications step above is valid but not with a valid Key 2.

mymichu avatar Aug 23 '24 15:08 mymichu