From 163aa57e5c28fcdbfd63915ca198fa6c7ec43c87 Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Fri, 1 Aug 2025 22:23:59 -0300 Subject: [PATCH] fix(tls): centralize the TLS configuration to ensure FIPS compliance BE-11979 (#960) --- .golangci.yaml | 5 ++ api/agent/version.go | 2 +- api/crypto/tls.go | 75 ++++++++++++++---- api/crypto/tls_test.go | 79 +++++++++++++++++++ api/docker/client/client.go | 3 +- api/docker/client/client_test.go | 26 ++++++ api/http/client/client.go | 15 +++- api/http/client/client_test.go | 31 ++++++++ api/http/handler/endpoints/endpoint_create.go | 12 +-- api/http/handler/websocket/initdial.go | 16 ++-- api/http/handler/websocket/initdial_test.go | 64 +++++++++++++++ api/http/proxy/factory/agent.go | 2 +- api/http/proxy/factory/docker.go | 2 +- api/http/proxy/factory/kubernetes.go | 13 ++- .../factory/kubernetes/agent_transport.go | 11 ++- .../factory/kubernetes/local_transport.go | 2 +- .../kubernetes/local_transport_test.go | 13 +++ api/internal/endpointutils/endpoint_setup.go | 10 +-- .../endpointutils/endpoint_setup_test.go | 12 +++ api/internal/snapshot/snapshot.go | 12 +-- api/ldap/ldap.go | 61 ++++++++------ api/ldap/ldap_test.go | 72 +++++++++++++++++ api/stacks/deployments/deploy.go | 11 +-- api/stacks/deployments/deploy_test.go | 6 +- pkg/networking/diagnostics.go | 11 ++- 25 files changed, 454 insertions(+), 112 deletions(-) create mode 100644 api/crypto/tls_test.go create mode 100644 api/docker/client/client_test.go create mode 100644 api/http/client/client_test.go create mode 100644 api/http/handler/websocket/initdial_test.go create mode 100644 api/http/proxy/factory/kubernetes/local_transport_test.go create mode 100644 api/internal/endpointutils/endpoint_setup_test.go create mode 100644 api/ldap/ldap_test.go diff --git a/.golangci.yaml b/.golangci.yaml index ea958e528..c3a176c27 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -14,8 +14,13 @@ linters: - perfsprint - ineffassign - bodyclose + - forbidigo linters-settings: + forbidigo: + forbid: + - p: ^tls\.Config$ + msg: 'Use crypto.CreateTLSConfiguration() instead' depguard: rules: main: diff --git a/api/agent/version.go b/api/agent/version.go index a9480850a..7e6885351 100644 --- a/api/agent/version.go +++ b/api/agent/version.go @@ -16,7 +16,7 @@ import ( // GetAgentVersionAndPlatform returns the agent version and platform // // it sends a ping to the agent and parses the version and platform from the headers -func GetAgentVersionAndPlatform(endpointUrl string, tlsConfig *tls.Config) (portainer.AgentPlatform, string, error) { +func GetAgentVersionAndPlatform(endpointUrl string, tlsConfig *tls.Config) (portainer.AgentPlatform, string, error) { //nolint:forbidigo httpCli := &http.Client{ Timeout: 3 * time.Second, } diff --git a/api/crypto/tls.go b/api/crypto/tls.go index 54e4cd43a..d06108ae4 100644 --- a/api/crypto/tls.go +++ b/api/crypto/tls.go @@ -1,14 +1,36 @@ package crypto import ( + "crypto/fips140" "crypto/tls" "crypto/x509" "os" + + portainer "github.com/portainer/portainer/api" ) // CreateTLSConfiguration creates a basic tls.Config with recommended TLS settings -func CreateTLSConfiguration() *tls.Config { - return &tls.Config{ +func CreateTLSConfiguration() *tls.Config { //nolint:forbidigo + // TODO: use fips.FIPSMode() instead + return createTLSConfiguration(fips140.Enabled()) +} + +func createTLSConfiguration(fipsEnabled bool) *tls.Config { //nolint:forbidigo + if fipsEnabled { + return &tls.Config{ //nolint:forbidigo + MinVersion: tls.VersionTLS12, + MaxVersion: tls.VersionTLS13, + CipherSuites: []uint16{ + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + }, + CurvePreferences: []tls.CurveID{tls.CurveP256, tls.CurveP384, tls.CurveP521}, + } + } + + return &tls.Config{ //nolint:forbidigo MinVersion: tls.VersionTLS12, CipherSuites: []uint16{ tls.TLS_AES_128_GCM_SHA256, @@ -34,19 +56,29 @@ func CreateTLSConfiguration() *tls.Config { // CreateTLSConfigurationFromBytes initializes a tls.Config using a CA certificate, a certificate and a key // loaded from memory. -func CreateTLSConfigurationFromBytes(caCert, cert, key []byte, skipClientVerification, skipServerVerification bool) (*tls.Config, error) { - config := CreateTLSConfiguration() - config.InsecureSkipVerify = skipServerVerification +func CreateTLSConfigurationFromBytes(useTLS bool, caCert, cert, key []byte, skipClientVerification, skipServerVerification bool) (*tls.Config, error) { //nolint:forbidigo + // TODO: use fips.FIPSMode() instead + return createTLSConfigurationFromBytes(fips140.Enabled(), useTLS, caCert, cert, key, skipClientVerification, skipServerVerification) +} - if !skipClientVerification { +func createTLSConfigurationFromBytes(fipsEnabled, useTLS bool, caCert, cert, key []byte, skipClientVerification, skipServerVerification bool) (*tls.Config, error) { //nolint:forbidigo + if !useTLS { + return nil, nil + } + + config := CreateTLSConfiguration() + config.InsecureSkipVerify = skipServerVerification && !fipsEnabled + + if !skipClientVerification || fipsEnabled { certificate, err := tls.X509KeyPair(cert, key) if err != nil { return nil, err } + config.Certificates = []tls.Certificate{certificate} } - if !skipServerVerification { + if !skipServerVerification || fipsEnabled { caCertPool := x509.NewCertPool() caCertPool.AppendCertsFromPEM(caCert) config.RootCAs = caCertPool @@ -57,29 +89,38 @@ func CreateTLSConfigurationFromBytes(caCert, cert, key []byte, skipClientVerific // CreateTLSConfigurationFromDisk initializes a tls.Config using a CA certificate, a certificate and a key // loaded from disk. -func CreateTLSConfigurationFromDisk(caCertPath, certPath, keyPath string, skipServerVerification bool) (*tls.Config, error) { - config := CreateTLSConfiguration() - config.InsecureSkipVerify = skipServerVerification +func CreateTLSConfigurationFromDisk(config portainer.TLSConfiguration) (*tls.Config, error) { //nolint:forbidigo + // TODO: use fips.FIPSMode() instead + return createTLSConfigurationFromDisk(fips140.Enabled(), config) +} - if certPath != "" && keyPath != "" { - cert, err := tls.LoadX509KeyPair(certPath, keyPath) +func createTLSConfigurationFromDisk(fipsEnabled bool, config portainer.TLSConfiguration) (*tls.Config, error) { //nolint:forbidigo + if !config.TLS { + return nil, nil + } + + tlsConfig := CreateTLSConfiguration() + tlsConfig.InsecureSkipVerify = config.TLSSkipVerify && !fipsEnabled + + if config.TLSCertPath != "" && config.TLSKeyPath != "" { + cert, err := tls.LoadX509KeyPair(config.TLSCertPath, config.TLSKeyPath) if err != nil { return nil, err } - config.Certificates = []tls.Certificate{cert} + tlsConfig.Certificates = []tls.Certificate{cert} } - if !skipServerVerification && caCertPath != "" { - caCert, err := os.ReadFile(caCertPath) + if !tlsConfig.InsecureSkipVerify && config.TLSCACertPath != "" { + caCert, err := os.ReadFile(config.TLSCACertPath) if err != nil { return nil, err } caCertPool := x509.NewCertPool() caCertPool.AppendCertsFromPEM(caCert) - config.RootCAs = caCertPool + tlsConfig.RootCAs = caCertPool } - return config, nil + return tlsConfig, nil } diff --git a/api/crypto/tls_test.go b/api/crypto/tls_test.go new file mode 100644 index 000000000..551397fce --- /dev/null +++ b/api/crypto/tls_test.go @@ -0,0 +1,79 @@ +package crypto + +import ( + "crypto/tls" + "testing" + + portainer "github.com/portainer/portainer/api" + + "github.com/stretchr/testify/require" +) + +func TestCreateTLSConfiguration(t *testing.T) { + config := CreateTLSConfiguration() + require.Equal(t, config.MinVersion, uint16(tls.VersionTLS12)) +} + +func TestCreateTLSConfigurationFIPS(t *testing.T) { + fips := true + + fipsCipherSuites := []uint16{ + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + } + + fipsCurvePreferences := []tls.CurveID{tls.CurveP256, tls.CurveP384, tls.CurveP521} + + config := createTLSConfiguration(fips) + require.Equal(t, config.MinVersion, uint16(tls.VersionTLS12)) + require.Equal(t, config.MaxVersion, uint16(tls.VersionTLS13)) + require.Equal(t, config.CipherSuites, fipsCipherSuites) + require.Equal(t, config.CurvePreferences, fipsCurvePreferences) +} + +func TestCreateTLSConfigurationFromBytes(t *testing.T) { + // No TLS + config, err := CreateTLSConfigurationFromBytes(false, nil, nil, nil, false, false) + require.Nil(t, err) + require.Nil(t, config) + + // Skip TLS client/server verifications + config, err = CreateTLSConfigurationFromBytes(true, nil, nil, nil, true, true) + require.NoError(t, err) + require.NotNil(t, config) + + // Empty TLS + config, err = CreateTLSConfigurationFromBytes(true, nil, nil, nil, false, false) + require.Error(t, err) + require.Nil(t, config) +} + +func TestCreateTLSConfigurationFromDisk(t *testing.T) { + // No TLS + config, err := CreateTLSConfigurationFromDisk(portainer.TLSConfiguration{}) + require.Nil(t, err) + require.Nil(t, config) + + // Skip TLS verifications + config, err = CreateTLSConfigurationFromDisk(portainer.TLSConfiguration{ + TLS: true, + TLSSkipVerify: true, + }) + require.NoError(t, err) + require.NotNil(t, config) +} + +func TestCreateTLSConfigurationFromDiskFIPS(t *testing.T) { + fips := true + + // Skipping TLS verifications cannot be done in FIPS mode + config, err := createTLSConfigurationFromDisk(fips, portainer.TLSConfiguration{ + TLS: true, + TLSSkipVerify: true, + }) + require.NoError(t, err) + require.NotNil(t, config) + require.False(t, config.InsecureSkipVerify) +} diff --git a/api/docker/client/client.go b/api/docker/client/client.go index d1a2a394a..9648656a0 100644 --- a/api/docker/client/client.go +++ b/api/docker/client/client.go @@ -181,10 +181,11 @@ func httpClient(endpoint *portainer.Endpoint, timeout *time.Duration) (*http.Cli } if endpoint.TLSConfig.TLS { - tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig.TLSCACertPath, endpoint.TLSConfig.TLSCertPath, endpoint.TLSConfig.TLSKeyPath, endpoint.TLSConfig.TLSSkipVerify) + tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig) if err != nil { return nil, err } + transport.TLSClientConfig = tlsConfig } diff --git a/api/docker/client/client_test.go b/api/docker/client/client_test.go new file mode 100644 index 000000000..4d7f767e0 --- /dev/null +++ b/api/docker/client/client_test.go @@ -0,0 +1,26 @@ +package client + +import ( + "testing" + + portainer "github.com/portainer/portainer/api" + "github.com/stretchr/testify/require" +) + +func TestHttpClient(t *testing.T) { + // Valid TLS configuration + endpoint := &portainer.Endpoint{} + endpoint.TLSConfig = portainer.TLSConfiguration{TLS: true} + + cli, err := httpClient(endpoint, nil) + require.NoError(t, err) + require.NotNil(t, cli) + + // Invalid TLS configuration + endpoint.TLSConfig.TLSCertPath = "/invalid/path/client.crt" + endpoint.TLSConfig.TLSKeyPath = "/invalid/path/client.key" + + cli, err = httpClient(endpoint, nil) + require.Error(t, err) + require.Nil(t, cli) +} diff --git a/api/http/client/client.go b/api/http/client/client.go index 2a7d0bf9b..ba253162f 100644 --- a/api/http/client/client.go +++ b/api/http/client/client.go @@ -1,7 +1,6 @@ package client import ( - "crypto/tls" "errors" "fmt" "io" @@ -11,6 +10,7 @@ import ( "time" portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/crypto" "github.com/rs/zerolog/log" "github.com/segmentio/encoding/json" @@ -105,21 +105,28 @@ func Get(url string, timeout int) ([]byte, error) { // ExecutePingOperation will send a SystemPing operation HTTP request to a Docker environment(endpoint) // using the specified host and optional TLS configuration. // It uses a new Http.Client for each operation. -func ExecutePingOperation(host string, tlsConfig *tls.Config) (bool, error) { +func ExecutePingOperation(host string, tlsConfiguration portainer.TLSConfiguration) (bool, error) { transport := &http.Transport{} scheme := "http" - if tlsConfig != nil { + + if tlsConfiguration.TLS { + tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(tlsConfiguration) + if err != nil { + return false, err + } + transport.TLSClientConfig = tlsConfig scheme = "https" } client := &http.Client{ - Timeout: time.Second * 3, + Timeout: 3 * time.Second, Transport: transport, } target := strings.Replace(host, "tcp://", scheme+"://", 1) + return pingOperation(client, target) } diff --git a/api/http/client/client_test.go b/api/http/client/client_test.go new file mode 100644 index 000000000..d7ffe99ff --- /dev/null +++ b/api/http/client/client_test.go @@ -0,0 +1,31 @@ +package client + +import ( + "testing" + + portainer "github.com/portainer/portainer/api" + + "github.com/stretchr/testify/require" +) + +func TestExecutePingOperationFailure(t *testing.T) { + host := "http://localhost:1" + config := portainer.TLSConfiguration{ + TLS: true, + TLSSkipVerify: true, + } + + // Invalid host + ok, err := ExecutePingOperation(host, config) + require.False(t, ok) + require.Error(t, err) + + // Invalid TLS configuration + config.TLSCertPath = "/invalid/path/to/cert" + config.TLSKeyPath = "/invalid/path/to/key" + + ok, err = ExecutePingOperation(host, config) + require.False(t, ok) + require.Error(t, err) + +} diff --git a/api/http/handler/endpoints/endpoint_create.go b/api/http/handler/endpoints/endpoint_create.go index 3cfe934bc..03f2bee44 100644 --- a/api/http/handler/endpoints/endpoint_create.go +++ b/api/http/handler/endpoints/endpoint_create.go @@ -1,7 +1,6 @@ package endpoints import ( - "crypto/tls" "errors" "net/http" "runtime" @@ -285,8 +284,6 @@ func (handler *Handler) endpointCreate(w http.ResponseWriter, r *http.Request) * } func (handler *Handler) createEndpoint(tx dataservices.DataStoreTx, payload *endpointCreatePayload) (*portainer.Endpoint, *httperror.HandlerError) { - var err error - switch payload.EndpointCreationType { case azureEnvironment: return handler.createAzureEndpoint(tx, payload) @@ -301,12 +298,9 @@ func (handler *Handler) createEndpoint(tx dataservices.DataStoreTx, payload *end endpointType := portainer.DockerEnvironment var agentVersion string if payload.EndpointCreationType == agentEnvironment { - var tlsConfig *tls.Config - if payload.TLS { - tlsConfig, err = crypto.CreateTLSConfigurationFromBytes(payload.TLSCACertFile, payload.TLSCertFile, payload.TLSKeyFile, payload.TLSSkipVerify, payload.TLSSkipClientVerify) - if err != nil { - return nil, httperror.InternalServerError("Unable to create TLS configuration", err) - } + tlsConfig, err := crypto.CreateTLSConfigurationFromBytes(payload.TLS, payload.TLSCACertFile, payload.TLSCertFile, payload.TLSKeyFile, payload.TLSSkipVerify, payload.TLSSkipClientVerify) + if err != nil { + return nil, httperror.InternalServerError("Unable to create TLS configuration", err) } agentPlatform, version, err := agent.GetAgentVersionAndPlatform(payload.URL, tlsConfig) diff --git a/api/http/handler/websocket/initdial.go b/api/http/handler/websocket/initdial.go index e9160ccf1..1be0e496f 100644 --- a/api/http/handler/websocket/initdial.go +++ b/api/http/handler/websocket/initdial.go @@ -21,16 +21,14 @@ func initDial(endpoint *portainer.Endpoint) (net.Conn, error) { host = url.Path } - if endpoint.TLSConfig.TLS { - tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig.TLSCACertPath, endpoint.TLSConfig.TLSCertPath, endpoint.TLSConfig.TLSKeyPath, endpoint.TLSConfig.TLSSkipVerify) - if err != nil { - return nil, err - } - - return tls.Dial(url.Scheme, host, tlsConfig) + if !endpoint.TLSConfig.TLS { + return createDial(url.Scheme, host) } - con, err := createDial(url.Scheme, host) + tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig) + if err != nil { + return nil, err + } - return con, err + return tls.Dial(url.Scheme, host, tlsConfig) } diff --git a/api/http/handler/websocket/initdial_test.go b/api/http/handler/websocket/initdial_test.go new file mode 100644 index 000000000..3179389f0 --- /dev/null +++ b/api/http/handler/websocket/initdial_test.go @@ -0,0 +1,64 @@ +package websocket + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" + + portainer "github.com/portainer/portainer/api" + + "github.com/stretchr/testify/require" +) + +func TestInitDial(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + tlsSrv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer tlsSrv.Close() + + f := func(srvURL string) { + u, err := url.Parse(srvURL) + require.NoError(t, err) + + isTLS := u.Scheme == "https" + + u.Scheme = "tcp" + + endpoint := &portainer.Endpoint{ + URL: u.String(), + TLSConfig: portainer.TLSConfiguration{ + TLS: isTLS, + TLSSkipVerify: true, + }, + } + + // Valid configuration + conn, err := initDial(endpoint) + require.NoError(t, err) + require.NotNil(t, conn) + + err = conn.Close() + require.NoError(t, err) + + if !isTLS { + return + } + + // Invalid TLS configuration + endpoint.TLSConfig.TLSCertPath = "/invalid/path/client.crt" + endpoint.TLSConfig.TLSKeyPath = "/invalid/path/client.key" + + conn, err = initDial(endpoint) + require.Error(t, err) + require.Nil(t, conn) + } + + f(srv.URL) + f(tlsSrv.URL) +} diff --git a/api/http/proxy/factory/agent.go b/api/http/proxy/factory/agent.go index 60323a195..ca74d2228 100644 --- a/api/http/proxy/factory/agent.go +++ b/api/http/proxy/factory/agent.go @@ -43,7 +43,7 @@ func (factory *ProxyFactory) NewAgentProxy(endpoint *portainer.Endpoint) (*Proxy httpTransport := &http.Transport{} if endpoint.TLSConfig.TLS || endpoint.TLSConfig.TLSSkipVerify { - config, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig.TLSCACertPath, endpoint.TLSConfig.TLSCertPath, endpoint.TLSConfig.TLSKeyPath, endpoint.TLSConfig.TLSSkipVerify) + config, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig) if err != nil { return nil, errors.WithMessage(err, "failed generating tls configuration") } diff --git a/api/http/proxy/factory/docker.go b/api/http/proxy/factory/docker.go index 31d183c9b..d832b3678 100644 --- a/api/http/proxy/factory/docker.go +++ b/api/http/proxy/factory/docker.go @@ -50,7 +50,7 @@ func (factory *ProxyFactory) newDockerHTTPProxy(endpoint *portainer.Endpoint) (h httpTransport := &http.Transport{} if endpoint.TLSConfig.TLS || endpoint.TLSConfig.TLSSkipVerify { - config, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig.TLSCACertPath, endpoint.TLSConfig.TLSCertPath, endpoint.TLSConfig.TLSKeyPath, endpoint.TLSConfig.TLSSkipVerify) + config, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig) if err != nil { return nil, err } diff --git a/api/http/proxy/factory/kubernetes.go b/api/http/proxy/factory/kubernetes.go index ea08467e5..43ed7d5b3 100644 --- a/api/http/proxy/factory/kubernetes.go +++ b/api/http/proxy/factory/kubernetes.go @@ -7,7 +7,6 @@ import ( "github.com/portainer/portainer/api/http/proxy/factory/kubernetes" portainer "github.com/portainer/portainer/api" - "github.com/portainer/portainer/api/crypto" ) func (factory *ProxyFactory) newKubernetesProxy(endpoint *portainer.Endpoint) (http.Handler, error) { @@ -93,19 +92,19 @@ func (factory *ProxyFactory) newKubernetesAgentHTTPSProxy(endpoint *portainer.En return nil, err } - tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig.TLSCACertPath, endpoint.TLSConfig.TLSCertPath, endpoint.TLSConfig.TLSKeyPath, endpoint.TLSConfig.TLSSkipVerify) - if err != nil { - return nil, err - } - tokenCache := factory.kubernetesTokenCacheManager.GetOrCreateTokenCache(endpoint.ID) tokenManager, err := kubernetes.NewTokenManager(kubecli, factory.dataStore, tokenCache, false) if err != nil { return nil, err } + transport, err := kubernetes.NewAgentTransport(factory.signatureService, tokenManager, endpoint, factory.kubernetesClientFactory, factory.dataStore, factory.jwtService) + if err != nil { + return nil, err + } + proxy := NewSingleHostReverseProxyWithHostHeader(remoteURL) - proxy.Transport = kubernetes.NewAgentTransport(factory.signatureService, tlsConfig, tokenManager, endpoint, factory.kubernetesClientFactory, factory.dataStore, factory.jwtService) + proxy.Transport = transport return proxy, nil } diff --git a/api/http/proxy/factory/kubernetes/agent_transport.go b/api/http/proxy/factory/kubernetes/agent_transport.go index b127b85fd..4a62e2367 100644 --- a/api/http/proxy/factory/kubernetes/agent_transport.go +++ b/api/http/proxy/factory/kubernetes/agent_transport.go @@ -1,11 +1,11 @@ package kubernetes import ( - "crypto/tls" "net/http" "strings" portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/crypto" "github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/kubernetes/cli" ) @@ -16,7 +16,12 @@ type agentTransport struct { } // NewAgentTransport returns a new transport that can be used to send signed requests to a Portainer agent -func NewAgentTransport(signatureService portainer.DigitalSignatureService, tlsConfig *tls.Config, tokenManager *tokenManager, endpoint *portainer.Endpoint, k8sClientFactory *cli.ClientFactory, dataStore dataservices.DataStore, jwtService portainer.JWTService) *agentTransport { +func NewAgentTransport(signatureService portainer.DigitalSignatureService, tokenManager *tokenManager, endpoint *portainer.Endpoint, k8sClientFactory *cli.ClientFactory, dataStore dataservices.DataStore, jwtService portainer.JWTService) (*agentTransport, error) { + tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig) + if err != nil { + return nil, err + } + transport := &agentTransport{ baseTransport: newBaseTransport( &http.Transport{ @@ -31,7 +36,7 @@ func NewAgentTransport(signatureService portainer.DigitalSignatureService, tlsCo signatureService: signatureService, } - return transport + return transport, nil } // RoundTrip is the implementation of the the http.RoundTripper interface diff --git a/api/http/proxy/factory/kubernetes/local_transport.go b/api/http/proxy/factory/kubernetes/local_transport.go index 6fe255ff6..bc832f35c 100644 --- a/api/http/proxy/factory/kubernetes/local_transport.go +++ b/api/http/proxy/factory/kubernetes/local_transport.go @@ -15,7 +15,7 @@ type localTransport struct { // NewLocalTransport returns a new transport that can be used to send requests to the local Kubernetes API func NewLocalTransport(tokenManager *tokenManager, endpoint *portainer.Endpoint, k8sClientFactory *cli.ClientFactory, dataStore dataservices.DataStore, jwtService portainer.JWTService) (*localTransport, error) { - config, err := crypto.CreateTLSConfigurationFromBytes(nil, nil, nil, true, true) + config, err := crypto.CreateTLSConfigurationFromBytes(true, nil, nil, nil, true, true) if err != nil { return nil, err } diff --git a/api/http/proxy/factory/kubernetes/local_transport_test.go b/api/http/proxy/factory/kubernetes/local_transport_test.go new file mode 100644 index 000000000..a22b1e1c3 --- /dev/null +++ b/api/http/proxy/factory/kubernetes/local_transport_test.go @@ -0,0 +1,13 @@ +package kubernetes + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNewLocalTransport(t *testing.T) { + transport, err := NewLocalTransport(nil, nil, nil, nil, nil) + require.NoError(t, err) + require.True(t, transport.baseTransport.httpTransport.TLSClientConfig.InsecureSkipVerify) +} diff --git a/api/internal/endpointutils/endpoint_setup.go b/api/internal/endpointutils/endpoint_setup.go index 3242d8875..0f6a7ca7f 100644 --- a/api/internal/endpointutils/endpoint_setup.go +++ b/api/internal/endpointutils/endpoint_setup.go @@ -5,7 +5,6 @@ import ( "strings" portainer "github.com/portainer/portainer/api" - "github.com/portainer/portainer/api/crypto" "github.com/portainer/portainer/api/dataservices" "github.com/portainer/portainer/api/http/client" @@ -108,12 +107,7 @@ func createTLSSecuredEndpoint(flags *portainer.CLIFlags, dataStore dataservices. } if strings.HasPrefix(endpoint.URL, "tcp://") { - tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(tlsConfiguration.TLSCACertPath, tlsConfiguration.TLSCertPath, tlsConfiguration.TLSKeyPath, tlsConfiguration.TLSSkipVerify) - if err != nil { - return err - } - - agentOnDockerEnvironment, err := client.ExecutePingOperation(endpoint.URL, tlsConfig) + agentOnDockerEnvironment, err := client.ExecutePingOperation(endpoint.URL, tlsConfiguration) if err != nil { return err } @@ -136,7 +130,7 @@ func createTLSSecuredEndpoint(flags *portainer.CLIFlags, dataStore dataservices. func createUnsecuredEndpoint(endpointURL string, dataStore dataservices.DataStore, snapshotService portainer.SnapshotService) error { if strings.HasPrefix(endpointURL, "tcp://") { - if _, err := client.ExecutePingOperation(endpointURL, nil); err != nil { + if _, err := client.ExecutePingOperation(endpointURL, portainer.TLSConfiguration{}); err != nil { return err } } diff --git a/api/internal/endpointutils/endpoint_setup_test.go b/api/internal/endpointutils/endpoint_setup_test.go new file mode 100644 index 000000000..64be9e254 --- /dev/null +++ b/api/internal/endpointutils/endpoint_setup_test.go @@ -0,0 +1,12 @@ +package endpointutils + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCreateOfflineUnsecuredEndpoint(t *testing.T) { + err := createUnsecuredEndpoint("tcp://localhost:1", nil, nil) + require.Error(t, err) +} diff --git a/api/internal/snapshot/snapshot.go b/api/internal/snapshot/snapshot.go index 7bc109459..205a82216 100644 --- a/api/internal/snapshot/snapshot.go +++ b/api/internal/snapshot/snapshot.go @@ -2,7 +2,6 @@ package snapshot import ( "context" - "crypto/tls" "errors" "time" @@ -138,14 +137,9 @@ func SupportDirectSnapshot(endpoint *portainer.Endpoint) bool { // If the snapshot is a success, it will be associated to the environment(endpoint). func (service *Service) SnapshotEndpoint(endpoint *portainer.Endpoint) error { if endpoint.Type == portainer.AgentOnDockerEnvironment || endpoint.Type == portainer.AgentOnKubernetesEnvironment { - var err error - var tlsConfig *tls.Config - - if endpoint.TLSConfig.TLS { - tlsConfig, err = crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig.TLSCACertPath, endpoint.TLSConfig.TLSCertPath, endpoint.TLSConfig.TLSKeyPath, endpoint.TLSConfig.TLSSkipVerify) - if err != nil { - return err - } + tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig) + if err != nil { + return err } _, version, err := agent.GetAgentVersionAndPlatform(endpoint.URL, tlsConfig) diff --git a/api/ldap/ldap.go b/api/ldap/ldap.go index 09e8e6450..5037666b9 100644 --- a/api/ldap/ldap.go +++ b/api/ldap/ldap.go @@ -4,11 +4,12 @@ import ( "fmt" "strings" - ldap "github.com/go-ldap/ldap/v3" - "github.com/pkg/errors" portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/crypto" httperrors "github.com/portainer/portainer/api/http/errors" + + ldap "github.com/go-ldap/ldap/v3" + "github.com/pkg/errors" ) var ( @@ -30,36 +31,44 @@ func createConnection(settings *portainer.LDAPSettings) (*ldap.Conn, error) { } func createConnectionForURL(url string, settings *portainer.LDAPSettings) (*ldap.Conn, error) { - if settings.TLSConfig.TLS || settings.StartTLS { - config, err := crypto.CreateTLSConfigurationFromDisk(settings.TLSConfig.TLSCACertPath, settings.TLSConfig.TLSCertPath, settings.TLSConfig.TLSKeyPath, settings.TLSConfig.TLSSkipVerify) - if err != nil { - return nil, err - } - config.ServerName = strings.Split(url, ":")[0] - - if settings.TLSConfig.TLS { - return ldap.DialTLS("tcp", url, config) - } - - conn, err := ldap.Dial("tcp", url) - if err != nil { - return nil, err - } - - err = conn.StartTLS(config) - if err != nil { - return nil, err - } - - return conn, nil + if !settings.TLSConfig.TLS && !settings.StartTLS { + return ldap.Dial("tcp", url) } - return ldap.Dial("tcp", url) + // Store the original value to ensure the TLSConfig is created + t := settings.TLSConfig.TLS + settings.TLSConfig.TLS = settings.TLSConfig.TLS || settings.StartTLS + + config, err := crypto.CreateTLSConfigurationFromDisk(settings.TLSConfig) + if err != nil { + return nil, err + } + + // Restore the original value + settings.TLSConfig.TLS = t + + if settings.TLSConfig.TLS || settings.StartTLS { + config.ServerName = strings.Split(url, ":")[0] + } + + if settings.TLSConfig.TLS { + return ldap.DialTLS("tcp", url, config) + } + + conn, err := ldap.Dial("tcp", url) + if err != nil { + return nil, err + } + + if err := conn.StartTLS(config); err != nil { + return nil, err + } + + return conn, nil } // AuthenticateUser is used to authenticate a user against a LDAP/AD. func (*Service) AuthenticateUser(username, password string, settings *portainer.LDAPSettings) error { - connection, err := createConnection(settings) if err != nil { return err diff --git a/api/ldap/ldap_test.go b/api/ldap/ldap_test.go new file mode 100644 index 000000000..1d16d2d0e --- /dev/null +++ b/api/ldap/ldap_test.go @@ -0,0 +1,72 @@ +package ldap + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" + + portainer "github.com/portainer/portainer/api" + + "github.com/stretchr/testify/require" +) + +func TestCreateConnectionForURL(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + tlsSrv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer tlsSrv.Close() + + srvURL, err := url.Parse(tlsSrv.URL) + require.NoError(t, err) + + // TCP + + settings := &portainer.LDAPSettings{ + URL: srvURL.Host, + } + + conn, err := createConnectionForURL(settings.URL, settings) + require.NoError(t, err) + require.NotNil(t, conn) + conn.Close() + + // TLS + + settings.TLSConfig = portainer.TLSConfiguration{ + TLS: true, + TLSSkipVerify: true, + } + + conn, err = createConnectionForURL(settings.URL, settings) + require.NoError(t, err) + require.NotNil(t, conn) + conn.Close() + + // Invalid TLS + + settings.TLSConfig = portainer.TLSConfiguration{ + TLS: true, + TLSSkipVerify: true, + TLSCertPath: "/invalid/path/cert", + TLSKeyPath: "/invalid/path/key", + } + + conn, err = createConnectionForURL(settings.URL, settings) + require.Error(t, err) + require.Nil(t, conn) + + // StartTLS + + settings.TLSConfig.TLS = false + settings.StartTLS = true + + conn, err = createConnectionForURL(settings.URL, settings) + require.Error(t, err) + require.Nil(t, conn) +} diff --git a/api/stacks/deployments/deploy.go b/api/stacks/deployments/deploy.go index d963e8686..627c3012a 100644 --- a/api/stacks/deployments/deploy.go +++ b/api/stacks/deployments/deploy.go @@ -2,7 +2,6 @@ package deployments import ( "cmp" - "crypto/tls" "fmt" "strconv" "time" @@ -215,13 +214,9 @@ func isEnvironmentOnline(endpoint *portainer.Endpoint) bool { return true } - var err error - var tlsConfig *tls.Config - if endpoint.TLSConfig.TLS { - tlsConfig, err = crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig.TLSCACertPath, endpoint.TLSConfig.TLSCertPath, endpoint.TLSConfig.TLSKeyPath, endpoint.TLSConfig.TLSSkipVerify) - if err != nil { - return false - } + tlsConfig, err := crypto.CreateTLSConfigurationFromDisk(endpoint.TLSConfig) + if err != nil { + return false } _, _, err = agent.GetAgentVersionAndPlatform(endpoint.URL, tlsConfig) diff --git a/api/stacks/deployments/deploy_test.go b/api/stacks/deployments/deploy_test.go index d45313cb9..9d13f22a5 100644 --- a/api/stacks/deployments/deploy_test.go +++ b/api/stacks/deployments/deploy_test.go @@ -10,6 +10,7 @@ import ( "testing" portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/crypto" "github.com/portainer/portainer/api/datastore" gittypes "github.com/portainer/portainer/api/git/types" "github.com/portainer/portainer/api/internal/testhelpers" @@ -127,9 +128,8 @@ func agentServer(t *testing.T) string { cert, err := tls.X509KeyPair([]byte(localhostCert), []byte(localhostKey)) require.NoError(t, err) - tlsConfig := &tls.Config{ - Certificates: []tls.Certificate{cert}, - } + tlsConfig := crypto.CreateTLSConfiguration() + tlsConfig.Certificates = []tls.Certificate{cert} l, err := tls.Listen("tcp", "127.0.0.1:0", tlsConfig) require.NoError(t, err) diff --git a/pkg/networking/diagnostics.go b/pkg/networking/diagnostics.go index 2c0e19f1d..bf1b05f6b 100644 --- a/pkg/networking/diagnostics.go +++ b/pkg/networking/diagnostics.go @@ -1,7 +1,7 @@ package networking import ( - "crypto/tls" + "crypto/fips140" "fmt" "net" "net/http" @@ -9,6 +9,8 @@ import ( "strings" "time" + "github.com/portainer/portainer/api/crypto" + "github.com/segmentio/encoding/json" ) @@ -71,13 +73,14 @@ func ProbeTelnetConnection(url string) string { func DetectProxy(url string) string { client := &http.Client{ Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: true, - }, + TLSClientConfig: crypto.CreateTLSConfiguration(), }, Timeout: 10 * time.Second, } + // TODO: use fips.CanTLSSkipVerify() instead + client.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify = !fips140.Enabled() + result := map[string]string{ "operation": "proxy detection", "local_address": "unknown",