Make unsafe uses of Verify function explicit / make Verify option…

…s safe by default (#3)

This change re-organizes the logic of `SignedEntityVerifier.Verify`'s functional options in order to make unsafe uses of the `Verify` function explicit / make `Verify`'s options safe by default.

After some discussion, it accomplishes this by:
- Introducing `WithoutArtifactUnsafe` and `WithoutIdentitiesUnsafe` policy options
- Reversing the logic that determines whether the `Verify` loop checks for the presence of artifacts and certificate identities; before, the default was to assume we would not check for artifacts or identities. Now that behaviour must be explicitly requested.
- Adding a params struct to `Verify` for encapsulating the optional funcs while giving us the opportunity to expand policy options in the future. 

Individual commits squashed by this merge commit:

* Renamed Verify to VerifyUnsafe.

Introduced VerifyWithArtifact and VerifyWithArtifactDigest to make it
explicit what the preferred usage path is; users should not use
VerifyUnsafe unless they are very very careful and know what they are
doing.

* gofmt -s

* Reverted VerifyUnsafe, VerifyWith* funcs.

* Introduced new WithoutIdentitiesUnsafe, WithoutArtifactUnsafe policy options.

* Updated comments.

* Encapsulated the construction of PolicyConfig inside PolicyBuilder,
added validation checks.

* Made receiver variables consistent with other PolicyOptions.

* Linting.

* Added a length check in case no certificate identities are provided.

* renamed variable to identityPolicies.

* Removed unnecessary pointer deref.

* Renamed VerifierConfigurator type to VerifierOption for consistency with PolicyOption.

* Toned down some of the comments around Unsafe PolicyOptions.

* On 2nd thought, BuildConfig ought to return a pointer.

This way, we can't accidentally use a default PolicyConfig struct.

* Added tests for policy options, made existing test use Safe path.

---------

Signed-off-by: Phill MV <[email protected]>

cmd/conformance/main.go

@@ -151,19 +151,17 @@ func main() {
 			},
 		}
 
-		policyConfig := []verify.PolicyOptionConfigurator{}
+		identityPolicies := []verify.PolicyOption{}
 		if *certOIDC != "" || *certSAN != "" {
 			certID, err := verify.NewShortCertificateIdentity(*certOIDC, *certSAN, "", "")
 			if err != nil {
 				fmt.Println(err)
 				os.Exit(1)
 			}
 
-			policyConfig = append(policyConfig, verify.WithCertificateIdentity(certID))
+			identityPolicies = append(identityPolicies, verify.WithCertificateIdentity(certID))
 		}
 
-		policyConfig = append(policyConfig, verify.WithArtifactDigest("sha256", fileDigest[:]))
-
 		// Load trust root
 		tr := getTrustedRoot()
 
@@ -178,7 +176,7 @@ func main() {
 			log.Fatal(err)
 		}
 
-		_, err = sev.Verify(bun, policyConfig...)
+		_, err = sev.Verify(bun, verify.NewPolicy(verify.WithArtifactDigest("sha256", fileDigest[:]), identityPolicies...))
 		if err != nil {
 			log.Fatal(err)
 		}
@@ -209,15 +207,15 @@ func main() {
 		}
 
 		// Configure verification options
-		policyConfig := []verify.PolicyOptionConfigurator{verify.WithArtifact(file)}
+		identityPolicies := []verify.PolicyOption{}
 		if *certOIDC != "" || *certSAN != "" {
 			certID, err := verify.NewShortCertificateIdentity(*certOIDC, *certSAN, "", "")
 			if err != nil {
 				fmt.Println(err)
 				os.Exit(1)
 			}
 
-			policyConfig = append(policyConfig, verify.WithCertificateIdentity(certID))
+			identityPolicies = append(identityPolicies, verify.WithCertificateIdentity(certID))
 		}
 
 		// Load trust root
@@ -229,7 +227,7 @@ func main() {
 			log.Fatal(err)
 		}
 
-		_, err = sev.Verify(b, policyConfig...)
+		_, err = sev.Verify(b, verify.NewPolicy(verify.WithArtifact(file), identityPolicies...))
 		if err != nil {
 			log.Fatal(err)
 		}

cmd/sigstore-go/main.go

@@ -95,8 +95,9 @@ func run() error {
 		}
 	}
 
-	verifierConfig := []verify.VerifierConfigurator{}
-	policyConfig := []verify.PolicyOptionConfigurator{}
+	verifierConfig := []verify.VerifierOption{}
+	identityPolicies := []verify.PolicyOption{}
+	var artifactPolicy verify.ArtifactPolicyOption
 
 	verifierConfig = append(verifierConfig, verify.WithSignedCertificateTimestamps(1))
 
@@ -117,7 +118,7 @@ func run() error {
 		if err != nil {
 			return err
 		}
-		policyConfig = append(policyConfig, verify.WithCertificateIdentity(certID))
+		identityPolicies = append(identityPolicies, verify.WithCertificateIdentity(certID))
 	}
 
 	var trustedMaterial = make(root.TrustedMaterialCollection, 0)
@@ -170,17 +171,17 @@ func run() error {
 		if err != nil {
 			return err
 		}
-		policyConfig = append(policyConfig, verify.WithArtifactDigest(*artifactDigestAlgorithm, artifactDigestBytes))
+		artifactPolicy = verify.WithArtifactDigest(*artifactDigestAlgorithm, artifactDigestBytes)
 	}
 	if *artifact != "" {
 		file, err := os.Open(*artifact)
 		if err != nil {
 			return err
 		}
-		policyConfig = append(policyConfig, verify.WithArtifact(file))
+		artifactPolicy = verify.WithArtifact(file)
 	}
 
-	res, err := sev.Verify(b, policyConfig...)
+	res, err := sev.Verify(b, verify.NewPolicy(artifactPolicy, identityPolicies...))
 	if err != nil {
 		return err
 	}

pkg/verify/signature_test.go

@@ -26,6 +26,8 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+var SkipArtifactAndIdentitiesPolicy = verify.NewPolicy(verify.WithoutArtifactUnsafe(), verify.WithoutIdentitiesUnsafe())
+
 func TestSignatureVerifier(t *testing.T) {
 	virtualSigstore, err := ca.NewVirtualSigstore()
 	assert.NoError(t, err)
@@ -70,21 +72,21 @@ func TestEnvelopeSubject(t *testing.T) {
 	verifier, err := verify.NewSignedEntityVerifier(virtualSigstore, verify.WithTransparencyLog(1))
 	assert.NoError(t, err)
 
-	_, err = verifier.Verify(entity)
+	_, err = verifier.Verify(entity, SkipArtifactAndIdentitiesPolicy)
 	assert.NoError(t, err)
 
-	_, err = verifier.Verify(entity, verify.WithArtifact(bytes.NewBufferString(subjectBody)))
+	_, err = verifier.Verify(entity, verify.NewPolicy(verify.WithArtifact(bytes.NewBufferString(subjectBody)), verify.WithoutIdentitiesUnsafe()))
 	assert.NoError(t, err)
 
-	_, err = verifier.Verify(entity, verify.WithArtifactDigest("sha256", digest))
+	_, err = verifier.Verify(entity, verify.NewPolicy(verify.WithArtifactDigest("sha256", digest), verify.WithoutIdentitiesUnsafe()))
 	assert.NoError(t, err)
 
 	// Error: incorrect artifact
-	_, err = verifier.Verify(entity, verify.WithArtifact(bytes.NewBufferString("Hi, I am a different subject!")))
+	_, err = verifier.Verify(entity, verify.NewPolicy(verify.WithArtifact(bytes.NewBufferString("Hi, I am a different subject!")), verify.WithoutIdentitiesUnsafe()))
 	assert.Error(t, err)
 
 	// Error: incorrect digest algorithm
-	_, err = verifier.Verify(entity, verify.WithArtifactDigest("sha512", digest))
+	_, err = verifier.Verify(entity, verify.NewPolicy(verify.WithArtifactDigest("sha512", digest), verify.WithoutIdentitiesUnsafe()))
 	assert.Error(t, err)
 }
 
@@ -99,15 +101,15 @@ func TestSignatureVerifierMessageSignature(t *testing.T) {
 	verifier, err := verify.NewSignedEntityVerifier(virtualSigstore, verify.WithTransparencyLog(1))
 	assert.NoError(t, err)
 
-	result, err := verifier.Verify(entity, verify.WithArtifact(bytes.NewBufferString(artifact)))
+	result, err := verifier.Verify(entity, verify.NewPolicy(verify.WithArtifact(bytes.NewBufferString(artifact)), verify.WithoutIdentitiesUnsafe()))
 	assert.NoError(t, err)
 
 	assert.Equal(t, result.Signature.Certificate.SubjectAlternativeName.Value, "[email protected]")
 	assert.Equal(t, result.VerifiedTimestamps[0].Type, "Tlog")
 
 	// should fail to verify with a different artifact
 	artifact2 := "Hi, I am a different artifact!"
-	result, err = verifier.Verify(entity, verify.WithArtifact(bytes.NewBufferString(artifact2)))
+	result, err = verifier.Verify(entity, verify.NewPolicy(verify.WithArtifact(bytes.NewBufferString(artifact2)), verify.WithoutIdentitiesUnsafe()))
 	assert.Error(t, err)
 	assert.Nil(t, result)
 }