diff --git a/container/verifier/verifier.go b/container/verifier/verifier.go index b057060..8f85049 100644 --- a/container/verifier/verifier.go +++ b/container/verifier/verifier.go @@ -15,6 +15,7 @@ import ( "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" "github.com/sigstore/sigstore-go/pkg/root" "github.com/sigstore/sigstore-go/pkg/verify" + "google.golang.org/protobuf/types/known/structpb" registry "github.com/stacklok/toolhive-core/registry/types" ) @@ -166,17 +167,73 @@ func isVerificationResultMatchingServerProvenance(r *verify.VerificationResult, return false } - // If the attestations are not set, we can skip this check - if p.Attestation != nil && r.Statement != nil && p.Attestation.Predicate != nil && r.Statement.Predicate != nil { - if p.Attestation.PredicateType != r.Statement.PredicateType { + return compareAttestation(r, p) +} + +// compareAttestation compares the attestation constraint declared in the +// server provenance against the in-toto statement carried by the verification +// result. Following the same per-field convention as compareBaseProperties, an +// unset expectation means "do not constrain that dimension" rather than "skip +// the check". +func compareAttestation(r *verify.VerificationResult, p *registry.Provenance) bool { + // A nil attestation places no constraint on the artifact. + if p.Attestation == nil { + return true + } + + // The provenance asks for an attestation, so the artifact has to carry + // one. Treating a missing statement as a match would skip the constraint + // exactly when it matters. + if r.Statement == nil { + return false + } + + if p.Attestation.PredicateType != "" && p.Attestation.PredicateType != r.Statement.PredicateType { + return false + } + + if p.Attestation.Predicate != nil { + if r.Statement.Predicate == nil { + return false + } + // Round both sides through structpb before comparing. The expected + // predicate is decoded registry data whose Go types depend on the + // serialisation format - encoding/json yields float64 for every + // number where yaml.v3 yields int - while the statement carries a + // *structpb.Struct. Comparing the raw values would make verification + // depend on how the registry entry happened to be decoded. + expected, err := normalizeAttestationPredicate(p.Attestation.Predicate) + if err != nil { + // An expected predicate we cannot normalise is one we cannot + // confirm, so treat it as a mismatch rather than a pass. + slog.Error("cannot normalize expected attestation predicate", "error", err) + return false + } + if !reflect.DeepEqual(expected, r.Statement.Predicate.AsMap()) { return false } - return reflect.DeepEqual(p.Attestation.Predicate, r.Statement.Predicate) } return true } +// normalizeAttestationPredicate renders an expected predicate from the registry +// into the same shape structpb produces for a statement predicate, so that +// semantically equal predicates compare equal regardless of whether they were +// decoded from JSON or YAML. An in-toto predicate is always an object, so an +// expectation that is not a map cannot match one. +func normalizeAttestationPredicate(predicate any) (map[string]any, error) { + fields, ok := predicate.(map[string]any) + if !ok { + return nil, fmt.Errorf("expected predicate is %T, want an object", predicate) + } + s, err := structpb.NewStruct(fields) + if err != nil { + return nil, fmt.Errorf("expected predicate is not representable as a struct: %w", err) + } + return s.AsMap(), nil +} + // compareBaseProperties compares the base properties of the verification result and the server provenance func compareBaseProperties(r *verify.VerificationResult, p *registry.Provenance) bool { // Extract the signer identity from the certificate diff --git a/container/verifier/verifier_test.go b/container/verifier/verifier_test.go index 464fa52..1bf61b8 100644 --- a/container/verifier/verifier_test.go +++ b/container/verifier/verifier_test.go @@ -4,11 +4,17 @@ package verifier import ( + "encoding/json" "testing" "github.com/google/go-containerregistry/pkg/authn" + in_toto "github.com/in-toto/attestation/go/v1" + "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" + "github.com/sigstore/sigstore-go/pkg/verify" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" + "gopkg.in/yaml.v3" registry "github.com/stacklok/toolhive-core/registry/types" ) @@ -64,3 +70,306 @@ func TestVerifyServer_NilProvenance_InvalidRef(t *testing.T) { err := s.VerifyServer("", nil) require.Error(t, err) } + +// ------ isVerificationResultMatchingServerProvenance ------ + +// testSAN is a Fulcio-style subject alternative name. compareBaseProperties +// runs signerIdentityFromCertificate unconditionally, which errors on an empty +// SAN, so every fixture certificate carries one. +const testSAN = "https://github.com/stacklok/toolhive/.github/workflows/build.yml@refs/heads/main" + +const ( + slsaPredicateType = "https://slsa.dev/provenance/v1" + buildTypeKey = "buildType" + buildTypeGHA = "gha" + attemptKey = "attempt" + builderKey = "builder" + builderID = "https://github.com/actions/runner" +) + +// newTestResult builds a minimal verification result whose certificate is +// populated enough for compareBaseProperties to run cleanly. +func newTestResult(statement *in_toto.Statement) *verify.VerificationResult { + return &verify.VerificationResult{ + Signature: &verify.SignatureVerificationResult{ + Certificate: &certificate.Summary{ + SubjectAlternativeName: testSAN, + }, + }, + Statement: statement, + } +} + +// newTestStatement builds an in-toto statement. A nil predicate leaves +// Statement.Predicate nil, modelling a statement that carries no predicate. +func newTestStatement(t *testing.T, predicateType string, predicate map[string]any) *in_toto.Statement { + t.Helper() + stmt := &in_toto.Statement{PredicateType: predicateType} + if predicate != nil { + s, err := structpb.NewStruct(predicate) + require.NoError(t, err) + stmt.Predicate = s + } + return stmt +} + +func TestIsVerificationResultMatchingServerProvenance_Attestation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + statement *in_toto.Statement + expected *registry.VerifiedAttestation + want bool + }{ + { + name: "nil expected attestation is unconstrained", + statement: newTestStatement(t, slsaPredicateType, map[string]any{buildTypeKey: buildTypeGHA}), + expected: nil, + want: true, + }, + { + name: "nil expected attestation with no statement is unconstrained", + statement: nil, + expected: nil, + want: true, + }, + { + // Regression: this used to fall through to a match, skipping the + // constraint exactly when the artifact carried no attestation. + name: "expected attestation but statement absent", + statement: nil, + expected: ®istry.VerifiedAttestation{PredicateType: slsaPredicateType}, + want: false, + }, + { + name: "empty expected attestation but statement absent", + statement: nil, + expected: ®istry.VerifiedAttestation{}, + want: false, + }, + { + // An empty expectation means "must carry some attestation", + // without constraining which one. + name: "empty expected attestation with statement present", + statement: newTestStatement(t, slsaPredicateType, map[string]any{buildTypeKey: buildTypeGHA}), + expected: ®istry.VerifiedAttestation{}, + want: true, + }, + { + name: "predicate type matches, predicate unconstrained", + statement: newTestStatement(t, slsaPredicateType, map[string]any{buildTypeKey: buildTypeGHA}), + expected: ®istry.VerifiedAttestation{PredicateType: slsaPredicateType}, + want: true, + }, + { + name: "predicate type mismatch", + statement: newTestStatement(t, "https://slsa.dev/provenance/v0.2", map[string]any{buildTypeKey: buildTypeGHA}), + expected: ®istry.VerifiedAttestation{PredicateType: slsaPredicateType}, + want: false, + }, + { + name: "expected predicate but statement carries none", + statement: newTestStatement(t, slsaPredicateType, nil), + expected: ®istry.VerifiedAttestation{ + PredicateType: slsaPredicateType, + Predicate: map[string]any{buildTypeKey: buildTypeGHA}, + }, + want: false, + }, + { + name: "predicate mismatch", + statement: newTestStatement(t, slsaPredicateType, map[string]any{buildTypeKey: buildTypeGHA}), + expected: ®istry.VerifiedAttestation{ + PredicateType: slsaPredicateType, + Predicate: map[string]any{buildTypeKey: "local"}, + }, + want: false, + }, + { + name: "predicate type and predicate both match", + statement: newTestStatement(t, slsaPredicateType, map[string]any{ + buildTypeKey: buildTypeGHA, + builderKey: map[string]any{"id": builderID}, + }), + expected: ®istry.VerifiedAttestation{ + PredicateType: slsaPredicateType, + Predicate: map[string]any{ + buildTypeKey: buildTypeGHA, + builderKey: map[string]any{"id": builderID}, + }, + }, + want: true, + }, + { + name: "predicate matches with predicate type unconstrained", + statement: newTestStatement(t, slsaPredicateType, map[string]any{buildTypeKey: buildTypeGHA}), + expected: ®istry.VerifiedAttestation{Predicate: map[string]any{buildTypeKey: buildTypeGHA}}, + want: true, + }, + { + // Numbers are normalised on both sides, so an expectation compares + // equal whether the decoder produced float64 (encoding/json) or + // int (yaml.v3). + name: "numeric predicate matches as float64", + statement: newTestStatement(t, slsaPredicateType, map[string]any{attemptKey: 1}), + expected: ®istry.VerifiedAttestation{Predicate: map[string]any{attemptKey: float64(1)}}, + want: true, + }, + { + name: "numeric predicate matches as int", + statement: newTestStatement(t, slsaPredicateType, map[string]any{attemptKey: 1}), + expected: ®istry.VerifiedAttestation{Predicate: map[string]any{attemptKey: 1}}, + want: true, + }, + { + name: "numeric predicate still mismatches on value", + statement: newTestStatement(t, slsaPredicateType, map[string]any{attemptKey: 1}), + expected: ®istry.VerifiedAttestation{Predicate: map[string]any{attemptKey: 2}}, + want: false, + }, + { + // An in-toto predicate is always an object, so a scalar + // expectation cannot match and must not pass. + name: "non-object expected predicate does not match", + statement: newTestStatement(t, slsaPredicateType, map[string]any{buildTypeKey: buildTypeGHA}), + expected: ®istry.VerifiedAttestation{Predicate: buildTypeGHA}, + want: false, + }, + { + // A predicate structpb cannot represent is one we cannot confirm, + // so it fails closed rather than passing. + name: "unrepresentable expected predicate does not match", + statement: newTestStatement(t, slsaPredicateType, map[string]any{buildTypeKey: buildTypeGHA}), + expected: ®istry.VerifiedAttestation{Predicate: map[string]any{buildTypeKey: make(chan int)}}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + r := newTestResult(tt.statement) + p := ®istry.Provenance{Attestation: tt.expected} + assert.Equal(t, tt.want, isVerificationResultMatchingServerProvenance(r, p)) + }) + } +} + +// Predicate equality must not depend on how the registry entry was decoded. +// The same provenance expressed as JSON and as YAML has to verify against the +// same statement, even though encoding/json decodes numbers to float64 while +// yaml.v3 decodes them to int. +func TestIsVerificationResultMatchingServerProvenance_PredicateDecoderIndependence(t *testing.T) { + t.Parallel() + + const jsonProvenance = `{ + "attestation": { + "predicate_type": "https://slsa.dev/provenance/v1", + "predicate": { + "buildType": "gha", + "attempt": 1, + "builder": {"id": "https://github.com/actions/runner", "version": 2}, + "steps": [1, 2] + } + } + }` + + const yamlProvenance = ` +attestation: + predicate_type: https://slsa.dev/provenance/v1 + predicate: + buildType: gha + attempt: 1 + builder: + id: https://github.com/actions/runner + version: 2 + steps: + - 1 + - 2 +` + + statement := newTestStatement(t, slsaPredicateType, map[string]any{ + buildTypeKey: buildTypeGHA, + attemptKey: 1, + builderKey: map[string]any{"id": builderID, "version": 2}, + "steps": []any{1, 2}, + }) + result := newTestResult(statement) + + var fromJSON registry.Provenance + require.NoError(t, json.Unmarshal([]byte(jsonProvenance), &fromJSON)) + + var fromYAML registry.Provenance + require.NoError(t, yaml.Unmarshal([]byte(yamlProvenance), &fromYAML)) + + // Guard the premise: the two decoders really do produce different Go + // types, so this test would be vacuous if they ever converged. + require.NotEqual(t, fromJSON.Attestation.Predicate, fromYAML.Attestation.Predicate, + "decoders expected to disagree on numeric types; test is vacuous otherwise") + + assert.True(t, isVerificationResultMatchingServerProvenance(result, &fromJSON), "JSON-decoded provenance should match") + assert.True(t, isVerificationResultMatchingServerProvenance(result, &fromYAML), "YAML-decoded provenance should match") +} + +func TestIsVerificationResultMatchingServerProvenance_Guards(t *testing.T) { + t.Parallel() + + statement := newTestStatement(t, slsaPredicateType, map[string]any{buildTypeKey: buildTypeGHA}) + + tests := []struct { + name string + result *verify.VerificationResult + prov *registry.Provenance + want bool + }{ + { + name: "nil result", + result: nil, + prov: ®istry.Provenance{}, + want: false, + }, + { + name: "nil provenance", + result: newTestResult(statement), + prov: nil, + want: false, + }, + { + name: "nil signature", + result: &verify.VerificationResult{Statement: statement}, + prov: ®istry.Provenance{}, + want: false, + }, + { + name: "nil certificate", + result: &verify.VerificationResult{ + Signature: &verify.SignatureVerificationResult{}, + Statement: statement, + }, + prov: ®istry.Provenance{}, + want: false, + }, + { + // Base properties are checked before the attestation, so a + // mismatch there short-circuits regardless of the attestation. + name: "base property mismatch short-circuits", + result: newTestResult(statement), + prov: ®istry.Provenance{RepositoryURI: "https://github.com/other/repo"}, + want: false, + }, + { + name: "no constraints at all", + result: newTestResult(statement), + prov: ®istry.Provenance{}, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, isVerificationResultMatchingServerProvenance(tt.result, tt.prov)) + }) + } +} diff --git a/go.mod b/go.mod index 3230ff2..db9a316 100644 --- a/go.mod +++ b/go.mod @@ -36,6 +36,7 @@ require ( github.com/go-openapi/swag/conv v0.29.0 github.com/golang-jwt/jwt/v5 v5.3.1 github.com/google/certificate-transparency-go v1.3.3 + github.com/in-toto/attestation v1.2.0 github.com/lestrrat-go/httprc/v3 v3.0.6 github.com/lestrrat-go/jwx/v3 v3.2.0 github.com/prometheus/client_golang v1.24.1 @@ -52,6 +53,7 @@ require ( go.opentelemetry.io/otel/sdk/log v0.21.0 go.opentelemetry.io/otel/trace v1.45.0 golang.org/x/oauth2 v0.36.0 + google.golang.org/protobuf v1.36.11 ) require ( @@ -112,7 +114,6 @@ require ( github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/go-retryablehttp v0.7.8 // indirect - github.com/in-toto/attestation v1.2.0 // indirect github.com/in-toto/in-toto-golang v0.11.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect @@ -168,6 +169,5 @@ require ( google.golang.org/genproto/googleapis/api v0.0.0-20260803160001-6ac0973c030d // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260803160001-6ac0973c030d // indirect google.golang.org/grpc v1.83.0 // indirect - google.golang.org/protobuf v1.36.11 // indirect k8s.io/klog/v2 v2.140.0 // indirect )