Refactor Verifiers to return multiple keys (#1601)

This supports DSSE and intoto types that allow for multiple keys/signatures. Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
sigstore · Jul 28, 2023 · cbc9c44 · cbc9c44
1 parent 8a30776
commit cbc9c44
Show file tree

Hide file tree

Showing 35 changed files with 125 additions and 72 deletions.
diff --git a/pkg/types/alpine/alpine_test.go b/pkg/types/alpine/alpine_test.go
@@ -43,7 +43,7 @@ func (u UnmarshalFailsTester) Unmarshal(_ models.ProposedEntry) error {
 	return errors.New("error")
 }
 
-func (u UnmarshalFailsTester) Verifier() (pki.PublicKey, error) {
+func (u UnmarshalFailsTester) Verifiers() ([]pki.PublicKey, error) {
 	return nil, nil
 }
 

diff --git a/pkg/types/alpine/v0.0.1/entry.go b/pkg/types/alpine/v0.0.1/entry.go
@@ -351,11 +351,15 @@ func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types
 	return &returnVal, nil
 }
 
-func (v V001Entry) Verifier() (pki.PublicKey, error) {
+func (v V001Entry) Verifiers() ([]pki.PublicKey, error) {
 	if v.AlpineModel.PublicKey == nil || v.AlpineModel.PublicKey.Content == nil {
 		return nil, errors.New("alpine v0.0.1 entry not initialized")
 	}
-	return x509.NewPublicKey(bytes.NewReader(*v.AlpineModel.PublicKey.Content))
+	key, err := x509.NewPublicKey(bytes.NewReader(*v.AlpineModel.PublicKey.Content))
+	if err != nil {
+		return nil, err
+	}
+	return []pki.PublicKey{key}, nil
 }
 
 func (v V001Entry) Insertable() (bool, error) {

diff --git a/pkg/types/alpine/v0.0.1/entry_test.go b/pkg/types/alpine/v0.0.1/entry_test.go
@@ -179,19 +179,19 @@ func TestCrossFieldValidation(t *testing.T) {
 			}
 		}
 
-		verifier, err := v.Verifier()
+		verifiers, err := v.Verifiers()
 		if tc.expectedVerifierSuccess {
 			if err != nil {
 				t.Errorf("%v: unexpected error, got %v", tc.caseDesc, err)
 			} else {
-				pub, _ := verifier.CanonicalValue()
+				pub, _ := verifiers[0].CanonicalValue()
 				if !reflect.DeepEqual(pub, keyBytes) {
 					t.Errorf("%v: verifier and public keys do not match: %v, %v", tc.caseDesc, string(pub), string(keyBytes))
 				}
 			}
 		} else {
 			if err == nil {
-				s, _ := verifier.CanonicalValue()
+				s, _ := verifiers[0].CanonicalValue()
 				t.Errorf("%v: expected error for %v, got %v", tc.caseDesc, string(s), err)
 			}
 		}

diff --git a/pkg/types/cose/cose_test.go b/pkg/types/cose/cose_test.go
@@ -44,7 +44,7 @@ func (u UnmarshalFailsTester) Unmarshal(_ models.ProposedEntry) error {
 	return errors.New("error")
 }
 
-func (u UnmarshalFailsTester) Verifier() (pki.PublicKey, error) {
+func (u UnmarshalFailsTester) Verifiers() ([]pki.PublicKey, error) {
 	return nil, nil
 }
 

diff --git a/pkg/types/cose/v0.0.1/entry.go b/pkg/types/cose/v0.0.1/entry.go
@@ -348,11 +348,15 @@ func (v V001Entry) CreateFromArtifactProperties(_ context.Context, props types.A
 	return &returnVal, nil
 }
 
-func (v V001Entry) Verifier() (pki.PublicKey, error) {
+func (v V001Entry) Verifiers() ([]pki.PublicKey, error) {
 	if v.CoseObj.PublicKey == nil {
 		return nil, errors.New("cose v0.0.1 entry not initialized")
 	}
-	return x509.NewPublicKey(bytes.NewReader(*v.CoseObj.PublicKey))
+	key, err := x509.NewPublicKey(bytes.NewReader(*v.CoseObj.PublicKey))
+	if err != nil {
+		return nil, err
+	}
+	return []pki.PublicKey{key}, nil
 }
 
 func (v V001Entry) Insertable() (bool, error) {

diff --git a/pkg/types/cose/v0.0.1/entry_test.go b/pkg/types/cose/v0.0.1/entry_test.go
@@ -278,10 +278,10 @@ func TestV001Entry_Unmarshal(t *testing.T) {
 				}
 			}
 
-			verifier, err := v.Verifier()
+			verifiers, err := v.Verifiers()
 			if !tt.wantVerifierErr {
 				if err != nil {
-					s, _ := verifier.CanonicalValue()
+					s, _ := verifiers[0].CanonicalValue()
 					t.Errorf("%v: unexpected error for %v, got %v", tt.name, string(s), err)
 				}
 
@@ -305,12 +305,12 @@ func TestV001Entry_Unmarshal(t *testing.T) {
 					}
 				}
 
-				pubV, _ := verifier.CanonicalValue()
+				pubV, _ := verifiers[0].CanonicalValue()
 				if !reflect.DeepEqual(pubV, pub) && !reflect.DeepEqual(pubV, pemBytes) {
 					t.Errorf("verifier and public keys do not match: %v, %v", string(pubV), string(pub))
 				}
 			} else if err == nil {
-				s, _ := verifier.CanonicalValue()
+				s, _ := verifiers[0].CanonicalValue()
 				t.Errorf("%v: expected error for %v, got %v", tt.name, string(s), err)
 			}
 		})

diff --git a/pkg/types/dsse/dsse_test.go b/pkg/types/dsse/dsse_test.go
@@ -44,7 +44,7 @@ func (u UnmarshalFailsTester) Unmarshal(_ models.ProposedEntry) error {
 	return errors.New("error")
 }
 
-func (u UnmarshalFailsTester) Verifier() (pki.PublicKey, error) {
+func (u UnmarshalFailsTester) Verifiers() ([]pki.PublicKey, error) {
 	return nil, nil
 }
 

diff --git a/pkg/types/dsse/v0.0.1/entry.go b/pkg/types/dsse/v0.0.1/entry.go
@@ -403,13 +403,20 @@ func verifyEnvelope(allPubKeyBytes [][]byte, env *dsse.Envelope) (map[string]*x5
 	return verifierBySig, nil
 }
 
-func (v V001Entry) Verifier() (pki.PublicKey, error) {
+func (v V001Entry) Verifiers() ([]pki.PublicKey, error) {
 	if len(v.DSSEObj.Signatures) == 0 {
 		return nil, errors.New("dsse v0.0.1 entry not initialized")
 	}
 
-	//TODO: return multiple pki.PublicKeys; sigstore/rekor issue #1278
-	return x509.NewPublicKey(bytes.NewReader(*v.DSSEObj.Signatures[0].Verifier))
+	var keys []pki.PublicKey
+	for _, s := range v.DSSEObj.Signatures {
+		key, err := x509.NewPublicKey(bytes.NewReader(*s.Verifier))
+		if err != nil {
+			return nil, err
+		}
+		keys = append(keys, key)
+	}
+	return keys, nil
 }
 
 func (v V001Entry) Insertable() (bool, error) {

diff --git a/pkg/types/entries.go b/pkg/types/entries.go
@@ -37,7 +37,7 @@ type EntryImpl interface {
 	Canonicalize(ctx context.Context) ([]byte, error) // marshal the canonical entry to be put into the tlog
 	Unmarshal(e models.ProposedEntry) error           // unmarshal the abstract entry into the specific struct for this versioned type
 	CreateFromArtifactProperties(context.Context, ArtifactProperties) (models.ProposedEntry, error)
-	Verifier() (pki.PublicKey, error)
+	Verifiers() ([]pki.PublicKey, error)
 	Insertable() (bool, error) // denotes whether the entry that was unmarshalled has the writeOnly fields required to validate and insert into the log
 }
 

diff --git a/pkg/types/hashedrekord/hashedrekord_test.go b/pkg/types/hashedrekord/hashedrekord_test.go
@@ -43,7 +43,7 @@ func (u UnmarshalFailsTester) Unmarshal(_ models.ProposedEntry) error {
 	return errors.New("error")
 }
 
-func (u UnmarshalFailsTester) Verifier() (pki.PublicKey, error) {
+func (u UnmarshalFailsTester) Verifiers() ([]pki.PublicKey, error) {
 	return nil, nil
 }
 

diff --git a/pkg/types/hashedrekord/v0.0.1/entry.go b/pkg/types/hashedrekord/v0.0.1/entry.go
@@ -246,11 +246,15 @@ func (v V001Entry) CreateFromArtifactProperties(_ context.Context, props types.A
 	return &returnVal, nil
 }
 
-func (v V001Entry) Verifier() (pki.PublicKey, error) {
+func (v V001Entry) Verifiers() ([]pki.PublicKey, error) {
 	if v.HashedRekordObj.Signature == nil || v.HashedRekordObj.Signature.PublicKey == nil || v.HashedRekordObj.Signature.PublicKey.Content == nil {
 		return nil, errors.New("hashedrekord v0.0.1 entry not initialized")
 	}
-	return x509.NewPublicKey(bytes.NewReader(v.HashedRekordObj.Signature.PublicKey.Content))
+	key, err := x509.NewPublicKey(bytes.NewReader(v.HashedRekordObj.Signature.PublicKey.Content))
+	if err != nil {
+		return nil, err
+	}
+	return []pki.PublicKey{key}, nil
 }
 
 func (v V001Entry) Insertable() (bool, error) {

diff --git a/pkg/types/hashedrekord/v0.0.1/entry_test.go b/pkg/types/hashedrekord/v0.0.1/entry_test.go
@@ -315,20 +315,20 @@ func TestCrossFieldValidation(t *testing.T) {
 			}
 		}
 
-		verifier, err := v.Verifier()
+		verifiers, err := v.Verifiers()
 		if tc.expectedVerifierSuccess {
 			if err != nil {
 				t.Errorf("%v: unexpected error, got %v", tc.caseDesc, err)
 			} else {
-				pub, _ := verifier.CanonicalValue()
+				pub, _ := verifiers[0].CanonicalValue()
 				// invalidKeyBytes is a valid ed25519 key
 				if !reflect.DeepEqual(pub, keyBytes) && !reflect.DeepEqual(pub, invalidKeyBytes) {
 					t.Errorf("verifier and public keys do not match: %v, %v", string(pub), string(keyBytes))
 				}
 			}
 		} else {
 			if err == nil {
-				s, _ := verifier.CanonicalValue()
+				s, _ := verifiers[0].CanonicalValue()
 				t.Errorf("%v: expected error for %v, got %v", tc.caseDesc, string(s), err)
 			}
 		}

diff --git a/pkg/types/helm/helm_test.go b/pkg/types/helm/helm_test.go
@@ -43,7 +43,7 @@ func (u UnmarshalFailsTester) Unmarshal(_ models.ProposedEntry) error {
 	return errors.New("error")
 }
 
-func (u UnmarshalFailsTester) Verifier() (pki.PublicKey, error) {
+func (u UnmarshalFailsTester) Verifiers() ([]pki.PublicKey, error) {
 	return nil, nil
 }
 

diff --git a/pkg/types/helm/v0.0.1/entry.go b/pkg/types/helm/v0.0.1/entry.go
@@ -349,11 +349,15 @@ func (v V001Entry) CreateFromArtifactProperties(ctx context.Context, props types
 	return &returnVal, nil
 }
 
-func (v V001Entry) Verifier() (pki.PublicKey, error) {
+func (v V001Entry) Verifiers() ([]pki.PublicKey, error) {
 	if v.HelmObj.PublicKey == nil || v.HelmObj.PublicKey.Content == nil {
 		return nil, errors.New("helm v0.0.1 entry not initialized")
 	}
-	return pgp.NewPublicKey(bytes.NewReader(*v.HelmObj.PublicKey.Content))
+	key, err := pgp.NewPublicKey(bytes.NewReader(*v.HelmObj.PublicKey.Content))
+	if err != nil {
+		return nil, err
+	}
+	return []pki.PublicKey{key}, nil
 }
 
 func (v V001Entry) Insertable() (bool, error) {

diff --git a/pkg/types/helm/v0.0.1/entry_test.go b/pkg/types/helm/v0.0.1/entry_test.go
@@ -211,20 +211,20 @@ func TestCrossFieldValidation(t *testing.T) {
 				}
 			}
 
-			verifier, err := v.Verifier()
+			verifiers, err := v.Verifiers()
 			if tc.expectedVerifierSuccess {
 				if err != nil {
 					t.Errorf("%v: unexpected error, got %v", tc.caseDesc, err)
 				} else {
 					// TODO: Improve this test once CanonicalValue returns same result as input for PGP keys
-					_, err := verifier.CanonicalValue()
+					_, err := verifiers[0].CanonicalValue()
 					if err != nil {
 						t.Errorf("%v: unexpected error getting canonical value, got %v", tc.caseDesc, err)
 					}
 				}
 			} else {
 				if err == nil {
-					s, _ := verifier.CanonicalValue()
+					s, _ := verifiers[0].CanonicalValue()
 					t.Errorf("%v: expected error for %v, got %v", tc.caseDesc, string(s), err)
 				}
 			}

diff --git a/pkg/types/intoto/intoto_test.go b/pkg/types/intoto/intoto_test.go
@@ -43,7 +43,7 @@ func (u UnmarshalFailsTester) Unmarshal(_ models.ProposedEntry) error {
 	return errors.New("error")
 }
 
-func (u UnmarshalFailsTester) Verifier() (pki.PublicKey, error) {
+func (u UnmarshalFailsTester) Verifiers() ([]pki.PublicKey, error) {
 	return nil, nil
 }
 

diff --git a/pkg/types/intoto/v0.0.1/entry.go b/pkg/types/intoto/v0.0.1/entry.go
@@ -353,11 +353,15 @@ func (v V001Entry) CreateFromArtifactProperties(_ context.Context, props types.A
 	return &returnVal, nil
 }
 
-func (v V001Entry) Verifier() (pki.PublicKey, error) {
+func (v V001Entry) Verifiers() ([]pki.PublicKey, error) {
 	if v.IntotoObj.PublicKey == nil {
 		return nil, errors.New("intoto v0.0.1 entry not initialized")
 	}
-	return x509.NewPublicKey(bytes.NewReader(*v.IntotoObj.PublicKey))
+	key, err := x509.NewPublicKey(bytes.NewReader(*v.IntotoObj.PublicKey))
+	if err != nil {
+		return nil, err
+	}
+	return []pki.PublicKey{key}, nil
 }
 
 func (v V001Entry) Insertable() (bool, error) {

diff --git a/pkg/types/intoto/v0.0.1/entry_test.go b/pkg/types/intoto/v0.0.1/entry_test.go
@@ -336,19 +336,19 @@ func TestV001Entry_Unmarshal(t *testing.T) {
 					t.Errorf("index keys from hydrated object do not match those generated from canonicalized (and re-hydrated) object: %v %v", got, canonicalIndexKeys)
 				}
 
-				verifier, err := v.Verifier()
+				verifiers, err := v.Verifiers()
 				if !tt.wantVerifierErr {
 					if err != nil {
 						t.Errorf("%v: unexpected error, got %v", tt.name, err)
 					} else {
-						pubV, _ := verifier.CanonicalValue()
+						pubV, _ := verifiers[0].CanonicalValue()
 						if !reflect.DeepEqual(pubV, pub) && !reflect.DeepEqual(pubV, pemBytes) {
 							t.Errorf("verifier and public keys do not match: %v, %v", string(pubV), string(pub))
 						}
 					}
 				} else {
 					if err == nil {
-						s, _ := verifier.CanonicalValue()
+						s, _ := verifiers[0].CanonicalValue()
 						t.Errorf("%v: expected error for %v, got %v", tt.name, string(s), err)
 					}
 				}

diff --git a/pkg/types/intoto/v0.0.2/entry.go b/pkg/types/intoto/v0.0.2/entry.go
@@ -457,7 +457,7 @@ func verifyEnvelope(allPubKeyBytes [][]byte, env *dsse.Envelope) (map[string]*x5
 	return verifierBySig, nil
 }
 
-func (v V002Entry) Verifier() (pki.PublicKey, error) {
+func (v V002Entry) Verifiers() ([]pki.PublicKey, error) {
 	if v.IntotoObj.Content == nil || v.IntotoObj.Content.Envelope == nil {
 		return nil, errors.New("intoto v0.0.2 entry not initialized")
 	}
@@ -467,7 +467,15 @@ func (v V002Entry) Verifier() (pki.PublicKey, error) {
 		return nil, errors.New("no signatures found on intoto entry")
 	}
 
-	return x509.NewPublicKey(bytes.NewReader(*v.IntotoObj.Content.Envelope.Signatures[0].PublicKey))
+	var keys []pki.PublicKey
+	for _, s := range v.IntotoObj.Content.Envelope.Signatures {
+		key, err := x509.NewPublicKey(bytes.NewReader(*s.PublicKey))
+		if err != nil {
+			return nil, err
+		}
+		keys = append(keys, key)
+	}
+	return keys, nil
 }
 
 func (v V002Entry) Insertable() (bool, error) {

diff --git a/pkg/types/intoto/v0.0.2/entry_test.go b/pkg/types/intoto/v0.0.2/entry_test.go
@@ -377,19 +377,19 @@ func TestV002Entry_Unmarshal(t *testing.T) {
 					t.Errorf("index keys from hydrated object do not match those generated from canonicalized (and re-hydrated) object: %v %v", got, canonicalIndexKeys)
 				}
 
-				verifier, err := v.Verifier()
+				verifiers, err := v.Verifiers()
 				if !tt.wantVerifierErr {
 					if err != nil {
 						t.Errorf("%v: unexpected error, got %v", tt.name, err)
 					} else {
-						pubV, _ := verifier.CanonicalValue()
+						pubV, _ := verifiers[0].CanonicalValue()
 						if !reflect.DeepEqual(pubV, pub) && !reflect.DeepEqual(pubV, pemBytes) {
 							t.Errorf("verifier and public keys do not match: %v, %v", string(pubV), string(pub))
 						}
 					}
 				} else {
 					if err == nil {
-						s, _ := verifier.CanonicalValue()
+						s, _ := verifiers[0].CanonicalValue()
 						t.Errorf("%v: expected error for %v, got %v", tt.name, string(s), err)
 					}
 				}

diff --git a/pkg/types/jar/jar_test.go b/pkg/types/jar/jar_test.go
@@ -42,7 +42,7 @@ func (u UnmarshalFailsTester) Unmarshal(_ models.ProposedEntry) error {
 	return errors.New("error")
 }
 
-func (u UnmarshalFailsTester) Verifier() (pki.PublicKey, error) {
+func (u UnmarshalFailsTester) Verifiers() ([]pki.PublicKey, error) {
 	return nil, nil
 }
 

diff --git a/pkg/types/jar/v0.0.1/entry.go b/pkg/types/jar/v0.0.1/entry.go
@@ -338,11 +338,15 @@ func (v *V001Entry) CreateFromArtifactProperties(ctx context.Context, props type
 	return &returnVal, nil
 }
 
-func (v V001Entry) Verifier() (pki.PublicKey, error) {
+func (v V001Entry) Verifiers() ([]pki.PublicKey, error) {
 	if v.JARModel.Signature == nil || v.JARModel.Signature.PublicKey == nil || v.JARModel.Signature.PublicKey.Content == nil {
 		return nil, errors.New("jar v0.0.1 entry not initialized")
 	}
-	return x509.NewPublicKey(bytes.NewReader(*v.JARModel.Signature.PublicKey.Content))
+	key, err := x509.NewPublicKey(bytes.NewReader(*v.JARModel.Signature.PublicKey.Content))
+	if err != nil {
+		return nil, err
+	}
+	return []pki.PublicKey{key}, nil
 }
 
 func (v V001Entry) Insertable() (bool, error) {

diff --git a/pkg/types/jar/v0.0.1/entry_test.go b/pkg/types/jar/v0.0.1/entry_test.go
@@ -142,19 +142,19 @@ Hr/+CxFvaJWmpYqNkLDGRU+9orzh5hI2RrcuaQ==
 			}
 		}
 
-		verifier, err := v.Verifier()
+		verifiers, err := v.Verifiers()
 		if tc.expectedVerifierSuccess {
 			if err != nil {
 				t.Errorf("%v: unexpected error, got %v", tc.caseDesc, err)
 			} else {
-				pub, _ := verifier.CanonicalValue()
+				pub, _ := verifiers[0].CanonicalValue()
 				if !reflect.DeepEqual(pub, []byte(certificate)) {
 					t.Errorf("verifier and public keys do not match: %v, %v", string(pub), certificate)
 				}
 			}
 		} else {
 			if err == nil {
-				s, _ := verifier.CanonicalValue()
+				s, _ := verifiers[0].CanonicalValue()
 				t.Errorf("%v: expected error for %v, got %v", tc.caseDesc, string(s), err)
 			}
 		}