From a0ee0601ef32c7df031736cc6348838cb09b8689 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 8 Apr 2021 17:00:37 +0200 Subject: [PATCH] Fix SSL configuration with Reactor Netty Prior to this commit, the SslServerCustomizer would use a Reactor Netty API that lets users customize the SSL configuration, but later override some of the choices with defaults. This commits moves from the new deprecated Reactor Netty API and instead uses a new variant that builds the defaults and lets developers override them if they want to. Fixes gh-25913 --- .../ReactiveCloudFoundrySecurityService.java | 11 ++-- .../spring-boot-dependencies/build.gradle | 2 +- .../netty/NettyRSocketServerFactory.java | 24 ++++---- .../netty/NettyReactiveWebServerFactory.java | 10 +++- .../embedded/netty/SslServerCustomizer.java | 60 ++++++++++--------- .../netty/NettyRSocketServerFactoryTests.java | 14 ++--- .../netty/SslServerCustomizerTests.java | 3 +- ...AbstractReactiveWebServerFactoryTests.java | 18 +++--- 8 files changed, 76 insertions(+), 66 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/reactive/ReactiveCloudFoundrySecurityService.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/reactive/ReactiveCloudFoundrySecurityService.java index bb82d7c203..6a747857d8 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/reactive/ReactiveCloudFoundrySecurityService.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/reactive/ReactiveCloudFoundrySecurityService.java @@ -20,10 +20,10 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import io.netty.handler.ssl.SslContextBuilder; import io.netty.handler.ssl.SslProvider; import io.netty.handler.ssl.util.InsecureTrustManagerFactory; import reactor.core.publisher.Mono; +import reactor.netty.http.Http11SslContextSpec; import reactor.netty.http.client.HttpClient; import org.springframework.boot.actuate.autoconfigure.cloudfoundry.AccessLevel; @@ -66,14 +66,13 @@ class ReactiveCloudFoundrySecurityService { } protected ReactorClientHttpConnector buildTrustAllSslConnector() { - HttpClient client = HttpClient.create() - .secure((sslContextSpec) -> sslContextSpec.sslContext(createSslContext())); + HttpClient client = HttpClient.create().secure((spec) -> spec.sslContext(createSslContextSpec())); return new ReactorClientHttpConnector(client); } - private SslContextBuilder createSslContext() { - return SslContextBuilder.forClient().sslProvider(SslProvider.JDK) - .trustManager(InsecureTrustManagerFactory.INSTANCE); + private Http11SslContextSpec createSslContextSpec() { + return Http11SslContextSpec.forClient().configure( + (builder) -> builder.sslProvider(SslProvider.JDK).trustManager(InsecureTrustManagerFactory.INSTANCE)); } /** diff --git a/spring-boot-project/spring-boot-dependencies/build.gradle b/spring-boot-project/spring-boot-dependencies/build.gradle index ce6436d3fa..65bf5f9857 100644 --- a/spring-boot-project/spring-boot-dependencies/build.gradle +++ b/spring-boot-project/spring-boot-dependencies/build.gradle @@ -1322,7 +1322,7 @@ bom { ] } } - library("Reactor Bom", "2020.0.5") { + library("Reactor Bom", "2020.0.6-SNAPSHOT") { group("io.projectreactor") { imports = [ "reactor-bom" diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/rsocket/netty/NettyRSocketServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/rsocket/netty/NettyRSocketServerFactory.java index 31c6c01f0d..1ea7e2b801 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/rsocket/netty/NettyRSocketServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/rsocket/netty/NettyRSocketServerFactory.java @@ -31,6 +31,7 @@ import io.rsocket.transport.netty.server.TcpServerTransport; import io.rsocket.transport.netty.server.WebsocketServerTransport; import reactor.core.publisher.Mono; import reactor.netty.http.server.HttpServer; +import reactor.netty.tcp.AbstractProtocolSslContextSpec; import reactor.netty.tcp.TcpServer; import org.springframework.boot.context.properties.PropertyMapper; @@ -38,7 +39,6 @@ import org.springframework.boot.rsocket.server.ConfigurableRSocketServerFactory; import org.springframework.boot.rsocket.server.RSocketServer; import org.springframework.boot.rsocket.server.RSocketServerCustomizer; import org.springframework.boot.rsocket.server.RSocketServerFactory; -import org.springframework.boot.web.embedded.netty.SslServerCustomizer; import org.springframework.boot.web.server.Ssl; import org.springframework.boot.web.server.SslStoreProvider; import org.springframework.http.client.reactive.ReactorResourceFactory; @@ -171,12 +171,18 @@ public class NettyRSocketServerFactory implements RSocketServerFactory, Configur httpServer = httpServer.runOn(this.resourceFactory.getLoopResources()); } if (this.ssl != null && this.ssl.isEnabled()) { - SslServerCustomizer sslServerCustomizer = new SslServerCustomizer(this.ssl, null, this.sslStoreProvider); - httpServer = sslServerCustomizer.apply(httpServer); + httpServer = customizeSslConfiguration(httpServer); } return WebsocketServerTransport.create(httpServer.bindAddress(this::getListenAddress)); } + @SuppressWarnings("deprecation") + private HttpServer customizeSslConfiguration(HttpServer httpServer) { + org.springframework.boot.web.embedded.netty.SslServerCustomizer sslServerCustomizer = new org.springframework.boot.web.embedded.netty.SslServerCustomizer( + this.ssl, null, this.sslStoreProvider); + return sslServerCustomizer.apply(httpServer); + } + private ServerTransport createTcpTransport() { TcpServer tcpServer = TcpServer.create(); if (this.resourceFactory != null) { @@ -196,19 +202,17 @@ public class NettyRSocketServerFactory implements RSocketServerFactory, Configur return new InetSocketAddress(this.port); } - private static final class TcpSslServerCustomizer extends SslServerCustomizer { + @SuppressWarnings("deprecation") + private static final class TcpSslServerCustomizer + extends org.springframework.boot.web.embedded.netty.SslServerCustomizer { private TcpSslServerCustomizer(Ssl ssl, SslStoreProvider sslStoreProvider) { super(ssl, null, sslStoreProvider); } private TcpServer apply(TcpServer server) { - try { - return server.secure((contextSpec) -> contextSpec.sslContext(getContextBuilder())); - } - catch (Exception ex) { - throw new IllegalStateException(ex); - } + AbstractProtocolSslContextSpec sslContextSpec = createSslContextSpec(); + return server.secure((spec) -> spec.sslContext(sslContextSpec)); } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactory.java index 6ed948333f..7bf0bdf14c 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactory.java @@ -166,9 +166,7 @@ public class NettyReactiveWebServerFactory extends AbstractReactiveWebServerFact server = server.bindAddress(this::getListenAddress); } if (getSsl() != null && getSsl().isEnabled()) { - SslServerCustomizer sslServerCustomizer = new SslServerCustomizer(getSsl(), getHttp2(), - getSslStoreProvider()); - server = sslServerCustomizer.apply(server); + server = customizeSslConfiguration(server); } if (getCompression() != null && getCompression().getEnabled()) { CompressionCustomizer compressionCustomizer = new CompressionCustomizer(getCompression()); @@ -178,6 +176,12 @@ public class NettyReactiveWebServerFactory extends AbstractReactiveWebServerFact return applyCustomizers(server); } + @SuppressWarnings("deprecation") + private HttpServer customizeSslConfiguration(HttpServer httpServer) { + SslServerCustomizer sslServerCustomizer = new SslServerCustomizer(getSsl(), getHttp2(), getSslStoreProvider()); + return sslServerCustomizer.apply(httpServer); + } + private HttpProtocol[] listProtocols() { if (getHttp2() != null && getHttp2().isEnabled() && getSsl() != null && getSsl().isEnabled()) { return new HttpProtocol[] { HttpProtocol.H2, HttpProtocol.HTTP11 }; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java index f8ff93d3c7..469c2db3df 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java @@ -38,9 +38,10 @@ import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509ExtendedKeyManager; import io.netty.handler.ssl.ClientAuth; -import io.netty.handler.ssl.SslContextBuilder; +import reactor.netty.http.Http11SslContextSpec; +import reactor.netty.http.Http2SslContextSpec; import reactor.netty.http.server.HttpServer; -import reactor.netty.tcp.SslProvider; +import reactor.netty.tcp.AbstractProtocolSslContextSpec; import org.springframework.boot.web.server.Http2; import org.springframework.boot.web.server.Ssl; @@ -57,7 +58,9 @@ import org.springframework.util.ResourceUtils; * @author Raheela Aslam * @author Chris Bono * @since 2.0.0 + * @deprecated this class is meant for Spring Boot internal use only. */ +@Deprecated public class SslServerCustomizer implements NettyServerCustomizer { private final Ssl ssl; @@ -74,38 +77,37 @@ public class SslServerCustomizer implements NettyServerCustomizer { @Override public HttpServer apply(HttpServer server) { - try { - return server.secure((contextSpec) -> { - SslProvider.DefaultConfigurationSpec spec = contextSpec.sslContext(getContextBuilder()); - if (this.http2 != null && this.http2.isEnabled()) { - spec.defaultConfiguration(SslProvider.DefaultConfigurationType.H2); - } - }); - } - catch (Exception ex) { - throw new IllegalStateException(ex); - } + AbstractProtocolSslContextSpec sslContextSpec = createSslContextSpec(); + return server.secure((spec) -> spec.sslContext(sslContextSpec)); } - protected SslContextBuilder getContextBuilder() { - SslContextBuilder builder = SslContextBuilder.forServer(getKeyManagerFactory(this.ssl, this.sslStoreProvider)) - .trustManager(getTrustManagerFactory(this.ssl, this.sslStoreProvider)); - if (this.ssl.getEnabledProtocols() != null) { - builder.protocols(this.ssl.getEnabledProtocols()); - } - if (this.ssl.getCiphers() != null) { - builder.ciphers(Arrays.asList(this.ssl.getCiphers())); + protected AbstractProtocolSslContextSpec createSslContextSpec() { + AbstractProtocolSslContextSpec sslContextSpec; + if (this.http2 != null && this.http2.isEnabled()) { + sslContextSpec = Http2SslContextSpec.forServer(getKeyManagerFactory(this.ssl, this.sslStoreProvider)); } - if (this.ssl.getClientAuth() == Ssl.ClientAuth.NEED) { - builder.clientAuth(ClientAuth.REQUIRE); + else { + sslContextSpec = Http11SslContextSpec.forServer(getKeyManagerFactory(this.ssl, this.sslStoreProvider)); } - else if (this.ssl.getClientAuth() == Ssl.ClientAuth.WANT) { - builder.clientAuth(ClientAuth.OPTIONAL); - } - return builder; + sslContextSpec.configure((builder) -> { + builder.trustManager(getTrustManagerFactory(this.ssl, this.sslStoreProvider)); + if (this.ssl.getEnabledProtocols() != null) { + builder.protocols(this.ssl.getEnabledProtocols()); + } + if (this.ssl.getCiphers() != null) { + builder.ciphers(Arrays.asList(this.ssl.getCiphers())); + } + if (this.ssl.getClientAuth() == Ssl.ClientAuth.NEED) { + builder.clientAuth(ClientAuth.REQUIRE); + } + else if (this.ssl.getClientAuth() == Ssl.ClientAuth.WANT) { + builder.clientAuth(ClientAuth.OPTIONAL); + } + }); + return sslContextSpec; } - protected KeyManagerFactory getKeyManagerFactory(Ssl ssl, SslStoreProvider sslStoreProvider) { + KeyManagerFactory getKeyManagerFactory(Ssl ssl, SslStoreProvider sslStoreProvider) { try { KeyStore keyStore = getKeyStore(ssl, sslStoreProvider); SslConfigurationValidator.validateKeyAlias(keyStore, ssl.getKeyAlias()); @@ -133,7 +135,7 @@ public class SslServerCustomizer implements NettyServerCustomizer { ssl.getKeyStorePassword()); } - protected TrustManagerFactory getTrustManagerFactory(Ssl ssl, SslStoreProvider sslStoreProvider) { + TrustManagerFactory getTrustManagerFactory(Ssl ssl, SslStoreProvider sslStoreProvider) { try { KeyStore store = getTrustStore(ssl, sslStoreProvider); TrustManagerFactory trustManagerFactory = TrustManagerFactory diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/rsocket/netty/NettyRSocketServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/rsocket/netty/NettyRSocketServerFactoryTests.java index 06a52b391b..fd88751014 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/rsocket/netty/NettyRSocketServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/rsocket/netty/NettyRSocketServerFactoryTests.java @@ -22,7 +22,6 @@ import java.util.Arrays; import java.util.concurrent.Callable; import io.netty.buffer.PooledByteBufAllocator; -import io.netty.handler.ssl.SslContextBuilder; import io.netty.handler.ssl.SslProvider; import io.netty.handler.ssl.util.InsecureTrustManagerFactory; import io.rsocket.ConnectionSetupPayload; @@ -37,6 +36,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.mockito.InOrder; import reactor.core.publisher.Mono; +import reactor.netty.http.Http11SslContextSpec; import reactor.netty.http.client.HttpClient; import reactor.netty.tcp.TcpClient; import reactor.test.StepVerifier; @@ -224,9 +224,9 @@ class NettyRSocketServerFactoryTests { private HttpClient createSecureHttpClient() { HttpClient httpClient = createHttpClient(); - SslContextBuilder builder = SslContextBuilder.forClient().sslProvider(SslProvider.JDK) - .trustManager(InsecureTrustManagerFactory.INSTANCE); - return httpClient.secure((spec) -> spec.sslContext(builder)); + Http11SslContextSpec sslContextSpec = Http11SslContextSpec.forClient().configure( + (builder) -> builder.sslProvider(SslProvider.JDK).trustManager(InsecureTrustManagerFactory.INSTANCE)); + return httpClient.secure((spec) -> spec.sslContext(sslContextSpec)); } private HttpClient createHttpClient() { @@ -237,9 +237,9 @@ class NettyRSocketServerFactoryTests { private TcpClient createSecureTcpClient() { TcpClient tcpClient = createTcpClient(); - SslContextBuilder builder = SslContextBuilder.forClient().sslProvider(SslProvider.JDK) - .trustManager(InsecureTrustManagerFactory.INSTANCE); - return tcpClient.secure((spec) -> spec.sslContext(builder)); + Http11SslContextSpec sslContextSpec = Http11SslContextSpec.forClient().configure( + (builder) -> builder.sslProvider(SslProvider.JDK).trustManager(InsecureTrustManagerFactory.INSTANCE)); + return tcpClient.secure((spec) -> spec.sslContext(sslContextSpec)); } private TcpClient createTcpClient() { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/SslServerCustomizerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/SslServerCustomizerTests.java index ea563b7f95..9ba91d5db6 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/SslServerCustomizerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/netty/SslServerCustomizerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,6 +31,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; * @author Andy Wilkinson * @author Raheela Aslam */ +@SuppressWarnings("deprecation") class SslServerCustomizerTests { @Test diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java index 5c793775f4..859f7fba4e 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java @@ -37,7 +37,6 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpResponse; -import io.netty.handler.ssl.SslContextBuilder; import io.netty.handler.ssl.SslProvider; import io.netty.handler.ssl.util.InsecureTrustManagerFactory; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; @@ -47,6 +46,7 @@ import org.junit.jupiter.api.Test; import reactor.core.publisher.Mono; import reactor.core.publisher.Sinks; import reactor.netty.NettyPipeline; +import reactor.netty.http.Http11SslContextSpec; import reactor.netty.http.client.HttpClient; import reactor.test.StepVerifier; @@ -195,10 +195,9 @@ public abstract class AbstractReactiveWebServerFactoryTests { } protected ReactorClientHttpConnector buildTrustAllSslConnector() { - SslContextBuilder builder = SslContextBuilder.forClient().sslProvider(SslProvider.JDK) - .trustManager(InsecureTrustManagerFactory.INSTANCE); - HttpClient client = HttpClient.create().wiretap(true) - .secure((sslContextSpec) -> sslContextSpec.sslContext(builder)); + Http11SslContextSpec sslContextSpec = Http11SslContextSpec.forClient().configure( + (builder) -> builder.sslProvider(SslProvider.JDK).trustManager(InsecureTrustManagerFactory.INSTANCE)); + HttpClient client = HttpClient.create().wiretap(true).secure((spec) -> spec.sslContext(sslContextSpec)); return new ReactorClientHttpConnector(client); } @@ -232,10 +231,11 @@ public abstract class AbstractReactiveWebServerFactoryTests { KeyManagerFactory clientKeyManagerFactory = KeyManagerFactory .getInstance(KeyManagerFactory.getDefaultAlgorithm()); clientKeyManagerFactory.init(clientKeyStore, "password".toCharArray()); - SslContextBuilder builder = SslContextBuilder.forClient().sslProvider(SslProvider.JDK) - .trustManager(InsecureTrustManagerFactory.INSTANCE).keyManager(clientKeyManagerFactory); - HttpClient client = HttpClient.create().wiretap(true) - .secure((sslContextSpec) -> sslContextSpec.sslContext(builder)); + + Http11SslContextSpec sslContextSpec = Http11SslContextSpec.forClient() + .configure((builder) -> builder.sslProvider(SslProvider.JDK) + .trustManager(InsecureTrustManagerFactory.INSTANCE).keyManager(clientKeyManagerFactory)); + HttpClient client = HttpClient.create().wiretap(true).secure((spec) -> spec.sslContext(sslContextSpec)); return new ReactorClientHttpConnector(client); }