Skip to content

Commit

Permalink
Introduce CONNECT Proxy URL ACL Support
Browse files Browse the repository at this point in the history
Add gitignore debug changes

WIP

Basic concept working

WIP

Cleaned up some things prereview

fixed tests

Removed extraneous yaml file
  • Loading branch information
pspieker-stripe committed Feb 14, 2024
1 parent 8c0fa26 commit bf0e961
Show file tree
Hide file tree
Showing 7 changed files with 137 additions and 22 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
*.swp
*.swo
*.swn
*debug.test*
41 changes: 31 additions & 10 deletions pkg/smokescreen/acl/v1/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

type Decider interface {
Decide(service, host string) (Decision, error)
Decide(service, host, connectProxyHost string) (Decision, error)
}

type ACL struct {
Expand All @@ -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 {
Expand All @@ -39,7 +40,6 @@ func New(logger *logrus.Logger, loader Loader, disabledActions []string) (*ACL,
if err != nil {
return nil, err
}

err = acl.DisablePolicies(disabledActions)
if err != nil {
return nil, err
Expand Down Expand Up @@ -80,11 +80,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)
Expand All @@ -97,6 +98,26 @@ func (acl *ACL) Decide(service, host string) (Decision, error) {
d.Project = rule.Project
d.Default = rule == acl.DefaultRule

if connectProxyHost != "" {
shouldDeny := true
fmt.Println("----------")
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) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/smokescreen/acl/v1/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
16 changes: 9 additions & 7 deletions pkg/smokescreen/acl/v1/yaml_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
21 changes: 18 additions & 3 deletions pkg/smokescreen/smokescreen.go
Original file line number Diff line number Diff line change
Expand Up @@ -642,8 +642,11 @@ func handleConnect(config *Config, pctx *goproxy.ProxyCtx) (string, error) {
return "", pctx.Error
}

// checkIfRequestShouldBeProxied can return an error if either the resolved address is disallowed,
// or if there is a DNS resolution failure.
/*
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.
*/
sctx.Decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, pctx.Req, destination)
if pctx.Error != nil {
// DNS resolution failure
Expand Down Expand Up @@ -910,6 +913,7 @@ func checkACLsForRequest(config *Config, req *http.Request, destination hostport
outboundHost: destination.String(),
}

fmt.Println(req)
if config.EgressACL == nil {
decision.allow = true
decision.reason = "Egress ACL is not configured"
Expand All @@ -932,7 +936,18 @@ 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.
var connectProxyHost string
if len(req.Header["X-Upstream-Https-Proxy"]) > 0 {
connectProxyHost = req.Header["X-Upstream-Https-Proxy"][0]
} else {
connectProxyHost = ""
}

ACLDecision, err := config.EgressACL.Decide(role, destination.Host, connectProxyHost)
decision.project = ACLDecision.Project
decision.reason = ACLDecision.Reason
if err != nil {
Expand Down
69 changes: 69 additions & 0 deletions pkg/smokescreen/smokescreen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1236,6 +1236,75 @@ func TestCustomRequestHandler(t *testing.T) {
})
}

func TestCONNECTProxyACLs(t *testing.T) {
// a := assert.New(t)
r := require.New(t)
cfg, err := testConfig("test-external-connect-proxy-srv")

r.NoError(err)
err = cfg.SetAllowAddresses([]string{"127.0.0.1"})
r.NoError(err)

// clientCh := make(chan bool)
serverCh := make(chan bool)
h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
serverCh <- true
<-serverCh
w.Write([]byte("OK"))
})

l, err := net.Listen("tcp", "localhost:0")
r.NoError(err)
cfg.Listener = l

t.Run("Blocks a non-approved proxy when the X-Upstream-Https-Proxy header is set", func(t *testing.T) {
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-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) {
// cfg.EgressACL.Rules["test-external-connect-proxy-srv"].ExternalProxyGlobs
// TODO: figure out how to change the rules at runtime here
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()
// runtime.Breakpoint()
fmt.Println(proxy.URL)
// fmt.Println(externalProxy.URL)
remote := httptest.NewTLSServer(h)
fmt.Println(remote.URL)
client, err := proxyClientWithConnectHeaders(proxy.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-srv", entry.Data["role"])
r.Equal(false, entry.Data["allow"])
})
}
func findCanonicalProxyDecision(logs []*logrus.Entry) *logrus.Entry {
for _, entry := range logs {
if entry.Message == CanonicalProxyDecision {
Expand Down
7 changes: 7 additions & 0 deletions pkg/smokescreen/testdata/acl.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ services:
- name: test-open-srv
project: security
action: open
- name: test-external-connect-proxy-srv
project: security
action: enforce
allowed_domains:
- httpbin.org
allowed_external_proxies:
- myproxy.com

global_deny_list:
- stripe.com

0 comments on commit bf0e961

Please sign in to comment.