From 632e42ecb00c80a569be5b265b85e56af385e5c0 Mon Sep 17 00:00:00 2001 From: qingliu Date: Wed, 11 Jun 2025 13:43:55 +0800 Subject: [PATCH 1/4] feat(oci): support insecure OCI registry - Add support for connecting to insecure OCI registries with self-signed certificates - Refactor remote options building into a dedicated method - Add comprehensive test coverage for both secure and insecure modes --- docs/config.md | 5 +- pkg/chains/storage/oci/legacy.go | 28 ++++++- pkg/chains/storage/oci/oci_test.go | 123 +++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+), 5 deletions(-) diff --git a/docs/config.md b/docs/config.md index c5e95f9226..1cec92f050 100644 --- a/docs/config.md +++ b/docs/config.md @@ -68,6 +68,7 @@ Supported keys include: |:-------------------------------------------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------| | `storage.gcs.bucket` | The GCS bucket for storage | | | | `storage.oci.repository` | The OCI repo to store OCI signatures and attestation in | If left undefined _and_ one of `artifacts.{oci,taskrun}.storage` includes `oci` storage, attestations will be stored alongside the stored OCI artifact itself. ([example on GCP](../images/attestations-in-artifact-registry.png)) Defining this value results in the OCI bundle stored in the designated location _instead of_ alongside the image. See [cosign documentation](https://github.com/sigstore/cosign#specifying-registry) for additional information. | | +| `storage.oci.repository.insecure` | Whether to use insecure connection when connecting to the OCI repository | `true`, `false` | `false` | | `storage.docdb.url` | The go-cloud URI reference to a docstore collection | `firestore://projects/[PROJECT]/databases/(default)/documents/[COLLECTION]?name_field=name` | | | `storage.docdb.mongo-server-url` (optional) | The value of MONGO_SERVER_URL env var with the MongoDB connection URI | Example: `mongodb://[USER]:[PASSWORD]@[HOST]:[PORT]/[DATABASE]` | | | `storage.docdb.mongo-server-url-dir` (optional) | The path of the directory that contains the file named MONGO_SERVER_URL that stores the value of MONGO_SERVER_URL env var | If the file `/mnt/mongo-creds-secret/MONGO_SERVER_URL` has the value of MONGO_SERVER_URL, then set `storage.docdb.mongo-server-url-dir: /mnt/mongo-creds-secret` | | @@ -106,9 +107,9 @@ You can provide MongoDB connection through different options * This field overrides all others (`mongo-server-url-dir, mongo-server-url, and MONGO_SERVER_URL env var`) * For instance, if `/mnt/mongo-creds-secret/mongo-server-url` contains the MongoDB URL, then set `storage.docdb.mongo-server-url-path`: `/mnt/mongo-creds-secret/mongo-server-url` -**NOTE** :- +**NOTE** :- * When using `storage.docdb.mongo-server-url-dir` or `storage.docdb.mongo-server-url-path` field, store the value of mongo server url in a secret and mount the secret. When the secret is updated, the new value will be fetched by Tekton Chains controller -* Also using `storage.docdb.mongo-server-url-dir` or `storage.docdb.mongo-server-url-path` field are recommended, using `storage.docdb.mongo-server-url` should be avoided since credentials are stored in a ConfigMap instead of a secret +* Also using `storage.docdb.mongo-server-url-dir` or `storage.docdb.mongo-server-url-path` field are recommended, using `storage.docdb.mongo-server-url` should be avoided since credentials are stored in a ConfigMap instead of a secret #### Grafeas diff --git a/pkg/chains/storage/oci/legacy.go b/pkg/chains/storage/oci/legacy.go index 950093220b..eed7e2df1f 100644 --- a/pkg/chains/storage/oci/legacy.go +++ b/pkg/chains/storage/oci/legacy.go @@ -15,9 +15,11 @@ package oci import ( "context" + "crypto/tls" "encoding/base64" "encoding/json" "fmt" + "net/http" "github.com/tektoncd/chains/pkg/chains/formats" "github.com/tektoncd/chains/pkg/chains/objects" @@ -75,7 +77,7 @@ func NewStorageBackend(ctx context.Context, client kubernetes.Interface, cfg con // StorePayload implements the storage.Backend interface. func (b *Backend) StorePayload(ctx context.Context, obj objects.TektonObject, rawPayload []byte, signature string, storageOpts config.StorageOpts) error { logger := logging.FromContext(ctx) - auth, err := b.getAuthenticator(ctx, obj, b.client) + remoteOpts, err := b.buildRemoteOptions(ctx, obj) if err != nil { return errors.Wrap(err, "getting oci authenticator") } @@ -87,7 +89,7 @@ func (b *Backend) StorePayload(ctx context.Context, obj objects.TektonObject, ra if err := json.Unmarshal(rawPayload, &format); err != nil { return errors.Wrap(err, "unmarshal simplesigning") } - return b.uploadSignature(ctx, format, rawPayload, signature, storageOpts, auth) + return b.uploadSignature(ctx, format, rawPayload, signature, storageOpts, remoteOpts...) } if _, ok := formats.IntotoAttestationSet[storageOpts.PayloadFormat]; ok { @@ -105,7 +107,7 @@ func (b *Backend) StorePayload(ctx context.Context, obj objects.TektonObject, ra return nil } - return b.uploadAttestation(ctx, &attestation, signature, storageOpts, auth) + return b.uploadAttestation(ctx, &attestation, signature, storageOpts, remoteOpts...) } // Fallback in case unsupported payload format is used or the deprecated "tekton" format @@ -113,6 +115,26 @@ func (b *Backend) StorePayload(ctx context.Context, obj objects.TektonObject, ra return nil } +// buildRemoteOptions build remote options for OCI storage backend +func (b *Backend) buildRemoteOptions(ctx context.Context, obj objects.TektonObject) ([]remote.Option, error) { + opts := []remote.Option{} + auth, err := b.getAuthenticator(ctx, obj, b.client) + if err != nil { + return nil, err + } + opts = append(opts, auth) + if b.cfg.Storage.OCI.Insecure { + // InsecureSkipVerify is used only when explicitly configured for testing or development environments + // This is controlled by the user through configuration and should not be used in production + opts = append(opts, remote.WithTransport(&http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, // #nosec G402 + }, + })) + } + return opts, nil +} + func (b *Backend) uploadSignature(ctx context.Context, format simple.SimpleContainerImage, rawPayload []byte, signature string, storageOpts config.StorageOpts, remoteOpts ...remote.Option) error { logger := logging.FromContext(ctx) diff --git a/pkg/chains/storage/oci/oci_test.go b/pkg/chains/storage/oci/oci_test.go index d760c975a4..183edd3edf 100644 --- a/pkg/chains/storage/oci/oci_test.go +++ b/pkg/chains/storage/oci/oci_test.go @@ -15,11 +15,14 @@ package oci import ( "context" + "crypto/tls" "encoding/json" + "fmt" "net/http/httptest" "net/url" "strings" "testing" + "time" "github.com/tektoncd/chains/pkg/chains/formats" "github.com/tektoncd/chains/pkg/chains/formats/simple" @@ -41,6 +44,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" logtesting "knative.dev/pkg/logging/testing" + "knative.dev/pkg/webhook/certificates/resources" ) const namespace = "oci-test" @@ -239,3 +243,122 @@ func TestBackend_StorePayload(t *testing.T) { }) } } + +// TestBackend_StorePayload_Insecure tests the StorePayload functionality with both secure and insecure configurations. +// It verifies that: +// 1. In secure mode, the backend should reject connections to untrusted registries +// 2. In insecure mode, the backend should attempt to connect but fail due to missing image +func TestBackend_StorePayload_Insecure(t *testing.T) { + // Setup test registry with self-signed certificate + s, registryURL := setupTestRegistry(t) + defer s.Close() + + testCases := []struct { + name string + insecure bool + wantErrMsg string + }{ + { + name: "secure mode - should reject untrusted registry", + insecure: false, + wantErrMsg: "tls: failed to verify certificate: x509:", + }, + { + name: "insecure mode - should attempt connection but fail due to missing image", + insecure: true, + wantErrMsg: "getting signed image: entity not found in registry", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Initialize backend with test configuration + b := &Backend{ + cfg: config.Config{ + Storage: config.StorageConfigs{ + OCI: config.OCIStorageConfig{ + Insecure: tc.insecure, + }, + }, + }, + getAuthenticator: func(context.Context, objects.TektonObject, kubernetes.Interface) (remote.Option, error) { + return remote.WithAuthFromKeychain(authn.DefaultKeychain), nil + }, + } + + // Create test reference and payload + ref := registryURL + "/task/test@sha256:0000000000000000000000000000000000000000000000000000000000000000" + simple := simple.SimpleContainerImage{ + Critical: payload.Critical{ + Identity: payload.Identity{ + DockerReference: registryURL + "/task/test", + }, + Image: payload.Image{ + DockerManifestDigest: strings.Split(ref, "@")[1], + }, + Type: payload.CosignSignatureType, + }, + } + + rawPayload, err := json.Marshal(simple) + if err != nil { + t.Fatalf("failed to marshal payload: %v", err) + } + + // Test StorePayload functionality + ctx := logtesting.TestContextWithLogger(t) + err = b.StorePayload(ctx, objects.NewTaskRunObjectV1(tr), rawPayload, "test", config.StorageOpts{ + PayloadFormat: formats.PayloadTypeSimpleSigning, + }) + + if err == nil { + t.Error("expected error but got nil") + return + } + if !strings.Contains(err.Error(), tc.wantErrMsg) { + t.Errorf("error message mismatch\ngot: %v\nwant: %v", err, tc.wantErrMsg) + } + }) + } +} + +// setupTestRegistry sets up a test registry with TLS configuration +func setupTestRegistry(t *testing.T) (*httptest.Server, string) { + t.Helper() + + cert, err := generateSelfSignedCert() + if err != nil { + t.Fatalf("failed to generate self-signed cert: %v", err) + } + + reg := registry.New() + s := httptest.NewUnstartedServer(reg) + s.TLS = &tls.Config{ + Certificates: []tls.Certificate{cert}, + } + s.StartTLS() + + u, _ := url.Parse(s.URL) + return s, u.Host +} + +// generateSelfSignedCert generates a self-signed certificate for testing purposes +// It uses knative's certificate generation utilities to create a proper certificate chain +func generateSelfSignedCert() (tls.Certificate, error) { + // Generate certificates with 24 hour validity + notAfter := time.Now().Add(24 * time.Hour) + + // Use test service name and namespace + serverKey, serverCert, _, err := resources.CreateCerts(context.Background(), "test-registry", "test-namespace", notAfter) + if err != nil { + return tls.Certificate{}, fmt.Errorf("failed to generate certificates: %w", err) + } + + // Parse the generated certificates + cert, err := tls.X509KeyPair(serverCert, serverKey) + if err != nil { + return tls.Certificate{}, fmt.Errorf("failed to parse certificate: %w", err) + } + + return cert, nil +} From 1dc885fae64b1ef595c353a50b8da8b16942da8b Mon Sep 17 00:00:00 2001 From: qingliu Date: Wed, 20 Aug 2025 22:28:36 +0800 Subject: [PATCH 2/4] chore(oci): add security warning for insecure OCI registry connections Add warning log when using insecure OCI registry configuration to alert users about potential security risks. This helps ensure the feature is only used in appropriate testing/development environments. --- pkg/chains/storage/oci/legacy.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/chains/storage/oci/legacy.go b/pkg/chains/storage/oci/legacy.go index eed7e2df1f..6004ae6da7 100644 --- a/pkg/chains/storage/oci/legacy.go +++ b/pkg/chains/storage/oci/legacy.go @@ -124,6 +124,8 @@ func (b *Backend) buildRemoteOptions(ctx context.Context, obj objects.TektonObje } opts = append(opts, auth) if b.cfg.Storage.OCI.Insecure { + logger := logging.FromContext(ctx) + logger.Warn("Using insecure OCI registry connection. This skips TLS certificate verification and poses security risks. Only use this in testing or development environments.") // InsecureSkipVerify is used only when explicitly configured for testing or development environments // This is controlled by the user through configuration and should not be used in production opts = append(opts, remote.WithTransport(&http.Transport{ From 5d01d66e171afa313e56a9c975b676fe0bb6e0fa Mon Sep 17 00:00:00 2001 From: qingliu Date: Sun, 24 Aug 2025 11:34:05 +0800 Subject: [PATCH 3/4] fix(oci): pass remote options to SignedUnknown calls - Ensure SignedUnknown uses the same remote options as SignedEntity - Fix insecure registry support for unknown/missing images - Improve test coverage for secure vs insecure mode validation --- pkg/chains/storage/oci/attestation.go | 2 +- pkg/chains/storage/oci/oci_test.go | 45 +++++++++++++++++---------- pkg/chains/storage/oci/simple.go | 2 +- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/pkg/chains/storage/oci/attestation.go b/pkg/chains/storage/oci/attestation.go index 2d2099058d..dbaeec25d6 100644 --- a/pkg/chains/storage/oci/attestation.go +++ b/pkg/chains/storage/oci/attestation.go @@ -63,7 +63,7 @@ func (s *AttestationStorer) Store(ctx context.Context, req *api.StoreRequest[nam se, err := ociremote.SignedEntity(req.Artifact, ociremote.WithRemoteOptions(s.remoteOpts...)) var entityNotFoundError *ociremote.EntityNotFoundError if errors.As(err, &entityNotFoundError) { - se = ociremote.SignedUnknown(req.Artifact) + se = ociremote.SignedUnknown(req.Artifact, ociremote.WithRemoteOptions(s.remoteOpts...)) } else if err != nil { return nil, errors.Wrap(err, "getting signed image") } diff --git a/pkg/chains/storage/oci/oci_test.go b/pkg/chains/storage/oci/oci_test.go index 183edd3edf..248ea0f10d 100644 --- a/pkg/chains/storage/oci/oci_test.go +++ b/pkg/chains/storage/oci/oci_test.go @@ -246,27 +246,33 @@ func TestBackend_StorePayload(t *testing.T) { // TestBackend_StorePayload_Insecure tests the StorePayload functionality with both secure and insecure configurations. // It verifies that: -// 1. In secure mode, the backend should reject connections to untrusted registries -// 2. In insecure mode, the backend should attempt to connect but fail due to missing image +// 1. In secure mode, the backend should reject connections to untrusted registries due to TLS certificate verification failure +// 2. In insecure mode, the backend should successfully connect and upload signatures, bypassing TLS verification func TestBackend_StorePayload_Insecure(t *testing.T) { // Setup test registry with self-signed certificate s, registryURL := setupTestRegistry(t) defer s.Close() testCases := []struct { - name string - insecure bool - wantErrMsg string + name string + insecure bool + wantErr bool + wantErrMsg string + description string }{ { - name: "secure mode - should reject untrusted registry", - insecure: false, - wantErrMsg: "tls: failed to verify certificate: x509:", + name: "secure mode with untrusted certificate", + insecure: false, + wantErr: true, + wantErrMsg: "tls: failed to verify certificate: x509:", + description: "Should reject connection to registry with self-signed certificate", }, { - name: "insecure mode - should attempt connection but fail due to missing image", - insecure: true, - wantErrMsg: "getting signed image: entity not found in registry", + name: "insecure mode bypassing TLS verification", + insecure: true, + wantErr: false, + wantErrMsg: "", + description: "Should successfully connect and upload signature despite untrusted certificate", }, } @@ -311,12 +317,17 @@ func TestBackend_StorePayload_Insecure(t *testing.T) { PayloadFormat: formats.PayloadTypeSimpleSigning, }) - if err == nil { - t.Error("expected error but got nil") - return - } - if !strings.Contains(err.Error(), tc.wantErrMsg) { - t.Errorf("error message mismatch\ngot: %v\nwant: %v", err, tc.wantErrMsg) + // Validate test results based on expected outcome + if tc.wantErr { + if err == nil { + t.Errorf("%s: expected error but got nil", tc.description) + return + } + if tc.wantErrMsg != "" && !strings.Contains(err.Error(), tc.wantErrMsg) { + t.Errorf("%s: error message mismatch\ngot: %v\nwant: %v", tc.description, err, tc.wantErrMsg) + } + } else if err != nil { + t.Errorf("%s: expected success but got error: %v", tc.description, err) } }) } diff --git a/pkg/chains/storage/oci/simple.go b/pkg/chains/storage/oci/simple.go index 30952ac7dc..ce970ad788 100644 --- a/pkg/chains/storage/oci/simple.go +++ b/pkg/chains/storage/oci/simple.go @@ -59,7 +59,7 @@ func (s *SimpleStorer) Store(ctx context.Context, req *api.StoreRequest[name.Dig se, err := ociremote.SignedEntity(req.Artifact, ociremote.WithRemoteOptions(s.remoteOpts...)) var entityNotFoundError *ociremote.EntityNotFoundError if errors.As(err, &entityNotFoundError) { - se = ociremote.SignedUnknown(req.Artifact) + se = ociremote.SignedUnknown(req.Artifact, ociremote.WithRemoteOptions(s.remoteOpts...)) } else if err != nil { return nil, errors.Wrap(err, "getting signed image") } From 16d950861e92bef5183f466442a696ae4305abb4 Mon Sep 17 00:00:00 2001 From: qingliu Date: Tue, 16 Dec 2025 15:56:49 +0800 Subject: [PATCH 4/4] docs(oci): add security warning for insecure registry flag Add comprehensive security warning documentation for the storage.oci.repository.insecure configuration option, highlighting: - Production environment risks - Man-in-the-Middle attack vulnerabilities - SLSA guarantee violations - Development-only usage recommendations --- docs/config.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/config.md b/docs/config.md index 1cec92f050..a6badf7209 100644 --- a/docs/config.md +++ b/docs/config.md @@ -78,6 +78,18 @@ Supported keys include: | `storage.grafeas.notehint` (optional) | This field is used to set the [human_readable_name](https://github.com/grafeas/grafeas/blob/cd23d4dc1bef740d6d6d90d5007db5c9a2431c41/proto/v1/attestation.proto#L49) field in the Grafeas ATTESTATION note. If it is not provided, the default `This attestation note was generated by Tekton Chains` will be used. | | | | `storage.archivista.url` | The URL endpoint for the Archivista service. | A valid HTTPS URL pointing to your Archivista instance (e.g. `https://archivista.testifysec.io`). | None | +> [!WARNING] +> **Security Considerations for `storage.oci.repository.insecure`** +> +> The `storage.oci.repository.insecure` flag allows connecting to OCI registries without TLS certificate verification. This feature is designed to ease developer overhead during testing and development where setting up HTTPS might be cumbersome. +> +> **Security Risks:** +> - **Production Environment Risk**: Enabling this flag in production environments can lead to serious security compromises. Administrators must ensure this flag is only enabled for development and testing purposes. +> - **Man-in-the-Middle Attacks**: Skipping TLS certificate verification makes the connection vulnerable to man-in-the-middle attacks where provenance could be tampered with. +> - **SLSA Guarantees Violation**: Tampered provenance can lead to violation of SLSA (Supply chain Levels for Software Artifacts) guarantees that Tekton Chains promises to provide. +> +> **Recommendation**: Only use `storage.oci.repository.insecure: true` in development or test environments. For production deployments, always use secure HTTPS connections with valid TLS certificates (`storage.oci.repository.insecure: false`, which is the default). + #### docstore You can read about the go-cloud docstore URI format [here](https://gocloud.dev/howto/docstore/). Tekton Chains supports the following docstore services: