From 7845832df94339d62e98865c4300793c667eca81 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 27 Nov 2024 12:59:18 -0500 Subject: [PATCH 1/7] Add SignatureFormat field to Authority Signed-off-by: Cody Soyland --- config/300-clusterimagepolicy.yaml | 6 ++++++ docs/api-types/index-v1alpha1.md | 1 + docs/api-types/index.md | 1 + pkg/apis/policy/v1alpha1/clusterimagepolicy_conversion.go | 2 ++ pkg/apis/policy/v1alpha1/clusterimagepolicy_types.go | 4 ++++ pkg/apis/policy/v1beta1/clusterimagepolicy_types.go | 4 ++++ pkg/webhook/clusterimagepolicy/clusterimagepolicy_types.go | 3 +++ 7 files changed, 21 insertions(+) diff --git a/config/300-clusterimagepolicy.yaml b/config/300-clusterimagepolicy.yaml index c5c3c28ea..941bd47c4 100644 --- a/config/300-clusterimagepolicy.yaml +++ b/config/300-clusterimagepolicy.yaml @@ -209,6 +209,9 @@ spec: trustRootRef: description: Use the Certificate Chain from the referred TrustRoot.TimeStampAuthorities type: string + signatureFormat: + description: SignatureFormat specifies the format the authority expects. Supported formats are "legacy" and "bundle". If not specified, the default is "legacy" (cosign's default). + type: string source: description: Sources sets the configuration to specify the sources from where to consume the signatures. type: array @@ -545,6 +548,9 @@ spec: trustRootRef: description: Use the Certificate Chain from the referred TrustRoot.TimeStampAuthorities type: string + signatureFormat: + description: SignatureFormat specifies the format the authority expects. Supported formats are "legacy" and "bundle". If not specified, the default is "legacy" (cosign's default). + type: string source: description: Sources sets the configuration to specify the sources from where to consume the signatures. type: array diff --git a/docs/api-types/index-v1alpha1.md b/docs/api-types/index-v1alpha1.md index a55f68104..0dbc3d4c1 100644 --- a/docs/api-types/index-v1alpha1.md +++ b/docs/api-types/index-v1alpha1.md @@ -172,6 +172,7 @@ Attestation defines the type of attestation to validate and optionally apply a p | ctlog | CTLog sets the configuration to verify the authority against a Rekor instance. | [TLog](#tlog) | false | | attestations | Attestations is a list of individual attestations for this authority, once the signature for this authority has been verified. | [][Attestation](#attestation) | false | | rfc3161timestamp | RFC3161Timestamp sets the configuration to verify the signature timestamp against a RFC3161 time-stamping instance. | [RFC3161Timestamp](#rfc3161timestamp) | false | +| signatureFormat | SignatureFormat specifies the format the authority expects. Supported formats are \"legacy\" and \"bundle\". If not specified, the default is \"legacy\" (cosign's default). | string | false | [Back to TOC](#table-of-contents) diff --git a/docs/api-types/index.md b/docs/api-types/index.md index c3cdbb512..56c93cdf0 100644 --- a/docs/api-types/index.md +++ b/docs/api-types/index.md @@ -49,6 +49,7 @@ The authorities block defines the rules for discovering and validating signature | ctlog | CTLog sets the configuration to verify the authority against a Rekor instance. | [TLog](#tlog) | false | | attestations | Attestations is a list of individual attestations for this authority, once the signature for this authority has been verified. | [][Attestation](#attestation) | false | | rfc3161timestamp | RFC3161Timestamp sets the configuration to verify the signature timestamp against a RFC3161 time-stamping instance. | [RFC3161Timestamp](#rfc3161timestamp) | false | +| signatureFormat | SignatureFormat specifies the format the authority expects. Supported formats are \"legacy\" and \"bundle\". If not specified, the default is \"legacy\" (cosign's default). | string | false | [Back to TOC](#table-of-contents) diff --git a/pkg/apis/policy/v1alpha1/clusterimagepolicy_conversion.go b/pkg/apis/policy/v1alpha1/clusterimagepolicy_conversion.go index 671fdba33..bebbe75d3 100644 --- a/pkg/apis/policy/v1alpha1/clusterimagepolicy_conversion.go +++ b/pkg/apis/policy/v1alpha1/clusterimagepolicy_conversion.go @@ -89,6 +89,7 @@ func (matchResource *MatchResource) ConvertTo(_ context.Context, sink *v1beta1.M func (authority *Authority) ConvertTo(ctx context.Context, sink *v1beta1.Authority) error { sink.Name = authority.Name + sink.SignatureFormat = authority.SignatureFormat if authority.CTLog != nil && authority.CTLog.URL != nil { sink.CTLog = &v1beta1.TLog{ URL: authority.CTLog.URL.DeepCopy(), @@ -244,6 +245,7 @@ func (spec *ClusterImagePolicySpec) ConvertFrom(ctx context.Context, source *v1b func (authority *Authority) ConvertFrom(ctx context.Context, source *v1beta1.Authority) error { authority.Name = source.Name + authority.SignatureFormat = source.SignatureFormat if source.CTLog != nil && source.CTLog.URL != nil { authority.CTLog = &TLog{ URL: source.CTLog.URL.DeepCopy(), diff --git a/pkg/apis/policy/v1alpha1/clusterimagepolicy_types.go b/pkg/apis/policy/v1alpha1/clusterimagepolicy_types.go index 32cf79782..75a991593 100644 --- a/pkg/apis/policy/v1alpha1/clusterimagepolicy_types.go +++ b/pkg/apis/policy/v1alpha1/clusterimagepolicy_types.go @@ -144,6 +144,10 @@ type Authority struct { // RFC3161Timestamp sets the configuration to verify the signature timestamp against a RFC3161 time-stamping instance. // +optional RFC3161Timestamp *RFC3161Timestamp `json:"rfc3161timestamp,omitempty"` + // SignatureFormat specifies the format the authority expects. Supported + // formats are "legacy" and "bundle". If not specified, the default + // is "legacy" (cosign's default). + SignatureFormat string `json:"signatureFormat,omitempty"` } // This references a public verification key stored in diff --git a/pkg/apis/policy/v1beta1/clusterimagepolicy_types.go b/pkg/apis/policy/v1beta1/clusterimagepolicy_types.go index 8e1b1b8b5..44c3adf16 100644 --- a/pkg/apis/policy/v1beta1/clusterimagepolicy_types.go +++ b/pkg/apis/policy/v1beta1/clusterimagepolicy_types.go @@ -143,6 +143,10 @@ type Authority struct { // RFC3161Timestamp sets the configuration to verify the signature timestamp against a RFC3161 time-stamping instance. // +optional RFC3161Timestamp *RFC3161Timestamp `json:"rfc3161timestamp,omitempty"` + // SignatureFormat specifies the format the authority expects. Supported + // formats are "legacy" and "bundle". If not specified, the default + // is "legacy" (cosign's default). + SignatureFormat string `json:"signatureFormat,omitempty"` } // This references a public verification key stored in diff --git a/pkg/webhook/clusterimagepolicy/clusterimagepolicy_types.go b/pkg/webhook/clusterimagepolicy/clusterimagepolicy_types.go index a01235eb0..e022d5d65 100644 --- a/pkg/webhook/clusterimagepolicy/clusterimagepolicy_types.go +++ b/pkg/webhook/clusterimagepolicy/clusterimagepolicy_types.go @@ -86,6 +86,8 @@ type Authority struct { Attestations []AttestationPolicy `json:"attestations,omitempty"` // +optional RFC3161Timestamp *RFC3161Timestamp `json:"rfc3161timestamp,omitempty"` + // +optional + SignatureFormat string `json:"signatureFormat,omitempty"` } // This references a public verification key stored in @@ -325,6 +327,7 @@ func convertAuthorityV1Alpha1ToWebhook(in v1alpha1.Authority) *Authority { CTLog: in.CTLog, RFC3161Timestamp: rfc3161Timestamp, Attestations: attestations, + SignatureFormat: in.SignatureFormat, } } From 60096f2420fa9b42dc629b337bd9a09ae9eb9e9c Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 27 Nov 2024 13:00:23 -0500 Subject: [PATCH 2/7] Add func to retrieve TrustedRoot from TUF Signed-off-by: Cody Soyland Sync TUF cache used for sigstore bundle verification (#166) * sync tuf cache used for sigstore bundle verification Signed-off-by: Meredith Lancaster * remove singleton err Signed-off-by: Meredith Lancaster * start adding lock Signed-off-by: Meredith Lancaster * Use RWMutex Signed-off-by: Meredith Lancaster * pr feedback Signed-off-by: Meredith Lancaster --------- Signed-off-by: Meredith Lancaster Fix shadowed trustedroot (#178) * Fix shadowed variable bug This code caused the singleton `trustedRoot` to be returned as nil on subsequent calls. The singleton was shadowed when the variable was redeclared in the `if` block. Signed-off-by: Cody Soyland * Remove unused singleton `singletonRootError` was never returned without being overwritten, so it was essentially unused. I think it's wise to always retry the TUF call on future invocations in case of network errors. Signed-off-by: Cody Soyland --------- Signed-off-by: Cody Soyland Update go.mod Signed-off-by: Cody Soyland --- go.mod | 2 +- pkg/tuf/repo.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 65af76b97..9206acd13 100644 --- a/go.mod +++ b/go.mod @@ -64,6 +64,7 @@ require ( github.com/go-jose/go-jose/v4 v4.0.5 github.com/sigstore/protobuf-specs v0.4.1 github.com/sigstore/scaffolding v0.7.22 + github.com/sigstore/sigstore-go v0.7.1 github.com/sigstore/sigstore/pkg/signature/kms/aws v1.9.3 github.com/sigstore/sigstore/pkg/signature/kms/azure v1.9.3 github.com/sigstore/sigstore/pkg/signature/kms/gcp v1.9.3 @@ -228,7 +229,6 @@ require ( github.com/sassoftware/relic v7.2.1+incompatible // indirect github.com/secure-systems-lab/go-securesystemslib v0.9.0 // indirect github.com/shibumi/go-pathspec v1.3.0 // indirect - github.com/sigstore/sigstore-go v0.7.1 // indirect github.com/sigstore/timestamp-authority v1.2.5 // indirect github.com/sirupsen/logrus v1.9.3 // indirect github.com/sourcegraph/conc v0.3.0 // indirect diff --git a/pkg/tuf/repo.go b/pkg/tuf/repo.go index 0b31c49d3..0d8b8a23f 100644 --- a/pkg/tuf/repo.go +++ b/pkg/tuf/repo.go @@ -28,9 +28,12 @@ import ( "path/filepath" "runtime" "strings" + "sync" "testing/fstest" "time" + "github.com/sigstore/sigstore-go/pkg/root" + "github.com/sigstore/sigstore/pkg/tuf" "github.com/theupdateframework/go-tuf/client" "sigs.k8s.io/release-utils/version" ) @@ -294,3 +297,43 @@ func ClientFromRemote(_ context.Context, mirror string, rootJSON []byte, targets } return tufClient, nil } + +var ( + mu sync.RWMutex + timestamp time.Time + trustedRoot *root.TrustedRoot +) + +// GetTrustedRoot returns the trusted root for the TUF repository. +func GetTrustedRoot() (*root.TrustedRoot, error) { + now := time.Now().UTC() + // check if timestamp has never been or if the current time is more + // than 24 hours after the current value of timestamp + if timestamp.IsZero() || now.After(timestamp.Add(24*time.Hour)) { + mu.Lock() + defer mu.Unlock() + + tufClient, err := tuf.NewFromEnv(context.Background()) + if err != nil { + return nil, fmt.Errorf("initializing tuf: %w", err) + } + // TODO: add support for custom trusted root path + targetBytes, err := tufClient.GetTarget("trusted_root.json") + if err != nil { + return nil, fmt.Errorf("error getting targets: %w", err) + } + trustedRoot, err = root.NewTrustedRootFromJSON(targetBytes) + if err != nil { + return nil, fmt.Errorf("error creating trusted root: %w", err) + } + + timestamp = now + + return trustedRoot, nil + } + + mu.RLock() + defer mu.RUnlock() + + return trustedRoot, nil +} From d8993d486c1c1518185c078a15de6d953059e76e Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 26 Jun 2024 14:45:49 -0600 Subject: [PATCH 3/7] Thread configurable trustroot resync period to bundle trustroot func (#171) * move trustroot resync period configration to different package Signed-off-by: Meredith Lancaster * add license Signed-off-by: Meredith Lancaster * comment Signed-off-by: Meredith Lancaster * rename files Signed-off-by: Meredith Lancaster --------- Signed-off-by: Meredith Lancaster --- cmd/webhook/main.go | 3 +- pkg/reconciler/trustroot/controller.go | 20 +--------- pkg/reconciler/trustroot/controller_test.go | 20 ---------- pkg/tuf/context.go | 41 ++++++++++++++++++++ pkg/tuf/context_test.go | 42 +++++++++++++++++++++ pkg/tuf/repo.go | 9 +++-- 6 files changed, 92 insertions(+), 43 deletions(-) create mode 100644 pkg/tuf/context.go create mode 100644 pkg/tuf/context_test.go diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index 52f329caa..6f84894fd 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -56,6 +56,7 @@ import ( "github.com/sigstore/sigstore/pkg/tuf" "github.com/sigstore/policy-controller/pkg/apis/config" + pctuf "github.com/sigstore/policy-controller/pkg/tuf" cwebhook "github.com/sigstore/policy-controller/pkg/webhook" ) @@ -136,7 +137,7 @@ func main() { // Set the policy and trust root resync periods ctx = clusterimagepolicy.ToContext(ctx, *policyResyncPeriod) - ctx = trustroot.ToContext(ctx, *trustrootResyncPeriod) + ctx = pctuf.ToContext(ctx, *trustrootResyncPeriod) // This must match the set of resources we configure in // cmd/webhook/main.go in the "types" map. diff --git a/pkg/reconciler/trustroot/controller.go b/pkg/reconciler/trustroot/controller.go index 8373b9804..66fffe2a7 100644 --- a/pkg/reconciler/trustroot/controller.go +++ b/pkg/reconciler/trustroot/controller.go @@ -16,7 +16,6 @@ package trustroot import ( "context" - "time" "k8s.io/client-go/tools/cache" kubeclient "knative.dev/pkg/client/injection/kube/client" @@ -30,6 +29,7 @@ import ( "github.com/sigstore/policy-controller/pkg/apis/config" trustrootinformer "github.com/sigstore/policy-controller/pkg/client/injection/informers/policy/v1alpha1/trustroot" trustrootreconciler "github.com/sigstore/policy-controller/pkg/client/injection/reconciler/policy/v1alpha1/trustroot" + "github.com/sigstore/policy-controller/pkg/tuf" cminformer "knative.dev/pkg/injection/clients/namespacedkube/informers/core/v1/configmap" ) @@ -37,8 +37,6 @@ import ( // use it in tests as well. const FinalizerName = "trustroots.policy.sigstore.dev" -type trustrootResyncPeriodKey struct{} - // NewController creates a Reconciler and returns the result of NewImpl. func NewController( ctx context.Context, @@ -78,22 +76,8 @@ func NewController( pkgreconciler.NamespaceFilterFunc(system.Namespace()), pkgreconciler.NameFilterFunc(config.SigstoreKeysConfigName)), Handler: controller.HandleAll(grCb), - }, FromContextOrDefaults(ctx)); err != nil { + }, tuf.FromContextOrDefaults(ctx)); err != nil { logging.FromContext(ctx).Warnf("Failed configMapInformer AddEventHandlerWithResyncPeriod() %v", err) } return impl } - -func ToContext(ctx context.Context, duration time.Duration) context.Context { - return context.WithValue(ctx, trustrootResyncPeriodKey{}, duration) -} - -// FromContextOrDefaults returns a stored trustrootResyncPeriod if attached. -// If not found, it returns a default duration -func FromContextOrDefaults(ctx context.Context) time.Duration { - x, ok := ctx.Value(trustrootResyncPeriodKey{}).(time.Duration) - if ok { - return x - } - return controller.DefaultResyncPeriod -} diff --git a/pkg/reconciler/trustroot/controller_test.go b/pkg/reconciler/trustroot/controller_test.go index 7d6b442a1..0377b5621 100644 --- a/pkg/reconciler/trustroot/controller_test.go +++ b/pkg/reconciler/trustroot/controller_test.go @@ -16,10 +16,8 @@ package trustroot import ( "testing" - "time" "knative.dev/pkg/configmap" - "knative.dev/pkg/controller" rtesting "knative.dev/pkg/reconciler/testing" // Fake injection informers @@ -39,21 +37,3 @@ func TestNew(t *testing.T) { t.Fatal("Expected NewController to return a non-nil value") } } - -func TestContextDuration(t *testing.T) { - ctx, _ := rtesting.SetupFakeContext(t) - - expected := controller.DefaultResyncPeriod - actual := FromContextOrDefaults(ctx) - if expected != actual { - t.Fatal("Expected the context to store the value and be retrievable") - } - - expected = time.Hour - ctx = ToContext(ctx, expected) - actual = FromContextOrDefaults(ctx) - - if expected != actual { - t.Fatal("Expected the context to store the value and be retrievable") - } -} diff --git a/pkg/tuf/context.go b/pkg/tuf/context.go new file mode 100644 index 000000000..3c9f81531 --- /dev/null +++ b/pkg/tuf/context.go @@ -0,0 +1,41 @@ +// +// Copyright 2024 The Sigstore Authors. +// +// 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 tuf + +import ( + "context" + "time" + + "knative.dev/pkg/controller" +) + +type trustrootResyncPeriodKey struct{} + +// ToContext returns a context that includes a key trustrootResyncPeriod +// set to the included duration +func ToContext(ctx context.Context, duration time.Duration) context.Context { + return context.WithValue(ctx, trustrootResyncPeriodKey{}, duration) +} + +// FromContextOrDefaults returns a stored trustrootResyncPeriod if attached. +// If not found, it returns a default duration +func FromContextOrDefaults(ctx context.Context) time.Duration { + x, ok := ctx.Value(trustrootResyncPeriodKey{}).(time.Duration) + if ok { + return x + } + return controller.DefaultResyncPeriod +} diff --git a/pkg/tuf/context_test.go b/pkg/tuf/context_test.go new file mode 100644 index 000000000..5537cb0af --- /dev/null +++ b/pkg/tuf/context_test.go @@ -0,0 +1,42 @@ +// +// Copyright 2024 The Sigstore Authors. +// +// 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 tuf + +import ( + "testing" + "time" + + "knative.dev/pkg/controller" + rtesting "knative.dev/pkg/reconciler/testing" +) + +func TestContextDuration(t *testing.T) { + ctx, _ := rtesting.SetupFakeContext(t) + + expected := controller.DefaultResyncPeriod + actual := FromContextOrDefaults(ctx) + if expected != actual { + t.Fatal("Expected the context to store the value and be retrievable") + } + + expected = time.Hour + ctx = ToContext(ctx, expected) + actual = FromContextOrDefaults(ctx) + + if expected != actual { + t.Fatal("Expected the context to store the value and be retrievable") + } +} diff --git a/pkg/tuf/repo.go b/pkg/tuf/repo.go index 0d8b8a23f..eb9573776 100644 --- a/pkg/tuf/repo.go +++ b/pkg/tuf/repo.go @@ -305,11 +305,12 @@ var ( ) // GetTrustedRoot returns the trusted root for the TUF repository. -func GetTrustedRoot() (*root.TrustedRoot, error) { +func GetTrustedRoot(ctx context.Context) (*root.TrustedRoot, error) { + resyncPeriodDuration := FromContextOrDefaults(ctx) now := time.Now().UTC() - // check if timestamp has never been or if the current time is more - // than 24 hours after the current value of timestamp - if timestamp.IsZero() || now.After(timestamp.Add(24*time.Hour)) { + // check if timestamp has never been set or if the current time + // is after the current timestamp value plus the included resync duration + if timestamp.IsZero() || now.After(timestamp.Add(resyncPeriodDuration)) { mu.Lock() defer mu.Unlock() From 1b9864a52ee9557c3c73179c856797bae577fdf8 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 4 Dec 2024 16:08:42 -0500 Subject: [PATCH 4/7] linter: remove redundant error checks Signed-off-by: Cody Soyland --- pkg/webhook/validator_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/webhook/validator_test.go b/pkg/webhook/validator_test.go index 5ff0f816a..bfc86c1bf 100644 --- a/pkg/webhook/validator_test.go +++ b/pkg/webhook/validator_test.go @@ -3032,7 +3032,7 @@ func TestFulcioCertsFromAuthority(t *testing.T) { } else if err.Error() != tc.wantErr { t.Errorf("unexpected error: %v wanted %q", err, tc.wantErr) } - } else if err == nil && tc.wantErr != "" { + } else if tc.wantErr != "" { t.Errorf("wanted error: %q got none", tc.wantErr) } if !roots.Equal(tc.wantRoots) { @@ -3140,7 +3140,7 @@ func TestRekorClientAndKeysFromAuthority(t *testing.T) { } else if err.Error() != tc.wantErr { t.Errorf("unexpected error: %v wanted %q", err, tc.wantErr) } - } else if err == nil && tc.wantErr != "" { + } else if tc.wantErr != "" { t.Errorf("wanted error: %q got none", tc.wantErr) } if tc.wantLogID != "" { @@ -3370,7 +3370,7 @@ func TestCheckOptsFromAuthority(t *testing.T) { } else if err.Error() != tc.wantErr { t.Errorf("unexpected error: %v wanted %q", err, tc.wantErr) } - } else if err == nil && tc.wantErr != "" { + } else if tc.wantErr != "" { t.Errorf("wanted error: %q got none", tc.wantErr) } if tc.wantClient && (gotCheckOpts == nil || gotCheckOpts.RekorClient == nil) { From 9ae61a3fa519160c23d0fbfbf6615d426162df2f Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Mon, 9 Dec 2024 14:34:38 -0500 Subject: [PATCH 5/7] Generate CheckOpts for verification of the new bundle format Signed-off-by: Cody Soyland --- pkg/webhook/validator.go | 66 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/pkg/webhook/validator.go b/pkg/webhook/validator.go index 53f533103..1f18eada3 100644 --- a/pkg/webhook/validator.go +++ b/pkg/webhook/validator.go @@ -45,6 +45,7 @@ import ( "github.com/sigstore/policy-controller/pkg/webhook/registryauth" rekor "github.com/sigstore/rekor/pkg/client" "github.com/sigstore/rekor/pkg/generated/client" + "github.com/sigstore/sigstore-go/pkg/root" "github.com/sigstore/sigstore/pkg/cryptoutils" "github.com/sigstore/sigstore/pkg/fulcioroots" "github.com/sigstore/sigstore/pkg/signature" @@ -1338,10 +1339,10 @@ func normalizeArchitecture(cf *v1.ConfigFile) string { func checkOptsFromAuthority(ctx context.Context, authority webhookcip.Authority, remoteOpts ...ociremote.Option) (*cosign.CheckOpts, error) { ret := &cosign.CheckOpts{ RegistryClientOpts: remoteOpts, + NewBundleFormat: authority.SignatureFormat == "bundle", } - // Add in the identities for verification purposes, as well as Fulcio URL - // and certificates + // Add in the identities for verification purposes if authority.Keyless != nil { for _, id := range authority.Keyless.Identities { ret.Identities = append(ret.Identities, @@ -1351,6 +1352,67 @@ func checkOptsFromAuthority(ctx context.Context, authority webhookcip.Authority, IssuerRegExp: id.IssuerRegExp, SubjectRegExp: id.SubjectRegExp}) } + } + + if ret.NewBundleFormat { + // The new bundle format is only supported for keyless authorities + // and the trustRootRef must be set. + if authority.Keyless == nil { + // TODO: Support the new bundle format for non-keyless authorities + return nil, fmt.Errorf("when using the new bundle format, the authority must be keyless") + } + trustRootRef := authority.Keyless.TrustRootRef + if trustRootRef != "" { + // Set up TrustedMaterial + sigstoreKeys, err := sigstoreKeysFromContext(ctx, trustRootRef) + if err != nil { + return nil, fmt.Errorf("getting SigstoreKeys: %w", err) + } + sk, ok := sigstoreKeys.SigstoreKeys[trustRootRef] + if !ok { + return nil, fmt.Errorf("trustRootRef %s not found", trustRootRef) + } + ret.TrustedMaterial, err = root.NewTrustedRootFromProtobuf(sk) + if err != nil { + return nil, fmt.Errorf("failed to create trusted root from protobuf: %w", err) + } + } else { + var err error + ret.TrustedMaterial, err = root.FetchTrustedRoot() + if err != nil { + return nil, fmt.Errorf("failed to fetch trusted root: %w", err) + } + } + if authority.Keyless.InsecureIgnoreSCT != nil && *authority.Keyless.InsecureIgnoreSCT { + ret.IgnoreSCT = *authority.Keyless.InsecureIgnoreSCT + } + + // Check for custom TSA + tsa := authority.RFC3161Timestamp + if tsa != nil { + if tsa.TrustRootRef != authority.Keyless.TrustRootRef { + return nil, fmt.Errorf("when using the new bundle format, the trustRootRef for the TSA must be the same as the trustRootRef for the Keyless authority") + } + ret.UseSignedTimestamps = true + } + + // Check for custom Rekor + tlog := authority.CTLog + if tlog != nil { + if tlog.TrustRootRef != authority.Keyless.TrustRootRef { + return nil, fmt.Errorf("when using the new bundle format, the trustRootRef for the TLog must be the same as the trustRootRef for the Keyless authority") + } + // Only require the TLog if we're not using signed timestamps + if ret.UseSignedTimestamps { + ret.IgnoreTlog = true + } + } + return ret, nil + } + + // If we're not using the new bundle verifier (TrustedMaterial), we need to assemble the other CheckOpts (Fulcio, Rekor, TSA, etc.) + + if authority.Keyless != nil { fulcioRoots, fulcioIntermediates, ctlogKeys, err := fulcioCertsFromAuthority(ctx, authority.Keyless) if err != nil { return nil, fmt.Errorf("getting Fulcio certs: %s: %w", authority.Name, err) From 2c06b7545a20374ee622a9bd99de2fde081d98b4 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Tue, 15 Apr 2025 14:01:28 -0400 Subject: [PATCH 6/7] Use cached trusted root Signed-off-by: Cody Soyland --- pkg/webhook/validator.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/webhook/validator.go b/pkg/webhook/validator.go index 1f18eada3..491af26ca 100644 --- a/pkg/webhook/validator.go +++ b/pkg/webhook/validator.go @@ -41,6 +41,7 @@ import ( "github.com/sigstore/policy-controller/pkg/apis/config" policyduckv1beta1 "github.com/sigstore/policy-controller/pkg/apis/duck/v1beta1" policycontrollerconfig "github.com/sigstore/policy-controller/pkg/config" + pctuf "github.com/sigstore/policy-controller/pkg/tuf" webhookcip "github.com/sigstore/policy-controller/pkg/webhook/clusterimagepolicy" "github.com/sigstore/policy-controller/pkg/webhook/registryauth" rekor "github.com/sigstore/rekor/pkg/client" @@ -1378,7 +1379,7 @@ func checkOptsFromAuthority(ctx context.Context, authority webhookcip.Authority, } } else { var err error - ret.TrustedMaterial, err = root.FetchTrustedRoot() + ret.TrustedMaterial, err = pctuf.GetTrustedRoot(ctx) if err != nil { return nil, fmt.Errorf("failed to fetch trusted root: %w", err) } From 27c8ab462914c17fc42a1fb510b506162f8bdb72 Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Wed, 16 Apr 2025 16:14:43 -0400 Subject: [PATCH 7/7] Add tests for bundle checkopts Signed-off-by: Cody Soyland --- pkg/webhook/validator_test.go | 91 ++++++++++++++++++++++++++++++++--- 1 file changed, 85 insertions(+), 6 deletions(-) diff --git a/pkg/webhook/validator_test.go b/pkg/webhook/validator_test.go index bfc86c1bf..4e3e9775e 100644 --- a/pkg/webhook/validator_test.go +++ b/pkg/webhook/validator_test.go @@ -33,6 +33,7 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/google/go-containerregistry/pkg/authn/k8schain" "github.com/google/go-containerregistry/pkg/name" "github.com/sigstore/cosign/v2/pkg/cosign" @@ -46,6 +47,8 @@ import ( "github.com/sigstore/policy-controller/pkg/apis/signaturealgo" policycontrollerconfig "github.com/sigstore/policy-controller/pkg/config" webhookcip "github.com/sigstore/policy-controller/pkg/webhook/clusterimagepolicy" + pbcommon "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" + "github.com/sigstore/sigstore-go/pkg/root" "github.com/sigstore/sigstore/pkg/cryptoutils" "github.com/sigstore/sigstore/pkg/fulcioroots" "github.com/sigstore/sigstore/pkg/tuf" @@ -3240,10 +3243,12 @@ func TestCheckOptsFromAuthority(t *testing.T) { }}, } skCombined := config.SigstoreKeys{ + MediaType: "application/vnd.dev.sigstore.trustedroot+json;version=0.1", Tlogs: []*config.TransparencyLogInstance{{ - PublicKey: pbpkRekor, - LogId: &config.LogID{KeyId: []byte("rekor-logid")}, - BaseUrl: "rekor.example.com", + PublicKey: pbpkRekor, + LogId: &config.LogID{KeyId: []byte("rekor-logid")}, + BaseUrl: "rekor.example.com", + HashAlgorithm: pbcommon.HashAlgorithm_SHA2_256, }}, CertificateAuthorities: []*config.CertificateAuthority{{ Subject: &config.DistinguishedName{ @@ -3253,8 +3258,9 @@ func TestCheckOptsFromAuthority(t *testing.T) { CertChain: certChainPB, }}, Ctlogs: []*config.TransparencyLogInstance{{ - LogId: &config.LogID{KeyId: []byte(ctfeLogID)}, - PublicKey: pbpkCTFE, + LogId: &config.LogID{KeyId: []byte(ctfeLogID)}, + PublicKey: pbpkCTFE, + HashAlgorithm: pbcommon.HashAlgorithm_SHA2_256, }}, } c := &config.Config{ @@ -3355,6 +3361,79 @@ func TestCheckOptsFromAuthority(t *testing.T) { }}, CTLogPubKeys: &cosign.TrustedTransparencyLogPubKeys{Keys: map[string]cosign.TransparencyLogPubKey{ctfeLogID: {PubKey: pkCTFE, Status: tuf.Active}}}, }, + }, { + name: "bundle format, with Identities and Rekor", + authority: webhookcip.Authority{ + SignatureFormat: "bundle", + CTLog: &v1alpha1.TLog{ + URL: apis.HTTPS("rekor.example.com"), + TrustRootRef: "test-trust-combined", + }, + Keyless: &webhookcip.KeylessRef{ + TrustRootRef: "test-trust-combined", + Identities: []v1alpha1.Identity{{ + Issuer: "issuer", + Subject: "subject", + }}, + }, + }, + ctx: testCtx, + wantCheckOpts: &cosign.CheckOpts{ + NewBundleFormat: true, + Identities: []cosign.Identity{{ + Issuer: "issuer", + Subject: "subject", + }}, + TrustedMaterial: &root.TrustedRoot{}, + }, + }, { + name: "bundle format, with TSA", + authority: webhookcip.Authority{ + SignatureFormat: "bundle", + // Test keys do not contain a TSA but that is okay as we are just constructing the checkOpts + RFC3161Timestamp: &webhookcip.RFC3161Timestamp{ + TrustRootRef: "test-trust-combined", + }, + Keyless: &webhookcip.KeylessRef{ + TrustRootRef: "test-trust-combined", + }, + }, + ctx: testCtx, + wantCheckOpts: &cosign.CheckOpts{ + NewBundleFormat: true, + UseSignedTimestamps: true, + TrustedMaterial: &root.TrustedRoot{}, + }, + }, { + name: "bundle format, bad TrustRootRef", + authority: webhookcip.Authority{ + SignatureFormat: "bundle", + Keyless: &webhookcip.KeylessRef{ + TrustRootRef: "not-there", + }, + }, + ctx: testCtx, + wantErr: "trustRootRef not-there not found", + }, { + name: "bundle format, unsupported different trustroots", + authority: webhookcip.Authority{ + SignatureFormat: "bundle", + CTLog: &v1alpha1.TLog{ + TrustRootRef: "test-trust-rekor", + }, + Keyless: &webhookcip.KeylessRef{ + TrustRootRef: "test-trust-combined", + }, + }, + ctx: testCtx, + wantErr: "when using the new bundle format, the trustRootRef for the TLog must be the same as the trustRootRef for the Keyless authority", + }, { + name: "bundle format, unsupported non-keyless", + authority: webhookcip.Authority{ + SignatureFormat: "bundle", + }, + ctx: testCtx, + wantErr: "when using the new bundle format, the authority must be keyless", }} for _, tc := range tests { @@ -3384,7 +3463,7 @@ func TestCheckOptsFromAuthority(t *testing.T) { if gotCheckOpts != nil { gotCheckOpts.RekorClient = nil } - if diff := cmp.Diff(gotCheckOpts, tc.wantCheckOpts); diff != "" { + if diff := cmp.Diff(gotCheckOpts, tc.wantCheckOpts, cmpopts.IgnoreUnexported(root.TrustedRoot{})); diff != "" { t.Errorf("CheckOpts differ: %s", diff) } })