diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ebfdf5..4bb5bed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Throws `\RuntimeException` with clear message when directory creation fails - Double `is_dir()` check handles race condition between check and `mkdir()` call +- Security: RequestConverter no longer trusts X-Forwarded-Proto header + unconditionally. HTTPS is now detected only from the actual SSL transport + layer. Users behind reverse proxies must configure Symfony's trusted + proxies. ([#152](https://github.com/crazy-goat/workerman-bundle/issues/152)) + ## [0.14.0] - 2026-04-14 ### Deprecated diff --git a/src/DTO/RequestConverter.php b/src/DTO/RequestConverter.php index a71c05b..d92bc54 100644 --- a/src/DTO/RequestConverter.php +++ b/src/DTO/RequestConverter.php @@ -33,13 +33,9 @@ public static function toSymfonyRequest(\Workerman\Protocols\Http\Request $rawRe // Build server bag with HTTP_* headers (CGI convention) $headers = $rawRequest->header() ?? []; - // Fallback to 127.0.0.1:0 for unit test scenarios where connection is null. - // In production, connection should always be present. - // Detect HTTPS from connection port or X-Forwarded-Proto header - $localPort = $rawRequest->connection?->getLocalPort(); - $forwardedProto = strtolower((string) $rawRequest->header('x-forwarded-proto', '')); - $isHttps = $localPort === 443 || $forwardedProto === 'https'; + // Detect HTTPS from Workerman's SSL transport (configured via https:// listen address) + $isHttps = isset($rawRequest->connection) && $rawRequest->connection->transport === 'ssl'; $requestTimeFloat = microtime(true); $server = [ @@ -48,7 +44,7 @@ public static function toSymfonyRequest(\Workerman\Protocols\Http\Request $rawRe 'SERVER_PROTOCOL' => 'HTTP/' . $rawRequest->protocolVersion(), 'REMOTE_ADDR' => $rawRequest->connection?->getRemoteIp() ?? '127.0.0.1', 'REMOTE_PORT' => $rawRequest->connection?->getRemotePort() ?? 0, - 'SERVER_PORT' => $localPort ?? ($isHttps ? 443 : 80), + 'SERVER_PORT' => $rawRequest->connection?->getLocalPort() ?? ($isHttps ? 443 : 80), 'SERVER_NAME' => $rawRequest->connection?->getLocalIp() ?? 'localhost', 'QUERY_STRING' => $rawRequest->queryString(), 'REQUEST_TIME' => (int) $requestTimeFloat, diff --git a/tests/RequestConverterTest.php b/tests/RequestConverterTest.php index 0cad1fc..7e44ab8 100644 --- a/tests/RequestConverterTest.php +++ b/tests/RequestConverterTest.php @@ -447,27 +447,41 @@ public function testGetPortReturnsPortFromHostHeaderWhenPresent(): void $this->assertSame(8080, $symfonyRequest->getPort()); } - public function testServerPortDefaultsTo443WhenNoConnectionButHttpsForwarded(): void + public function testHttpsDetectedFromSslTransport(): void { - $buffer = "GET /test HTTP/1.1\r\nHost: localhost\r\nX-Forwarded-Proto: https\r\n\r\n"; + $buffer = "GET /test HTTP/1.1\r\nHost: example.com\r\n\r\n"; $rawRequest = new Request($buffer); - $rawRequest->connection = null; + $rawRequest->connection = $this->createMockConnection(443, 'ssl'); $symfonyRequest = RequestConverter::toSymfonyRequest($rawRequest); $this->assertSame(443, $symfonyRequest->server->get('SERVER_PORT')); $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); + $this->assertSame('https', $symfonyRequest->getScheme()); } - public function testHttpsDetectedFromPort443(): void + public function testHttpDetectedFromTcpTransport(): void { $buffer = "GET /test HTTP/1.1\r\nHost: example.com\r\n\r\n"; $rawRequest = new Request($buffer); - $rawRequest->connection = $this->createMockConnection(443); + $rawRequest->connection = $this->createMockConnection(80, 'tcp'); $symfonyRequest = RequestConverter::toSymfonyRequest($rawRequest); - $this->assertSame(443, $symfonyRequest->server->get('SERVER_PORT')); + $this->assertSame(80, $symfonyRequest->server->get('SERVER_PORT')); + $this->assertNull($symfonyRequest->server->get('HTTPS')); + $this->assertSame('http', $symfonyRequest->getScheme()); + } + + public function testHttpsDetectedFromSslTransportOnAnyPort(): void + { + $buffer = "GET /test HTTP/1.1\r\nHost: example.com\r\n\r\n"; + $rawRequest = new Request($buffer); + $rawRequest->connection = $this->createMockConnection(8443, 'ssl'); + + $symfonyRequest = RequestConverter::toSymfonyRequest($rawRequest); + + $this->assertSame(8443, $symfonyRequest->server->get('SERVER_PORT')); $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); $this->assertSame('https', $symfonyRequest->getScheme()); } @@ -476,13 +490,26 @@ public function testGetSchemeAndHttpHostOmitsPort443ForHttps(): void { $buffer = "GET /test HTTP/1.1\r\nHost: example.com\r\n\r\n"; $rawRequest = new Request($buffer); - $rawRequest->connection = $this->createMockConnection(443); + $rawRequest->connection = $this->createMockConnection(443, 'ssl'); $symfonyRequest = RequestConverter::toSymfonyRequest($rawRequest); $this->assertSame('https://example.com', $symfonyRequest->getSchemeAndHttpHost()); } + public function testXForwardedProtoIgnoredOnTcpTransport(): void + { + $buffer = "GET /test HTTP/1.1\r\nHost: localhost\r\nX-Forwarded-Proto: https\r\n\r\n"; + $rawRequest = new Request($buffer); + $rawRequest->connection = $this->createMockConnection(80, 'tcp'); + + $symfonyRequest = RequestConverter::toSymfonyRequest($rawRequest); + + $this->assertNull($symfonyRequest->server->get('HTTPS')); + $this->assertFalse($symfonyRequest->isSecure()); + $this->assertSame('http', $symfonyRequest->getScheme()); + } + public function testRequestTimeAndRequestTimeFloatAreSet(): void { $buffer = "GET /test HTTP/1.1\r\nHost: localhost\r\n\r\n"; @@ -552,12 +579,13 @@ public function testFormUrlEncodedPreservesContent(): void $this->assertSame('key=value', $symfonyRequest->getContent()); } - private function createMockConnection(int $localPort): \Workerman\Connection\TcpConnection + private function createMockConnection(int $localPort, string $transport = 'tcp'): \Workerman\Connection\TcpConnection { - return new class ($localPort) extends \Workerman\Connection\TcpConnection { - public function __construct(private readonly int $port) + return new class ($localPort, $transport) extends \Workerman\Connection\TcpConnection { + public function __construct(private readonly int $port, string $transport) { $this->remoteAddress = '192.168.1.1:12345'; + $this->transport = $transport; } public function getLocalPort(): int diff --git a/tests/SymfonyControllerTest.php b/tests/SymfonyControllerTest.php index d157b69..03f0484 100644 --- a/tests/SymfonyControllerTest.php +++ b/tests/SymfonyControllerTest.php @@ -1078,15 +1078,15 @@ public function testStreamedJsonResponseE2E(): void public function testHttpsE2E(): void { - // E2E test: Verify HTTPS detection works through full stack (#64) - $symfonyResponse = new SymfonyResponse('secure'); + // E2E test: Verify port 443 without SSL transport is HTTP (#64) + $symfonyResponse = new SymfonyResponse('plain'); $kernel = new TestRequestTrackingKernel($symfonyResponse); $responseConverter = $this->createResponseConverter(); $controller = new SymfonyController($kernel, $responseConverter); - // Create a mock connection with SSL port (443) - $buffer = "GET /secure HTTP/1.1\r\nHost: localhost\r\n\r\n"; + // Port 443 alone should NOT imply HTTPS — must use SSL transport + $buffer = "GET /plain HTTP/1.1\r\nHost: localhost\r\n\r\n"; $request = new Request($buffer); $request->connection = $this->createMockConnection(443); @@ -1095,38 +1095,11 @@ public function testHttpsE2E(): void $this->assertNotNull($kernel->receivedRequest); $symfonyRequest = $kernel->receivedRequest; - // HTTPS should be detected from port 443 - $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); - $this->assertTrue($symfonyRequest->isSecure()); - $this->assertSame('https', $symfonyRequest->getScheme()); - $this->assertSame(443, $symfonyRequest->getPort()); - } - - public function testXForwardedProtoHttpsE2E(): void - { - // E2E test: Verify X-Forwarded-Proto header is detected (#64) - $symfonyResponse = new SymfonyResponse('proxied'); - $kernel = new TestRequestTrackingKernel($symfonyResponse); - $responseConverter = $this->createResponseConverter(); - - $controller = new SymfonyController($kernel, $responseConverter); - - // Create request with X-Forwarded-Proto header (connection port is regular 80) - $buffer = "GET /proxied HTTP/1.1\r\nHost: localhost\r\nX-Forwarded-Proto: https\r\n\r\n"; - $request = new Request($buffer); - $request->connection = $this->createMockConnection(80); - - $controller($request, $this->connection); - - $this->assertNotNull($kernel->receivedRequest); - $symfonyRequest = $kernel->receivedRequest; - - // HTTPS should be detected from X-Forwarded-Proto header - // Note: when isHttps is true, SERVER_PORT defaults to 443 - $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); - $this->assertTrue($symfonyRequest->isSecure()); - $this->assertSame('https', $symfonyRequest->getScheme()); - $this->assertSame(443, $symfonyRequest->getPort()); + // Port 443 without SSL transport = HTTP (Symfony defaults to 80 for HTTP scheme) + $this->assertNull($symfonyRequest->server->get('HTTPS')); + $this->assertFalse($symfonyRequest->isSecure()); + $this->assertSame('http', $symfonyRequest->getScheme()); + $this->assertSame(80, $symfonyRequest->getPort()); } public function testHttpE2E(): void @@ -1155,37 +1128,36 @@ public function testHttpE2E(): void $this->assertSame(80, $symfonyRequest->getPort()); } - public function testXForwardedProtoCaseInsensitiveE2E(): void + public function testHttpsE2EWithSslTransport(): void { - // E2E test: Verify X-Forwarded-Proto works with uppercase values (#64) - $symfonyResponse = new SymfonyResponse('proxied'); + $symfonyResponse = new SymfonyResponse('secure'); $kernel = new TestRequestTrackingKernel($symfonyResponse); $responseConverter = $this->createResponseConverter(); $controller = new SymfonyController($kernel, $responseConverter); - // Test with uppercase HTTPS value - $buffer = "GET /proxied HTTP/1.1\r\nHost: localhost\r\nX-Forwarded-Proto: HTTPS\r\n\r\n"; + $buffer = "GET /secure HTTP/1.1\r\nHost: localhost\r\n\r\n"; $request = new Request($buffer); - $request->connection = $this->createMockConnection(80); + $request->connection = $this->createMockConnection(443, 'ssl'); $controller($request, $this->connection); $this->assertNotNull($kernel->receivedRequest); $symfonyRequest = $kernel->receivedRequest; - // HTTPS should be detected from uppercase X-Forwarded-Proto header $this->assertSame('on', $symfonyRequest->server->get('HTTPS')); $this->assertTrue($symfonyRequest->isSecure()); $this->assertSame('https', $symfonyRequest->getScheme()); + $this->assertSame(443, $symfonyRequest->getPort()); } - private function createMockConnection(int $port): \Workerman\Connection\TcpConnection + private function createMockConnection(int $port, string $transport = 'tcp'): \Workerman\Connection\TcpConnection { - return new class ($port) extends \Workerman\Connection\TcpConnection { - public function __construct(private readonly int $port) + return new class ($port, $transport) extends \Workerman\Connection\TcpConnection { + public function __construct(private readonly int $port, string $transport) { $this->remoteAddress = '192.168.1.1:12345'; + $this->transport = $transport; } public function getLocalPort(): int