fix(security): make session-cookie hardening work under TLS termination

The passive scan flagged 3x MEDIUM 'PHPSESSID missing Secure' and an
admin-login cookie missing HttpOnly. Root cause was not the cookie code
but scheme detection: every call site read $_SERVER['HTTPS'] directly,
which is unset when TLS is terminated in front of PHP-FPM, so the Secure
flag was silently dropped.

Add RequestUtil::isSecure()/baseUrl() as the single scheme detector:
  1. $_SERVER['HTTPS'] (direct TLS)
  2. X-Forwarded-Proto — only from a loopback/private peer, so a remote
     client cannot spoof it
  3. SERVER_PORT == 443
Replace all 8 inline $_SERVER['HTTPS'] idioms (App, AdminAuth, 4x filepond
endpoints, validate-access, admin/acces, SystemController self-ping).

nginx: forward the scheme to php-fpm in all 4 fastcgi blocks
(fastcgi_param HTTPS $https + X-Forwarded-Proto $scheme) and uncomment
server_tokens off. Config validated with nginx -t (real config, alpine
container).

AdminAuth: logout() now clears the session cookie with the same
path/domain/secure/httponly/samesite it was set with, instead of the
legacy positional setcookie() that dropped SameSite.

Tests (+24, 340 total):
  - RequestUtilTest: scheme detection incl. proxy trust boundaries
  - SessionCookieTest: logout emits a hardened deletion cookie
  - PeerTubeIntegrationTest: PeerTube keeps using its absolute instance_url
    and is unaffected by scheme changes (Guzzle MockHandler)
  - NginxConfigTest: pins HTTPS forwarding, HSTS includeSubDomains,
    server_tokens off

PeerTube uses an absolute instance_url from the DB and never derives URLs
from the request, so the integration is unchanged — the regression tests
lock that invariant in.
This commit is contained in:
Pontoporeia committed 2026-10-14 10:31:23 +02:00
1 parent e1f9cc9a9a
commit 554fc57455
17 files changed
+652 -29

No files matched your search

+138
View File
@@ -0,0 +1,138 @@
<?php
use PHPUnit\Framework\TestCase;
/**
* RequestUtilTest — scheme detection under direct TLS and reverse-proxy
* TLS termination.
*
* Regression context: production terminates TLS in front of PHP-FPM, so
* $_SERVER['HTTPS'] is unset. The previous inline idiom
* (empty($_SERVER['HTTPS']) && ... !== 'off') silently returned false and
* session cookies lost their Secure flag. These tests lock in the
* proxy-aware behaviour.
*/
class RequestUtilTest extends TestCase
{
public function testDirectTlsIsSecure(): void
{
$this->assertTrue(RequestUtil::isSecure(['HTTPS' => 'on']));
$this->assertTrue(RequestUtil::isSecure(['HTTPS' => '1']));
$this->assertTrue(RequestUtil::isSecure(['HTTPS' => 'https']));
$this->assertTrue(RequestUtil::isSecure(['HTTPS' => 'ON']));
}
public function testExplicitHttpsOffIsInsecure(): void
{
$this->assertFalse(RequestUtil::isSecure(['HTTPS' => 'off']));
$this->assertFalse(RequestUtil::isSecure(['HTTPS' => 'OFF']));
$this->assertFalse(RequestUtil::isSecure(['HTTPS' => '']));
}
public function testPlainHttpWithLoopbackPeerIsInsecure(): void
{
// No HTTPS var, no proxy header, port 80 → plain HTTP.
$this->assertFalse(RequestUtil::isSecure([
'REMOTE_ADDR' => '127.0.0.1',
'SERVER_PORT' => '80',
]));
}
public function testForwardedProtoHttpsFromTrustedProxyIsSecure(): void
{
// The production topology: nginx terminates TLS and forwards to
// PHP-FPM on loopback, setting X-Forwarded-Proto.
$this->assertTrue(RequestUtil::isSecure([
'REMOTE_ADDR' => '127.0.0.1',
'HTTP_X_FORWARDED_PROTO' => 'https',
]));
}
public function testForwardedProtoHttpsFromPrivateProxyIsSecure(): void
{
$this->assertTrue(RequestUtil::isSecure([
'REMOTE_ADDR' => '10.0.0.5',
'HTTP_X_FORWARDED_PROTO' => 'https',
]));
$this->assertTrue(RequestUtil::isSecure([
'REMOTE_ADDR' => '192.168.1.10',
'HTTP_X_FORWARDED_PROTO' => 'https',
]));
$this->assertTrue(RequestUtil::isSecure([
'REMOTE_ADDR' => '::1',
'HTTP_X_FORWARDED_PROTO' => 'https',
]));
}
public function testForwardedProtoIgnoredFromUntrustedPeer(): void
{
// A remote client sending the header over plain HTTP must NOT be
// treated as secure — otherwise the header is trivially spoofable.
$this->assertFalse(RequestUtil::isSecure([
'REMOTE_ADDR' => '203.0.113.42',
'HTTP_X_FORWARDED_PROTO' => 'https',
]));
}
public function testForwardedProtoListUsesFirstValue(): void
{
$this->assertTrue(RequestUtil::isSecure([
'REMOTE_ADDR' => '127.0.0.1',
'HTTP_X_FORWARDED_PROTO' => 'https, http',
]));
$this->assertFalse(RequestUtil::isSecure([
'REMOTE_ADDR' => '127.0.0.1',
'HTTP_X_FORWARDED_PROTO' => 'http, https',
]));
}
public function testForwardedProtoHttpValueIsInsecure(): void
{
$this->assertFalse(RequestUtil::isSecure([
'REMOTE_ADDR' => '127.0.0.1',
'HTTP_X_FORWARDED_PROTO' => 'http',
]));
}
public function testServerPortFallback(): void
{
$this->assertTrue(RequestUtil::isSecure(['SERVER_PORT' => 443]));
$this->assertTrue(RequestUtil::isSecure(['SERVER_PORT' => '443']));
$this->assertFalse(RequestUtil::isSecure(['SERVER_PORT' => 8080]));
}
public function testDirectTlsBeatsUntrustedProxyHeader(): void
{
// HTTPS=on is authoritative even if a bogus proxy header says http.
$this->assertTrue(RequestUtil::isSecure([
'HTTPS' => 'on',
'REMOTE_ADDR' => '203.0.113.42',
'HTTP_X_FORWARDED_PROTO' => 'http',
]));
}
public function testBaseUrlRespectsProxyTermination(): void
{
$this->assertSame('https://xamxam.erg.be', RequestUtil::baseUrl([
'REMOTE_ADDR' => '127.0.0.1',
'HTTP_X_FORWARDED_PROTO' => 'https',
'HTTP_HOST' => 'xamxam.erg.be',
]));
}
public function testBaseUrlFallsBackToLocalhost(): void
{
$this->assertSame('http://localhost', RequestUtil::baseUrl([]));
}
public function testIsTrustedProxyClassification(): void
{
$this->assertTrue(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => '127.0.0.1']));
$this->assertTrue(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => '::1']));
$this->assertTrue(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => '172.16.0.1']));
$this->assertTrue(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => 'fd00::1']));
$this->assertFalse(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => '8.8.8.8']));
$this->assertFalse(RequestUtil::isTrustedProxy(['REMOTE_ADDR' => '']));
$this->assertFalse(RequestUtil::isTrustedProxy([]));
}
}