diff --git a/api/http/handler/registries/registry_configure.go b/api/http/handler/registries/registry_configure.go index 4b1a1ea0f..15e479241 100644 --- a/api/http/handler/registries/registry_configure.go +++ b/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 diff --git a/api/http/handler/registries/registry_configure_test.go b/api/http/handler/registries/registry_configure_test.go new file mode 100644 index 000000000..945a5f404 --- /dev/null +++ b/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 := ®istryConfigurePayload{} + 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 := ®istryConfigurePayload{} + 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 := ®istryConfigurePayload{} + if err := p.Validate(req); err == nil { + t.Fatalf("expected validation to fail on partial cert upload") + } + }) + } +} diff --git a/api/http/handler/registries/registry_create.go b/api/http/handler/registries/registry_create.go index 48b5d32f5..1f4bd6368 100644 --- a/api/http/handler/registries/registry_create.go +++ b/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 { diff --git a/app/portainer/models/registry.js b/app/portainer/models/registry.js index 6a7d81dcd..aba2fcad4 100644 --- a/app/portainer/models/registry.js +++ b/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; diff --git a/pkg/registryhttp/client.go b/pkg/registryhttp/client.go index fb95e6654..d1d39b24c 100644 --- a/pkg/registryhttp/client.go +++ b/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 } } diff --git a/pkg/registryhttp/client_test.go b/pkg/registryhttp/client_test.go index 41cfe6f60..53bb1a20d 100644 --- a/pkg/registryhttp/client_test.go +++ b/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) +} diff --git a/pkg/registryhttp/transport.go b/pkg/registryhttp/transport.go new file mode 100644 index 000000000..8fca9e007 --- /dev/null +++ b/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 +}