Skip to content

Commit

Permalink
Fix SSL configuration with Reactor Netty
Browse files Browse the repository at this point in the history 
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 spring-projectsgh-25913
  • Loading branch information
@bclozel
bclozel committed Apr 8, 2021
1 parent b52902e commit 903be6c
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 66 deletions.
11 changes: 5 additions & 6 deletions ...boot/actuate/autoconfigure/cloudfoundry/reactive/ReactiveCloudFoundrySecurityService.java
View file
Expand Up @@ -20,10 +20,10 @@
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;
Expand Down Expand Up @@ -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));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion spring-boot-project/spring-boot-dependencies/build.gradle
View file
Expand Up @@ -1388,7 +1388,7 @@ bom {
]
}
}
library("Reactor Bom", "2020.0.5") {
library("Reactor Bom", "2020.0.6-SNAPSHOT") {
group("io.projectreactor") {
imports = [
"reactor-bom"
Expand Down
24 changes: 14 additions & 10 deletions ...-boot/src/main/java/org/springframework/boot/rsocket/netty/NettyRSocketServerFactory.java
View file
Expand Up @@ -31,14 +31,14 @@
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;
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;
Expand Down Expand Up @@ -171,12 +171,18 @@ private ServerTransport<CloseableChannel> createWebSocketTransport() {
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<CloseableChannel> createTcpTransport() {
TcpServer tcpServer = TcpServer.create();
if (this.resourceFactory != null) {
Expand All @@ -196,19 +202,17 @@ private InetSocketAddress getListenAddress() {
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));
}

}
Expand Down
10 changes: 7 additions & 3 deletions .../main/java/org/springframework/boot/web/embedded/netty/NettyReactiveWebServerFactory.java
View file
Expand Up @@ -166,9 +166,7 @@ private HttpServer createHttpServer() {
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());
Expand All @@ -178,6 +176,12 @@ private HttpServer createHttpServer() {
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 };
Expand Down
60 changes: 31 additions & 29 deletions ...g-boot/src/main/java/org/springframework/boot/web/embedded/netty/SslServerCustomizer.java
View file
Expand Up @@ -38,9 +38,10 @@
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;
Expand All @@ -57,7 +58,9 @@
* @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;
Expand All @@ -74,38 +77,37 @@ public SslServerCustomizer(Ssl ssl, Http2 http2, SslStoreProvider sslStoreProvid

@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());
Expand Down Expand Up @@ -133,7 +135,7 @@ private KeyStore getKeyStore(Ssl ssl, SslStoreProvider sslStoreProvider) throws
ssl.getKeyStorePassword());
}

protected TrustManagerFactory getTrustManagerFactory(Ssl ssl, SslStoreProvider sslStoreProvider) {
TrustManagerFactory getTrustManagerFactory(Ssl ssl, SslStoreProvider sslStoreProvider) {
try {
KeyStore store = getTrustStore(ssl, sslStoreProvider);
TrustManagerFactory trustManagerFactory = TrustManagerFactory
Expand Down
14 changes: 7 additions & 7 deletions .../src/test/java/org/springframework/boot/rsocket/netty/NettyRSocketServerFactoryTests.java
View file
Expand Up @@ -22,7 +22,6 @@
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;
Expand All @@ -37,6 +36,7 @@
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;
Expand Down Expand Up @@ -224,9 +224,9 @@ private RSocketRequester createSecureRSocketWebSocketClient() {

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() {
Expand All @@ -237,9 +237,9 @@ private HttpClient createHttpClient() {

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() {
Expand Down
3 changes: 2 additions & 1 deletion ...t/src/test/java/org/springframework/boot/web/embedded/netty/SslServerCustomizerTests.java
View file
@@ -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.
Expand Down Expand Up @@ -31,6 +31,7 @@
* @author Andy Wilkinson
* @author Raheela Aslam
*/
@SuppressWarnings("deprecation")
class SslServerCustomizerTests {

@Test
Expand Down
18 changes: 9 additions & 9 deletions ...a/org/springframework/boot/web/reactive/server/AbstractReactiveWebServerFactoryTests.java
View file
Expand Up @@ -37,7 +37,6 @@
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;
Expand All @@ -47,6 +46,7 @@
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;

Expand Down Expand Up @@ -195,10 +195,9 @@ protected void assertThatSslWithInvalidAliasCallFails(ThrowingCallable call) {
}

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);
}

Expand Down Expand Up @@ -232,10 +231,11 @@ protected ReactorClientHttpConnector buildTrustAllSslWithClientKeyConnector() th
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);
}

Expand Down

0 comments on commit 903be6c

Please sign in to comment.