diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go index 3856d82d..b6b3430d 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -23,7 +23,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "net/http" "os" "path" @@ -1058,7 +1057,8 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) { ) // Load a test chart - chartData, err := ioutil.ReadFile(chartPath) + chartData, err := os.ReadFile(chartPath) + g.Expect(err).NotTo(HaveOccurred()) // Upload the test chart metadata, err := loadTestChartToOCI(chartData, chartPath, testRegistryServer) @@ -2333,16 +2333,16 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) { builder := fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()) workspaceDir := t.TempDir() - server, err := setupRegistryServer(ctx, workspaceDir, tt.registryOpts) + server, err := setupRegistryServer(ctx, workspaceDir, tt.registryOpts) g.Expect(err).NotTo(HaveOccurred()) // Load a test chart - chartData, err := ioutil.ReadFile(chartPath) + chartData, err := os.ReadFile(chartPath) + g.Expect(err).ToNot(HaveOccurred()) // Upload the test chart metadata, err := loadTestChartToOCI(chartData, chartPath, server) - g.Expect(err).NotTo(HaveOccurred()) g.Expect(err).ToNot(HaveOccurred()) repo := &helmv1.HelmRepository{ @@ -2452,7 +2452,8 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T ) // Load a test chart - chartData, err := ioutil.ReadFile(chartPath) + chartData, err := os.ReadFile(chartPath) + g.Expect(err).ToNot(HaveOccurred()) // Upload the test chart metadata, err := loadTestChartToOCI(chartData, chartPath, server) @@ -2504,10 +2505,10 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T }, want: sreconcile.ResultEmpty, wantErr: true, - wantErrMsg: "chart verification error: failed to verify : no matching signatures:", + wantErrMsg: "chart verification error: failed to verify : no signatures found for image", assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify : no matching signatures:"), - *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "chart verification error: failed to verify : no matching signatures:"), + *conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify : no signatures found for image"), + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "chart verification error: failed to verify : no signatures found for image"), }, }, { @@ -2522,8 +2523,8 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T want: sreconcile.ResultEmpty, wantErr: true, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify : no matching signatures:"), - *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "chart verification error: failed to verify : no matching signatures:"), + *conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify : no signatures found for image"), + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "chart verification error: failed to verify : no signatures found for image"), }, }, { @@ -2696,7 +2697,7 @@ func loadTestChartToOCI(chartData []byte, chartPath string, server *registryClie } // Load a test chart - chartData, err = ioutil.ReadFile(chartPath) + chartData, err = os.ReadFile(chartPath) if err != nil { return nil, err } diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go index 111b40ce..072e9811 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/internal/controller/ocirepository_controller_test.go @@ -1095,7 +1095,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) { assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '' for ''"), *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '' for ''"), - *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "failed to verify the signature using provider ' keyless': no matching signatures"), + *conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "failed to verify the signature using provider ' keyless': no signatures found for image"), }, }, { @@ -1193,6 +1193,8 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + obj := &ociv1.OCIRepository{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "verify-oci-source-signature-", diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index ad1798e8..d45779c7 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -21,7 +21,6 @@ import ( "context" "fmt" "io" - "io/ioutil" "math/rand" "os" "path/filepath" @@ -164,8 +163,7 @@ func setupRegistryServer(ctx context.Context, workspaceDir string, opts registry } htpasswdPath := filepath.Join(workspaceDir, testRegistryHtpasswdFileBasename) - err = ioutil.WriteFile(htpasswdPath, []byte(fmt.Sprintf("%s:%s\n", testRegistryUsername, string(pwBytes))), 0644) - if err != nil { + if err = os.WriteFile(htpasswdPath, []byte(fmt.Sprintf("%s:%s\n", testRegistryUsername, string(pwBytes))), 0644); err != nil { return nil, fmt.Errorf("failed to create htpasswd file: %s", err) } diff --git a/internal/oci/verifier.go b/internal/oci/verifier.go index 1cb35280..77306c7d 100644 --- a/internal/oci/verifier.go +++ b/internal/oci/verifier.go @@ -21,15 +21,14 @@ import ( "crypto" "fmt" + "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/sigstore/cosign/v2/cmd/cosign/cli/fulcio" + coptions "github.com/sigstore/cosign/v2/cmd/cosign/cli/options" "github.com/sigstore/cosign/v2/cmd/cosign/cli/rekor" "github.com/sigstore/cosign/v2/pkg/cosign" - ociremote "github.com/sigstore/cosign/v2/pkg/oci/remote" - - "github.com/google/go-containerregistry/pkg/name" - coptions "github.com/sigstore/cosign/v2/cmd/cosign/cli/options" "github.com/sigstore/cosign/v2/pkg/oci" + ociremote "github.com/sigstore/cosign/v2/pkg/oci/remote" "github.com/sigstore/sigstore/pkg/cryptoutils" "github.com/sigstore/sigstore/pkg/signature" ) @@ -93,6 +92,11 @@ func NewCosignVerifier(ctx context.Context, opts ...Options) (*CosignVerifier, e // If there is no public key provided, it will try keyless verification. // https://github.com/sigstore/cosign/blob/main/KEYLESS.md. if len(o.PublicKey) > 0 { + checkOpts.Offline = true + // TODO(hidde): this is an oversight in our implementation. As it is + // theoretically possible to have a custom PK, without disabling tlog. + checkOpts.IgnoreTlog = true + pubKeyRaw, err := cryptoutils.UnmarshalPEMToPublicKey(o.PublicKey) if err != nil { return nil, err @@ -102,32 +106,32 @@ func NewCosignVerifier(ctx context.Context, opts ...Options) (*CosignVerifier, e if err != nil { return nil, err } - - checkOpts.Offline = true - } else { - rcerts, err := fulcio.GetRoots() - if err != nil { - return nil, fmt.Errorf("unable to get Fulcio root certs: %w", err) - } - checkOpts.RootCerts = rcerts - - icerts, err := fulcio.GetIntermediates() - if err != nil { - return nil, fmt.Errorf("unable to get Fulcio intermediate certs: %w", err) - } - checkOpts.IntermediateCerts = icerts - - rc, err := rekor.NewClient(coptions.DefaultRekorURL) + checkOpts.RekorClient, err = rekor.NewClient(coptions.DefaultRekorURL) if err != nil { return nil, fmt.Errorf("unable to create Rekor client: %w", err) } - checkOpts.RekorClient = rc - checkOpts.RekorPubKeys, err = cosign.GetRekorPubs(ctx) - if err != nil { + // This performs an online fetch of the Rekor public keys, but this is needed + // for verifying tlog entries (both online and offline). + // TODO(hidde): above note is important to keep in mind when we implement + // "offline" tlog above. + if checkOpts.RekorPubKeys, err = cosign.GetRekorPubs(ctx); err != nil { return nil, fmt.Errorf("unable to get Rekor public keys: %w", err) } + + checkOpts.CTLogPubKeys, err = cosign.GetCTLogPubs(ctx) + if err != nil { + return nil, fmt.Errorf("unable to get CTLog public keys: %w", err) + } + + if checkOpts.RootCerts, err = fulcio.GetRoots(); err != nil { + return nil, fmt.Errorf("unable to get Fulcio root certs: %w", err) + } + + if checkOpts.IntermediateCerts, err = fulcio.GetIntermediates(); err != nil { + return nil, fmt.Errorf("unable to get Fulcio intermediate certs: %w", err) + } } return &CosignVerifier{