From c329c5fe5b21e7207e36aee6bded8dbd93b49137 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Thu, 18 Apr 2024 16:46:33 +0800 Subject: [PATCH 1/2] Add `spring.rabbitmq.template.allowed-list-patterns` property Update `RabbitProperties` and `RabbitTemplateConfigurer` to support a `spring.rabbitmq.template.allowed-list-patterns` property. The can be used to prevent errors of the form: java.lang.SecurityException: Attempt to deserialize unauthorized class com.example.domain.Message; add allowed class name patterns to the message converter or, if you trust the message orginiator, set environment variable 'SPRING_AMQP_DESERIALIZATION_TRUST_ALL' or system property 'spring.amqp.deserialization.trust.all' to true See gh-40421 --- .../autoconfigure/amqp/RabbitProperties.java | 14 +++++++++ .../amqp/RabbitTemplateConfigurer.java | 8 +++++ .../amqp/RabbitAutoConfigurationTests.java | 30 +++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java index d4e744b522b..abc949cad3b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitProperties.java @@ -48,6 +48,7 @@ import org.springframework.util.unit.DataSize; * @author Rafael Carvalho * @author Scott Frederick * @author Lasse Wulff + * @author Yanming Zhou * @since 1.0.0 */ @ConfigurationProperties(prefix = "spring.rabbitmq") @@ -1015,6 +1016,11 @@ public class RabbitProperties { */ private boolean observationEnabled; + /** + * Simple patterns for allowable packages/classes for deserialization. + */ + private List allowedListPatterns; + public Retry getRetry() { return this.retry; } @@ -1075,6 +1081,14 @@ public class RabbitProperties { this.observationEnabled = observationEnabled; } + public List getAllowedListPatterns() { + return this.allowedListPatterns; + } + + public void setAllowedListPatterns(List allowedListPatterns) { + this.allowedListPatterns = allowedListPatterns; + } + } public static class Retry { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitTemplateConfigurer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitTemplateConfigurer.java index b51443f87c4..5ae73cd3774 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitTemplateConfigurer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitTemplateConfigurer.java @@ -21,6 +21,7 @@ import java.util.List; import org.springframework.amqp.rabbit.connection.ConnectionFactory; import org.springframework.amqp.rabbit.core.RabbitTemplate; +import org.springframework.amqp.support.converter.AllowedListDeserializingMessageConverter; import org.springframework.amqp.support.converter.MessageConverter; import org.springframework.boot.context.properties.PropertyMapper; import org.springframework.util.Assert; @@ -29,6 +30,7 @@ import org.springframework.util.Assert; * Configure {@link RabbitTemplate} with sensible defaults. * * @author Stephane Nicoll + * @author Yanming Zhou * @since 2.3.0 */ public class RabbitTemplateConfigurer { @@ -102,6 +104,12 @@ public class RabbitTemplateConfigurer { map.from(templateProperties::getRoutingKey).to(template::setRoutingKey); map.from(templateProperties::getDefaultReceiveQueue).whenNonNull().to(template::setDefaultReceiveQueue); map.from(templateProperties::isObservationEnabled).to(template::setObservationEnabled); + if (templateProperties.getAllowedListPatterns() != null) { + MessageConverter messageConverter = template.getMessageConverter(); + if (messageConverter instanceof AllowedListDeserializingMessageConverter mc) { + mc.setAllowedListPatterns(templateProperties.getAllowedListPatterns()); + } + } } private boolean determineMandatoryFlag() { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitAutoConfigurationTests.java index b3303e7e340..9dd780f025c 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitAutoConfigurationTests.java @@ -17,6 +17,7 @@ package org.springframework.boot.autoconfigure.amqp; import java.security.NoSuchAlgorithmException; +import java.util.Collection; import java.util.List; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicInteger; @@ -35,6 +36,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledForJreRange; import org.junit.jupiter.api.condition.JRE; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.InOrder; import org.springframework.amqp.core.AcknowledgeMode; @@ -59,7 +62,9 @@ import org.springframework.amqp.rabbit.listener.DirectMessageListenerContainer; import org.springframework.amqp.rabbit.listener.RabbitListenerContainerFactory; import org.springframework.amqp.rabbit.listener.SimpleMessageListenerContainer; import org.springframework.amqp.rabbit.retry.MessageRecoverer; +import org.springframework.amqp.support.converter.AllowedListDeserializingMessageConverter; import org.springframework.amqp.support.converter.MessageConverter; +import org.springframework.amqp.support.converter.SerializerMessageConverter; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.ssl.SslAutoConfiguration; @@ -107,6 +112,7 @@ import static org.mockito.Mockito.mock; * @author Andy Wilkinson * @author Phillip Webb * @author Scott Frederick + * @author Yanming Zhou */ @ExtendWith(OutputCaptureExtension.class) class RabbitAutoConfigurationTests { @@ -796,6 +802,20 @@ class RabbitAutoConfigurationTests { }); } + @SuppressWarnings("unchecked") + @ParameterizedTest + @ValueSource(classes = { TestConfiguration.class, TestConfiguration6.class }) + void customizeAllowedListPatterns(Class configuration) { + this.contextRunner.withUserConfiguration(configuration) + .withPropertyValues("spring.rabbitmq.template.allowed-list-patterns:*") + .run((context) -> { + MessageConverter messageConverter = context.getBean(RabbitTemplate.class).getMessageConverter(); + assertThat(messageConverter).isInstanceOfSatisfying(AllowedListDeserializingMessageConverter.class, + (mc) -> assertThat(mc).extracting("allowedListPatterns") + .isInstanceOfSatisfying(Collection.class, (set) -> assertThat(set).contains("*"))); + }); + } + @Test void noSslByDefault() { this.contextRunner.withUserConfiguration(TestConfiguration.class).run((context) -> { @@ -1113,6 +1133,16 @@ class RabbitAutoConfigurationTests { } + @Configuration(proxyBeanMethods = false) + static class TestConfiguration6 { + + @Bean + MessageConverter messageConverter() { + return new SerializerMessageConverter(); + } + + } + @Configuration(proxyBeanMethods = false) static class MessageConvertersConfiguration { From d243d7eb503fab4632ee894208127075dcf58b7c Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 18 Apr 2024 12:46:30 -0700 Subject: [PATCH 2/2] Polish 'Add `spring.rabbitmq.template.allowed-list-patterns` property' See gh-40421 --- .../amqp/RabbitTemplateConfigurer.java | 19 ++++++--- .../amqp/RabbitAutoConfigurationTests.java | 42 ++++++++++++++++--- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitTemplateConfigurer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitTemplateConfigurer.java index 5ae73cd3774..199bec92526 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitTemplateConfigurer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/amqp/RabbitTemplateConfigurer.java @@ -24,7 +24,9 @@ import org.springframework.amqp.rabbit.core.RabbitTemplate; import org.springframework.amqp.support.converter.AllowedListDeserializingMessageConverter; import org.springframework.amqp.support.converter.MessageConverter; import org.springframework.boot.context.properties.PropertyMapper; +import org.springframework.boot.context.properties.source.InvalidConfigurationPropertyValueException; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; /** * Configure {@link RabbitTemplate} with sensible defaults. @@ -104,12 +106,19 @@ public class RabbitTemplateConfigurer { map.from(templateProperties::getRoutingKey).to(template::setRoutingKey); map.from(templateProperties::getDefaultReceiveQueue).whenNonNull().to(template::setDefaultReceiveQueue); map.from(templateProperties::isObservationEnabled).to(template::setObservationEnabled); - if (templateProperties.getAllowedListPatterns() != null) { - MessageConverter messageConverter = template.getMessageConverter(); - if (messageConverter instanceof AllowedListDeserializingMessageConverter mc) { - mc.setAllowedListPatterns(templateProperties.getAllowedListPatterns()); - } + map.from(templateProperties::getAllowedListPatterns) + .whenNot(CollectionUtils::isEmpty) + .to((allowListPatterns) -> setAllowedListPatterns(template.getMessageConverter(), allowListPatterns)); + } + + private void setAllowedListPatterns(MessageConverter messageConverter, List allowListPatterns) { + if (messageConverter instanceof AllowedListDeserializingMessageConverter allowedListDeserializingMessageConverter) { + allowedListDeserializingMessageConverter.setAllowedListPatterns(allowListPatterns); + return; } + throw new InvalidConfigurationPropertyValueException("spring.rabbitmq.template.allow-list-patterns", + allowListPatterns, + "Allow list patterns can only be applied to a AllowedListDeserializingMessageConverter"); } private boolean determineMandatoryFlag() { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitAutoConfigurationTests.java index 9dd780f025c..f377076a31c 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/amqp/RabbitAutoConfigurationTests.java @@ -43,6 +43,7 @@ import org.mockito.InOrder; import org.springframework.amqp.core.AcknowledgeMode; import org.springframework.amqp.core.AmqpAdmin; import org.springframework.amqp.core.Message; +import org.springframework.amqp.core.MessageProperties; import org.springframework.amqp.rabbit.annotation.EnableRabbit; import org.springframework.amqp.rabbit.annotation.RabbitListener; import org.springframework.amqp.rabbit.config.AbstractRabbitListenerContainerFactory; @@ -62,12 +63,13 @@ import org.springframework.amqp.rabbit.listener.DirectMessageListenerContainer; import org.springframework.amqp.rabbit.listener.RabbitListenerContainerFactory; import org.springframework.amqp.rabbit.listener.SimpleMessageListenerContainer; import org.springframework.amqp.rabbit.retry.MessageRecoverer; -import org.springframework.amqp.support.converter.AllowedListDeserializingMessageConverter; +import org.springframework.amqp.support.converter.MessageConversionException; import org.springframework.amqp.support.converter.MessageConverter; import org.springframework.amqp.support.converter.SerializerMessageConverter; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.ssl.SslAutoConfiguration; +import org.springframework.boot.context.properties.source.InvalidConfigurationPropertyValueException; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.assertj.AssertableApplicationContext; import org.springframework.boot.test.context.runner.ApplicationContextRunner; @@ -802,20 +804,27 @@ class RabbitAutoConfigurationTests { }); } - @SuppressWarnings("unchecked") @ParameterizedTest @ValueSource(classes = { TestConfiguration.class, TestConfiguration6.class }) + @SuppressWarnings("unchecked") void customizeAllowedListPatterns(Class configuration) { this.contextRunner.withUserConfiguration(configuration) .withPropertyValues("spring.rabbitmq.template.allowed-list-patterns:*") .run((context) -> { MessageConverter messageConverter = context.getBean(RabbitTemplate.class).getMessageConverter(); - assertThat(messageConverter).isInstanceOfSatisfying(AllowedListDeserializingMessageConverter.class, - (mc) -> assertThat(mc).extracting("allowedListPatterns") - .isInstanceOfSatisfying(Collection.class, (set) -> assertThat(set).contains("*"))); + assertThat(messageConverter).extracting("allowedListPatterns") + .isInstanceOfSatisfying(Collection.class, (set) -> assertThat(set).contains("*")); }); } + @Test + void customizeAllowedListPatternsWhenHasNoAllowedListDeserializingMessageConverter() { + this.contextRunner.withUserConfiguration(CustomMessageConverterConfiguration.class) + .withPropertyValues("spring.rabbitmq.template.allowed-list-patterns:*") + .run((context) -> assertThat(context).getFailure() + .hasRootCauseInstanceOf(InvalidConfigurationPropertyValueException.class)); + } + @Test void noSslByDefault() { this.contextRunner.withUserConfiguration(TestConfiguration.class).run((context) -> { @@ -1417,6 +1426,29 @@ class RabbitAutoConfigurationTests { } + @Configuration + static class CustomMessageConverterConfiguration { + + @Bean + MessageConverter messageConverter() { + return new MessageConverter() { + + @Override + public Message toMessage(Object object, MessageProperties messageProperties) + throws MessageConversionException { + return new Message(object.toString().getBytes()); + } + + @Override + public Object fromMessage(Message message) throws MessageConversionException { + return new String(message.getBody()); + } + + }; + } + + } + static class TestListener { @RabbitListener(queues = "test", autoStartup = "false")