Skip to content

Commit

Permalink
internal/envoy: switch to safe regex for path match
Browse files Browse the repository at this point in the history
Updates #1351

Switch path regex matching to safe regex. Refactor matcher.RegexMatcher
code to its own file and helper so it can be reused across
internal/envoy and internal/contour.

Also of note, internal/contour longestRouteFirst comparator has intimate
knowledge of RouteMatch_SafeRegex which is probably not ideal.

Signed-off-by: Dave Cheney <dave@cheney.net>
  • Loading branch information
davecheney authored and stevesloka committed Nov 22, 2019
1 parent a2cb6a0 commit 5c9497a
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 47 deletions.
14 changes: 5 additions & 9 deletions internal/contour/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,27 +255,23 @@ func (l longestRouteFirst) Less(i, j int) bool {
return true
case -1:
return false
case 0:
default:
return longestRouteByHeaders(l[i], l[j])
}

panic("bad compare")
}
case *envoy_api_v2_route.RouteMatch_Regex:
case *envoy_api_v2_route.RouteMatch_SafeRegex:
switch b := l[j].Match.PathSpecifier.(type) {
case *envoy_api_v2_route.RouteMatch_Regex:
cmp := strings.Compare(a.Regex, b.Regex)
case *envoy_api_v2_route.RouteMatch_SafeRegex:
cmp := strings.Compare(a.SafeRegex.Regex, b.SafeRegex.Regex)
switch cmp {
case 1:
// Sort longest regex first.
return true
case -1:
return false
case 0:
default:
return longestRouteByHeaders(l[i], l[j])
}

panic("bad compare")
case *envoy_api_v2_route.RouteMatch_Prefix:
return true
}
Expand Down
32 changes: 32 additions & 0 deletions internal/envoy/regex.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright © 2019 VMware
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package envoy

import (
matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher"
"github.com/projectcontour/contour/internal/protobuf"
)

// SafeRegexMatch retruns a matcher.RegexMatcher for the supplied regex.
// SafeRegexMatch does not escape regex meta characters.
func SafeRegexMatch(regex string) *matcher.RegexMatcher {
return &matcher.RegexMatcher{
EngineType: &matcher.RegexMatcher_GoogleRe2{
GoogleRe2: &matcher.RegexMatcher_GoogleRE2{
MaxProgramSize: protobuf.UInt32(uint32(len(regex))),
},
},
Regex: regex,
}
}
69 changes: 69 additions & 0 deletions internal/envoy/regex_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright © 2019 VMware
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package envoy

import (
"testing"

matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher"
"github.com/projectcontour/contour/internal/assert"
"github.com/projectcontour/contour/internal/protobuf"
)

func TestSafeRegexMatch(t *testing.T) {
tests := map[string]struct {
regex string
want *matcher.RegexMatcher
}{
"blank regex": {
regex: "",
want: &matcher.RegexMatcher{
EngineType: &matcher.RegexMatcher_GoogleRe2{
GoogleRe2: &matcher.RegexMatcher_GoogleRE2{
MaxProgramSize: protobuf.UInt32(0),
},
},
},
},
"simple": {
regex: "chrome",
want: &matcher.RegexMatcher{
EngineType: &matcher.RegexMatcher_GoogleRe2{
GoogleRe2: &matcher.RegexMatcher_GoogleRE2{
MaxProgramSize: protobuf.UInt32(6),
},
},
Regex: "chrome",
},
},
"regex meta": {
regex: "[a-z]+$",
want: &matcher.RegexMatcher{
EngineType: &matcher.RegexMatcher_GoogleRe2{
GoogleRe2: &matcher.RegexMatcher_GoogleRE2{
MaxProgramSize: protobuf.UInt32(7),
},
},
Regex: "[a-z]+$", // meta characters are not escaped.
},
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
got := SafeRegexMatch(tc.regex)
assert.Equal(t, tc.want, got)
})
}
}
15 changes: 4 additions & 11 deletions internal/envoy/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2"
envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
envoy_api_v2_route "github.com/envoyproxy/go-control-plane/envoy/api/v2/route"
matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher"
"github.com/golang/protobuf/ptypes/duration"
"github.com/projectcontour/contour/internal/dag"
"github.com/projectcontour/contour/internal/protobuf"
Expand Down Expand Up @@ -58,8 +57,8 @@ func RouteMatch(route *dag.Route) *envoy_api_v2_route.RouteMatch {
// RouteRegex returns a regex matcher.
func RouteRegex(regex string, headers ...dag.HeaderCondition) *envoy_api_v2_route.RouteMatch {
return &envoy_api_v2_route.RouteMatch{
PathSpecifier: &envoy_api_v2_route.RouteMatch_Regex{
Regex: regex,
PathSpecifier: &envoy_api_v2_route.RouteMatch_SafeRegex{
SafeRegex: SafeRegexMatch(regex),
},
Headers: headerMatcher(headers),
}
Expand Down Expand Up @@ -134,6 +133,7 @@ func mirrorPolicy(r *dag.Route) *envoy_api_v2_route.RouteAction_RequestMirrorPol
if r.MirrorPolicy == nil {
return nil
}

return &envoy_api_v2_route.RouteAction_RequestMirrorPolicy{
Cluster: Clustername(r.MirrorPolicy.Cluster),
}
Expand Down Expand Up @@ -309,13 +309,6 @@ func containsMatch(s string) *envoy_api_v2_route.HeaderMatcher_SafeRegexMatch {
regex := fmt.Sprintf(".*%s.*", regexp.QuoteMeta(s))

return &envoy_api_v2_route.HeaderMatcher_SafeRegexMatch{
SafeRegexMatch: &matcher.RegexMatcher{
EngineType: &matcher.RegexMatcher_GoogleRe2{
GoogleRe2: &matcher.RegexMatcher_GoogleRE2{
MaxProgramSize: protobuf.UInt32(uint32(len(regex))),
},
},
Regex: regex,
},
SafeRegexMatch: SafeRegexMatch(regex),
}
}
32 changes: 5 additions & 27 deletions internal/envoy/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2"
envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
envoy_api_v2_route "github.com/envoyproxy/go-control-plane/envoy/api/v2/route"
matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher"
"github.com/projectcontour/contour/internal/assert"
"github.com/projectcontour/contour/internal/dag"
"github.com/projectcontour/contour/internal/protobuf"
Expand Down Expand Up @@ -620,14 +619,7 @@ func TestRouteMatch(t *testing.T) {
Name: "x-header",
InvertMatch: false,
HeaderMatchSpecifier: &envoy_api_v2_route.HeaderMatcher_SafeRegexMatch{
SafeRegexMatch: &matcher.RegexMatcher{
EngineType: &matcher.RegexMatcher_GoogleRe2{
GoogleRe2: &matcher.RegexMatcher_GoogleRE2{
MaxProgramSize: protobuf.UInt32(15),
},
},
Regex: ".*11-22-33-44.*",
},
SafeRegexMatch: SafeRegexMatch(".*11-22-33-44.*"),
},
}},
},
Expand All @@ -646,14 +638,7 @@ func TestRouteMatch(t *testing.T) {
Name: "x-header",
InvertMatch: false,
HeaderMatchSpecifier: &envoy_api_v2_route.HeaderMatcher_SafeRegexMatch{
SafeRegexMatch: &matcher.RegexMatcher{
EngineType: &matcher.RegexMatcher_GoogleRe2{
GoogleRe2: &matcher.RegexMatcher_GoogleRE2{
MaxProgramSize: protobuf.UInt32(18),
},
},
Regex: ".*11\\.22\\.33\\.44.*",
},
SafeRegexMatch: SafeRegexMatch(".*11\\.22\\.33\\.44.*"),
},
}},
},
Expand All @@ -672,14 +657,7 @@ func TestRouteMatch(t *testing.T) {
Name: "x-header",
InvertMatch: false,
HeaderMatchSpecifier: &envoy_api_v2_route.HeaderMatcher_SafeRegexMatch{
SafeRegexMatch: &matcher.RegexMatcher{
EngineType: &matcher.RegexMatcher_GoogleRe2{
GoogleRe2: &matcher.RegexMatcher_GoogleRE2{
MaxProgramSize: protobuf.UInt32(24),
},
},
Regex: ".*11\\.\\[22\\]\\.\\*33\\.44.*",
},
SafeRegexMatch: SafeRegexMatch(".*11\\.\\[22\\]\\.\\*33\\.44.*"),
},
}},
},
Expand All @@ -703,11 +681,11 @@ func TestRouteMatch(t *testing.T) {
},
},
want: &envoy_api_v2_route.RouteMatch{
PathSpecifier: &envoy_api_v2_route.RouteMatch_Regex{
PathSpecifier: &envoy_api_v2_route.RouteMatch_SafeRegex{
// note, unlike header conditions this is not a quoted regex because
// the value comes directly from the Ingress.Paths.Path value which
// is permitted to be a bare regex.
Regex: "/v.1/*",
SafeRegex: SafeRegexMatch("/v.1/*"),
},
},
},
Expand Down

0 comments on commit 5c9497a

Please sign in to comment.