Skip to content

Configure Envoy alpn_protocols based on service protocol #14356

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.

Sign up for GitHub

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

Merged
malizz merged 22 commits into from
Oct 10, 2022
Merged

Configure Envoy alpn_protocols based on service protocol #14356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7b3722a
Configure Envoy alpn_protocols based on service protocol
oulman Aug 26, 2022
ffcd695
define alpnProtocols in a more standard way
oulman Aug 26, 2022
7e97544
http2 protocol should be h2 only
oulman Sep 2, 2022
c371d46
formatting
oulman Sep 3, 2022
e2e342b
add test for getAlpnProtocol()
oulman Sep 3, 2022
b9fd9a8
create changelog entry
oulman Sep 3, 2022
d0612af
change scope is connect-proxy
oulman Sep 3, 2022
b4cb253
ignore errors on ParseProxyConfig; fixes linter
oulman Sep 5, 2022
bd6ec7f
add tests for grpc and http2 public listeners
oulman Sep 5, 2022
0091d17
remove newlines from PR
oulman Sep 5, 2022
4346e02
Add alpn_protocol configuration for ingress gateway
oulman Sep 8, 2022
2f7fb76
Guard against nil tlsContext
oulman Sep 8, 2022
bf3f44f
add ingress gateway w/ TLS tests for gRPC and HTTP2
oulman Sep 9, 2022
e7122c3
getAlpnProtocols: add TCP protocol test
oulman Sep 13, 2022
2676416
add tests for ingress gateway with grpc/http2 and per-listener TLS co…
oulman Sep 13, 2022
8b2f2dd
add tests for ingress gateway with grpc/http2 and per-listener TLS co…
oulman Sep 13, 2022
f887940
add Gateway level TLS config with mixed protocol listeners to validat…
oulman Sep 13, 2022
b0a2657
update changelog to include ingress-gateway
oulman Sep 13, 2022
18bd910
add http/1.1 to http2 ALPN
oulman Sep 19, 2022
66bcd7e
Merge branch 'main' into f-blake-alpn-config
oulman Sep 29, 2022
3fe0200
go fmt
oulman Sep 29, 2022
7d9ad70
fix test on custom-trace-listener
oulman Oct 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changelog/14356.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
xds: configure Envoy `alpn_protocols` for connect-proxy and ingress-gateway based on service protocol.
320 changes: 320 additions & 0 deletions agent/proxycfg/testing_ingress_gateway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1485,6 +1485,326 @@ func TestConfigSnapshotIngressGateway_SingleTLSListener(t testing.T) *ConfigSnap
})
}

func TestConfigSnapshotIngressGateway_SingleTLSListener_GRPC(t testing.T) *ConfigSnapshot {
var (
s1 = structs.NewServiceName("s1", nil)
s1UID = NewUpstreamIDFromServiceName(s1)
s1Chain = discoverychain.TestCompileConfigEntries(t, "s1", "default", "default", "dc1", connect.TestClusterID+".consul", nil)

s2 = structs.NewServiceName("s2", nil)
s2UID = NewUpstreamIDFromServiceName(s2)
s2Chain = discoverychain.TestCompileConfigEntries(t, "s2", "default", "default", "dc1", connect.TestClusterID+".consul", nil)
)
return TestConfigSnapshotIngressGateway(t, true, "grpc", "simple", nil,
func(entry *structs.IngressGatewayConfigEntry) {
entry.Listeners = []structs.IngressListener{
{
Port: 8080,
Protocol: "grpc",
Services: []structs.IngressService{
{Name: "s1"},
},
},
{
Port: 8081,
Protocol: "grpc",
Services: []structs.IngressService{
{Name: "s2"},
},
TLS: &structs.GatewayTLSConfig{
Enabled: true,
TLSMinVersion: types.TLSv1_2,
},
},
}
}, []UpdateEvent{
{
CorrelationID: gatewayServicesWatchID,
Result: &structs.IndexedGatewayServices{
// One listener should inherit non-TLS gateway config, another
// listener configures TLS with an explicit minimum version
Services: []*structs.GatewayService{
{
Service: s1,
Port: 8080,
Protocol: "grpc",
},
{
Service: s2,
Port: 8081,
Protocol: "grpc",
},
},
},
},
{
CorrelationID: "discovery-chain:" + s1UID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: s1Chain,
},
},
{
CorrelationID: "discovery-chain:" + s2UID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: s2Chain,
},
},
{
CorrelationID: "upstream-target:" + s1Chain.ID() + ":" + s1UID.String(),
Result: &structs.IndexedCheckServiceNodes{
Nodes: TestUpstreamNodes(t, "s1"),
},
},
{
CorrelationID: "upstream-target:" + s2Chain.ID() + ":" + s2UID.String(),
Result: &structs.IndexedCheckServiceNodes{
Nodes: TestUpstreamNodes(t, "s2"),
},
},
})
}

func TestConfigSnapshotIngressGateway_SingleTLSListener_HTTP2(t testing.T) *ConfigSnapshot {
var (
s1 = structs.NewServiceName("s1", nil)
s1UID = NewUpstreamIDFromServiceName(s1)
s1Chain = discoverychain.TestCompileConfigEntries(t, "s1", "default", "default", "dc1", connect.TestClusterID+".consul", nil)

s2 = structs.NewServiceName("s2", nil)
s2UID = NewUpstreamIDFromServiceName(s2)
s2Chain = discoverychain.TestCompileConfigEntries(t, "s2", "default", "default", "dc1", connect.TestClusterID+".consul", nil)
)
return TestConfigSnapshotIngressGateway(t, true, "http2", "simple", nil,
func(entry *structs.IngressGatewayConfigEntry) {
entry.Listeners = []structs.IngressListener{
{
Port: 8080,
Protocol: "http2",
Services: []structs.IngressService{
{Name: "s1"},
},
},
{
Port: 8081,
Protocol: "http2",
Services: []structs.IngressService{
{Name: "s2"},
},
TLS: &structs.GatewayTLSConfig{
Enabled: true,
TLSMinVersion: types.TLSv1_2,
},
},
}
}, []UpdateEvent{
{
CorrelationID: gatewayServicesWatchID,
Result: &structs.IndexedGatewayServices{
// One listener should inherit non-TLS gateway config, another
// listener configures TLS with an explicit minimum version
Services: []*structs.GatewayService{
{
Service: s1,
Port: 8080,
Protocol: "http2",
},
{
Service: s2,
Port: 8081,
Protocol: "http2",
},
},
},
},
{
CorrelationID: "discovery-chain:" + s1UID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: s1Chain,
},
},
{
CorrelationID: "discovery-chain:" + s2UID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: s2Chain,
},
},
{
CorrelationID: "upstream-target:" + s1Chain.ID() + ":" + s1UID.String(),
Result: &structs.IndexedCheckServiceNodes{
Nodes: TestUpstreamNodes(t, "s1"),
},
},
{
CorrelationID: "upstream-target:" + s2Chain.ID() + ":" + s2UID.String(),
Result: &structs.IndexedCheckServiceNodes{
Nodes: TestUpstreamNodes(t, "s2"),
},
},
})
}

func TestConfigSnapshotIngressGateway_MultiTLSListener_MixedHTTP2gRPC(t testing.T) *ConfigSnapshot {
var (
s1 = structs.NewServiceName("s1", nil)
s1UID = NewUpstreamIDFromServiceName(s1)
s1Chain = discoverychain.TestCompileConfigEntries(t, "s1", "default", "default", "dc1", connect.TestClusterID+".consul", nil)

s2 = structs.NewServiceName("s2", nil)
s2UID = NewUpstreamIDFromServiceName(s2)
s2Chain = discoverychain.TestCompileConfigEntries(t, "s2", "default", "default", "dc1", connect.TestClusterID+".consul", nil)
)
return TestConfigSnapshotIngressGateway(t, true, "tcp", "simple", nil,
func(entry *structs.IngressGatewayConfigEntry) {
entry.Listeners = []structs.IngressListener{
{
Port: 8080,
Protocol: "grpc",
Services: []structs.IngressService{
{Name: "s1"},
},
TLS: &structs.GatewayTLSConfig{
Enabled: true,
TLSMinVersion: types.TLSv1_2,
},
},
{
Port: 8081,
Protocol: "http2",
Services: []structs.IngressService{
{Name: "s2"},
},
TLS: &structs.GatewayTLSConfig{
Enabled: true,
TLSMinVersion: types.TLSv1_2,
},
},
}
}, []UpdateEvent{
{
CorrelationID: gatewayServicesWatchID,
Result: &structs.IndexedGatewayServices{
// One listener should inherit non-TLS gateway config, another
// listener configures TLS with an explicit minimum version
Services: []*structs.GatewayService{
{
Service: s1,
Port: 8080,
Protocol: "grpc",
},
{
Service: s2,
Port: 8081,
Protocol: "http2",
},
},
},
},
{
CorrelationID: "discovery-chain:" + s1UID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: s1Chain,
},
},
{
CorrelationID: "discovery-chain:" + s2UID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: s2Chain,
},
},
{
CorrelationID: "upstream-target:" + s1Chain.ID() + ":" + s1UID.String(),
Result: &structs.IndexedCheckServiceNodes{
Nodes: TestUpstreamNodes(t, "s1"),
},
},
{
CorrelationID: "upstream-target:" + s2Chain.ID() + ":" + s2UID.String(),
Result: &structs.IndexedCheckServiceNodes{
Nodes: TestUpstreamNodes(t, "s2"),
},
},
})
}

func TestConfigSnapshotIngressGateway_GWTLSListener_MixedHTTP2gRPC(t testing.T) *ConfigSnapshot {
var (
s1 = structs.NewServiceName("s1", nil)
s1UID = NewUpstreamIDFromServiceName(s1)
s1Chain = discoverychain.TestCompileConfigEntries(t, "s1", "default", "default", "dc1", connect.TestClusterID+".consul", nil)

s2 = structs.NewServiceName("s2", nil)
s2UID = NewUpstreamIDFromServiceName(s2)
s2Chain = discoverychain.TestCompileConfigEntries(t, "s2", "default", "default", "dc1", connect.TestClusterID+".consul", nil)
)
return TestConfigSnapshotIngressGateway(t, true, "tcp", "simple", nil,
func(entry *structs.IngressGatewayConfigEntry) {
entry.TLS = structs.GatewayTLSConfig{
Enabled: true,
TLSMinVersion: types.TLSv1_2,
}
entry.Listeners = []structs.IngressListener{
{
Port: 8080,
Protocol: "grpc",
Services: []structs.IngressService{
{Name: "s1"},
},
},
{
Port: 8081,
Protocol: "http2",
Services: []structs.IngressService{
{Name: "s2"},
},
},
}
}, []UpdateEvent{
{
CorrelationID: gatewayServicesWatchID,
Result: &structs.IndexedGatewayServices{
// One listener should inherit non-TLS gateway config, another
// listener configures TLS with an explicit minimum version
Services: []*structs.GatewayService{
{
Service: s1,
Port: 8080,
Protocol: "grpc",
},
{
Service: s2,
Port: 8081,
Protocol: "http2",
},
},
},
},
{
CorrelationID: "discovery-chain:" + s1UID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: s1Chain,
},
},
{
CorrelationID: "discovery-chain:" + s2UID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: s2Chain,
},
},
{
CorrelationID: "upstream-target:" + s1Chain.ID() + ":" + s1UID.String(),
Result: &structs.IndexedCheckServiceNodes{
Nodes: TestUpstreamNodes(t, "s1"),
},
},
{
CorrelationID: "upstream-target:" + s2Chain.ID() + ":" + s2UID.String(),
Result: &structs.IndexedCheckServiceNodes{
Nodes: TestUpstreamNodes(t, "s2"),
},
},
})
}

func TestConfigSnapshotIngressGateway_TLSMixedMinVersionListeners(t testing.T) *ConfigSnapshot {
var (
s1 = structs.NewServiceName("s1", nil)
Expand Down
22 changes: 22 additions & 0 deletions agent/xds/listeners.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,19 @@ func (s *ResourceGenerator) injectConnectTLSForPublicListener(cfgSnap *proxycfg.
return nil
}

func getAlpnProtocols(protocol string) []string {
var alpnProtocols []string

switch protocol {
case "grpc", "http2":
alpnProtocols = append(alpnProtocols, "h2", "http/1.1")
case "http":
alpnProtocols = append(alpnProtocols, "http/1.1")
}

return alpnProtocols
}

func createDownstreamTransportSocketForConnectTLS(cfgSnap *proxycfg.ConfigSnapshot, peerBundles []*pbpeering.PeeringTrustBundle) (*envoy_core_v3.TransportSocket, error) {
switch cfgSnap.Kind {
case structs.ServiceKindConnectProxy:
Expand All @@ -1081,13 +1094,22 @@ func createDownstreamTransportSocketForConnectTLS(cfgSnap *proxycfg.ConfigSnapsh
return nil, fmt.Errorf("cannot inject peering trust bundles for kind %q", cfgSnap.Kind)
}

// Determine listener protocol type from configured service protocol. Don't hard fail on a config typo,
//The parse func returns default config if there is an error, so it's safe to continue.
cfg, _ := ParseProxyConfig(cfgSnap.Proxy.Config)

Copy link
Contributor

@malizz malizz Sep 13, 2022

Choose a reason for hiding this comment

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

The default value for protocol in ParseProxyConfig function is tcp which is missing in getAlpnProtocols function. We can either add it to the switch cases or return an error if it's tcp.

Copy link
Contributor Author

@oulman oulman Sep 13, 2022

Choose a reason for hiding this comment

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

🤔 Right now calling getAlpnProtocols with tcp is returning an implicit nil because it's not matched. To confirm, we want to add an case statement for tcp but it should still return the empty alpnProtocols slice.

// Create TLS validation context for mTLS with leaf certificate and root certs.
tlsContext := makeCommonTLSContext(
cfgSnap.Leaf(),
cfgSnap.RootPEMs(),
makeTLSParametersFromProxyTLSConfig(cfgSnap.MeshConfigTLSIncoming()),
)

if tlsContext != nil {
// Configure alpn protocols on CommonTLSContext
tlsContext.AlpnProtocols = getAlpnProtocols(cfg.Protocol)
}

// Inject peering trust bundles if this service is exported to peered clusters.
if len(peerBundles) > 0 {
spiffeConfig, err := makeSpiffeValidatorConfig(
Expand Down
11 changes: 10 additions & 1 deletion agent/xds/listeners_ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ func makeDownstreamTLSContextFromSnapshotListenerConfig(cfgSnap *proxycfg.Config
}

if tlsContext != nil {
// Configure alpn protocols on TLSContext
tlsContext.AlpnProtocols = getAlpnProtocols(listenerCfg.Protocol)

downstreamContext = &envoy_tls_v3.DownstreamTlsContext{
CommonTlsContext: tlsContext,
RequireClientCertificate: &wrappers.BoolValue{Value: false},
Expand Down Expand Up @@ -325,8 +328,14 @@ func makeSDSOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot,
return nil, err
}

commonTlsContext := makeCommonTLSContextFromGatewayServiceTLSConfig(*svc.TLS)
if commonTlsContext != nil {
// Configure alpn protocols on TLSContext
commonTlsContext.AlpnProtocols = getAlpnProtocols(listenerCfg.Protocol)
}

tlsContext := &envoy_tls_v3.DownstreamTlsContext{
CommonTlsContext: makeCommonTLSContextFromGatewayServiceTLSConfig(*svc.TLS),
CommonTlsContext: commonTlsContext,
RequireClientCertificate: &wrappers.BoolValue{Value: false},
}

Expand Down
Loading