From 8d12c1b67412fe1a3173ecec90a5d20ed1fc90a6 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 24 Mar 2022 15:46:10 -0700 Subject: [PATCH] HTTP listener: default disabled and may only bind to loopback interfaces --- Dockerfile | 2 +- deploy/supervisor/deployment.yaml | 4 +- deploy/supervisor/values.yaml | 33 +++---- internal/config/supervisor/config.go | 52 ++++++++++- internal/config/supervisor/config_test.go | 93 +++++++++++++++++-- .../docs/howto/configure-supervisor.md | 24 ++--- .../concierge-and-supervisor-demo.md | 2 +- 7 files changed, 165 insertions(+), 45 deletions(-) diff --git a/Dockerfile b/Dockerfile index 5588ff77..c5087fee 100644 --- a/Dockerfile +++ b/Dockerfile @@ -30,7 +30,7 @@ FROM gcr.io/distroless/static:nonroot@sha256:80c956fb0836a17a565c43a4026c9c80b20 COPY --from=build-env /usr/local/bin /usr/local/bin # Document the default server ports for the various server apps -EXPOSE 8080 8443 8444 10250 +EXPOSE 8443 8444 10250 # Run as non-root for security posture # Use the same non-root user as https://github.com/GoogleContainerTools/distroless/blob/fc3c4eaceb0518900f886aae90407c43be0a42d9/base/base.bzl#L9 diff --git a/deploy/supervisor/deployment.yaml b/deploy/supervisor/deployment.yaml index 1bd49903..b4c60ec2 100644 --- a/deploy/supervisor/deployment.yaml +++ b/deploy/supervisor/deployment.yaml @@ -1,4 +1,4 @@ -#! Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +#! Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. #! SPDX-License-Identifier: Apache-2.0 #@ load("@ytt:data", "data") @@ -115,8 +115,6 @@ spec: readOnly: false #! writable to allow for socket use #@ end ports: - - containerPort: 8080 - protocol: TCP - containerPort: 8443 protocol: TCP env: diff --git a/deploy/supervisor/values.yaml b/deploy/supervisor/values.yaml index 9474da11..e22d0306 100644 --- a/deploy/supervisor/values.yaml +++ b/deploy/supervisor/values.yaml @@ -1,4 +1,4 @@ -#! Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +#! Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. #! SPDX-License-Identifier: Apache-2.0 #@data/values @@ -74,17 +74,17 @@ api_group_suffix: pinniped.dev https_proxy: #! e.g. http://proxy.example.com no_proxy: "$(KUBERNETES_SERVICE_HOST),169.254.169.254,127.0.0.1,localhost,.svc,.cluster.local" #! do not proxy Kubernetes endpoints -#! Control the https and http listeners of the Supervisor. +#! Control the HTTP and HTTPS listeners of the Supervisor. #! #! The schema of this config is as follows: #! #! endpoints: #! https: #! network: tcp | unix | disabled -#! address: interface:port when network=tcp or /pinniped_socket/socketfile.sock when network=unix +#! address: host:port when network=tcp or /pinniped_socket/socketfile.sock when network=unix #! http: #! network: same as above -#! address: same as above +#! address: same as above, except that when network=tcp then the address is only allowed to bind to loopback interfaces #! #! Setting network to disabled turns off that particular listener. #! See https://pkg.go.dev/net#Listen and https://pkg.go.dev/net#Dial for a description of what can be @@ -98,23 +98,20 @@ no_proxy: "$(KUBERNETES_SERVICE_HOST),169.254.169.254,127.0.0.1,localhost,.svc,. #! network: tcp #! address: :8443 #! http: -#! network: tcp -#! address: :8080 +#! network: disabled #! -#! These defaults mean: bind to all interfaces using TCP. Use port 8443 for https and 8080 for http. -#! The defaults will change over time. Users should explicitly set this value if they wish to avoid -#! any changes on upgrade. +#! These defaults mean: For HTTPS listening, bind to all interfaces using TCP on port 8443. +#! Disable HTTP listening by default. #! -#! A future version of the Supervisor app may include a breaking change to adjust the default -#! behavior of the http listener to only listen on 127.0.0.1 (or perhaps even to be disabled). +#! The HTTP listener can only be bound to loopback interfaces. This allows the listener to accept +#! traffic from within the pod, e.g. from a service mesh sidecar. The HTTP listener should not be +#! used to accept traffic from outside the pod, since that would mean that the network traffic could be +#! transmitted unencrypted. The HTTPS listener should be used instead to accept traffic from outside the pod. +#! Ingresses and load balancers that terminate TLS connections should re-encrypt the data and route traffic +#! to the HTTPS listener. Unix domain sockets may also be used for integrations with service meshes. #! -#! Binding the http listener to addresses other than 127.0.0.1 or ::1 is deprecated. -#! -#! Unix domain sockets are recommended for integrations with service meshes. Ingresses that terminate -#! TLS connections at the edge should re-encrypt the data and route traffic to the https listener. -#! -#! Changing the port numbers used must be accompanied with matching changes to the service and deployment -#! manifests. Changes to the https listener must be coordinated with the deployment health checks. +#! Changing the HTTPS port number must be accompanied by matching changes to the service and deployment +#! manifests. Changes to the HTTPS listener must be coordinated with the deployment health checks. #! #! Optional. endpoints: diff --git a/internal/config/supervisor/config.go b/internal/config/supervisor/config.go index 1118c0ec..3e8412f3 100644 --- a/internal/config/supervisor/config.go +++ b/internal/config/supervisor/config.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package supervisor contains functionality to load/store Config's from/to @@ -8,6 +8,7 @@ package supervisor import ( "fmt" "io/ioutil" + "net" "strings" "k8s.io/utils/pointer" @@ -66,8 +67,7 @@ func FromPath(path string) (*Config, error) { Address: ":8443", }) maybeSetEndpointDefault(&config.Endpoints.HTTP, Endpoint{ - Network: NetworkTCP, - Address: ":8080", + Network: NetworkDisabled, }) if err := validateEndpoint(*config.Endpoints.HTTPS); err != nil { @@ -76,6 +76,9 @@ func FromPath(path string) (*Config, error) { if err := validateEndpoint(*config.Endpoints.HTTP); err != nil { return nil, fmt.Errorf("validate http endpoint: %w", err) } + if err := validateAdditionalHTTPEndpointRequirements(*config.Endpoints.HTTP); err != nil { + return nil, fmt.Errorf("validate http endpoint: %w", err) + } if err := validateAtLeastOneEnabledEndpoint(*config.Endpoints.HTTPS, *config.Endpoints.HTTP); err != nil { return nil, fmt.Errorf("validate endpoints: %w", err) } @@ -128,6 +131,16 @@ func validateEndpoint(endpoint Endpoint) error { } } +func validateAdditionalHTTPEndpointRequirements(endpoint Endpoint) error { + if endpoint.Network == NetworkTCP && !addrIsOnlyOnLoopback(endpoint.Address) { + return fmt.Errorf( + "http listener address %q for %q network may only bind to loopback interfaces", + endpoint.Address, + endpoint.Network) + } + return nil +} + func validateAtLeastOneEnabledEndpoint(endpoints ...Endpoint) error { for _, endpoint := range endpoints { if endpoint.Network != NetworkDisabled { @@ -136,3 +149,36 @@ func validateAtLeastOneEnabledEndpoint(endpoints ...Endpoint) error { } return constable.Error("all endpoints are disabled") } + +// For tcp networks, the address can be in several formats: host:port, host:, and :port. +// See address description in https://pkg.go.dev/net#Listen and https://pkg.go.dev/net#Dial. +// The host may be a literal IP address, or a host name that can be resolved to IP addresses, +// or a literal unspecified IP address (as in "0.0.0.0:80" or "[::]:80"), or empty. +// If the host is a literal IPv6 address it must be enclosed in square brackets, as in "[2001:db8::1]:80" or +// "[fe80::1%zone]:80". The zone specifies the scope of the literal IPv6 address as defined in RFC 4007. +// The port may be a literal port number or a service name, the value 0, or empty. +// Returns true if a net.Listen listener at this address would only listen on loopback interfaces. +// Returns false if the listener would listen on any non-loopback interfaces, or when called with illegal input. +func addrIsOnlyOnLoopback(addr string) bool { + // First try parsing as a `host:port`. net.SplitHostPort allows empty host and empty port. + host, _, err := net.SplitHostPort(addr) + if err != nil { + // Illegal input. + return false + } + if host == "" { + // Input was :port. This would bind to all interfaces, so it is not only on loopback. + return false + } + if host == "localhost" { + // This is only on loopback. + return true + } + // The host could be a hostname, an IPv4 address, or an IPv6 address. + ip := net.ParseIP(host) + if ip == nil { + // The address was not an IP. It must have been some hostname other than "localhost". + return false + } + return ip.IsLoopback() +} diff --git a/internal/config/supervisor/config_test.go b/internal/config/supervisor/config_test.go index 3953238e..285867b4 100644 --- a/internal/config/supervisor/config_test.go +++ b/internal/config/supervisor/config_test.go @@ -1,16 +1,16 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package supervisor import ( + "fmt" "io/ioutil" "os" "testing" - "k8s.io/utils/pointer" - "github.com/stretchr/testify/require" + "k8s.io/utils/pointer" "go.pinniped.dev/internal/here" ) @@ -37,7 +37,8 @@ func TestFromPath(t *testing.T) { network: unix address: :1234 http: - network: disabled + network: tcp + address: 127.0.0.1:1234 `), wantConfig: &Config{ APIGroupSuffix: pointer.StringPtr("some.suffix.com"), @@ -54,7 +55,8 @@ func TestFromPath(t *testing.T) { Address: ":1234", }, HTTP: &Endpoint{ - Network: "disabled", + Network: "tcp", + Address: "127.0.0.1:1234", }, }, }, @@ -78,8 +80,7 @@ func TestFromPath(t *testing.T) { Address: ":8443", }, HTTP: &Endpoint{ - Network: "tcp", - Address: ":8080", + Network: "disabled", }, }, }, @@ -126,6 +127,21 @@ func TestFromPath(t *testing.T) { `), wantError: `validate http endpoint: unknown network "bar"`, }, + { + name: "http endpoint uses tcp but binds to more than only loopback interfaces", + yaml: here.Doc(` + --- + names: + defaultTLSCertificateSecret: my-secret-name + endpoints: + https: + network: disabled + http: + network: tcp + address: :8080 + `), + wantError: `validate http endpoint: http listener address ":8080" for "tcp" network may only bind to loopback interfaces`, + }, { name: "endpoint disabled with non-empty address", yaml: here.Doc(` @@ -208,3 +224,66 @@ func TestFromPath(t *testing.T) { }) } } + +func TestAddrIsOnlyOnLoopback(t *testing.T) { + tests := []struct { + addr string + want bool + }{ + {addr: "localhost:", want: true}, + {addr: "localhost:0", want: true}, + {addr: "localhost:80", want: true}, + {addr: "localhost:http", want: true}, + {addr: "127.0.0.1:", want: true}, + {addr: "127.0.0.1:0", want: true}, + {addr: "127.0.0.1:80", want: true}, + {addr: "127.0.0.1:http", want: true}, + {addr: "[::1]:", want: true}, + {addr: "[::1]:0", want: true}, + {addr: "[::1]:80", want: true}, + {addr: "[::1]:http", want: true}, + {addr: "[0:0:0:0:0:0:0:1]:", want: true}, + {addr: "[0:0:0:0:0:0:0:1]:0", want: true}, + {addr: "[0:0:0:0:0:0:0:1]:80", want: true}, + {addr: "[0:0:0:0:0:0:0:1]:http", want: true}, + {addr: "", want: false}, // illegal input, can't be empty + {addr: "host", want: false}, // illegal input, need colon + {addr: "localhost", want: false}, // illegal input, need colon + {addr: "127.0.0.1", want: false}, // illegal input, need colon + {addr: ":", want: false}, // illegal input, need either host or port + {addr: "2001:db8::1:80", want: false}, // illegal input, forgot square brackets + {addr: ":0", want: false}, + {addr: ":80", want: false}, + {addr: ":http", want: false}, + {addr: "notlocalhost:", want: false}, + {addr: "notlocalhost:0", want: false}, + {addr: "notlocalhost:80", want: false}, + {addr: "notlocalhost:http", want: false}, + {addr: "0.0.0.0:", want: false}, + {addr: "0.0.0.0:0", want: false}, + {addr: "0.0.0.0:80", want: false}, + {addr: "0.0.0.0:http", want: false}, + {addr: "[::]:", want: false}, + {addr: "[::]:0", want: false}, + {addr: "[::]:80", want: false}, + {addr: "[::]:http", want: false}, + {addr: "42.42.42.42:", want: false}, + {addr: "42.42.42.42:0", want: false}, + {addr: "42.42.42.42:80", want: false}, + {addr: "42.42.42.42:http", want: false}, + {addr: "[2001:db8::1]:", want: false}, + {addr: "[2001:db8::1]:0", want: false}, + {addr: "[2001:db8::1]:80", want: false}, + {addr: "[2001:db8::1]:http", want: false}, + {addr: "[fe80::1%zone]:", want: false}, + {addr: "[fe80::1%zone]:0", want: false}, + {addr: "[fe80::1%zone]:80", want: false}, + {addr: "[fe80::1%zone]:http", want: false}, + } + for _, test := range tests { + tt := test + t.Run(fmt.Sprintf("address %s should be %t", tt.addr, tt.want), func(t *testing.T) { + require.Equal(t, tt.want, addrIsOnlyOnLoopback(tt.addr)) + }) + } +} diff --git a/site/content/docs/howto/configure-supervisor.md b/site/content/docs/howto/configure-supervisor.md index b6daf592..cfbee2cf 100644 --- a/site/content/docs/howto/configure-supervisor.md +++ b/site/content/docs/howto/configure-supervisor.md @@ -46,26 +46,26 @@ The most common ways are: configured with TLS certificates and will terminate the TLS connection itself (see the section about FederationDomain below). The LoadBalancer Service should be configured to use the HTTPS port 443 of the Supervisor pods as its `targetPort`. - *Warning:* Never expose the Supervisor's HTTP port 8080 to the public. It would not be secure for the OIDC protocol - to use HTTP, because the user's secret OIDC tokens would be transmitted across the network without encryption. - - Or, define an [Ingress resource](https://kubernetes.io/docs/concepts/services-networking/ingress/). In this case, the [Ingress typically terminates TLS](https://kubernetes.io/docs/concepts/services-networking/ingress/#tls) - and then talks plain HTTP to its backend, - which would be a NodePort or LoadBalancer Service in front of the HTTP port 8080 of the Supervisor pods. - However, because the Supervisor's endpoints deal with sensitive credentials, it is much better if the - traffic is encrypted using TLS all the way into the Supervisor's Pods. Some Ingress implementations - may support re-encrypting the traffic before sending it to the backend. If your Ingress controller does not - support this, then consider using one of the other configurations described here instead of using an Ingress. + and then talks plain HTTP to its backend. + However, because the Supervisor's endpoints deal with sensitive credentials, the ingress must be configured to re-encrypt + traffic using TLS on the backend (upstream) into the Supervisor's Pods. It would not be secure for the OIDC protocol + to use HTTP, because the user's secret OIDC tokens would be transmitted across the network without encryption. + If your Ingress controller does not support this feature, then consider using one of the other configurations + described here instead of using an Ingress. The backend of the Ingress would typically point to a NodePort or + LoadBalancer Service which exposes the HTTPS port 8443 of the Supervisor pods. The required configuration of the Ingress is specific to your cluster's Ingress Controller, so please refer to the documentation from your Kubernetes provider. If you are using a cluster from a cloud provider, then you'll probably want to start with that provider's documentation. For example, if your cluster is a Google GKE cluster, refer to - the [GKE documentation for Ingress](https://cloud.google.com/kubernetes-engine/docs/concepts/ingress). + the [GKE documentation for Ingress](https://cloud.google.com/kubernetes-engine/docs/concepts/ingress) and the + [GKE documentation for enabling TLS on the backend of an Ingress](https://cloud.google.com/kubernetes-engine/docs/concepts/ingress-xlb#https_tls_between_load_balancer_and_your_application). Otherwise, the Kubernetes documentation provides a list of popular [Ingress Controllers](https://kubernetes.io/docs/concepts/services-networking/ingress-controllers/), including - [Contour](https://projectcontour.io/) and many others. + [Contour](https://projectcontour.io/) and many others. Contour is an example of an ingress implementation which + [supports TLS on the backend](https://projectcontour.io/docs/main/config/upstream-tls/). - Or, expose the Supervisor app using a Kubernetes service mesh technology (e.g. [Istio](https://istio.io/)). @@ -133,7 +133,7 @@ spec: ports: - protocol: TCP port: 443 - targetPort: 8443 # 8443 is the TLS port. Do not expose port 8080. + targetPort: 8443 # 8443 is the TLS port. ``` ### Example: Creating a NodePort Service diff --git a/site/content/docs/tutorials/concierge-and-supervisor-demo.md b/site/content/docs/tutorials/concierge-and-supervisor-demo.md index 33cab765..5131aba1 100644 --- a/site/content/docs/tutorials/concierge-and-supervisor-demo.md +++ b/site/content/docs/tutorials/concierge-and-supervisor-demo.md @@ -218,7 +218,7 @@ spec: ports: - protocol: TCP port: 443 - targetPort: 8443 # 8443 is the TLS port. Do not expose port 8080. + targetPort: 8443 # 8443 is the TLS port. EOF ```