Trying workaround

.gitignore

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

pkg/smokescreen/acl/v1/acl.go

@@ -9,7 +9,7 @@ import (
 )
 
 type Decider interface {
- Decide(service, host, connectProxyHost string) (Decision, error)
+ Decide(service, host string) (Decision, error)
 }
 
 type ACL struct {
@@ -22,10 +22,9 @@ type ACL struct {
 }
 
 type Rule struct {
- Project string
- Policy EnforcementPolicy
- DomainGlobs []string
- ExternalProxyGlobs []string
+ Project string
+ Policy EnforcementPolicy
+ DomainGlobs []string
 }
 
 type Decision struct {
@@ -81,12 +80,11 @@ func (acl *ACL) Add(svc string, r Rule) error {
 }
 
 // Decide takes uses the rule configured for the given service to determine if
-// 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) {
+// 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) {
  var d Decision
 
  rule := acl.Rule(service)
@@ -99,25 +97,6 @@ func (acl *ACL) Decide(service, host, connectProxyHost 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,11 +26,10 @@ type YAMLConfig struct {
 }
 
 type YAMLRule struct {
- Name string `yaml:"name"`
- Project string `yaml:"project"` // owner
- Action string `yaml:"action"`
- AllowedHosts []string `yaml:"allowed_domains"`
- AllowedExternalProxyHosts []string `yaml:"allowed_external_proxies"`
+ Name string `yaml:"name"`
+ Project string `yaml:"project"` // owner
+ Action string `yaml:"action"`
+ AllowedHosts []string `yaml:"allowed_domains"`
 }
 
 func (yc *YAMLConfig) ValidateConfig() error {
@@ -79,10 +78,9 @@ func (cfg *YAMLConfig) Load() (*ACL, error) {
  }
 
  r := Rule{
- Project: v.Project,
- Policy: p,
- DomainGlobs: v.AllowedHosts,
- ExternalProxyGlobs: v.AllowedExternalProxyHosts,
+ Project: v.Project,
+ Policy: p,
+ DomainGlobs: v.AllowedHosts,
  }
 
  err = acl.Add(v.Name, r)

pkg/smokescreen/smokescreen.go

@@ -640,8 +640,7 @@ 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 the subsequent proxy host (specified by the
- // X-Https-Upstream-Proxy header in the CONNECT request to _this_ proxy) is disallowed.
+ // or if there is a DNS resolution failure.
  sctx.Decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, pctx.Req, destination)
  if pctx.Error != nil {
  // DNS resolution failure
@@ -922,23 +921,15 @@ func checkACLsForRequest(config *Config, req *http.Request, destination hostport
  }
 
  decision.role = role
-
+ // test just making a change
  // This host validation prevents IPv6 addresses from being used as destinations.
  // Added for backwards compatibility.
  if strings.ContainsAny(destination.Host, ":") {
  decision.reason = "Destination host cannot be determined"
  return decision
  }
 
- // 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)
+ ACLDecision, err := config.EgressACL.Decide(role, destination.Host)
  decision.project = ACLDecision.Project
  decision.reason = ACLDecision.Reason
  if err != nil {

pkg/smokescreen/smokescreen_test.go

@@ -1236,77 +1236,6 @@ 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{"localhost"}})
- 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"])
- })
-}
 func findCanonicalProxyDecision(logs []*logrus.Entry) *logrus.Entry {
  for _, entry := range logs {
  if entry.Message == CanonicalProxyDecision {
@@ -1326,10 +1255,6 @@ func findCanonicalProxyClose(logs []*logrus.Entry) *logrus.Entry {
 }
 
 func testConfig(role string) (*Config, error) {
- return testConfigFromACLFile(role, "testdata/acl.yaml")
-}
-
-func testConfigFromACLFile(role string, filepath string) (*Config, error) {
  conf := NewConfig()
 
  if err := conf.SetAllowRanges(allowRanges); err != nil {
@@ -1339,7 +1264,7 @@ func testConfigFromACLFile(role string, filepath string) (*Config, error) {
  conf.ExitTimeout = 10 * time.Second
  conf.AdditionalErrorMessageOnDeny = "Proxy denied"
  conf.Resolver = &net.Resolver{}
- conf.SetupEgressAcl(filepath)
+ conf.SetupEgressAcl("testdata/acl.yaml")
  conf.RoleFromRequest = func(req *http.Request) (string, error) {
  return role, nil
  }

pkg/smokescreen/testdata/acl.yaml

@@ -15,22 +15,6 @@ 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 
- - thisisaproxy.com
 
 global_deny_list:
  - stripe.com