diff --git a/.github/config.php b/.github/config.php index f337cc3..44af390 100644 --- a/.github/config.php +++ b/.github/config.php @@ -8,8 +8,23 @@ $timeout = 5; -// Test-only: lets the suite exercise IP-forward-header handling via the X-Test-Ip-Forward-Header -// request header. Gated on the local test-server URL so a stray copy to production is inert. -if (strpos($MATOMO_URL, '/tests/server/') !== false && !empty($_SERVER['HTTP_X_TEST_IP_FORWARD_HEADER'])) { +// Test-only request headers (below) let the suite exercise config variations per-request. +// Gated on the local test-server URL so a stray copy to production is inert. +$isTestServer = strpos($MATOMO_URL, '/tests/server/') !== false; + +// Exercise IP-forward-header handling. +if ($isTestServer && !empty($_SERVER['HTTP_X_TEST_IP_FORWARD_HEADER'])) { $http_ip_forward_header = $_SERVER['HTTP_X_TEST_IP_FORWARD_HEADER']; } + +// Exercise cookie-allowlist filtering (comma-separated entries; empty value = explicit empty allowlist). +if ($isTestServer && isset($_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST'])) { + $COOKIE_ALLOWLIST = $_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST'] === '' + ? array() + : explode(',', $_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST']); +} + +// Exercise misconfiguration handling (a non-array $COOKIE_ALLOWLIST). +if ($isTestServer && isset($_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST_INVALID'])) { + $COOKIE_ALLOWLIST = $_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST_INVALID']; +} diff --git a/README.md b/README.md index f4f3b5b..11969f1 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,20 @@ Because the proxy sits between your visitors and Matomo, it has to tell Matomo t > ⚠️ **Breaking change:** previously `$http_ip_forward_header` was sent *in addition* to `cip`+`token_auth`; the proxy now treats it as the *sole* IP mechanism and injects nothing else. If you already set it, make sure Matomo's trusted-proxy configuration above is in place — otherwise leave it empty to keep using `cip`. +### Cookie forwarding + +By default, the proxy forwards the visitor's entire `Cookie` header to Matomo unchanged. If your site also sets other cookies (session, consent-management, A/B testing, etc.) alongside Matomo's, those are forwarded too. + +To restrict this, set `$COOKIE_ALLOWLIST` in `config.php` to an array of cookie names. Each entry is matched either as an **exact name** (e.g. `mtm_consent`) or, if it ends with `*`, as a **prefix** (e.g. `_pk_id*` matches both `_pk_id` and `_pk_id.1.1fff`). Prefix entries are needed for Matomo's default id/session/referrer/custom-variable cookies, since the JavaScript tracker appends a per-site/per-domain suffix to their base name. Cookies matching no entry are stripped before the request reaches Matomo. Matching is case-sensitive; a prefix match is a plain string prefix, not delimiter-aware (so `_pk_id*` could in theory also match an unrelated cookie sharing that prefix, though unlikely in practice). + +> Note: leaving `$COOKIE_ALLOWLIST` unset keeps today's behavior of forwarding all cookies unchanged. Setting it — even to an empty array — switches the proxy into allowlist mode; an empty array forwards no cookies at all. A bare `*` or empty-string entry is a no-op (matches nothing), not "allow everything", so a stray `array('*')` can't defeat the allowlist. + +> ⚠️ **Always keep `matomo_ignore`** (or whatever cookie your Matomo's opt-out/consent setup relies on) in the allowlist. Dropping it silently re-enables tracking for visitors who opted out, with no visible error. + +> ⚠️ **Check your Matomo tracker config before copying the example list below.** A custom `setCookieNamePrefix()` replaces `_pk_*` with your own prefix, and enabled plugins (e.g. HeatmapSessionRecording) or third-party tracking code may set additional cookies not covered by the example. +> +> A dropped tracking cookie doesn't error — Matomo just stops recognizing returning visitors, silently inflating visit counts. After configuring this, load a tracked page twice and confirm Matomo logs one visit, not two. + ### Auth-protected tracking parameters Some tracking parameters (`cip`, `cdt`, `cdo`, `country`, `region`, `city`, `lat`, `long`) are only honored by Matomo for an authenticated request. The proxy never lends its `$TOKEN_AUTH` to a request — or to an individual entry of a bulk request — that carries one of these override parameters or its own `token_auth`: @@ -153,11 +167,26 @@ $PROXY_URL = 'http://localhost/'; $TOKEN_AUTH = 'xyz'; $timeout = 5; -// Test-only: lets the suite exercise IP-forward-header handling via the X-Test-Ip-Forward-Header -// request header. Gated on the local test-server URL so a stray copy to production is inert. -if (strpos($MATOMO_URL, '/tests/server/') !== false && !empty($_SERVER['HTTP_X_TEST_IP_FORWARD_HEADER'])) { +// Test-only request headers (below) let the suite exercise config variations per-request. +// Gated on the local test-server URL so a stray copy to production is inert. +$isTestServer = strpos($MATOMO_URL, '/tests/server/') !== false; + +// Exercise IP-forward-header handling. +if ($isTestServer && !empty($_SERVER['HTTP_X_TEST_IP_FORWARD_HEADER'])) { $http_ip_forward_header = $_SERVER['HTTP_X_TEST_IP_FORWARD_HEADER']; } + +// Exercise cookie-allowlist filtering (comma-separated entries; empty value = explicit empty allowlist). +if ($isTestServer && isset($_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST'])) { + $COOKIE_ALLOWLIST = $_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST'] === '' + ? array() + : explode(',', $_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST']); +} + +// Exercise misconfiguration handling (a non-array $COOKIE_ALLOWLIST). +if ($isTestServer && isset($_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST_INVALID'])) { + $COOKIE_ALLOWLIST = $_SERVER['HTTP_X_TEST_COOKIE_ALLOWLIST_INVALID']; +} ``` The tests need a webserver to be pointed to the root of this repository. The simplest way is to just use Vagrant: diff --git a/config.php.example b/config.php.example index dab8fda..2e21430 100644 --- a/config.php.example +++ b/config.php.example @@ -49,3 +49,25 @@ $user_agent = ''; // for nginx see https://www.nginx.com/resources/wiki/start/topics/examples/forwarded/ // $http_ip_forward_header = ''; + +// By default, the proxy forwards the visitor's entire Cookie header to Matomo unchanged, which +// also forwards unrelated site cookies (session, consent tools, A/B testing, etc.). +// +// Set $COOKIE_ALLOWLIST to restrict what's forwarded. Each entry matches either an exact name +// (e.g. 'mtm_consent') or, if it ends with '*', a prefix (e.g. '_pk_id*' matches '_pk_id.1.1fff'). +// Prefix entries are needed for Matomo's id/session/referrer/custom-variable cookies, since the +// JS tracker appends a per-site/per-domain suffix to their base name. Matching is case-sensitive. +// +// Typical Matomo cookie names/prefixes: '_pk_id*', '_pk_ses*', '_pk_ref*', '_pk_cvar*', '_pk_hsr*', +// 'mtm_consent', 'mtm_consent_removed', 'matomo_ignore'. Check your tracker config first: a custom +// setCookieNamePrefix() replaces '_pk_*' entirely, and enabled plugins may set cookies of their own. +// +// Leaving this unset (the default) forwards all cookies, for backward compatibility. Setting it to +// an empty array, or to a non-array value by mistake, forwards NO cookies (fails closed). A bare +// '*' or empty-string entry is a no-op, not "allow everything", so a typo can't defeat the allowlist. +// +// Always keep 'matomo_ignore' (or your opt-out cookie) in this list - dropping it silently +// re-enables tracking for opted-out visitors. A dropped tracking cookie otherwise just stops +// Matomo recognizing returning visitors, inflating visit counts; verify by loading a page twice. +// +// $COOKIE_ALLOWLIST = array('_pk_id*', '_pk_ses*', '_pk_ref*', '_pk_cvar*', '_pk_hsr*', 'mtm_consent', 'mtm_consent_removed', 'matomo_ignore'); diff --git a/proxy.php b/proxy.php index 9ffbe15..ceba137 100644 --- a/proxy.php +++ b/proxy.php @@ -278,12 +278,58 @@ function handleHeaderLine($curl, $headerLine) return $originalByteCount; } +function cookieNameIsAllowed($name, $allowlist) +{ + foreach ($allowlist as $entry) { + $entry = trim($entry); + if ($entry === '' || $entry === '*') { + // No-op: guards against a typo silently allowing every cookie through. + continue; + } + + // Trailing '*' = prefix match; otherwise exact match only. + if (substr($entry, -1) === '*') { + $prefix = substr($entry, 0, -1); + if (strncmp($name, $prefix, strlen($prefix)) === 0) { + return true; + } + } elseif ($name === $entry) { + return true; + } + } + + return false; +} + +function filterCookieHeader($cookieHeaderValue, $allowlist) +{ + $keptPairs = array(); + + foreach (explode(';', $cookieHeaderValue) as $segment) { + $segment = trim($segment); + if ($segment === '') { + continue; + } + + $parts = explode('=', $segment, 2); + $name = trim($parts[0]); + $value = isset($parts[1]) ? trim($parts[1]) : ''; + + if (cookieNameIsAllowed($name, $allowlist)) { + $keptPairs[] = $name . '=' . $value; + } + } + + return implode('; ', $keptPairs); +} + function getHttpContentAndStatus($url, $timeout, $user_agent, $postBody = '') { global $httpResponseHeaders; global $DEBUG_PROXY; global $NO_VERIFY_SSL; global $http_ip_forward_header; + global $COOKIE_ALLOWLIST; $useFopen = @ini_get('allow_url_fopen') == '1'; @@ -300,7 +346,20 @@ function getHttpContentAndStatus($url, $timeout, $user_agent, $postBody = '') } if (isset($_SERVER['HTTP_COOKIE'])) { - $header[] = "Cookie: " . $_SERVER['HTTP_COOKIE']; + $cookieHeaderValue = $_SERVER['HTTP_COOKIE']; + if (isset($COOKIE_ALLOWLIST)) { + if (!is_array($COOKIE_ALLOWLIST)) { + // Fail closed: a misconfigured allowlist must not silently forward everything + // unfiltered. error_log(), not trigger_error(), so this can never leak into the + // response body when display_errors is on. + error_log('$COOKIE_ALLOWLIST must be an array; treating it as empty (no cookies forwarded) until fixed.'); + $COOKIE_ALLOWLIST = array(); + } + $cookieHeaderValue = filterCookieHeader($cookieHeaderValue, $COOKIE_ALLOWLIST); + } + if ($cookieHeaderValue !== '') { + $header[] = "Cookie: " . $cookieHeaderValue; + } } $stream_options = array( diff --git a/tests/ProxyTest.php b/tests/ProxyTest.php index 88b7713..b66e91a 100644 --- a/tests/ProxyTest.php +++ b/tests/ProxyTest.php @@ -893,6 +893,289 @@ public function test_proxy_works_with_ipv6_in_header() $this->assertEquals($expected, $responseBody); } + public function test_cookie_header_forwarded_unchanged_by_default() + { + $response = $this->send('foo=bar', null, null, ['Cookie' => 'a=1; _pk_id.1.abc=qux']); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +array ( + 'COOKIE' => 'a=1; _pk_id.1.abc=qux', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_cookie_allowlist_filters_to_prefix_match() + { + $headers = [ + 'Cookie' => 'a=1; _pk_id.1.abc=qux; other=2', + 'X-Test-Cookie-Allowlist' => '_pk_id*', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +array ( + 'COOKIE' => '_pk_id.1.abc=qux', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_cookie_allowlist_exact_entry_does_not_match_longer_name() + { + $headers = [ + 'Cookie' => 'mtm_consent=1; mtm_consent_removed=1', + 'X-Test-Cookie-Allowlist' => 'mtm_consent', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +array ( + 'COOKIE' => 'mtm_consent=1', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_empty_cookie_allowlist_drops_all_cookies() + { + $headers = [ + 'Cookie' => 'a=1; _pk_id.1.abc=qux', + 'X-Test-Cookie-Allowlist' => '', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_cookie_allowlist_is_case_sensitive() + { + $headers = [ + 'Cookie' => '_PK_ID.1.abc=xyz', + 'X-Test-Cookie-Allowlist' => '_pk_id*', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_cookie_value_with_equals_sign_is_preserved() + { + $headers = [ + 'Cookie' => '_pk_id.1.abc=YWJjZA==', + 'X-Test-Cookie-Allowlist' => '_pk_id*', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +array ( + 'COOKIE' => '_pk_id.1.abc=YWJjZA==', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_no_cookie_header_sent_with_allowlist_configured() + { + $headers = ['X-Test-Cookie-Allowlist' => '_pk_id*']; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_cookie_allowlist_wildcard_entry_is_treated_as_noop_not_match_all() + { + $headers = [ + 'Cookie' => 'a=1; _pk_id.1.abc=qux', + 'X-Test-Cookie-Allowlist' => '*', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + // '*' alone is a no-op, not "allow everything" - all cookies are dropped. + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_cookie_allowlist_empty_entry_among_real_entries_is_noop() + { + $headers = [ + 'Cookie' => 'mtm_consent=1; other=2', + // Leading comma produces an empty entry alongside a real one. + 'X-Test-Cookie-Allowlist' => ',mtm_consent', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + // The empty entry is a no-op; 'mtm_consent' still matches normally. + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +array ( + 'COOKIE' => 'mtm_consent=1', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_cookie_allowlist_with_multiple_mixed_entries() + { + $headers = [ + 'Cookie' => '_pk_id.1.abc=qux; mtm_consent=1; mtm_consent_removed=1; other=2', + 'X-Test-Cookie-Allowlist' => '_pk_id*,mtm_consent', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + // Prefix ('_pk_id*') and exact ('mtm_consent') entries combined; only matching cookies survive. + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +array ( + 'COOKIE' => '_pk_id.1.abc=qux; mtm_consent=1', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_cookie_allowlist_with_whitespace_around_equals_still_matches() + { + $headers = [ + 'Cookie' => '_pk_id.1.abc = qux', + 'X-Test-Cookie-Allowlist' => '_pk_id*', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +array ( + 'COOKIE' => '_pk_id.1.abc=qux', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + + public function test_non_array_cookie_allowlist_fails_closed_and_drops_all_cookies() + { + $headers = [ + 'Cookie' => 'a=1; _pk_id.1.abc=qux', + 'X-Test-Cookie-Allowlist-Invalid' => '_pk_id*', + ]; + $response = $this->send('foo=bar', null, null, $headers); + + $responseBody = $this->getBody($response); + + // Non-array allowlist fails closed: no cookies forwarded, not silently forwarding everything. + $expected = << '127.0.0.1', + 'token_auth' => '', + 'foo' => 'bar', +) +RESPONSE; + + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals($expected, $responseBody); + } + private function send($query = null, DateTime $modifiedSince = null, $matomoUrl = null, $addHeaders = null, $path = null, $method = 'GET', $body = null, $forceIpV6 = false) { diff --git a/tests/server/matomo.php b/tests/server/matomo.php index 18918b1..853d186 100644 --- a/tests/server/matomo.php +++ b/tests/server/matomo.php @@ -23,7 +23,7 @@ } $headers = array(); -foreach (array('DNT', 'X_DO_NOT_TRACK', 'X_FORWARDED_FOR') as $headerName) { +foreach (array('DNT', 'X_DO_NOT_TRACK', 'X_FORWARDED_FOR', 'COOKIE') as $headerName) { if (isset($_SERVER['HTTP_' . $headerName])) { $headers[$headerName] = $_SERVER['HTTP_' . $headerName]; }