bug symfony#52469 Check whether secrets are empty and mark them all as sensitive (nicolas-grekas)

This PR was merged into the 6.4 branch.

Discussion
----------

Check whether secrets are empty and mark them all as sensitive

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Some security-hardening.

I made this throw a deprecation (as was already the case in some places) but I'd be fine making them throw an exception right away since an empty secret is a bold mistake. WDYT? (Webhook was experimental in 6.3 so it's fine throwing there I think.)

Commits
-------

52ca699 Check whether secrets are empty and mark them all as sensitive

Author: nicolas-grekas

src/Symfony/Component/HttpFoundation/UriSigner.php

@@ -12,8 +12,6 @@
 namespace Symfony\Component\HttpFoundation;
 
 /**
- * Signs URIs.
- *
  * @author Fabien Potencier <[email protected]>
  */
 class UriSigner
@@ -22,11 +20,14 @@ class UriSigner
     private string $parameter;
 
     /**
-     * @param string $secret    A secret
      * @param string $parameter Query string parameter to use
      */
     public function __construct(#[\SensitiveParameter] string $secret, string $parameter = '_hash')
     {
+        if (!$secret) {
+            throw new \InvalidArgumentException('A non-empty secret is required.');
+        }
+
         $this->secret = $secret;
         $this->parameter = $parameter;
     }

src/Symfony/Component/Mailer/Bridge/Brevo/Webhook/BrevoRequestParser.php

@@ -41,7 +41,7 @@ protected function getRequestMatcher(): RequestMatcherInterface
         ]);
     }
 
-    protected function doParse(Request $request, string $secret): ?AbstractMailerEvent
+    protected function doParse(Request $request, #[\SensitiveParameter] string $secret): ?AbstractMailerEvent
     {
         $content = $request->toArray();
         if (

src/Symfony/Component/Mailer/Bridge/Mailgun/Webhook/MailgunRequestParser.php

@@ -17,6 +17,7 @@
 use Symfony\Component\HttpFoundation\RequestMatcher\MethodRequestMatcher;
 use Symfony\Component\HttpFoundation\RequestMatcherInterface;
 use Symfony\Component\Mailer\Bridge\Mailgun\RemoteEvent\MailgunPayloadConverter;
+use Symfony\Component\Mailer\Exception\InvalidArgumentException;
 use Symfony\Component\RemoteEvent\Event\Mailer\AbstractMailerEvent;
 use Symfony\Component\RemoteEvent\Exception\ParseException;
 use Symfony\Component\Webhook\Client\AbstractRequestParser;
@@ -37,8 +38,12 @@ protected function getRequestMatcher(): RequestMatcherInterface
         ]);
     }
 
-    protected function doParse(Request $request, string $secret): ?AbstractMailerEvent
+    protected function doParse(Request $request, #[\SensitiveParameter] string $secret): ?AbstractMailerEvent
     {
+        if (!$secret) {
+            throw new InvalidArgumentException('A non-empty secret is required.');
+        }
+
         $content = $request->toArray();
         if (
             !isset($content['signature']['timestamp'])
@@ -60,7 +65,7 @@ protected function doParse(Request $request, string $secret): ?AbstractMailerEve
         }
     }
 
-    private function validateSignature(array $signature, string $secret): void
+    private function validateSignature(array $signature, #[\SensitiveParameter] string $secret): void
     {
         // see https://documentation.mailgun.com/en/latest/user_manual.html#webhooks-1
         if (!hash_equals($signature['signature'], hash_hmac('sha256', $signature['timestamp'].$signature['token'], $secret))) {

src/Symfony/Component/Mailer/Bridge/Mailjet/Webhook/MailjetRequestParser.php

@@ -37,7 +37,7 @@ protected function getRequestMatcher(): RequestMatcherInterface
         ]);
     }
 
-    protected function doParse(Request $request, string $secret): ?AbstractMailerEvent
+    protected function doParse(Request $request, #[\SensitiveParameter] string $secret): ?AbstractMailerEvent
     {
         try {
             return $this->converter->convert($request->toArray());

src/Symfony/Component/Mailer/Bridge/Postmark/Webhook/PostmarkRequestParser.php

@@ -41,7 +41,7 @@ protected function getRequestMatcher(): RequestMatcherInterface
         ]);
     }
 
-    protected function doParse(Request $request, string $secret): ?AbstractMailerEvent
+    protected function doParse(Request $request, #[\SensitiveParameter] string $secret): ?AbstractMailerEvent
     {
         $payload = $request->toArray();
         if (

src/Symfony/Component/Mailer/Bridge/Sendgrid/Webhook/SendgridRequestParser.php

@@ -17,6 +17,7 @@
 use Symfony\Component\HttpFoundation\RequestMatcher\MethodRequestMatcher;
 use Symfony\Component\HttpFoundation\RequestMatcherInterface;
 use Symfony\Component\Mailer\Bridge\Sendgrid\RemoteEvent\SendgridPayloadConverter;
+use Symfony\Component\Mailer\Exception\InvalidArgumentException;
 use Symfony\Component\RemoteEvent\Event\Mailer\AbstractMailerEvent;
 use Symfony\Component\RemoteEvent\Exception\ParseException;
 use Symfony\Component\Webhook\Client\AbstractRequestParser;
@@ -86,12 +87,12 @@ protected function doParse(Request $request, string $secret): ?AbstractMailerEve
      *
      * @see https://docs.sendgrid.com/for-developers/tracking-events/getting-started-event-webhook-security-features
      */
-    private function validateSignature(
-        string $signature,
-        string $timestamp,
-        string $payload,
-        string $secret,
-    ): void {
+    private function validateSignature(string $signature, string $timestamp, string $payload, #[\SensitiveParameter] string $secret): void
+    {
+        if (!$secret) {
+            throw new InvalidArgumentException('A non-empty secret is required.');
+        }
+
         $timestampedPayload = $timestamp.$payload;
 
         // Sendgrid provides the verification key as base64-encoded DER data. Openssl wants a PEM format, which is a multiline version of the base64 data.

src/Symfony/Component/Mailer/Transport/Smtp/Auth/CramMd5Authenticator.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Mailer\Transport\Smtp\Auth;
 
+use Symfony\Component\Mailer\Exception\InvalidArgumentException;
 use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport;
 
 /**
@@ -41,6 +42,10 @@ public function authenticate(EsmtpTransport $client): void
      */
     private function getResponse(#[\SensitiveParameter] string $secret, string $challenge): string
     {
+        if (!$secret) {
+            throw new InvalidArgumentException('A non-empty secret is required.');
+        }
+
         if (\strlen($secret) > 64) {
             $secret = pack('H32', md5($secret));
         }

src/Symfony/Component/Notifier/Bridge/Twilio/Webhook/TwilioRequestParser.php

@@ -25,7 +25,7 @@ protected function getRequestMatcher(): RequestMatcherInterface
         return new MethodRequestMatcher('POST');
     }
 
-    protected function doParse(Request $request, string $secret): ?SmsEvent
+    protected function doParse(Request $request, #[\SensitiveParameter] string $secret): ?SmsEvent
     {
         // Statuses: https://www.twilio.com/docs/sms/api/message-resource#message-status-values
         // Payload examples: https://www.twilio.com/docs/sms/outbound-message-logging

src/Symfony/Component/Notifier/Bridge/Vonage/Webhook/VonageRequestParser.php

@@ -16,6 +16,7 @@
 use Symfony\Component\HttpFoundation\RequestMatcher\IsJsonRequestMatcher;
 use Symfony\Component\HttpFoundation\RequestMatcher\MethodRequestMatcher;
 use Symfony\Component\HttpFoundation\RequestMatcherInterface;
+use Symfony\Component\Notifier\Exception\InvalidArgumentException;
 use Symfony\Component\RemoteEvent\Event\Sms\SmsEvent;
 use Symfony\Component\Webhook\Client\AbstractRequestParser;
 use Symfony\Component\Webhook\Exception\RejectWebhookException;
@@ -30,8 +31,12 @@ protected function getRequestMatcher(): RequestMatcherInterface
         ]);
     }
 
-    protected function doParse(Request $request, string $secret): ?SmsEvent
+    protected function doParse(Request $request, #[\SensitiveParameter] string $secret): ?SmsEvent
     {
+        if (!$secret) {
+            throw new InvalidArgumentException('A non-empty secret is required.');
+        }
+
         // Signed webhooks: https://developer.vonage.com/en/getting-started/concepts/webhooks#validating-signed-webhooks
         if (!$request->headers->has('Authorization')) {
             throw new RejectWebhookException(406, 'Missing "Authorization" header.');
@@ -70,7 +75,7 @@ protected function doParse(Request $request, string $secret): ?SmsEvent
         return $event;
     }
 
-    private function validateSignature(string $jwt, string $secret): void
+    private function validateSignature(string $jwt, #[\SensitiveParameter] string $secret): void
     {
         $tokenParts = explode('.', $jwt);
         if (3 !== \count($tokenParts)) {

src/Symfony/Component/Security/Core/Authentication/Token/RememberMeToken.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Security\Core\Authentication\Token;
 
+use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
 use Symfony\Component\Security\Core\User\UserInterface;
 
 /**
@@ -32,12 +33,12 @@ public function __construct(UserInterface $user, string $firewallName, #[\Sensit
     {
         parent::__construct($user->getRoles());
 
-        if (empty($secret)) {
-            throw new \InvalidArgumentException('$secret must not be empty.');
+        if (!$secret) {
+            throw new InvalidArgumentException('A non-empty secret is required.');
         }
 
-        if ('' === $firewallName) {
-            throw new \InvalidArgumentException('$firewallName must not be empty.');
+        if (!$firewallName) {
+            throw new InvalidArgumentException('$firewallName must not be empty.');
         }
 
         $this->firewallName = $firewallName;