relax the need for option TolerateInsecureOrigins

jub0bs · Feb 29, 2024 · a7d2fd1 · a7d2fd1
1 parent bc779c2
commit a7d2fd1
Show file tree

Hide file tree

Showing 4 changed files with 134 additions and 26 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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...)