Skip to content

Commit bfaa483

Browse files
fabpotfabpot
committed
feature #62230 [Messenger] Support signing messages per handler (nicolas-grekas)
This PR was merged into the 7.4 branch. Discussion ---------- [Messenger] Support signing messages per handler | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT At the moment, if one is able to inject a forged payload into a queue, one can trigger any handler via the messenger consumer, including eg `RunProcessHandler`. This is not a security issue in Symfony itself because queues should be protected from arbitrary payload injection. But it'd still be nice to harden this. This PR adds a new `sign` attribute to the `messenger.message_handler` DI tag (which can be set either via explicit config or via `#[AsMessageHandler]`). When at least one handler does so, a `SigningSerializer` decorator is added to all transport serializers. This then computes the signature when encoding a message bound to such handlers, and verifies it when decoding one. The `sign` attribute is enabled for `RunProcessHandler` and `RunCommandHandler`, and can be for any others of yours. Submitting for 7.4 as having a hardened LTS looks important to me. Commits ------- 21c5ac56aca [Messenger] Support signing messages per handler
2 parents 7311853 + 38cf6cf commit bfaa483

File tree

5 files changed

+76
-12
lines changed

5 files changed

+76
-12
lines changed

‎DependencyInjection/FrameworkExtension.php‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2556,6 +2556,7 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
25562556
$senderAliases = [];
25572557
$transportRetryReferences = [];
25582558
$transportRateLimiterReferences = [];
2559+
$serializerIds = [];
25592560
foreach ($config['transports'] as $name => $transport) {
25602561
$serializerId = $transport['serializer'] ?? 'messenger.default_serializer';
25612562
$tags = [
@@ -2572,6 +2573,7 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
25722573
;
25732574
$container->setDefinition($transportId = 'messenger.transport.'.$name, $transportDefinition);
25742575
$senderAliases[$name] = $transportId;
2576+
$serializerIds[$transportId] = $serializerId;
25752577

25762578
if (null !== $transport['retry_strategy']['service']) {
25772579
$transportRetryReferences[$name] = new Reference($transport['retry_strategy']['service']);
@@ -2599,13 +2601,11 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
25992601
}
26002602

26012603
$senderReferences = [];
2602-
// alias => service_id
2603-
foreach ($senderAliases as $alias => $serviceId) {
2604-
$senderReferences[$alias] = new Reference($serviceId);
2604+
foreach ($senderAliases as $alias => $transportId) {
2605+
$senderReferences[$alias] = new Reference($transportId);
26052606
}
2606-
// service_id => service_id
2607-
foreach ($senderAliases as $serviceId) {
2608-
$senderReferences[$serviceId] = new Reference($serviceId);
2607+
foreach ($senderAliases as $transportId) {
2608+
$senderReferences[$transportId] = new Reference($transportId);
26092609
}
26102610

26112611
foreach ($config['transports'] as $name => $transport) {
@@ -2616,7 +2616,7 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
26162616
}
26172617
}
26182618

2619-
$failureTransportReferencesByTransportName = array_map(fn ($failureTransportName) => $senderReferences[$failureTransportName], $failureTransportsByName);
2619+
$failureTransportReferencesByTransportName = array_map(static fn ($failureTransportName) => $senderReferences[$failureTransportName], $failureTransportsByName);
26202620

26212621
$messageToSendersMapping = [];
26222622
foreach ($config['routing'] as $message => $messageConfiguration) {
@@ -2645,6 +2645,18 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder
26452645
->replaceArgument(1, $sendersServiceLocator)
26462646
;
26472647

2648+
$messageToSerializersMapping = [];
2649+
foreach ($messageToSendersMapping as $message => $senders) {
2650+
foreach ($senders as $sender) {
2651+
$serializerId = $serializerIds[$senderAliases[$sender] ?? $sender];
2652+
$messageToSerializersMapping[$message][$serializerId] = $serializerId;
2653+
}
2654+
$messageToSerializersMapping[$message] = array_keys($messageToSerializersMapping[$message]);
2655+
}
2656+
2657+
$container->getDefinition('messenger.signing_serializer')
2658+
->replaceArgument(2, $messageToSerializersMapping);
2659+
26482660
$container->getDefinition('messenger.retry.send_failed_message_for_retry_listener')
26492661
->replaceArgument(0, $sendersServiceLocator)
26502662
;
@@ -2943,7 +2955,7 @@ private function registerCachingHttpClient(array $options, array $defaultOptions
29432955

29442956
$container
29452957
->register($name.'.caching', CachingHttpClient::class)
2946-
->setDecoratedService($name, null, 15)
2958+
->setDecoratedService($name, null, 15)
29472959
->setArguments([
29482960
new Reference('.inner'),
29492961
new Reference($options['cache_pool']),

‎Resources/config/console.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,6 @@
407407
->args([
408408
service('console.messenger.application'),
409409
])
410-
->tag('messenger.message_handler')
410+
->tag('messenger.message_handler', ['sign' => true])
411411
;
412412
};

‎Resources/config/messenger.php‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\DependencyInjection\Loader\Configurator;
1313

14+
use Symfony\Component\DependencyInjection\Parameter;
1415
use Symfony\Component\DependencyInjection\ServiceLocator;
1516
use Symfony\Component\Messenger\Bridge\AmazonSqs\Transport\AmazonSqsTransportFactory;
1617
use Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpTransportFactory;
@@ -44,13 +45,13 @@
4445
use Symfony\Component\Messenger\Transport\Serialization\PhpSerializer;
4546
use Symfony\Component\Messenger\Transport\Serialization\Serializer;
4647
use Symfony\Component\Messenger\Transport\Serialization\SerializerInterface;
48+
use Symfony\Component\Messenger\Transport\Serialization\SigningSerializer;
4749
use Symfony\Component\Messenger\Transport\Sync\SyncTransportFactory;
4850
use Symfony\Component\Messenger\Transport\TransportFactory;
51+
use Symfony\Component\String\LazyString;
4952

5053
return static function (ContainerConfigurator $container) {
5154
$container->services()
52-
->alias(SerializerInterface::class, 'messenger.default_serializer')
53-
5455
// Asynchronous
5556
->set('messenger.senders_locator', SendersLocator::class)
5657
->args([
@@ -79,6 +80,22 @@
7980

8081
->set('messenger.transport.native_php_serializer', PhpSerializer::class)
8182
->alias('messenger.default_serializer', 'messenger.transport.native_php_serializer')
83+
->alias(SerializerInterface::class, 'messenger.default_serializer')
84+
85+
->set('messenger.signing_serializer', SigningSerializer::class)
86+
->abstract()
87+
->args([
88+
service('.inner'),
89+
inline_service('string') // wrap the signing key in a lazy string to prevent a hard dependency on the kernel.secret parameter
90+
->factory(class_exists(LazyString::class) ? [LazyString::class, 'fromCallable'] : 'current')
91+
->args([
92+
class_exists(LazyString::class, false) ? service_closure('.messenger.signing_serializer.signing_key') : [new Parameter('kernel.secret')],
93+
]),
94+
abstract_arg('message types to serializers'), // read and replaced by MessengerPass
95+
])
96+
->set('.messenger.signing_serializer.signing_key', 'string')
97+
->factory('current')
98+
->args([[new Parameter('kernel.secret')]])
8299

83100
// Middleware
84101
->set('messenger.middleware.handle_message', HandleMessageMiddleware::class)

‎Resources/config/process.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@
1717
$container
1818
->services()
1919
->set('process.messenger.process_message_handler', RunProcessMessageHandler::class)
20-
->tag('messenger.message_handler')
20+
->tag('messenger.message_handler', ['sign' => true])
2121
;
2222
};

‎Tests/DependencyInjection/PhpFrameworkExtensionTest.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\DependencyInjection\Exception\LogicException;
1919
use Symfony\Component\DependencyInjection\Exception\OutOfBoundsException;
2020
use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
21+
use Symfony\Component\Messenger\Tests\Fixtures\DummyMessage;
2122
use Symfony\Component\RateLimiter\CompoundRateLimiterFactory;
2223
use Symfony\Component\RateLimiter\RateLimiterFactoryInterface;
2324
use Symfony\Component\Validator\Constraints\Email;
@@ -425,6 +426,40 @@ public static function emailValidationModeProvider()
425426
}
426427
yield ['loose'];
427428
}
429+
430+
public function testMessengerSigningSerializerWiring()
431+
{
432+
$container = $this->createContainerFromClosure(function (ContainerBuilder $container) {
433+
$container->register('signed_handler', 'stdClass')
434+
->addTag('messenger.message_handler', ['handles' => DummyMessage::class, 'sign' => true]);
435+
436+
$container->loadFromExtension('framework', [
437+
'annotations' => false,
438+
'http_method_override' => false,
439+
'handle_all_throwables' => true,
440+
'php_errors' => ['log' => true],
441+
'messenger' => [
442+
'transports' => [
443+
'async' => ['dsn' => 'in-memory://'],
444+
],
445+
'routing' => [
446+
DummyMessage::class => ['senders' => ['async']],
447+
],
448+
'buses' => [
449+
'message_bus' => ['default_middleware' => ['enabled' => true]],
450+
],
451+
],
452+
]);
453+
});
454+
455+
$this->assertTrue($container->hasDefinition('messenger.signing_serializer'));
456+
$mapping = $container->getDefinition('messenger.signing_serializer')->getArgument(2);
457+
$this->assertArrayHasKey(DummyMessage::class, $mapping);
458+
$this->assertNotEmpty($mapping[DummyMessage::class]);
459+
460+
$this->assertTrue($container->hasDefinition('message_bus'));
461+
$this->assertSame('message_bus', (string) $container->getAlias('messenger.default_bus'));
462+
}
428463
}
429464

430465
class WorkflowValidatorWithConstructor implements DefinitionValidatorInterface

0 commit comments

Comments
 (0)