Skip to content
Permalink
Browse files Browse the repository at this point in the history
cherrypicks for v1.17.1 (#3909)
* Merge pull request from GHSA-5ph6-qq5x-7jwc

Signed-off-by: Nick Young <ynick@vmware.com>

* Fix spelling and lint errors that slipped into the ExternalName PR (#3908)

Signed-off-by: Nick Young <ynick@vmware.com>

Co-authored-by: Nick Young <ynick@vmware.com>
  • Loading branch information
skriss and youngnick committed Jul 22, 2021
1 parent b8136f7 commit b53a5c4
Show file tree
Hide file tree
Showing 19 changed files with 393 additions and 35 deletions.
24 changes: 15 additions & 9 deletions cmd/contour/serve.go
Expand Up @@ -714,29 +714,35 @@ func getDAGBuilder(ctx *serveContext, clients *k8s.Clients, clientCert, fallback
responseHeadersPolicy.Remove = append(responseHeadersPolicy.Remove, ctx.Config.Policy.ResponseHeadersPolicy.Remove...)
}

log.Debugf("EnableExternalNameService is set to %t", ctx.Config.EnableExternalNameService)
// Get the appropriate DAG processors.
dagProcessors := []dag.Processor{
&dag.IngressProcessor{
FieldLogger: log.WithField("context", "IngressProcessor"),
ClientCertificate: clientCert,
EnableExternalNameService: ctx.Config.EnableExternalNameService,
FieldLogger: log.WithField("context", "IngressProcessor"),
ClientCertificate: clientCert,
},
&dag.ExtensionServiceProcessor{
// Note that ExtensionService does not support ExternalName, if it does get added,
// need to bring EnableExternalNameService in here too.
FieldLogger: log.WithField("context", "ExtensionServiceProcessor"),
ClientCertificate: clientCert,
},
&dag.HTTPProxyProcessor{
DisablePermitInsecure: ctx.Config.DisablePermitInsecure,
FallbackCertificate: fallbackCert,
DNSLookupFamily: ctx.Config.Cluster.DNSLookupFamily,
ClientCertificate: clientCert,
RequestHeadersPolicy: &requestHeadersPolicy,
ResponseHeadersPolicy: &responseHeadersPolicy,
EnableExternalNameService: ctx.Config.EnableExternalNameService,
DisablePermitInsecure: ctx.Config.DisablePermitInsecure,
FallbackCertificate: fallbackCert,
DNSLookupFamily: ctx.Config.Cluster.DNSLookupFamily,
ClientCertificate: clientCert,
RequestHeadersPolicy: &requestHeadersPolicy,
ResponseHeadersPolicy: &responseHeadersPolicy,
},
}

if ctx.Config.GatewayConfig != nil && clients.ResourcesExist(k8s.GatewayAPIResources()...) {
dagProcessors = append(dagProcessors, &dag.GatewayAPIProcessor{
FieldLogger: log.WithField("context", "GatewayAPIProcessor"),
EnableExternalNameService: ctx.Config.EnableExternalNameService,
FieldLogger: log.WithField("context", "GatewayAPIProcessor"),
})
}

Expand Down
7 changes: 7 additions & 0 deletions examples/contour/01-contour-config.yaml
Expand Up @@ -51,6 +51,13 @@ data:
# leaderelection:
# configmap-name: leader-elect
# configmap-namespace: projectcontour
####
# ExternalName Services are disabled by default due to CVE-2021-XXXXX
# You can re-enable them by setting this setting to `true`.
# This is not recommended without understanding the security implications.
# Please see the advisory at https://github.com/projectcontour/contour/security/advisories/GHSA-5ph6-qq5x-7jwc for the details.
# enableExternalNameService: false
##
### Logging options
# Default setting
accesslog-format: envoy
Expand Down
7 changes: 7 additions & 0 deletions examples/render/contour-gateway.yaml
Expand Up @@ -88,6 +88,13 @@ data:
# leaderelection:
# configmap-name: leader-elect
# configmap-namespace: projectcontour
####
# ExternalName Services are disabled by default due to CVE-2021-XXXXX
# You can re-enable them by setting this setting to `true`.
# This is not recommended without understanding the security implications.
# Please see the advisory at https://github.com/projectcontour/contour/security/advisories/GHSA-5ph6-qq5x-7jwc for the details.
# enableExternalNameService: false
##
### Logging options
# Default setting
accesslog-format: envoy
Expand Down
7 changes: 7 additions & 0 deletions examples/render/contour.yaml
Expand Up @@ -85,6 +85,13 @@ data:
# leaderelection:
# configmap-name: leader-elect
# configmap-namespace: projectcontour
####
# ExternalName Services are disabled by default due to CVE-2021-XXXXX
# You can re-enable them by setting this setting to `true`.
# This is not recommended without understanding the security implications.
# Please see the advisory at https://github.com/projectcontour/contour/security/advisories/GHSA-5ph6-qq5x-7jwc for the details.
# enableExternalNameService: false
##
### Logging options
# Default setting
accesslog-format: envoy
Expand Down
1 change: 1 addition & 0 deletions go.mod
Expand Up @@ -5,6 +5,7 @@ go 1.15
require (
github.com/ahmetb/gen-crd-api-reference-docs v0.3.0
github.com/bombsimon/logrusr v1.0.0
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/envoyproxy/go-control-plane v0.9.9-0.20210111201334-f1f47757da33
github.com/go-logr/logr v0.4.0
github.com/golang/protobuf v1.5.2
Expand Down
39 changes: 38 additions & 1 deletion internal/dag/accessors.go
Expand Up @@ -51,12 +51,17 @@ func (dag *DAG) GetService(meta types.NamespacedName, port int32) *Service {
// EnsureService looks for a Kubernetes service in the cache matching the provided
// namespace, name and port, and returns a DAG service for it. If a matching service
// cannot be found in the cache, an error is returned.
func (dag *DAG) EnsureService(meta types.NamespacedName, port intstr.IntOrString, cache *KubernetesCache) (*Service, error) {
func (dag *DAG) EnsureService(meta types.NamespacedName, port intstr.IntOrString, cache *KubernetesCache, enableExternalNameSvc bool) (*Service, error) {
svc, svcPort, err := cache.LookupService(meta, port)
if err != nil {
return nil, err
}

err = validateExternalName(svc, enableExternalNameSvc)
if err != nil {
return nil, err
}

if dagSvc := dag.GetService(k8s.NamespacedNameOf(svc), svcPort.Port); dagSvc != nil {
return dagSvc, nil
}
Expand All @@ -78,6 +83,38 @@ func (dag *DAG) EnsureService(meta types.NamespacedName, port intstr.IntOrString
return dagSvc, nil
}

func validateExternalName(svc *v1.Service, enableExternalNameSvc bool) error {

// If this isn't an ExternalName Service, we're all good here.
en := externalName(svc)
if en == "" {
return nil
}

// If ExternalNames are disabled, then we don't want to add this to the DAG.
if !enableExternalNameSvc {
return fmt.Errorf("%s/%s is an ExternalName service, these are not currently enabled. See the config.enableExternalNameService config file setting", svc.Namespace, svc.Name)
}

// Check against a list of known localhost names, using a map to approximate a set.
// TODO(youngnick) This is a very porous hack, and we should probably look into doing a DNS
// lookup to check what the externalName resolves to, but I'm worried about the
// performance impact of doing one or more DNS lookups per DAG run, so we're
// going to go with a specific blocklist for now.
localhostNames := map[string]struct{}{
"localhost": {},
"localhost.localdomain": {},
"local.projectcontour.io": {},
}

_, localhost := localhostNames[en]
if localhost {
return fmt.Errorf("%s/%s is an ExternalName service that points to localhost, this is not allowed", svc.Namespace, svc.Name)
}

return nil
}

func upstreamProtocol(svc *v1.Service, port v1.ServicePort) string {
up := annotation.ParseUpstreamProtocols(svc.Annotations)
protocol := up[port.Name]
Expand Down
78 changes: 73 additions & 5 deletions internal/dag/accessors_test.go
Expand Up @@ -40,15 +40,53 @@ func TestBuilderLookupService(t *testing.T) {
}},
},
}

externalNameValid := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "externalnamevalid",
Namespace: "default",
},
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeExternalName,
ExternalName: "external.projectcontour.io",
Ports: []v1.ServicePort{{
Name: "http",
Protocol: "TCP",
Port: 80,
TargetPort: intstr.FromInt(80),
}},
},
}

externalNameLocalhost := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "externalnamelocalhost",
Namespace: "default",
},
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeExternalName,
ExternalName: "localhost",
Ports: []v1.ServicePort{{
Name: "http",
Protocol: "TCP",
Port: 80,
TargetPort: intstr.FromInt(80),
}},
},
}

services := map[types.NamespacedName]*v1.Service{
{Name: "service1", Namespace: "default"}: s1,
{Name: "service1", Namespace: "default"}: s1,
{Name: "externalnamevalid", Namespace: "default"}: externalNameValid,
{Name: "externalnamelocalhost", Namespace: "default"}: externalNameLocalhost,
}

tests := map[string]struct {
types.NamespacedName
port intstr.IntOrString
want *Service
wantErr error
port intstr.IntOrString
enableExternalNameSvc bool
want *Service
wantErr error
}{
"lookup service by port number": {
NamespacedName: types.NamespacedName{Name: "service1", Namespace: "default"},
Expand Down Expand Up @@ -80,6 +118,36 @@ func TestBuilderLookupService(t *testing.T) {
port: intstr.FromString("9999"),
wantErr: errors.New(`port "9999" on service "default/service1" not matched`),
},
"When ExternalName Services are not disabled no error is returned": {
NamespacedName: types.NamespacedName{Name: "externalnamevalid", Namespace: "default"},
port: intstr.FromString("80"),
want: &Service{
Weighted: WeightedService{
Weight: 1,
ServiceName: "externalnamevalid",
ServiceNamespace: "default",
ServicePort: v1.ServicePort{
Name: "http",
Protocol: "TCP",
Port: 80,
TargetPort: intstr.FromInt(80),
},
},
ExternalName: "external.projectcontour.io",
},
enableExternalNameSvc: true,
},
"When ExternalName Services are disabled an error is returned": {
NamespacedName: types.NamespacedName{Name: "externalnamevalid", Namespace: "default"},
port: intstr.FromString("80"),
wantErr: errors.New(`default/externalnamevalid is an ExternalName service, these are not currently enabled. See the config.enableExternalNameService config file setting`),
},
"When ExternalName Services are enabled but a localhost ExternalName is used an error is returned": {
NamespacedName: types.NamespacedName{Name: "externalnamelocalhost", Namespace: "default"},
port: intstr.FromString("80"),
wantErr: errors.New(`default/externalnamelocalhost is an ExternalName service that points to localhost, this is not allowed`),
enableExternalNameSvc: true,
},
}

for name, tc := range tests {
Expand All @@ -93,7 +161,7 @@ func TestBuilderLookupService(t *testing.T) {

var dag DAG

got, gotErr := dag.EnsureService(tc.NamespacedName, tc.port, &b.Source)
got, gotErr := dag.EnsureService(tc.NamespacedName, tc.port, &b.Source, tc.enableExternalNameSvc)
assert.Equal(t, tc.want, got)
assert.Equal(t, tc.wantErr, gotErr)
})
Expand Down
68 changes: 65 additions & 3 deletions internal/dag/builder_test.go
Expand Up @@ -6536,6 +6536,25 @@ func TestDAGInsert(t *testing.T) {
},
}

ingressExternalNameService := &networking_v1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "externalname",
Namespace: "default",
},
Spec: networking_v1.IngressSpec{
Rules: []networking_v1.IngressRule{{
Host: "example.com",
IngressRuleValue: networking_v1.IngressRuleValue{
HTTP: &networking_v1.HTTPIngressRuleValue{
Paths: []networking_v1.HTTPIngressPath{{
Backend: *backendv1(s14.GetName(), intstr.FromInt(80)),
}},
},
},
}},
},
}

proxyExternalNameService := &contour_api_v1.HTTPProxy{
ObjectMeta: metav1.ObjectMeta{
Name: "example-com",
Expand Down Expand Up @@ -6582,6 +6601,7 @@ func TestDAGInsert(t *testing.T) {
tests := map[string]struct {
objs []interface{}
disablePermitInsecure bool
enableExternalNameSvc bool
fallbackCertificateName string
fallbackCertificateNamespace string
want []Vertex
Expand Down Expand Up @@ -8980,11 +9000,48 @@ func TestDAGInsert(t *testing.T) {
},
),
},
"insert ingress with externalName service": {
objs: []interface{}{
ingressExternalNameService,
s14,
},
enableExternalNameSvc: true,
want: listeners(
&Listener{
Port: 80,
VirtualHosts: virtualhosts(
virtualhost("example.com", &Route{
PathMatchCondition: prefixString("/"),
Clusters: []*Cluster{{
Upstream: &Service{
ExternalName: "externalservice.io",
Weighted: WeightedService{
Weight: 1,
ServiceName: s14.Name,
ServiceNamespace: s14.Namespace,
ServicePort: s14.Spec.Ports[0],
},
},
}},
}),
),
},
),
},
"insert ingress with externalName service, but externalName services disabled": {
objs: []interface{}{
ingressExternalNameService,
s14,
},
enableExternalNameSvc: false,
want: listeners(),
},
"insert proxy with externalName service": {
objs: []interface{}{
proxyExternalNameService,
s14,
},
enableExternalNameSvc: true,
want: listeners(
&Listener{
Port: 80,
Expand Down Expand Up @@ -9014,6 +9071,7 @@ func TestDAGInsert(t *testing.T) {
s14,
sec1,
},
enableExternalNameSvc: true,
want: listeners(
&Listener{
Port: 443,
Expand Down Expand Up @@ -9073,6 +9131,7 @@ func TestDAGInsert(t *testing.T) {
proxyReplaceHostHeaderRoute,
s14,
},
enableExternalNameSvc: true,
want: listeners(
&Listener{
Port: 80,
Expand Down Expand Up @@ -9111,7 +9170,8 @@ func TestDAGInsert(t *testing.T) {
proxyReplaceHostHeaderService,
s14,
},
want: listeners(),
enableExternalNameSvc: true,
want: listeners(),
},
"insert proxy with response header policy - route - host header": {
objs: []interface{}{
Expand Down Expand Up @@ -9754,10 +9814,12 @@ func TestDAGInsert(t *testing.T) {
},
Processors: []Processor{
&IngressProcessor{
FieldLogger: fixture.NewTestLogger(t),
FieldLogger: fixture.NewTestLogger(t),
EnableExternalNameService: tc.enableExternalNameSvc,
},
&HTTPProxyProcessor{
DisablePermitInsecure: tc.disablePermitInsecure,
EnableExternalNameService: tc.enableExternalNameSvc,
DisablePermitInsecure: tc.disablePermitInsecure,
FallbackCertificate: &types.NamespacedName{
Name: tc.fallbackCertificateName,
Namespace: tc.fallbackCertificateNamespace,
Expand Down
2 changes: 2 additions & 0 deletions internal/dag/extension_processor.go
Expand Up @@ -177,6 +177,8 @@ func (p *ExtensionServiceProcessor) buildExtensionService(
}

// TODO(jpeach): Add ExternalName support in https://github.com/projectcontour/contour/issues/2875.
// TODO(youngnick): If ExternalName support is added, we must pass down the EnableExternalNameService bool
// and check it first.
if svc.Spec.ExternalName != "" {
validCondition.AddErrorf(contour_api_v1.ConditionTypeServiceError, "UnsupportedServiceType",
"Service %q is of unsupported type %q.", svcName, corev1.ServiceTypeExternalName)
Expand Down

0 comments on commit b53a5c4

Please sign in to comment.