fix(registry): allow trusted tls custom registries [r8s-489] (#1116)

api/http/handler/registries/registry_configure.go

@@ -59,23 +59,28 @@ func (payload *registryConfigurePayload) Validate(r *http.Request) error {
 	payload.TLSSkipVerify = skipTLSVerify
 
 	if useTLS && !skipTLSVerify {
+		numCertsProvided := 0
 		cert, _, err := request.RetrieveMultiPartFormFile(r, "TLSCertFile")
-		if err != nil {
-			return errors.New("invalid certificate file. Ensure that the file is uploaded correctly")
+		if err == nil {
+			payload.TLSCertFile = cert
+			numCertsProvided++
 		}
-		payload.TLSCertFile = cert
 
 		key, _, err := request.RetrieveMultiPartFormFile(r, "TLSKeyFile")
-		if err != nil {
-			return errors.New("invalid key file. Ensure that the file is uploaded correctly")
+		if err == nil {
+			payload.TLSKeyFile = key
+			numCertsProvided++
 		}
-		payload.TLSKeyFile = key
 
 		ca, _, err := request.RetrieveMultiPartFormFile(r, "TLSCACertFile")
-		if err != nil {
-			return errors.New("invalid CA certificate file. Ensure that the file is uploaded correctly")
+		if err == nil {
+			payload.TLSCACertFile = ca
+			numCertsProvided++
+		}
+
+		if numCertsProvided != 0 && numCertsProvided != 3 {
+			return errors.New("invalid TLS configuration: provide TLS certificate, key, and CA together, or none")
 		}
-		payload.TLSCACertFile = ca
 	}
 
 	return nil

api/http/handler/registries/registry_configure_test.go

@@ -0,0 +1,101 @@
+package registries
+
+import (
+	"bytes"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// helper to build a multipart request for registry configure validation
+func newConfigureRequest(t *testing.T, tls bool, skipVerify bool, includeCert bool, includeKey bool, includeCA bool) *http.Request {
+	t.Helper()
+
+	body := &bytes.Buffer{}
+	writer := multipart.NewWriter(body)
+
+	// flags
+	_ = writer.WriteField("TLS", map[bool]string{true: "true", false: "false"}[tls])
+	_ = writer.WriteField("TLSSkipVerify", map[bool]string{true: "true", false: "false"}[skipVerify])
+
+	// files
+	if includeCert {
+		fw, err := writer.CreateFormFile("TLSCertFile", "cert.pem")
+		if err != nil {
+			t.Fatalf("failed to create cert file: %v", err)
+		}
+		_, _ = fw.Write([]byte("CERTDATA"))
+	}
+	if includeKey {
+		fw, err := writer.CreateFormFile("TLSKeyFile", "key.pem")
+		if err != nil {
+			t.Fatalf("failed to create key file: %v", err)
+		}
+		_, _ = fw.Write([]byte("KEYDATA"))
+	}
+	if includeCA {
+		fw, err := writer.CreateFormFile("TLSCACertFile", "ca.pem")
+		if err != nil {
+			t.Fatalf("failed to create ca file: %v", err)
+		}
+		_, _ = fw.Write([]byte("CADATA"))
+	}
+
+	_ = writer.Close()
+
+	req := httptest.NewRequest(http.MethodPost, "/registries/1/configure", body)
+	req.Header.Set("Content-Type", writer.FormDataContentType())
+	return req
+}
+
+func Test_registryConfigurePayload_Validate_TLSBundleRules(t *testing.T) {
+	// passes when all three are uploaded
+	{
+		req := newConfigureRequest(t, true, false, true, true, true)
+		p := &registryConfigurePayload{}
+		if err := p.Validate(req); err != nil {
+			t.Fatalf("expected validation to pass when all certs provided, got error: %v", err)
+		}
+		if len(p.TLSCertFile) == 0 || len(p.TLSKeyFile) == 0 || len(p.TLSCACertFile) == 0 {
+			t.Fatalf("expected payload to contain all cert bytes")
+		}
+	}
+
+	// passes when none are uploaded
+	{
+		req := newConfigureRequest(t, true, false, false, false, false)
+		p := &registryConfigurePayload{}
+		if err := p.Validate(req); err != nil {
+			t.Fatalf("expected validation to pass when no certs provided, got error: %v", err)
+		}
+		if len(p.TLSCertFile) != 0 || len(p.TLSKeyFile) != 0 || len(p.TLSCACertFile) != 0 {
+			t.Fatalf("expected payload to have no cert bytes when none provided")
+		}
+	}
+
+	// fails on partial uploads (1 or 2 of the files)
+	partialCases := []struct {
+		name string
+		cert bool
+		key  bool
+		ca   bool
+	}{
+		{"only-cert", true, false, false},
+		{"only-key", false, true, false},
+		{"only-ca", false, false, true},
+		{"cert-and-key", true, true, false},
+		{"cert-and-ca", true, false, true},
+		{"key-and-ca", false, true, true},
+	}
+
+	for _, tc := range partialCases {
+		t.Run(tc.name, func(t *testing.T) {
+			req := newConfigureRequest(t, true, false, tc.cert, tc.key, tc.ca)
+			p := &registryConfigurePayload{}
+			if err := p.Validate(req); err == nil {
+				t.Fatalf("expected validation to fail on partial cert upload")
+			}
+		})
+	}
+}

api/http/handler/registries/registry_create.go

@@ -41,6 +41,8 @@ type registryCreatePayload struct {
 	Quay portainer.QuayRegistryData
 	// ECR specific details, required when type = 7
 	Ecr portainer.EcrData
+	// Use TLS
+	TLS bool `example:"true"`
 }
 
 func (payload *registryCreatePayload) Validate(_ *http.Request) error {
@@ -120,6 +122,7 @@ func (handler *Handler) registryCreate(w http.ResponseWriter, r *http.Request) *
 	}
 
 	registry.ManagementConfiguration = syncConfig(registry)
+	registry.ManagementConfiguration.TLSConfig.TLS = payload.TLS
 
 	registries, err := handler.DataStore.Registry().ReadAll()
 	if err != nil {

app/portainer/models/registry.js

@@ -59,6 +59,11 @@ export function RegistryCreateRequest(model) {
   this.URL = _.replace(model.URL, /^https?\:\/\//i, '');
   this.URL = _.replace(this.URL, /\/$/, '');
   this.Authentication = model.Authentication;
+  // default TLS based on URL scheme: enable TLS unless explicitly http://
+  const isHttp = /^http:\/\//i.test(model.URL);
+  if (!isHttp) {
+    this.TLS = true;
+  }
   if (model.Authentication) {
     this.Username = model.Username;
     this.Password = model.Password;

pkg/registryhttp/client.go

@@ -4,7 +4,6 @@ import (
 	"net/http"
 
 	portainer "github.com/portainer/portainer/api"
-	"github.com/portainer/portainer/api/crypto"
 
 	"github.com/rs/zerolog/log"
 	"oras.land/oras-go/v2/registry/remote/retry"
@@ -13,37 +12,50 @@ import (
 // CreateClient creates an HTTP client with appropriate TLS configuration based on registry type.
 // All registries use retry clients for better resilience.
 // Returns the HTTP client, whether to use plainHTTP, and any error.
-func CreateClient(registry *portainer.Registry) (*http.Client, bool, error) {
+func CreateClient(registry *portainer.Registry) (httpClient *http.Client, usePlainHttp bool, err error) {
 	switch registry.Type {
 	case portainer.AzureRegistry, portainer.EcrRegistry, portainer.GithubRegistry, portainer.GitlabRegistry, portainer.DockerHubRegistry:
 		// Cloud registries use the default retry client with built-in TLS
 		return retry.DefaultClient, false, nil
 	default:
-		// For all other registry types, check if custom TLS is needed
-		if registry.ManagementConfiguration != nil && registry.ManagementConfiguration.TLSConfig.TLS {
-			// Need custom TLS configuration - create a retry client with custom transport
-			baseTransport := &http.Transport{
-				Proxy: http.ProxyFromEnvironment,
-			}
+		// For all other registry types, use shared helper to build transport and scheme
 
-			tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(
-				registry.ManagementConfiguration.TLSConfig,
-			)
-			if err != nil {
-				log.Error().Err(err).Msg("Failed to create TLS configuration")
-				return nil, false, err
-			}
-			baseTransport.TLSClientConfig = tlsConfig
-
-			// Create a retry transport wrapping our custom base transport
-			retryTransport := retry.NewTransport(baseTransport)
-			httpClient := &http.Client{
-				Transport: retryTransport,
-			}
-			return httpClient, false, nil
+		// if no management configuration, treat as plain HTTP for custom registries
+		hasConfiguration := registry.ManagementConfiguration != nil
+		if !hasConfiguration {
+			return retry.DefaultClient, true, nil
 		}
 
-		// Default to HTTP for non-cloud registries without TLS configuration
-		return retry.DefaultClient, true, nil
+		tlsCfg := registry.ManagementConfiguration.TLSConfig
+
+		// If TLS is disabled, use plain HTTP with default client
+		if !tlsCfg.TLS {
+			return retry.DefaultClient, true, nil
+		}
+
+		// If TLS is enabled and uses trusted system CA (no custom bundle, no skip-verify),
+		// use the default retry client over HTTPS
+		usesTrustedSystemCA := !tlsCfg.TLSSkipVerify && tlsCfg.TLSCACertPath == "" && tlsCfg.TLSCertPath == "" && tlsCfg.TLSKeyPath == ""
+		if usesTrustedSystemCA {
+			return retry.DefaultClient, false, nil
+		}
+
+		transport, scheme, err := BuildTransportAndSchemeFromTLSConfig(tlsCfg)
+		if err != nil {
+			log.Error().Err(err).Msg("Failed to create TLS configuration")
+			return nil, false, err
+		}
+
+		// If scheme is http, we can use the default client and instruct callers to use plainHTTP
+		if scheme == "http" {
+			return retry.DefaultClient, true, nil
+		}
+
+		// For https, wrap our transport with retry
+		retryTransport := retry.NewTransport(transport)
+		httpClient := &http.Client{
+			Transport: retryTransport,
+		}
+		return httpClient, false, nil
 	}
 }

pkg/registryhttp/client_test.go

@@ -202,3 +202,47 @@ func TestCreateClient_CustomTLSConfiguration(t *testing.T) {
 		assert.Equal(t, retry.DefaultClient, client, "No management config should use default client")
 	})
 }
+
+func TestCreateClient_TLSWithTrustedCerts_UsesDefaultClientHTTPS(t *testing.T) {
+	registry := &portainer.Registry{
+		Type: portainer.CustomRegistry,
+		URL:  "my-registry.local",
+		ManagementConfiguration: &portainer.RegistryManagementConfiguration{
+			TLSConfig: portainer.TLSConfiguration{
+				TLS:           true,
+				TLSSkipVerify: false,
+				TLSCACertPath: "",
+				TLSCertPath:   "",
+				TLSKeyPath:    "",
+			},
+		},
+	}
+
+	client, usePlainHTTP, err := CreateClient(registry)
+
+	require.NoError(t, err)
+	assert.False(t, usePlainHTTP, "Trusted TLS should use HTTPS")
+	assert.Equal(t, retry.DefaultClient, client, "Trusted TLS should use default retry client")
+}
+
+func TestCreateClient_CustomTLS_WithCertPathsMissing_ReturnsError(t *testing.T) {
+	registry := &portainer.Registry{
+		Type: portainer.CustomRegistry,
+		URL:  "my-registry.local",
+		ManagementConfiguration: &portainer.RegistryManagementConfiguration{
+			TLSConfig: portainer.TLSConfiguration{
+				TLS:           true,
+				TLSSkipVerify: false,
+				TLSCACertPath: "/not/found/ca.pem",
+				TLSCertPath:   "/not/found/cert.pem",
+				TLSKeyPath:    "/not/found/key.pem",
+			},
+		},
+	}
+
+	client, usePlainHTTP, err := CreateClient(registry)
+
+	require.Error(t, err)
+	assert.Nil(t, client)
+	assert.False(t, usePlainHTTP)
+}

pkg/registryhttp/transport.go

@@ -0,0 +1,38 @@
+package registryhttp
+
+import (
+	"net/http"
+
+	portainer "github.com/portainer/portainer/api"
+	"github.com/portainer/portainer/api/crypto"
+)
+
+// BuildTransportAndSchemeFromTLSConfig returns a base HTTP transport configured
+// with ProxyFromEnvironment and, when needed, a TLSClientConfig derived from the
+// provided TLS settings. It also returns the scheme ("http" or "https") that
+// should be used to contact the registry based on the TLS settings.
+func BuildTransportAndSchemeFromTLSConfig(tlsCfg portainer.TLSConfiguration) (*http.Transport, string, error) { //nolint:forbidigo
+	baseTransport := &http.Transport{
+		Proxy: http.ProxyFromEnvironment,
+	}
+
+	if !tlsCfg.TLS {
+		return baseTransport, "http", nil
+	}
+
+	// If TLS is enabled but uses trusted system CA (no custom bundle) and verification isn't skipped,
+	// we can use the default transport TLS settings.
+	usesTrustedSystemCA := !tlsCfg.TLSSkipVerify && tlsCfg.TLSCACertPath == "" && tlsCfg.TLSCertPath == "" && tlsCfg.TLSKeyPath == ""
+	if usesTrustedSystemCA {
+		return baseTransport, "https", nil
+	}
+
+	// Otherwise, build a custom TLS config from disk (covers skip-verify and/or custom bundle)
+	tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(tlsCfg)
+	if err != nil {
+		return nil, "", err
+	}
+	baseTransport.TLSClientConfig = tlsConfig
+
+	return baseTransport, "https", nil
+}