-
Notifications
You must be signed in to change notification settings - Fork 5k
add per-endpoint CA certificate support for registry endpoints #22535
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
fixes goharbor#22203 Allowing Harbor to trust self-signed or private CA certificates for individual registry endpoints without modifying the system-level trust store. 1. Database schema changes, API updates with PEM validation. 2. HTTP transport layer modifications across all the registry adapters. 3. UI field to fill in the certificate. The feature is backward compatible - existing installations using system-level CA trust will continue to work without any changes. Signed-off-by: wang yan <[email protected]>
Signed-off-by: wang yan <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #22535 +/- ##
===========================================
+ Coverage 45.36% 65.84% +20.47%
===========================================
Files 244 1073 +829
Lines 13333 116269 +102936
Branches 2719 2931 +212
===========================================
+ Hits 6049 76554 +70505
- Misses 6983 35464 +28481
- Partials 301 4251 +3950
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Signed-off-by: wang yan <[email protected]>
|
Should the |
Signed-off-by: wang yan <[email protected]>
make sense and I've resolved it in the latest commit. Please help to review it again, thanks. |
chlins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- There is some UI logic issue that I tested and fixed.
- Added server-side validation logic
- Also updated the wording
here is a patch
From 751fd800212b534a1d067f49ecf7f2384341315e Mon Sep 17 00:00:00 2001
From: Vadim Bauer <[email protected]>
Date: Thu, 6 Nov 2025 22:40:00 +0100
Subject: [PATCH] refactor: enhance CA certificate handling and validation
logic
---
src/common/http/transport.go | 18 +++++++++++----
.../create-edit-endpoint.component.html | 6 ++---
.../create-edit-endpoint.component.ts | 20 ++++++++++++++++
src/portal/src/i18n/lang/en-us-lang.json | 4 ++--
src/server/v2.0/handler/registry.go | 23 ++++++++++++++++---
5 files changed, 58 insertions(+), 13 deletions(-)
diff --git a/src/common/http/transport.go b/src/common/http/transport.go
index bb1a880f4a..0794541034 100644
--- a/src/common/http/transport.go
+++ b/src/common/http/transport.go
@@ -158,10 +158,14 @@ func WithCACert(caCert string) TransportOption {
// GetHTTPTransport returns HttpTransport based on insecure configuration and CA certificate.
//
// Priority:
-// 1. Custom CA certificate (if provided) - creates a new transport with custom CA
-// 2. Insecure mode (if enabled) - returns shared transport that skips TLS verification
+// 1. Insecure mode (if enabled) - returns shared transport that skips TLS verification
+// 2. Custom CA certificate (if provided) - creates a new transport with custom CA
// 3. Default - returns shared transport that uses system CA pool
//
+// Note: Insecure mode and custom CA certificate are mutually exclusive. The API layer
+// validates and rejects requests that provide both. This priority order provides
+// defense-in-depth if both are somehow set.
+//
// Backward Compatibility:
// Existing Harbor installations that rely on system-level CA trust stores will continue
// to work after upgrade. The custom CA certificate feature is optional, and when no
@@ -172,14 +176,18 @@ func GetHTTPTransport(opts ...TransportOption) http.RoundTripper {
opt(cfg)
}
+ // Insecure mode takes precedence - skip all TLS verification
+ if cfg.Insecure {
+ return insecureHTTPTransport
+ }
+
+ // Use custom CA if provided
if cfg.CACertificate != "" {
return NewTransport(
WithCustomCACert(cfg.CACertificate),
)
}
- if cfg.Insecure {
- return insecureHTTPTransport
- }
+ // Default: use system CA pool
return secureHTTPTransport
}
diff --git a/src/portal/src/app/base/left-side-nav/registries/create-edit-endpoint/create-edit-endpoint.component.html b/src/portal/src/app/base/left-side-nav/registries/create-edit-endpoint/create-edit-endpoint.component.html
index 0cfc72a155..b590b0c2bf 100644
--- a/src/portal/src/app/base/left-side-nav/registries/create-edit-endpoint/create-edit-endpoint.component.html
+++ b/src/portal/src/app/base/left-side-nav/registries/create-edit-endpoint/create-edit-endpoint.component.html
@@ -239,7 +239,7 @@
clrCheckbox
#insecure
id="destination_insecure"
- [disabled]="testOngoing || !editable"
+ [disabled]="isVerifyRemoteCertDisabled()"
name="insecure"
[ngModel]="!target.insecure"
(ngModelChange)="setInsecureValue($event)" />
@@ -272,8 +272,8 @@
placeholder="-----BEGIN CERTIFICATE----- ... -----END CERTIFICATE-----"
name="ca_certificate"
[disabled]="testOngoing || target.insecure || !editable"
- [required]="!target.insecure"
- [(ngModel)]="target.ca_certificate"></textarea>
+ [(ngModel)]="target.ca_certificate"
+ (ngModelChange)="onCACertificateChange()"></textarea>
<clr-control-helper>
{{ 'DESTINATION.CA_CERTIFICATE_HELPER' | translate }}
</clr-control-helper>
diff --git a/src/portal/src/app/base/left-side-nav/registries/create-edit-endpoint/create-edit-endpoint.component.ts b/src/portal/src/app/base/left-side-nav/registries/create-edit-endpoint/create-edit-endpoint.component.ts
index b868755059..a02c1bf120 100644
--- a/src/portal/src/app/base/left-side-nav/registries/create-edit-endpoint/create-edit-endpoint.component.ts
+++ b/src/portal/src/app/base/left-side-nav/registries/create-edit-endpoint/create-edit-endpoint.component.ts
@@ -150,6 +150,25 @@ export class CreateEditEndpointComponent
setInsecureValue($event: any) {
this.target.insecure = !$event;
+ // Clear CA certificate when switching to insecure mode
+ if (this.target.insecure) {
+ this.target.ca_certificate = '';
+ }
+ }
+
+ onCACertificateChange() {
+ // If CA certificate is provided, force verification to be enabled
+ if (this.hasCACertificate()) {
+ this.target.insecure = false;
+ }
+ }
+
+ hasCACertificate(): boolean {
+ return !!this.target.ca_certificate && this.target.ca_certificate.trim().length > 0;
+ }
+
+ isVerifyRemoteCertDisabled(): boolean {
+ return this.testOngoing || !this.editable || this.hasCACertificate();
}
ngOnDestroy(): void {
@@ -303,6 +322,7 @@ export class CreateEditEndpointComponent
payload.access_key = this.target.credential.access_key;
payload.access_secret = this.target.credential.access_secret;
payload.insecure = this.target.insecure;
+ payload.ca_certificate = this.target.ca_certificate;
} else {
let changes: { [key: string]: any } = this.getChanges();
for (let prop of Object.keys(payload)) {
diff --git a/src/portal/src/i18n/lang/en-us-lang.json b/src/portal/src/i18n/lang/en-us-lang.json
index 5ce0ed756c..c3824fb473 100644
--- a/src/portal/src/i18n/lang/en-us-lang.json
+++ b/src/portal/src/i18n/lang/en-us-lang.json
@@ -777,8 +777,8 @@
"FAILED_TO_DELETE_TARGET_IN_USED": "Failed to delete the endpoint in use.",
"PLACEHOLDER": "We couldn't find any endpoints!",
"CA_CERTIFICATE": "CA Certificate",
- "CA_CERTIFICATE_TOOLTIP": "Provide a PEM-encoded CA certificate to verify the registry's TLS certificate. This field is required when 'Verify Remote Cert' is enabled. Uncheck 'Verify Remote Cert' to skip certificate verification entirely.",
- "CA_CERTIFICATE_HELPER": "Required when certificate verification is enabled. Paste the PEM-encoded CA certificate here to trust self-signed or private CA certificates for this endpoint."
+ "CA_CERTIFICATE_TOOLTIP": "Provide a PEM-encoded CA certificate to verify the registry's TLS certificate. Leave empty to use the system CA pool. Providing a CA certificate will automatically enable 'Verify Remote Cert'.",
+ "CA_CERTIFICATE_HELPER": "Optional. Paste the PEM-encoded CA certificate here to trust self-signed or private CA certificates for this endpoint. This field is only available when certificate verification is enabled."
},
"REPOSITORY": {
"COPY_DIGEST_ID": "Copy Digest",
diff --git a/src/server/v2.0/handler/registry.go b/src/server/v2.0/handler/registry.go
index dd28893a1b..d8cdc949e4 100644
--- a/src/server/v2.0/handler/registry.go
+++ b/src/server/v2.0/handler/registry.go
@@ -47,6 +47,12 @@ func (r *registryAPI) CreateRegistry(ctx context.Context, params operation.Creat
if err := r.RequireSystemAccess(ctx, rbac.ActionCreate, rbac.ResourceRegistry); err != nil {
return r.SendError(ctx, err)
}
+ // Validate mutual exclusivity: insecure mode and CA certificate cannot both be set
+ if params.Registry.Insecure && params.Registry.CaCertificate != nil && *params.Registry.CaCertificate != "" {
+ return r.SendError(ctx, errors.New(nil).WithCode(errors.BadRequestCode).
+ WithMessage("ca_certificate cannot be provided when insecure mode is enabled"))
+ }
+
registry := &model.Registry{
Name: params.Registry.Name,
Description: params.Registry.Description,
@@ -54,7 +60,7 @@ func (r *registryAPI) CreateRegistry(ctx context.Context, params operation.Creat
URL: params.Registry.URL,
Insecure: params.Registry.Insecure,
}
- if params.Registry.CaCertificate != nil {
+ if params.Registry.CaCertificate != nil && *params.Registry.CaCertificate != "" {
// Validate CA certificate format
if err := commonhttp.ValidateCACertificate(*params.Registry.CaCertificate); err != nil {
return r.SendError(ctx, errors.New(nil).WithCode(errors.BadRequestCode).WithMessage(err.Error()))
@@ -151,13 +157,18 @@ func (r *registryAPI) UpdateRegistry(ctx context.Context, params operation.Updat
if params.Registry.Insecure != nil {
registry.Insecure = *params.Registry.Insecure
}
- if params.Registry.CaCertificate != nil {
+ if params.Registry.CaCertificate != nil && *params.Registry.CaCertificate != "" {
// Validate CA certificate format
if err := commonhttp.ValidateCACertificate(*params.Registry.CaCertificate); err != nil {
return r.SendError(ctx, errors.New(nil).WithCode(errors.BadRequestCode).WithMessage(err.Error()))
}
registry.CACertificate = *params.Registry.CaCertificate
}
+ // Validate mutual exclusivity after all fields are set
+ if registry.Insecure && registry.CACertificate != "" {
+ return r.SendError(ctx, errors.New(nil).WithCode(errors.BadRequestCode).
+ WithMessage("ca_certificate cannot be provided when insecure mode is enabled"))
+ }
if registry.Credential == nil {
registry.Credential = &model.Credential{}
}
@@ -268,7 +279,7 @@ func (r *registryAPI) PingRegistry(ctx context.Context, params operation.PingReg
}
registry.Credential.AccessSecret = *params.Registry.AccessSecret
}
- if params.Registry.CaCertificate != nil {
+ if params.Registry.CaCertificate != nil && *params.Registry.CaCertificate != "" {
// Validate CA certificate format
if err := commonhttp.ValidateCACertificate(*params.Registry.CaCertificate); err != nil {
return r.SendError(ctx, errors.New(nil).WithCode(errors.BadRequestCode).WithMessage(err.Error()))
@@ -277,6 +288,12 @@ func (r *registryAPI) PingRegistry(ctx context.Context, params operation.PingReg
}
}
+ // Validate mutual exclusivity after all fields are set
+ if registry.Insecure && registry.CACertificate != "" {
+ return r.SendError(ctx, errors.New(nil).WithCode(errors.BadRequestCode).
+ WithMessage("ca_certificate cannot be provided when insecure mode is enabled"))
+ }
+
if len(registry.Type) == 0 || len(registry.URL) == 0 {
return r.SendError(ctx, errors.New(nil).WithCode(errors.BadRequestCode).WithMessage("type or url cannot be empty"))
}
--
2.49.0
```
thanks for the review! I made a few updates based on your feedback. I removed the [required] attribute from the CA certificate field and updated all the i18n text to show it’s optional now. When “Verify Remote Cert” is checked, the CA field is enabled but optional. It’s simpler, clearer for users, and still validated by the backend. |
bc4eb37 to
b271fab
Compare
bupd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
638f8db to
7913d5a
Compare
Signed-off-by: wang yan <[email protected]>
6937fa7 to
51580ce
Compare
fixes #22203
Allowing Harbor to trust self-signed or private CA certificates for individual registry endpoints without modifying the system-level trust store.
The feature is backward compatible - existing installations using system-level CA trust will continue to work without any changes.
Thank you for contributing to Harbor!
Comprehensive Summary of your change
Issue being fixed
Fixes #(issue)
Please indicate you've done the following: