From c905517c2c98d18a522fb5df19b48631a6658b4e Mon Sep 17 00:00:00 2001 From: Harold Simpson Date: Tue, 17 Sep 2024 21:12:29 +0200 Subject: [PATCH] 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