From 506e36214896342daaf7bf6935a7724657c85626 Mon Sep 17 00:00:00 2001 From: Harold Simpson Date: Mon, 9 Sep 2024 21:49:44 +0200 Subject: [PATCH 1/6] go get -d github.com/stripe/goproxy@latest && go mod vendor --- go.mod | 2 +- go.sum | 2 ++ vendor/github.com/stripe/goproxy/ctx.go | 5 +++-- vendor/github.com/stripe/goproxy/https.go | 13 +++++++++---- vendor/modules.txt | 2 +- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 7e4b6bf5..bf3f0a74 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/rs/xid v1.2.1 github.com/sirupsen/logrus v1.9.0 github.com/stretchr/testify v1.8.0 - github.com/stripe/goproxy v0.0.0-20240711170433-75b93c00dfb0 + github.com/stripe/goproxy v0.0.0-20240909161309-906e92b723dd golang.org/x/net v0.17.0 gopkg.in/urfave/cli.v1 v1.20.0 gopkg.in/yaml.v2 v2.4.0 diff --git a/go.sum b/go.sum index b786af9b..6c703351 100644 --- a/go.sum +++ b/go.sum @@ -224,6 +224,8 @@ github.com/stripe/goproxy v0.0.0-20240702232545-72d7dbc6d4fe h1:RzjmXDVOjWq9sPRc github.com/stripe/goproxy v0.0.0-20240702232545-72d7dbc6d4fe/go.mod h1:hF2CVgH4++5ijZiy9grGVP8Fsi4u+SMOtbnIKYbMUjY= github.com/stripe/goproxy v0.0.0-20240711170433-75b93c00dfb0 h1:1xKgjoLWAosf0yoKocNN4mel1++AKqJw9axpJyz1JRw= github.com/stripe/goproxy v0.0.0-20240711170433-75b93c00dfb0/go.mod h1:hF2CVgH4++5ijZiy9grGVP8Fsi4u+SMOtbnIKYbMUjY= +github.com/stripe/goproxy v0.0.0-20240909161309-906e92b723dd h1:+GFSUmU0/RS984Zp8KvYxjU95DxVIGJflsOfITHkF68= +github.com/stripe/goproxy v0.0.0-20240909161309-906e92b723dd/go.mod h1:hF2CVgH4++5ijZiy9grGVP8Fsi4u+SMOtbnIKYbMUjY= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/vendor/github.com/stripe/goproxy/ctx.go b/vendor/github.com/stripe/goproxy/ctx.go index 76a4c6dc..55f9ca3f 100644 --- a/vendor/github.com/stripe/goproxy/ctx.go +++ b/vendor/github.com/stripe/goproxy/ctx.go @@ -25,8 +25,9 @@ type ProxyCtx struct { // call of RespHandler UserData interface{} // Will connect a request to a response - Session int64 - proxy *ProxyHttpServer + Session int64 + proxy *ProxyHttpServer + ConnectAction ConnectActionLiteral } type RoundTripper interface { diff --git a/vendor/github.com/stripe/goproxy/https.go b/vendor/github.com/stripe/goproxy/https.go index a60a1953..114718fc 100644 --- a/vendor/github.com/stripe/goproxy/https.go +++ b/vendor/github.com/stripe/goproxy/https.go @@ -43,9 +43,10 @@ var ( ) type ConnectAction struct { - Action ConnectActionLiteral - Hijack func(req *http.Request, client net.Conn, ctx *ProxyCtx) - TLSConfig func(host string, ctx *ProxyCtx) (*tls.Config, error) + Action ConnectActionLiteral + Hijack func(req *http.Request, client net.Conn, ctx *ProxyCtx) + TLSConfig func(host string, ctx *ProxyCtx) (*tls.Config, error) + MitmMutateRequest func(req *http.Request, ctx *ProxyCtx) } func stripPort(s string) string { @@ -114,6 +115,7 @@ func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request break } } + ctx.ConnectAction = todo.Action switch todo.Action { case ConnectAccept: if !hasPort.MatchString(host) { @@ -264,7 +266,7 @@ func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request req, err := http.ReadRequest(clientTlsReader) // Set the RoundTripper on the ProxyCtx within the `HandleConnect` action of goproxy, then // inject the roundtripper here in order to use a custom round tripper while mitm. - var ctx = &ProxyCtx{Req: req, Session: atomic.AddInt64(&proxy.sess, 1), proxy: proxy, UserData: ctx.UserData, RoundTripper: ctx.RoundTripper} + var ctx = &ProxyCtx{Req: req, Session: atomic.AddInt64(&proxy.sess, 1), proxy: proxy, UserData: ctx.UserData, RoundTripper: ctx.RoundTripper, ConnectAction: ctx.ConnectAction} if err != nil && err != io.EOF { return } @@ -273,6 +275,9 @@ func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request return } req.RemoteAddr = r.RemoteAddr // since we're converting the request, need to carry over the original connecting IP as well + if todo.MitmMutateRequest != nil { + todo.MitmMutateRequest(req, ctx) + } ctx.Logf("req %v", r.Host) if !httpsRegexp.MatchString(req.URL.String()) { diff --git a/vendor/modules.txt b/vendor/modules.txt index c86ba751..9ade687b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -69,7 +69,7 @@ github.com/sirupsen/logrus/hooks/test ## explicit; go 1.13 github.com/stretchr/testify/assert github.com/stretchr/testify/require -# github.com/stripe/goproxy v0.0.0-20240711170433-75b93c00dfb0 +# github.com/stripe/goproxy v0.0.0-20240909161309-906e92b723dd ## explicit; go 1.13 github.com/stripe/goproxy # golang.org/x/mod v0.8.0 From 734c34395dbaa6f9b11f28018b987c88ee47841a Mon Sep 17 00:00:00 2001 From: Harold Simpson Date: Tue, 10 Sep 2024 08:38:30 +0200 Subject: [PATCH 2/6] Add MITM support to Smokescreen --- .vscode/launch.json | 16 + Development.md | 306 ++++++++++++++++++ README.md | 36 +-- pkg/smokescreen/acl/v1/acl.go | 50 ++- pkg/smokescreen/acl/v1/acl_test.go | 26 ++ .../acl/v1/testdata/mitm_config.yaml | 23 ++ pkg/smokescreen/acl/v1/yaml_loader.go | 48 ++- pkg/smokescreen/config.go | 4 +- pkg/smokescreen/config_loader.go | 23 +- pkg/smokescreen/smokescreen.go | 170 +++++++--- pkg/smokescreen/smokescreen_test.go | 105 ++++++ pkg/smokescreen/testdata/acl.yaml | 10 + 12 files changed, 729 insertions(+), 88 deletions(-) create mode 100644 .vscode/launch.json create mode 100644 Development.md create mode 100644 pkg/smokescreen/acl/v1/testdata/mitm_config.yaml diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 00000000..2a5a27d2 --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,16 @@ +{ + // Use IntelliSense to learn about possible attributes. + // Hover to view descriptions of existing attributes. + // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 + "version": "0.2.0", + "configurations": [ + { + "name": "smokescreen", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "./", + "args": ["--config-file", "config.yaml", "--egress-acl-file", "acl.yaml"] + } + ] +} \ No newline at end of file diff --git a/Development.md b/Development.md new file mode 100644 index 00000000..ad8e408e --- /dev/null +++ b/Development.md @@ -0,0 +1,306 @@ + +# Development and Testing + +## Testing +```bash +go test ./... +``` + +## Running locally + +This section describes how to run Smokescreen locally with different scenarios and using `curl` as a client. + +- [HTTP Proxy](#http-proxy) +- [HTTP CONNECT Proxy](#http-connect-proxy) +- [Monitor metrics Smokescreen emits](#monitor-metrics-smokescreen-emits) +- [HTTP CONNECT Proxy over TLS](#http-connect-proxy-over-tls) +- [MITM (Man in the middle) Proxy](#mitm-man-in-the-middle-proxy) +- [MITM (Man in the middle) Proxy over TLS](#mitm-man-in-the-middle-proxy-over-tls) + +### HTTP Proxy + +#### Configurations + +```yaml +# config.yaml +--- +allow_missing_role: true # skip mTLS client validation (use default ACL) +``` + +```yaml +# acl.yaml +--- +version: v1 +services: [] +default: + name: default + project: security + action: enforce + allowed_domains: + - example.com +``` + +#### Run + +```bash +# Run smokescreen (in a different shell) +go run . --config-file config.yaml --egress-acl-file acl.yaml + +# Curl +curl -x localhost:4750 http://example.com +# Curl with ALL_PROXY +ALL_PROXY=localhost:4750 curl -v http://example.com +``` + +### HTTP CONNECT Proxy + +#### Configurations + +```yaml +# config.yaml +--- +allow_missing_role: true # skip mTLS client validation (use default ACL) +``` + +```yaml +# acl.yaml +--- +version: v1 +services: [] +default: + name: default + project: security + action: enforce + allowed_domains: + - api.github.com +``` + +#### Run + +```bash +# Run smokescreen (in a different shell) +go run . --config-file config.yaml --egress-acl-file acl.yaml + +# Curl +curl --proxytunnel -x localhost:4750 https://api.github.com/zen +# Curl with HTTPS_PROXY +HTTPS_PROXY=localhost:4750 curl https://api.github.com/zen +``` + +### Monitor metrics Smokescreen emits + +#### Configurations + +```yaml +# config.yaml +--- +allow_missing_role: true # skip mTLS client validation (use default ACL) +statsd_address: 127.0.0.1:8200 +``` + +```yaml +# acl.yaml +--- +version: v1 +services: [] +default: + name: default + project: security + action: enforce + allowed_domains: + - api.github.com +``` + +#### Run + +```bash +# Listen to a local port with nc (in a different shell) +nc -uklv 127.0.0.1 8200 + +# Run smokescreen (in a different shell) +go run . --config-file config.yaml --egress-acl-file acl.yaml + +# Curl +curl --proxytunnel -x localhost:4750 https://api.github.com/zen +# Curl with HTTPS_PROXY +HTTPS_PROXY=localhost:4750 curl https://api.github.com/zen +``` + +### HTTP CONNECT Proxy over TLS + +#### Set-up + +##### Generate certificates +```bash +mkdir -p mtls_setup +# Private keys for CAs +openssl genrsa -out mtls_setup/server-ca.key 2048 +openssl genrsa -out mtls_setup/client-ca.key 2048 + +# Generate client and server CA certificates +openssl req -new -x509 -nodes -days 1000 -key mtls_setup/server-ca.key -out mtls_setup/server-ca.crt \ + -subj "/C=AQ/ST=Petrel Island/L=Dumont-d'Urville +/O=Penguin/OU=Publishing house/CN=server CA" + +openssl req -new -x509 -nodes -days 1000 -key mtls_setup/client-ca.key -out mtls_setup/client-ca.crt \ + -subj "/C=MA/ST=Tarfaya/L=Tarfaya/O=Fennec/OU=Aviator/CN=Client CA" + +# Generate a certificate signing request (client CN is localhost which is used by smokescreen as the service name by default) +openssl req -newkey rsa:2048 -nodes -keyout mtls_setup/server.key -out mtls_setup/server.req \ + -subj "/C=AQ/ST=Petrel Island/L=Dumont-d'Urville/O=Chionis/OU=Publishing house/CN=server req" +openssl req -newkey rsa:2048 -nodes -keyout mtls_setup/client.key -out mtls_setup/client.req \ + -subj "/C=MA/ST=Tarfaya/L=Tarfaya/O=Addax/OU=Writer/CN=localhost" + +# Have the CA sign the certificate requests and output the certificates. +echo "authorityKeyIdentifier=keyid,issuer +basicConstraints=CA:FALSE +keyUsage = digitalSignature, nonRepudiation, keyEncipherment, dataEncipherment +subjectAltName = @alt_names + +[alt_names] +DNS.1 = localhost +" > mtls_setup/localhost.ext + +openssl x509 -req -in mtls_setup/server.req -days 1000 -CA mtls_setup/server-ca.crt -CAkey mtls_setup/server-ca.key -set_serial 01 -out mtls_setup/server.crt -extfile mtls_setup/localhost.ext + +openssl x509 -req -in mtls_setup/client.req -days 1000 -CA mtls_setup/client-ca.crt -CAkey mtls_setup/client-ca.key -set_serial 01 -out mtls_setup/client.crt +``` + +##### Configurations + +```yaml +# config.yaml +--- +tls: + cert_file: "mtls_setup/server.crt" + key_file: "mtls_setup/server.key" + client_ca_files: + - "mtls_setup/client-ca.crt" +``` + +```yaml +# acl.yaml +--- +version: v1 +services: + - name: localhost + project: github + action: enforce + allowed_domains: + - api.github.com +default: + name: default + project: security + action: enforce + allowed_domains: [] +``` + +#### Run + +```bash +# Run smokescreen (in a different shell) +go run . --config-file config.yaml --egress-acl-file acl.yaml + +# Curl +curl --proxytunnel -x https://localhost:4750 --proxy-cacert mtls_setup/server-ca.crt --proxy-cert mtls_setup/client.crt --proxy-key mtls_setup/client.key https://api.github.com/zen +# Curl with HTTPS_PROXY +HTTPS_PROXY=https://localhost:4750 curl --proxy-cacert mtls_setup/server-ca.crt --proxy-cert mtls_setup/client.crt --proxy-key mtls_setup/client.key https://api.github.com/zen +``` + +### MITM (Man in the middle) Proxy + +#### Set-up + +```yaml +# config.yaml +--- +allow_missing_role: true # skip mTLS client validation (use default ACL) +# Re-using goproxy library CA and key +mitm_ca_cert_file: "vendor/github.com/stripe/goproxy/ca.pem" +mitm_ca_key_file: "vendor/github.com/stripe/goproxy/key.pem" +``` + +```yaml +# acl.yaml +--- +version: v1 +services: [] +default: + name: default + project: security + action: enforce + allowed_domains: [] + allowed_domains_mitm: + - domain: wttr.in + add_headers: + Accept-Language: el + detailed_http_logs: true + detailed_http_logs_full_headers: + - User-Agent +``` + +#### Run + +```bash +# Run smokescreen (in a different shell) +go run . --config-file config.yaml --egress-acl-file acl.yaml + +# Curl (weather should be in Greek since we set the Accept-Language header) +curl --proxytunnel -x localhost:4750 --cacert vendor/github.com/stripe/goproxy/ca.pem https://wttr.in +# Curl with HTTPS_PROXY +HTTPS_PROXY=localhost:4750 curl --cacert vendor/github.com/stripe/goproxy/ca.pem https://wttr.in +``` + +### MITM (Man in the middle) Proxy over TLS + +#### Set-up + +Please generate the certificates from the TLS Generate certificates section. + +```yaml +# config.yaml +--- +tls: + cert_file: "mtls_setup/server.crt" + key_file: "mtls_setup/server.key" + client_ca_files: + - "mtls_setup/client-ca.crt" +# Re-using goproxy library CA and key +mitm_ca_cert_file: "vendor/github.com/stripe/goproxy/ca.pem" +mitm_ca_key_file: "vendor/github.com/stripe/goproxy/key.pem" +``` + +```yaml +# acl.yaml +--- +version: v1 +services: + - name: localhost + project: github + action: enforce + allowed_domains: [] + allowed_domains_mitm: + - domain: wttr.in + add_headers: + Accept-Language: el + detailed_http_logs: true + detailed_http_logs_full_headers: + - User-Agent +default: + name: default + project: security + action: enforce + allowed_domains: [] +``` + +#### Run + +```bash +# Run smokescreen (in a different shell) +go run . --config-file config.yaml --egress-acl-file acl.yaml + +# Curl (weather should be in Greek since we set the Accept-Language header) +curl --proxytunnel -x https://localhost:4750 --cacert vendor/github.com/stripe/goproxy/ca.pem --proxy-cacert mtls_setup/server-ca.crt --proxy-cert mtls_setup/client.crt --proxy-key mtls_setup/client.key https://wttr.in +# Curl with HTTPS_PROXY +HTTPS_PROXY=https://localhost:4750 curl --cacert vendor/github.com/stripe/goproxy/ca.pem --proxy-cacert mtls_setup/server-ca.crt --proxy-cert mtls_setup/client.crt --proxy-key mtls_setup/client.key https://wttr.in +``` diff --git a/README.md b/README.md index ef7f5cbb..b0624f8f 100644 --- a/README.md +++ b/README.md @@ -171,41 +171,7 @@ If a domain matches both the `global_allow_list` and the `global_deny_list`, the # Development and Testing -## Running locally - -To run Smokescreen locally, you can provide a minimal configuration file and use `curl` as a client. For example: - -```yaml -# config.yaml ---- -allow_missing_role: true # skip mTLS client validation -statsd_address: 127.0.0.1:8200 -``` - -If you want to see metrics Smokescreen emits, listen on a local port: - -```shellsession -$ nc -uklv 127.0.0.1 8200 -``` - -Build and run Smokescreen: - -```shellsession -$ go run . --config-file config.yaml -{"level":"info","msg":"starting","time":"2022-11-30T15:19:08-08:00"} -``` - -Make a request using `curl`: - -```shellsession -$ curl --proxytunnel -x localhost:4750 https://stripe.com/ -``` - -## Testing - -```shellsession -$ go test ./... -``` +See [Development.md](Development.md) # Contributors diff --git a/pkg/smokescreen/acl/v1/acl.go b/pkg/smokescreen/acl/v1/acl.go index f5bf3a4a..6540cdc8 100644 --- a/pkg/smokescreen/acl/v1/acl.go +++ b/pkg/smokescreen/acl/v1/acl.go @@ -25,14 +25,27 @@ type Rule struct { Project string Policy EnforcementPolicy DomainGlobs []string + DomainMitmGlobs []MitmDomain ExternalProxyGlobs []string } +type MitmDomain struct { + MitmConfig + Domain string +} + +type MitmConfig struct { + AddHeaders map[string]string + DetailedHttpLogs bool + DetailedHttpLogsFullHeaders []string +} + type Decision struct { - Reason string - Default bool - Result DecisionResult - Project string + Reason string + Default bool + Result DecisionResult + Project string + MitmConfig *MitmConfig } func New(logger *logrus.Logger, loader Loader, disabledActions []string) (*ACL, error) { @@ -68,7 +81,7 @@ func (acl *ACL) Add(svc string, r Rule) error { return err } - err = acl.ValidateDomainGlobs(svc, r.DomainGlobs) + err = acl.ValidateRuleDomainsGlobs(svc, r) if err != nil { return err } @@ -126,6 +139,15 @@ func (acl *ACL) Decide(service, host, connectProxyHost string) (Decision, error) } } + // if the host matches any of the rule's allowed domains with MITM config, allow + for _, dg := range rule.DomainMitmGlobs { + if HostMatchesGlob(host, dg.Domain) { + d.Result, d.Reason = Allow, "host matched allowed domain in rule" + d.MitmConfig = (*MitmConfig)(&dg.MitmConfig) + return d, nil + } + } + // if the host matches any of the global deny list, deny for _, dg := range acl.GlobalDenyList { if HostMatchesGlob(host, dg) { @@ -180,7 +202,7 @@ func (acl *ACL) DisablePolicies(actions []string) error { // and is not utilizing a disabled enforcement policy. func (acl *ACL) Validate() error { for svc, r := range acl.Rules { - err := acl.ValidateDomainGlobs(svc, r.DomainGlobs) + err := acl.ValidateRuleDomainsGlobs(svc, r) if err != nil { return err } @@ -192,6 +214,22 @@ func (acl *ACL) Validate() error { return nil } +func (acl *ACL) ValidateRuleDomainsGlobs(svc string, r Rule) error { + err := acl.ValidateDomainGlobs(svc, r.DomainGlobs) + if err != nil { + return err + } + mitmDomainGlobs := make([]string, len(r.DomainMitmGlobs)) + for i, d := range r.DomainMitmGlobs { + mitmDomainGlobs[i] = d.Domain + } + err = acl.ValidateDomainGlobs(svc, mitmDomainGlobs) + if err != nil { + return err + } + return nil +} + // ValidateDomainGlobs takes a slice of domain globs and verifies they conform to smokescreen's // domain glob policy. // diff --git a/pkg/smokescreen/acl/v1/acl_test.go b/pkg/smokescreen/acl/v1/acl_test.go index 9539458a..c299e700 100644 --- a/pkg/smokescreen/acl/v1/acl_test.go +++ b/pkg/smokescreen/acl/v1/acl_test.go @@ -358,3 +358,29 @@ func TestHostMatchesGlob(t *testing.T) { }) } } + +func TestMitmComfig(t *testing.T) { + a := assert.New(t) + + yl := NewYAMLLoader(path.Join("testdata", "mitm_config.yaml")) + acl, err := New(logrus.New(), yl, []string{}) + + a.NoError(err) + a.NotNil(acl) + + mitmService := "enforce-dummy-mitm-srv" + + proj, err := acl.Project(mitmService) + a.NoError(err) + a.Equal("usersec", proj) + + d, err := acl.Decide(mitmService, "example-mitm.com", "") + a.NoError(err) + a.Equal(Allow, d.Result) + a.Equal("host matched allowed domain in rule", d.Reason) + + a.NotNil(d.MitmConfig) + a.Equal(true, d.MitmConfig.DetailedHttpLogs) + a.Equal([]string{"User-Agent"}, d.MitmConfig.DetailedHttpLogsFullHeaders) + a.Equal(map[string]string{"Accept-Language": "el"}, d.MitmConfig.AddHeaders) +} diff --git a/pkg/smokescreen/acl/v1/testdata/mitm_config.yaml b/pkg/smokescreen/acl/v1/testdata/mitm_config.yaml new file mode 100644 index 00000000..b9fde18f --- /dev/null +++ b/pkg/smokescreen/acl/v1/testdata/mitm_config.yaml @@ -0,0 +1,23 @@ +--- +version: v1 +services: + - name: enforce-dummy-mitm-srv + project: usersec + action: enforce + allowed_domains: + - examplea.com + - exampleb.com + allowed_domains_mitm: + - domain: example-mitm.com + add_headers: + Accept-Language: el + detailed_http_logs: true + detailed_http_logs_full_headers: + - User-Agent + +default: + project: other + action: enforce + allowed_domains: + - default.example.com + diff --git a/pkg/smokescreen/acl/v1/yaml_loader.go b/pkg/smokescreen/acl/v1/yaml_loader.go index 78bdaa27..db39803d 100644 --- a/pkg/smokescreen/acl/v1/yaml_loader.go +++ b/pkg/smokescreen/acl/v1/yaml_loader.go @@ -26,11 +26,19 @@ 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"` + AllowedHostsMitm []YAMLMitmRule `yaml:"allowed_domains_mitm"` + AllowedExternalProxyHosts []string `yaml:"allowed_external_proxies"` +} + +type YAMLMitmRule struct { + Domain string `yaml:"domain"` + AddHeaders map[string]string `yaml:"add_headers"` + DetailedHttpLogs bool `yaml:"detailed_http_logs"` + DetailedHttpLogsFullHeaders []string `yaml:"detailed_http_logs_full_headers"` } func (yc *YAMLConfig) ValidateConfig() error { @@ -78,10 +86,25 @@ func (cfg *YAMLConfig) Load() (*ACL, error) { return nil, err } + var allowedHostsMitm []MitmDomain + + for _, w := range v.AllowedHostsMitm { + mitmDomain := MitmDomain{ + MitmConfig: MitmConfig{ + AddHeaders: w.AddHeaders, + DetailedHttpLogs: w.DetailedHttpLogs, + DetailedHttpLogsFullHeaders: w.DetailedHttpLogsFullHeaders, + }, + Domain: w.Domain, + } + allowedHostsMitm = append(allowedHostsMitm, mitmDomain) + } + r := Rule{ Project: v.Project, Policy: p, DomainGlobs: v.AllowedHosts, + DomainMitmGlobs: allowedHostsMitm, ExternalProxyGlobs: v.AllowedExternalProxyHosts, } @@ -97,10 +120,25 @@ func (cfg *YAMLConfig) Load() (*ACL, error) { return nil, err } + var allowedHostsMitm []MitmDomain + + for _, w := range cfg.Default.AllowedHostsMitm { + mitmDomain := MitmDomain{ + MitmConfig: MitmConfig{ + AddHeaders: w.AddHeaders, + DetailedHttpLogs: w.DetailedHttpLogs, + DetailedHttpLogsFullHeaders: w.DetailedHttpLogsFullHeaders, + }, + Domain: w.Domain, + } + allowedHostsMitm = append(allowedHostsMitm, mitmDomain) + } + acl.DefaultRule = &Rule{ Project: cfg.Default.Project, Policy: p, DomainGlobs: cfg.Default.AllowedHosts, + DomainMitmGlobs: allowedHostsMitm, ExternalProxyGlobs: cfg.Default.AllowedExternalProxyHosts, } } diff --git a/pkg/smokescreen/config.go b/pkg/smokescreen/config.go index bf60aa4a..bb074774 100644 --- a/pkg/smokescreen/config.go +++ b/pkg/smokescreen/config.go @@ -73,7 +73,7 @@ type Config struct { TransportMaxIdleConns int TransportMaxIdleConnsPerHost int - // These are the http and https address for the upstream proxy + // These are the http and https address for the upstream proxy UpstreamHttpProxyAddr string UpstreamHttpsProxyAddr string @@ -99,6 +99,8 @@ type Config struct { // If smokescreen denies a request, this handler is not called. // If the handler returns an error, smokescreen will deny the request. PostDecisionRequestHandler func(*http.Request) error + // MitmCa is used to provide a custom CA for MITM + MitmCa *tls.Certificate } type missingRoleError struct { diff --git a/pkg/smokescreen/config_loader.go b/pkg/smokescreen/config_loader.go index b3f608be..b64c05a7 100644 --- a/pkg/smokescreen/config_loader.go +++ b/pkg/smokescreen/config_loader.go @@ -1,6 +1,8 @@ package smokescreen import ( + "crypto/tls" + "crypto/x509" "errors" "fmt" "io/ioutil" @@ -48,7 +50,9 @@ type yamlConfig struct { Tls *yamlConfigTls // Currently not configurable via YAML: RoleFromRequest, Log, DisabledAclPolicyActions - UnsafeAllowPrivateRanges bool `yaml:"unsafe_allow_private_ranges"` + UnsafeAllowPrivateRanges bool `yaml:"unsafe_allow_private_ranges"` + MitmCaCertFile string `yaml:"mitm_ca_cert_file"` + MitmCaKeyFile string `yaml:"mitm_ca_key_file"` } func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { @@ -151,6 +155,23 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { c.TimeConnect = yc.TimeConnect c.UnsafeAllowPrivateRanges = yc.UnsafeAllowPrivateRanges + if yc.MitmCaCertFile != "" || yc.MitmCaKeyFile != "" { + if yc.MitmCaCertFile == "" { + return errors.New("mitm_ca_cert_file required when mitm_ca_key_file is set") + } + if yc.MitmCaKeyFile == "" { + return errors.New("mitm_ca_key_file required when mitm_ca_cert_file is set") + } + mitmCa, err := tls.LoadX509KeyPair(yc.MitmCaCertFile, yc.MitmCaKeyFile) + if err != nil { + return fmt.Errorf("could not load mitmCa: %v", err) + } + if mitmCa.Leaf, err = x509.ParseCertificate(mitmCa.Certificate[0]); err != nil { + return fmt.Errorf("could not populate x509 Leaf value: %v", err) + } + c.MitmCa = &mitmCa + } + return nil } diff --git a/pkg/smokescreen/smokescreen.go b/pkg/smokescreen/smokescreen.go index 831e1dcf..592ef3dc 100644 --- a/pkg/smokescreen/smokescreen.go +++ b/pkg/smokescreen/smokescreen.go @@ -60,6 +60,9 @@ const ( CanonicalProxyDecision = "CANONICAL-PROXY-DECISION" LogFieldConnEstablishMS = "conn_establish_time_ms" LogFieldDNSLookupTime = "dns_lookup_time_ms" + LogMitmReqUrl = "mitm_req_url" + LogMitmReqMethod = "mitm_req_method" + LogMitmReqHeaders = "mitm_req_headers" ) type ipType int @@ -69,6 +72,7 @@ type ACLDecision struct { ResolvedAddr *net.TCPAddr allow bool enforceWouldDeny bool + MitmConfig *acl.MitmConfig } type SmokescreenContext struct { @@ -314,9 +318,20 @@ func dialContext(ctx context.Context, network, addr string) (net.Conn, error) { } sctx.logger = sctx.logger.WithFields(fields) - // Only wrap CONNECT conns with an InstrumentedConn. Connections used for traditional HTTP proxy + // Only wrap CONNECT conns and MITM http conns with an InstrumentedConn. Connections used for traditional HTTP proxy // requests are pooled and reused by net.Transport. - if sctx.proxyType == connectProxy { + if sctx.proxyType == connectProxy || pctx.ConnectAction == goproxy.ConnectMitm { + // If we have a MITM and option is enabled, we can add detailed Request log fields + if pctx.ConnectAction == goproxy.ConnectMitm && sctx.Decision.MitmConfig != nil && sctx.Decision.MitmConfig.DetailedHttpLogs { + fields := logrus.Fields{ + LogMitmReqUrl: pctx.Req.URL.String(), + LogMitmReqMethod: pctx.Req.Method, + LogMitmReqHeaders: redactHeaders(pctx.Req.Header, sctx.Decision.MitmConfig.DetailedHttpLogsFullHeaders), + } + + sctx.logger = sctx.logger.WithFields(fields) + + } ic := sctx.cfg.ConnTracker.NewInstrumentedConnWithTimeout(conn, sctx.cfg.IdleTimeout, sctx.logger, d.role, d.outboundHost, sctx.proxyType) pctx.ConnErrorHandler = ic.Error conn = ic @@ -455,6 +470,13 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer { // Handle traditional HTTP proxy proxy.OnRequest().DoFunc(func(req *http.Request, pctx *goproxy.ProxyCtx) (*http.Request, *http.Response) { + // Set this on every request as every request mints a new goproxy.ProxyCtx + pctx.RoundTripper = rtFn + + // For MITM requests intended for the remote host, the sole requirement was to configure the RoundTripper + if pctx.ConnectAction == goproxy.ConnectMitm { + return req, nil + } // We are intentionally *not* setting pctx.HTTPErrorHandler because with traditional HTTP // proxy requests we are able to specify the request during the call to OnResponse(). @@ -469,9 +491,7 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer { req.Header.Del(traceHeader) }() - // Set this on every request as every request mints a new goproxy.ProxyCtx - pctx.RoundTripper = rtFn - + sctx.logger.WithField("url", req.RequestURI).Debug("received HTTP proxy request") // Build an address parsable by net.ResolveTCPAddr destination, err := hostport.NewWithScheme(req.Host, req.URL.Scheme, false) if err != nil { @@ -479,7 +499,6 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer { return req, rejectResponse(pctx, pctx.Error) } - sctx.logger.WithField("url", req.RequestURI).Debug("received HTTP proxy request") sctx.Decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, req, destination) // Returning any kind of response in this handler is goproxy's way of short circuiting @@ -512,15 +531,15 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer { // Defer logging the proxy event here because logProxy relies // on state set in handleConnect - defer logProxy(config, pctx) + defer logProxy(pctx) defer pctx.Req.Header.Del(traceHeader) - destination, err := handleConnect(config, pctx) + connectAction, destination, err := handleConnect(config, pctx) if err != nil { pctx.Resp = rejectResponse(pctx, err) return goproxy.RejectConnect, "" } - return goproxy.OkConnect, destination + return connectAction, destination }) // Strangely, goproxy can invoke this same function twice for a single HTTP request. @@ -552,9 +571,17 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer { return rejectResponse(pctx, pctx.Error) } - // In case of an error, this function is called a second time to filter the - // response we generate so this logger will be called once. - logProxy(config, pctx) + if pctx.ConnectAction == goproxy.ConnectMitm { + // If the connection is a MITM + // 1 we don't want to log as it will be done in HandleConnectFunc + // 2 we want to close idle connections as they are not closed by default + // and CANONICAL-PROXY-CN-CLOSE is called on InstrumentedConn.Close + proxy.Tr.CloseIdleConnections() + } else { + // In case of an error, this function is called a second time to filter the + // response we generate so this logger will be called once. + logProxy(pctx) + } return resp }) @@ -573,32 +600,11 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer { return proxy } -func logProxy(config *Config, pctx *goproxy.ProxyCtx) { +func logProxy(pctx *goproxy.ProxyCtx) { sctx := pctx.UserData.(*SmokescreenContext) fields := logrus.Fields{} - - // attempt to retrieve information about the host originating the proxy request - if pctx.Req.TLS != nil && len(pctx.Req.TLS.PeerCertificates) > 0 { - fields[LogFieldInRemoteX509CN] = pctx.Req.TLS.PeerCertificates[0].Subject.CommonName - var ouEntries = pctx.Req.TLS.PeerCertificates[0].Subject.OrganizationalUnit - if len(ouEntries) > 0 { - fields[LogFieldInRemoteX509OU] = ouEntries[0] - } - } - decision := sctx.Decision - if sctx.Decision != nil { - fields[LogFieldRole] = decision.role - fields[LogFieldProject] = decision.project - } - - // add the above fields to all future log messages sent using this smokescreen context's logger - sctx.logger = sctx.logger.WithFields(fields) - - // start a new set of fields used only in this log message - fields = logrus.Fields{} - // If a lookup takes less than 1ms it will be rounded down to zero. This can separated from // actual failures where the default zero value will also have the error field set. fields[LogFieldDNSLookupTime] = sctx.lookupTime.Milliseconds() @@ -630,14 +636,35 @@ func logProxy(config *Config, pctx *goproxy.ProxyCtx) { logMethod(CanonicalProxyDecision) } -func handleConnect(config *Config, pctx *goproxy.ProxyCtx) (string, error) { +func extractContextLogFields(pctx *goproxy.ProxyCtx, sctx *SmokescreenContext) logrus.Fields { + fields := logrus.Fields{} + + // attempt to retrieve information about the host originating the proxy request + if pctx.Req.TLS != nil && len(pctx.Req.TLS.PeerCertificates) > 0 { + fields[LogFieldInRemoteX509CN] = pctx.Req.TLS.PeerCertificates[0].Subject.CommonName + var ouEntries = pctx.Req.TLS.PeerCertificates[0].Subject.OrganizationalUnit + if len(ouEntries) > 0 { + fields[LogFieldInRemoteX509OU] = ouEntries[0] + } + } + + // Retrieve information from the ACL decision + decision := sctx.Decision + if sctx.Decision != nil { + fields[LogFieldRole] = decision.role + fields[LogFieldProject] = decision.project + } + return fields +} + +func handleConnect(config *Config, pctx *goproxy.ProxyCtx) (*goproxy.ConnectAction, string, error) { sctx := pctx.UserData.(*SmokescreenContext) // Check if requesting role is allowed to talk to remote destination, err := hostport.New(pctx.Req.Host, false) if err != nil { pctx.Error = denyError{err} - return "", pctx.Error + return nil, "", pctx.Error } // checkIfRequestShouldBeProxied can return an error if either the resolved address is disallowed, @@ -646,10 +673,14 @@ func handleConnect(config *Config, pctx *goproxy.ProxyCtx) (string, error) { sctx.Decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, pctx.Req, destination) if pctx.Error != nil { // DNS resolution failure - return "", pctx.Error + return nil, "", pctx.Error } + + // add context fields to all future log messages sent using this smokescreen context's logger + sctx.logger = sctx.logger.WithFields(extractContextLogFields(pctx, sctx)) + if !sctx.Decision.allow { - return "", denyError{errors.New(sctx.Decision.reason)} + return nil, "", denyError{errors.New(sctx.Decision.reason)} } // Call the custom request handler if it exists @@ -657,11 +688,40 @@ func handleConnect(config *Config, pctx *goproxy.ProxyCtx) (string, error) { err = config.PostDecisionRequestHandler(pctx.Req) if err != nil { pctx.Error = denyError{err} - return "", pctx.Error + return nil, "", pctx.Error + } + } + + connectAction := goproxy.OkConnect + // If the ACLDecision matched a MITM rule + if sctx.Decision.MitmConfig != nil { + if config.MitmCa == nil { + deny := denyError{errors.New("ACLDecision specified MITM but Smokescreen doesn't have MITM enabled")} + sctx.Decision.allow = false + sctx.Decision.MitmConfig = nil + sctx.Decision.reason = deny.Error() + return nil, "", deny + } + mitm := sctx.Decision.MitmConfig + + var mitmMutateRequest func(req *http.Request, ctx *goproxy.ProxyCtx) + + if len(mitm.AddHeaders) > 0 { + mitmMutateRequest = func(req *http.Request, ctx *goproxy.ProxyCtx) { + for k, v := range mitm.AddHeaders { + req.Header.Set(k, v) + } + } + } + + connectAction = &goproxy.ConnectAction{ + Action: goproxy.ConnectMitm, + TLSConfig: goproxy.TLSConfigFromCA(config.MitmCa), + MitmMutateRequest: mitmMutateRequest, } } - return destination.String(), nil + return connectAction, destination.String(), nil } func findListener(ip string, defaultPort uint16) (net.Listener, error) { @@ -964,6 +1024,7 @@ func checkACLsForRequest(config *Config, req *http.Request, destination hostport ACLDecision, err := config.EgressACL.Decide(role, destination.Host, connectProxyHost) decision.project = ACLDecision.Project decision.reason = ACLDecision.Reason + decision.MitmConfig = ACLDecision.MitmConfig if err != nil { config.Log.WithFields(logrus.Fields{ "error": err, @@ -1007,3 +1068,32 @@ func checkACLsForRequest(config *Config, req *http.Request, destination hostport return decision } + +func redactHeaders(originalHeaders http.Header, allowedHeaders []string) http.Header { + // Create a new map to store the redacted headers + redactedHeaders := make(http.Header) + + // Convert allowedHeaders to a map for faster lookup + allowedHeadersMap := make(map[string]bool) + for _, h := range allowedHeaders { + allowedHeadersMap[strings.ToLower(h)] = true + } + + // Iterate through the original headers + for key, values := range originalHeaders { + lowerKey := strings.ToLower(key) + if allowedHeadersMap[lowerKey] { + // If the header is in the allowed list, copy it as is + redactedHeaders[key] = values + } else { + // If not, redact the values + redactedValues := make([]string, len(values)) + for i := range values { + redactedValues[i] = "[REDACTED]" + } + redactedHeaders[key] = redactedValues + } + } + + return redactedHeaders +} diff --git a/pkg/smokescreen/smokescreen_test.go b/pkg/smokescreen/smokescreen_test.go index f9a0981f..7281ede1 100644 --- a/pkg/smokescreen/smokescreen_test.go +++ b/pkg/smokescreen/smokescreen_test.go @@ -6,6 +6,7 @@ package smokescreen import ( "context" "crypto/tls" + "crypto/x509" "errors" "fmt" "io" @@ -15,6 +16,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "sync/atomic" "testing" "time" @@ -23,6 +25,7 @@ import ( logrustest "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/stripe/goproxy" "github.com/stripe/smokescreen/pkg/smokescreen/conntrack" "github.com/stripe/smokescreen/pkg/smokescreen/metrics" ) @@ -1387,6 +1390,108 @@ func TestCONNECTProxyACLs(t *testing.T) { r.Equal("host matched allowed domain in rule", second_entry.Data["decision_reason"]) }) } + +func TestMitm(t *testing.T) { + t.Run("CONNECT proxy", func(t *testing.T) { + a := assert.New(t) + r := require.New(t) + + cfg, err := testConfig("test-mitm") + r.NoError(err) + // We use the default test certificates from Goproxy + mitmCa, err := tls.X509KeyPair(goproxy.CA_CERT, goproxy.CA_KEY) + r.NoError(err) + mitmCa.Leaf, err = x509.ParseCertificate(mitmCa.Certificate[0]) + r.NoError(err) + cfg.MitmCa = &mitmCa + 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 + // This handlers returns a body with a string containing all the request headers it received. + var sb strings.Builder + for name, values := range r.Header { + for _, value := range values { + sb.WriteString(name) + sb.WriteString(": ") + sb.WriteString(value) + sb.WriteString(";") + } + } + io.WriteString(w, sb.String()) + w.Write([]byte(sb.String())) + }) + + logHook := proxyLogHook(cfg) + l, err := net.Listen("tcp", "localhost:0") + r.NoError(err) + cfg.Listener = l + + proxy := proxyServer(cfg) + remote := httptest.NewTLSServer(h) + client, err := proxyClient(proxy.URL) + r.NoError(err) + + req, err := http.NewRequest("GET", remote.URL, nil) + r.NoError(err) + + go func() { + resp, err := client.Do(req) + r.NoError(err) + body, err := ioutil.ReadAll(resp.Body) + r.NoError(err) + resp.Body.Close() + // We check the response body to see if the Mitm-Header-Inject header was injected by the Mitm handler + a.Contains(string(body), "Accept-Language: el") + clientCh <- true + }() + + <-serverCh + count := 0 + cfg.ConnTracker.Range(func(k, v interface{}) bool { + count++ + return true + }) + a.Equal(1, count, "connTracker should contain one tracked connection") + + serverCh <- true + <-clientCh + + // Metrics should show one successful connection and a corresponding successful + // DNS request along with its timing metric. + tmc, ok := cfg.MetricsClient.(*metrics.MockMetricsClient) + r.True(ok) + i, err := tmc.GetCount("cn.atpt.total", map[string]string{"success": "true"}) + r.NoError(err) + r.Equal(i, uint64(1)) + lookups, err := tmc.GetCount("resolver.attempts_total", make(map[string]string)) + r.NoError(err) + r.Equal(lookups, uint64(1)) + ltime, err := tmc.GetCount("resolver.lookup_time", make(map[string]string)) + r.NoError(err) + r.Equal(ltime, uint64(1)) + + proxyDecision := findCanonicalProxyDecision(logHook.AllEntries()) + r.NotNil(proxyDecision) + r.Contains(proxyDecision.Data, "proxy_type") + r.Equal("connect", proxyDecision.Data["proxy_type"]) + // check proxyclose log entry has information about the request headers + proxyClose := findCanonicalProxyClose(logHook.AllEntries()) + r.NotNil(proxyClose) + r.Equal("GET", proxyClose.Data["mitm_req_method"]) + r.Contains(proxyClose.Data["mitm_req_url"], "https://127.0.0.1") + mitmReqHeaders, ok := proxyClose.Data["mitm_req_headers"].(http.Header) + r.True(ok) + r.Equal("[REDACTED]", mitmReqHeaders.Get("Accept-Language")) + r.Equal("Go-http-client/1.1", mitmReqHeaders.Get("User-Agent")) + }) +} + func findCanonicalProxyDecision(logs []*logrus.Entry) *logrus.Entry { for _, entry := range logs { if entry.Message == CanonicalProxyDecision { diff --git a/pkg/smokescreen/testdata/acl.yaml b/pkg/smokescreen/testdata/acl.yaml index 5e7230a9..c0de1c7a 100644 --- a/pkg/smokescreen/testdata/acl.yaml +++ b/pkg/smokescreen/testdata/acl.yaml @@ -33,6 +33,16 @@ services: - myproxy.com - myproxy2.com - thisisaproxy.com + - name: test-mitm + project: security + action: enforce + allowed_domains_mitm: + - domain: 127.0.0.1 + add_headers: + Accept-Language: el + detailed_http_logs: true + detailed_http_logs_full_headers: + - User-Agent global_deny_list: - stripe.com From 4e1b3e2f4dc0fd0464860571055105cc36bfc1d1 Mon Sep 17 00:00:00 2001 From: Harold Simpson Date: Thu, 12 Sep 2024 16:25:20 +0200 Subject: [PATCH 3/6] Use MitmTLSConfig in the config instead of MitmCa --- pkg/smokescreen/config.go | 3 ++- pkg/smokescreen/config_loader.go | 3 ++- pkg/smokescreen/smokescreen.go | 4 ++-- pkg/smokescreen/smokescreen_test.go | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/smokescreen/config.go b/pkg/smokescreen/config.go index bb074774..63897b13 100644 --- a/pkg/smokescreen/config.go +++ b/pkg/smokescreen/config.go @@ -19,6 +19,7 @@ import ( "time" log "github.com/sirupsen/logrus" + "github.com/stripe/goproxy" acl "github.com/stripe/smokescreen/pkg/smokescreen/acl/v1" "github.com/stripe/smokescreen/pkg/smokescreen/conntrack" "github.com/stripe/smokescreen/pkg/smokescreen/metrics" @@ -100,7 +101,7 @@ type Config struct { // If the handler returns an error, smokescreen will deny the request. PostDecisionRequestHandler func(*http.Request) error // MitmCa is used to provide a custom CA for MITM - MitmCa *tls.Certificate + MitmTLSConfig func(host string, ctx *goproxy.ProxyCtx) (*tls.Config, error) } type missingRoleError struct { diff --git a/pkg/smokescreen/config_loader.go b/pkg/smokescreen/config_loader.go index b64c05a7..3734a26d 100644 --- a/pkg/smokescreen/config_loader.go +++ b/pkg/smokescreen/config_loader.go @@ -10,6 +10,7 @@ import ( "strconv" "time" + "github.com/stripe/goproxy" "gopkg.in/yaml.v2" ) @@ -169,7 +170,7 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { if mitmCa.Leaf, err = x509.ParseCertificate(mitmCa.Certificate[0]); err != nil { return fmt.Errorf("could not populate x509 Leaf value: %v", err) } - c.MitmCa = &mitmCa + c.MitmTLSConfig = goproxy.TLSConfigFromCA(&mitmCa) } return nil diff --git a/pkg/smokescreen/smokescreen.go b/pkg/smokescreen/smokescreen.go index 592ef3dc..b71b35ba 100644 --- a/pkg/smokescreen/smokescreen.go +++ b/pkg/smokescreen/smokescreen.go @@ -695,7 +695,7 @@ func handleConnect(config *Config, pctx *goproxy.ProxyCtx) (*goproxy.ConnectActi connectAction := goproxy.OkConnect // If the ACLDecision matched a MITM rule if sctx.Decision.MitmConfig != nil { - if config.MitmCa == nil { + if config.MitmTLSConfig == nil { deny := denyError{errors.New("ACLDecision specified MITM but Smokescreen doesn't have MITM enabled")} sctx.Decision.allow = false sctx.Decision.MitmConfig = nil @@ -716,7 +716,7 @@ func handleConnect(config *Config, pctx *goproxy.ProxyCtx) (*goproxy.ConnectActi connectAction = &goproxy.ConnectAction{ Action: goproxy.ConnectMitm, - TLSConfig: goproxy.TLSConfigFromCA(config.MitmCa), + TLSConfig: config.MitmTLSConfig, MitmMutateRequest: mitmMutateRequest, } } diff --git a/pkg/smokescreen/smokescreen_test.go b/pkg/smokescreen/smokescreen_test.go index 7281ede1..9c8c618c 100644 --- a/pkg/smokescreen/smokescreen_test.go +++ b/pkg/smokescreen/smokescreen_test.go @@ -1403,7 +1403,7 @@ func TestMitm(t *testing.T) { r.NoError(err) mitmCa.Leaf, err = x509.ParseCertificate(mitmCa.Certificate[0]) r.NoError(err) - cfg.MitmCa = &mitmCa + cfg.MitmTLSConfig = goproxy.TLSConfigFromCA(&mitmCa) r.NoError(err) err = cfg.SetAllowAddresses([]string{"127.0.0.1"}) r.NoError(err) From ddde90f082836d1e7f6e21fa82621c5ba430579e Mon Sep 17 00:00:00 2001 From: Harold Simpson Date: Fri, 13 Sep 2024 11:55:54 +0200 Subject: [PATCH 4/6] PR feedback + remove CloseIdleConnections --- README.md | 1 + pkg/smokescreen/acl/v1/acl.go | 85 ++++++++++++++------------- pkg/smokescreen/acl/v1/acl_test.go | 2 +- pkg/smokescreen/acl/v1/yaml_loader.go | 27 ++++----- pkg/smokescreen/config_loader.go | 8 ++- pkg/smokescreen/smokescreen.go | 64 +++++++++----------- pkg/smokescreen/smokescreen_test.go | 6 +- 7 files changed, 97 insertions(+), 96 deletions(-) diff --git a/README.md b/README.md index b0624f8f..1ab88070 100644 --- a/README.md +++ b/README.md @@ -186,3 +186,4 @@ See [Development.md](Development.md) - Evan Broder - Marc-André Tremblay - Ryan Koppenhaver +- Harold Simpson diff --git a/pkg/smokescreen/acl/v1/acl.go b/pkg/smokescreen/acl/v1/acl.go index 6540cdc8..5c48b643 100644 --- a/pkg/smokescreen/acl/v1/acl.go +++ b/pkg/smokescreen/acl/v1/acl.go @@ -30,8 +30,10 @@ type Rule struct { } type MitmDomain struct { - MitmConfig - Domain string + AddHeaders map[string]string + DetailedHttpLogs bool + DetailedHttpLogsFullHeaders []string + Domain string } type MitmConfig struct { @@ -142,8 +144,12 @@ func (acl *ACL) Decide(service, host, connectProxyHost string) (Decision, error) // if the host matches any of the rule's allowed domains with MITM config, allow for _, dg := range rule.DomainMitmGlobs { if HostMatchesGlob(host, dg.Domain) { - d.Result, d.Reason = Allow, "host matched allowed domain in rule" - d.MitmConfig = (*MitmConfig)(&dg.MitmConfig) + d.Result, d.Reason = Allow, "host matched allowed domain in MITM rule" + d.MitmConfig = &MitmConfig{ + AddHeaders: dg.AddHeaders, + DetailedHttpLogs: dg.DetailedHttpLogs, + DetailedHttpLogsFullHeaders: dg.DetailedHttpLogsFullHeaders, + } return d, nil } } @@ -215,59 +221,58 @@ func (acl *ACL) Validate() error { } func (acl *ACL) ValidateRuleDomainsGlobs(svc string, r Rule) error { - err := acl.ValidateDomainGlobs(svc, r.DomainGlobs) - if err != nil { - return err - } - mitmDomainGlobs := make([]string, len(r.DomainMitmGlobs)) - for i, d := range r.DomainMitmGlobs { - mitmDomainGlobs[i] = d.Domain + var err error + for _, d := range r.DomainGlobs { + err = acl.ValidateDomainGlob(svc, d) + if err != nil { + return err + } } - err = acl.ValidateDomainGlobs(svc, mitmDomainGlobs) - if err != nil { - return err + for _, d := range r.DomainMitmGlobs { + err = acl.ValidateDomainGlob(svc, d.Domain) + if err != nil { + return err + } } return nil } -// ValidateDomainGlobs takes a slice of domain globs and verifies they conform to smokescreen's +// ValidateDomainGlob takes a domain glob and verifies they conform to smokescreen's // domain glob policy. // // Wildcards are valid only at the beginning of a domain glob, and only a single wildcard per glob // pattern is allowed. Globs must include text after a wildcard. // // Domains must use their normalized form (e.g., Punycode) -func (acl *ACL) ValidateDomainGlobs(svc string, globs []string) error { - for _, glob := range globs { - if glob == "" { - return fmt.Errorf("glob cannot be empty") - } +func (*ACL) ValidateDomainGlob(svc string, glob string) error { + if glob == "" { + return fmt.Errorf("glob cannot be empty") + } - if glob == "*" || glob == "*." { - return fmt.Errorf("%v: %v: domain glob must not match everything", svc, glob) - } + if glob == "*" || glob == "*." { + return fmt.Errorf("%v: %v: domain glob must not match everything", svc, glob) + } - if !strings.HasPrefix(glob, "*.") && strings.HasPrefix(glob, "*") { - return fmt.Errorf("%v: %v: domain glob must represent a full prefix (sub)domain", svc, glob) - } + if !strings.HasPrefix(glob, "*.") && strings.HasPrefix(glob, "*") { + return fmt.Errorf("%v: %v: domain glob must represent a full prefix (sub)domain", svc, glob) + } - domainToCheck := strings.TrimPrefix(glob, "*") - if strings.Contains(domainToCheck, "*") { - return fmt.Errorf("%v: %v: domain globs are only supported as prefix", svc, glob) - } + domainToCheck := strings.TrimPrefix(glob, "*") + if strings.Contains(domainToCheck, "*") { + return fmt.Errorf("%v: %v: domain globs are only supported as prefix", svc, glob) + } - normalizedDomain, err := hostport.NormalizeHost(domainToCheck, false) + normalizedDomain, err := hostport.NormalizeHost(domainToCheck, false) - if err != nil { - return fmt.Errorf("%v: %v: incorrect ACL entry: %v", svc, glob, err) - } else if normalizedDomain != domainToCheck { - // There was no error but the config contains a non-normalized form - if strings.HasPrefix(glob, "*.") { - // (Re-add) wildcard if one was provided (for the error message) - normalizedDomain = "*." + normalizedDomain - } - return fmt.Errorf("%v: %v: incorrect ACL entry; use %q", svc, glob, normalizedDomain) + if err != nil { + return fmt.Errorf("%v: %v: incorrect ACL entry: %v", svc, glob, err) + // There was no error but the config contains a non-normalized form + } else if normalizedDomain != domainToCheck { + if strings.HasPrefix(glob, "*.") { + // (Re-add) wildcard if one was provided (for the error message) + normalizedDomain = "*." + normalizedDomain } + return fmt.Errorf("%v: %v: incorrect ACL entry; use %q", svc, glob, normalizedDomain) } return nil } diff --git a/pkg/smokescreen/acl/v1/acl_test.go b/pkg/smokescreen/acl/v1/acl_test.go index c299e700..bbb2b84b 100644 --- a/pkg/smokescreen/acl/v1/acl_test.go +++ b/pkg/smokescreen/acl/v1/acl_test.go @@ -377,7 +377,7 @@ func TestMitmComfig(t *testing.T) { d, err := acl.Decide(mitmService, "example-mitm.com", "") a.NoError(err) a.Equal(Allow, d.Result) - a.Equal("host matched allowed domain in rule", d.Reason) + a.Equal("host matched allowed domain in MITM rule", d.Reason) a.NotNil(d.MitmConfig) a.Equal(true, d.MitmConfig.DetailedHttpLogs) diff --git a/pkg/smokescreen/acl/v1/yaml_loader.go b/pkg/smokescreen/acl/v1/yaml_loader.go index db39803d..13554f30 100644 --- a/pkg/smokescreen/acl/v1/yaml_loader.go +++ b/pkg/smokescreen/acl/v1/yaml_loader.go @@ -89,14 +89,7 @@ func (cfg *YAMLConfig) Load() (*ACL, error) { var allowedHostsMitm []MitmDomain for _, w := range v.AllowedHostsMitm { - mitmDomain := MitmDomain{ - MitmConfig: MitmConfig{ - AddHeaders: w.AddHeaders, - DetailedHttpLogs: w.DetailedHttpLogs, - DetailedHttpLogsFullHeaders: w.DetailedHttpLogsFullHeaders, - }, - Domain: w.Domain, - } + mitmDomain := NewMITMDomain(w) allowedHostsMitm = append(allowedHostsMitm, mitmDomain) } @@ -123,14 +116,7 @@ func (cfg *YAMLConfig) Load() (*ACL, error) { var allowedHostsMitm []MitmDomain for _, w := range cfg.Default.AllowedHostsMitm { - mitmDomain := MitmDomain{ - MitmConfig: MitmConfig{ - AddHeaders: w.AddHeaders, - DetailedHttpLogs: w.DetailedHttpLogs, - DetailedHttpLogsFullHeaders: w.DetailedHttpLogsFullHeaders, - }, - Domain: w.Domain, - } + mitmDomain := NewMITMDomain(w) allowedHostsMitm = append(allowedHostsMitm, mitmDomain) } @@ -155,3 +141,12 @@ func (cfg *YAMLConfig) Load() (*ACL, error) { return &acl, nil } + +func NewMITMDomain(w YAMLMitmRule) MitmDomain { + return MitmDomain{ + AddHeaders: w.AddHeaders, + DetailedHttpLogs: w.DetailedHttpLogs, + DetailedHttpLogsFullHeaders: w.DetailedHttpLogsFullHeaders, + Domain: w.Domain, + } +} diff --git a/pkg/smokescreen/config_loader.go b/pkg/smokescreen/config_loader.go index 3734a26d..e9b6f715 100644 --- a/pkg/smokescreen/config_loader.go +++ b/pkg/smokescreen/config_loader.go @@ -165,10 +165,14 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { } mitmCa, err := tls.LoadX509KeyPair(yc.MitmCaCertFile, yc.MitmCaKeyFile) if err != nil { - return fmt.Errorf("could not load mitmCa: %v", err) + return fmt.Errorf("mitm_ca_key_file error tls.LoadX509KeyPair: %w", err) + } + // set the leaf certificat to reduce per-handshake processing + if len(mitmCa.Certificate) == 0 { + return errors.New("mitm_ca_key_file error: mitm_ca_key_file contains no certificates") } if mitmCa.Leaf, err = x509.ParseCertificate(mitmCa.Certificate[0]); err != nil { - return fmt.Errorf("could not populate x509 Leaf value: %v", err) + return fmt.Errorf("could not populate x509 Leaf value: %w", err) } c.MitmTLSConfig = goproxy.TLSConfigFromCA(&mitmCa) } diff --git a/pkg/smokescreen/smokescreen.go b/pkg/smokescreen/smokescreen.go index b71b35ba..5dc95135 100644 --- a/pkg/smokescreen/smokescreen.go +++ b/pkg/smokescreen/smokescreen.go @@ -289,10 +289,7 @@ func dialContext(ctx context.Context, network, addr string) (net.Conn, error) { conn, err = sctx.cfg.ProxyDialTimeout(ctx, network, d.ResolvedAddr.String(), sctx.cfg.ConnectTimeout) } connTime := time.Since(start) - - fields := logrus.Fields{ - LogFieldConnEstablishMS: connTime.Milliseconds(), - } + sctx.logger = sctx.logger.WithFields(dialContextLoggerFields(pctx, sctx, conn, connTime)) if sctx.cfg.TimeConnect { sctx.cfg.MetricsClient.TimingWithTags("cn.atpt.connect.time", connTime, map[string]string{"domain": sctx.requestedHost}, 1) @@ -307,6 +304,22 @@ func dialContext(ctx context.Context, network, addr string) (net.Conn, error) { sctx.cfg.MetricsClient.IncrWithTags("cn.atpt.total", map[string]string{"success": "true"}, 1) sctx.cfg.ConnTracker.RecordAttempt(sctx.requestedHost, true) + // Only wrap CONNECT conns with an InstrumentedConn. Connections used for traditional HTTP proxy + // requests are pooled and reused by net.Transport. + if sctx.proxyType == connectProxy { + ic := sctx.cfg.ConnTracker.NewInstrumentedConnWithTimeout(conn, sctx.cfg.IdleTimeout, sctx.logger, d.role, d.outboundHost, sctx.proxyType) + pctx.ConnErrorHandler = ic.Error + conn = ic + } else { + conn = NewTimeoutConn(conn, sctx.cfg.IdleTimeout) + } + + return conn, nil +} +func dialContextLoggerFields(pctx *goproxy.ProxyCtx, sctx *SmokescreenContext, conn net.Conn, connTime time.Duration) logrus.Fields { + fields := logrus.Fields{ + LogFieldConnEstablishMS: connTime.Milliseconds(), + } if conn != nil { if addr := conn.LocalAddr(); addr != nil { fields[LogFieldOutLocalAddr] = addr.String() @@ -316,30 +329,14 @@ func dialContext(ctx context.Context, network, addr string) (net.Conn, error) { fields[LogFieldOutRemoteAddr] = addr.String() } } - sctx.logger = sctx.logger.WithFields(fields) - - // Only wrap CONNECT conns and MITM http conns with an InstrumentedConn. Connections used for traditional HTTP proxy - // requests are pooled and reused by net.Transport. - if sctx.proxyType == connectProxy || pctx.ConnectAction == goproxy.ConnectMitm { - // If we have a MITM and option is enabled, we can add detailed Request log fields - if pctx.ConnectAction == goproxy.ConnectMitm && sctx.Decision.MitmConfig != nil && sctx.Decision.MitmConfig.DetailedHttpLogs { - fields := logrus.Fields{ - LogMitmReqUrl: pctx.Req.URL.String(), - LogMitmReqMethod: pctx.Req.Method, - LogMitmReqHeaders: redactHeaders(pctx.Req.Header, sctx.Decision.MitmConfig.DetailedHttpLogsFullHeaders), - } - - sctx.logger = sctx.logger.WithFields(fields) - - } - ic := sctx.cfg.ConnTracker.NewInstrumentedConnWithTimeout(conn, sctx.cfg.IdleTimeout, sctx.logger, d.role, d.outboundHost, sctx.proxyType) - pctx.ConnErrorHandler = ic.Error - conn = ic - } else { - conn = NewTimeoutConn(conn, sctx.cfg.IdleTimeout) + // If we have a MITM and option is enabled, we can add detailed Request log fields + if pctx.ConnectAction == goproxy.ConnectMitm && sctx.Decision.MitmConfig != nil && sctx.Decision.MitmConfig.DetailedHttpLogs { + fields[LogMitmReqUrl] = pctx.Req.URL.String() + fields[LogMitmReqMethod] = pctx.Req.Method + fields[LogMitmReqHeaders] = redactHeaders(pctx.Req.Header, sctx.Decision.MitmConfig.DetailedHttpLogsFullHeaders) } - return conn, nil + return fields } // HTTPErrorHandler allows returning a custom error response when smokescreen @@ -468,12 +465,14 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer { } } - // Handle traditional HTTP proxy + // Handle traditional HTTP proxy and MITM outgoing requests (smokescreen - remote ) proxy.OnRequest().DoFunc(func(req *http.Request, pctx *goproxy.ProxyCtx) (*http.Request, *http.Response) { // Set this on every request as every request mints a new goproxy.ProxyCtx pctx.RoundTripper = rtFn - // For MITM requests intended for the remote host, the sole requirement was to configure the RoundTripper + // In the context of MITM request. Once the originating request (client - smokescreen) has been allowed + // goproxy/https.go calls proxy.filterRequest on the outgoing request (smokescreen - remote host) which calls this function + // in this case we ony want to configure the RoundTripper if pctx.ConnectAction == goproxy.ConnectMitm { return req, nil } @@ -571,13 +570,8 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer { return rejectResponse(pctx, pctx.Error) } - if pctx.ConnectAction == goproxy.ConnectMitm { - // If the connection is a MITM - // 1 we don't want to log as it will be done in HandleConnectFunc - // 2 we want to close idle connections as they are not closed by default - // and CANONICAL-PROXY-CN-CLOSE is called on InstrumentedConn.Close - proxy.Tr.CloseIdleConnections() - } else { + // We don't want to log if the connection is a MITM as it will be done in HandleConnectFunc + if pctx.ConnectAction != goproxy.ConnectMitm { // In case of an error, this function is called a second time to filter the // response we generate so this logger will be called once. logProxy(pctx) diff --git a/pkg/smokescreen/smokescreen_test.go b/pkg/smokescreen/smokescreen_test.go index 9c8c618c..0f7b0120 100644 --- a/pkg/smokescreen/smokescreen_test.go +++ b/pkg/smokescreen/smokescreen_test.go @@ -1432,9 +1432,10 @@ func TestMitm(t *testing.T) { r.NoError(err) cfg.Listener = l - proxy := proxyServer(cfg) + proxy := BuildProxy(cfg) + httpProxy := httptest.NewServer(proxy) remote := httptest.NewTLSServer(h) - client, err := proxyClient(proxy.URL) + client, err := proxyClient(httpProxy.URL) r.NoError(err) req, err := http.NewRequest("GET", remote.URL, nil) @@ -1480,6 +1481,7 @@ func TestMitm(t *testing.T) { r.NotNil(proxyDecision) r.Contains(proxyDecision.Data, "proxy_type") r.Equal("connect", proxyDecision.Data["proxy_type"]) + proxy.Tr.CloseIdleConnections() // check proxyclose log entry has information about the request headers proxyClose := findCanonicalProxyClose(logHook.AllEntries()) r.NotNil(proxyClose) From 92537efb91da11f73e7ac0a3a335f1376b3ed2cc Mon Sep 17 00:00:00 2001 From: Harold Simpson Date: Tue, 17 Sep 2024 21:12:29 +0200 Subject: [PATCH 5/6] Refactor allowed_domains_mitm to mitm_domains --- Development.md | 10 +++-- pkg/smokescreen/acl/v1/acl.go | 45 ++++++++++++------- pkg/smokescreen/acl/v1/acl_test.go | 28 +++++++++++- .../acl/v1/testdata/mitm_config.yaml | 3 +- pkg/smokescreen/acl/v1/yaml_loader.go | 33 +++++++------- pkg/smokescreen/acl/v1/yaml_loader_test.go | 10 +++++ pkg/smokescreen/testdata/acl.yaml | 4 +- 7 files changed, 93 insertions(+), 40 deletions(-) diff --git a/Development.md b/Development.md index ad8e408e..13b007a7 100644 --- a/Development.md +++ b/Development.md @@ -229,8 +229,9 @@ default: name: default project: security action: enforce - allowed_domains: [] - allowed_domains_mitm: + allowed_domains: + - wttr.in + mitm_domains: - domain: wttr.in add_headers: Accept-Language: el @@ -278,8 +279,9 @@ services: - name: localhost project: github action: enforce - allowed_domains: [] - allowed_domains_mitm: + allowed_domains: + - wttr.in + mitm_domains: - domain: wttr.in add_headers: Accept-Language: el diff --git a/pkg/smokescreen/acl/v1/acl.go b/pkg/smokescreen/acl/v1/acl.go index 5c48b643..c4bc7eed 100644 --- a/pkg/smokescreen/acl/v1/acl.go +++ b/pkg/smokescreen/acl/v1/acl.go @@ -25,7 +25,7 @@ type Rule struct { Project string Policy EnforcementPolicy DomainGlobs []string - DomainMitmGlobs []MitmDomain + MitmDomains []MitmDomain ExternalProxyGlobs []string } @@ -83,7 +83,7 @@ func (acl *ACL) Add(svc string, r Rule) error { return err } - err = acl.ValidateRuleDomainsGlobs(svc, r) + err = acl.ValidateRuleDomainsGlobsAndMitm(svc, r) if err != nil { return err } @@ -137,18 +137,16 @@ func (acl *ACL) Decide(service, host, connectProxyHost string) (Decision, error) for _, dg := range rule.DomainGlobs { if HostMatchesGlob(host, dg) { d.Result, d.Reason = Allow, "host matched allowed domain in rule" - return d, nil - } - } - - // if the host matches any of the rule's allowed domains with MITM config, allow - for _, dg := range rule.DomainMitmGlobs { - if HostMatchesGlob(host, dg.Domain) { - d.Result, d.Reason = Allow, "host matched allowed domain in MITM rule" - d.MitmConfig = &MitmConfig{ - AddHeaders: dg.AddHeaders, - DetailedHttpLogs: dg.DetailedHttpLogs, - DetailedHttpLogsFullHeaders: dg.DetailedHttpLogsFullHeaders, + // Check if we can find a matching MITM config + for _, dg := range rule.MitmDomains { + if HostMatchesGlob(host, dg.Domain) { + d.MitmConfig = &MitmConfig{ + AddHeaders: dg.AddHeaders, + DetailedHttpLogs: dg.DetailedHttpLogs, + DetailedHttpLogsFullHeaders: dg.DetailedHttpLogsFullHeaders, + } + return d, nil + } } return d, nil } @@ -208,7 +206,7 @@ func (acl *ACL) DisablePolicies(actions []string) error { // and is not utilizing a disabled enforcement policy. func (acl *ACL) Validate() error { for svc, r := range acl.Rules { - err := acl.ValidateRuleDomainsGlobs(svc, r) + err := acl.ValidateRuleDomainsGlobsAndMitm(svc, r) if err != nil { return err } @@ -220,7 +218,7 @@ func (acl *ACL) Validate() error { return nil } -func (acl *ACL) ValidateRuleDomainsGlobs(svc string, r Rule) error { +func (acl *ACL) ValidateRuleDomainsGlobsAndMitm(svc string, r Rule) error { var err error for _, d := range r.DomainGlobs { err = acl.ValidateDomainGlob(svc, d) @@ -228,11 +226,16 @@ func (acl *ACL) ValidateRuleDomainsGlobs(svc string, r Rule) error { return err } } - for _, d := range r.DomainMitmGlobs { + for _, d := range r.MitmDomains { err = acl.ValidateDomainGlob(svc, d.Domain) if err != nil { return err } + // Check if the MITM config domain is also in DomainGlobs + // Replace with slices.ContainsString when project upgraded to > 1.21 + if !containsString(r.DomainGlobs, d.Domain) { + return fmt.Errorf("domain %s was added to mitm_domains but is missing in allowed_domains", d.Domain) + } } return nil } @@ -327,3 +330,11 @@ func HostMatchesGlob(host string, domainGlob string) bool { } return false } +func containsString(slice []string, str string) bool { + for _, item := range slice { + if item == str { + return true + } + } + return false +} diff --git a/pkg/smokescreen/acl/v1/acl_test.go b/pkg/smokescreen/acl/v1/acl_test.go index bbb2b84b..c2b9f860 100644 --- a/pkg/smokescreen/acl/v1/acl_test.go +++ b/pkg/smokescreen/acl/v1/acl_test.go @@ -377,10 +377,36 @@ func TestMitmComfig(t *testing.T) { d, err := acl.Decide(mitmService, "example-mitm.com", "") a.NoError(err) a.Equal(Allow, d.Result) - a.Equal("host matched allowed domain in MITM rule", d.Reason) + a.Equal("host matched allowed domain in rule", d.Reason) a.NotNil(d.MitmConfig) a.Equal(true, d.MitmConfig.DetailedHttpLogs) a.Equal([]string{"User-Agent"}, d.MitmConfig.DetailedHttpLogsFullHeaders) a.Equal(map[string]string{"Accept-Language": "el"}, d.MitmConfig.AddHeaders) } + +func TestInvalidMitmComfig(t *testing.T) { + a := assert.New(t) + + acl := &ACL{ + Rules: map[string]Rule{ + "enforce-dummy-mitm-srv": { + Project: "usersec", + Policy: Enforce, + DomainGlobs: []string{ + "example.com", + }, + MitmDomains: []MitmDomain{{ + Domain: "example-mitm.com", + AddHeaders: map[string]string{ + "Accept-Language": "el", + }, + DetailedHttpLogs: true, + }}, + }, + }, + } + + err := acl.Validate() + a.Error(err) +} diff --git a/pkg/smokescreen/acl/v1/testdata/mitm_config.yaml b/pkg/smokescreen/acl/v1/testdata/mitm_config.yaml index b9fde18f..524ceb6c 100644 --- a/pkg/smokescreen/acl/v1/testdata/mitm_config.yaml +++ b/pkg/smokescreen/acl/v1/testdata/mitm_config.yaml @@ -7,7 +7,8 @@ services: allowed_domains: - examplea.com - exampleb.com - allowed_domains_mitm: + - example-mitm.com + mitm_domains: - domain: example-mitm.com add_headers: Accept-Language: el diff --git a/pkg/smokescreen/acl/v1/yaml_loader.go b/pkg/smokescreen/acl/v1/yaml_loader.go index 13554f30..4968ea36 100644 --- a/pkg/smokescreen/acl/v1/yaml_loader.go +++ b/pkg/smokescreen/acl/v1/yaml_loader.go @@ -26,15 +26,15 @@ type YAMLConfig struct { } type YAMLRule struct { - Name string `yaml:"name"` - Project string `yaml:"project"` // owner - Action string `yaml:"action"` - AllowedHosts []string `yaml:"allowed_domains"` - AllowedHostsMitm []YAMLMitmRule `yaml:"allowed_domains_mitm"` - AllowedExternalProxyHosts []string `yaml:"allowed_external_proxies"` + Name string `yaml:"name"` + Project string `yaml:"project"` // owner + Action string `yaml:"action"` + AllowedHosts []string `yaml:"allowed_domains"` + MitmDomains []YAMLMitmDomain `yaml:"mitm_domains"` + AllowedExternalProxyHosts []string `yaml:"allowed_external_proxies"` } -type YAMLMitmRule struct { +type YAMLMitmDomain struct { Domain string `yaml:"domain"` AddHeaders map[string]string `yaml:"add_headers"` DetailedHttpLogs bool `yaml:"detailed_http_logs"` @@ -86,18 +86,18 @@ func (cfg *YAMLConfig) Load() (*ACL, error) { return nil, err } - var allowedHostsMitm []MitmDomain + var mitmDomains []MitmDomain - for _, w := range v.AllowedHostsMitm { + for _, w := range v.MitmDomains { mitmDomain := NewMITMDomain(w) - allowedHostsMitm = append(allowedHostsMitm, mitmDomain) + mitmDomains = append(mitmDomains, mitmDomain) } r := Rule{ Project: v.Project, Policy: p, DomainGlobs: v.AllowedHosts, - DomainMitmGlobs: allowedHostsMitm, + MitmDomains: mitmDomains, ExternalProxyGlobs: v.AllowedExternalProxyHosts, } @@ -113,18 +113,19 @@ func (cfg *YAMLConfig) Load() (*ACL, error) { return nil, err } - var allowedHostsMitm []MitmDomain + var mitmDomains []MitmDomain + + for _, w := range cfg.Default.MitmDomains { - for _, w := range cfg.Default.AllowedHostsMitm { mitmDomain := NewMITMDomain(w) - allowedHostsMitm = append(allowedHostsMitm, mitmDomain) + mitmDomains = append(mitmDomains, mitmDomain) } acl.DefaultRule = &Rule{ Project: cfg.Default.Project, Policy: p, DomainGlobs: cfg.Default.AllowedHosts, - DomainMitmGlobs: allowedHostsMitm, + MitmDomains: mitmDomains, ExternalProxyGlobs: cfg.Default.AllowedExternalProxyHosts, } } @@ -142,7 +143,7 @@ func (cfg *YAMLConfig) Load() (*ACL, error) { return &acl, nil } -func NewMITMDomain(w YAMLMitmRule) MitmDomain { +func NewMITMDomain(w YAMLMitmDomain) MitmDomain { return MitmDomain{ AddHeaders: w.AddHeaders, DetailedHttpLogs: w.DetailedHttpLogs, diff --git a/pkg/smokescreen/acl/v1/yaml_loader_test.go b/pkg/smokescreen/acl/v1/yaml_loader_test.go index 1276beee..dd2b5d23 100644 --- a/pkg/smokescreen/acl/v1/yaml_loader_test.go +++ b/pkg/smokescreen/acl/v1/yaml_loader_test.go @@ -47,6 +47,16 @@ func TestYAMLLoader(t *testing.T) { a.NotNil(err) a.Nil(acl) } + + // Load a valid MITM config + { + yl := NewYAMLLoader("testdata/mitm_config.yaml") + acl, err := New(logrus.New(), yl, []string{}) + a.Nil(err) + a.NotNil(acl) + a.Equal(1, len(acl.Rules)) + a.Equal(1, len(acl.Rules["enforce-dummy-mitm-srv"].MitmDomains)) + } } func TestYAMLLoaderInvalidGlob(t *testing.T) { diff --git a/pkg/smokescreen/testdata/acl.yaml b/pkg/smokescreen/testdata/acl.yaml index c0de1c7a..609c56db 100644 --- a/pkg/smokescreen/testdata/acl.yaml +++ b/pkg/smokescreen/testdata/acl.yaml @@ -36,7 +36,9 @@ services: - name: test-mitm project: security action: enforce - allowed_domains_mitm: + allowed_domains: + - 127.0.0.1 + mitm_domains: - domain: 127.0.0.1 add_headers: Accept-Language: el From 4cf6e0bb1ef3c0f8ba2bb92a679e0d89d017dc05 Mon Sep 17 00:00:00 2001 From: Harold Simpson Date: Tue, 17 Sep 2024 22:41:33 +0200 Subject: [PATCH 6/6] Rename ValidateRule --- pkg/smokescreen/acl/v1/acl.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/smokescreen/acl/v1/acl.go b/pkg/smokescreen/acl/v1/acl.go index c4bc7eed..e75346d0 100644 --- a/pkg/smokescreen/acl/v1/acl.go +++ b/pkg/smokescreen/acl/v1/acl.go @@ -83,7 +83,7 @@ func (acl *ACL) Add(svc string, r Rule) error { return err } - err = acl.ValidateRuleDomainsGlobsAndMitm(svc, r) + err = acl.ValidateRule(svc, r) if err != nil { return err } @@ -206,7 +206,7 @@ func (acl *ACL) DisablePolicies(actions []string) error { // and is not utilizing a disabled enforcement policy. func (acl *ACL) Validate() error { for svc, r := range acl.Rules { - err := acl.ValidateRuleDomainsGlobsAndMitm(svc, r) + err := acl.ValidateRule(svc, r) if err != nil { return err } @@ -218,7 +218,7 @@ func (acl *ACL) Validate() error { return nil } -func (acl *ACL) ValidateRuleDomainsGlobsAndMitm(svc string, r Rule) error { +func (acl *ACL) ValidateRule(svc string, r Rule) error { var err error for _, d := range r.DomainGlobs { err = acl.ValidateDomainGlob(svc, d)