From a51d08a090fde14ed8058dd5bc464bb22a8cd1e6 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 27 Sep 2024 06:05:59 -0400 Subject: [PATCH 1/4] caddyhttp: Escaping placeholders in CEL --- modules/caddyhttp/celmatcher.go | 15 +++++++++++++-- modules/caddyhttp/celmatcher_test.go | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/modules/caddyhttp/celmatcher.go b/modules/caddyhttp/celmatcher.go index a5565eb9840..fe956846844 100644 --- a/modules/caddyhttp/celmatcher.go +++ b/modules/caddyhttp/celmatcher.go @@ -126,6 +126,10 @@ func (m *MatchExpression) Provision(ctx caddy.Context) error { // light (and possibly naïve) syntactic sugar m.expandedExpr = placeholderRegexp.ReplaceAllString(m.Expr, placeholderExpansion) + // as a second pass, we'll strip the escape character from an escaped + // placeholder, so that it can be used as an input to other CEL functions + m.expandedExpr = escapedPlaceholderRegexp.ReplaceAllString(m.expandedExpr, escapedPlaceholderExpansion) + // our type adapter expands CEL's standard type support m.ta = celTypeAdapter{} @@ -701,8 +705,15 @@ func isCELStringListLiteral(e ast.Expr) bool { // expressions with a proper CEL function call; this is // just for syntactic sugar. var ( - placeholderRegexp = regexp.MustCompile(`{([a-zA-Z][\w.-]+)}`) - placeholderExpansion = `caddyPlaceholder(request, "${1}")` + // The placeholder may not be preceded by a backslash; the expansion + // will include the preceding character if it is not a backslash. + placeholderRegexp = regexp.MustCompile(`([^\\]|^){([a-zA-Z][\w.-]+)}`) + placeholderExpansion = `${1}caddyPlaceholder(request, "${2}")` + + // As a second pass, we need to strip the escape character in front of + // the placeholder, if it exists. + escapedPlaceholderRegexp = regexp.MustCompile(`\\{([a-zA-Z][\w.-]+)}`) + escapedPlaceholderExpansion = `{${1}}` CELTypeJSON = cel.MapType(cel.StringType, cel.DynType) ) diff --git a/modules/caddyhttp/celmatcher_test.go b/modules/caddyhttp/celmatcher_test.go index 25fdcf45e66..aa9e55e4b25 100644 --- a/modules/caddyhttp/celmatcher_test.go +++ b/modules/caddyhttp/celmatcher_test.go @@ -69,6 +69,24 @@ eqp31wM9il1n+guTNyxJd+FzVAH+hCZE5K+tCgVDdVFUlDEHHbS/wqb2PSIoouLV httpHeader: &http.Header{"Field": []string{"foo", "bar"}}, wantResult: true, }, + { + name: "header matches an escaped placeholder value (MatchHeader)", + expression: &MatchExpression{ + Expr: `header({'Field': '\\\{foobar}'})`, + }, + urlTarget: "https://example.com/foo", + httpHeader: &http.Header{"Field": []string{"{foobar}"}}, + wantResult: true, + }, + { + name: "header matches an placeholder replaced during the header matcher (MatchHeader)", + expression: &MatchExpression{ + Expr: `header({'Field': '\{http.request.uri.path}'})`, + }, + urlTarget: "https://example.com/foo", + httpHeader: &http.Header{"Field": []string{"/foo"}}, + wantResult: true, + }, { name: "header error (MatchHeader)", expression: &MatchExpression{ From 8ec8d26b12557916e2148da0ecf2e8b098abbfd9 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 27 Sep 2024 06:24:25 -0400 Subject: [PATCH 2/4] Simplify some of the test cases --- modules/caddyhttp/celmatcher_test.go | 48 ++++++++++++---------------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/modules/caddyhttp/celmatcher_test.go b/modules/caddyhttp/celmatcher_test.go index aa9e55e4b25..ecb32830cc4 100644 --- a/modules/caddyhttp/celmatcher_test.go +++ b/modules/caddyhttp/celmatcher_test.go @@ -88,13 +88,18 @@ eqp31wM9il1n+guTNyxJd+FzVAH+hCZE5K+tCgVDdVFUlDEHHbS/wqb2PSIoouLV wantResult: true, }, { - name: "header error (MatchHeader)", + name: "header error, invalid escape sequence (MatchHeader)", + expression: &MatchExpression{ + Expr: `header({'Field': '\\{foobar}'})`, + }, + wantErr: true, + }, + { + name: "header error, needs to be JSON syntax with field as key (MatchHeader)", expression: &MatchExpression{ Expr: `header('foo')`, }, - urlTarget: "https://example.com/foo", - httpHeader: &http.Header{"Field": []string{"foo", "bar"}}, - wantErr: true, + wantErr: true, }, { name: "header_regexp matches (MatchHeaderRE)", @@ -128,9 +133,7 @@ eqp31wM9il1n+guTNyxJd+FzVAH+hCZE5K+tCgVDdVFUlDEHHbS/wqb2PSIoouLV expression: &MatchExpression{ Expr: `header_regexp('foo')`, }, - urlTarget: "https://example.com/foo", - httpHeader: &http.Header{"Field": []string{"foo", "bar"}}, - wantErr: true, + wantErr: true, }, { name: "host matches localhost (MatchHost)", @@ -161,8 +164,7 @@ eqp31wM9il1n+guTNyxJd+FzVAH+hCZE5K+tCgVDdVFUlDEHHbS/wqb2PSIoouLV expression: &MatchExpression{ Expr: `host(80)`, }, - urlTarget: "http://localhost:80", - wantErr: true, + wantErr: true, }, { name: "method does not match (MatchMethod)", @@ -187,9 +189,7 @@ eqp31wM9il1n+guTNyxJd+FzVAH+hCZE5K+tCgVDdVFUlDEHHbS/wqb2PSIoouLV expression: &MatchExpression{ Expr: `method()`, }, - urlTarget: "https://foo.example.com", - httpMethod: "PUT", - wantErr: true, + wantErr: true, }, { name: "path matches substring (MatchPath)", @@ -284,24 +284,21 @@ eqp31wM9il1n+guTNyxJd+FzVAH+hCZE5K+tCgVDdVFUlDEHHbS/wqb2PSIoouLV expression: &MatchExpression{ Expr: `protocol()`, }, - urlTarget: "https://example.com", - wantErr: true, + wantErr: true, }, { name: "protocol invocation error too many args (MatchProtocol)", expression: &MatchExpression{ Expr: `protocol('grpc', 'https')`, }, - urlTarget: "https://example.com", - wantErr: true, + wantErr: true, }, { name: "protocol invocation error wrong arg type (MatchProtocol)", expression: &MatchExpression{ Expr: `protocol(true)`, }, - urlTarget: "https://example.com", - wantErr: true, + wantErr: true, }, { name: "query does not match against a specific value (MatchQuery)", @@ -348,40 +345,35 @@ eqp31wM9il1n+guTNyxJd+FzVAH+hCZE5K+tCgVDdVFUlDEHHbS/wqb2PSIoouLV expression: &MatchExpression{ Expr: `query({1: "1"})`, }, - urlTarget: "https://example.com/foo", - wantErr: true, + wantErr: true, }, { name: "query error typed struct instead of map (MatchQuery)", expression: &MatchExpression{ Expr: `query(Message{field: "1"})`, }, - urlTarget: "https://example.com/foo", - wantErr: true, + wantErr: true, }, { name: "query error bad map value type (MatchQuery)", expression: &MatchExpression{ Expr: `query({"debug": 1})`, }, - urlTarget: "https://example.com/foo/?debug=1", - wantErr: true, + wantErr: true, }, { name: "query error no args (MatchQuery)", expression: &MatchExpression{ Expr: `query()`, }, - urlTarget: "https://example.com/foo/?debug=1", - wantErr: true, + wantErr: true, }, { name: "remote_ip error no args (MatchRemoteIP)", expression: &MatchExpression{ Expr: `remote_ip()`, }, - urlTarget: "https://example.com/foo", - wantErr: true, + wantErr: true, }, { name: "remote_ip single IP match (MatchRemoteIP)", From e1ddf96b741f77bc52ff56b6a449b22c8f8747fd Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 27 Sep 2024 07:39:21 -0400 Subject: [PATCH 3/4] Implement vars and vars_regexp in CEL --- modules/caddyhttp/celmatcher_test.go | 67 +++++++++++++++++++++ modules/caddyhttp/matchers.go | 4 +- modules/caddyhttp/vars.go | 89 ++++++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 2 deletions(-) diff --git a/modules/caddyhttp/celmatcher_test.go b/modules/caddyhttp/celmatcher_test.go index ecb32830cc4..26491b7ca3f 100644 --- a/modules/caddyhttp/celmatcher_test.go +++ b/modules/caddyhttp/celmatcher_test.go @@ -383,6 +383,67 @@ eqp31wM9il1n+guTNyxJd+FzVAH+hCZE5K+tCgVDdVFUlDEHHbS/wqb2PSIoouLV urlTarget: "https://example.com/foo", wantResult: true, }, + { + name: "vars value (VarsMatcher)", + expression: &MatchExpression{ + Expr: `vars({'foo': 'bar'})`, + }, + urlTarget: "https://example.com/foo", + wantResult: true, + }, + { + name: "vars matches placeholder, needs escape (VarsMatcher)", + expression: &MatchExpression{ + Expr: `vars({'\{http.request.uri.path}': '/foo'})`, + }, + urlTarget: "https://example.com/foo", + wantResult: true, + }, + { + name: "vars error wrong syntax (VarsMatcher)", + expression: &MatchExpression{ + Expr: `vars('foo', 'bar')`, + }, + wantErr: true, + }, + { + name: "vars error no args (VarsMatcher)", + expression: &MatchExpression{ + Expr: `vars()`, + }, + wantErr: true, + }, + { + name: "vars_regexp value (MatchVarsRE)", + expression: &MatchExpression{ + Expr: `vars_regexp('foo', 'ba?r')`, + }, + urlTarget: "https://example.com/foo", + wantResult: true, + }, + { + name: "vars_regexp value with name (MatchVarsRE)", + expression: &MatchExpression{ + Expr: `vars_regexp('name', 'foo', 'ba?r')`, + }, + urlTarget: "https://example.com/foo", + wantResult: true, + }, + { + name: "vars_regexp matches placeholder, needs escape (MatchVarsRE)", + expression: &MatchExpression{ + Expr: `vars_regexp('\{http.request.uri.path}', '/fo?o')`, + }, + urlTarget: "https://example.com/foo", + wantResult: true, + }, + { + name: "vars_regexp error no args (MatchVarsRE)", + expression: &MatchExpression{ + Expr: `vars_regexp()`, + }, + wantErr: true, + }, } ) @@ -406,6 +467,9 @@ func TestMatchExpressionMatch(t *testing.T) { } repl := caddy.NewReplacer() ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl) + ctx = context.WithValue(ctx, VarsCtxKey, map[string]any{ + "foo": "bar", + }) req = req.WithContext(ctx) addHTTPVarsToReplacer(repl, req, httptest.NewRecorder()) @@ -446,6 +510,9 @@ func BenchmarkMatchExpressionMatch(b *testing.B) { } repl := caddy.NewReplacer() ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl) + ctx = context.WithValue(ctx, VarsCtxKey, map[string]any{ + "foo": "bar", + }) req = req.WithContext(ctx) addHTTPVarsToReplacer(repl, req, httptest.NewRecorder()) if tc.clientCertificate != nil { diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go index a0bc6d63a61..93a36237bd1 100644 --- a/modules/caddyhttp/matchers.go +++ b/modules/caddyhttp/matchers.go @@ -1562,8 +1562,8 @@ var ( _ CELLibraryProducer = (*MatchHeader)(nil) _ CELLibraryProducer = (*MatchHeaderRE)(nil) _ CELLibraryProducer = (*MatchProtocol)(nil) - // _ CELLibraryProducer = (*VarsMatcher)(nil) - // _ CELLibraryProducer = (*MatchVarsRE)(nil) + _ CELLibraryProducer = (*VarsMatcher)(nil) + _ CELLibraryProducer = (*MatchVarsRE)(nil) _ json.Marshaler = (*MatchNot)(nil) _ json.Unmarshaler = (*MatchNot)(nil) diff --git a/modules/caddyhttp/vars.go b/modules/caddyhttp/vars.go index 9e86dd7164a..77e06e3cbed 100644 --- a/modules/caddyhttp/vars.go +++ b/modules/caddyhttp/vars.go @@ -18,8 +18,12 @@ import ( "context" "fmt" "net/http" + "reflect" "strings" + "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/types/ref" + "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" ) @@ -203,6 +207,28 @@ func (m VarsMatcher) Match(r *http.Request) bool { return false } +// CELLibrary produces options that expose this matcher for use in CEL +// expression matchers. +// +// Example: +// +// expression vars({'{magic_number}': ['3', '5']}) +// expression vars({'{foo}': 'single_value'}) +func (VarsMatcher) CELLibrary(_ caddy.Context) (cel.Library, error) { + return CELMatcherImpl( + "vars", + "vars_matcher_request_map", + []*cel.Type{CELTypeJSON}, + func(data ref.Val) (RequestMatcher, error) { + mapStrListStr, err := CELValueToMapStrList(data) + if err != nil { + return nil, err + } + return VarsMatcher(mapStrListStr), nil + }, + ) +} + // MatchVarsRE matches the value of the context variables by a given regular expression. // // Upon a match, it adds placeholders to the request: `{http.regexp.name.capture_group}` @@ -302,6 +328,69 @@ func (m MatchVarsRE) Match(r *http.Request) bool { return false } +// CELLibrary produces options that expose this matcher for use in CEL +// expression matchers. +// +// Example: +// +// expression vars_regexp('foo', '{magic_number}', '[0-9]+') +// expression vars_regexp('{magic_number}', '[0-9]+') +func (MatchVarsRE) CELLibrary(ctx caddy.Context) (cel.Library, error) { + unnamedPattern, err := CELMatcherImpl( + "vars_regexp", + "vars_regexp_request_string_string", + []*cel.Type{cel.StringType, cel.StringType}, + func(data ref.Val) (RequestMatcher, error) { + refStringList := reflect.TypeOf([]string{}) + params, err := data.ConvertToNative(refStringList) + if err != nil { + return nil, err + } + strParams := params.([]string) + matcher := MatchVarsRE{} + matcher[strParams[0]] = &MatchRegexp{ + Pattern: strParams[1], + Name: ctx.Value(MatcherNameCtxKey).(string), + } + err = matcher.Provision(ctx) + return matcher, err + }, + ) + if err != nil { + return nil, err + } + namedPattern, err := CELMatcherImpl( + "vars_regexp", + "vars_regexp_request_string_string_string", + []*cel.Type{cel.StringType, cel.StringType, cel.StringType}, + func(data ref.Val) (RequestMatcher, error) { + refStringList := reflect.TypeOf([]string{}) + params, err := data.ConvertToNative(refStringList) + if err != nil { + return nil, err + } + strParams := params.([]string) + name := strParams[0] + if name == "" { + name = ctx.Value(MatcherNameCtxKey).(string) + } + matcher := MatchVarsRE{} + matcher[strParams[1]] = &MatchRegexp{ + Pattern: strParams[2], + Name: name, + } + err = matcher.Provision(ctx) + return matcher, err + }, + ) + if err != nil { + return nil, err + } + envOpts := append(unnamedPattern.CompileOptions(), namedPattern.CompileOptions()...) + prgOpts := append(unnamedPattern.ProgramOptions(), namedPattern.ProgramOptions()...) + return NewMatcherCELLibrary(envOpts, prgOpts), nil +} + // Validate validates m's regular expressions. func (m MatchVarsRE) Validate() error { for _, rm := range m { From 1f4ab6c0c1161cf6326bc149602f348d5ba9b677 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Fri, 27 Sep 2024 07:45:29 -0400 Subject: [PATCH 4/4] dupl lint is dumb --- .golangci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index e8b2a55d3f5..aecff563eed 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -171,6 +171,12 @@ issues: - path: modules/logging/filters.go linters: - dupl + - path: modules/caddyhttp/matchers.go + linters: + - dupl + - path: modules/caddyhttp/vars.go + linters: + - dupl - path: _test\.go linters: - errcheck