Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration of ciphers is ignored for TCP #1543

Closed
Johannes-Rost opened this issue Mar 10, 2021 · 6 comments · Fixed by #1573
Closed

Configuration of ciphers is ignored for TCP #1543

Johannes-Rost opened this issue Mar 10, 2021 · 6 comments · Fixed by #1573
Assignees
Labels
type/bug A general bug
Milestone

Comments

@Johannes-Rost
Copy link

Expected Behavior

Configuring a list of ciphers should result in a running server with this ciphers configured for Spring Boot 2.4.x with reactor-netty 1.0.x

Actual Behavior

reactor.netty.tcp.SslProvider is overriding the list of ciphers with null since commit 41e7326 for reactor.netty.tcp.SslProvider.DefaultConfigurationType#TCP. This results in a running server which falls back to default ciphers

Steps to Reproduce

Start a Spring Boot WebFlux Application with SSL support on localhost port 8443 and the following properties set

server.ssl.enabled-protocols=TLSv1.2,TLSv1.3
server.ssl.ciphers=TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384

Open SSL should not be able to complete a SSL handshake for ECDHE-RSA-AES256-SHA, but it is:

my-machine:johannesrost$ openssl s_client -cipher ECDHE-RSA-AES256-SHA -connect localhost:8443

[...]
SSL handshake has read 2940 bytes and written 225 bytes
---
New, TLSv1/SSLv3, Cipher is ECDHE-RSA-AES256-SHA
Server public key is 2048 bit
Secure Renegotiation IS supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1.2
    Cipher    : ECDHE-RSA-AES256-SHA
[...]

Spring Boot 2.3.x with reactor-netty 0.9.x is working as expected (handshake is not successfull for the same configuration)

Possible Solution

case TCP in reactor.netty.tcp.SslProvider#updateDefaultConfiguration() should copy the configured ciphers instead of just using null

Your Environment

Spring Boot 2.4.3 with reactor-netty-core 1.0.3 on JDK11

@Johannes-Rost Johannes-Rost added status/need-triage A new issue that still need to be evaluated as a whole type/bug A general bug labels Mar 10, 2021
@Johannes-Rost
Copy link
Author

In case of reactor.netty.tcp.SslProvider.DefaultConfigurationType#H2 the externally configured value for ciphers is also ignored, because it is replaced with reactor.netty.tcp.SslProvider#HTTP2_CIPHERS.
As far as I understand, with http/2 there are only certain allowed ciphers, so the list of ciphers must be restricted if http/2 is to be used.
Nevertheless, it should be possible to use only a subset of the http/2 ciphers. Therefore, simple overriding is not a good solution either.

@violetagg violetagg added for/team-attention This issue needs team attention status/need-decision This needs team attention and discussion and removed status/need-triage A new issue that still need to be evaluated as a whole labels Mar 12, 2021
@violetagg violetagg added this to the 1.0.6 milestone Mar 12, 2021
@violetagg
Copy link
Member

@Johannes-Rost The change in 1.0.x is intentional as we want to guarantee that if the user does not provide configuration type the default configuration will be applied on top of the provided SslContextBuilder.
As a workaround you can try to specify reactor.netty.tcp.SslProvider.DefaultConfigurationType#NONE, or to provide directly SslContext and not SslContextBuilder

@Johannes-Rost
Copy link
Author

Johannes-Rost commented Mar 12, 2021

@violetagg My reactor.netty.http.server.HttpServer is currently assembled by Spring Boot and the NettyReactiveWebServerFactory uses the io.netty.handler.ssl.SslContextBuilder to create the SslContext.
When the web server created by Spring Boot is started, reactor.netty.http.server.HttpServerBind overwrites the cipher configuration with null (or the default ciphers for http/2).
With Spring Boot auto configuration there is no way to work around this behavior, because you can't set the reactor.netty.tcp.SslProvider.DefaultConfiguration and Spring Boot always uses SslContextBuilder.
Therefore, I currently do not know how I could implement your workaround.

I have tried writing a local fix within reactor.netty.tcp.SslProvider#updateDefaultConfiguration() for the problem. However, you can't query the set values at the SslContextBuilder, so you would have to actually build the SslContext to query the values at it. That seemed messy to me.

Alternatively, you could pass the SslContext already present in reactor.netty.tcp.SslProvider into the method as an optional blueprint.

@Johannes-Rost
Copy link
Author

Maybe this should be fixed by changing the SslContextBuilderfrom netty, see #1549 (comment)

@violetagg
Copy link
Member

@Johannes-Rost Reactor Netty will expose a new API that will be used by Spring Boot

@violetagg violetagg removed for/team-attention This issue needs team attention status/need-decision This needs team attention and discussion labels Mar 16, 2021
@violetagg violetagg self-assigned this Mar 16, 2021
violetagg added a commit that referenced this issue Mar 31, 2021
…or any other configuration.

SslProvider.SslContextSpec#sslContext(SslProvider.ProtocolSslContextSpec)
provides, specific for the protocol, default configuration (DefaultSslContextSpec,
TcpSslContextSpec, Http11SslContextSpec, Http2SslContextSpec).
As opposed to SslProvider.SslContextSpec#sslContext(SslContextBuilder),
the default configuration is applied before any other custom configuration.

Fixes #1543
violetagg added a commit that referenced this issue Apr 7, 2021
…or any other configuration (#1573)

SslProvider.SslContextSpec#sslContext(SslProvider.ProtocolSslContextSpec)
provides, specific for the protocol, default configuration (DefaultSslContextSpec,
TcpSslContextSpec, Http11SslContextSpec, Http2SslContextSpec).
As opposed to SslProvider.SslContextSpec#sslContext(SslContextBuilder),
the default configuration is applied before any other custom configuration.

Fixes #1543
@violetagg
Copy link
Member

@bclozel

With version Reactor Netty v1.0.6, can you start using the following API for configuring the security on the server

/**
 * SslContext builder that provides, specific for the protocol, default configuration
 * e.g. {@link DefaultSslContextSpec}, {@link TcpSslContextSpec} etc.
 * As opposed to {@link #sslContext(SslContextBuilder)}, the default configuration is applied before
 * any other custom configuration.
 *
 * @param spec SslContext builder that provides, specific for the protocol, default configuration
 * @return {@literal this}
 * @since 1.0.6
 */
reactor.netty.tcp.SslProvider.SslContextSpec#sslContext(reactor.netty.tcp.SslProvider.ProtocolSslContextSpec)

Examples

HTTP/1.1

Http11SslContextSpec http11SslContextSpec =
        Http11SslContextSpec.forServer(cert, key)
                            .configure(sslContextBuilder -> sslContextBuilder...);

HttpServer.create()
          .secure(spec -> spec.sslContext(http11SslContextSpec))

HTTP/2

Http2SslContextSpec http11SslContextSpec =
        Http2SslContextSpec.forServer(cert, key)
                           .configure(sslContextBuilder -> sslContextBuilder...);

HttpServer.create()
          .secure(spec -> spec.sslContext(http11SslContextSpec))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug A general bug
Projects
None yet
2 participants