From a7d2fd1a6b05950318afd20466113588c0245f45 Mon Sep 17 00:00:00 2001 From: jub0bs Date: Thu, 29 Feb 2024 12:06:48 +0100 Subject: [PATCH] relax the need for option TolerateInsecureOrigins --- fcors_anonymous_single_origin_test.go | 1 - fcors_invalid_policies_test.go | 102 ++++++++++++++++++++++---- internal/middleware.go | 42 ++++++++++- internal/options.go | 15 ++-- 4 files changed, 134 insertions(+), 26 deletions(-) diff --git a/fcors_anonymous_single_origin_test.go b/fcors_anonymous_single_origin_test.go index 924c99a..f464693 100644 --- a/fcors_anonymous_single_origin_test.go +++ b/fcors_anonymous_single_origin_test.go @@ -1677,7 +1677,6 @@ func Test_AllowAccess_From_Single_Insecure_Origin_With_Method_And_Header_And_Exp fcors.ExposeResponseHeaders(exposedResponseHeader), fcors.PreflightSuccessStatus(dummyPreflightSuccessStatus), fcors.MaxAgeInSeconds(dummyMaxAge), - risky.TolerateInsecureOrigins(), ) if err != nil { t.Errorf("got error with message %q; want nil error", err.Error()) diff --git a/fcors_invalid_policies_test.go b/fcors_invalid_policies_test.go index 3af7f3d..c654446 100644 --- a/fcors_invalid_policies_test.go +++ b/fcors_invalid_policies_test.go @@ -22,9 +22,21 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) { options: []fcors.OptionAnon{fcors.FromOrigins(" http://example.com:6060 ")}, errorMsg: `fcors: invalid origin pattern " http://example.com:6060 "`, }, { - desc: "specified origin is insecure", - options: []fcors.OptionAnon{fcors.FromOrigins("http://example.com:6060")}, - errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option risky.TolerateInsecureOrigins`, + desc: "option PrivateNetworkAccess is used and specified origin is insecure", + options: []fcors.OptionAnon{ + fcors.FromOrigins("http://example.com:6060"), + risky.PrivateNetworkAccess(), + }, + errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` + + `risky.TolerateInsecureOrigins when Private Network Access is enabled`, + }, { + desc: "option PrivateNetworkAccessInNoCorsModeOnly is used and specified origin is insecure", + options: []fcors.OptionAnon{ + fcors.FromOrigins("http://example.com:6060"), + risky.PrivateNetworkAccessInNoCorsModeOnly(), + }, + errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` + + `risky.TolerateInsecureOrigins when Private Network Access is enabled`, }, { desc: "specified origin's host is an invalid IP address", options: []fcors.OptionAnon{fcors.FromOrigins("http://[::1]1:6060")}, @@ -86,9 +98,27 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) { options: []fcors.OptionAnon{fcors.FromOrigins(" http://*.example.com:6060 ")}, errorMsg: `fcors: invalid origin pattern " http://*.example.com:6060 "`, }, { - desc: "specified base origin is insecure", - options: []fcors.OptionAnon{fcors.FromOrigins("http://*.example.com:6060")}, - errorMsg: `fcors: insecure origin pattern "http://*.example.com:6060" requires option risky.TolerateInsecureOrigins`, + desc: "option PrivateNetworkAccess is used and some origin patterns are insecure", + options: []fcors.OptionAnon{ + fcors.FromOrigins( + "http://example.com:6060", + "http://*.example.com:6060", + ), + risky.PrivateNetworkAccess(), + }, + errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` + + `require option risky.TolerateInsecureOrigins when Private Network Access is enabled`, + }, { + desc: "option PrivateNetworkAccessInNoCorsModeOnly is used and some origin patterns are insecure", + options: []fcors.OptionAnon{ + fcors.FromOrigins( + "http://example.com:6060", + "http://*.example.com:6060", + ), + risky.PrivateNetworkAccessInNoCorsModeOnly(), + }, + errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` + + `require option risky.TolerateInsecureOrigins when Private Network Access is enabled`, }, { desc: "specified base origin's host is an invalid IP address", options: []fcors.OptionAnon{fcors.FromOrigins("http://*.[::1]1:6060")}, @@ -547,7 +577,6 @@ func TestInvalidPoliciesForAllowAccess(t *testing.T) { `fcors: option WithRequestHeaders used multiple times`, `fcors: specified max-age value 86401 exceeds upper bound 86400`, `fcors: option MaxAgeInSeconds used multiple times`, - `fcors: insecure origin pattern "http://example.com" requires option risky.TolerateInsecureOrigins`, }, "\n"), }, } @@ -582,9 +611,28 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) { options: []fcors.Option{fcors.FromOrigins(" http://example.com:6060 ")}, errorMsg: `fcors: invalid origin pattern " http://example.com:6060 "`, }, { - desc: "specified origin is insecure", - options: []fcors.Option{fcors.FromOrigins("http://example.com:6060")}, - errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option risky.TolerateInsecureOrigins`, + desc: "specified origin is insecure", + options: []fcors.Option{fcors.FromOrigins("http://example.com:6060")}, + errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires ` + + `option risky.TolerateInsecureOrigins when credentialed access is enabled`, + }, { + desc: "option PrivateNetworkAccess is used and specified origin is insecure", + options: []fcors.Option{ + fcors.FromOrigins("http://example.com:6060"), + risky.PrivateNetworkAccess(), + }, + errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` + + `risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` + + `Private Network Access is enabled`, + }, { + desc: "option PrivateNetworkAccessInNoCorsModeOnly is used and specified origin is insecure", + options: []fcors.Option{ + fcors.FromOrigins("http://example.com:6060"), + risky.PrivateNetworkAccessInNoCorsModeOnly(), + }, + errorMsg: `fcors: insecure origin pattern "http://example.com:6060" requires option ` + + `risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` + + `Private Network Access is enabled`, }, { desc: "specified origin's host is an invalid IP address", options: []fcors.Option{fcors.FromOrigins("http://[::1]1:6060")}, @@ -646,9 +694,34 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) { options: []fcors.Option{fcors.FromOrigins(" http://*.example.com:6060 ")}, errorMsg: `fcors: invalid origin pattern " http://*.example.com:6060 "`, }, { - desc: "specified base origin is insecure", - options: []fcors.Option{fcors.FromOrigins("http://*.example.com:6060")}, - errorMsg: `fcors: insecure origin pattern "http://*.example.com:6060" requires option risky.TolerateInsecureOrigins`, + desc: "specified base origin is insecure", + options: []fcors.Option{fcors.FromOrigins("http://*.example.com:6060")}, + errorMsg: `fcors: insecure origin pattern "http://*.example.com:6060" requires option ` + + `risky.TolerateInsecureOrigins when credentialed access is enabled`, + }, { + desc: "option PrivateNetworkAccess is used and some origin patterns are insecure", + options: []fcors.Option{ + fcors.FromOrigins( + "http://example.com:6060", + "http://*.example.com:6060", + ), + risky.PrivateNetworkAccess(), + }, + errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` + + `require option risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` + + `Private Network Access is enabled`, + }, { + desc: "option PrivateNetworkAccessInNoCorsModeOnly is used and some origin patterns are insecure", + options: []fcors.Option{ + fcors.FromOrigins( + "http://example.com:6060", + "http://*.example.com:6060", + ), + risky.PrivateNetworkAccessInNoCorsModeOnly(), + }, + errorMsg: `fcors: insecure origin patterns "http://example.com:6060", "http://*.example.com:6060" ` + + `require option risky.TolerateInsecureOrigins when credentialed access is enabled and/or ` + + `Private Network Access is enabled`, }, { desc: "specified base origin's host is an invalid IP address", options: []fcors.Option{fcors.FromOrigins("http://*.[::1]1:6060")}, @@ -1034,7 +1107,8 @@ func TestInvalidPoliciesForAllowAccessWithCredentials(t *testing.T) { `fcors: option WithRequestHeaders used multiple times`, `fcors: specified max-age value 86401 exceeds upper bound 86400`, `fcors: option MaxAgeInSeconds used multiple times`, - `fcors: insecure origin pattern "http://example.com" requires option risky.TolerateInsecureOrigins`, + `fcors: insecure origin pattern "http://example.com" requires option ` + + `risky.TolerateInsecureOrigins when credentialed access is enabled`, }, "\n"), }, } diff --git a/internal/middleware.go b/internal/middleware.go index 64d689f..014e65a 100644 --- a/internal/middleware.go +++ b/internal/middleware.go @@ -58,7 +58,7 @@ func init() { type TempConfig struct { PublicSuffixError error - InsecureOriginPatternError error + InsecureOriginPatterns []string SingleNonWildcardOrigin string AllowedMethods util.Set[string] AllowedRequestHeaders util.Set[string] @@ -107,9 +107,45 @@ func newConfig(creds bool) *Config { func (cfg *Config) validate() error { var errs []error - if cfg.tmp.InsecureOriginPatternError != nil && + if len(cfg.tmp.InsecureOriginPatterns) > 0 && + (cfg.AllowCredentials || cfg.PrivateNetworkAccess || cfg.PrivateNetworkAccessInNoCorsModeOnly) && !cfg.tmp.TolerateInsecureOrigins { - errs = append(errs, cfg.tmp.InsecureOriginPatternError) + // Note: We don't require risky.TolerateInsecureOrigins + // when users specify one or more insecure origin patterns + // in anonymous-only mode and without PNA; + // in such cases, insecure origins like http://example.com + // are indeed no less insecure than * is, + // which itself doesn't require risky.TolerateInsecureOrigins. + var errorMsg strings.Builder + var patterns = cfg.tmp.InsecureOriginPatterns + if len(cfg.tmp.InsecureOriginPatterns) == 1 { + errorMsg.WriteString("insecure origin pattern ") + errorMsg.WriteByte('"') + errorMsg.WriteString(patterns[0]) + errorMsg.WriteString(`" requires `) + } else { + errorMsg.WriteString("insecure origin patterns ") + for i := 0; i < len(patterns)-1; i++ { + errorMsg.WriteByte('"') + errorMsg.WriteString(patterns[i]) + errorMsg.WriteString(`", `) + } + errorMsg.WriteByte('"') + errorMsg.WriteString(patterns[len(patterns)-1]) + errorMsg.WriteString(`" require `) + } + errorMsg.WriteString("option risky." + optSIOC + " when ") + if cfg.AllowCredentials { + errorMsg.WriteString("credentialed access is enabled") + } + if cfg.PrivateNetworkAccess || cfg.PrivateNetworkAccessInNoCorsModeOnly { + if cfg.AllowCredentials { + errorMsg.WriteString(" and/or ") + } + errorMsg.WriteString("Private Network Access is enabled") + } + err := util.NewError(errorMsg.String()) + errs = append(errs, err) } if cfg.tmp.PublicSuffixError != nil && !cfg.tmp.SkipPublicSuffixCheck { diff --git a/internal/options.go b/internal/options.go index 849c410..1746034 100644 --- a/internal/options.go +++ b/internal/options.go @@ -88,19 +88,18 @@ func NewMiddleware[A applier](cred bool, one A, others ...A) (Middleware, error) func FromOrigins(one string, others ...string) Option { var ( - setOfPatterns = make(util.Set[origin.Pattern]) - publicSuffixError error - insecureOriginPatternError error - nonWildcardOrigin string + setOfPatterns = make(util.Set[origin.Pattern]) + publicSuffixError error + insecureOriginPatterns []string + nonWildcardOrigin string ) processOnePattern := func(raw string) error { pattern, err := origin.ParsePattern(raw) if err != nil { return err } - if pattern.IsDeemedInsecure() && insecureOriginPatternError == nil { - const tmpl = "insecure origin pattern %q requires option risky." + optSIOC - insecureOriginPatternError = util.Errorf(tmpl, raw) + if pattern.IsDeemedInsecure() { + insecureOriginPatterns = append(insecureOriginPatterns, raw) } if pattern.Kind != origin.PatternKindSubdomains && nonWildcardOrigin == "" { nonWildcardOrigin = raw @@ -131,7 +130,7 @@ func FromOrigins(one string, others ...string) Option { errs = append(errs, err) } cfg.tmp.FromOriginsCalled = true - cfg.tmp.InsecureOriginPatternError = insecureOriginPatternError + cfg.tmp.InsecureOriginPatterns = insecureOriginPatterns cfg.tmp.PublicSuffixError = publicSuffixError if len(errs) != 0 { return errors.Join(errs...)