Modify default behavior of maxhops to truncate

README.md

@@ -17,15 +17,18 @@ files and with other plugins.
 ## Syntax
 ```Caddyfile
 realip [cidr] {
-    header name
-    from   cidr [cidr... ]
+    header  name
+    from    cidr [cidr... ]
+    maxhops hops
     strict
 }
 ```
 
 name is the name of the header containing the actual IP address. Default is  X-Forwarded-For.
 
-cidr is the address range of expected proxy servers. As a security measure, IP headers are only accepted from known proxy servers. Must be a valid cidr block notation. This may be specified multiple times.
+cidr is the address range of expected proxy servers. As a security measure, IP headers are only accepted from known proxy servers. Must be a valid CIDR block notation. This may be specified multiple times.
+
+hops is the number of proxy hops allowed. In strict mode, setting this will result in a 403 if it is exceeded. Otherwise it will truncate down to the maximum number of hops and continue processing as otherwise. This can be used in cases where the number of proxies out to the internet will be fixed, but either there are too many CIDR ranges to practically specify or they cannot be known ahead of time.
 
 strict, if specified, will reject requests from unkown proxy IPs with a 403 status. If not specified, it will simply leave the original IP in place.
 

module.go

@@ -2,10 +2,11 @@ package realip
 
 import (
 	"fmt"
-	"github.com/mholt/caddy/caddyhttp/httpserver"
 	"net"
 	"net/http"
 	"strings"
+
+	"github.com/mholt/caddy/caddyhttp/httpserver"
 )
 
 type module struct {
@@ -52,16 +53,21 @@ func (m *module) ServeHTTP(w http.ResponseWriter, req *http.Request) (int, error
 	if hVal := req.Header.Get(m.Header); hVal != "" {
 		//restore original host:port format
 		parts := strings.Split(hVal, ",")
-		if m.MaxHops != -1 && len(parts) > m.MaxHops {
-			return 403, fmt.Errorf("Too many forward addresses")
-		}
 		ip := net.ParseIP(parts[len(parts)-1])
 		if ip == nil {
 			if m.Strict {
 				return 403, fmt.Errorf("Unrecognized proxy ip address: %s", parts[len(parts)-1])
 			}
 			return m.next.ServeHTTP(w, req)
 		}
+		if m.MaxHops != -1 && len(parts) > m.MaxHops {
+			if m.Strict {
+				return 403, fmt.Errorf("Too many forward addresses")
+			} else {
+				// Chop off everything exceeding MaxHops, going from right to left
+				parts = parts[len(parts)-m.MaxHops : len(parts)]
+			}
+		}
 		req.RemoteAddr = net.JoinHostPort(parts[len(parts)-1], port)
 		for i := len(parts) - 1; i >= 0; i-- {
 			req.RemoteAddr = net.JoinHostPort(parts[i], port)

realip_test.go

@@ -8,6 +8,7 @@ import (
 
 	"bytes"
 	"fmt"
+
 	"github.com/mholt/caddy"
 	"github.com/mholt/caddy/caddyhttp/httpserver"
 )
@@ -30,9 +31,10 @@ func TestRealIP(t *testing.T) {
 
 		{"4.5.2.3:123", "1.2.6.7,5.6.7.8,4.5.6.7", "5.6.7.8:123"},
 
-		// expectedIP is empty because the server should have returned a 403
+		// expectedIP is 4.5.0.1 because of truncation
 		// since the chain is longer than the configured max (5)
-		{"4.5.2.3:123", "1.2.6.7,5.6.7.8,4.5.6.7,5.6.7.8,4.5.6.7,1.2.3.4", ""},
+		{"4.5.2.3:123", "1.2.6.7,4.5.0.1,4.5.4.5,4.5.6.7,4.5.8.9,4.5.10.11", "4.5.0.1:123"},
+		{"4.5.2.3:123", "1.2.6.7,4.5.4.5,4.5.6.7,4.5.8.9,4.5.10.11", "1.2.6.7:123"},
 	} {
 		remoteAddr := ""
 		_, ipnet, err := net.ParseCIDR("4.5.0.0/16") // "4.5.x.x"
@@ -68,6 +70,70 @@ func TestRealIP(t *testing.T) {
 	}
 }
 
+func TestStrictRealIP(t *testing.T) {
+	for i, test := range []struct {
+		actualIP   string
+		headerVal  string
+		expectedIP string
+	}{
+		// 1.2.3.4 is NOT in a trusted subnet, it will error
+		{"1.2.3.4:123", "", ""},
+		// 4.4.255.255 is NOT in a trusted subnet, it will error
+		{"4.4.255.255:123", "", ""},
+		{"4.5.0.0:123", "1.2.3.4", "1.2.3.4:123"},
+
+		// because 111.111.111.111 is NOT in a trusted subnet, it will error
+		{"4.5.2.3:123", "1.2.6.7,5.6.7.8,111.111.111.111", ""},
+		// because NOTANIP is not a recognized IP address, it will error
+		{"4.5.5.5:123", "NOTANIP", ""},
+		// because aaaaaa is not a recognized IP address, it will error
+		{"aaaaaa", "1.2.3.4", ""},
+		// because aaaaaa is not a recognized IP address, it will error
+		{"aaaaaa:123", "1.2.3.4", ""},
+
+		{"4.5.2.3:123", "1.2.6.7,4.5.6.7", "1.2.6.7:123"},
+		{"4.5.2.3:123", "1.2.6.7,5.6.7.8,4.5.6.7", ""},
+
+		// expectedIP is empty because the server should have returned a 403
+		// since the chain is longer than the configured max (5)
+		{"4.5.2.3:123", "1.2.6.7,4.5.0.1,4.5.4.5,4.5.6.7,4.5.8.9,4.5.10.11", ""},
+		{"4.5.2.3:123", "1.2.6.7,4.5.4.5,4.5.6.7,4.5.8.9,4.5.10.11", "1.2.6.7:123"},
+	} {
+		remoteAddr := ""
+		_, ipnet, err := net.ParseCIDR("4.5.0.0/16") // "4.5.x.x"
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		he := &module{
+			next: httpserver.HandlerFunc(func(w http.ResponseWriter, r *http.Request) (int, error) {
+				remoteAddr = r.RemoteAddr
+				return 0, nil
+			}),
+			Strict:  true,
+			Header:  "X-Forwarded-For",
+			MaxHops: 5,
+			From:    []*net.IPNet{ipnet},
+		}
+
+		req, err := http.NewRequest("GET", "http://foo.tld/", nil)
+		if err != nil {
+			t.Fatalf("Test %d: Could not create HTTP request: %v", i, err)
+		}
+		req.RemoteAddr = test.actualIP
+		if test.headerVal != "" {
+			req.Header.Set("X-Forwarded-For", test.headerVal)
+		}
+
+		rec := httptest.NewRecorder()
+		he.ServeHTTP(rec, req)
+
+		if remoteAddr != test.expectedIP {
+			t.Errorf("Test %d: Expected '%s', but found '%s'", i, test.expectedIP, remoteAddr)
+		}
+	}
+}
+
 func TestCidrAndPresets(t *testing.T) {
 	tests := []struct {
 		rule     string