From d09d882dc6d7a796cae6c3b5265641054ffe8d44 Mon Sep 17 00:00:00 2001
From: Krzysztof Ostrowski <kostrows@redhat.com>
Date: Tue, 7 Nov 2023 21:40:54 +0100
Subject: [PATCH] c/k/a/opts: check for secure connection on h2c,rh

h2c is unencrypted http/2 and we don't want to allow it outside of
localhost.
request headers functionality is setting confidential in the headers, we
want this to happen only through mTLS. We can't guarantee that upstream
verifies client certificates.
---
 Makefile                                      |  6 +-
 .../app/options/proxyoptions.go               | 68 ++++++++++++++++++-
 2 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 885887d1a..d1a980edd 100644
--- a/Makefile
+++ b/Makefile
@@ -94,8 +94,10 @@ test-unit:
 test-e2e:
 	go test -timeout 55m -v ./test/e2e/ $(TEST_RUN_ARGS) --kubeconfig=$(KUBECONFIG)
 
-test-local:
-	@echo 'run: VERSION=local make clean container kind-create-cluster test'
+test-local-setup: VERSION = local
+test-local-setup: VERSION_SEMVER = $(shell cat VERSION)
+test-local-setup: clean container kind-create-cluster
+test-local: test-local-setup test-e2e
 
 kind-delete-cluster:
 	kind delete cluster
diff --git a/cmd/kube-rbac-proxy/app/options/proxyoptions.go b/cmd/kube-rbac-proxy/app/options/proxyoptions.go
index b8640652c..b8742c13f 100644
--- a/cmd/kube-rbac-proxy/app/options/proxyoptions.go
+++ b/cmd/kube-rbac-proxy/app/options/proxyoptions.go
@@ -17,10 +17,14 @@ limitations under the License.
 package options
 
 import (
+	"context"
+	"crypto/tls"
 	"fmt"
+	"net"
 	"net/url"
 	"os"
 	"path"
+	"time"
 
 	"github.com/ghodss/yaml"
 	"github.com/spf13/pflag"
@@ -36,8 +40,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 +62,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 +109,26 @@ func (o *ProxyOptions) Validate() []error {
 		}
 	}
 
+	// Verify secure connection settings.
+	isSecureConnectionRequired := len(o.UpstreamHeader.GroupsFieldName) > 0 ||
+		len(o.UpstreamHeader.UserFieldName) > 0 ||
+		o.UpstreamForceH2C
+	if isSecureConnectionRequired {
+		ctx, cancel := context.WithTimeout(context.Background(), time.Duration(o.UpstreamDNSTimeout)*time.Second)
+		defer cancel()
+
+		couldBeMTLS, mTLSErr := isMTLSMaybe(o.UpstreamClientCertFile, o.UpstreamClientKeyFile)
+		isLoopback, loopbackErr := isLoopbackAddress(ctx, o.Upstream)
+
+		if !couldBeMTLS && !isLoopback {
+			errs = append(
+				errs,
+				fmt.Errorf("secure connection is required, but not configured: %w", mTLSErr),
+				fmt.Errorf("secure connection is required, but not configured: %w", loopbackErr),
+			)
+		}
+	}
+
 	return errs
 }
 
@@ -132,6 +158,44 @@ func (o *ProxyOptions) ApplyTo(c *server.KubeRBACProxyInfo, a *serverconfig.Auth
 	return nil
 }
 
+func isMTLSMaybe(upstreamClientCertPath, upstreamClientKeyPath string) (bool, error) {
+	if len(upstreamClientCertPath) > 0 {
+		_, err := tls.LoadX509KeyPair(upstreamClientCertPath, upstreamClientKeyPath)
+		if err != nil {
+			return false, fmt.Errorf("failed to read upstream client cert/key: %w", err)
+		}
+
+		return true, nil
+	}
+
+	return false, 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"`
 }