Skip to content

Commit

Permalink
fix(meshexternalservice): fix missing tls context (#12162)
Browse files Browse the repository at this point in the history
## Motivation

#11985 it seems that TLS context
wasn't set correctly for MES.

## Implementation information

There were two main problems:

* We didn't set the TLS context on the egress.
* We didn't correctly extract the metadata from the dataplane to check
`SystemCaPath`.

Both issues resulted in the TLS context not being set correctly. Added a
change that retrieved correct metadata and fixed the test. I also added
an e2e test that first verifies we cannot communicate with the TLS 1.3
server when using TLS 1.2. It then switches to TLS 1.3 to confirm that
communication works.

## Supporting documentation

<!-- Is there a MADR? An Issue? A related PR? -->

Fix #11985

<!--
> Changelog: skip
-->
<!--
Uncomment the above section to explicitly set a [`> Changelog:` entry
here](https://github.com/kumahq/kuma/blob/master/CONTRIBUTING.md#submitting-a-patch)?
-->

---------

Signed-off-by: Lukasz Dziedziak <[email protected]>
  • Loading branch information
lukidzi authored Dec 9, 2024
1 parent 66cc95f commit 39369e0
Show file tree
Hide file tree
Showing 11 changed files with 297 additions and 93 deletions.
7 changes: 7 additions & 0 deletions pkg/core/xds/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,13 @@ func (m *DataplaneMetadata) GetDNSPort() uint32 {
return m.DNSPort
}

func (m *DataplaneMetadata) GetSystemCaPath() string {
if m == nil {
return ""
}
return m.SystemCaPath
}

func (m *DataplaneMetadata) GetDynamicMetadata(key string) string {
if m == nil || m.DynamicMetadata == nil {
return ""
Expand Down
4 changes: 2 additions & 2 deletions pkg/xds/context/mesh_context_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,9 @@ func (m *meshContextBuilder) BuildIfChanged(ctx context.Context, meshName string
zoneIngresses := resources.ZoneIngresses().Items
zoneEgresses := resources.ZoneEgresses().Items
externalServices := resources.ExternalServices().Items
endpointMap := xds_topology.BuildEdsEndpointMap(mesh, m.zone, meshServicesByName, meshMultiZoneServices, meshExternalServices, dataplanes, zoneIngresses, zoneEgresses, externalServices)
endpointMap := xds_topology.BuildEdsEndpointMap(ctx, mesh, m.zone, meshServicesByName, meshMultiZoneServices, meshExternalServices, dataplanes, zoneIngresses, zoneEgresses, externalServices, loader)
esEndpointMap := xds_topology.BuildExternalServicesEndpointMap(ctx, mesh, externalServices, loader, m.zone)
ingressEndpointMap := xds_topology.BuildIngressEndpointMap(mesh, m.zone, meshServicesByName, meshMultiZoneServices, meshExternalServices, dataplanes, externalServices, resources.Gateways().Items, zoneEgresses)
ingressEndpointMap := xds_topology.BuildIngressEndpointMap(ctx, mesh, m.zone, meshServicesByName, meshMultiZoneServices, meshExternalServices, dataplanes, externalServices, resources.Gateways().Items, zoneEgresses, loader)

crossMeshEndpointMap := map[string]xds.EndpointMap{}
for otherMeshName, gateways := range resources.gatewaysAndDataplanesForMesh(mesh) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/xds/generator/egress/external_services_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (g *ExternalServicesGenerator) Generate(
services,
endpointMap,
proxy.ZoneEgressProxy.ZoneEgressResource.IsIPv6(),
proxy.Metadata.GetDynamicMetadata(core_xds.FieldSystemCaPath),
proxy.Metadata.GetSystemCaPath(),
)
if err != nil {
return nil, err
Expand Down
15 changes: 15 additions & 0 deletions pkg/xds/topology/ingress_outbound_test.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,34 @@
package topology_test

import (
"context"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
"github.com/kumahq/kuma/pkg/core/datasource"
core_mesh "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
"github.com/kumahq/kuma/pkg/core/resources/apis/meshservice/api/v1alpha1"
"github.com/kumahq/kuma/pkg/core/resources/model"
"github.com/kumahq/kuma/pkg/core/secrets/cipher"
secret_manager "github.com/kumahq/kuma/pkg/core/secrets/manager"
secret_store "github.com/kumahq/kuma/pkg/core/secrets/store"
core_xds "github.com/kumahq/kuma/pkg/core/xds"
"github.com/kumahq/kuma/pkg/plugins/resources/memory"
"github.com/kumahq/kuma/pkg/test/resources/builders"
test_model "github.com/kumahq/kuma/pkg/test/resources/model"
"github.com/kumahq/kuma/pkg/test/resources/samples"
"github.com/kumahq/kuma/pkg/xds/topology"
)

var _ = Describe("IngressTrafficRoute", func() {
var dataSourceLoader datasource.Loader

BeforeEach(func() {
secretManager := secret_manager.NewSecretManager(secret_store.NewSecretStore(memory.NewStore()), cipher.None(), nil, false)
dataSourceLoader = datasource.NewDataSourceLoader(secretManager)
})
Describe("BuildEndpointMap()", func() {
type testCase struct {
mesh *core_mesh.MeshResource
Expand All @@ -33,6 +46,7 @@ var _ = Describe("IngressTrafficRoute", func() {
meshServicesByName[model.NewResourceIdentifier(ms)] = ms
}
endpoints := topology.BuildIngressEndpointMap(
context.Background(),
given.mesh,
"east",
meshServicesByName,
Expand All @@ -42,6 +56,7 @@ var _ = Describe("IngressTrafficRoute", func() {
given.externalServices,
nil,
given.zoneEgress,
dataSourceLoader,
)

// then
Expand Down
167 changes: 95 additions & 72 deletions pkg/xds/topology/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func BuildEgressEndpointMap(
}

func BuildIngressEndpointMap(
ctx context.Context,
mesh *core_mesh.MeshResource,
localZone string,
meshServicesByName map[model.ResourceIdentifier]*meshservice_api.MeshServiceResource,
Expand All @@ -86,15 +87,17 @@ func BuildIngressEndpointMap(
externalServices []*core_mesh.ExternalServiceResource,
gateways []*core_mesh.MeshGatewayResource,
zoneEgresses []*core_mesh.ZoneEgressResource,
loader datasource.Loader,
) core_xds.EndpointMap {
// Build EDS endpoint map just like for regular DPP, but without list of Ingress.
// This way we only keep local endpoints.
outbound := BuildEdsEndpointMap(mesh, localZone, meshServicesByName, meshMultiZoneServices, meshExternalServices, dataplanes, nil, zoneEgresses, externalServices)
outbound := BuildEdsEndpointMap(ctx, mesh, localZone, meshServicesByName, meshMultiZoneServices, meshExternalServices, dataplanes, nil, zoneEgresses, externalServices, loader)
fillLocalCrossMeshOutbounds(outbound, mesh, dataplanes, gateways, 1, localZone)
return outbound
}

func BuildEdsEndpointMap(
ctx context.Context,
mesh *core_mesh.MeshResource,
localZone string,
meshServicesByName map[model.ResourceIdentifier]*meshservice_api.MeshServiceResource,
Expand All @@ -104,6 +107,7 @@ func BuildEdsEndpointMap(
zoneIngresses []*core_mesh.ZoneIngressResource,
zoneEgresses []*core_mesh.ZoneEgressResource,
externalServices []*core_mesh.ExternalServiceResource,
loader datasource.Loader,
) core_xds.EndpointMap {
outbound := core_xds.EndpointMap{}

Expand All @@ -127,7 +131,7 @@ func BuildEdsEndpointMap(

fillRemoteMeshServices(outbound, meshServices, zoneIngresses, mesh, localZone)

fillExternalServicesOutboundsThroughEgress(outbound, externalServices, meshExternalServices, zoneEgresses, mesh, localZone)
fillExternalServicesOutboundsThroughEgress(ctx, outbound, externalServices, meshExternalServices, zoneEgresses, mesh, localZone, loader)

// it has to be last because it reuses endpoints for other cases
fillMeshMultiZoneServices(outbound, meshServicesByName, meshMultiZoneServices)
Expand Down Expand Up @@ -657,70 +661,9 @@ func createMeshExternalServiceEndpoint(
meshName := mesh.GetMeta().GetName()
tls := mes.Spec.Tls
if tls != nil && tls.Enabled {
var caCert, clientCert, clientKey []byte
es.TLSEnabled = tls.Enabled
es.FallbackToSystemCa = true
es.AllowRenegotiation = tls.AllowRenegotiation

var err error
if tls.Verification != nil {
if tls.Verification.CaCert != nil {
caCert, err = loadBytes(ctx, tls.Verification.CaCert.ConvertToProto(), meshName, loader)
if err != nil {
return errors.Wrap(err, "could not load caCert")
}
es.CaCert = caCert
}
if tls.Verification.ClientKey != nil && tls.Verification.ClientCert != nil {
clientCert, err = loadBytes(ctx, tls.Verification.ClientCert.ConvertToProto(), meshName, loader)
if err != nil {
return errors.Wrap(err, "could not load clientCert")
}
clientKey, err = loadBytes(ctx, tls.Verification.ClientKey.ConvertToProto(), meshName, loader)
if err != nil {
return errors.Wrap(err, "could not load clientKey")
}
es.ClientCert = clientCert
es.ClientKey = clientKey
}
if pointer.Deref(tls.Verification.ServerName) != "" {
es.ServerName = pointer.Deref(tls.Verification.ServerName)
}
for _, san := range pointer.Deref(tls.Verification.SubjectAltNames) {
es.SANs = append(es.SANs, core_xds.SAN{
MatchType: core_xds.MatchType(san.Type),
Value: san.Value,
})
}
if tls.Version != nil {
if tls.Version.Min != nil {
es.MinTlsVersion = pointer.To(common_tls.ToTlsVersion(tls.Version.Min))
}
if tls.Version.Max != nil {
es.MaxTlsVersion = pointer.To(common_tls.ToTlsVersion(tls.Version.Max))
}
}
// Server name and SNI we need to add
// mes.Spec.Tls.Verification.SubjectAltNames
if tls.Verification.Mode != nil {
switch *tls.Verification.Mode {
case meshexternalservice_api.TLSVerificationSkipSAN:
es.ServerName = ""
es.SANs = []core_xds.SAN{}
es.SkipHostnameVerification = true
case meshexternalservice_api.TLSVerificationSkipCA:
es.CaCert = nil
es.FallbackToSystemCa = false
case meshexternalservice_api.TLSVerificationSkipAll:
es.FallbackToSystemCa = false
es.CaCert = nil
es.ClientKey = nil
es.ClientCert = nil
es.ServerName = ""
es.SANs = []core_xds.SAN{}
es.SkipHostnameVerification = true
}
}
err := setTlsConfiguration(ctx, tls, es, meshName, loader)
if err != nil {
return err
}
}

Expand All @@ -742,6 +685,75 @@ func createMeshExternalServiceEndpoint(
return nil
}

func setTlsConfiguration(ctx context.Context, tls *meshexternalservice_api.Tls, es *core_xds.ExternalService, meshName string, loader datasource.Loader) error {
var caCert, clientCert, clientKey []byte
es.TLSEnabled = tls.Enabled
es.FallbackToSystemCa = true
es.AllowRenegotiation = tls.AllowRenegotiation

if tls.Version != nil {
if tls.Version.Min != nil {
es.MinTlsVersion = pointer.To(common_tls.ToTlsVersion(tls.Version.Min))
}
if tls.Version.Max != nil {
es.MaxTlsVersion = pointer.To(common_tls.ToTlsVersion(tls.Version.Max))
}
}
var err error
if tls.Verification != nil {
if tls.Verification.CaCert != nil {
caCert, err = loadBytes(ctx, tls.Verification.CaCert.ConvertToProto(), meshName, loader)
if err != nil {
return errors.Wrap(err, "could not load caCert")
}
es.CaCert = caCert
}
if tls.Verification.ClientKey != nil && tls.Verification.ClientCert != nil {
clientCert, err = loadBytes(ctx, tls.Verification.ClientCert.ConvertToProto(), meshName, loader)
if err != nil {
return errors.Wrap(err, "could not load clientCert")
}
clientKey, err = loadBytes(ctx, tls.Verification.ClientKey.ConvertToProto(), meshName, loader)
if err != nil {
return errors.Wrap(err, "could not load clientKey")
}
es.ClientCert = clientCert
es.ClientKey = clientKey
}
if pointer.Deref(tls.Verification.ServerName) != "" {
es.ServerName = pointer.Deref(tls.Verification.ServerName)
}
for _, san := range pointer.Deref(tls.Verification.SubjectAltNames) {
es.SANs = append(es.SANs, core_xds.SAN{
MatchType: core_xds.MatchType(san.Type),
Value: san.Value,
})
}
// Server name and SNI we need to add
// mes.Spec.Tls.Verification.SubjectAltNames
if tls.Verification.Mode != nil {
switch *tls.Verification.Mode {
case meshexternalservice_api.TLSVerificationSkipSAN:
es.ServerName = ""
es.SANs = []core_xds.SAN{}
es.SkipHostnameVerification = true
case meshexternalservice_api.TLSVerificationSkipCA:
es.CaCert = nil
es.FallbackToSystemCa = false
case meshexternalservice_api.TLSVerificationSkipAll:
es.FallbackToSystemCa = false
es.CaCert = nil
es.ClientKey = nil
es.ClientCert = nil
es.ServerName = ""
es.SANs = []core_xds.SAN{}
es.SkipHostnameVerification = true
}
}
}
return nil
}

func createExternalServiceEndpoint(
ctx context.Context,
outbound core_xds.EndpointMap,
Expand All @@ -760,12 +772,14 @@ func createExternalServiceEndpoint(
}

func fillExternalServicesOutboundsThroughEgress(
ctx context.Context,
outbound core_xds.EndpointMap,
externalServices []*core_mesh.ExternalServiceResource,
meshExternalServices []*meshexternalservice_api.MeshExternalServiceResource,
zoneEgresses []*core_mesh.ZoneEgressResource,
mesh *core_mesh.MeshResource,
localZone string,
loader datasource.Loader,
) {
if mesh.ZoneEgressEnabled() {
for _, externalService := range externalServices {
Expand Down Expand Up @@ -799,6 +813,18 @@ func fillExternalServicesOutboundsThroughEgress(
serviceTags := maps.Clone(mes.Meta.GetLabels())
serviceName := mes.DestinationName(uint32(mes.Spec.Match.Port))
locality := GetLocality(localZone, getZone(serviceTags), mesh.LocalityAwareLbEnabled())
tls := mes.Spec.Tls
es := &core_xds.ExternalService{
Protocol: mes.Spec.Match.Protocol,
OwnerResource: pointer.To(core_rules.UniqueKey(mes, "")),
}
if tls != nil && tls.Enabled {
err := setTlsConfiguration(ctx, tls, es, mes.Meta.GetMesh(), loader)
if err != nil {
outboundLog.Error(err, "unable to create MeshExternalService endpoint for egress. Endpoint won't be included in the XDS.", "name", mes.Meta.GetName(), "mesh", mes.Meta.GetMesh())
continue
}
}

for _, ze := range zoneEgresses {
zeNetworking := ze.Spec.GetNetworking()
Expand All @@ -811,12 +837,9 @@ func fillExternalServicesOutboundsThroughEgress(
Tags: serviceTags,
// AS it's a role of zone egress to load balance traffic between
// instances, we can safely set weight to 1
Weight: 1,
Locality: locality,
ExternalService: &core_xds.ExternalService{
Protocol: mes.Spec.Match.Protocol,
OwnerResource: pointer.To(core_rules.UniqueKey(mes, "")),
},
Weight: 1,
Locality: locality,
ExternalService: es,
}

outbound[serviceName] = append(outbound[serviceName], endpoint)
Expand Down
Loading

0 comments on commit 39369e0

Please sign in to comment.