1.16: Fix/enforce regex inutils (#9735) (#9763)

* Fix/enforce regex inutils (#9735)

* regexutils: Add poc for how we can validate regex, does not include all locations just for cors

* cors: trailing }

* cors: testing with invalid regex

* docs: update transforms to actually work. Note routes already use regex compile and transforms use envoy validation

* changelogs

* changelogs

* Adding changelog file to new location

* Deleting changelog file from old location

* plugins/cors/test: add bad RE2

* projects/pkg/cors: regex

* regexutils: naming update

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: changelog-bot <changelog-bot>

* changelogs

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>

changelog/v1.16.18/cors-regex-safety.yaml

@@ -0,0 +1,7 @@
+changelog:
+  - type: FIX
+    issueLink: https://github.com/solo-io/gloo/issues/7524
+    resolvesIssue: false
+    description: >-
+      Fix regex safety check for CORS allowed origins. 
+      Other instances in gloo already checked the regex either through compile or envoy validate.

docs/content/guides/traffic_management/request_processing/transformations/_index.md

@@ -51,7 +51,7 @@ virtualHost:
       regular:
         requestTransforms:
         - matcher:
-            - prefix: '/parent'
+            prefix: '/parent'
           requestTransformation:
             transformationTemplate:
               headers:

pkg/utils/regexutils/regex.go

@@ -2,6 +2,7 @@ package regexutils
 
 import (
 	"context"
+	"regexp"
 
 	envoy_type_matcher_v3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
 	wrappers "github.com/golang/protobuf/ptypes/wrappers"
@@ -11,22 +12,49 @@ import (
 	"github.com/solo-io/solo-kit/pkg/errors"
 )
 
-func NewRegex(ctx context.Context, regex string) *envoy_type_matcher_v3.RegexMatcher {
+// NewCheckedRegex creates a new regex matcher with the given regex.
+// It is tightly coupled to envoy's implementation of regex.
+func NewCheckedRegex(ctx context.Context, candidateRegex string) (*envoy_type_matcher_v3.RegexMatcher, error) {
+	if err := CheckRegexString(candidateRegex); err != nil {
+		return nil, err
+	}
+	return NewRegex(ctx, candidateRegex), nil
+}
+
+// CheckRegexString to make sure the string is a valid RE2 expression
+func CheckRegexString(candidateRegex string) error {
+	// https://github.com/envoyproxy/envoy/blob/v1.30.0/source/common/common/regex.cc#L19C8-L19C14
+	// Envoy uses the RE2 library for regex matching in google's owned c++ impl.
+	// go has https://pkg.go.dev/regexp which implements RE2 with a single caveat.
+	_, err := regexp.Compile(candidateRegex)
+	return err
+}
+
+// NewRegex creates a new regex matcher with the given regex.
+// It is tightly coupled to envoy's implementation of regex.
+// Wraps NewRegexFromSettings which wraps NewRegexWithProgramSize which leads to the tight coupling.
+// NOTE: Call this after having checked regex with CheckRegexString.
+func NewRegex(ctx context.Context, candidateRegex string) *envoy_type_matcher_v3.RegexMatcher {
 	settings := settingsutil.MaybeFromContext(ctx)
-	return NewRegexFromSettings(settings, regex)
+	return NewRegexFromSettings(settings, candidateRegex)
 }
 
-func NewRegexFromSettings(settings *v1.Settings, regex string) *envoy_type_matcher_v3.RegexMatcher {
+// NewRegexFromSettings wraps NewRegexWithProgramSize with the program size from the settings.
+// NOTE: Call this after having checked regex with CheckRegexString.
+func NewRegexFromSettings(settings *v1.Settings, candidateRegex string) *envoy_type_matcher_v3.RegexMatcher {
 	var programsize *uint32
 	if settings != nil {
 		if max_size := settings.GetGloo().GetRegexMaxProgramSize(); max_size != nil {
 			programsize = &max_size.Value
 		}
 	}
-	return NewRegexWithProgramSize(regex, programsize)
+	return NewRegexWithProgramSize(candidateRegex, programsize)
 }
 
-func NewRegexWithProgramSize(regex string, programsize *uint32) *envoy_type_matcher_v3.RegexMatcher {
+// NewRegexWithProgramSize creates a new regex matcher with the given program size.
+// This means its tightly coupled to envoy's implementation of regex.
+// NOTE: Call this after having checked regex with CheckRegexString.
+func NewRegexWithProgramSize(candidateRegex string, programsize *uint32) *envoy_type_matcher_v3.RegexMatcher {
 
 	var maxProgramSize *wrappers.UInt32Value
 	if programsize != nil {
@@ -39,7 +67,7 @@ func NewRegexWithProgramSize(regex string, programsize *uint32) *envoy_type_matc
 		EngineType: &envoy_type_matcher_v3.RegexMatcher_GoogleRe2{
 			GoogleRe2: &envoy_type_matcher_v3.RegexMatcher_GoogleRE2{MaxProgramSize: maxProgramSize},
 		},
-		Regex: regex,
+		Regex: candidateRegex,
 	}
 }
 

projects/gloo/pkg/plugins/cors/plugin.go

@@ -76,7 +76,7 @@ func (p *plugin) ProcessVirtualHost(
 	p.filterRequiredForListener[params.HttpListener] = struct{}{}
 	corsPolicy := &envoy_config_cors_v3.CorsPolicy{}
 	if err := p.translateCommonUserCorsConfig(params.Ctx, corsPlugin, corsPolicy); err != nil {
-		return err
+		return errors.Join(errors.New("invalid cors config"), err)
 	}
 
 	return pluginutils.SetVhostPerFilterConfig(out, wellknown.CORS, corsPolicy)
@@ -134,8 +134,12 @@ func (p *plugin) translateCommonUserCorsConfig(
 		})
 	}
 	for _, ao := range in.GetAllowOriginRegex() {
+		regexStruct, err := regexutils.NewCheckedRegex(ctx, ao)
+		if err != nil {
+			return err
+		}
 		out.AllowOriginStringMatch = append(out.GetAllowOriginStringMatch(), &envoy_type_matcher_v3.StringMatcher{
-			MatchPattern: &envoy_type_matcher_v3.StringMatcher_SafeRegex{SafeRegex: regexutils.NewRegex(ctx, ao)},
+			MatchPattern: &envoy_type_matcher_v3.StringMatcher_SafeRegex{SafeRegex: regexStruct},
 		})
 	}
 	out.AllowMethods = strings.Join(in.GetAllowMethods(), ",")

projects/gloo/pkg/plugins/cors/route_plugin_test.go

@@ -30,7 +30,8 @@ var _ = Describe("Route Plugin", func() {
 
 		// values used in first example
 		allowOrigin1      = []string{"solo.io", "github.com"}
-		allowOriginRegex1 = []string{`*\.solo\.io`, `git.*\.com`}
+		allowOriginRegex1 = []string{`.*\.solo\.io`, `git.*\.com`}
+		badOriginRegex1   = []string{`*\.solo\.io`, `git.*\.com`} // has a * at the front which is invalid
 		allowMethods1     = []string{"GET", "POST"}
 		allowHeaders1     = []string{"allowH1", "allow2"}
 		exposeHeaders1    = []string{"exHeader", "eh2"}
@@ -135,6 +136,14 @@ var _ = Describe("Route Plugin", func() {
 			Expect(outRoute.TypedPerFilterConfig).To(Equal(expected.TypedPerFilterConfig))
 
 		})
+		It("should reject  bad CORS", func() {
+			inRoute := routeWithCors(&cors.CorsPolicy{
+				AllowOriginRegex: badOriginRegex1,
+			})
+			outRoute := basicEnvoyRoute()
+			err := plugin.(plugins.RoutePlugin).ProcessRoute(params, inRoute, outRoute)
+			Expect(err).To(HaveOccurred(), "any regex starting with a * should not be RE2 compliant")
+		})
 		It("should process empty specification", func() {
 			inRoute := routeWithCors(&cors.CorsPolicy{})
 			outRoute := basicEnvoyRoute()

projects/gloo/pkg/plugins/cors/vhost_plugin_test.go

@@ -27,7 +27,8 @@ var _ = Describe("VirtualHost Plugin", func() {
 
 		// values used in first example
 		allowOrigin1      = []string{"solo.io", "github.com"}
-		allowOriginRegex1 = []string{`*\.solo\.io`, `git.*\.com`}
+		allowOriginRegex1 = []string{`.*\.solo\.io`, `git.*\.com`}
+		badOriginRegex1   = []string{`*\.solo\.io`, `git.*\.com`} // has a * at the front which is invalid
 		allowMethods1     = []string{"GET", "POST"}
 		allowHeaders1     = []string{"allowH1", "allow2"}
 		exposeHeaders1    = []string{"exHeader", "eh2"}
@@ -134,6 +135,18 @@ var _ = Describe("VirtualHost Plugin", func() {
 			}
 			Expect(out).To(Equal(envoy1min))
 		})
+		It("should reject bad CORS virtual hosts - minimal specification", func() {
+			out := &envoy_config_route_v3.VirtualHost{}
+			inRoute := &v1.VirtualHost{
+				Options: &v1.VirtualHostOptions{
+					Cors: &cors.CorsPolicy{
+						AllowOriginRegex: badOriginRegex1,
+					},
+				},
+			}
+			err := plugin.(plugins.VirtualHostPlugin).ProcessVirtualHost(params, inRoute, out)
+			Expect(err).To(HaveOccurred(), "any regex starting with a * should not be RE2 compliant")
+		})
 		It("should process virtual hosts - empty specification", func() {
 			out := &envoy_config_route_v3.VirtualHost{}
 			inRoute := &v1.VirtualHost{