config: fix policy matching for regular expressions (#2966)

* config: fix policy matching for regular expressions

* compile regex in validate, add test

* fix test

config/options_test.go

@@ -17,7 +17,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var cmpOptIgnoreUnexported = cmpopts.IgnoreUnexported(Options{})
+var cmpOptIgnoreUnexported = cmpopts.IgnoreUnexported(Options{}, Policy{})
 
 func Test_Validate(t *testing.T) {
 	t.Parallel()

config/policy.go

@@ -44,9 +44,10 @@ type Policy struct {
 	Source *StringURL `yaml:",omitempty" json:"source,omitempty" hash:"ignore"`
 
 	// Additional route matching options
-	Prefix string `mapstructure:"prefix" yaml:"prefix,omitempty" json:"prefix,omitempty"`
+	Prefix        string `mapstructure:"prefix" yaml:"prefix,omitempty" json:"prefix,omitempty"`
-	Path   string `mapstructure:"path" yaml:"path,omitempty" json:"path,omitempty"`
+	Path          string `mapstructure:"path" yaml:"path,omitempty" json:"path,omitempty"`
-	Regex  string `mapstructure:"regex" yaml:"regex,omitempty" json:"regex,omitempty"`
+	Regex         string `mapstructure:"regex" yaml:"regex,omitempty" json:"regex,omitempty"`
+	compiledRegex *regexp.Regexp
 
 	// Path Rewrite Options
 	PrefixRewrite            string `mapstructure:"prefix_rewrite" yaml:"prefix_rewrite,omitempty" json:"prefix_rewrite,omitempty"`
@@ -496,6 +497,17 @@ func (p *Policy) Validate() error {
 		return fmt.Errorf("config: only prefix_rewrite or regex_rewrite_pattern can be specified, but not both")
 	}
 
+	if p.Regex != "" {
+		rawRE := p.Regex
+		if !strings.HasPrefix(rawRE, "^") {
+			rawRE = "^" + rawRE
+		}
+		if !strings.HasSuffix(rawRE, "$") {
+			rawRE += "$"
+		}
+		p.compiledRegex, _ = regexp.Compile(rawRE)
+	}
+
 	return nil
 }
 
@@ -564,9 +576,8 @@ func (p *Policy) Matches(requestURL url.URL) bool {
 		}
 	}
 
-	if p.Regex != "" {
+	if p.compiledRegex != nil {
-		re, err := regexp.Compile(p.Regex)
+		if !p.compiledRegex.MatchString(requestURL.Path) {
-		if err == nil && !re.MatchString(requestURL.String()) {
 			return false
 		}
 	}

config/policy_test.go

@@ -6,10 +6,12 @@ import (
 	"testing"
 
 	envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
-	"github.com/golang/protobuf/proto"
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
+
+	"github.com/pomerium/pomerium/internal/urlutil"
 )
 
 func Test_PolicyValidate(t *testing.T) {
@@ -233,3 +235,38 @@ func TestPolicy_FromToPb(t *testing.T) {
 		assert.Equal(t, p.Redirect.HTTPSRedirect, policyFromProto.Redirect.HTTPSRedirect)
 	})
 }
+
+func TestPolicy_Matches(t *testing.T) {
+	t.Run("full", func(t *testing.T) {
+		p := &Policy{
+			From:  "https://www.example.com",
+			To:    mustParseWeightedURLs(t, "https://localhost"),
+			Regex: `/foo`,
+		}
+		assert.NoError(t, p.Validate())
+
+		assert.False(t, p.Matches(urlutil.MustParseAndValidateURL(`https://www.example.com/foo/bar`)),
+			"regex should only match full string")
+	})
+	t.Run("issue2952", func(t *testing.T) {
+		p := &Policy{
+			From:  "https://www.example.com",
+			To:    mustParseWeightedURLs(t, "https://localhost"),
+			Regex: `^\/foo\/bar\/[0-9a-f]\/{0,1}$`,
+		}
+		assert.NoError(t, p.Validate())
+
+		assert.True(t, p.Matches(urlutil.MustParseAndValidateURL(`https://www.example.com/foo/bar/0`)))
+	})
+	t.Run("issue2592-test2", func(t *testing.T) {
+		p := &Policy{
+			From:  "https://www.example.com",
+			To:    mustParseWeightedURLs(t, "https://localhost"),
+			Regex: `/admin/.*`,
+		}
+		assert.NoError(t, p.Validate())
+
+		assert.True(t, p.Matches(urlutil.MustParseAndValidateURL(`https://www.example.com/admin/foo`)))
+		assert.True(t, p.Matches(urlutil.MustParseAndValidateURL(`https://www.example.com/admin/bar`)))
+	})
+}

internal/urlutil/url.go

@@ -50,6 +50,16 @@ func ParseAndValidateURL(rawurl string) (*url.URL, error) {
 	return u, nil
 }
 
+// MustParseAndValidateURL parses the URL via ParseAndValidateURL but panics if there is an error.
+// (useful for testing)
+func MustParseAndValidateURL(rawURL string) url.URL {
+	u, err := ParseAndValidateURL(rawURL)
+	if err != nil {
+		panic(err)
+	}
+	return *u
+}
+
 // ValidateURL wraps standard library's default url.Parse because
 // it's much more lenient about what type of urls it accepts than pomerium.
 func ValidateURL(u *url.URL) error {