feat: ✨ Add caddy client_ip variable parsing (#184)

* feat: ✨ Add caddy client_ip variable parsing

* fix: 🐛 panic when client_ip ctx var is not set

* feat: ✅ separate to an utils function and add tests

* fix: 🚨 linting

* feat: ✅ Add integration test

* fix: 🚨 linting

* fix: 💡 Add comment about trusted_proxies in Caddyfile

---------

Co-authored-by: Juan Pablo Tosso <[email protected]>

coraza_test.go

@@ -109,6 +109,27 @@ func TestPostMultipart(t *testing.T) {
 	time.Sleep(1 * time.Second)
 }
 
+func TestClientIpRule(t *testing.T) {
+	tester, err := newTester("test.init.config", t)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// client_ip will be 127.0.0.1
+	req, _ := http.NewRequest("GET", baseURL+"/", nil)
+	tester.AssertResponseCode(req, 200)
+
+	time.Sleep(1 * time.Second)
+
+	// client_ip will be 127.0.0.2
+	req, _ = http.NewRequest("GET", baseURL+"/", nil)
+	req.Header.Add("X-Forwarded-For", "127.0.0.2")
+	tester.AssertResponseCode(req, 403)
+
+	time.Sleep(1 * time.Second)
+
+}
+
 func multipartRequest(req *http.Request) error {
 	var b bytes.Buffer
 	w := multipart.NewWriter(&b)

http.go

@@ -8,25 +8,15 @@ import (
 	"io"
 	"net"
 	"net/http"
-	"strconv"
-	"strings"
 
 	"github.com/corazawaf/coraza/v3/types"
 )
 
 // Copied from https://github.com/corazawaf/coraza/blob/main/http/middleware.go
 
 func processRequest(tx types.Transaction, req *http.Request) (*types.Interruption, error) {
-	var (
-		client string
-		cport  int
-	)
-	// IMPORTANT: Some http.Request.RemoteAddr implementations will not contain port or contain IPV6: [2001:db8::1]:8080
-	idx := strings.LastIndexByte(req.RemoteAddr, ':')
-	if idx != -1 {
-		client = req.RemoteAddr[:idx]
-		cport, _ = strconv.Atoi(req.RemoteAddr[idx+1:])
-	}
+
+	client, cport := getClientAddress(req)
 
 	var in *types.Interruption
 	// There is no socket access in the request object, so we neither know the server client nor port.

test.init.config

@@ -3,6 +3,11 @@
     debug
     auto_https off
     order coraza_waf first
+    servers {
+        # Enable trusted_proxies to make Caddy populate client_ip variable from X-Forwarded-For header, in the case Caddy is behind a reverse proxy
+        # See : https://caddyserver.com/docs/caddyfile/options#trusted-proxies
+        trusted_proxies static private_ranges
+    }
 }
 
 :8080 {
@@ -11,6 +16,7 @@
 			SecAction "id:149,pass,log, msg:'Some test msg',logdata:'logdata'"
 			SecRule REQUEST_URI "test5" "id:2, deny, log, phase:1,status:403"
 			SecRule REQUEST_URI "test6" "id:4, deny, log, phase:3,status:403"
+			SecRule REMOTE_ADDR "@ipMatch 127.0.0.2" "id:5, deny, log, phase:1,status:403"
 			Include testdata/sample1.conf
 			Include testdata/sample2.conf
 		`

utils.go

@@ -5,9 +5,14 @@ package coraza
 
 import (
 	"math/rand"
+	"net"
+	"net/http"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
+
+	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
 )
 
 const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
@@ -45,3 +50,33 @@ func randomString(n int) string {
 
 	return sb.String()
 }
+
+func getClientAddress(req *http.Request) (string, int) {
+
+	var (
+		clientIp   string
+		clientPort int
+	)
+
+	if address, ok := caddyhttp.GetVar(req.Context(), caddyhttp.ClientIPVarKey).(string); ok && len(address) > 0 {
+		ip, port, _ := net.SplitHostPort(address)
+		if ip != "" {
+			clientIp = ip
+		} else {
+			clientIp = address
+		}
+		clientPort, _ = strconv.Atoi(port)
+	} else {
+		idx := strings.LastIndexByte(req.RemoteAddr, ':')
+		if idx != -1 {
+			clientIp = req.RemoteAddr[:idx]
+			clientPort, _ = strconv.Atoi(req.RemoteAddr[idx+1:])
+		} else {
+			clientIp = req.RemoteAddr
+			clientPort = 0
+		}
+	}
+
+	return clientIp, clientPort
+
+}

utils_test.go

@@ -0,0 +1,51 @@
+// Copyright 2023 The OWASP Coraza contributors
+// SPDX-License-Identifier: Apache-2.0
+
+package coraza
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"testing"
+
+	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParsegClientAddress(t *testing.T) {
+
+	remoteIp := "127.0.0.1"
+	remotePort := 9090
+	clientIp := "127.0.0.2"
+	clientPort := 8080
+
+	req, _ := http.NewRequest("GET", "/", nil)
+
+	req.RemoteAddr = fmt.Sprintf("%v:%v", remoteIp, remotePort)
+	ip, port := getClientAddress(req)
+	require.Equal(t, remoteIp, ip)
+	require.Equal(t, remotePort, port)
+
+	req.RemoteAddr = remoteIp
+	ip, port = getClientAddress(req)
+	require.Equal(t, remoteIp, ip)
+	require.Equal(t, 0, port)
+
+	req = req.WithContext(context.WithValue(req.Context(), caddyhttp.VarsCtxKey, make(map[string]any)))
+	req.RemoteAddr = fmt.Sprintf("%v:%v", remoteIp, remotePort)
+
+	ip, port = getClientAddress(req)
+	require.Equal(t, remoteIp, ip)
+	require.Equal(t, remotePort, port)
+
+	caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, fmt.Sprintf("%v:%v", clientIp, clientPort))
+	ip, port = getClientAddress(req)
+	require.Equal(t, clientIp, ip)
+	require.Equal(t, clientPort, port)
+
+	caddyhttp.SetVar(req.Context(), caddyhttp.ClientIPVarKey, clientIp)
+	ip, port = getClientAddress(req)
+	require.Equal(t, clientIp, ip)
+	require.Equal(t, 0, port)
+}