c/k/a/options: enforce loopback or mTLS for some

h2c must happen on a loopback connection as the connection is not using TLS at
all.
identity headers must use a loopback connection or a mTLS conection is
required. Trust in both directions is important. kube-rbac-proxy needs
to provide certs, such that upstream can verify the authenticity of the
headers. upstream needs certs, such that we can be sure to not leak
secrets.

cmd/kube-rbac-proxy/app/options/proxyoptions.go

@@ -17,10 +17,16 @@ limitations under the License.
 package options
 
 import (
+	"context"
+	"crypto/tls"
+	"crypto/x509"
+	"errors"
 	"fmt"
+	"net"
 	"net/url"
 	"os"
 	"path"
+	"time"
 
 	"github.com/ghodss/yaml"
 	"github.com/spf13/pflag"
@@ -36,8 +42,9 @@ import (
 
 // ProxyOptions are options specific to the kube-rbac-proxy
 type ProxyOptions struct {
-	Upstream         string
-	UpstreamForceH2C bool
+	Upstream           string
+	UpstreamForceH2C   bool
+	UpstreamDNSTimeout int
 
 	UpstreamCAFile         string
 	UpstreamClientCertFile string
@@ -57,6 +64,7 @@ type ProxyOptions struct {
 func (o *ProxyOptions) AddFlags(flagset *pflag.FlagSet) {
 	flagset.StringVar(&o.Upstream, "upstream", "", "The upstream URL to proxy to once requests have successfully been authenticated and authorized.")
 	flagset.BoolVar(&o.UpstreamForceH2C, "upstream-force-h2c", false, "Force h2c to communicate with the upstream. This is required when the upstream speaks h2c(http/2 cleartext - insecure variant of http/2) only. For example, go-grpc server in the insecure mode, such as helm's tiller w/o TLS, speaks h2c only")
+	flagset.IntVar(&o.UpstreamDNSTimeout, "upstream-dns-timeout", 5, "The timeout in seconds for DNS lookups of the upstream. If set to 0, no timeout is set.")
 
 	// upstream tls options
 	flagset.StringVar(&o.UpstreamCAFile, "upstream-ca-file", "", "The CA the upstream uses for TLS connection. This is required when the upstream uses TLS and its own CA certificate")
@@ -103,6 +111,17 @@ func (o *ProxyOptions) Validate() []error {
 		}
 	}
 
+	// Verify secure connection settings, if necessary.
+	if identityheaders.HasIdentityHeadersEnabled(o.UpstreamHeader) || o.UpstreamForceH2C {
+		isSecure, err := hasSecureConnection(o)
+		if err != nil {
+			errs = append(errs, err)
+		}
+		if !isSecure {
+			errs = append(errs, errors.New("configuration requires a secure connection (mTLS or loopback) to the upstream (h2c/identity headers)"))
+		}
+	}
+
 	return errs
 }
 
@@ -125,13 +144,87 @@ func (o *ProxyOptions) ApplyTo(c *server.KubeRBACProxyInfo, a *serverconfig.Auth
 		}
 	}
 
+	c.UpstreamHeaders = o.UpstreamHeader
 	c.IgnorePaths = o.IgnorePaths
 	c.AllowPaths = o.AllowPaths
 	a.APIAudiences = o.TokenAudiences
 
 	return nil
 }
 
+func hasSecureConnection(o *ProxyOptions) (bool, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(o.UpstreamDNSTimeout)*time.Second)
+	defer cancel()
+
+	isLoopback, err := isLoopbackAddress(ctx, o.Upstream)
+	if err == nil && isLoopback {
+		return true, nil
+	}
+
+	if o.UpstreamForceH2C {
+		return false, fmt.Errorf("loopback address is required for h2c")
+	}
+
+	// isMTLSPossible emphasizes that we can't verify that upstream verifies the
+	// client's certificate.
+	isMTLSPossible, err := isMTLSConfigured(o.UpstreamClientCertFile, o.UpstreamClientKeyFile, o.UpstreamCAFile)
+	if !isMTLSPossible || err != nil {
+		return false, err
+	}
+
+	return true, nil
+}
+
+func isMTLSConfigured(upstreamClientCertPath, upstreamClientKeyPath, upstreamCAPath string) (bool, error) {
+	// Check if client is configured to provide a certificiate.
+	if len(upstreamClientCertPath) > 0 {
+		return false, nil
+	}
+
+	_, err := tls.LoadX509KeyPair(upstreamClientCertPath, upstreamClientKeyPath)
+	if err != nil {
+		return false, fmt.Errorf("failed to read upstream client cert/key: %w", err)
+	}
+
+	// Check if we have a CA to verify upstream.
+	upstreamCAPEM, err := os.ReadFile(upstreamCAPath)
+	if err != nil {
+		return false, fmt.Errorf("failed to read the upstream CA file: %w", err)
+	}
+
+	upstreamCACertPool := x509.NewCertPool()
+	if ok := upstreamCACertPool.AppendCertsFromPEM(upstreamCAPEM); !ok {
+		return false, errors.New("error parsing upstream CA certificate")
+	}
+
+	return true, nil
+}
+
+func isLoopbackAddress(ctx context.Context, address string) (bool, error) {
+	u, err := url.Parse(address)
+	if err != nil {
+		return false, fmt.Errorf("failed to parse upstream URL: %w", err)
+	}
+
+	ip := net.ParseIP(u.Hostname())
+	if ip != nil {
+		return ip.IsLoopback(), nil
+	}
+
+	ips, err := (&net.Resolver{}).LookupIPAddr(ctx, u.Hostname())
+	if err != nil {
+		return false, fmt.Errorf("failed to lookup ip: %w", err)
+	}
+
+	for _, ip := range ips {
+		if !ip.IP.IsLoopback() {
+			return false, nil
+		}
+	}
+
+	return true, nil
+}
+
 type configfile struct {
 	AuthorizationConfig *authz.AuthzConfig `json:"authorization,omitempty"`
 }

pkg/authn/identityheaders/identityheaders.go

@@ -38,8 +38,7 @@ type AuthnHeaderConfig struct {
 // WithAuthHeaders adds identity information to the headers.
 // Must not be used, if connection is not encrypted with TLS.
 func WithAuthHeaders(handler http.Handler, cfg *AuthnHeaderConfig) http.Handler {
-	upstreamHeadersEnabled := len(cfg.GroupsFieldName) > 0 || len(cfg.UserFieldName) > 0
-	if !upstreamHeadersEnabled {
+	if !HasIdentityHeadersEnabled(cfg) {
 		return handler
 	}
 
@@ -55,3 +54,7 @@ func WithAuthHeaders(handler http.Handler, cfg *AuthnHeaderConfig) http.Handler
 		handler.ServeHTTP(w, req)
 	})
 }
+
+func HasIdentityHeadersEnabled(cfg *AuthnHeaderConfig) bool {
+	return len(cfg.GroupsFieldName) > 0 || len(cfg.UserFieldName) > 0
+}

test/e2e/h2c-upstream/deployment-proxy-non-loopback.yaml

@@ -0,0 +1,35 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: kube-rbac-proxy
+  namespace: default
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app: kube-rbac-proxy
+  template:
+    metadata:
+      labels:
+        app: kube-rbac-proxy
+    spec:
+      serviceAccountName: kube-rbac-proxy
+      containers:
+        - name: kube-rbac-proxy
+          image: quay.io/brancz/kube-rbac-proxy:local
+          args:
+            - "--secure-port=8443"
+            - "--upstream=http://http-echo-service.default.svc.cluster.local:80/"
+            - "--authentication-skip-lookup"
+            - "--upstream-force-h2c=true"
+            - "--logtostderr=true"
+            - "--v=10"
+          ports:
+            - containerPort: 8443
+              name: https
+        - name: prometheus-example-app
+          image: quay.io/brancz/prometheus-example-app:v0.4.0
+          args:
+            - "--bind=127.0.0.1:8081"
+            - "--h2c=true"
+

test/e2e/h2c-upstream/deployment-upstream-non-loopback.yaml

@@ -0,0 +1,36 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: http-echo
+  labels:
+    app: http-echo
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app: http-echo
+  template:
+    metadata:
+      labels:
+        app: http-echo
+    spec:
+      containers:
+      - name: http-echo
+        image: mendhak/http-https-echo
+        env:
+        - name: HTTP_PORT
+          value: 8080
+        ports:
+        - containerPort: 8080
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: http-echo-service
+spec:
+  ports:
+  - port: 80
+    targetPort: 8080
+  selector:
+    app: http-echo
+

test/e2e/h2c_upstream.go

@@ -29,7 +29,29 @@ func testH2CUpstream(client kubernetes.Interface) kubetest.TestSuite {
 		command := `curl --connect-timeout 5 -v -s -k --fail -H "Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" https://kube-rbac-proxy.default.svc.cluster.local:8443/metrics`
 
 		kubetest.Scenario{
-			Name: "With H2C Upstream",
+			Name: "With H2C non-local upstream",
+
+			Given: kubetest.Actions(
+				kubetest.CreatedManifests(
+					client,
+					"h2c-upstream/clusterRole.yaml",
+					"h2c-upstream/clusterRoleBinding.yaml",
+					"h2c-upstream/deployment-proxy-non-loopback.yaml",
+					"h2c-upstream/deployment-upstream-non-loopback.yaml",
+					"h2c-upstream/service.yaml",
+					"h2c-upstream/serviceAccount.yaml",
+				),
+			),
+			Then: kubetest.Actions(
+				kubetest.PodIsCrashLoopBackOff(
+					client,
+					"kube-rbac-proxy",
+				),
+			),
+		}.Run(t)
+
+		kubetest.Scenario{
+			Name: "With H2C local upstream",
 
 			Given: kubetest.Actions(
 				kubetest.CreatedManifests(

test/e2e/identityheaders.go

@@ -0,0 +1,117 @@
+package e2e
+
+import (
+	"testing"
+
+	"github.com/brancz/kube-rbac-proxy/test/kubetest"
+	"k8s.io/client-go/kubernetes"
+)
+
+func testIdentityHeaders(client kubernetes.Interface) kubetest.TestSuite {
+	return func(t *testing.T) {
+		command := `curl --connect-timeout 5 -v -s -k --fail -H "Authorization: Bearer $(cat /var/run/secrets/kubernetes.io/serviceaccount/token)" https://kube-rbac-proxy.default.svc.cluster.local:8443/metrics`
+
+		kubetest.Scenario{
+			Name: "With x-remote-user",
+			Description: `
+				Verifies that remote user is set to the service account, when
+				upstreama is listening on loopback through a HTTP connection.
+			`,
+
+			Given: kubetest.Actions(
+				kubetest.CreatedManifests(
+					client,
+					"identityheaders/default/clusterRole-client.yaml",
+					"identityheaders/default/clusterRole.yaml",
+					"identityheaders/default/clusterRoleBinding-client.yaml",
+					"identityheaders/default/clusterRoleBinding.yaml",
+					"identityheaders/default/configmap-nginx.yaml",
+					"identityheaders/default/deployment.yaml",
+					"identityheaders/default/service.yaml",
+					"identityheaders/default/serviceAccount.yaml",
+				),
+			),
+			When: kubetest.Actions(
+				kubetest.PodsAreReady(
+					client,
+					1,
+					"app=kube-rbac-proxy",
+				),
+				kubetest.ServiceIsReady(
+					client,
+					"kube-rbac-proxy",
+				),
+			),
+			Then: kubetest.Actions(
+				kubetest.ClientLogsContain(
+					client,
+					command,
+					[]string{`< x-remote-user: system:serviceaccount:default:default`},
+					nil,
+				),
+			),
+		}.Run(t)
+
+		kubetest.Scenario{
+			Name: "With http on no loopback",
+			Description: `
+					Verifies that the proxy is not able to connect to the remote upstream service,
+					if upstream isn't offering TLS, when identity headers are being used.
+				`,
+
+			Given: kubetest.Actions(
+				kubetest.CreatedManifests(
+					client,
+					"identityheaders/insecure/clusterRole-client.yaml",
+					"identityheaders/insecure/clusterRole.yaml",
+					"identityheaders/insecure/clusterRoleBinding-client.yaml",
+					"identityheaders/insecure/clusterRoleBinding.yaml",
+					"identityheaders/insecure/deployment-proxy.yaml",
+					"identityheaders/insecure/deployment-upstream.yaml",
+					"identityheaders/insecure/service.yaml",
+					"identityheaders/insecure/serviceAccount.yaml",
+				),
+			),
+			Then: kubetest.Actions(
+				kubetest.PodIsCrashLoopBackOff(
+					client,
+					"kube-rbac-proxy",
+				),
+			),
+		}.Run(t)
+
+		kubetest.Scenario{
+			Name: "With https on no loopback",
+			Description: `
+					Verifies that the proxy is able to connect to the remote upstream service,
+					through a mTLS connection, when providing identity headers.
+				`,
+
+			Given: kubetest.Actions(
+				kubetest.CreateServerCerts(client, "nginx"),
+				kubetest.CreateClientCerts(client, "kube-rbac-proxy-client"),
+				kubetest.CreateServerCerts(client, "kube-rbac-proxy"),
+				kubetest.CreatedManifests(
+					client,
+					"identityheaders/secure/clusterRole-client.yaml",
+					"identityheaders/secure/clusterRole.yaml",
+					"identityheaders/secure/clusterRoleBinding-client.yaml",
+					"identityheaders/secure/clusterRoleBinding.yaml",
+					"identityheaders/secure/configmap-nginx.yaml",
+					"identityheaders/secure/deployment-proxy.yaml",
+					"identityheaders/secure/deployment-upstream.yaml",
+					"identityheaders/secure/service-proxy.yaml",
+					"identityheaders/secure/service-upstream.yaml",
+					"identityheaders/secure/serviceAccount.yaml",
+				),
+			),
+			Then: kubetest.Actions(
+				kubetest.ClientSucceeds(
+					client,
+					command,
+					nil,
+				),
+			),
+		}.Run(t)
+	}
+}

test/e2e/identityheaders/default/clusterRole-client.yaml

@@ -0,0 +1,7 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: metrics
+rules:
+  - nonResourceURLs: ["/metrics"]
+    verbs: ["get"]