Add support for Smokescreen -> HTTPS CONNECT Proxy ACLs

.gitignore

@@ -3,3 +3,4 @@
 *.swp
 *.swo
 *.swn
+*debug.test*

pkg/smokescreen/acl/v1/acl.go

@@ -9,7 +9,7 @@ import (
 )
 
 type Decider interface {
-	Decide(service, host string) (Decision, error)
+	Decide(service, host, connectProxyHost string) (Decision, error)
 }
 
 type ACL struct {
@@ -22,9 +22,10 @@ type ACL struct {
 }
 
 type Rule struct {
-	Project     string
-	Policy      EnforcementPolicy
-	DomainGlobs []string
+	Project            string
+	Policy             EnforcementPolicy
+	DomainGlobs        []string
+	ExternalProxyGlobs []string
 }
 
 type Decision struct {
@@ -80,11 +81,12 @@ func (acl *ACL) Add(svc string, r Rule) error {
 }
 
 // Decide takes uses the rule configured for the given service to determine if
-//   1. The host is in the rule's allowed domain
-//   2. The host has been globally denied
-//   3. The host has been globally allowed
-//   4. There is a default rule for the ACL
-func (acl *ACL) Decide(service, host string) (Decision, error) {
+//  1. The CONNECT proxy host is in the rule's allowed domain
+//  2. The host is in the rule's allowed domain
+//  3. The host has been globally denied
+//  4. The host has been globally allowed
+//  5. There is a default rule for the ACL
+func (acl *ACL) Decide(service, host, connectProxyHost string) (Decision, error) {
 	var d Decision
 
 	rule := acl.Rule(service)
@@ -97,6 +99,25 @@ func (acl *ACL) Decide(service, host string) (Decision, error) {
 	d.Project = rule.Project
 	d.Default = rule == acl.DefaultRule
 
+	if connectProxyHost != "" {
+		shouldDeny := true
+		for _, dg := range rule.ExternalProxyGlobs {
+			if HostMatchesGlob(connectProxyHost, dg) {
+				shouldDeny = false
+				break
+			}
+		}
+
+		// We can only break out early and return if we know that we should deny;
+		// at this point the host hasn't been allowed by the rule, so we need to
+		// continue to check it below (unless we know we should deny it already)
+		if shouldDeny {
+			d.Result = Deny
+			d.Reason = "connect proxy host not allowed in rule"
+			return d, nil
+		}
+	}
+
 	// if the host matches any of the rule's allowed domains, allow
 	for _, dg := range rule.DomainGlobs {
 		if HostMatchesGlob(host, dg) {

pkg/smokescreen/acl/v1/acl_test.go

@@ -186,7 +186,7 @@ func TestACLDecision(t *testing.T) {
 			a.NoError(err)
 			a.Equal(testCase.expectProject, proj)
 
-			d, err := acl.Decide(testCase.service, testCase.host)
+			d, err := acl.Decide(testCase.service, testCase.host, "")
 			a.NoError(err)
 			a.Equal(testCase.expectDecision, d.Result)
 			a.Equal(testCase.expectDecisionReason, d.Reason)
@@ -207,7 +207,7 @@ func TestACLUnknownServiceWithoutDefault(t *testing.T) {
 	a.Equal("no rule for service: unk", err.Error())
 	a.Empty(proj)
 
-	d, err := acl.Decide("unk", "example.com")
+	d, err := acl.Decide("unk", "example.com", "")
 	a.Equal(Deny, d.Result)
 	a.False(d.Default)
 	a.Nil(err)

pkg/smokescreen/acl/v1/yaml_loader.go

@@ -26,10 +26,11 @@ type YAMLConfig struct {
 }
 
 type YAMLRule struct {
-	Name         string   `yaml:"name"`
-	Project      string   `yaml:"project"` // owner
-	Action       string   `yaml:"action"`
-	AllowedHosts []string `yaml:"allowed_domains"`
+	Name                      string   `yaml:"name"`
+	Project                   string   `yaml:"project"` // owner
+	Action                    string   `yaml:"action"`
+	AllowedHosts              []string `yaml:"allowed_domains"`
+	AllowedExternalProxyHosts []string `yaml:"allowed_external_proxies"`
 }
 
 func (yc *YAMLConfig) ValidateConfig() error {
@@ -78,9 +79,10 @@ func (cfg *YAMLConfig) Load() (*ACL, error) {
 		}
 
 		r := Rule{
-			Project:     v.Project,
-			Policy:      p,
-			DomainGlobs: v.AllowedHosts,
+			Project:            v.Project,
+			Policy:             p,
+			DomainGlobs:        v.AllowedHosts,
+			ExternalProxyGlobs: v.AllowedExternalProxyHosts,
 		}
 
 		err = acl.Add(v.Name, r)

pkg/smokescreen/smokescreen.go

@@ -640,7 +640,8 @@ func handleConnect(config *Config, pctx *goproxy.ProxyCtx) (string, error) {
 	}
 
 	// checkIfRequestShouldBeProxied can return an error if either the resolved address is disallowed,
-	// or if there is a DNS resolution failure.
+	// or if there is a DNS resolution failure, or if the subsequent proxy host (specified by the
+	// X-Https-Upstream-Proxy header in the CONNECT request to _this_ proxy) is disallowed.
 	sctx.Decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, pctx.Req, destination)
 	if pctx.Error != nil {
 		// DNS resolution failure
@@ -929,7 +930,15 @@ func checkACLsForRequest(config *Config, req *http.Request, destination hostport
 		return decision
 	}
 
-	ACLDecision, err := config.EgressACL.Decide(role, destination.Host)
+	// X-Upstream-Https-Proxy is a header that can be set by the client to specify
+	// a _subsequent_ proxy to use for the CONNECT request. This is used to allow traffic
+	// flow as in: client -(TLS)-> smokescreen -(TLS)-> external proxy -(TLS)-> destination.
+	// Without this header, there's no way for the client to specify a subsequent proxy.
+	// Also note - Get returns the first value for a given header, or the empty string,
+	// which is the behavior we want here.
+	connectProxyHost := req.Header.Get("X-Upstream-Https-Proxy")
+
+	ACLDecision, err := config.EgressACL.Decide(role, destination.Host, connectProxyHost)
 	decision.project = ACLDecision.Project
 	decision.reason = ACLDecision.Reason
 	if err != nil {

pkg/smokescreen/smokescreen_test.go

@@ -1236,6 +1236,124 @@ func TestCustomRequestHandler(t *testing.T) {
 	})
 }
 
+func TestCONNECTProxyACLs(t *testing.T) {
+	t.Run("Blocks a non-approved proxy when the X-Upstream-Https-Proxy header is set", func(t *testing.T) {
+		h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte("OK"))
+		})
+		r := require.New(t)
+		l, err := net.Listen("tcp", "localhost:0")
+		r.NoError(err)
+		cfg, err := testConfig("test-external-connect-proxy-blocked-srv")
+		r.NoError(err)
+		cfg.Listener = l
+
+		err = cfg.SetAllowAddresses([]string{"127.0.0.1"})
+		r.NoError(err)
+
+		internalToStripeProxy := proxyServer(cfg)
+		logHook := proxyLogHook(cfg)
+		remote := httptest.NewTLSServer(h)
+
+		client, err := proxyClientWithConnectHeaders(internalToStripeProxy.URL, http.Header{"X-Upstream-Https-Proxy": []string{"https://google.com"}})
+		r.NoError(err)
+
+		req, err := http.NewRequest("GET", remote.URL, nil)
+		r.NoError(err)
+
+		client.Do(req)
+
+		entry := findCanonicalProxyDecision(logHook.AllEntries())
+		r.NotNil(entry)
+		r.Equal("connect proxy host not allowed in rule", entry.Data["decision_reason"])
+		r.Equal("test-external-connect-proxy-blocked-srv", entry.Data["role"])
+		r.Equal(false, entry.Data["allow"])
+	})
+
+	t.Run("Allows an approved proxy when the X-Upstream-Https-Proxy header is set", func(t *testing.T) {
+		h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte("OK"))
+		})
+		r := require.New(t)
+		l, err := net.Listen("tcp", "localhost:0")
+		r.NoError(err)
+		cfg, err := testConfig("test-external-connect-proxy-allowed-srv")
+		r.NoError(err)
+		cfg.Listener = l
+
+		err = cfg.SetAllowAddresses([]string{"127.0.0.1"})
+		r.NoError(err)
+
+		proxy := proxyServer(cfg)
+		logHook := proxyLogHook(cfg)
+
+		// The External proxy is a HTTPS proxy that will be used to connect to the remote server
+		externalProxy := httptest.NewUnstartedServer(BuildProxy(cfg))
+		externalProxy.StartTLS()
+
+		remote := httptest.NewTLSServer(h)
+		client, err := proxyClientWithConnectHeaders(proxy.URL, http.Header{"X-Upstream-Https-Proxy": []string{"myproxy.com"}})
+		r.NoError(err)
+
+		req, err := http.NewRequest("GET", remote.URL, nil)
+		r.NoError(err)
+
+		client.Do(req)
+
+		entry := findCanonicalProxyDecision(logHook.AllEntries())
+		r.NotNil(entry)
+		r.Equal("host matched allowed domain in rule", entry.Data["decision_reason"])
+		r.Equal("test-external-connect-proxy-allowed-srv", entry.Data["role"])
+		r.Equal(true, entry.Data["allow"])
+	})
+
+	t.Run("Allows multiple approved proxies when the X-Upstream-Https-Proxy header is set", func(t *testing.T) {
+		h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte("OK"))
+		})
+		r := require.New(t)
+		l, err := net.Listen("tcp", "localhost:0")
+		r.NoError(err)
+		cfg, err := testConfig("test-external-connect-proxy-allowed-srv")
+		r.NoError(err)
+		cfg.Listener = l
+
+		err = cfg.SetAllowAddresses([]string{"127.0.0.1"})
+		r.NoError(err)
+
+		proxy := proxyServer(cfg)
+		logHook := proxyLogHook(cfg)
+
+		// The External proxy is a HTTPS proxy that will be used to connect to the remote server
+		externalProxy := httptest.NewUnstartedServer(BuildProxy(cfg))
+		externalProxy.StartTLS()
+
+		remote := httptest.NewTLSServer(h)
+		first_client, err := proxyClientWithConnectHeaders(proxy.URL, http.Header{"X-Upstream-Https-Proxy": []string{"myproxy.com"}})
+		r.NoError(err)
+
+		first_req, err := http.NewRequest("GET", remote.URL, nil)
+		r.NoError(err)
+
+		first_client.Do(first_req)
+
+		second_client, err := proxyClientWithConnectHeaders(proxy.URL, http.Header{"X-Upstream-Https-Proxy": []string{"myproxy2.com"}})
+		r.NoError(err)
+
+		second_req, err := http.NewRequest("GET", remote.URL, nil)
+		r.NoError(err)
+
+		second_client.Do(second_req)
+
+		entries := logHook.AllEntries()
+		r.Equal(2, len(entries))
+
+		first_entry := entries[0]
+		second_entry := entries[1]
+		r.Equal("host matched allowed domain in rule", first_entry.Data["decision_reason"])
+		r.Equal("host matched allowed domain in rule", second_entry.Data["decision_reason"])
+	})
+}
 func findCanonicalProxyDecision(logs []*logrus.Entry) *logrus.Entry {
 	for _, entry := range logs {
 		if entry.Message == CanonicalProxyDecision {

pkg/smokescreen/testdata/acl.yaml

@@ -15,6 +15,24 @@ services:
   - name: test-open-srv
     project: security
     action: open
+  - name: test-external-connect-proxy-blocked-srv
+    project: security
+    action: enforce
+    allowed_domains:
+      - 127.0.0.1
+    allowed_external_proxies:
+      - myproxy.com
+      - otherproxy.org
+  - name: test-external-connect-proxy-allowed-srv
+    project: security
+    action: enforce
+    allowed_domains:
+      - 127.0.0.1
+    allowed_external_proxies:
+      - localhost 
+      - myproxy.com
+      - myproxy2.com
+      - thisisaproxy.com
 
 global_deny_list:
   - stripe.com