feat(oci): support insecure OCI registry #1374

l-qing · 2025-06-11T07:42:16Z

Changes

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

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

feat(oci): add complete insecure OCI registry support with security warnings

tekton-robot · 2025-06-11T08:00:25Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/oci/legacy.go	40.0%	44.5%	4.5

tekton-robot · 2025-06-11T08:09:44Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/oci/legacy.go	40.0%	44.5%	4.5

tekton-robot · 2025-06-11T08:27:32Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/oci/legacy.go	40.0%	44.5%	4.5

l-qing · 2025-06-26T02:39:37Z

/cc @PuneetPunamiya

pkg/chains/storage/oci/legacy.go

- 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

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.

l-qing · 2025-08-20T14:30:21Z

@waveywaves I’ve adjusted the code based on your suggestions. Please take a look when you have a moment. ❤️

tekton-robot · 2025-08-20T14:34:19Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/oci/legacy.go	40.0%	45.4%	5.4

l-qing · 2025-08-21T17:49:48Z

/retest

l-qing · 2025-08-24T03:36:37Z

pkg/chains/storage/oci/attestation.go

 	var entityNotFoundError *ociremote.EntityNotFoundError
 	if errors.As(err, &entityNotFoundError) {
-		se = ociremote.SignedUnknown(req.Artifact)
+		se = ociremote.SignedUnknown(req.Artifact, ociremote.WithRemoteOptions(s.remoteOpts...))


I understand that the method added in #1395 requires these opts to be passed.

tekton-robot · 2025-08-24T03:39:50Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/oci/legacy.go	40.0%	45.4%	5.4
pkg/chains/storage/oci/simple.go	83.3%	86.7%	3.3

- 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

tekton-robot · 2025-08-24T03:47:04Z

The following is the coverage report on the affected files.
Say /test pull-tekton-chains-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/chains/storage/oci/legacy.go	40.0%	45.4%	5.4
pkg/chains/storage/oci/simple.go	83.3%	86.7%	3.3

waveywaves · 2025-08-24T07:41:37Z

@l-qing really appreciate the extra test coverage 🤗
I will take a look at this PR soon

waveywaves

@l-qing thank you for the PR it looks good to me. This is a very useful feature and will help better test chains in environments with insecure registries like the one running in my laptop. The test for the insecure registry is good as well.

cc @lcarva

waveywaves · 2025-08-24T08:10:22Z

pkg/chains/storage/oci/oci_test.go

+// It verifies that:
+// 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) {


waveywaves · 2025-08-24T08:13:48Z

/approve

waveywaves · 2025-08-27T08:19:14Z

cc @anithapriyanatarajan

waveywaves · 2025-08-27T08:19:27Z

@PuneetPunamiya can you help merge this ?

waveywaves · 2025-09-11T08:24:02Z

cc @jkhelil

anithapriyanatarajan · 2025-12-13T07:03:47Z

/approve

anithapriyanatarajan · 2025-12-13T07:25:28Z

/hold

anithapriyanatarajan · 2025-12-13T07:25:58Z

/approve cancel

tekton-robot · 2025-12-13T07:26:02Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: waveywaves
To complete the pull request process, please assign jkhelil after the PR has been reviewed.
You can assign the PR to them by writing /assign @jkhelil in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

anithapriyanatarajan · 2025-12-13T07:45:23Z

@l-qing @waveywaves - IMHO, Though this PR may value add to a dev/test environment, allowing this in chains might pave way for security compromise in production. Provenance could be tampered leading to violation of SLSA guarantees that chains promises. @vdemeester @jkhelil - Though the implementation quality of the feature if good, I feel this contradicts what chains aims to accomplish. Please share your views?

l-qing · 2025-12-14T14:28:37Z

@anithapriyanatarajan Thank you for raising these security concerns - they're absolutely valid and important.

I'd like to address them with a few points:

1. Industry Standard Practice

This pattern exists across mature CNCF projects:

Docker's --insecure-registry flag
Kubernetes containerd/CRI-O insecure registry config
Skopeo, Podman, and other image tools

All face the same security tradeoff but provide necessary flexibility for dev/test environments.

2. Built-in Safeguards

✅ Disabled by default: insecure: false
✅ Admin-only: Requires cluster admin permission to modify Chains ConfigMap
✅ Explicit warnings: Security warning logged when enabled
✅ Clear documentation: Marked for dev/test use only

3. Real-World Use Case

In dev/test environments, we often need to work with private registries using self-signed certificates:

Local Harbor test instances
Internal development environments
CI/CD testing pipelines

Without this option, developers must configure CA certificates in every environment, which is impractical for rapid iteration.

4. Production Environment Concerns

You're right that enabling this in production would violate SLSA guarantees. However, this responsibility lies with cluster administrators - just as they shouldn't configure Docker's --insecure-registry in production.

Our role is to:

✅ Provide the tool
✅ Clearly warn about risks
✅ Default to secure configuration
❌ NOT override admin judgment

The feature doesn't compromise production security if admins follow best practices - which they must do anyway for Docker, Kubernetes, and other tools.

Looking forward to your thoughts. Happy to discuss further improvements.

waveywaves · 2025-12-15T06:53:51Z

@anithapriyanatarajan this flag will help the most when testing in a dev environment where setting up https might be cumbersome. As @l-qing here noted, the insecure flag is provided by multiple clients to ease developer overhead while testing as well. The attack vector related to insecure registries only gets activated if the admin of a registry in production makes the incorrect decision of enabling this flag. I believe the feature request as-is will help developers have a better chains dev env.

Though this PR may value add to a dev/test environment, allowing this in chains might pave way for security compromise in production.

This is a configuration that the admin needs to make sure is only enabled for developers, this is the admins prerogative

Provenance could be tampered leading to violation of SLSA guarantees that chains promises.

Documentation regarding this would be good as well and this should be added in the docs as well that the insecure flag can cause these issues @l-qing

tekton-robot requested review from lcarva and wlynch June 11, 2025 07:42

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 11, 2025

l-qing force-pushed the feat/support-insecure-oci-registry branch from c115260 to e23a70d Compare June 11, 2025 07:53

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 11, 2025

l-qing force-pushed the feat/support-insecure-oci-registry branch from e23a70d to 3706615 Compare June 11, 2025 07:54

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 11, 2025

l-qing force-pushed the feat/support-insecure-oci-registry branch from 20a3458 to 1e3bf22 Compare June 11, 2025 08:20

tekton-robot requested a review from PuneetPunamiya June 26, 2025 02:39

waveywaves suggested changes Aug 19, 2025

View reviewed changes

pkg/chains/storage/oci/legacy.go Show resolved Hide resolved

l-qing added 2 commits August 20, 2025 22:25

feat(oci): support insecure OCI registry

e484b16

- 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

chore(oci): add security warning for insecure OCI registry connections

590d65d

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.

l-qing force-pushed the feat/support-insecure-oci-registry branch from 1e3bf22 to 590d65d Compare August 20, 2025 14:29

l-qing commented Aug 24, 2025

View reviewed changes

fix(oci): pass remote options to SignedUnknown calls

ec9b775

- 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

l-qing force-pushed the feat/support-insecure-oci-registry branch from 885b1ba to ec9b775 Compare August 24, 2025 03:42

waveywaves approved these changes Aug 24, 2025

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2025

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 13, 2025

tekton-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2025

feat(oci): support insecure OCI registry #1374

Are you sure you want to change the base?

feat(oci): support insecure OCI registry #1374

Conversation

l-qing commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Submitter Checklist

Release Notes

Uh oh!

tekton-robot commented Jun 11, 2025

Uh oh!

tekton-robot commented Jun 11, 2025

Uh oh!

tekton-robot commented Jun 11, 2025

Uh oh!

l-qing commented Jun 26, 2025

Uh oh!

Uh oh!

l-qing commented Aug 20, 2025

Uh oh!

tekton-robot commented Aug 20, 2025

Uh oh!

l-qing commented Aug 21, 2025

Uh oh!

l-qing Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

tekton-robot commented Aug 24, 2025

Uh oh!

tekton-robot commented Aug 24, 2025

Uh oh!

waveywaves commented Aug 24, 2025

Uh oh!

waveywaves left a comment

Choose a reason for hiding this comment

Uh oh!

waveywaves Aug 24, 2025

Choose a reason for hiding this comment

Uh oh!

waveywaves commented Aug 24, 2025

Uh oh!

waveywaves commented Aug 27, 2025

Uh oh!

waveywaves commented Aug 27, 2025

Uh oh!

waveywaves commented Sep 11, 2025

Uh oh!

anithapriyanatarajan commented Dec 13, 2025

Uh oh!

anithapriyanatarajan commented Dec 13, 2025

Uh oh!

anithapriyanatarajan commented Dec 13, 2025

Uh oh!

tekton-robot commented Dec 13, 2025

Uh oh!

anithapriyanatarajan commented Dec 13, 2025

Uh oh!

l-qing commented Dec 14, 2025

1. Industry Standard Practice

2. Built-in Safeguards

3. Real-World Use Case

4. Production Environment Concerns

Uh oh!

waveywaves commented Dec 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

l-qing commented Jun 11, 2025 •

edited

Loading