From 03a45bad881aa84b795366724c4aa0de84380131 Mon Sep 17 00:00:00 2001 From: Mikhail Swift Date: Thu, 22 Aug 2024 17:36:11 -0400 Subject: [PATCH 1/2] revert: fix: repetitive and incorrect log lines on `witness verify` (#317)" This reverts commit 26ee2dc6de37ccad233695aeb38ab18fb8108013. This commit breaks policy evaluations, causing good policies to fail. Signed-off-by: Mikhail Swift --- policy/policy.go | 33 ++++++++++++++++++++------------- policy/step.go | 2 +- source/memory.go | 24 ++++++++++++++---------- 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/policy/policy.go b/policy/policy.go index 908aefab..5ab2408e 100644 --- a/policy/policy.go +++ b/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 } diff --git a/policy/step.go b/policy/step.go index 1dae6fd7..5cd5630d 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 ee40f43e..8f7450e7 100644 --- a/source/memory.go +++ b/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) } From b46fc009b6b9ab7664451347c5c514b19636f268 Mon Sep 17 00:00:00 2001 From: Mikhail Swift Date: Thu, 22 Aug 2024 19:00:40 -0400 Subject: [PATCH 2/2] test: add end to end policy verification test Signed-off-by: Mikhail Swift --- verify_test.go | 218 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 verify_test.go diff --git a/verify_test.go b/verify_test.go new file mode 100644 index 00000000..6557c64f --- /dev/null +++ b/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 +}