Fix 22203

[wy65701436]

Thank you for contributing to Harbor!

Comprehensive Summary of your change

Issue being fixed

Fixes #(issue)

Please indicate you've done the following:

• Well Written Title and Summary of the PR
• Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
• Accepted the DCO. Commits without the DCO will delay acceptance.
• Made sure tests are passing and test coverage is added if needed.
• Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

[cursor]

Bug: CA Cert Silently Overrides Insecure Flag

When a custom CA certificate is provided, GetHTTPTransport creates a new transport with only the custom CA configured, ignoring the Insecure flag. This means if both WithInsecure(true) and WithCACert(cert) are passed, the insecure setting is silently dropped, potentially causing TLS verification to fail when the user expects it to be skipped.

 

[Copilot]

Pull Request Overview

This PR adds CA certificate support for registry endpoints in Harbor, allowing users to provide custom PEM-encoded CA certificates to verify TLS connections to registries using self-signed or private CA certificates.

• Added CACertificate field to registry models throughout the codebase (API, database, UI)
• Implemented CA certificate validation and custom transport configuration
• Updated all registry adapter implementations to support custom CA certificates

Reviewed Changes

Copilot reviewed 44 out of 44 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/common/http/transport.go	Added CA certificate validation and custom transport configuration with WithCustomCACert and ValidateCACertificate functions
src/common/http/transport_test.go	Added comprehensive tests for CA certificate validation and transport configuration
src/pkg/registry/client.go	Added NewClientWithCACert function and updated NewClientWithAuthorizer to support CA certificates
src/pkg/registry/auth/authorizer.go	Updated NewAuthorizer to accept optional CA certificate parameter
src/pkg/reg/model/registry.go	Added CACertificate field to Registry model
src/pkg/reg/dao/model.go	Added ca_certificate column to database model
src/pkg/reg/manager.go	Updated model conversion functions to include CA certificate field
src/pkg/reg/util/util.go	Updated GetHTTPTransport and Ping to support CA certificates
src/server/v2.0/handler/registry.go	Added CA certificate validation in CreateRegistry, UpdateRegistry, and PingRegistry endpoints
src/server/v2.0/handler/replication.go	Updated convertRegistry to include CA certificate in response
src/portal/src/app/shared/services/interface.ts	Added ca_certificate field to Endpoint and PingEndpoint interfaces
src/portal/src/app/base/left-side-nav/registries/create-edit-endpoint/create-edit-endpoint.component.ts	Added CA certificate to payload in ping operation
src/portal/src/app/base/left-side-nav/registries/create-edit-endpoint/create-edit-endpoint.component.html	Added CA certificate textarea field with tooltip, disabled when insecure mode is enabled
src/portal/src/i18n/lang/*.json	Added translations for CA certificate labels, tooltips, and helper text in 10 languages
make/migrations/postgresql/0180_2.15.0_schema.up.sql	Added database migration to add ca_certificate column to registry table
api/v2.0/swagger.yaml	Updated API schema to include ca_certificate field in registry-related definitions
src/pkg/reg/adapter/*	Updated all registry adapter implementations (AWS ECR, Azure ACR, AliACR, GitHub CR, GitLab, Harbor, Huawei, JFrog, Quay, Tencent CR, Volcengine CR, Docker Hub, DTR) to pass CA certificate when creating HTTP transports
src/controller/proxy/local.go	Updated local proxy helper to pass empty CA certificate string to maintain backward compatibility

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Same issue as above: RootCAs.Subjects() is deprecated as of Go 1.18. Consider using an alternative method to verify the certificate pool is empty.

[Copilot]

The commented-out code for certificate signature verification should either be removed or uncommented with a clear explanation. Keeping commented code in production can lead to confusion about intended behavior and creates maintenance debt.

Suggested change

	// Verify signature for self-signed certificates (root CAs)
	//if cert.Subject.String() == cert.Issuer.String() {
	// if err := cert.CheckSignature(cert.SignatureAlgorithm, cert.RawTBSCertificate, cert.Signature); err != nil {
	// log.Errorf("CA certificate signature verification failed: %v", err)
	// continue
	// }
	//}
	// Verify signature for self-signed certificates (root CAs).
	// This ensures that only valid root CA certificates are trusted.
	if cert.Subject.String() == cert.Issuer.String() {
	if err := cert.CheckSignature(cert.SignatureAlgorithm, cert.RawTBSCertificate, cert.Signature); err != nil {
	log.Errorf("CA certificate signature verification failed: %v", err)
	continue
	}
	}

[Copilot]

When a CA certificate is provided in GetHTTPTransport, the insecure flag is not applied to the transport. This means that if both WithInsecure(true) and WithCACert(caCert) are passed, InsecureSkipVerify will not be set on the TLSClientConfig, which may not match user expectations. Consider either:

1. Setting InsecureSkipVerify when insecure=true even with a custom CA, or
2. Documenting this behavior clearly in the function comment

Currently, the priority documentation mentions CA cert takes precedence, but doesn't explicitly state that the insecure flag is ignored when a CA cert is provided.

[Copilot]

The validation only checks for the presence of "-----BEGIN CERTIFICATE-----" and "-----END CERTIFICATE-----" markers, but doesn't validate that they appear in the correct order or that there's actual content between them. A certificate like "-----END CERTIFICATE-----\n-----BEGIN CERTIFICATE-----" would pass these checks but fail later. Consider using pem.Decode to check proper formatting before attempting to parse.

Suggested change

	if !strings.HasPrefix(caCert, "-----BEGIN CERTIFICATE-----") {
	return errors.New("invalid CA certificate: must start with -----BEGIN CERTIFICATE-----")
	}
	if !strings.HasSuffix(caCert, "-----END CERTIFICATE-----") {
	return errors.New("invalid CA certificate: must end with -----END CERTIFICATE-----")
	}
	block, _ := pem.Decode([]byte(caCert))
	if block == nil || block.Type != "CERTIFICATE" || len(block.Bytes) == 0 {
	return errors.New("invalid CA certificate: not a valid PEM encoded CERTIFICATE block")
	}

[Copilot]

When insecure=true and a CA certificate is provided together in NewAuthorizer, the transport will have a custom CA but won't have InsecureSkipVerify set (due to the priority in GetHTTPTransport). This could lead to unexpected behavior where certificate validation still occurs despite the insecure flag being true. Consider handling this case explicitly or documenting the interaction between these two parameters.

Suggested change

	if len(caCert) > 0 && caCert[0] != "" {
	transport = commonhttp.GetHTTPTransport(
	commonhttp.WithInsecure(insecure),
	commonhttp.WithCACert(caCert[0]),
	)
	// If both insecure and caCert are provided, ensure InsecureSkipVerify is set
	if len(caCert) > 0 && caCert[0] != "" {
	if insecure {
	// Both insecure and CA cert provided: set both options
	transport = commonhttp.GetHTTPTransport(
	commonhttp.WithInsecure(true),
	commonhttp.WithCACert(caCert[0]),
	)
	} else {
	transport = commonhttp.GetHTTPTransport(
	commonhttp.WithCACert(caCert[0]),
	)
	}

[Copilot]

When caCert != "", the function creates a new transport with only the CA certificate option, ignoring the insecure parameter. If insecure=true is passed along with a CA certificate, the InsecureSkipVerify flag won't be set on the TLSClientConfig. This is inconsistent with the behavior in NewAuthorizer where both options are passed to GetHTTPTransport. Consider applying the insecure flag when creating the transport:

if caCert != "" {
    transport = commonhttp.GetHTTPTransport(
        commonhttp.WithInsecure(insecure),
        commonhttp.WithCACert(caCert))
}

Suggested change

	// Custom CA certificate takes precedence
	transport = commonhttp.GetHTTPTransport(commonhttp.WithCACert(caCert))
	// Apply both insecure and custom CA certificate options
	transport = commonhttp.GetHTTPTransport(
	commonhttp.WithInsecure(insecure),
	commonhttp.WithCACert(caCert))

[Copilot]

The test verifies that RootCAs is not nil and empty for invalid certificates, but there's a potential issue: RootCAs.Subjects() is deprecated as of Go 1.18. Consider using len(tr.TLSClientConfig.RootCAs.Subjects()) == 0 or checking in a different way that doesn't rely on the deprecated Subjects() method to ensure forward compatibility.

[github-actions]

This PR is being marked stale due to a period of inactivty. If this PR is still relevant, please comment or remove the stale label. Otherwise, this PR will close in 30 days.

[cursor]

Wrong URL passed to upload status query

High Severity

The getUploadStatus call incorrectly passes url instead of location. The url variable contains query parameters added by buildChunkBlobUploadURL (including digest for the last chunk), while getUploadStatus expects the base upload location URL without query parameters. This causes the upload status query to fail or return incorrect results when a chunk upload fails, breaking error recovery for chunked blob uploads.