From db5d67cc227314d3f952d714b36a77da64a288d0 Mon Sep 17 00:00:00 2001 From: chaosinthecrd Date: Thu, 1 Aug 2024 16:45:14 +0100 Subject: [PATCH 1/3] removing repeated log lines and unnecessary searches (memory source search) Signed-off-by: chaosinthecrd --- policy/policy.go | 33 +++++++++++++-------------------- policy/step.go | 2 +- source/memory.go | 24 ++++++++++-------------- 3 files changed, 24 insertions(+), 35 deletions(-) diff --git a/policy/policy.go b/policy/policy.go index 81a4f34b..dfac8358 100644 --- a/policy/policy.go +++ b/policy/policy.go @@ -217,30 +217,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 { - // Use search to get all the attestations that match the supplied step name and subjects + // initialize the result for this step if it hasn't been already + if _, ok := resultsByStep[stepName]; !ok { + resultsByStep[stepName] = StepResult{Step: stepName} + } + collections, err := vo.verifiedSource.Search(ctx, stepName, vo.subjectDigests, attestationsByStep[stepName]) if err != nil { return false, nil, err } if len(collections) == 0 { - collections = append(collections, source.CollectionVerificationResult{Errors: []error{ErrNoCollections{Step: stepName}}}) + continue } - // Verify the functionaries + // Verify the functionaries and validate attestations collections = step.checkFunctionaries(collections, trustBundles) - stepResult := step.validateAttestations(collections) - // 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 - } + // Merge the results + 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 { @@ -303,13 +303,6 @@ 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 } diff --git a/policy/step.go b/policy/step.go index 5cd5630d..1dae6fd7 100644 --- a/policy/step.go +++ b/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, diff --git a/source/memory.go b/source/memory.go index 8f7450e7..ee40f43e 100644 --- a/source/memory.go +++ b/source/memory.go @@ -22,6 +22,7 @@ import ( "os" "github.com/in-toto/go-witness/dsse" + "github.com/in-toto/go-witness/log" ) type ErrDuplicateReference string @@ -31,6 +32,7 @@ func (e ErrDuplicateReference) Error() string { } type MemorySource struct { + searched bool envelopesByReference map[string]CollectionEnvelope referencesByCollectionName map[string][]string subjectDigestsByReference map[string]map[string]struct{} @@ -39,6 +41,7 @@ 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{}), @@ -104,6 +107,13 @@ 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] @@ -125,20 +135,6 @@ 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) } From 3c3f4af1f078871e9ba9a7b7d5baef74b4874b00 Mon Sep 17 00:00:00 2001 From: chaosinthecrd Date: Wed, 4 Sep 2024 17:11:45 +0100 Subject: [PATCH 2/3] fixing issues with search depth Signed-off-by: chaosinthecrd --- policy/policy.go | 2 +- policy/policy_test.go | 2 +- source/archivista.go | 2 +- source/memory.go | 7 +++---- source/memory_test.go | 2 +- source/multi.go | 4 ++-- source/source.go | 2 +- source/verified.go | 6 +++--- 8 files changed, 13 insertions(+), 14 deletions(-) diff --git a/policy/policy.go b/policy/policy.go index 908aefab..b9c4981b 100644 --- a/policy/policy.go +++ b/policy/policy.go @@ -244,7 +244,7 @@ func (p Policy) Verify(ctx context.Context, opts ...VerifyOption) (bool, map[str resultsByStep[stepName] = StepResult{Step: stepName} } - collections, err := vo.verifiedSource.Search(ctx, stepName, vo.subjectDigests, attestationsByStep[stepName]) + collections, err := vo.verifiedSource.Search(ctx, depth, stepName, vo.subjectDigests, attestationsByStep[stepName]) if err != nil { return false, nil, err } diff --git a/policy/policy_test.go b/policy/policy_test.go index 761237c5..3336b7b8 100644 --- a/policy/policy_test.go +++ b/policy/policy_test.go @@ -417,7 +417,7 @@ func newDummyVerifiedSourcer(verifiedCollections []source.CollectionVerification return &dummyVerifiedSourcer{verifiedCollections} } -func (s *dummyVerifiedSourcer) Search(ctx context.Context, collectionName string, subjectDigests, attestations []string) ([]source.CollectionVerificationResult, error) { +func (s *dummyVerifiedSourcer) Search(ctx context.Context, depth int, collectionName string, subjectDigests, attestations []string) ([]source.CollectionVerificationResult, error) { return s.verifiedCollections, nil } diff --git a/source/archivista.go b/source/archivista.go index 2f74f722..9048bb21 100644 --- a/source/archivista.go +++ b/source/archivista.go @@ -32,7 +32,7 @@ func NewArchvistSource(client *archivista.Client) *ArchivistaSource { } } -func (s *ArchivistaSource) Search(ctx context.Context, collectionName string, subjectDigests, attestations []string) ([]CollectionEnvelope, error) { +func (s *ArchivistaSource) Search(ctx context.Context, depth int, collectionName string, subjectDigests, attestations []string) ([]CollectionEnvelope, error) { gitoids, err := s.client.SearchGitoids(ctx, archivista.SearchGitoidVariables{ CollectionName: collectionName, SubjectDigests: subjectDigests, diff --git a/source/memory.go b/source/memory.go index ee40f43e..795fcb9e 100644 --- a/source/memory.go +++ b/source/memory.go @@ -106,12 +106,11 @@ func (s *MemorySource) LoadEnvelope(reference string, env dsse.Envelope) error { return nil } -func (s *MemorySource) Search(ctx context.Context, collectionName string, subjectDigests, attestations []string) ([]CollectionEnvelope, error) { - if s.searched { +func (s *MemorySource) Search(ctx context.Context, depth int, collectionName string, subjectDigests, attestations []string) ([]CollectionEnvelope, error) { + if depth > 0 { + fmt.Println("skipping memory source search") log.Debug("skipping memory source search: already performed") return []CollectionEnvelope{}, nil - } else { - s.searched = true } matches := make([]CollectionEnvelope, 0) diff --git a/source/memory_test.go b/source/memory_test.go index 1995cc09..5f1cbd32 100644 --- a/source/memory_test.go +++ b/source/memory_test.go @@ -304,7 +304,7 @@ func TestSearch(t *testing.T) { } // Run the search query on the MemorySource - got, err := s.Search(tt.searchQuery.ctx, tt.searchQuery.collectionName, tt.searchQuery.subDigest, tt.searchQuery.attestations) + got, err := s.Search(tt.searchQuery.ctx, 0, tt.searchQuery.collectionName, tt.searchQuery.subDigest, tt.searchQuery.attestations) if (err != nil) != tt.wantErr { t.Fatalf("MemorySource.Search() error = %v, wantErr %v", err, tt.wantErr) } diff --git a/source/multi.go b/source/multi.go index 500fc587..7443d474 100644 --- a/source/multi.go +++ b/source/multi.go @@ -28,7 +28,7 @@ func NewMultiSource(sources ...Sourcer) *MultiSource { } // Search concurrently queries all sources and returns the combined results. -func (s *MultiSource) Search(ctx context.Context, collectionName string, subjectDigests, attestations []string) ([]CollectionEnvelope, error) { +func (s *MultiSource) Search(ctx context.Context, depth int, collectionName string, subjectDigests, attestations []string) ([]CollectionEnvelope, error) { results := []CollectionEnvelope{} errors := []error{} @@ -61,7 +61,7 @@ func (s *MultiSource) Search(ctx context.Context, collectionName string, subject // Goroutine for querying a source and collecting the results or error go func(src Sourcer) { defer wg.Done() - res, err := src.Search(ctx, collectionName, subjectDigests, attestations) + res, err := src.Search(ctx, depth, collectionName, subjectDigests, attestations) if err != nil { errs <- err } else { diff --git a/source/source.go b/source/source.go index 1aad2bbd..c2d0f990 100644 --- a/source/source.go +++ b/source/source.go @@ -31,7 +31,7 @@ type CollectionEnvelope struct { } type Sourcer interface { - Search(ctx context.Context, collectionName string, subjectDigests, attestations []string) ([]CollectionEnvelope, error) + Search(ctx context.Context, depth int, collectionName string, subjectDigests, attestations []string) ([]CollectionEnvelope, error) } func envelopeToCollectionEnvelope(reference string, env dsse.Envelope) (CollectionEnvelope, error) { diff --git a/source/verified.go b/source/verified.go index fab6404b..54b7d2cf 100644 --- a/source/verified.go +++ b/source/verified.go @@ -31,7 +31,7 @@ type CollectionVerificationResult struct { } type VerifiedSourcer interface { - Search(ctx context.Context, collectionName string, subjectDigests, attestations []string) ([]CollectionVerificationResult, error) + Search(ctx context.Context, depth int, collectionName string, subjectDigests, attestations []string) ([]CollectionVerificationResult, error) } type VerifiedSource struct { @@ -43,8 +43,8 @@ func NewVerifiedSource(source Sourcer, verifyOpts ...dsse.VerificationOption) *V return &VerifiedSource{source, verifyOpts} } -func (s *VerifiedSource) Search(ctx context.Context, collectionName string, subjectDigests, attestations []string) ([]CollectionVerificationResult, error) { - unverified, err := s.source.Search(ctx, collectionName, subjectDigests, attestations) +func (s *VerifiedSource) Search(ctx context.Context, depth int, collectionName string, subjectDigests, attestations []string) ([]CollectionVerificationResult, error) { + unverified, err := s.source.Search(ctx, depth, collectionName, subjectDigests, attestations) if err != nil { return nil, err } From cf7190c7c2bbaea0bfc5ba388799bd8d6f786a90 Mon Sep 17 00:00:00 2001 From: chaosinthecrd Date: Wed, 4 Sep 2024 17:17:02 +0100 Subject: [PATCH 3/3] removing redundant boolean Signed-off-by: chaosinthecrd --- source/memory.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/source/memory.go b/source/memory.go index 795fcb9e..8d0a7bdf 100644 --- a/source/memory.go +++ b/source/memory.go @@ -32,7 +32,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 +40,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{}),