fix: reject non-HTTPS originating requests

- CloudFoundry apps (such as the this Cloud Service Broker) only need to
  handle HTTP as CloudFoundry handles TLS termination, simplifying the
  app
- It was noted that although the broker is always registered as an HTTPS
  endpoint, it can still accept HTTP requests, which adds potential for
  erroneous configuration and a subsequent plaintext transmission of
  service instance credentials.
- This fix changes the CSB to examine the "X-Forwarded-Proto" HTTP
  header to ensure that all requests were sent to CloudFoundry as HTTPS,
  and if not they are redirected to the appropriate HTTPS endpoint
- Some folks used to run the CSB in Kubernetes or other non-CloudFoundry
  environments, and we don't believe anyone does this any more. But if
  anyone does they can:
  - Disable this behavior by setting `CSB_LISTENER_ALLOW_HTTP`
  - Raising an issue, so that the authors are made aware that we broke
    you and that you have a non-CloudFoundry use case.

[#187357359](https://www.pivotaltracker.com/story/show/187357359)

cmd/serve.go

@@ -27,6 +27,7 @@ import (
 	"github.com/cloudfoundry/cloud-service-broker/dbservice"
 	"github.com/cloudfoundry/cloud-service-broker/internal/displaycatalog"
 	"github.com/cloudfoundry/cloud-service-broker/internal/encryption"
+	"github.com/cloudfoundry/cloud-service-broker/internal/httpsmiddleware"
 	"github.com/cloudfoundry/cloud-service-broker/internal/infohandler"
 	"github.com/cloudfoundry/cloud-service-broker/internal/storage"
 	pakBroker "github.com/cloudfoundry/cloud-service-broker/pkg/broker"
@@ -46,6 +47,7 @@ const (
 	apiPasswordProp     = "api.password"
 	apiPortProp         = "api.port"
 	apiHostProp         = "api.host"
+	apiAllowHTTP        = "api.allow.http" // Disables the middleware that redirects http traffic to https
 	encryptionPasswords = "db.encryption.passwords"
 	encryptionEnabled   = "db.encryption.enabled"
 )
@@ -79,6 +81,7 @@ func init() {
 	_ = viper.BindEnv(apiHostProp, "CSB_LISTENER_HOST")
 	_ = viper.BindEnv(encryptionPasswords, "ENCRYPTION_PASSWORDS")
 	_ = viper.BindEnv(encryptionEnabled, "ENCRYPTION_ENABLED")
+	_ = viper.BindEnv(apiAllowHTTP, "CSB_LISTENER_ALLOW_HTTP")
 }
 
 func serve() {
@@ -201,10 +204,11 @@ func startServer(registry pakBroker.BrokerRegistry, db *sql.DB, brokerapi http.H
 		}
 	})
 
+	allowHTTP := len(viper.GetString(apiAllowHTTP)) != 0
 	port := viper.GetString(apiPortProp)
 	host := viper.GetString(apiHostProp)
 	logger.Info("Serving", lager.Data{"port": port})
-	_ = http.ListenAndServe(fmt.Sprintf("%s:%s", host, port), router)
+	_ = http.ListenAndServe(fmt.Sprintf("%s:%s", host, port), httpsmiddleware.EnsureHTTPS(router, logger, allowHTTP))
 }
 
 func labelName(label string) string {

integrationtest/fixtures/https-redirect/fake-service.yml

@@ -0,0 +1,15 @@
+version: 1
+name: alpha-service
+
+id: 76c5725c-b246-11eb-871f-ffc97563fbd0
+description: description
+display_name: Alpha Service
+image_url: https://example.com/icon.jpg
+documentation_url: https://example.com
+support_url: https://example.com/support.html
+plans:
+- name: alpha
+  id: 8b52a460-b246-11eb-a8f5-d349948e2480
+  description: Alpha plan
+  display_name: Alpha
+

integrationtest/fixtures/https-redirect/manifest.yml

@@ -0,0 +1,16 @@
+packversion: 1
+name: fake-brokerpak
+version: 0.1.0
+metadata:
+  author: [email protected]
+platforms:
+- os: linux
+  arch: amd64
+- os: darwin
+  arch: amd64
+terraform_binaries:
+- name: tofu
+  version: 1.6.0
+  source: https://github.com/opentofu/opentofu/archive/refs/tags/v1.6.0.zip
+service_definitions:
+- fake-service.yml

integrationtest/https_redirect_test.go

@@ -0,0 +1,46 @@
+package integrationtest_test
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/cloudfoundry/cloud-service-broker/integrationtest/packer"
+	"github.com/cloudfoundry/cloud-service-broker/internal/testdrive"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("HTTPS redirect", func() {
+	var broker *testdrive.Broker
+
+	BeforeEach(func() {
+		brokerpak := must(packer.BuildBrokerpak(csb, fixtures("https-redirect")))
+		broker = must(testdrive.StartBroker(csb, brokerpak, database, testdrive.WithOutputs(GinkgoWriter, GinkgoWriter), testdrive.WithHTTPRedirect()))
+
+		DeferCleanup(func() {
+			Expect(broker.Stop()).To(Succeed())
+			cleanup(brokerpak)
+		})
+	})
+
+	It("redirects HTTP traffic", func() {
+		By("redirecting an HTTP connection")
+		errRedirected := fmt.Errorf("redirected")
+		client := http.Client{
+			CheckRedirect: func(*http.Request, []*http.Request) error {
+				return errRedirected
+			},
+		}
+
+		_, err := client.Get(fmt.Sprintf("http://localhost:%d/info", broker.Port))
+		Expect(err).To(MatchError(errRedirected))
+
+		By("allowing a connection that originated as HTTPS")
+		request, err := http.NewRequest(http.MethodGet, fmt.Sprintf("http://localhost:%d/info", broker.Port), nil)
+		Expect(err).NotTo(HaveOccurred())
+		request.Header.Add("X-Forwarded-Proto", "https")
+		response, err := client.Do(request)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(response).To(HaveHTTPStatus(http.StatusOK))
+	})
+})

internal/httpsmiddleware/ensurehttps.go

@@ -0,0 +1,51 @@
+// Package httpsmiddleware is used to ensure that the broker only accepts HTTPS connections
+package httpsmiddleware
+
+import (
+	"net/http"
+	"strings"
+
+	"code.cloudfoundry.org/lager/v3"
+)
+
+func EnsureHTTPS(next http.Handler, logger lager.Logger, disable bool) http.Handler {
+	if disable {
+		logger.Info("https-redirect-disabled")
+		return next
+	}
+
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		proto := r.Header.Values("X-Forwarded-Proto")
+		if isHTTPS(proto) {
+			next.ServeHTTP(w, r)
+			return
+		}
+
+		location := "https://" + r.Host + r.URL.String()
+		logger.Debug("redirecting-to-https", lager.Data{"to": location, "X-Forwarded-Proto": proto})
+		http.Redirect(w, r, location, http.StatusMovedPermanently)
+	})
+}
+
+// isHTTPS returns true when the protocol is definitely HTTPS
+//
+// From the docs: https://docs.vmware.com/en/VMware-Tanzu-Application-Service/4.0/tas-for-vms/http-routing.html
+//
+//	Developers can configure their apps to reject insecure requests by inspecting the X-Forwarded-Proto
+//	HTTP header on incoming traffic. The header may have multiple values represented as a comma-separated
+//	list, so developers must ensure the app rejects traffic that includes any X-Forwarded-Proto values
+//	that are not HTTPS.
+func isHTTPS(xForwardProto []string) bool {
+	xForwardProto = strings.Split(strings.Join(xForwardProto, ","), ",")
+	if len(xForwardProto) == 0 {
+		return false
+	}
+
+	for _, proto := range xForwardProto {
+		if proto != "https" {
+			return false
+		}
+	}
+
+	return true
+}

internal/httpsmiddleware/ensurehttps_test.go

@@ -0,0 +1,87 @@
+package httpsmiddleware_test
+
+import (
+	"net/http"
+	"net/url"
+
+	"code.cloudfoundry.org/lager/v3/lagertest"
+	"github.com/cloudfoundry/cloud-service-broker/internal/httpsmiddleware"
+	"github.com/cloudfoundry/cloud-service-broker/internal/httpsmiddleware/httpsmiddlewarefakes"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate
+//counterfeiter:generate net/http.Handler
+//counterfeiter:generate net/http.ResponseWriter
+
+var _ = Describe("EnsureHTTPS()", func() {
+	const xForwardProto = "X-Forwarded-Proto"
+
+	var (
+		logger             *lagertest.TestLogger
+		fakeNext           *httpsmiddlewarefakes.FakeHandler
+		fakeResponseWriter *httpsmiddlewarefakes.FakeResponseWriter
+		fakeResponseHeader http.Header
+		fakeURL            url.URL
+	)
+
+	BeforeEach(func() {
+		logger = lagertest.NewTestLogger("test")
+		fakeNext = &httpsmiddlewarefakes.FakeHandler{}
+		fakeURL = url.URL{
+			Scheme: "http",
+			Host:   "fake-host",
+			Path:   "fake-path",
+		}
+
+		fakeResponseWriter = &httpsmiddlewarefakes.FakeResponseWriter{}
+		fakeResponseHeader = make(http.Header)
+		fakeResponseWriter.HeaderReturns(fakeResponseHeader)
+	})
+
+	It("does nothing when disabled", func() {
+		httpsmiddleware.EnsureHTTPS(fakeNext, logger, true).ServeHTTP(fakeResponseWriter, &http.Request{
+			Method: http.MethodHead,
+			URL:    &fakeURL,
+			Header: http.Header{xForwardProto: []string{"http"}},
+		})
+
+		Expect(fakeNext.ServeHTTPCallCount()).To(Equal(1))
+		Expect(fakeResponseWriter.WriteHeaderCallCount()).To(BeZero())
+	})
+
+	DescribeTable(
+		"redirects non-HTTPS connections",
+		func(header http.Header) {
+			httpsmiddleware.EnsureHTTPS(fakeNext, logger, false).ServeHTTP(fakeResponseWriter, &http.Request{
+				Method: http.MethodHead,
+				URL:    &fakeURL,
+				Header: header,
+			})
+
+			Expect(fakeNext.ServeHTTPCallCount()).To(BeZero())
+			Expect(fakeResponseWriter.WriteHeaderCallCount()).To(Equal(1))
+			Expect(fakeResponseWriter.WriteHeaderArgsForCall(0)).To(Equal(http.StatusMovedPermanently))
+			Expect(fakeResponseHeader.Get("Location")).To(Equal("https://http://fake-host/fake-path"))
+
+			Expect(logger.Buffer().Contents()).To(ContainSubstring("redirecting-to-https"))
+		},
+		Entry("empty", nil),
+		Entry("http", http.Header{xForwardProto: []string{"http"}}),
+		Entry("other", http.Header{xForwardProto: []string{"foo"}}),
+		Entry("https with extra (comma separated)", http.Header{xForwardProto: []string{"https,foo"}}),
+		Entry("https with extra (slice)", http.Header{xForwardProto: []string{"https", "foo"}}),
+	)
+
+	It("allows an HTTPS connection", func() {
+		httpsmiddleware.EnsureHTTPS(fakeNext, logger, false).ServeHTTP(fakeResponseWriter, &http.Request{
+			Method: http.MethodHead,
+			URL:    &fakeURL,
+			Header: http.Header{xForwardProto: []string{"https"}},
+		})
+
+		Expect(fakeNext.ServeHTTPCallCount()).To(Equal(1))
+		Expect(fakeResponseWriter.WriteHeaderCallCount()).To(BeZero())
+	})
+})

internal/httpsmiddleware/httpsmiddleware_suite_test.go

@@ -0,0 +1,13 @@
+package httpsmiddleware_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestHTTPSMiddleware(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "HTTPS middleware Suite")
+}