From 336e5b0e4dcb1e91bd471f046971067bc4b752bc Mon Sep 17 00:00:00 2001 From: Jian Zhu Date: Tue, 7 Apr 2026 09:54:22 +0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=8C=B1=20Add=20TLS=20profile=20compliance?= =?UTF-8?q?=20for=20gRPC=20server=20(#1471)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add TLS profile compliance to the gRPC server, completing TLS support for all hub components. The operator reads the ocm-tls-profile ConfigMap and injects --tls-min-version and --tls-cipher-suites flags into the gRPC server deployment, matching the pattern used by all other hub component deployments. Changes: - Add TLS flag injection to gRPC server deployment manifest - Wire TLS flags from common options to gRPC server via closure - Call ApplyTLSToCommand for the 8443 health server endpoint - Apply TLS overrides to the 8090 gRPC port via SDK ApplyTLSFlags - Update vendored sdk-go with CipherSuites support for gRPC server - Add unit, controller, and integration tests Assisted by Claude Signed-off-by: zhujian --- go.mod | 2 +- go.sum | 4 +- .../management/grpc-server/deployment.yaml | 6 ++ pkg/cmd/hub/grpc_server.go | 18 ++++- pkg/cmd/hub/grpc_server_test.go | 57 +++++++++++++++ .../clustermanager_controller_test.go | 55 +++++++++++++++ pkg/server/grpc/options.go | 17 +++++ pkg/server/grpc/options_test.go | 69 +++++++++++++++++++ .../operator/clustermanager_test.go | 23 +++++++ vendor/modules.txt | 2 +- .../sdk-go/pkg/server/grpc/options.go | 63 ++++++++++++++--- .../sdk-go/pkg/server/grpc/server.go | 13 +++- .../sdk-go/pkg/tls/config.go | 14 ++-- .../sdk-go/pkg/tls/configmap.go | 4 +- 14 files changed, 321 insertions(+), 26 deletions(-) diff --git a/go.mod b/go.mod index 09cc752bf..957c6906d 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ require ( k8s.io/utils v0.0.0-20251002143259-bc988d571ff4 open-cluster-management.io/addon-framework v1.2.1-0.20260306083712-b6f9759b7b6d open-cluster-management.io/api v1.2.1-0.20260401094502-0bf966f2e990 - open-cluster-management.io/sdk-go v1.2.1-0.20260331032456-0cc72e52e4f2 + open-cluster-management.io/sdk-go v1.2.1-0.20260403012606-883108b32db8 sigs.k8s.io/about-api v0.0.0-20250131010323-518069c31c03 sigs.k8s.io/cluster-inventory-api v0.0.0-20251124125836-445319b6307a sigs.k8s.io/controller-runtime v0.23.3 diff --git a/go.sum b/go.sum index 0fb39214a..be06fd7c2 100644 --- a/go.sum +++ b/go.sum @@ -589,8 +589,8 @@ open-cluster-management.io/addon-framework v1.2.1-0.20260306083712-b6f9759b7b6d open-cluster-management.io/addon-framework v1.2.1-0.20260306083712-b6f9759b7b6d/go.mod h1:Bpw37w4GANroADMDR3F/ZUFoEuTKV9GIn4ijwICcK6E= open-cluster-management.io/api v1.2.1-0.20260401094502-0bf966f2e990 h1:FIegzf9obfPNS8VYxKM3G61uppH0advxtJuorUzNznA= open-cluster-management.io/api v1.2.1-0.20260401094502-0bf966f2e990/go.mod h1:ZpXs1bFTIIqKstMHdLO9IY0NFlbCvZgEtByvvNSmab0= -open-cluster-management.io/sdk-go v1.2.1-0.20260331032456-0cc72e52e4f2 h1:8azc9t2KtyyDdtj6dLsbbuegyxntjkTAyCw5iUdjPZI= -open-cluster-management.io/sdk-go v1.2.1-0.20260331032456-0cc72e52e4f2/go.mod h1:lDef+5BvifXww0S7cseux+Wi8melkH29bAf33OZ0ZVg= +open-cluster-management.io/sdk-go v1.2.1-0.20260403012606-883108b32db8 h1:NUGS/0gO3gxC/y8Qu6/27jYyRud72rSf+0ck6AQOJ2o= +open-cluster-management.io/sdk-go v1.2.1-0.20260403012606-883108b32db8/go.mod h1:lDef+5BvifXww0S7cseux+Wi8melkH29bAf33OZ0ZVg= sigs.k8s.io/about-api v0.0.0-20250131010323-518069c31c03 h1:1ShFiMjGQOR/8jTBkmZrk1gORxnvMwm1nOy2/DbHg4U= sigs.k8s.io/about-api v0.0.0-20250131010323-518069c31c03/go.mod h1:F1pT4mK53U6F16/zuaPSYpBaR7x5Kjym6aKJJC0/DHU= sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.2 h1:jpcvIRr3GLoUoEKRkHKSmGjxb6lWwrBlJsXc+eUYQHM= diff --git a/manifests/cluster-manager/management/grpc-server/deployment.yaml b/manifests/cluster-manager/management/grpc-server/deployment.yaml index 94587984c..1c93a63b8 100644 --- a/manifests/cluster-manager/management/grpc-server/deployment.yaml +++ b/manifests/cluster-manager/management/grpc-server/deployment.yaml @@ -64,6 +64,12 @@ spec: {{ if .HostedMode }} - "--kubeconfig=/var/run/secrets/hub/kubeconfig" {{ end }} + {{- if .TLSMinVersion }} + - "--tls-min-version={{ .TLSMinVersion }}" + {{- end }} + {{- if .TLSCipherSuites }} + - "--tls-cipher-suites={{ .TLSCipherSuites }}" + {{- end }} env: - name: POD_NAMESPACE valueFrom: diff --git a/pkg/cmd/hub/grpc_server.go b/pkg/cmd/hub/grpc_server.go index 9a90b092d..944c54991 100644 --- a/pkg/cmd/hub/grpc_server.go +++ b/pkg/cmd/hub/grpc_server.go @@ -8,16 +8,17 @@ import ( "k8s.io/utils/clock" commonoptions "open-cluster-management.io/ocm/pkg/common/options" - "open-cluster-management.io/ocm/pkg/server/grpc" + grpcopts "open-cluster-management.io/ocm/pkg/server/grpc" "open-cluster-management.io/ocm/pkg/version" ) func NewGRPCServerCommand() *cobra.Command { opts := commonoptions.NewOptions() - grpcServerOpts := grpc.NewGRPCServerOptions() + grpcServerOpts := grpcopts.NewGRPCServerOptions() // Disable leader election to allow multiple gRPC server instances to run concurrently. - cmdConfig := controllercmd.NewControllerCommandConfig("grpc-server", version.Get(), opts.StartWithQPS(grpcServerOpts.Run), clock.RealClock{}) + cmdConfig := controllercmd.NewControllerCommandConfig("grpc-server", version.Get(), + opts.StartWithQPS(grpcStartFunc(opts, grpcServerOpts)), clock.RealClock{}) cmdConfig.DisableLeaderElection = true cmd := cmdConfig.NewCommandWithContext(context.TODO()) @@ -27,6 +28,17 @@ func NewGRPCServerCommand() *cobra.Command { flags := cmd.Flags() opts.AddFlags(flags) grpcServerOpts.AddFlags(flags) + opts.ApplyTLSToCommand(cmd) return cmd } + +// grpcStartFunc bridges TLS flags from common options to gRPC server options +// before starting the gRPC server. Extracted for testability. +func grpcStartFunc(opts *commonoptions.Options, grpcServerOpts *grpcopts.GRPCServerOptions) controllercmd.StartFunc { + return func(ctx context.Context, cc *controllercmd.ControllerContext) error { + grpcServerOpts.TLSMinVersionOverride = opts.TLSMinVersion + grpcServerOpts.TLSCipherSuitesOverride = opts.TLSCipherSuites + return grpcServerOpts.Run(ctx, cc) + } +} diff --git a/pkg/cmd/hub/grpc_server_test.go b/pkg/cmd/hub/grpc_server_test.go index 8e49b8c30..43df77246 100644 --- a/pkg/cmd/hub/grpc_server_test.go +++ b/pkg/cmd/hub/grpc_server_test.go @@ -1,9 +1,17 @@ package hub import ( + "context" + "strings" "testing" + "time" + "github.com/openshift/library-go/pkg/controller/controllercmd" "github.com/spf13/cobra" + "k8s.io/client-go/rest" + + commonoptions "open-cluster-management.io/ocm/pkg/common/options" + grpcopts "open-cluster-management.io/ocm/pkg/server/grpc" ) func TestNewGRPCServerCommand(t *testing.T) { @@ -70,3 +78,52 @@ func TestGRPCServerCommandExecution(t *testing.T) { t.Errorf("Command execution with --help failed: %v", err) } } + +func TestGRPCStartFuncBridgesTLSFlags(t *testing.T) { + opts := commonoptions.NewOptions() + opts.TLSMinVersion = "VersionTLS12" + opts.TLSCipherSuites = "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" + + grpcServerOpts := grpcopts.NewGRPCServerOptions() + startFunc := grpcStartFunc(opts, grpcServerOpts) + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + + // Run will fail (no real kube context), but the TLS fields should be bridged first. + err := startFunc(ctx, &controllercmd.ControllerContext{ + KubeConfig: &rest.Config{Host: "https://example.com"}, + }) + + // Verify TLS flags were bridged before Run failed + if grpcServerOpts.TLSMinVersionOverride != "VersionTLS12" { + t.Errorf("expected TLSMinVersionOverride=VersionTLS12, got %q", grpcServerOpts.TLSMinVersionOverride) + } + if grpcServerOpts.TLSCipherSuitesOverride != "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" { + t.Errorf("expected TLSCipherSuitesOverride=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, got %q", + grpcServerOpts.TLSCipherSuitesOverride) + } + + // The error should be from client creation, not TLS parsing + if err != nil && strings.Contains(err.Error(), "failed to apply gRPC TLS overrides") { + t.Errorf("unexpected TLS error: %v", err) + } +} + +func TestGRPCServerCommandTLSWiring(t *testing.T) { + cmd := NewGRPCServerCommand() + + // ApplyTLSToCommand should have set PersistentPreRunE + if cmd.PersistentPreRunE == nil { + t.Error("Expected PersistentPreRunE to be set by ApplyTLSToCommand") + } + + // TLS flags should be registered via common options + flags := cmd.Flags() + if flags.Lookup("tls-min-version") == nil { + t.Error("Expected --tls-min-version flag to be registered") + } + if flags.Lookup("tls-cipher-suites") == nil { + t.Error("Expected --tls-cipher-suites flag to be registered") + } +} diff --git a/pkg/operator/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller_test.go b/pkg/operator/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller_test.go index 67b5b7a47..43ca8762f 100644 --- a/pkg/operator/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller_test.go +++ b/pkg/operator/operators/clustermanager/controllers/clustermanagercontroller/clustermanager_controller_test.go @@ -1561,6 +1561,61 @@ func TestPlacementFeatureGate(t *testing.T) { } } +// TestSyncDeployWithTLSConfigGRPC verifies that TLS flags are injected into the +// gRPC server deployment when gRPC auth is enabled and TLS config is set. +func TestSyncDeployWithTLSConfigGRPC(t *testing.T) { + clusterManager := newClusterManager("testhub") + clusterManager.Spec.RegistrationConfiguration = &operatorapiv1.RegistrationHubConfiguration{ + RegistrationDrivers: []operatorapiv1.RegistrationDriverHub{ + {AuthType: operatorapiv1.GRPCAuthType}, + }, + } + + tc := newTestController(t, clusterManager) + tc.clusterManagerController.tlsMinVersion = "VersionTLS13" + tc.clusterManagerController.tlsCipherSuites = "TLS_AES_256_GCM_SHA384" + + clusterManagerNamespace := helpers.ClusterManagerNamespace( + clusterManager.Name, clusterManager.Spec.DeployOption.Mode) + cd := setDeployment(clusterManager.Name, clusterManagerNamespace) + setup(t, tc, cd) + + syncContext := testingcommon.NewFakeSyncContext(t, clusterManager.Name) + if err := tc.clusterManagerController.sync(ctx, syncContext, clusterManager.Name); err != nil { + t.Fatalf("unexpected sync error: %v", err) + } + + // Find the gRPC server deployment in sync actions. + kubeActions := append(tc.hubKubeClient.Actions(), tc.managementKubeClient.Actions()...) + var grpcDeploy *appsv1.Deployment + for _, action := range kubeActions { + if action.GetVerb() != createVerb && action.GetVerb() != "update" { + continue + } + var d *appsv1.Deployment + if action.GetVerb() == createVerb { + d, _ = action.(clienttesting.CreateActionImpl).Object.(*appsv1.Deployment) + } else { + d, _ = action.(clienttesting.UpdateActionImpl).Object.(*appsv1.Deployment) + } + if d != nil && d.Name == "testhub-grpc-server" { + grpcDeploy = d + } + } + + if grpcDeploy == nil { + t.Fatal("gRPC server deployment not found in sync actions") + } + + args := grpcDeploy.Spec.Template.Spec.Containers[0].Args + if !containsArg(args, "--tls-min-version=VersionTLS13") { + t.Errorf("gRPC server deployment missing --tls-min-version flag (args: %v)", args) + } + if !containsArg(args, "--tls-cipher-suites=TLS_AES_256_GCM_SHA384") { + t.Errorf("gRPC server deployment missing --tls-cipher-suites flag (args: %v)", args) + } +} + func containsArg(args []string, prefix string) bool { for _, a := range args { if strings.HasPrefix(a, prefix) { diff --git a/pkg/server/grpc/options.go b/pkg/server/grpc/options.go index 31e670a9f..bc1ea8bfa 100644 --- a/pkg/server/grpc/options.go +++ b/pkg/server/grpc/options.go @@ -2,10 +2,12 @@ package grpc import ( "context" + "fmt" "github.com/openshift/library-go/pkg/controller/controllercmd" "github.com/spf13/pflag" "google.golang.org/grpc" + "k8s.io/klog/v2" v1alpha1addonce "open-cluster-management.io/sdk-go/pkg/cloudevents/clients/addon/v1alpha1" v1beta1addonce "open-cluster-management.io/sdk-go/pkg/cloudevents/clients/addon/v1beta1" @@ -35,6 +37,11 @@ import ( type GRPCServerOptions struct { GRPCServerConfig string grpcBrokerOptions *cloudeventsgrpc.BrokerOptions + + // TLS overrides from CLI flags (set by grpc_server.go from common options). + // These take precedence over the YAML config file loaded by LoadGRPCServerOptions. + TLSMinVersionOverride string + TLSCipherSuitesOverride string } func NewGRPCServerOptions() *GRPCServerOptions { @@ -54,6 +61,16 @@ func (o *GRPCServerOptions) Run(ctx context.Context, controllerContext *controll return err } + // Override gRPC server TLS settings from CLI flags if provided. + // These flags are injected by the cluster-manager operator via the deployment manifest. + if o.TLSMinVersionOverride != "" || o.TLSCipherSuitesOverride != "" { + if err := serverOptions.ApplyTLSFlags(o.TLSMinVersionOverride, o.TLSCipherSuitesOverride); err != nil { + return fmt.Errorf("failed to apply gRPC TLS overrides: %w", err) + } + klog.Infof("gRPC server TLS overridden: minVersion=%s, cipherSuites=%v", + o.TLSMinVersionOverride, serverOptions.CipherSuites) + } + clients, err := NewClients(controllerContext) if err != nil { return err diff --git a/pkg/server/grpc/options_test.go b/pkg/server/grpc/options_test.go index 9b862b9a0..8e1a5eaed 100644 --- a/pkg/server/grpc/options_test.go +++ b/pkg/server/grpc/options_test.go @@ -115,6 +115,75 @@ func TestGRPCServerOptionsRunWithInvalidKubeConfig(t *testing.T) { } } +func TestGRPCServerOptionsRunWithTLSOverride(t *testing.T) { + cases := []struct { + name string + tlsMinVersion string + tlsCipherSuites string + wantError bool + }{ + { + name: "valid TLS 1.3 override", + tlsMinVersion: "VersionTLS13", + }, + { + name: "valid TLS 1.2 override", + tlsMinVersion: "VersionTLS12", + }, + { + name: "valid cipher suites override", + tlsCipherSuites: "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + }, + { + name: "valid TLS 1.2 with cipher suites", + tlsMinVersion: "VersionTLS12", + tlsCipherSuites: "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + }, + { + name: "invalid TLS version returns error", + tlsMinVersion: "BadVersion", + wantError: true, + }, + { + name: "invalid cipher suite returns error", + tlsCipherSuites: "NOT_A_REAL_CIPHER", + wantError: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + opts := NewGRPCServerOptions() + opts.TLSMinVersionOverride = c.tlsMinVersion + opts.TLSCipherSuitesOverride = c.tlsCipherSuites + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + + controllerContext := &controllercmd.ControllerContext{ + KubeConfig: &rest.Config{Host: "https://example.com"}, + } + + err := opts.Run(ctx, controllerContext) + if c.wantError { + if err == nil { + t.Fatal("expected an error but got none") + } + if !strings.Contains(err.Error(), "TLS") && !strings.Contains(err.Error(), "tls") { + t.Errorf("expected TLS-related error, got: %v", err) + } + return + } + // For valid TLS overrides, Run() will proceed past TLS parsing but eventually + // fail due to missing certificates or kubeconfig — that's expected. + // We just verify it didn't fail on TLS parsing. + if err != nil && (strings.Contains(err.Error(), "failed to apply gRPC TLS overrides") || strings.Contains(err.Error(), "tls-min-version")) { + t.Errorf("unexpected TLS parsing error: %v", err) + } + }) + } +} + func TestGRPCServerOptionsRunWithValidConfigFile(t *testing.T) { // Create a temporary config file for testing tempDir := t.TempDir() diff --git a/test/integration/operator/clustermanager_test.go b/test/integration/operator/clustermanager_test.go index 57f96d828..7a42c7975 100644 --- a/test/integration/operator/clustermanager_test.go +++ b/test/integration/operator/clustermanager_test.go @@ -1692,4 +1692,27 @@ var _ = ginkgo.Describe("ClusterManager TLS Profile", func() { }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil()) } }) + + ginkgo.It("should inject tls-min-version into gRPC server deployment when gRPC auth is enabled", func() { + // Enable gRPC auth so the operator creates the gRPC server deployment. + gomega.Eventually(func() error { + return enableGRPCAuth(operatorClient, clusterManagerName) + }, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.HaveOccurred()) + + hubGRPCServerDeployment := fmt.Sprintf("%s-grpc-server", clusterManagerName) + gomega.Eventually(func() error { + actual, err := kubeClient.AppsV1().Deployments(hubNamespace).Get( + context.Background(), hubGRPCServerDeployment, metav1.GetOptions{}) + if err != nil { + return err + } + for _, arg := range actual.Spec.Template.Spec.Containers[0].Args { + if arg == "--tls-min-version=VersionTLS13" { + return nil + } + } + return fmt.Errorf("deployment %s: --tls-min-version=VersionTLS13 not found in args %v", + hubGRPCServerDeployment, actual.Spec.Template.Spec.Containers[0].Args) + }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeNil()) + }) }) diff --git a/vendor/modules.txt b/vendor/modules.txt index dbd5b05a9..a568bddac 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1972,7 +1972,7 @@ open-cluster-management.io/api/operator/v1 open-cluster-management.io/api/utils/work/v1/workapplier open-cluster-management.io/api/work/v1 open-cluster-management.io/api/work/v1alpha1 -# open-cluster-management.io/sdk-go v1.2.1-0.20260331032456-0cc72e52e4f2 +# open-cluster-management.io/sdk-go v1.2.1-0.20260403012606-883108b32db8 ## explicit; go 1.25.0 open-cluster-management.io/sdk-go/pkg/apis/cluster/v1alpha1 open-cluster-management.io/sdk-go/pkg/apis/cluster/v1beta1 diff --git a/vendor/open-cluster-management.io/sdk-go/pkg/server/grpc/options.go b/vendor/open-cluster-management.io/sdk-go/pkg/server/grpc/options.go index c3c1fd2f7..5de858364 100644 --- a/vendor/open-cluster-management.io/sdk-go/pkg/server/grpc/options.go +++ b/vendor/open-cluster-management.io/sdk-go/pkg/server/grpc/options.go @@ -10,14 +10,17 @@ import ( "github.com/spf13/pflag" "gopkg.in/yaml.v2" "k8s.io/klog/v2" + + pkgtls "open-cluster-management.io/sdk-go/pkg/tls" ) type GRPCServerOptions struct { TLSCertFile string `json:"tls_cert_file" yaml:"tls_cert_file"` TLSKeyFile string `json:"tls_key_file" yaml:"tls_key_file"` ClientCAFile string `json:"client_ca_file" yaml:"client_ca_file"` - TLSMinVersion uint16 `json:"tls_min_version" yaml:"tls_min_version"` - TLSMaxVersion uint16 `json:"tls_max_version" yaml:"tls_max_version"` + TLSMinVersion string `json:"tls_min_version" yaml:"tls_min_version"` + TLSMaxVersion string `json:"tls_max_version" yaml:"tls_max_version"` + CipherSuites string `json:"cipher_suites" yaml:"cipher_suites"` ServerBindPort string `json:"server_bind_port" yaml:"server_bind_port"` MaxConcurrentStreams uint32 `json:"max_concurrent_streams" yaml:"max_concurrent_streams"` MaxReceiveMessageSize int `json:"max_receive_message_size" yaml:"max_receive_message_size"` @@ -31,6 +34,11 @@ type GRPCServerOptions struct { ServerPingTimeout time.Duration `json:"server_ping_timeout" yaml:"server_ping_timeout"` PermitPingWithoutStream bool `json:"permit_ping_without_stream" yaml:"permit_ping_without_stream"` CertWatchInterval time.Duration `json:"cert_watch_interval" yaml:"cert_watch_interval"` + + // Parsed TLS settings, populated by Validate(). + tlsMinVersion uint16 + tlsMaxVersion uint16 + cipherSuiteIDs []uint16 } func LoadGRPCServerOptions(configPath string) (*GRPCServerOptions, error) { @@ -61,8 +69,8 @@ func NewGRPCServerOptions() *GRPCServerOptions { ClientCAFile: "/var/run/secrets/hub/grpc/ca/ca-bundle.crt", TLSCertFile: "/var/run/secrets/hub/grpc/serving-cert/tls.crt", TLSKeyFile: "/var/run/secrets/hub/grpc/serving-cert/tls.key", - TLSMinVersion: tls.VersionTLS12, - TLSMaxVersion: tls.VersionTLS13, + TLSMinVersion: "VersionTLS12", + TLSMaxVersion: "VersionTLS13", ServerBindPort: "8090", MaxConcurrentStreams: math.MaxUint32, MaxReceiveMessageSize: 1024 * 1024 * 4, @@ -99,16 +107,53 @@ func (o *GRPCServerOptions) AddFlags(flags *pflag.FlagSet) { // Validate checks option ranges and cross-field constraints. func (o *GRPCServerOptions) Validate() error { - // Enforce sane floor for TLS for security posture. - if o.TLSMinVersion < tls.VersionTLS12 { - return fmt.Errorf("tls_min_version (%d) is lower than TLS 1.2 (771); minimum supported is TLS 1.2", o.TLSMinVersion) + minVer, err := pkgtls.ParseTLSVersion(o.TLSMinVersion) + if err != nil { + return fmt.Errorf("invalid tls_min_version %q: %w", o.TLSMinVersion, err) } - if o.TLSMinVersion > o.TLSMaxVersion { - return fmt.Errorf("tls_min_version (%d) must be <= tls_max_version (%d)", o.TLSMinVersion, o.TLSMaxVersion) + maxVer, err := pkgtls.ParseTLSVersion(o.TLSMaxVersion) + if err != nil { + return fmt.Errorf("invalid tls_max_version %q: %w", o.TLSMaxVersion, err) } + if minVer < tls.VersionTLS12 { + return fmt.Errorf("tls_min_version %q is lower than TLS 1.2; minimum supported is TLS 1.2", o.TLSMinVersion) + } + if minVer > maxVer { + return fmt.Errorf("tls_min_version %q must be <= tls_max_version %q", o.TLSMinVersion, o.TLSMaxVersion) + } + o.tlsMinVersion = minVer + o.tlsMaxVersion = maxVer // Validate certificate watch interval to prevent time.NewTicker panic if o.CertWatchInterval <= 30*time.Second { return fmt.Errorf("cert_watch_interval (%v) must be greater than 30 seconds", o.CertWatchInterval) } + + return o.parseCipherSuiteIDs() +} + +// ApplyTLSFlags overrides TLS settings loaded from the config file with values +// from --tls-min-version and --tls-cipher-suites command-line flags. +// Called after LoadGRPCServerOptions so flags take precedence over the config file. +func (o *GRPCServerOptions) ApplyTLSFlags(minVersion, cipherSuites string) error { + if minVersion != "" { + o.TLSMinVersion = minVersion + } + if cipherSuites != "" { + o.CipherSuites = cipherSuites + } + return o.Validate() +} + +// parseCipherSuiteIDs converts the CipherSuites IANA names into uint16 IDs +// using the shared pkg/tls parsing utilities. +func (o *GRPCServerOptions) parseCipherSuiteIDs() error { + if o.CipherSuites == "" { + return nil + } + ids, unsupported := pkgtls.ParseCipherSuites(o.CipherSuites) + if len(unsupported) > 0 { + return fmt.Errorf("unrecognized cipher suite: %s", unsupported[0]) + } + o.cipherSuiteIDs = ids return nil } diff --git a/vendor/open-cluster-management.io/sdk-go/pkg/server/grpc/server.go b/vendor/open-cluster-management.io/sdk-go/pkg/server/grpc/server.go index 538de3f6b..932329945 100644 --- a/vendor/open-cluster-management.io/sdk-go/pkg/server/grpc/server.go +++ b/vendor/open-cluster-management.io/sdk-go/pkg/server/grpc/server.go @@ -67,6 +67,10 @@ func (b *GRPCServer) WithStreamAuthorizer(authorizer authz.StreamAuthorizer) *GR } func (b *GRPCServer) Run(ctx context.Context) error { + if err := b.options.Validate(); err != nil { + return err + } + var grpcServerOptions []grpc.ServerOption grpcServerOptions = append(grpcServerOptions, grpc.MaxRecvMsgSize(b.options.MaxReceiveMessageSize)) grpcServerOptions = append(grpcServerOptions, grpc.MaxSendMsgSize(b.options.MaxSendMessageSize)) @@ -106,8 +110,13 @@ func (b *GRPCServer) Run(ctx context.Context) error { // Use GetCertificate callback from certwatcher // This allows dynamic certificate reloading on each TLS handshake GetCertificate: certWatcher.GetCertificate, - MinVersion: b.options.TLSMinVersion, - MaxVersion: b.options.TLSMaxVersion, + MinVersion: b.options.tlsMinVersion, + MaxVersion: b.options.tlsMaxVersion, + } + + // TLS 1.3 cipher suites are not configurable in Go — only set for TLS 1.2 and below. + if len(b.options.cipherSuiteIDs) > 0 && b.options.tlsMinVersion < tls.VersionTLS13 { + tlsConfig.CipherSuites = b.options.cipherSuiteIDs } if b.options.ClientCAFile != "" { diff --git a/vendor/open-cluster-management.io/sdk-go/pkg/tls/config.go b/vendor/open-cluster-management.io/sdk-go/pkg/tls/config.go index 2f4c42acf..7d10f7826 100644 --- a/vendor/open-cluster-management.io/sdk-go/pkg/tls/config.go +++ b/vendor/open-cluster-management.io/sdk-go/pkg/tls/config.go @@ -60,8 +60,10 @@ type TLSConfig struct { CipherSuites []uint16 } -// parseTLSVersion converts a TLS version string to the corresponding crypto/tls constant -func parseTLSVersion(version string) (uint16, error) { +// ParseTLSVersion converts a TLS version string to the corresponding crypto/tls constant. +// Accepted formats: "VersionTLS10"/"TLSv1.0" through "VersionTLS13"/"TLSv1.3". +// An empty string defaults to TLS 1.2. +func ParseTLSVersion(version string) (uint16, error) { version = strings.TrimSpace(version) switch version { case "VersionTLS10", "TLSv1.0": @@ -78,11 +80,11 @@ func parseTLSVersion(version string) (uint16, error) { } } -// parseCipherSuites converts IANA cipher suite names to Go crypto/tls constants. +// ParseCipherSuites converts IANA cipher suite names to Go crypto/tls constants. // Secure ciphers (tls.CipherSuites) are accepted silently. Insecure ciphers // (tls.InsecureCipherSuites) are accepted but logged as a warning. // Returns a list of cipher suite IDs and a list of unrecognized cipher names. -func parseCipherSuites(cipherString string) ([]uint16, []string) { +func ParseCipherSuites(cipherString string) ([]uint16, []string) { if strings.TrimSpace(cipherString) == "" { return nil, nil } @@ -132,7 +134,7 @@ func ConfigFromFlags(minVersion, cipherSuites string) (*TLSConfig, error) { // Parse min version if minVersion != "" { - ver, err := parseTLSVersion(minVersion) + ver, err := ParseTLSVersion(minVersion) if err != nil { return nil, fmt.Errorf("invalid --tls-min-version: %w", err) } @@ -143,7 +145,7 @@ func ConfigFromFlags(minVersion, cipherSuites string) (*TLSConfig, error) { // Parse cipher suites if cipherSuites != "" { - suites, unsupported := parseCipherSuites(cipherSuites) + suites, unsupported := ParseCipherSuites(cipherSuites) if len(unsupported) > 0 { return nil, fmt.Errorf("unsupported cipher suites: %v", unsupported) } diff --git a/vendor/open-cluster-management.io/sdk-go/pkg/tls/configmap.go b/vendor/open-cluster-management.io/sdk-go/pkg/tls/configmap.go index 7eb30fa4b..850204ba0 100644 --- a/vendor/open-cluster-management.io/sdk-go/pkg/tls/configmap.go +++ b/vendor/open-cluster-management.io/sdk-go/pkg/tls/configmap.go @@ -148,7 +148,7 @@ func parseTLSConfigFromConfigMap(cm *corev1.ConfigMap) (*TLSConfig, error) { minVersionStr = defaultMinTLSVersion } - minVersion, err := parseTLSVersion(minVersionStr) + minVersion, err := ParseTLSVersion(minVersionStr) if err != nil { return nil, fmt.Errorf("invalid minTLSVersion in ConfigMap: %w", err) } @@ -157,7 +157,7 @@ func parseTLSConfigFromConfigMap(cm *corev1.ConfigMap) (*TLSConfig, error) { // Parse cipher suites cipherSuitesStr := cm.Data[ConfigMapKeyCipherSuites] if cipherSuitesStr != "" { - cipherSuites, unsupported := parseCipherSuites(cipherSuitesStr) + cipherSuites, unsupported := ParseCipherSuites(cipherSuitesStr) if len(unsupported) > 0 { klog.Warningf("Unsupported cipher suites in ConfigMap %s/%s: %v", cm.Namespace, cm.Name, unsupported) if len(cipherSuites) == 0 {