Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: match SNI when using TLS listeners with hostname #2942

Merged
merged 8 commits into from
May 10, 2024
2 changes: 1 addition & 1 deletion internal/gatewayapi/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR XdsIRMap
// Infra IR proxy ports must be unique.
foundPorts := make(map[string][]*protocolPort)
t.validateConflictedLayer7Listeners(gateways)
t.validateConflictedLayer4Listeners(gateways, gwapiv1.TCPProtocolType, gwapiv1.TLSProtocolType)
t.validateConflictedLayer4Listeners(gateways, gwapiv1.TCPProtocolType)
t.validateConflictedLayer4Listeners(gateways, gwapiv1.UDPProtocolType)
if t.MergeGateways {
t.validateConflictedMergedListeners(gateways)
Expand Down
12 changes: 10 additions & 2 deletions internal/gatewayapi/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour
if irListener != nil {
irRoute := &ir.TCPRoute{
Name: irTCPRouteName(tlsRoute),
TLS: &ir.TLS{Passthrough: &ir.TLSInspectorConfig{
TLS: &ir.TLS{TLSInspectorConfig: &ir.TLSInspectorConfig{
SNIs: hosts,
}},
Destination: &ir.RouteDestination{
Expand Down Expand Up @@ -1082,6 +1082,14 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
accepted = true
irKey := t.getIRKey(listener.gateway)

tls := ir.TLS{
Copy link
Contributor

@arkodg arkodg May 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnt this be executed conditionally ?
e.g.

if listener.TLS != nil {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anything's changed here, previously we set unconditionally:

TLS: &ir.TLS{Terminate: irTLSConfigs(listener.tlsSecrets)},

irTLSConfigs returns nil if there aren't any secrets. The line you linked to is also gated by checking the listener protocol.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool thanks for the checking!

Terminate: irTLSConfigs(listener.tlsSecrets),
}
if listener.Hostname != nil {
tls.TLSInspectorConfig = &ir.TLSInspectorConfig{
SNIs: []string{string(*listener.Hostname)},
}
}
gwXdsIR := xdsIR[irKey]
irListener := gwXdsIR.GetTCPListener(irListenerName(listener))
if irListener != nil {
Expand All @@ -1091,7 +1099,7 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
Name: irRouteDestinationName(tcpRoute, -1 /*rule index*/),
Settings: destSettings,
},
TLS: &ir.TLS{Terminate: irTLSConfigs(listener.tlsSecrets)},
TLS: &tls,
}
irListener.Routes = append(irListener.Routes, irRoute)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,6 @@ xdsIR:
weight: 1
name: tlsroute/default/tlsroute-1
tls:
passthrough:
inspector:
snis:
- foo.bar.com
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ gateways:
kind: TCPRoute
- attachedRoutes: 1
conditions:
- lastTransitionTime: null
message: Only one TCP/TLS listener is allowed in a given port
reason: ProtocolConflict
status: "True"
type: Conflicted
- lastTransitionTime: null
message: Listener must have TLS set when protocol is TLS.
reason: Invalid
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@ gateways:
allowedRoutes:
namespaces:
from: All
- name: tls-hostname
hostname: "foo.bar.com"
protocol: TLS
port: 90
tls:
certificateRefs:
- group: ""
kind: Secret
name: tls-secret-1
mode: Terminate
allowedRoutes:
namespaces:
from: All
tcpRoutes:
- apiVersion: gateway.networking.k8s.io/v1alpha2
kind: TCPRoute
Expand All @@ -29,10 +42,25 @@ tcpRoutes:
parentRefs:
- namespace: envoy-gateway
name: gateway-1
sectionName: tls
rules:
- backendRefs:
- name: service-1
port: 8080
- apiVersion: gateway.networking.k8s.io/v1alpha2
kind: TCPRoute
metadata:
namespace: default
name: tcproute-2
spec:
parentRefs:
- namespace: envoy-gateway
name: gateway-1
sectionName: tls-hostname
rules:
- backendRefs:
- name: service-2
port: 8080

secrets:
- apiVersion: v1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@ gateways:
kind: Secret
name: tls-secret-1
mode: Terminate
- allowedRoutes:
namespaces:
from: All
hostname: foo.bar.com
name: tls-hostname
port: 90
protocol: TLS
tls:
certificateRefs:
- group: ""
kind: Secret
name: tls-secret-1
mode: Terminate
status:
listeners:
- attachedRoutes: 1
Expand All @@ -43,6 +56,27 @@ gateways:
supportedKinds:
- group: gateway.networking.k8s.io
kind: TCPRoute
- attachedRoutes: 1
conditions:
- lastTransitionTime: null
message: Sending translated listener configuration to the data plane
reason: Programmed
status: "True"
type: Programmed
- lastTransitionTime: null
message: Listener has been successfully translated
reason: Accepted
status: "True"
type: Accepted
- lastTransitionTime: null
message: Listener references have been resolved
reason: ResolvedRefs
status: "True"
type: ResolvedRefs
name: tls-hostname
supportedKinds:
- group: gateway.networking.k8s.io
kind: TCPRoute
infraIR:
envoy-gateway/gateway-1:
proxy:
Expand Down Expand Up @@ -70,6 +104,7 @@ tcpRoutes:
parentRefs:
- name: gateway-1
namespace: envoy-gateway
sectionName: tls
rules:
- backendRefs:
- name: service-1
Expand All @@ -91,6 +126,40 @@ tcpRoutes:
parentRef:
name: gateway-1
namespace: envoy-gateway
sectionName: tls
- apiVersion: gateway.networking.k8s.io/v1alpha2
kind: TCPRoute
metadata:
creationTimestamp: null
name: tcproute-2
namespace: default
spec:
parentRefs:
- name: gateway-1
namespace: envoy-gateway
sectionName: tls-hostname
rules:
- backendRefs:
- name: service-2
port: 8080
status:
parents:
- conditions:
- lastTransitionTime: null
message: Route is accepted
reason: Accepted
status: "True"
type: Accepted
- lastTransitionTime: null
message: Resolved all the Object references for the Route
reason: ResolvedRefs
status: "True"
type: ResolvedRefs
controllerName: gateway.envoyproxy.io/gatewayclass-controller
parentRef:
name: gateway-1
namespace: envoy-gateway
sectionName: tls-hostname
xdsIR:
envoy-gateway/gateway-1:
accessLog:
Expand All @@ -117,3 +186,26 @@ xdsIR:
- name: envoy-gateway/tls-secret-1
privateKey: LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUV2UUlCQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQktjd2dnU2pBZ0VBQW9JQkFRQ2QwZlBDYWtweE1nUnUKT0VXQjFiQk5FM3ZseW55aTZWbkV2VWF1OUhvakR2UHVPTFJIaGI4MmoyY1ovMHhnL1lKR09LelBuV2JERkxGNApHdWh3dDRENmFUR0xYNklPODEwTDZ0SXZIWGZNUXRJS2VwdTZ3K3p1WVo4bG1yejB1RjZlWEtqamVIbHhyb2ZrCnVNekM3OUVaU0lYZlZlczJ1SmdVRSs4VGFzSDUzQ2Y4MFNSRGlIeEdxckttdVNjWCtwejBreGdCZ1VWYTVVS20KUWdTZDFmVUxLOUEwNXAxOXkrdURPM204bVhRNkxVQ0N1STFwZHNROGFlNS9zamlxa0VjWlJjMTdWYVgxWjVVaQpvcGZnNW9SY05VTG9VTHNiek9aNTR0YlVDUmdSV2VLbGZxaElINEZ6OUlkVlUyR3dFdEdhMmV6TjgyMVBaQ3QzCjZhbVRIelJsQWdNQkFBRUNnZ0VBWTFGTUlLNDVXTkVNUHJ6RTZUY3NNdVV2RkdhQVZ4bVk5NW5SMEtwajdvb3IKY21CVys2ZXN0TTQ4S1AwaitPbXd3VFpMY29Cd3VoWGN0V1Bob1lXcDhteWUxRUlEdjNyaHRHMDdocEQ1NGg2dgpCZzh3ejdFYStzMk9sT0N6UnlKNzBSY281YlhjWDNGaGJjdnFlRWJwaFFyQnpOSEtLMjZ4cmZqNWZIT3p6T1FGCmJHdUZ3SDVic3JGdFhlajJXM3c4eW90N0ZQSDV3S3RpdnhvSWU5RjMyOXNnOU9EQnZqWnpiaG1LVTArckFTK1kKRGVield2bFJyaEUrbXVmQTN6M0N0QXhDOFJpNzNscFNoTDRQQWlvcG1SUXlxZXRXMjYzOFFxcnM0R3hnNzhwbApJUXJXTmNBc2s3Slg5d3RZenV6UFBXSXRWTTFscFJiQVRhNTJqdFl2NVFLQmdRRE5tMTFtZTRYam1ZSFV2cStZCmFTUzdwK2UybXZEMHVaOU9JeFluQnBWMGkrckNlYnFFMkE1Rm5hcDQ5Yld4QTgwUElldlVkeUpCL2pUUkoxcVMKRUpXQkpMWm1LVkg2K1QwdWw1ZUtOcWxFTFZHU0dCSXNpeE9SUXpDZHBoMkx0UmtBMHVjSVUzY3hiUmVMZkZCRQpiSkdZWENCdlNGcWd0VDlvZTFldVpMVmFOd0tCZ1FERWdENzJENk81eGIweEQ1NDQ1M0RPMUJhZmd6aThCWDRTCk1SaVd2LzFUQ0w5N05sRWtoeXovNmtQd1owbXJRcE5CMzZFdkpKZFVteHdkU2MyWDhrOGcxMC85NVlLQkdWQWoKL3d0YVZYbE9WeEFvK0ZSelpZeFpyQ29uWWFSMHVwUzFybDRtenN4REhlZU9mUVZUTUgwUjdZN0pnbTA5dXQ4SwplanAvSXZBb1F3S0JnQjNaRWlRUWhvMVYrWjBTMlpiOG5KS0plMy9zMmxJTXFHM0ZkaS9RS3Q0eWViQWx6OGY5ClBZVXBzRmZEQTg5Z3grSU1nSm5sZVptdTk2ZnRXSjZmdmJSenllN216TG5zZU05TXZua1lHbGFGWmJRWnZubXMKN3ZoRmtzY3dHRlh4d21GMlBJZmU1Z3pNMDRBeVdjeTFIaVhLS2dNOXM3cGsxWUdyZGowZzdacmRBb0dCQUtLNApDR3MrbkRmMEZTMFJYOWFEWVJrRTdBNy9YUFhtSG5YMkRnU1h5N0Q4NTRPaWdTTWNoUmtPNTErbVNJejNQbllvCk41T1FXM2lHVVl1M1YvYmhnc0VSUzM1V2xmRk9BdDBzRUR5bjF5SVdXcDF5dG93d3BUNkVvUXVuZ2NYZjA5RjMKS1NROXowd3M4VmsvRWkvSFVXcU5LOWFXbU51cmFaT0ZqL2REK1ZkOUFvR0FMWFN3dEE3K043RDRkN0VEMURSRQpHTWdZNVd3OHFvdDZSdUNlNkpUY0FnU3B1MkhNU3JVY2dXclpiQnJZb09FUnVNQjFoMVJydk5ybU1qQlM0VW9FClgyZC8vbGhpOG1wL2VESWN3UDNRa2puanBJRFJWMFN1eWxrUkVaZURKZjVZb3R6eDdFdkJhbzFIbkQrWEg4eUIKVUtmWGJTaHZKVUdhRmgxT3Q1Y3JoM1k9Ci0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0K
serverCertificate: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUREVENDQWZXZ0F3SUJBZ0lVRUZNaFA5ZUo5WEFCV3NRNVptNmJSazJjTE5Rd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0ZqRVVNQklHQTFVRUF3d0xabTl2TG1KaGNpNWpiMjB3SGhjTk1qUXdNakk1TURrek1ERXdXaGNOTXpRdwpNakkyTURrek1ERXdXakFXTVJRd0VnWURWUVFEREF0bWIyOHVZbUZ5TG1OdmJUQ0NBU0l3RFFZSktvWklodmNOCkFRRUJCUUFEZ2dFUEFEQ0NBUW9DZ2dFQkFKbEk2WXhFOVprQ1BzNnBDUXhickNtZWl4OVA1RGZ4OVJ1NUxENFQKSm1kVzdJS2R0UVYvd2ZMbXRzdTc2QithVGRDaldlMEJUZmVPT1JCYlIzY1BBRzZFbFFMaWNsUVVydW4zcStncwpKcEsrSTdjSStqNXc4STY4WEg1V1E3clZVdGJ3SHBxYncrY1ZuQnFJVU9MaUlhdGpJZjdLWDUxTTF1RjljZkVICkU0RG5jSDZyYnI1OS9SRlpCc2toeHM1T3p3Sklmb2hreXZGd2V1VHd4Sy9WcGpJKzdPYzQ4QUJDWHBOTzlEL3EKRWgrck9hdWpBTWNYZ0hRSVRrQ2lpVVRjVW82TFNIOXZMWlB0YXFmem9acTZuaE1xcFc2NUUxcEF3RjNqeVRUeAphNUk4SmNmU0Zqa2llWjIwTFVRTW43TThVNHhIamFvL2d2SDBDQWZkQjdSTFUyc0NBd0VBQWFOVE1GRXdIUVlEClZSME9CQllFRk9SQ0U4dS8xRERXN2loWnA3Y3g5dFNtUG02T01COEdBMVVkSXdRWU1CYUFGT1JDRTh1LzFERFcKN2loWnA3Y3g5dFNtUG02T01BOEdBMVVkRXdFQi93UUZNQU1CQWY4d0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQgpBRnQ1M3pqc3FUYUg1YThFMmNodm1XQWdDcnhSSzhiVkxNeGl3TkdqYm1FUFJ6K3c2TngrazBBOEtFY0lEc0tjClNYY2k1OHU0b1didFZKQmx6YS9adWpIUjZQMUJuT3BsK2FveTc4NGJiZDRQMzl3VExvWGZNZmJCQ20xdmV2aDkKQUpLbncyWnRxcjRta2JMY3hFcWxxM3NCTEZBUzlzUUxuS05DZTJjR0xkVHAyYm9HK3FjZ3lRZ0NJTTZmOEVNdgpXUGlmQ01NR3V6Sy9HUkY0YlBPL1lGNDhld0R1M1VlaWgwWFhkVUFPRTlDdFVhOE5JaGMxVVBhT3pQcnRZVnFyClpPR2t2L0t1K0I3OGg4U0VzTzlYclFjdXdiT25KeDZLdFIrYWV5a3ZBcFhDUTNmWkMvYllLQUFSK1A4QUpvUVoKYndJVW1YaTRnajVtK2JLUGhlK2lyK0U9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0=
- address: 0.0.0.0
name: envoy-gateway/gateway-1/tls-hostname
port: 10090
routes:
- destination:
name: tcproute/default/tcproute-2/rule/-1
settings:
- addressType: IP
endpoints:
- host: 7.7.7.7
port: 8080
protocol: TCP
weight: 1
name: tcproute/default/tcproute-2
tls:
inspector:
snis:
- foo.bar.com
terminate:
certificates:
- name: envoy-gateway/tls-secret-1
privateKey: LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUV2UUlCQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQktjd2dnU2pBZ0VBQW9JQkFRQ2QwZlBDYWtweE1nUnUKT0VXQjFiQk5FM3ZseW55aTZWbkV2VWF1OUhvakR2UHVPTFJIaGI4MmoyY1ovMHhnL1lKR09LelBuV2JERkxGNApHdWh3dDRENmFUR0xYNklPODEwTDZ0SXZIWGZNUXRJS2VwdTZ3K3p1WVo4bG1yejB1RjZlWEtqamVIbHhyb2ZrCnVNekM3OUVaU0lYZlZlczJ1SmdVRSs4VGFzSDUzQ2Y4MFNSRGlIeEdxckttdVNjWCtwejBreGdCZ1VWYTVVS20KUWdTZDFmVUxLOUEwNXAxOXkrdURPM204bVhRNkxVQ0N1STFwZHNROGFlNS9zamlxa0VjWlJjMTdWYVgxWjVVaQpvcGZnNW9SY05VTG9VTHNiek9aNTR0YlVDUmdSV2VLbGZxaElINEZ6OUlkVlUyR3dFdEdhMmV6TjgyMVBaQ3QzCjZhbVRIelJsQWdNQkFBRUNnZ0VBWTFGTUlLNDVXTkVNUHJ6RTZUY3NNdVV2RkdhQVZ4bVk5NW5SMEtwajdvb3IKY21CVys2ZXN0TTQ4S1AwaitPbXd3VFpMY29Cd3VoWGN0V1Bob1lXcDhteWUxRUlEdjNyaHRHMDdocEQ1NGg2dgpCZzh3ejdFYStzMk9sT0N6UnlKNzBSY281YlhjWDNGaGJjdnFlRWJwaFFyQnpOSEtLMjZ4cmZqNWZIT3p6T1FGCmJHdUZ3SDVic3JGdFhlajJXM3c4eW90N0ZQSDV3S3RpdnhvSWU5RjMyOXNnOU9EQnZqWnpiaG1LVTArckFTK1kKRGVield2bFJyaEUrbXVmQTN6M0N0QXhDOFJpNzNscFNoTDRQQWlvcG1SUXlxZXRXMjYzOFFxcnM0R3hnNzhwbApJUXJXTmNBc2s3Slg5d3RZenV6UFBXSXRWTTFscFJiQVRhNTJqdFl2NVFLQmdRRE5tMTFtZTRYam1ZSFV2cStZCmFTUzdwK2UybXZEMHVaOU9JeFluQnBWMGkrckNlYnFFMkE1Rm5hcDQ5Yld4QTgwUElldlVkeUpCL2pUUkoxcVMKRUpXQkpMWm1LVkg2K1QwdWw1ZUtOcWxFTFZHU0dCSXNpeE9SUXpDZHBoMkx0UmtBMHVjSVUzY3hiUmVMZkZCRQpiSkdZWENCdlNGcWd0VDlvZTFldVpMVmFOd0tCZ1FERWdENzJENk81eGIweEQ1NDQ1M0RPMUJhZmd6aThCWDRTCk1SaVd2LzFUQ0w5N05sRWtoeXovNmtQd1owbXJRcE5CMzZFdkpKZFVteHdkU2MyWDhrOGcxMC85NVlLQkdWQWoKL3d0YVZYbE9WeEFvK0ZSelpZeFpyQ29uWWFSMHVwUzFybDRtenN4REhlZU9mUVZUTUgwUjdZN0pnbTA5dXQ4SwplanAvSXZBb1F3S0JnQjNaRWlRUWhvMVYrWjBTMlpiOG5KS0plMy9zMmxJTXFHM0ZkaS9RS3Q0eWViQWx6OGY5ClBZVXBzRmZEQTg5Z3grSU1nSm5sZVptdTk2ZnRXSjZmdmJSenllN216TG5zZU05TXZua1lHbGFGWmJRWnZubXMKN3ZoRmtzY3dHRlh4d21GMlBJZmU1Z3pNMDRBeVdjeTFIaVhLS2dNOXM3cGsxWUdyZGowZzdacmRBb0dCQUtLNApDR3MrbkRmMEZTMFJYOWFEWVJrRTdBNy9YUFhtSG5YMkRnU1h5N0Q4NTRPaWdTTWNoUmtPNTErbVNJejNQbllvCk41T1FXM2lHVVl1M1YvYmhnc0VSUzM1V2xmRk9BdDBzRUR5bjF5SVdXcDF5dG93d3BUNkVvUXVuZ2NYZjA5RjMKS1NROXowd3M4VmsvRWkvSFVXcU5LOWFXbU51cmFaT0ZqL2REK1ZkOUFvR0FMWFN3dEE3K043RDRkN0VEMURSRQpHTWdZNVd3OHFvdDZSdUNlNkpUY0FnU3B1MkhNU3JVY2dXclpiQnJZb09FUnVNQjFoMVJydk5ybU1qQlM0VW9FClgyZC8vbGhpOG1wL2VESWN3UDNRa2puanBJRFJWMFN1eWxrUkVaZURKZjVZb3R6eDdFdkJhbzFIbkQrWEg4eUIKVUtmWGJTaHZKVUdhRmgxT3Q1Y3JoM1k9Ci0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0K
serverCertificate: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUREVENDQWZXZ0F3SUJBZ0lVRUZNaFA5ZUo5WEFCV3NRNVptNmJSazJjTE5Rd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0ZqRVVNQklHQTFVRUF3d0xabTl2TG1KaGNpNWpiMjB3SGhjTk1qUXdNakk1TURrek1ERXdXaGNOTXpRdwpNakkyTURrek1ERXdXakFXTVJRd0VnWURWUVFEREF0bWIyOHVZbUZ5TG1OdmJUQ0NBU0l3RFFZSktvWklodmNOCkFRRUJCUUFEZ2dFUEFEQ0NBUW9DZ2dFQkFKbEk2WXhFOVprQ1BzNnBDUXhickNtZWl4OVA1RGZ4OVJ1NUxENFQKSm1kVzdJS2R0UVYvd2ZMbXRzdTc2QithVGRDaldlMEJUZmVPT1JCYlIzY1BBRzZFbFFMaWNsUVVydW4zcStncwpKcEsrSTdjSStqNXc4STY4WEg1V1E3clZVdGJ3SHBxYncrY1ZuQnFJVU9MaUlhdGpJZjdLWDUxTTF1RjljZkVICkU0RG5jSDZyYnI1OS9SRlpCc2toeHM1T3p3Sklmb2hreXZGd2V1VHd4Sy9WcGpJKzdPYzQ4QUJDWHBOTzlEL3EKRWgrck9hdWpBTWNYZ0hRSVRrQ2lpVVRjVW82TFNIOXZMWlB0YXFmem9acTZuaE1xcFc2NUUxcEF3RjNqeVRUeAphNUk4SmNmU0Zqa2llWjIwTFVRTW43TThVNHhIamFvL2d2SDBDQWZkQjdSTFUyc0NBd0VBQWFOVE1GRXdIUVlEClZSME9CQllFRk9SQ0U4dS8xRERXN2loWnA3Y3g5dFNtUG02T01COEdBMVVkSXdRWU1CYUFGT1JDRTh1LzFERFcKN2loWnA3Y3g5dFNtUG02T01BOEdBMVVkRXdFQi93UUZNQU1CQWY4d0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQgpBRnQ1M3pqc3FUYUg1YThFMmNodm1XQWdDcnhSSzhiVkxNeGl3TkdqYm1FUFJ6K3c2TngrazBBOEtFY0lEc0tjClNYY2k1OHU0b1didFZKQmx6YS9adWpIUjZQMUJuT3BsK2FveTc4NGJiZDRQMzl3VExvWGZNZmJCQ20xdmV2aDkKQUpLbncyWnRxcjRta2JMY3hFcWxxM3NCTEZBUzlzUUxuS05DZTJjR0xkVHAyYm9HK3FjZ3lRZ0NJTTZmOEVNdgpXUGlmQ01NR3V6Sy9HUkY0YlBPL1lGNDhld0R1M1VlaWgwWFhkVUFPRTlDdFVhOE5JaGMxVVBhT3pQcnRZVnFyClpPR2t2L0t1K0I3OGg4U0VzTzlYclFjdXdiT25KeDZLdFIrYWV5a3ZBcFhDUTNmWkMvYllLQUFSK1A4QUpvUVoKYndJVW1YaTRnajVtK2JLUGhlK2lyK0U9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0=
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,6 @@ xdsIR:
weight: 1
name: tlsroute/default/tlsroute-1
tls:
passthrough:
inspector:
snis:
- foo.com
4 changes: 2 additions & 2 deletions internal/gatewayapi/testdata/tlsroute-multiple.out.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ xdsIR:
weight: 1
name: tlsroute/default/tlsroute-1
tls:
passthrough:
inspector:
snis:
- foo.com
- destination:
Expand All @@ -157,6 +157,6 @@ xdsIR:
weight: 1
name: tlsroute/default/tlsroute-2
tls:
passthrough:
inspector:
snis:
- bar.com
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,6 @@ xdsIR:
weight: 1
name: tlsroute/default/tlsroute-1
tls:
passthrough:
inspector:
snis:
- foo.com
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,6 @@ xdsIR:
weight: 1
name: tlsroute/default/tlsroute-1
tls:
passthrough:
inspector:
snis:
- '*'
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,6 @@ xdsIR:
weight: 1
name: tlsroute/default/tlsroute-1
tls:
passthrough:
inspector:
snis:
- foo.com
9 changes: 4 additions & 5 deletions internal/ir/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -1215,7 +1215,7 @@ type TCPRoute struct {
type TLS struct {
// TLS information required for TLS Passthrough, If provided, incoming
// connections' server names are inspected and routed to backends accordingly.
Passthrough *TLSInspectorConfig `json:"passthrough,omitempty" yaml:"passthrough,omitempty"`
TLSInspectorConfig *TLSInspectorConfig `json:"inspector,omitempty" yaml:"inspector,omitempty"`
// TLS information required for TLS Termination
Terminate *TLSConfig `json:"terminate,omitempty" yaml:"terminate,omitempty"`
}
Expand Down Expand Up @@ -1246,8 +1246,8 @@ func (t TCPRoute) Validate() error {
errs = errors.Join(errs, ErrRouteNameEmpty)
}

if t.TLS != nil && t.TLS.Passthrough != nil {
if err := t.TLS.Passthrough.Validate(); err != nil {
if t.TLS != nil && t.TLS.TLSInspectorConfig != nil {
if err := t.TLS.TLSInspectorConfig.Validate(); err != nil {
errs = errors.Join(errs, err)
}
}
Expand Down Expand Up @@ -1280,13 +1280,12 @@ func (t TCPRoute) Validate() error {
}

// TLSInspectorConfig holds the configuration required for inspecting TLS
// passthrough connections.
// connections.
// +k8s:deepcopy-gen=true
type TLSInspectorConfig struct {
// Server names that are compared against the server names of a new connection.
// Wildcard hosts are supported in the prefix form. Partial wildcards are not
// supported, and values like *w.example.com are invalid.
// SNIs are used only in case of TLS Passthrough.
SNIs []string `json:"snis,omitempty" yaml:"snis,omitempty"`
Copy link
Member

@zhaohuabing zhaohuabing Mar 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving SNIs up to TLSConfig and removing TLSInspectorConfig?

TLS Inspector is an implementation detail of Envoy, so we probably don't need to expose it to ir.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's already exposed as TLS.Passthrough isn't it? I can add SNIs directly to TLSConfig if you prefer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd vote for making TLSInspectorConfig an embedded field within TLS

type TLS struct {
, removing the need for a dedicated Passthrough field

  • passthrough logic can be computed using - isTLSPassthrough := irListener.TLS != nil && irListener.TLS.Terminate == nil
  • this also eliminates the need to add Inspector into the TLSConfig field, which is common to HTTPS, which uses another field (listener.Hostname) for SNI inspection

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed an update here, it changes a different part of the IR though, passthrough -> inspector, since that's now common between mode: Terminate and mode: Passthrough

}

Expand Down
4 changes: 2 additions & 2 deletions internal/ir/xds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ var (
// TCPRoute
happyTCPRouteTLSPassthrough = TCPRoute{
Name: "happy-tls-passthrough",
TLS: &TLS{Passthrough: &TLSInspectorConfig{SNIs: []string{"example.com"}}},
TLS: &TLS{TLSInspectorConfig: &TLSInspectorConfig{SNIs: []string{"example.com"}}},
Destination: &happyRouteDestination,
}
happyTCPRouteTLSTermination = TCPRoute{
Expand All @@ -138,7 +138,7 @@ var (

invalidSNITCPRoute = TCPRoute{
Name: "invalid-sni",
TLS: &TLS{Passthrough: &TLSInspectorConfig{SNIs: []string{}}},
TLS: &TLS{TLSInspectorConfig: &TLSInspectorConfig{SNIs: []string{}}},
Destination: &happyRouteDestination,
}

Expand Down
4 changes: 2 additions & 2 deletions internal/ir/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 9 additions & 2 deletions internal/xds/translator/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ func addXdsTCPFilterChain(xdsListener *listenerv3.Listener, irRoute *ir.TCPRoute
return errors.New("tcp listener is nil")
}

isTLSPassthrough := irRoute.TLS != nil && irRoute.TLS.Passthrough != nil
isTLSPassthrough := irRoute.TLS != nil && irRoute.TLS.TLSInspectorConfig != nil
isTLSTerminate := irRoute.TLS != nil && irRoute.TLS.Terminate != nil
statPrefix := "tcp"
if isTLSPassthrough {
Expand Down Expand Up @@ -430,12 +430,19 @@ func addXdsTCPFilterChain(xdsListener *listenerv3.Listener, irRoute *ir.TCPRoute
}

if isTLSPassthrough {
if err := addServerNamesMatch(xdsListener, filterChain, irRoute.TLS.Passthrough.SNIs); err != nil {
if err := addServerNamesMatch(xdsListener, filterChain, irRoute.TLS.TLSInspectorConfig.SNIs); err != nil {
return err
}
}

if isTLSTerminate {
var snis []string
if cfg := irRoute.TLS.TLSInspectorConfig; cfg != nil {
snis = cfg.SNIs
}
if err := addServerNamesMatch(xdsListener, filterChain, snis); err != nil {
return err
}
tSocket, err := buildXdsDownstreamTLSSocket(irRoute.TLS.Terminate)
if err != nil {
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ tcp:
port: 10082
tcpKeepalive: {}
tls:
passthrough:
inspector:
snis:
- bar.com
destination:
Expand Down
Loading
Loading