revert: fix: repetitive and incorrect log lines on witness verify (…

policy/policy.go

@@ -239,30 +239,30 @@ func (p Policy) Verify(ctx context.Context, opts ...VerifyOption) (bool, map[str
 	resultsByStep := make(map[string]StepResult)
 	for depth := 0; depth < vo.searchDepth; depth++ {
 		for stepName, step := range p.Steps {
-			// initialize the result for this step if it hasn't been already
-			if _, ok := resultsByStep[stepName]; !ok {
-				resultsByStep[stepName] = StepResult{Step: stepName}
-			}
-
+			// Use search to get all the attestations that match the supplied step name and subjects
 			collections, err := vo.verifiedSource.Search(ctx, stepName, vo.subjectDigests, attestationsByStep[stepName])
 			if err != nil {
 				return false, nil, err
 			}
 
 			if len(collections) == 0 {
-				continue
+				collections = append(collections, source.CollectionVerificationResult{Errors: []error{ErrNoCollections{Step: stepName}}})
 			}
 
-			// Verify the functionaries and validate attestations
+			// Verify the functionaries
 			collections = step.checkFunctionaries(collections, trustBundles)
-			stepResult := step.validateAttestations(collections)
 
-			// Merge the results
-			if result, ok := resultsByStep[stepName]; ok {
-				result.Passed = append(result.Passed, stepResult.Passed...)
-				result.Rejected = append(result.Rejected, stepResult.Rejected...)
+			stepResult := step.validateAttestations(collections)
 
-				resultsByStep[stepName] = result
+			// We perform many searches against the same step, so we need to merge the relevant fields
+			if resultsByStep[stepName].Step == "" {
+				resultsByStep[stepName] = stepResult
+			} else {
+				if result, ok := resultsByStep[stepName]; ok {
+					result.Passed = append(result.Passed, stepResult.Passed...)
+					result.Rejected = append(result.Rejected, stepResult.Rejected...)
+					resultsByStep[stepName] = result
+				}
 			}
 
 			for _, coll := range stepResult.Passed {
@@ -325,6 +325,13 @@ func (p Policy) verifyArtifacts(resultsByStep map[string]StepResult) (map[string
 	for _, step := range p.Steps {
 		accepted := false
 		if len(resultsByStep[step.Name].Passed) == 0 {
+			if result, ok := resultsByStep[step.Name]; ok {
+				result.Rejected = append(result.Rejected, RejectedCollection{Reason: fmt.Errorf("failed to verify artifacts for step %s: no passed collections present", step.Name)})
+				resultsByStep[step.Name] = result
+			} else {
+				return nil, fmt.Errorf("failed to find step %s in step results map", step.Name)
+			}
+
 			continue
 		}
 

policy/step.go

@@ -184,7 +184,7 @@ func (s Step) validateAttestations(collectionResults []source.CollectionVerifica
 		if passed {
 			result.Passed = append(result.Passed, collection)
 		} else {
-			r := strings.Join(reasons, "\n - ")
+			r := strings.Join(reasons, ",\n - ")
 			reason := fmt.Sprintf("collection validation failed:\n - %s", r)
 			result.Rejected = append(result.Rejected, RejectedCollection{
 				Collection: collection,

source/memory.go

@@ -22,7 +22,6 @@ import (
 	"os"
 
 	"github.com/in-toto/go-witness/dsse"
-	"github.com/in-toto/go-witness/log"
 )
 
 type ErrDuplicateReference string
@@ -32,7 +31,6 @@ func (e ErrDuplicateReference) Error() string {
 }
 
 type MemorySource struct {
-	searched                   bool
 	envelopesByReference       map[string]CollectionEnvelope
 	referencesByCollectionName map[string][]string
 	subjectDigestsByReference  map[string]map[string]struct{}
@@ -41,7 +39,6 @@ type MemorySource struct {
 
 func NewMemorySource() *MemorySource {
 	return &MemorySource{
-		searched:                   false,
 		envelopesByReference:       make(map[string]CollectionEnvelope),
 		referencesByCollectionName: make(map[string][]string),
 		subjectDigestsByReference:  make(map[string]map[string]struct{}),
@@ -107,13 +104,6 @@ func (s *MemorySource) LoadEnvelope(reference string, env dsse.Envelope) error {
 }
 
 func (s *MemorySource) Search(ctx context.Context, collectionName string, subjectDigests, attestations []string) ([]CollectionEnvelope, error) {
-	if s.searched {
-		log.Debug("skipping memory source search: already performed")
-		return []CollectionEnvelope{}, nil
-	} else {
-		s.searched = true
-	}
-
 	matches := make([]CollectionEnvelope, 0)
 	for _, potentialMatchReference := range s.referencesByCollectionName[collectionName] {
 		env, ok := s.envelopesByReference[potentialMatchReference]
@@ -135,6 +125,20 @@ func (s *MemorySource) Search(ctx context.Context, collectionName string, subjec
 			continue
 		}
 
+		// make sure all the expected attestations appear in the collection
+		attestationsMatched := true
+		indexAttestations := s.attestationsByReference[potentialMatchReference]
+		for _, checkAttestation := range attestations {
+			if _, ok := indexAttestations[checkAttestation]; !ok {
+				attestationsMatched = false
+				break
+			}
+		}
+
+		if !attestationsMatched {
+			continue
+		}
+
 		matches = append(matches, env)
 	}
 

verify_test.go

@@ -0,0 +1,218 @@
+// Copyright 2024 The Witness Contributors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package witness
+
+import (
+	"bytes"
+	"context"
+	"crypto"
+	"crypto/rand"
+	"crypto/rsa"
+	"encoding/json"
+	"fmt"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/in-toto/go-witness/attestation"
+	"github.com/in-toto/go-witness/attestation/commandrun"
+	"github.com/in-toto/go-witness/attestation/material"
+	"github.com/in-toto/go-witness/attestation/product"
+	"github.com/in-toto/go-witness/cryptoutil"
+	"github.com/in-toto/go-witness/dsse"
+	"github.com/in-toto/go-witness/policy"
+	"github.com/in-toto/go-witness/source"
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestVerify(t *testing.T) {
+	policy, functionarySigner := makepolicyRSA(t)
+	policyEnvelope, policySigner := signPolicyRSA(t, policy)
+	policyVerifier, err := policySigner.Verifier()
+	require.NoError(t, err)
+	workingDir := t.TempDir()
+
+	step1Result, err := Run(
+		"step01",
+		RunWithSigners(functionarySigner),
+		RunWithAttestors([]attestation.Attestor{
+			material.New(),
+			commandrun.New(
+				commandrun.WithCommand([]string{"bash", "-c", "echo 'test01' > test.txt"}),
+			),
+			product.New(),
+		}),
+		RunWithAttestationOpts(
+			attestation.WithWorkingDir(workingDir),
+		),
+	)
+	require.NoError(t, err)
+
+	subjects := []cryptoutil.DigestSet{}
+	artifactSubject, err := cryptoutil.CalculateDigestSetFromFile(
+		filepath.Join(workingDir, "test.txt"),
+		[]cryptoutil.DigestValue{
+			{
+				GitOID: false,
+				Hash:   crypto.SHA256,
+			},
+		},
+	)
+	require.NoError(t, err)
+	subjects = append(subjects, artifactSubject)
+
+	step2Result, err := Run(
+		"step02",
+		RunWithSigners(functionarySigner),
+		RunWithAttestors([]attestation.Attestor{
+			material.New(),
+			commandrun.New(
+				commandrun.WithCommand([]string{"bash", "-c", "echo 'test02' >> test.txt"}),
+			),
+			product.New(),
+		}),
+		RunWithAttestationOpts(
+			attestation.WithWorkingDir(workingDir),
+		),
+	)
+	require.NoError(t, err)
+
+	artifactSubject, err = cryptoutil.CalculateDigestSetFromFile(
+		filepath.Join(workingDir, "test.txt"),
+		[]cryptoutil.DigestValue{
+			{
+				GitOID: false,
+				Hash:   crypto.SHA256,
+			},
+		},
+	)
+	require.NoError(t, err)
+	subjects = append(subjects, artifactSubject)
+
+	t.Run("Pass", func(t *testing.T) {
+		memorySource := source.NewMemorySource()
+		require.NoError(t, memorySource.LoadEnvelope("step01", step1Result.SignedEnvelope))
+		require.NoError(t, memorySource.LoadEnvelope("step02", step2Result.SignedEnvelope))
+
+		results, err := Verify(
+			context.Background(),
+			policyEnvelope,
+			[]cryptoutil.Verifier{policyVerifier},
+			VerifyWithCollectionSource(memorySource),
+			VerifyWithSubjectDigests(subjects),
+		)
+
+		require.NoError(t, err, fmt.Sprintf("failed with results: %+v", results))
+	})
+
+	t.Run("Fail with missing collection", func(t *testing.T) {
+		memorySource := source.NewMemorySource()
+		require.NoError(t, memorySource.LoadEnvelope("step01", step1Result.SignedEnvelope))
+
+		results, err := Verify(
+			context.Background(),
+			policyEnvelope,
+			[]cryptoutil.Verifier{policyVerifier},
+			VerifyWithCollectionSource(memorySource),
+			VerifyWithSubjectDigests(subjects),
+		)
+
+		require.Error(t, err, fmt.Sprintf("passed with results: %+v", results))
+	})
+}
+
+func makepolicy(functionary policy.Functionary, publicKey policy.PublicKey, roots map[string]policy.Root) policy.Policy {
+	step01 := policy.Step{
+		Name:          "step01",
+		Functionaries: []policy.Functionary{functionary},
+		Attestations:  []policy.Attestation{{Type: commandrun.Type}},
+	}
+
+	step02 := policy.Step{
+		Name:          "step02",
+		Functionaries: []policy.Functionary{functionary},
+		Attestations:  []policy.Attestation{{Type: commandrun.Type}},
+		ArtifactsFrom: []string{"step01"},
+	}
+
+	p := policy.Policy{
+		Expires:    metav1.Time{Time: time.Now().Add(1 * time.Hour)},
+		PublicKeys: map[string]policy.PublicKey{},
+		Steps:      map[string]policy.Step{},
+	}
+
+	if functionary.CertConstraint.Roots != nil {
+		p.Roots = roots
+	}
+
+	p.Steps = make(map[string]policy.Step)
+	p.Steps[step01.Name] = step01
+	p.Steps[step02.Name] = step02
+
+	if publicKey.KeyID != "" {
+		p.PublicKeys[publicKey.KeyID] = publicKey
+	}
+
+	return p
+}
+
+func makepolicyRSA(t *testing.T) (policy.Policy, cryptoutil.Signer) {
+	signer, err := createTestRSAKey()
+	require.NoError(t, err)
+	verifier, err := signer.Verifier()
+	require.NoError(t, err)
+	keyID, err := verifier.KeyID()
+	require.NoError(t, err)
+	functionary := policy.Functionary{
+		Type:        "PublicKey",
+		PublicKeyID: keyID,
+	}
+
+	pub, err := verifier.Bytes()
+	require.NoError(t, err)
+
+	pk := policy.PublicKey{
+		KeyID: keyID,
+		Key:   pub,
+	}
+
+	p := makepolicy(functionary, pk, nil)
+	return p, signer
+}
+
+func signPolicyRSA(t *testing.T, p policy.Policy) (dsse.Envelope, cryptoutil.Signer) {
+	signer, err := createTestRSAKey()
+	require.NoError(t, err)
+	pBytes, err := json.Marshal(p)
+	require.NoError(t, err)
+	reader := bytes.NewReader(pBytes)
+	outBytes := []byte{}
+	writer := bytes.NewBuffer(outBytes)
+	require.NoError(t, Sign(reader, policy.PolicyPredicate, writer, dsse.SignWithSigners(signer)))
+	env := dsse.Envelope{}
+	require.NoError(t, json.Unmarshal(writer.Bytes(), &env))
+	return env, signer
+}
+
+func createTestRSAKey() (cryptoutil.Signer, error) {
+	privKey, err := rsa.GenerateKey(rand.Reader, 512)
+	if err != nil {
+		return nil, err
+	}
+
+	signer := cryptoutil.NewRSASigner(privKey, crypto.SHA256)
+	return signer, nil
+}