From 1baa135c125738cf9b8bda480cf74cc27eaab199 Mon Sep 17 00:00:00 2001 From: Allain Legacy Date: Mon, 19 Aug 2024 16:54:52 -0400 Subject: [PATCH] CNF-13958: Enable TLS verification for in-cluster deployments This configures the TLS clients using the service account root CA bundles so that we can properly verify the identity of backend servers. Signed-off-by: Allain Legacy --- .../authentication/handler_wrapper_test.go | 4 -- internal/controllers/utils/constants.go | 6 +- internal/controllers/utils/utils.go | 64 ++++++++++++++++++- internal/service/alarm_fetcher.go | 13 ++-- internal/service/alarm_handler.go | 13 ++-- .../service/deployment_manager_handler.go | 15 ++--- internal/service/resource_fetcher.go | 13 ++-- internal/service/resource_handler.go | 16 ++--- internal/service/resource_pool_fetcher.go | 13 ++-- internal/service/resource_pool_handler.go | 14 ++-- internal/service/resource_type_handler.go | 14 ++-- internal/service/suite_test.go | 7 ++ 12 files changed, 119 insertions(+), 73 deletions(-) diff --git a/internal/authentication/handler_wrapper_test.go b/internal/authentication/handler_wrapper_test.go index 196c861d..3b04b76c 100644 --- a/internal/authentication/handler_wrapper_test.go +++ b/internal/authentication/handler_wrapper_test.go @@ -716,10 +716,6 @@ var _ = Describe("Handler wapper", func() { It("Doesn't load insecure keys by default", func() { var err error - // TODO(alegacy): Remove this override once it is fixed for production - err = os.Setenv(utils.TLSSkipVerifyEnvName, "false") - Expect(err).ToNot(HaveOccurred()) - // Prepare the server: server, ca := MakeTCPTLSServer() defer func() { diff --git a/internal/controllers/utils/constants.go b/internal/controllers/utils/constants.go index 50117aa4..e887d9d0 100644 --- a/internal/controllers/utils/constants.go +++ b/internal/controllers/utils/constants.go @@ -77,7 +77,9 @@ var ( // Default values for backend URL and token: const ( defaultBackendURL = "https://kubernetes.default.svc" - defaultBackendTokenFile = "/run/secrets/kubernetes.io/serviceaccount/token" // nolint: gosec // hardcoded path only + defaultBackendTokenFile = "/run/secrets/kubernetes.io/serviceaccount/token" // nolint: gosec // hardcoded path only + defaultBackendCABundle = "/run/secrets/kubernetes.io/serviceaccount/ca.crt" // nolint: gosec // hardcoded path only + defaultServiceCAFile = "/run/secrets/kubernetes.io/serviceaccount/service-ca.crt" // nolint: gosec // hardcoded path only ) // ClusterInstance template constants @@ -128,5 +130,5 @@ const ( // Environment variable names const ( TLSSkipVerifyEnvName = "INSECURE_SKIP_VERIFY" - TLSSkipVerifyDefaultValue = true // TODO(alegacy): need to set to false for production + TLSSkipVerifyDefaultValue = false ) diff --git a/internal/controllers/utils/utils.go b/internal/controllers/utils/utils.go index bc68b1bc..047d152f 100644 --- a/internal/controllers/utils/utils.go +++ b/internal/controllers/utils/utils.go @@ -3,7 +3,10 @@ package utils import ( "bytes" "context" + "crypto/tls" + "crypto/x509" "fmt" + "net/http" "os" "reflect" "slices" @@ -11,6 +14,9 @@ import ( "strings" "text/template" + "k8s.io/apimachinery/pkg/util/net" + ctrl "sigs.k8s.io/controller-runtime" + sprig "github.com/go-task/slim-sprig/v3" "github.com/xeipuuv/gojsonschema" @@ -23,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/retry" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/yaml" @@ -720,3 +725,60 @@ func GetTLSSkipVerify() bool { return result } + +// loadDefaultCABundles loads the default service account and ingress CA bundles. This should only be invoked if TLS +// verification has not been disabled since the expectation is that it will only need to be disabled when testing as a +// standalone binary in which case the paths to the bundles won't be present. Otherwise, we always expect the bundles +// to be present when running in-cluster. +func loadDefaultCABundles(config *tls.Config) error { + config.RootCAs = x509.NewCertPool() + if data, err := os.ReadFile(defaultBackendCABundle); err != nil { + // This should not happen unless the binary is being tested in standalone mode in which case the developer + // should have disabled the TLS verification which would prevent this function from being invoked. + return fmt.Errorf("failed to read CA bundle '%s': %w", defaultBackendCABundle, err) + // This should not happen, but if it does continue anyway + } else { + // This will enable accessing public facing API endpoints signed by the default ingress controller certificate + config.RootCAs.AppendCertsFromPEM(data) + } + + if data, err := os.ReadFile(defaultServiceCAFile); err != nil { + return fmt.Errorf("failed to read service CA file '%s': %w", defaultServiceCAFile, err) + } else { + // This will enable accessing internal services signed by the service account signer. + config.RootCAs.AppendCertsFromPEM(data) + } + + return nil +} + +// GetDefaultTLSConfig sets the TLS configuration attributes appropriately to enable communication between internal +// services and accessing the public facing API endpoints. +func GetDefaultTLSConfig(config *tls.Config) (*tls.Config, error) { + if config == nil { + config = &tls.Config{MinVersion: tls.VersionTLS12} + } + + // Allow developers to override the TLS verification + config.InsecureSkipVerify = GetTLSSkipVerify() + if !config.InsecureSkipVerify { + // TLS verification is enabled therefore we need to load the CA bundles that are injected into our filesystem + // automatically; which happens since we are defined as using a service-account + err := loadDefaultCABundles(config) + if err != nil { + return nil, fmt.Errorf("error loading default CABundles: %w", err) + } + } + + return config, nil +} + +// GetDefaultBackendTransport returns an HTTP transport with the proper TLS defaults set. +func GetDefaultBackendTransport() (http.RoundTripper, error) { + tlsConfig, err := GetDefaultTLSConfig(&tls.Config{MinVersion: tls.VersionTLS12}) + if err != nil { + return nil, err + } + + return net.SetTransportDefaults(&http.Transport{TLSClientConfig: tlsConfig}), nil +} diff --git a/internal/service/alarm_fetcher.go b/internal/service/alarm_fetcher.go index a58d4250..984de883 100644 --- a/internal/service/alarm_fetcher.go +++ b/internal/service/alarm_fetcher.go @@ -16,7 +16,6 @@ package service import ( "context" - "crypto/tls" "errors" "fmt" "log/slog" @@ -24,8 +23,6 @@ import ( neturl "net/url" "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" - "k8s.io/apimachinery/pkg/util/net" - "github.com/openshift-kni/oran-o2ims/internal/data" "github.com/openshift-kni/oran-o2ims/internal/jq" "github.com/openshift-kni/oran-o2ims/internal/k8s" @@ -154,11 +151,11 @@ func (b *AlarmFetcherBuilder) Build() ( } // Create the HTTP client that we will use to connect to the backend: - var backendTransport http.RoundTripper = net.SetTransportDefaults(&http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: utils.GetTLSSkipVerify(), // nolint: gosec // defaulted to false; logged if disabled - }, - }) + backendTransport, err := utils.GetDefaultBackendTransport() + if err != nil { + err = fmt.Errorf("failed to create default HTTP backend transport: %w", err) + return + } if b.transportWrapper != nil { backendTransport = b.transportWrapper(backendTransport) } diff --git a/internal/service/alarm_handler.go b/internal/service/alarm_handler.go index 795fb630..cecca18a 100644 --- a/internal/service/alarm_handler.go +++ b/internal/service/alarm_handler.go @@ -16,7 +16,6 @@ package service import ( "context" - "crypto/tls" "errors" "fmt" "log/slog" @@ -24,8 +23,6 @@ import ( "slices" "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" - "k8s.io/apimachinery/pkg/util/net" - "github.com/openshift-kni/oran-o2ims/internal/data" "github.com/openshift-kni/oran-o2ims/internal/search" ) @@ -150,11 +147,11 @@ func (b *AlarmHandlerBuilder) Build() ( } // Create the HTTP client that we will use to connect to the backend: - var backendTransport http.RoundTripper = net.SetTransportDefaults(&http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: utils.GetTLSSkipVerify(), // nolint: gosec // defaulted to false; logged if disabled - }, - }) + backendTransport, err := utils.GetDefaultBackendTransport() + if err != nil { + err = fmt.Errorf("failed to create default HTTP backend transport: %w", err) + return + } if b.transportWrapper != nil { backendTransport = b.transportWrapper(backendTransport) } diff --git a/internal/service/deployment_manager_handler.go b/internal/service/deployment_manager_handler.go index ee8811cf..c74072c0 100644 --- a/internal/service/deployment_manager_handler.go +++ b/internal/service/deployment_manager_handler.go @@ -16,7 +16,6 @@ package service import ( "context" - "crypto/tls" "encoding/json" "errors" "fmt" @@ -26,11 +25,9 @@ import ( "slices" "sync" - "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" - "k8s.io/apimachinery/pkg/util/net" - "github.com/imdario/mergo" jsoniter "github.com/json-iterator/go" + "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" "gopkg.in/yaml.v3" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -177,11 +174,11 @@ func (b *DeploymentManagerHandlerBuilder) Build() ( } // Create the HTTP client that we will use to connect to the backend: - var backendTransport http.RoundTripper = net.SetTransportDefaults(&http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: utils.GetTLSSkipVerify(), // nolint: gosec // defaulted to false; logged if disabled - }, - }) + backendTransport, err := utils.GetDefaultBackendTransport() + if err != nil { + err = fmt.Errorf("failed to create default HTTP backend transport: %w", err) + return + } if b.loggingWrapper != nil { backendTransport = b.loggingWrapper(backendTransport) } diff --git a/internal/service/resource_fetcher.go b/internal/service/resource_fetcher.go index 09e0fbbd..9d32908f 100644 --- a/internal/service/resource_fetcher.go +++ b/internal/service/resource_fetcher.go @@ -17,7 +17,6 @@ package service import ( "bytes" "context" - "crypto/tls" "encoding/json" "errors" "fmt" @@ -26,8 +25,6 @@ import ( "net/http" "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" - "k8s.io/apimachinery/pkg/util/net" - "github.com/openshift-kni/oran-o2ims/internal/data" "github.com/openshift-kni/oran-o2ims/internal/k8s" "github.com/openshift-kni/oran-o2ims/internal/model" @@ -137,11 +134,11 @@ func (b *ResourceFetcherBuilder) Build() ( } // Create the HTTP client that we will use to connect to the backend: - var backendTransport http.RoundTripper = net.SetTransportDefaults(&http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: utils.GetTLSSkipVerify(), // nolint: gosec // defaulted to false; logged if disabled - }, - }) + backendTransport, err := utils.GetDefaultBackendTransport() + if err != nil { + err = fmt.Errorf("failed to create default HTTP backend transport: %w", err) + return + } if b.transportWrapper != nil { backendTransport = b.transportWrapper(backendTransport) } diff --git a/internal/service/resource_handler.go b/internal/service/resource_handler.go index be45b0cb..4cef4378 100644 --- a/internal/service/resource_handler.go +++ b/internal/service/resource_handler.go @@ -16,16 +16,14 @@ package service import ( "context" - "crypto/tls" "errors" + "fmt" "log/slog" "net/http" "slices" - "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" - "k8s.io/apimachinery/pkg/util/net" - "github.com/itchyny/gojq" + "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" "github.com/openshift-kni/oran-o2ims/internal/data" "github.com/openshift-kni/oran-o2ims/internal/graphql" @@ -149,11 +147,11 @@ func (b *ResourceHandlerBuilder) Build() ( } // Create the HTTP client that we will use to connect to the backend: - var backendTransport http.RoundTripper = net.SetTransportDefaults(&http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: utils.GetTLSSkipVerify(), // nolint: gosec // defaulted to false; logged if disabled - }, - }) + backendTransport, err := utils.GetDefaultBackendTransport() + if err != nil { + err = fmt.Errorf("failed to create default HTTP backend transport: %w", err) + return + } if b.transportWrapper != nil { backendTransport = b.transportWrapper(backendTransport) } diff --git a/internal/service/resource_pool_fetcher.go b/internal/service/resource_pool_fetcher.go index c72b980a..8ac31ff6 100644 --- a/internal/service/resource_pool_fetcher.go +++ b/internal/service/resource_pool_fetcher.go @@ -17,7 +17,6 @@ package service import ( "bytes" "context" - "crypto/tls" "encoding/json" "errors" "fmt" @@ -27,8 +26,6 @@ import ( "strings" "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" - "k8s.io/apimachinery/pkg/util/net" - "github.com/openshift-kni/oran-o2ims/internal/data" "github.com/openshift-kni/oran-o2ims/internal/graphql" "github.com/openshift-kni/oran-o2ims/internal/jq" @@ -148,11 +145,11 @@ func (b *ResourcePoolFetcherBuilder) Build() ( } // Create the HTTP client that we will use to connect to the backend: - var backendTransport http.RoundTripper = net.SetTransportDefaults(&http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: utils.GetTLSSkipVerify(), // nolint: gosec // defaulted to false; logged if disabled - }, - }) + backendTransport, err := utils.GetDefaultBackendTransport() + if err != nil { + err = fmt.Errorf("failed to create default HTTP backend transport: %w", err) + return + } if b.transportWrapper != nil { backendTransport = b.transportWrapper(backendTransport) } diff --git a/internal/service/resource_pool_handler.go b/internal/service/resource_pool_handler.go index 0fcfbeda..e3d33b3c 100644 --- a/internal/service/resource_pool_handler.go +++ b/internal/service/resource_pool_handler.go @@ -16,15 +16,13 @@ package service import ( "context" - "crypto/tls" "errors" + "fmt" "log/slog" "net/http" "slices" "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" - "k8s.io/apimachinery/pkg/util/net" - "github.com/openshift-kni/oran-o2ims/internal/data" "github.com/openshift-kni/oran-o2ims/internal/graphql" "github.com/openshift-kni/oran-o2ims/internal/model" @@ -145,11 +143,11 @@ func (b *ResourcePoolHandlerBuilder) Build() ( } // Create the HTTP client that we will use to connect to the backend: - var backendTransport http.RoundTripper = net.SetTransportDefaults(&http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: utils.GetTLSSkipVerify(), // nolint: gosec // defaulted to false; logged if disabled - }, - }) + backendTransport, err := utils.GetDefaultBackendTransport() + if err != nil { + err = fmt.Errorf("failed to create default HTTP backend transport: %w", err) + return + } if b.transportWrapper != nil { backendTransport = b.transportWrapper(backendTransport) } diff --git a/internal/service/resource_type_handler.go b/internal/service/resource_type_handler.go index c7d136e8..5fe7860b 100644 --- a/internal/service/resource_type_handler.go +++ b/internal/service/resource_type_handler.go @@ -16,14 +16,12 @@ package service import ( "context" - "crypto/tls" "errors" + "fmt" "log/slog" "net/http" "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" - "k8s.io/apimachinery/pkg/util/net" - "github.com/openshift-kni/oran-o2ims/internal/data" "github.com/openshift-kni/oran-o2ims/internal/files" "github.com/openshift-kni/oran-o2ims/internal/model" @@ -146,11 +144,11 @@ func (b *ResourceTypeHandlerBuilder) Build() ( } // Create the HTTP client that we will use to connect to the backend: - var backendTransport http.RoundTripper = net.SetTransportDefaults(&http.Transport{ - TLSClientConfig: &tls.Config{ - InsecureSkipVerify: utils.GetTLSSkipVerify(), // nolint: gosec // defaulted to false; logged if disabled - }, - }) + backendTransport, err := utils.GetDefaultBackendTransport() + if err != nil { + err = fmt.Errorf("failed to create default HTTP backend transport: %w", err) + return + } if b.transportWrapper != nil { backendTransport = b.transportWrapper(backendTransport) } diff --git a/internal/service/suite_test.go b/internal/service/suite_test.go index 8e92109b..4c2e7171 100644 --- a/internal/service/suite_test.go +++ b/internal/service/suite_test.go @@ -16,10 +16,12 @@ package service import ( "log/slog" + "os" "testing" . "github.com/onsi/ginkgo/v2/dsl/core" . "github.com/onsi/gomega" + "github.com/openshift-kni/oran-o2ims/internal/controllers/utils" ) func TestService(t *testing.T) { @@ -37,4 +39,9 @@ var _ = BeforeSuite(func() { } handler := slog.NewJSONHandler(GinkgoWriter, options) logger = slog.New(handler) + + // Disable TLS verification, none of these tests use a TLS server anyway so there is no point in running (and + // failing) the code that attempts to setup the TLS client. + err := os.Setenv(utils.TLSSkipVerifyEnvName, "true") + Expect(err).NotTo(HaveOccurred()) })