Skip to content

Commit 3a10709

Browse files
vepatelvepatel
authored
Add support for ssl ciphers related annotations (#8447)
1 parent 99dc7d1 commit 3a10709

File tree

10 files changed

+761
-45
lines changed

10 files changed

+761
-45
lines changed

‎internal/configs/annotations.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ const BasicAuthSecretAnnotation = "nginx.org/basic-auth-secret" // #nosec G101
1818
// PathRegexAnnotation is the annotation where the regex location (path) modifier is specified.
1919
const PathRegexAnnotation = "nginx.org/path-regex"
2020

21+
// SSLCiphersAnnotation is the annotation where SSL ciphers are specified.
22+
const SSLCiphersAnnotation = "nginx.org/ssl-ciphers"
23+
24+
// SSLPreferServerCiphersAnnotation is the annotation where SSL prefer server ciphers is specified.
25+
const SSLPreferServerCiphersAnnotation = "nginx.org/ssl-prefer-server-ciphers"
26+
2127
// UseClusterIPAnnotation is the annotation where the use-cluster-ip boolean is specified.
2228
const UseClusterIPAnnotation = "nginx.org/use-cluster-ip"
2329

@@ -60,6 +66,8 @@ var minionDenylist = map[string]bool{
6066
"nginx.org/listen-ports": true,
6167
"nginx.org/listen-ports-ssl": true,
6268
"nginx.org/server-snippets": true,
69+
"nginx.org/ssl-ciphers": true,
70+
"nginx.org/ssl-prefer-server-ciphers": true,
6371
"appprotect.f5.com/app_protect_enable": true,
6472
"appprotect.f5.com/app_protect_policy": true,
6573
"appprotect.f5.com/app_protect_security_log_enable": true,
@@ -252,6 +260,18 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool
252260
}
253261
}
254262

263+
if sslCiphers, exists := ingEx.Ingress.Annotations[SSLCiphersAnnotation]; exists {
264+
cfgParams.ServerSSLCiphers = sslCiphers
265+
}
266+
267+
if sslPreferServerCiphers, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, SSLPreferServerCiphersAnnotation, ingEx.Ingress); exists {
268+
if err != nil {
269+
nl.Error(l, err)
270+
} else {
271+
cfgParams.ServerSSLPreferServerCiphers = sslPreferServerCiphers
272+
}
273+
}
274+
255275
if proxyBuffering, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.org/proxy-buffering", ingEx.Ingress); exists {
256276
if err != nil {
257277
nl.Error(l, err)

‎internal/configs/annotations_test.go‎

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,3 +311,245 @@ func BenchmarkMergeMasterAnnotationsIntoMinion(b *testing.B) {
311311
mergeMasterAnnotationsIntoMinion(minionAnnotations, masterAnnotations)
312312
}
313313
}
314+
315+
// TestSSLCipherAnnotationParsing tests the parsing of SSL cipher annotations
316+
func TestSSLCipherAnnotationParsing(t *testing.T) {
317+
t.Parallel()
318+
319+
tests := []struct {
320+
name string
321+
annotations map[string]string
322+
expected ConfigParams
323+
}{
324+
{
325+
name: "SSL ciphers annotation only",
326+
annotations: map[string]string{
327+
"nginx.org/ssl-ciphers": "HIGH:!aNULL:!MD5",
328+
},
329+
expected: ConfigParams{
330+
ServerSSLCiphers: "HIGH:!aNULL:!MD5",
331+
ServerSSLPreferServerCiphers: false,
332+
},
333+
},
334+
{
335+
name: "SSL prefer server ciphers annotation only - true",
336+
annotations: map[string]string{
337+
"nginx.org/ssl-prefer-server-ciphers": "true",
338+
},
339+
expected: ConfigParams{
340+
ServerSSLCiphers: "",
341+
ServerSSLPreferServerCiphers: true,
342+
},
343+
},
344+
{
345+
name: "SSL prefer server ciphers annotation only - True",
346+
annotations: map[string]string{
347+
"nginx.org/ssl-prefer-server-ciphers": "True",
348+
},
349+
expected: ConfigParams{
350+
ServerSSLCiphers: "",
351+
ServerSSLPreferServerCiphers: true,
352+
},
353+
},
354+
{
355+
name: "SSL prefer server ciphers annotation only - false",
356+
annotations: map[string]string{
357+
"nginx.org/ssl-prefer-server-ciphers": "false",
358+
},
359+
expected: ConfigParams{
360+
ServerSSLCiphers: "",
361+
ServerSSLPreferServerCiphers: false,
362+
},
363+
},
364+
{
365+
name: "Both SSL cipher annotations",
366+
annotations: map[string]string{
367+
"nginx.org/ssl-ciphers": "ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256",
368+
"nginx.org/ssl-prefer-server-ciphers": "true",
369+
},
370+
expected: ConfigParams{
371+
ServerSSLCiphers: "ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256",
372+
ServerSSLPreferServerCiphers: true,
373+
},
374+
},
375+
{
376+
name: "Empty SSL ciphers annotation",
377+
annotations: map[string]string{
378+
"nginx.org/ssl-ciphers": "",
379+
},
380+
expected: ConfigParams{
381+
ServerSSLCiphers: "",
382+
ServerSSLPreferServerCiphers: false,
383+
},
384+
},
385+
{
386+
name: "No SSL cipher annotations",
387+
annotations: map[string]string{
388+
"nginx.org/proxy-connect-timeout": "30s",
389+
},
390+
expected: ConfigParams{
391+
ServerSSLCiphers: "",
392+
ServerSSLPreferServerCiphers: false,
393+
},
394+
},
395+
}
396+
397+
for _, tt := range tests {
398+
t.Run(tt.name, func(t *testing.T) {
399+
ingress := &networking.Ingress{
400+
ObjectMeta: metav1.ObjectMeta{
401+
Name: "test-ingress",
402+
Namespace: "default",
403+
Annotations: tt.annotations,
404+
},
405+
}
406+
407+
ingEx := &IngressEx{
408+
Ingress: ingress,
409+
}
410+
411+
baseCfgParams := NewDefaultConfigParams(context.Background(), false)
412+
result := parseAnnotations(ingEx, baseCfgParams, false, false, false, false, false)
413+
414+
if result.ServerSSLCiphers != tt.expected.ServerSSLCiphers {
415+
t.Errorf("Expected ServerSSLCiphers %q, got %q", tt.expected.ServerSSLCiphers, result.ServerSSLCiphers)
416+
}
417+
418+
if result.ServerSSLPreferServerCiphers != tt.expected.ServerSSLPreferServerCiphers {
419+
t.Errorf("Expected ServerSSLPreferServerCiphers %v, got %v", tt.expected.ServerSSLPreferServerCiphers, result.ServerSSLPreferServerCiphers)
420+
}
421+
})
422+
}
423+
}
424+
425+
// TestSSLCipherAnnotationFiltering tests that SSL cipher annotations are filtered correctly for minions
426+
func TestSSLCipherAnnotationFiltering(t *testing.T) {
427+
t.Parallel()
428+
429+
tests := []struct {
430+
name string
431+
annotations map[string]string
432+
filterFunc func(map[string]string) []string
433+
expectedRemoved []string
434+
expectedAnnotations map[string]string
435+
}{
436+
{
437+
name: "SSL cipher annotations removed from minions",
438+
annotations: map[string]string{
439+
"nginx.org/ssl-ciphers": "HIGH:!aNULL:!MD5",
440+
"nginx.org/ssl-prefer-server-ciphers": "true",
441+
"nginx.org/proxy-connect-timeout": "30s",
442+
"nginx.org/server-snippets": "add_header X-Frame-Options SAMEORIGIN;",
443+
},
444+
filterFunc: filterMinionAnnotations,
445+
expectedRemoved: []string{
446+
"nginx.org/ssl-ciphers",
447+
"nginx.org/ssl-prefer-server-ciphers",
448+
"nginx.org/server-snippets",
449+
},
450+
expectedAnnotations: map[string]string{
451+
"nginx.org/proxy-connect-timeout": "30s",
452+
},
453+
},
454+
{
455+
name: "SSL cipher annotations allowed in masters",
456+
annotations: map[string]string{
457+
"nginx.org/ssl-ciphers": "HIGH:!aNULL:!MD5",
458+
"nginx.org/ssl-prefer-server-ciphers": "true",
459+
"nginx.org/rewrites": "serviceName=test rewrite=/",
460+
"nginx.org/proxy-connect-timeout": "30s",
461+
},
462+
filterFunc: filterMasterAnnotations,
463+
expectedRemoved: []string{
464+
"nginx.org/rewrites",
465+
},
466+
expectedAnnotations: map[string]string{
467+
"nginx.org/ssl-ciphers": "HIGH:!aNULL:!MD5",
468+
"nginx.org/ssl-prefer-server-ciphers": "true",
469+
"nginx.org/proxy-connect-timeout": "30s",
470+
},
471+
},
472+
}
473+
474+
for _, tt := range tests {
475+
t.Run(tt.name, func(t *testing.T) {
476+
// Make a copy of annotations to avoid modifying the test data
477+
annotations := make(map[string]string)
478+
for k, v := range tt.annotations {
479+
annotations[k] = v
480+
}
481+
482+
removedAnnotations := tt.filterFunc(annotations)
483+
484+
// Sort slices for comparison
485+
sort.Strings(removedAnnotations)
486+
sort.Strings(tt.expectedRemoved)
487+
488+
if !reflect.DeepEqual(removedAnnotations, tt.expectedRemoved) {
489+
t.Errorf("Expected removed annotations %v, got %v", tt.expectedRemoved, removedAnnotations)
490+
}
491+
492+
if !reflect.DeepEqual(annotations, tt.expectedAnnotations) {
493+
t.Errorf("Expected remaining annotations %v, got %v", tt.expectedAnnotations, annotations)
494+
}
495+
})
496+
}
497+
}
498+
499+
// TestSSLCipherAnnotationBooleanValues tests both valid and invalid boolean values for ssl-prefer-server-ciphers
500+
func TestSSLCipherAnnotationBooleanValues(t *testing.T) {
501+
t.Parallel()
502+
503+
testCases := []struct {
504+
value string
505+
expected bool
506+
isValid bool
507+
}{
508+
// Valid boolean values
509+
{"true", true, true},
510+
{"TRUE", true, true},
511+
{"True", true, true},
512+
{"1", true, true},
513+
{"false", false, true},
514+
{"FALSE", false, true},
515+
{"False", false, true},
516+
{"0", false, true},
517+
// Invalid boolean values (should default to false)
518+
{"invalid", false, false},
519+
{"yes", false, false},
520+
{"no", false, false},
521+
{"2", false, false},
522+
{"", false, false},
523+
{"maybe", false, false},
524+
{"on", false, false},
525+
}
526+
527+
for _, tc := range testCases {
528+
t.Run(tc.value, func(t *testing.T) {
529+
ingress := &networking.Ingress{
530+
ObjectMeta: metav1.ObjectMeta{
531+
Name: "test-ingress",
532+
Namespace: "default",
533+
Annotations: map[string]string{
534+
"nginx.org/ssl-prefer-server-ciphers": tc.value,
535+
},
536+
},
537+
}
538+
539+
ingEx := &IngressEx{
540+
Ingress: ingress,
541+
}
542+
543+
baseCfgParams := NewDefaultConfigParams(context.Background(), false)
544+
result := parseAnnotations(ingEx, baseCfgParams, false, false, false, false, false)
545+
546+
if result.ServerSSLPreferServerCiphers != tc.expected {
547+
validityMsg := "valid"
548+
if !tc.isValid {
549+
validityMsg = "invalid"
550+
}
551+
t.Errorf("Expected ServerSSLPreferServerCiphers to be %v for %s value %q, got %v", tc.expected, validityMsg, tc.value, result.ServerSSLPreferServerCiphers)
552+
}
553+
})
554+
}
555+
}

‎internal/configs/config_params.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ type ConfigParams struct {
8787
ResolverValid string
8888
ServerSnippets []string
8989
ServerTokens string
90+
ServerSSLCiphers string
91+
ServerSSLPreferServerCiphers bool
9092
SlowStart string
9193
SSLRedirect bool
9294
UpstreamZoneSize string

‎internal/configs/ingress.go‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -151,30 +151,32 @@ func generateNginxCfg(p NginxCfgParams) (version1.IngressNginxConfig, Warnings)
151151
statusZone := rule.Host
152152

153153
server := version1.Server{
154-
Name: serverName,
155-
ServerTokens: cfgParams.ServerTokens,
156-
HTTP2: cfgParams.HTTP2,
157-
RedirectToHTTPS: cfgParams.RedirectToHTTPS,
158-
SSLRedirect: cfgParams.SSLRedirect,
159-
ProxyProtocol: cfgParams.ProxyProtocol,
160-
HSTS: cfgParams.HSTS,
161-
HSTSMaxAge: cfgParams.HSTSMaxAge,
162-
HSTSIncludeSubdomains: cfgParams.HSTSIncludeSubdomains,
163-
HSTSBehindProxy: cfgParams.HSTSBehindProxy,
164-
StatusZone: statusZone,
165-
RealIPHeader: cfgParams.RealIPHeader,
166-
SetRealIPFrom: cfgParams.SetRealIPFrom,
167-
RealIPRecursive: cfgParams.RealIPRecursive,
168-
ProxyHideHeaders: cfgParams.ProxyHideHeaders,
169-
ProxyPassHeaders: cfgParams.ProxyPassHeaders,
170-
ServerSnippets: cfgParams.ServerSnippets,
171-
Ports: cfgParams.Ports,
172-
SSLPorts: cfgParams.SSLPorts,
173-
TLSPassthrough: p.staticParams.TLSPassthrough,
174-
AppProtectEnable: cfgParams.AppProtectEnable,
175-
AppProtectLogEnable: cfgParams.AppProtectLogEnable,
176-
SpiffeCerts: cfgParams.SpiffeServerCerts,
177-
DisableIPV6: p.staticParams.DisableIPV6,
154+
Name: serverName,
155+
ServerTokens: cfgParams.ServerTokens,
156+
HTTP2: cfgParams.HTTP2,
157+
RedirectToHTTPS: cfgParams.RedirectToHTTPS,
158+
SSLRedirect: cfgParams.SSLRedirect,
159+
SSLCiphers: cfgParams.ServerSSLCiphers,
160+
SSLPreferServerCiphers: cfgParams.ServerSSLPreferServerCiphers,
161+
ProxyProtocol: cfgParams.ProxyProtocol,
162+
HSTS: cfgParams.HSTS,
163+
HSTSMaxAge: cfgParams.HSTSMaxAge,
164+
HSTSIncludeSubdomains: cfgParams.HSTSIncludeSubdomains,
165+
HSTSBehindProxy: cfgParams.HSTSBehindProxy,
166+
StatusZone: statusZone,
167+
RealIPHeader: cfgParams.RealIPHeader,
168+
SetRealIPFrom: cfgParams.SetRealIPFrom,
169+
RealIPRecursive: cfgParams.RealIPRecursive,
170+
ProxyHideHeaders: cfgParams.ProxyHideHeaders,
171+
ProxyPassHeaders: cfgParams.ProxyPassHeaders,
172+
ServerSnippets: cfgParams.ServerSnippets,
173+
Ports: cfgParams.Ports,
174+
SSLPorts: cfgParams.SSLPorts,
175+
TLSPassthrough: p.staticParams.TLSPassthrough,
176+
AppProtectEnable: cfgParams.AppProtectEnable,
177+
AppProtectLogEnable: cfgParams.AppProtectLogEnable,
178+
SpiffeCerts: cfgParams.SpiffeServerCerts,
179+
DisableIPV6: p.staticParams.DisableIPV6,
178180
}
179181

180182
warnings := addSSLConfig(&server, p.ingEx.Ingress, rule.Host, p.ingEx.Ingress.Spec.TLS, p.ingEx.SecretRefs, p.isWildcardEnabled)

0 commit comments

Comments
 (0)