Skip to content

Commit 3fac15e

Browse files
committed
⚠️ refactor(grpc): change TLSMinVersion/TLSMaxVersion from uint16 to string
Change TLSMinVersion and TLSMaxVersion from uint16 to string in GRPCServerOptions, so config files use human-readable names like "VersionTLS12" instead of raw numeric constants like 771. The string values are parsed to uint16 by Validate() using pkgtls.ParseTLSVersion, keeping the runtime behavior identical. ApplyTLSFlags is simplified to just set the string directly. Signed-off-by: Jia Zhu <jiazhu@redhat.com> Signed-off-by: zhujian <jiazhu@redhat.com>
1 parent 996da9f commit 3fac15e

3 files changed

Lines changed: 32 additions & 25 deletions

File tree

pkg/server/grpc/options.go

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ type GRPCServerOptions struct {
1818
TLSCertFile string `json:"tls_cert_file" yaml:"tls_cert_file"`
1919
TLSKeyFile string `json:"tls_key_file" yaml:"tls_key_file"`
2020
ClientCAFile string `json:"client_ca_file" yaml:"client_ca_file"`
21-
TLSMinVersion uint16 `json:"tls_min_version" yaml:"tls_min_version"`
22-
TLSMaxVersion uint16 `json:"tls_max_version" yaml:"tls_max_version"`
21+
TLSMinVersion string `json:"tls_min_version" yaml:"tls_min_version"`
22+
TLSMaxVersion string `json:"tls_max_version" yaml:"tls_max_version"`
2323
CipherSuites string `json:"cipher_suites" yaml:"cipher_suites"`
2424
ServerBindPort string `json:"server_bind_port" yaml:"server_bind_port"`
2525
MaxConcurrentStreams uint32 `json:"max_concurrent_streams" yaml:"max_concurrent_streams"`
@@ -35,7 +35,9 @@ type GRPCServerOptions struct {
3535
PermitPingWithoutStream bool `json:"permit_ping_without_stream" yaml:"permit_ping_without_stream"`
3636
CertWatchInterval time.Duration `json:"cert_watch_interval" yaml:"cert_watch_interval"`
3737

38-
// cipherSuiteIDs holds the parsed uint16 IDs from CipherSuites, populated by Validate().
38+
// Parsed TLS settings, populated by Validate().
39+
tlsMinVersion uint16
40+
tlsMaxVersion uint16
3941
cipherSuiteIDs []uint16
4042
}
4143

@@ -67,8 +69,8 @@ func NewGRPCServerOptions() *GRPCServerOptions {
6769
ClientCAFile: "/var/run/secrets/hub/grpc/ca/ca-bundle.crt",
6870
TLSCertFile: "/var/run/secrets/hub/grpc/serving-cert/tls.crt",
6971
TLSKeyFile: "/var/run/secrets/hub/grpc/serving-cert/tls.key",
70-
TLSMinVersion: tls.VersionTLS12,
71-
TLSMaxVersion: tls.VersionTLS13,
72+
TLSMinVersion: "VersionTLS12",
73+
TLSMaxVersion: "VersionTLS13",
7274
ServerBindPort: "8090",
7375
MaxConcurrentStreams: math.MaxUint32,
7476
MaxReceiveMessageSize: 1024 * 1024 * 4,
@@ -105,13 +107,22 @@ func (o *GRPCServerOptions) AddFlags(flags *pflag.FlagSet) {
105107

106108
// Validate checks option ranges and cross-field constraints.
107109
func (o *GRPCServerOptions) Validate() error {
108-
// Enforce sane floor for TLS for security posture.
109-
if o.TLSMinVersion < tls.VersionTLS12 {
110-
return fmt.Errorf("tls_min_version (%d) is lower than TLS 1.2 (771); minimum supported is TLS 1.2", o.TLSMinVersion)
110+
minVer, err := pkgtls.ParseTLSVersion(o.TLSMinVersion)
111+
if err != nil {
112+
return fmt.Errorf("invalid tls_min_version %q: %w", o.TLSMinVersion, err)
113+
}
114+
maxVer, err := pkgtls.ParseTLSVersion(o.TLSMaxVersion)
115+
if err != nil {
116+
return fmt.Errorf("invalid tls_max_version %q: %w", o.TLSMaxVersion, err)
117+
}
118+
if minVer < tls.VersionTLS12 {
119+
return fmt.Errorf("tls_min_version %q is lower than TLS 1.2; minimum supported is TLS 1.2", o.TLSMinVersion)
111120
}
112-
if o.TLSMinVersion > o.TLSMaxVersion {
113-
return fmt.Errorf("tls_min_version (%d) must be <= tls_max_version (%d)", o.TLSMinVersion, o.TLSMaxVersion)
121+
if minVer > maxVer {
122+
return fmt.Errorf("tls_min_version %q must be <= tls_max_version %q", o.TLSMinVersion, o.TLSMaxVersion)
114123
}
124+
o.tlsMinVersion = minVer
125+
o.tlsMaxVersion = maxVer
115126
// Validate certificate watch interval to prevent time.NewTicker panic
116127
if o.CertWatchInterval <= 30*time.Second {
117128
return fmt.Errorf("cert_watch_interval (%v) must be greater than 30 seconds", o.CertWatchInterval)
@@ -125,11 +136,7 @@ func (o *GRPCServerOptions) Validate() error {
125136
// Called after LoadGRPCServerOptions so flags take precedence over the config file.
126137
func (o *GRPCServerOptions) ApplyTLSFlags(minVersion, cipherSuites string) error {
127138
if minVersion != "" {
128-
ver, err := pkgtls.ParseTLSVersion(minVersion)
129-
if err != nil {
130-
return fmt.Errorf("invalid --tls-min-version: %w", err)
131-
}
132-
o.TLSMinVersion = ver
139+
o.TLSMinVersion = minVersion
133140
}
134141
if cipherSuites != "" {
135142
o.CipherSuites = cipherSuites

pkg/server/grpc/options_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ cert_watch_interval: 60s
5656
TLSCertFile: "/test/tls.crt",
5757
TLSKeyFile: "/test/tls.key",
5858
ClientCAFile: "/test/ca.crt",
59-
TLSMinVersion: tls.VersionTLS12,
60-
TLSMaxVersion: tls.VersionTLS13,
59+
TLSMinVersion: "VersionTLS12",
60+
TLSMaxVersion: "VersionTLS13",
6161
ServerBindPort: "9999",
6262
MaxConcurrentStreams: 100,
6363
MaxReceiveMessageSize: 2048,
@@ -320,8 +320,8 @@ func TestApplyTLSFlags(t *testing.T) {
320320
t.Fatalf("unexpected error: %v", err)
321321
}
322322

323-
if opts.TLSMinVersion != tt.expectedMinVer {
324-
t.Errorf("expected TLSMinVersion %d, got %d", tt.expectedMinVer, opts.TLSMinVersion)
323+
if opts.tlsMinVersion != tt.expectedMinVer {
324+
t.Errorf("expected TLSMinVersion %d, got %d", tt.expectedMinVer, opts.tlsMinVersion)
325325
}
326326

327327
if tt.expectedCipherCount > 0 {
@@ -336,7 +336,7 @@ func TestApplyTLSFlags(t *testing.T) {
336336
func TestApplyTLSFlags_OverridesConfigFile(t *testing.T) {
337337
opts := NewGRPCServerOptions()
338338
// Simulate config file values
339-
opts.TLSMinVersion = tls.VersionTLS12
339+
opts.TLSMinVersion = "VersionTLS12"
340340
opts.CipherSuites = "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"
341341

342342
// Flags override
@@ -346,8 +346,8 @@ func TestApplyTLSFlags_OverridesConfigFile(t *testing.T) {
346346
t.Fatalf("unexpected error: %v", err)
347347
}
348348

349-
if opts.TLSMinVersion != tls.VersionTLS13 {
350-
t.Errorf("expected TLSMinVersion TLS 1.3, got %d", opts.TLSMinVersion)
349+
if opts.tlsMinVersion != tls.VersionTLS13 {
350+
t.Errorf("expected TLSMinVersion TLS 1.3, got %d", opts.tlsMinVersion)
351351
}
352352
if len(opts.cipherSuiteIDs) != 2 {
353353
t.Errorf("expected 2 parsed cipher IDs, got %d", len(opts.cipherSuiteIDs))

pkg/server/grpc/server.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,12 @@ func (b *GRPCServer) Run(ctx context.Context) error {
110110
// Use GetCertificate callback from certwatcher
111111
// This allows dynamic certificate reloading on each TLS handshake
112112
GetCertificate: certWatcher.GetCertificate,
113-
MinVersion: b.options.TLSMinVersion,
114-
MaxVersion: b.options.TLSMaxVersion,
113+
MinVersion: b.options.tlsMinVersion,
114+
MaxVersion: b.options.tlsMaxVersion,
115115
}
116116

117117
// TLS 1.3 cipher suites are not configurable in Go — only set for TLS 1.2 and below.
118-
if len(b.options.cipherSuiteIDs) > 0 && b.options.TLSMinVersion < tls.VersionTLS13 {
118+
if len(b.options.cipherSuiteIDs) > 0 && b.options.tlsMinVersion < tls.VersionTLS13 {
119119
tlsConfig.CipherSuites = b.options.cipherSuiteIDs
120120
}
121121

0 commit comments

Comments
 (0)