From 8c2a72ad07578ceba74ce04a9cdc45f1c6e2faeb Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Wed, 7 Feb 2024 10:09:29 -0500 Subject: [PATCH] matchers: Drop `forwarded` option from `remote_ip` matcher (#6085) --- modules/caddyhttp/celmatcher_test.go | 16 --------------- modules/caddyhttp/ip_matchers.go | 30 +++------------------------- 2 files changed, 3 insertions(+), 43 deletions(-) diff --git a/modules/caddyhttp/celmatcher_test.go b/modules/caddyhttp/celmatcher_test.go index 3604562b..e67b87c7 100644 --- a/modules/caddyhttp/celmatcher_test.go +++ b/modules/caddyhttp/celmatcher_test.go @@ -373,22 +373,6 @@ eqp31wM9il1n+guTNyxJd+FzVAH+hCZE5K+tCgVDdVFUlDEHHbS/wqb2PSIoouLV urlTarget: "https://example.com/foo", wantResult: true, }, - { - name: "remote_ip forwarded (MatchRemoteIP)", - expression: &MatchExpression{ - Expr: `remote_ip('forwarded', '192.0.2.1')`, - }, - urlTarget: "https://example.com/foo", - wantResult: true, - }, - { - name: "remote_ip forwarded not first (MatchRemoteIP)", - expression: &MatchExpression{ - Expr: `remote_ip('192.0.2.1', 'forwarded')`, - }, - urlTarget: "https://example.com/foo", - wantErr: true, - }, } ) diff --git a/modules/caddyhttp/ip_matchers.go b/modules/caddyhttp/ip_matchers.go index 30be6120..d5571802 100644 --- a/modules/caddyhttp/ip_matchers.go +++ b/modules/caddyhttp/ip_matchers.go @@ -37,13 +37,6 @@ type MatchRemoteIP struct { // The IPs or CIDR ranges to match. Ranges []string `json:"ranges,omitempty"` - // If true, prefer the first IP in the request's X-Forwarded-For - // header, if present, rather than the immediate peer's IP, as - // the reference IP against which to match. Note that it is easy - // to spoof request headers. Default: false - // DEPRECATED: This is insecure, MatchClientIP should be used instead. - Forwarded bool `json:"forwarded,omitempty"` - // cidrs and zones vars should aligned always in the same // length and indexes for matching later cidrs []*netip.Prefix @@ -82,11 +75,7 @@ func (m *MatchRemoteIP) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { d.Next() // consume matcher name for d.NextArg() { if d.Val() == "forwarded" { - if len(m.Ranges) > 0 { - return d.Err("if used, 'forwarded' must be first argument") - } - m.Forwarded = true - continue + return d.Err("the 'forwarded' option is no longer supported; use the 'client_ip' matcher instead") } if d.Val() == "private_ranges" { m.Ranges = append(m.Ranges, PrivateRangesCIDR()...) @@ -105,7 +94,7 @@ func (m *MatchRemoteIP) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // // Example: // -// expression remote_ip('forwarded', '192.168.0.0/16', '172.16.0.0/12', '10.0.0.0/8') +// expression remote_ip('192.168.0.0/16', '172.16.0.0/12', '10.0.0.0/8') func (MatchRemoteIP) CELLibrary(ctx caddy.Context) (cel.Library, error) { return CELMatcherImpl( // name of the macro, this is the function name that users see when writing expressions. @@ -126,11 +115,7 @@ func (MatchRemoteIP) CELLibrary(ctx caddy.Context) (cel.Library, error) { for _, input := range strList.([]string) { if input == "forwarded" { - if len(m.Ranges) > 0 { - return nil, errors.New("if used, 'forwarded' must be first argument") - } - m.Forwarded = true - continue + return nil, errors.New("the 'forwarded' option is no longer supported; use the 'client_ip' matcher instead") } m.Ranges = append(m.Ranges, input) } @@ -151,21 +136,12 @@ func (m *MatchRemoteIP) Provision(ctx caddy.Context) error { m.cidrs = cidrs m.zones = zones - if m.Forwarded { - m.logger.Warn("remote_ip's forwarded mode is deprecated; use the 'client_ip' matcher instead") - } - return nil } // Match returns true if r matches m. func (m MatchRemoteIP) Match(r *http.Request) bool { address := r.RemoteAddr - if m.Forwarded { - if fwdFor := r.Header.Get("X-Forwarded-For"); fwdFor != "" { - address = strings.TrimSpace(strings.Split(fwdFor, ",")[0]) - } - } clientIP, zoneID, err := parseIPZoneFromString(address) if err != nil { m.logger.Error("getting remote IP", zap.Error(err))