bug #42707 [Messenger] [AMQP] Do not leak any credentials when connection fails (ruudk)

fabpot · fabpot · commit d3369268585f · 2021-08-25T06:45:12.000+02:00
This PR was submitted for the 5.4 branch but it was merged into the 5.3 branch instead. Discussion ---------- [Messenger] [AMQP] Do not leak any credentials when connection fails | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? |no | Deprecations? | no | Tickets | | License | MIT | Doc PR | I noticed that when the connection to AMQP fails for whatever reason all the DSN credentials are leaked. ``` Aug 24 15:58:40 CRITICAL: Error thrown while running command "messenger:consume async". Message: "Could not connect to the AMQP server. Please verify the provided DSN. ({"host":my-hostname-on-some-server","port":my-port,"vhost":"the-real-vhost", "login":"the-real-username","password":"********"}) ``` Yes, the password is masked. But it still leaks the server, port, username and vhost. I think these things should be private and not be logged to a logger server or error capture service. Commits ------- 5812a49 [AMQP] [Messenger] Do not leak any credentials when connection fails
diff --git a/src/Symfony/Component/Messenger/Bridge/Amqp/Tests/Transport/ConnectionTest.php b/src/Symfony/Component/Messenger/Bridge/Amqp/Tests/Transport/ConnectionTest.php
@@ -559,10 +559,10 @@ public function testItDelaysTheMessageWithADifferentRoutingKeyAndTTLs()
         $connection->publish('{}', [], 120000);
     }
 
-    public function testObfuscatePasswordInDsn()
+    public function testNoCredentialLeakageWhenConnectionFails()
     {
         $this->expectException(\AMQPException::class);
-        $this->expectExceptionMessage('Could not connect to the AMQP server. Please verify the provided DSN. ({"host":"localhost","port":5672,"vhost":"/","login":"user","password":"********"})');
+        $this->expectExceptionMessage('Could not connect to the AMQP server. Please verify the provided DSN.');
         $factory = new TestAmqpFactory(
             $amqpConnection = $this->createMock(\AMQPConnection::class),
             $amqpChannel = $this->createMock(\AMQPChannel::class),
diff --git a/src/Symfony/Component/Messenger/Bridge/Amqp/Transport/Connection.php b/src/Symfony/Component/Messenger/Bridge/Amqp/Transport/Connection.php
@@ -495,11 +495,7 @@ public function channel(): \AMQPChannel
             try {
                 $connection->{$connectMethod}();
             } catch (\AMQPConnectionException $e) {
-                $credentials = $this->connectionOptions;
-                $credentials['password'] = '********';
-                unset($credentials['delay']);
-
-                throw new \AMQPException(sprintf('Could not connect to the AMQP server. Please verify the provided DSN. (%s).', json_encode($credentials, \JSON_UNESCAPED_SLASHES)), 0, $e);
+                throw new \AMQPException('Could not connect to the AMQP server. Please verify the provided DSN.', 0, $e);
             }
             $this->amqpChannel = $this->amqpFactory->createChannel($connection);
 

Original file line number	Diff line number	Diff line change
`@@ -559,10 +559,10 @@ public function testItDelaysTheMessageWithADifferentRoutingKeyAndTTLs()`
`559`	`559`	`$connection->publish('{}', [], 120000);`
`560`	`560`	`}`
`561`	`561`
`562`		`- public function testObfuscatePasswordInDsn()`
	`562`	`+ public function testNoCredentialLeakageWhenConnectionFails()`
`563`	`563`	`{`
`564`	`564`	`$this->expectException(\AMQPException::class);`
`565`		`- $this->expectExceptionMessage('Could not connect to the AMQP server. Please verify the provided DSN. ({"host":"localhost","port":5672,"vhost":"/","login":"user","password":"********"})');`
	`565`	`+ $this->expectExceptionMessage('Could not connect to the AMQP server. Please verify the provided DSN.');`
`566`	`566`	`$factory = new TestAmqpFactory(`
`567`	`567`	`$amqpConnection = $this->createMock(\AMQPConnection::class),`
`568`	`568`	`$amqpChannel = $this->createMock(\AMQPChannel::class),`