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

Added support for upstream verification for TCPProxy #6079

Merged
merged 3 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions changelogs/unreleased/6079-tsaarni-minor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## Upstream TLS validation and client certificate for TCPProxy

TCPProxy now supports validating server certificate and using client certificate for upstream TLS connections.
Set `httpproxy.spec.tcpproxy.services.validation.caSecret` and `subjectName` to enable optional validation and `tls.envoy-client-certificate` configuration file field or `ContourConfiguration.spec.envoy.clientCertificate` to set the optional client certificate.
52 changes: 41 additions & 11 deletions internal/dag/httpproxy_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -951,17 +951,8 @@

var uv *PeerValidationContext
if (protocol == "tls" || protocol == "h2") && service.UpstreamValidation != nil {
caCertNamespacedName := k8s.NamespacedNameFrom(service.UpstreamValidation.CACertificate, k8s.DefaultNamespace(proxy.Namespace))
// we can only validate TLS connections to services that talk TLS
uv, err = p.source.LookupUpstreamValidation(service.UpstreamValidation, caCertNamespacedName, proxy.Namespace)
if err != nil {
if _, ok := err.(DelegationNotPermittedError); ok {
validCond.AddErrorf(contour_api_v1.ConditionTypeTLSError, "CACertificateNotDelegated",
"service.UpstreamValidation.CACertificate Secret %q is not configured for certificate delegation", caCertNamespacedName)
} else {
validCond.AddErrorf(contour_api_v1.ConditionTypeServiceError, "TLSUpstreamValidation",
"Service [%s:%d] TLS upstream validation policy error: %s", service.Name, service.Port, err)
}
uv = p.peerValidationContext(validCond, proxy, service)
if uv == nil {
return nil
}
}
Expand Down Expand Up @@ -1212,6 +1203,25 @@
return false
}

var uv *PeerValidationContext
if (protocol == "tls" || protocol == "h2") && service.UpstreamValidation != nil {
uv = p.peerValidationContext(validCond, httpproxy, service)
if uv == nil {
return false
}

Check warning on line 1211 in internal/dag/httpproxy_processor.go

View check run for this annotation

Codecov / codecov/patch

internal/dag/httpproxy_processor.go#L1210-L1211

Added lines #L1210 - L1211 were not covered by tests
}

var clientCertSecret *Secret
if p.ClientCertificate != nil {
// Since the client certificate is configured by admin, explicit delegation is not required.
clientCertSecret, err = p.source.LookupTLSSecretInsecure(*p.ClientCertificate)
if err != nil {
validCond.AddErrorf(contour_api_v1.ConditionTypeTLSError, "SecretNotValid",
"tls.envoy-client-certificate Secret %q is invalid: %s", p.ClientCertificate, err)
return false
}
}

proxy.Clusters = append(proxy.Clusters, &Cluster{
Upstream: s,
Weight: uint32(service.Weight),
Expand All @@ -1220,6 +1230,9 @@
TCPHealthCheckPolicy: healthPolicy,
SNI: s.ExternalName,
tsaarni marked this conversation as resolved.
Show resolved Hide resolved
TimeoutPolicy: ClusterTimeoutPolicy{ConnectTimeout: p.ConnectTimeout},
UpstreamTLS: p.UpstreamTLS,
UpstreamValidation: uv,
ClientCertificate: clientCertSecret,
skriss marked this conversation as resolved.
Show resolved Hide resolved
})
}

Expand Down Expand Up @@ -1482,6 +1495,23 @@
return nil
}

func (p *HTTPProxyProcessor) peerValidationContext(validCond *contour_api_v1.DetailedCondition, httpproxy *contour_api_v1.HTTPProxy, service contour_api_v1.Service) *PeerValidationContext {
caCertNamespacedName := k8s.NamespacedNameFrom(service.UpstreamValidation.CACertificate, k8s.DefaultNamespace(httpproxy.Namespace))
// we can only validate TLS connections to services that talk TLS
uv, err := p.source.LookupUpstreamValidation(service.UpstreamValidation, caCertNamespacedName, httpproxy.Namespace)
if err != nil {
if _, ok := err.(DelegationNotPermittedError); ok {
validCond.AddErrorf(contour_api_v1.ConditionTypeTLSError, "CACertificateNotDelegated",
"service.UpstreamValidation.CACertificate Secret %q is not configured for certificate delegation", caCertNamespacedName)
} else {
validCond.AddErrorf(contour_api_v1.ConditionTypeServiceError, "TLSUpstreamValidation",
"Service [%s:%d] TLS upstream validation policy error: %s", service.Name, service.Port, err)
}
return nil
}
return uv
}

// expandPrefixMatches adds new Routes to account for the difference
// between prefix replacement when matching on '/foo' and '/foo/'.
//
Expand Down
38 changes: 35 additions & 3 deletions internal/featuretests/v3/backendcavalidation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
"github.com/projectcontour/contour/internal/featuretests"
"github.com/projectcontour/contour/internal/fixture"
"github.com/projectcontour/contour/internal/ref"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/intstr"
)
Expand Down Expand Up @@ -100,13 +101,15 @@ func TestClusterServiceTLSBackendCAValidation(t *testing.T) {
TypeUrl: listenerType,
})

// assert that the cluster now has a certificate and subject name.
c.Request(clusterType).Equals(&envoy_discovery_v3.DiscoveryResponse{
expectedResponse := &envoy_discovery_v3.DiscoveryResponse{
Resources: resources(t,
tlsCluster(cluster("default/kuard/443/c6ccd34de5", "default/kuard/securebackend", "default_kuard_443"), caSecret, "subjname", "", nil, nil),
),
TypeUrl: clusterType,
})
}

// assert that the cluster now has a certificate and subject name.
c.Request(clusterType).Equals(expectedResponse)

// Contour does not use SDS to transmit the CA for upstream validation, issue 1405,
// assert that SDS is empty.
Expand Down Expand Up @@ -163,4 +166,33 @@ func TestClusterServiceTLSBackendCAValidation(t *testing.T) {
Resources: nil,
TypeUrl: secretType,
})

rh.OnDelete(hp1)

serverSecret := featuretests.TLSSecret(t, "secret", &featuretests.ServerCertificate)
rh.OnAdd(serverSecret)

tcpproxy := fixture.NewProxy("tcpproxy").WithSpec(
contour_api_v1.HTTPProxySpec{
VirtualHost: &contour_api_v1.VirtualHost{
Fqdn: "www.example.com",
TLS: &contour_api_v1.TLS{
SecretName: serverSecret.Name,
},
},
TCPProxy: &contour_api_v1.TCPProxy{
Services: []contour_api_v1.Service{{
Name: svc.Name,
Port: 443,
Protocol: ref.To("tls"),
UpstreamValidation: &contour_api_v1.UpstreamValidation{
CACertificate: caSecret.Name,
SubjectName: "subjname",
},
}},
},
})
rh.OnAdd(tcpproxy)

c.Request(clusterType).Equals(expectedResponse)
}
34 changes: 32 additions & 2 deletions internal/featuretests/v3/backendclientauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,10 @@ func TestBackendClientAuthenticationWithHTTPProxy(t *testing.T) {
defer done()

clientSecret := featuretests.TLSSecret(t, "envoyclientsecret", &featuretests.ClientCertificate)
serverSecret := featuretests.TLSSecret(t, "envoyserversecret", &featuretests.ServerCertificate)
caSecret := featuretests.CASecret(t, "backendcacert", &featuretests.CACertificate)
rh.OnAdd(clientSecret)
rh.OnAdd(serverSecret)
rh.OnAdd(caSecret)

svc := fixture.NewService("backend").
Expand All @@ -94,12 +96,40 @@ func TestBackendClientAuthenticationWithHTTPProxy(t *testing.T) {
})
rh.OnAdd(proxy)

c.Request(clusterType).Equals(&envoy_discovery_v3.DiscoveryResponse{
expectedResponse := &envoy_discovery_v3.DiscoveryResponse{
Resources: resources(t,
tlsCluster(cluster("default/backend/443/950c17581f", "default/backend/http", "default_backend_443"), caSecret, "subjname", "", clientSecret, nil),
),
TypeUrl: clusterType,
})
}

c.Request(clusterType).Equals(expectedResponse)

rh.OnDelete(proxy)

tcpproxy := fixture.NewProxy("tcpproxy").WithSpec(
projcontour.HTTPProxySpec{
VirtualHost: &projcontour.VirtualHost{
Fqdn: "www.example.com",
TLS: &projcontour.TLS{
SecretName: serverSecret.Name,
},
},
TCPProxy: &projcontour.TCPProxy{
Services: []projcontour.Service{{
Name: svc.Name,
Port: 443,
Protocol: ref.To("tls"),
UpstreamValidation: &projcontour.UpstreamValidation{
CACertificate: caSecret.Name,
SubjectName: "subjname",
},
}},
},
})
rh.OnAdd(tcpproxy)

c.Request(clusterType).Equals(expectedResponse)

// Test the error branch when Envoy client certificate secret does not exist.
rh.OnDelete(clientSecret)
Expand Down
Loading