oci: sort remaining quirks in cosign verify logic

This commit properly sets `IgnoreTlog` to `true` when a public key is
provided to check the signature against, which matches the (silent)
default behavior from cosign v1.

However, during this exercise it has become apparant that this
assumption isn't necessarily true. As you can theoretically have a
custom key and a tlog entry.

Given this, we should inventarise the possible configuration options
and the potential value they have to users (e.g. defining a custom
Rekor URL seems to be valuable as well), and extend our API to
facilitate these needs.

In addition to the above, the CTLog public keys are now properly
retrieved to avoid a `none of the CTFE keys have been found` error.

Signed-off-by: Hidde Beydals <[email protected]>

Author: hiddeco

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 <url>: no matching signatures:",
+			wantErrMsg: "chart verification error: failed to verify <url>: no signatures found for image",
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify <url>: no matching signatures:"),
-				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "chart verification error: failed to verify <url>: no matching signatures:"),
+				*conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify <url>: no signatures found for image"),
+				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "chart verification error: failed to verify <url>: 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 <url>: no matching signatures:"),
-				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "chart verification error: failed to verify <url>: no matching signatures:"),
+				*conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartVerificationError", "chart verification error: failed to verify <url>: no signatures found for image"),
+				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "chart verification error: failed to verify <url>: 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
 	}

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 '<revision>' for '<url>'"),
 				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<revision>' for '<url>'"),
-				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "failed to verify the signature using provider '<provider> keyless': no matching signatures"),
+				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "failed to verify the signature using provider '<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-",

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)
 		}
 

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,31 +106,31 @@ func NewCosignVerifier(ctx context.Context, opts ...Options) (*CosignVerifier, e
 		if err != nil {
 			return nil, err
 		}
-
-		checkOpts.Offline = true
-
 	} else {
-		rcerts, err := fulcio.GetRoots()
+		checkOpts.RekorClient, err = rekor.NewClient(coptions.DefaultRekorURL)
 		if err != nil {
-			return nil, fmt.Errorf("unable to get Fulcio root certs: %w", err)
+			return nil, fmt.Errorf("unable to create Rekor client: %w", err)
 		}
-		checkOpts.RootCerts = rcerts
 
-		icerts, err := fulcio.GetIntermediates()
-		if err != nil {
-			return nil, fmt.Errorf("unable to get Fulcio intermediate certs: %w", err)
+		// 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.IntermediateCerts = icerts
 
-		rc, err := rekor.NewClient(coptions.DefaultRekorURL)
+		checkOpts.CTLogPubKeys, err = cosign.GetCTLogPubs(ctx)
 		if err != nil {
-			return nil, fmt.Errorf("unable to create Rekor client: %w", err)
+			return nil, fmt.Errorf("unable to get CTLog public keys: %w", err)
 		}
-		checkOpts.RekorClient = rc
 
-		checkOpts.RekorPubKeys, err = cosign.GetRekorPubs(ctx)
-		if err != nil {
-			return nil, fmt.Errorf("unable to get Rekor 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)
 		}
 	}