IBX-11181: Filtered client-supplied X-Forwarded-* headers in Varnish VCL

.github/workflows/backend-ci.yaml

@@ -57,3 +57,20 @@ jobs:
 
             -   name: Run test suite
                 run: composer run-script --timeout=600 test
+
+    varnish-vcl-tests:
+        name: Varnish VCL tests
+        runs-on: "ubuntu-24.04"
+        timeout-minutes: 20
+
+        steps:
+            -   uses: actions/checkout@v6
+
+            # Builds varnish-modules from source, as the xkey vmod is not packaged for Varnish 6.0LTS.
+            -   name: Build varnishtest image
+                run: docker build -t ibexa-varnishtest:6.0 tests/varnish
+
+            -   name: Run varnishtest suite
+                env:
+                    IMAGE: ibexa-varnishtest:6.0
+                run: tests/varnish/run.sh

resources/upsun/common/5.0/.platform/varnish.vcl

@@ -9,7 +9,7 @@
 //import std;
 import xkey;
 
-// Includes not available on Platform.sh, so inlining parameters.vlc:
+// Includes not available on Platform.sh, so inlining parameters.vcl:
 acl invalidators {
     "127.0.0.1";
     "192.168.0.0"/16;
@@ -32,6 +32,27 @@ sub vcl_recv {
     // Add a Surrogate-Capability header to announce ESI support.
     set req.http.Surrogate-Capability = "abc=ESI/1.0";
 
+    // Stop clients from injecting reverse proxy headers, as the application trusts them once
+    // framework.trusted_proxies is configured.
+    //
+    // The Ibexa Cloud router is authoritative for X-Forwarded-Proto, X-Client-IP and Client-Cdn,
+    // and discards the values a client sends for them, so those are kept as is.
+    // See https://fixed.docs.upsun.com/development/headers.html
+    //
+    // The headers below are not filtered by the router, so strip them here:
+    unset req.http.Forwarded;
+    unset req.http.X-Forwarded-Host;
+    unset req.http.X-Forwarded-Prefix;
+
+    // X-Forwarded-For is only appended to by the router, so any leading entry is what the client
+    // sent. X-Client-IP is always set by the router, and holds the end-user IP also when the
+    // request came in via a CDN, so use that as the only value.
+    if (req.http.X-Client-IP) {
+        set req.http.X-Forwarded-For = req.http.X-Client-IP;
+    } else {
+        unset req.http.X-Forwarded-For;
+    }
+
     // Ensure that the Symfony Router generates URLs correctly with Varnish
     if (req.http.X-Forwarded-Proto == "https" ) {
         set req.http.X-Forwarded-Port = "443";

tests/varnish/Dockerfile

@@ -0,0 +1,44 @@
+# Image for running the varnishtest suite in tests/varnish/.
+#
+# Mirrors doc/docker/Dockerfile-varnish from ibexa/docker. It has to be Varnish 6.0LTS specifically:
+# this VCL returns "miss" from vcl_hit, which 6.5 and later reject. The xkey vmod is not packaged
+# for 6.0LTS, so varnish-modules is built from source, as in the image this mirrors.
+
+FROM debian:bullseye-slim
+
+ENV DEBIAN_FRONTEND=noninteractive
+
+ARG PACKAGECLOUD_URL=https://packagecloud.io/install/repositories/varnishcache/varnish60lts/script.deb.sh
+ARG VARNISH_MODULES_VERSION=0.15.0
+
+RUN set -xe \
+        && buildDeps=" \
+            make \
+            automake \
+            autotools-dev \
+            libedit-dev \
+            libjemalloc-dev \
+            libncurses-dev \
+            libpcre3-dev \
+            libtool \
+            pkg-config \
+            python3-docutils \
+            varnish-dev=6.0.12-1~bullseye \
+        " \
+        && apt-get update -q -y \
+        && apt-get install -q -y --no-install-recommends ca-certificates curl \
+        \
+        && curl -s ${PACKAGECLOUD_URL} | bash \
+        && apt-get install -q -y --allow-unauthenticated --no-install-recommends varnish=6.0.12-1~bullseye $buildDeps \
+        \
+        && curl -A "Docker" -o /tmp/varnish-modules.tar.gz -D - -L -s https://github.com/varnish/varnish-modules/archive/refs/tags/${VARNISH_MODULES_VERSION}.tar.gz \
+        && tar zxpf /tmp/varnish-modules.tar.gz -C /tmp/ \
+        && cd /tmp/varnish-modules-${VARNISH_MODULES_VERSION} \
+        && ./bootstrap \
+        && ./configure \
+        && make \
+        && make install \
+        && rm -f /tmp/varnish-modules.tar.gz && rm -Rf /tmp/varnish-modules-${VARNISH_MODULES_VERSION} \
+        \
+        && apt-get purge -y --auto-remove -o APT::AutoRemove::RecommendsImportant=false $buildDeps \
+        && rm -rf /var/lib/apt/lists/*

tests/varnish/README.md

@@ -0,0 +1,41 @@
+Varnish VCL tests
+=================
+
+`varnishtest` cases for `resources/upsun/common/5.0/.platform/varnish.vcl`, the VCL shipped to
+Upsun / Ibexa Cloud installations.
+
+They load the real VCL file into a real Varnish and assert on the request the **backend** receives,
+which is where the reverse proxy header filtering in `vcl_recv` can be observed.
+
+Cases
+-----
+
+| Case | Scenario | Asserts |
+|---|---|---|
+| `no-cdn.vtc` | request straight through the Ibexa Cloud router | the client supplied part of `X-Forwarded-For` is dropped and the header is rebuilt from `X-Client-IP`; `X-Forwarded-Host` / `-Prefix` / `Forwarded` are stripped; `X-Forwarded-Proto` is kept |
+| `via-cdn.vtc` | request arriving through a supported CDN | `X-Forwarded-For`, `X-Client-IP` and `Client-Cdn` are kept as the router left them, so Fastly detection still works |
+| `no-client-ip.vtc` | no `X-Client-IP`, so the request did not come through the router | `X-Forwarded-For` is dropped entirely rather than trusted |
+
+The header behaviour being relied on is documented at
+<https://fixed.docs.upsun.com/development/headers.html>.
+
+Running them
+------------
+
+```bash
+docker build -t ibexa-varnishtest:6.0 tests/varnish
+IMAGE=ibexa-varnishtest:6.0 tests/varnish/run.sh
+```
+
+Notes
+-----
+
+- The image has to be **Varnish 6.0LTS specifically**. This VCL returns `miss` from `vcl_hit`, which
+  6.5 and later reject, and the xkey vmod it imports is not packaged for 6.0LTS — hence the source
+  build in `Dockerfile`, mirroring `doc/docker/Dockerfile-varnish` in `ibexa/docker`.
+- The VCL cannot be loaded as it stands: Platform.sh supplies the VCL version declaration, the `std`
+  import and the `app.backend()` director. `run.sh` prepends the first two and swaps the director for
+  a stub, so the shipped file itself is what gets tested.
+- The backend listens on a fixed port (9081) so the stub can point at it.
+- Each case uses its own URL and its own Varnish instance: `X-Forwarded-*` is not part of the cache
+  key, so cases sharing a cache would be served each other's responses.

tests/varnish/no-cdn.vtc

@@ -0,0 +1,38 @@
+varnishtest "Without a CDN, X-Forwarded-For is rebuilt from the router supplied X-Client-IP"
+
+# The Ibexa Cloud router only *appends* the real client IP to whatever the client sent in
+# X-Forwarded-For, so every leading entry is client controlled and must not survive.
+# See https://fixed.docs.upsun.com/development/headers.html
+
+server s1 -listen "127.0.0.1:9081" {
+    rxreq
+
+    # Client supplied prefix dropped: only the router's own value is left.
+    expect req.http.X-Forwarded-For == "203.0.113.9"
+
+    # Not filtered by the router, so the VCL strips them.
+    expect req.http.X-Forwarded-Host   == <undef>
+    expect req.http.X-Forwarded-Prefix == <undef>
+    expect req.http.Forwarded          == <undef>
+
+    # Router authoritative, kept as is.
+    expect req.http.X-Client-IP       == "203.0.113.9"
+    expect req.http.X-Forwarded-Proto == "https"
+    expect req.http.X-Forwarded-Port  == "443"
+
+    txresp
+} -start
+
+varnish v1 -arg "-f /etc/varnish/default.vcl" -start
+
+client c1 {
+    txreq -url "/no-cdn" \
+        -hdr "X-Forwarded-For: 6.6.6.6, 203.0.113.9" \
+        -hdr "X-Client-IP: 203.0.113.9" \
+        -hdr "X-Forwarded-Proto: https" \
+        -hdr "X-Forwarded-Host: evil.example" \
+        -hdr "X-Forwarded-Prefix: /admin" \
+        -hdr "Forwarded: for=6.6.6.6;host=evil.example;proto=https"
+    rxresp
+    expect resp.status == 200
+} -run

tests/varnish/no-client-ip.vtc

@@ -0,0 +1,29 @@
+varnishtest "Without X-Client-IP there is nothing trustworthy, so X-Forwarded-For is dropped"
+
+# X-Client-IP is always set by the Ibexa Cloud router. If it is absent the request did not come
+# through the router, so the client supplied X-Forwarded-For must not be handed to the application.
+
+server s1 -listen "127.0.0.1:9081" {
+    rxreq
+
+    expect req.http.X-Forwarded-For == <undef>
+    expect req.http.X-Client-IP     == <undef>
+
+    expect req.http.X-Forwarded-Host   == <undef>
+    expect req.http.X-Forwarded-Prefix == <undef>
+    expect req.http.Forwarded          == <undef>
+
+    txresp
+} -start
+
+varnish v1 -arg "-f /etc/varnish/default.vcl" -start
+
+client c1 {
+    txreq -url "/no-client-ip" \
+        -hdr "X-Forwarded-For: 6.6.6.6" \
+        -hdr "X-Forwarded-Host: evil.example" \
+        -hdr "X-Forwarded-Prefix: /admin" \
+        -hdr "Forwarded: for=6.6.6.6;host=evil.example"
+    rxresp
+    expect resp.status == 200
+} -run

tests/varnish/run.sh

@@ -0,0 +1,42 @@
+#!/bin/bash
+#
+# Runs the varnishtest suite for the Platform.sh / Ibexa Cloud VCL.
+#
+# That VCL is not loadable as it stands: Platform.sh supplies the VCL version declaration, the std
+# import and the app.backend() director. This script prepends the first two and swaps the director
+# for a stub pointing at the test backend, then loads the resulting file into a real Varnish, so
+# what is tested is the shipped file rather than a copy of it.
+#
+# Usage:
+#   docker build -t ibexa-varnishtest:7 tests/varnish
+#   IMAGE=ibexa-varnishtest:7 tests/varnish/run.sh
+
+set -euo pipefail
+
+TESTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+ROOTDIR="$(cd "$TESTDIR/../.." && pwd)"
+VCL="${VCL:-$ROOTDIR/resources/upsun/common/5.0/.platform/varnish.vcl}"
+IMAGE="${IMAGE:-ibexa-varnishtest:7}"
+
+WORKDIR="$(mktemp -d)"
+trap 'rm -rf "$WORKDIR"' EXIT
+
+{
+    echo "// Prologue supplied by Platform.sh at runtime, added here so the file can be loaded."
+    echo "vcl 4.1;"
+    echo "import std;"
+    echo "backend stub { .host = \"127.0.0.1\"; .port = \"9081\"; }"
+    sed 's/app\.backend()/stub/' "$VCL"
+} > "$WORKDIR/default.vcl"
+
+echo "==> $(basename "$(dirname "$(dirname "$VCL")")")/$(basename "$VCL") (${IMAGE})"
+
+for vtc in "$TESTDIR"/*.vtc; do
+    echo "--> $(basename "$vtc")"
+    docker run --rm \
+        -v "$WORKDIR/default.vcl:/etc/varnish/default.vcl:ro" \
+        -v "$vtc:/case.vtc:ro" \
+        --entrypoint varnishtest "$IMAGE" /case.vtc
+done
+
+echo "All varnishtest cases passed."

tests/varnish/via-cdn.vtc

@@ -0,0 +1,36 @@
+varnishtest "Behind a supported CDN, the router supplied end client IP and Client-Cdn are kept"
+
+# When the request arrives via a CDN the router replaces X-Forwarded-For with the end client IP and
+# sets Client-Cdn, discarding whatever the client sent for either. Ibexa DXP uses Client-Cdn to
+# detect Fastly, so it has to reach the application.
+# See https://fixed.docs.upsun.com/development/headers.html
+
+server s1 -listen "127.0.0.1:9081" {
+    rxreq
+
+    expect req.http.X-Forwarded-For == "198.51.100.7"
+    expect req.http.X-Client-IP     == "198.51.100.7"
+    expect req.http.Client-Cdn      == "fastly"
+
+    expect req.http.X-Forwarded-Host   == <undef>
+    expect req.http.X-Forwarded-Prefix == <undef>
+    expect req.http.Forwarded          == <undef>
+
+    expect req.http.X-Forwarded-Proto == "https"
+    expect req.http.X-Forwarded-Port  == "443"
+
+    txresp
+} -start
+
+varnish v1 -arg "-f /etc/varnish/default.vcl" -start
+
+client c1 {
+    txreq -url "/via-cdn" \
+        -hdr "X-Forwarded-For: 198.51.100.7" \
+        -hdr "X-Client-IP: 198.51.100.7" \
+        -hdr "Client-Cdn: fastly" \
+        -hdr "X-Forwarded-Proto: https" \
+        -hdr "X-Forwarded-Host: evil.example"
+    rxresp
+    expect resp.status == 200
+} -run