diff --git a/pkg/ingress/kube/annotations/downstreamtls.go b/pkg/ingress/kube/annotations/downstreamtls.go index 32a66897cc..e992e61012 100644 --- a/pkg/ingress/kube/annotations/downstreamtls.go +++ b/pkg/ingress/kube/annotations/downstreamtls.go @@ -16,6 +16,7 @@ package annotations import ( "strings" + "fmt" networking "istio.io/api/networking/v1alpha3" gatewaytool "istio.io/istio/pkg/config/gateway" @@ -27,9 +28,11 @@ import ( ) const ( - authTLSSecret = "auth-tls-secret" - sslCipher = "ssl-cipher" - gatewaySdsCaSuffix = "-cacert" + authTLSSecret = "auth-tls-secret" + sslCipher = "ssl-cipher" + gatewaySdsCaSuffix = "-cacert" + annotationMinTLSVersion = "tls-min-protocol-version" + annotationMaxTLSVersion = "tls-max-protocol-version" ) var ( @@ -41,6 +44,8 @@ type DownstreamTLSConfig struct { CipherSuites []string Mode networking.ServerTLSSettings_TLSmode CASecretName types.NamespacedName + MinVersion string + MaxVersion string } type downstreamTLS struct{} @@ -81,6 +86,14 @@ func (d downstreamTLS) Parse(annotations Annotations, config *Ingress, _ *Global downstreamTLSConfig.CipherSuites = validCipherSuite } + + if minVersion, err := annotations.ParseStringASAP(annotationMinTLSVersion); err == nil { + downstreamTLSConfig.MinVersion = minVersion + } + + if maxVersion, err := annotations.ParseStringASAP(annotationMaxTLSVersion); err == nil { + downstreamTLSConfig.MaxVersion = maxVersion + } return nil } @@ -107,11 +120,45 @@ func (d downstreamTLS) ApplyGateway(gateway *networking.Gateway, config *Ingress if len(downstreamTLSConfig.CipherSuites) != 0 { server.Tls.CipherSuites = downstreamTLSConfig.CipherSuites } + + if downstreamTLSConfig.MinVersion != "" { + if version, err := convertTLSVersion(downstreamTLSConfig.MinVersion); err != nil { + IngressLog.Errorf("Invalid minimum TLS version: %v", err) + } else { + server.Tls.MinProtocolVersion = version + } + } + + if downstreamTLSConfig.MaxVersion != "" { + if version, err := convertTLSVersion(downstreamTLSConfig.MaxVersion); err != nil { + IngressLog.Errorf("Invalid maximum TLS version: %v", err) + } else { + server.Tls.MaxProtocolVersion = version + } + } + } } } func needDownstreamTLS(annotations Annotations) bool { return annotations.HasASAP(sslCipher) || - annotations.HasASAP(authTLSSecret) + annotations.HasASAP(authTLSSecret)|| + annotations.HasASAP(annotationMinTLSVersion) || + annotations.HasASAP(annotationMaxTLSVersion) +} + +func convertTLSVersion(version string) (networking.ServerTLSSettings_TLSProtocol, error) { + switch version { + case "TLSv1.0": + return networking.ServerTLSSettings_TLSV1_0 , nil + case "TLSv1.1": + return networking.ServerTLSSettings_TLSV1_1 , nil + case "TLSv1.2": + return networking.ServerTLSSettings_TLSV1_2 , nil + case "TLSv1.3": + default: + return networking.ServerTLSSettings_TLS_AUTO, fmt.Errorf("invalid TLS version: %s. Valid values are: TLSv1.0, TLSv1.1, TLSv1.2, TLSv1.3", version) + } + return networking.ServerTLSSettings_TLS_AUTO, fmt.Errorf("unreachable code, but required by compiler") } diff --git a/pkg/ingress/kube/annotations/downstreamtls_test.go b/pkg/ingress/kube/annotations/downstreamtls_test.go index 5aeba194a0..1bf36517ae 100644 --- a/pkg/ingress/kube/annotations/downstreamtls_test.go +++ b/pkg/ingress/kube/annotations/downstreamtls_test.go @@ -26,11 +26,15 @@ var parser = downstreamTLS{} func TestParse(t *testing.T) { testCases := []struct { + name string input map[string]string expect *DownstreamTLSConfig }{ - {}, { + name: "empty config", + }, + { + name: "ssl cipher only", input: map[string]string{ buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", }, @@ -40,45 +44,105 @@ func TestParse(t *testing.T) { }, }, { + name: "with TLS version config", input: map[string]string{ - buildNginxAnnotationKey(authTLSSecret): "test", - buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1.2", + buildNginxAnnotationKey(annotationMaxTLSVersion): "TLSv1.3", }, expect: &DownstreamTLSConfig{ - CASecretName: types.NamespacedName{ - Namespace: "foo", - Name: "test", - }, - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384", "AES128-SHA"}, + Mode: networking.ServerTLSSettings_SIMPLE, + MinVersion: "TLSv1.2", + MaxVersion: "TLSv1.3", }, }, { + name: "complete config", input: map[string]string{ - buildHigressAnnotationKey(authTLSSecret): "test/foo", - DefaultAnnotationsPrefix + "/" + sslCipher: "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", + buildNginxAnnotationKey(authTLSSecret): "test", + buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384:AES128-SHA", + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1.2", + buildNginxAnnotationKey(annotationMaxTLSVersion): "TLSv1.3", }, expect: &DownstreamTLSConfig{ CASecretName: types.NamespacedName{ - Namespace: "test", - Name: "foo", + Namespace: "foo", + Name: "test", }, Mode: networking.ServerTLSSettings_MUTUAL, CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384", "AES128-SHA"}, + MinVersion: "TLSv1.2", + MaxVersion: "TLSv1.3", }, }, } - for _, testCase := range testCases { - t.Run("", func(t *testing.T) { + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { config := &Ingress{ Meta: Meta{ Namespace: "foo", }, } - _ = parser.Parse(testCase.input, config, nil) - if !reflect.DeepEqual(testCase.expect, config.DownstreamTLS) { - t.Fatalf("Should be equal") + err := parser.Parse(tc.input, config, nil) + if err != nil { + t.Fatalf("Parse failed: %v", err) + } + if !reflect.DeepEqual(tc.expect, config.DownstreamTLS) { + t.Fatalf("Parse result mismatch:\nExpect: %+v\nGot: %+v", tc.expect, config.DownstreamTLS) + } + }) + } +} + +func TestConvertTLSVersion(t *testing.T) { + testCases := []struct { + name string + version string + expect networking.ServerTLSSettings_TLSProtocol + wantErr bool + }{ + { + name: "TLS 1.0", + version: "TLSv1.0", + expect: networking.ServerTLSSettings_TLSV1_0, + }, + { + name: "TLS 1.1", + version: "TLSv1.1", + expect: networking.ServerTLSSettings_TLSV1_1, + }, + { + name: "TLS 1.2", + version: "TLSv1.2", + expect: networking.ServerTLSSettings_TLSV1_2, + }, + { + name: "TLS 1.3", + version: "TLSv1.3", + expect: networking.ServerTLSSettings_TLSV1_3, + }, + { + name: "invalid version", + version: "invalid", + expect: networking.ServerTLSSettings_TLS_AUTO, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result, err := convertTLSVersion(tc.version) + if tc.wantErr { + if err == nil { + t.Error("Expected error but got none") + } + } else { + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if result != tc.expect { + t.Errorf("Expected %v but got %v", tc.expect, result) + } } }) } @@ -86,11 +150,13 @@ func TestParse(t *testing.T) { func TestApplyGateway(t *testing.T) { testCases := []struct { + name string input *networking.Gateway config *Ingress expect *networking.Gateway }{ { + name: "apply TLS version", input: &networking.Gateway{ Servers: []*networking.Server{ { @@ -105,7 +171,8 @@ func TestApplyGateway(t *testing.T) { }, config: &Ingress{ DownstreamTLS: &DownstreamTLSConfig{ - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + MinVersion: "TLSv1.2", + MaxVersion: "TLSv1.3", }, }, expect: &networking.Gateway{ @@ -115,14 +182,16 @@ func TestApplyGateway(t *testing.T) { Protocol: "HTTPS", }, Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + Mode: networking.ServerTLSSettings_SIMPLE, + MinProtocolVersion: networking.ServerTLSSettings_TLSV1_2, + MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_3, }, }, }, }, }, { + name: "complete config", input: &networking.Gateway{ Servers: []*networking.Server{ { @@ -144,24 +213,28 @@ func TestApplyGateway(t *testing.T) { }, Mode: networking.ServerTLSSettings_MUTUAL, CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + MinVersion: "TLSv1.2", + MaxVersion: "TLSv1.3", }, }, expect: &networking.Gateway{ Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", - }, + {Port: &networking.Port{ + Protocol: "HTTPS", + }, Tls: &networking.ServerTLSSettings{ - CredentialName: "kubernetes-ingress://cluster/foo/bar", - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + CredentialName: "kubernetes-ingress://cluster/foo/bar", + Mode: networking.ServerTLSSettings_MUTUAL, + CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + MinProtocolVersion: networking.ServerTLSSettings_TLSV1_2, + MaxProtocolVersion: networking.ServerTLSSettings_TLSV1_3, }, }, }, }, }, { + name: "invalid TLS version", input: &networking.Gateway{ Servers: []*networking.Server{ { @@ -169,20 +242,15 @@ func TestApplyGateway(t *testing.T) { Protocol: "HTTPS", }, Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - CredentialName: "kubernetes-ingress://cluster/foo/bar", + Mode: networking.ServerTLSSettings_SIMPLE, }, }, }, }, config: &Ingress{ DownstreamTLS: &DownstreamTLSConfig{ - CASecretName: types.NamespacedName{ - Namespace: "foo", - Name: "bar-cacert", - }, - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + MinVersion: "invalid", + MaxVersion: "invalid", }, }, expect: &networking.Gateway{ @@ -192,60 +260,70 @@ func TestApplyGateway(t *testing.T) { Protocol: "HTTPS", }, Tls: &networking.ServerTLSSettings{ - CredentialName: "kubernetes-ingress://cluster/foo/bar", - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, + Mode: networking.ServerTLSSettings_SIMPLE, + // Invalid versions should default to TLS_AUTO + MinProtocolVersion: networking.ServerTLSSettings_TLS_AUTO, + MaxProtocolVersion: networking.ServerTLSSettings_TLS_AUTO, }, }, }, }, }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + parser.ApplyGateway(tc.input, tc.config) + if !reflect.DeepEqual(tc.input, tc.expect) { + t.Fatalf("ApplyGateway result mismatch for %s:\nExpect: %+v\nGot: %+v", + tc.name, tc.expect, tc.input) + } + }) + } +} + +func TestNeedDownstreamTLS(t *testing.T) { + testCases := []struct { + name string + annotations map[string]string + expect bool + }{ { - input: &networking.Gateway{ - Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - Mode: networking.ServerTLSSettings_SIMPLE, - CredentialName: "kubernetes-ingress://cluster/foo/bar", - }, - }, - }, + name: "empty annotations", + annotations: map[string]string{}, + expect: false, + }, + { + name: "with ssl cipher", + annotations: map[string]string{ + buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384", }, - config: &Ingress{ - DownstreamTLS: &DownstreamTLSConfig{ - CASecretName: types.NamespacedName{ - Namespace: "bar", - Name: "foo", - }, - Mode: networking.ServerTLSSettings_MUTUAL, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, - }, + expect: true, + }, + { + name: "with TLS version", + annotations: map[string]string{ + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1.2", }, - expect: &networking.Gateway{ - Servers: []*networking.Server{ - { - Port: &networking.Port{ - Protocol: "HTTPS", - }, - Tls: &networking.ServerTLSSettings{ - CredentialName: "kubernetes-ingress://cluster/foo/bar", - Mode: networking.ServerTLSSettings_SIMPLE, - CipherSuites: []string{"ECDHE-RSA-AES256-GCM-SHA384"}, - }, - }, - }, + expect: true, + }, + { + name: "with multiple TLS configs", + annotations: map[string]string{ + buildNginxAnnotationKey(sslCipher): "ECDHE-RSA-AES256-GCM-SHA384", + buildNginxAnnotationKey(annotationMinTLSVersion): "TLSv1.2", + buildNginxAnnotationKey(annotationMaxTLSVersion): "TLSv1.3", }, + expect: true, }, } - for _, testCase := range testCases { - t.Run("", func(t *testing.T) { - parser.ApplyGateway(testCase.input, testCase.config) - if !reflect.DeepEqual(testCase.input, testCase.expect) { - t.Fatalf("Should be equal") + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := needDownstreamTLS(tc.annotations) + if result != tc.expect { + t.Errorf("needDownstreamTLS() for %s = %v, want %v", + tc.name, result, tc.expect) } }) }