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

[netty-handler] Certificate Validation #11213

Open
emirot opened this issue Apr 30, 2021 · 18 comments
Open

[netty-handler] Certificate Validation #11213

emirot opened this issue Apr 30, 2021 · 18 comments

Comments

@emirot
Copy link

emirot commented Apr 30, 2021

I'm posting this here if it's not the right place I apologize in advance, it looks to be known an open issue for some time.
I tried(https://groups.google.com/d/forum/netty-security) , but got "Content unavailable please retry."
https://snyk.io/vuln/SNYK-JAVA-IONETTY-1042268
Is this vulnerability still ongoing or was it fixed in netty-4.1.63.Final? I cannot find references elsewhere than Snyk.
Will this be solved by: #11210 ?

@hyperxpro
Copy link
Contributor

Duplicate of #9930

@emirot
Copy link
Author

emirot commented Apr 30, 2021

Is there a way to mitigate that? I've seen this #8537 (comment) but is there a recommended way?

@hyperxpro
Copy link
Contributor

Well, using Java default Truststore is the only recommended way. You can follow those comment instructions.

@emirot
Copy link
Author

emirot commented Apr 30, 2021

Is this something that will be fixed soon in this project?

@hyperxpro
Copy link
Contributor

Certificate Validation will be turned on by default in Netty 5. Netty 4.1 will continue to have Certificate Validation disabled.

@emirot
Copy link
Author

emirot commented Apr 30, 2021

Great, thank you! when is the approximate timeline for Netty 5

@hyperxpro
Copy link
Contributor

Netty 5 release is not decided yet. Maybe next year (2022)? Can't say.

@emirot
Copy link
Author

emirot commented Apr 30, 2021

Ok thank you a lot, truly appreciate your answers

@emirot
Copy link
Author

emirot commented Apr 30, 2021

By reading through more threads I've seen #8537 (comment) maybe I'm not even concerned by that issue?

@nmck257
Copy link

nmck257 commented Nov 22, 2021

Certificate Validation will be turned on by default in Netty 5. Netty 4.1 will continue to have Certificate Validation disabled.

@hyperxpro - now that Netty 5 isn't planned (#4466), do you see any path to a future release with cert validation enabled by default?

Clients can put that config in themselves, but it seems like a sensible default for the uninformed, and closing the Snyk vulnerability would be a big boon for enterprise users.

@hyperxpro
Copy link
Contributor

Netty 5 is coming. An alpha release is expected to be released around February 2022 [1].

Certificate Validation will be turned on by default in Netty 5 [2].

1: https://github.com/netty/netty/projects/1
2: #8537

Try this for enabling Certificate Validation in Netty 4.1.

            TrustManagerFactory trustManagerFactory;
            if (acceptAllCerts) {
                trustManagerFactory = InsecureTrustManagerFactory.INSTANCE;
            } else {
                try {
                    trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
                    trustManagerFactory.init((KeyStore) null);
                } catch (Exception ex) {
                    throw new IllegalArgumentException("Error occurred while building TrustManagerFactory", ex);
                }
            }

            SslContextBuilder sslContextBuilder = SslContextBuilder.forClient()
                    .sslProvider(OpenSsl.isAvailable() ? SslProvider.OPENSSL : SslProvider.JDK)
                    .protocols(Protocol.getProtocols(protocols))
                    .clientAuth(mutualTLS.clientAuth())
                    .trustManager(trustManagerFactory)
                    .startTls(useStartTLS)
                    .applicationProtocolConfig(new ApplicationProtocolConfig(
                            ApplicationProtocolConfig.Protocol.ALPN,
                            ApplicationProtocolConfig.SelectorFailureBehavior.NO_ADVERTISE,
                            ApplicationProtocolConfig.SelectedListenerFailureBehavior.ACCEPT,
                            ApplicationProtocolNames.HTTP_2,
                            ApplicationProtocolNames.HTTP_1_1));

@Mango2020
Copy link

@hyperxpro is Netty 5 released now? Thanks.

@hyperxpro
Copy link
Contributor

@hyperxpro is Netty 5 released now? Thanks.

Netty 5 Alpha is out but TLS certificate validation is not enabled yet.

I'll do PR for it in next few days for this feature so next in Alpha release, we'll have this feature.

@Mango2020
Copy link

@hyperxpro Thanks for your rapid response.
As we need a list of tasks to associate with the vulnerability issue fix, can you share the target date for the release of Netty 5? Thanks in advance.

@hyperxpro
Copy link
Contributor

@hyperxpro Thanks for your rapid response. As we need a list of tasks to associate with the vulnerability issue fix, can you share the target date for the release of Netty 5? Thanks in advance.

There are no absolute dates but maybe in Q1 of 2023.

@DanS006
Copy link

DanS006 commented Apr 4, 2023

Note, via:
https://netty.io/downloads.html
4.1.x is "supported" and trips:
https://security.snyk.io/vuln/SNYK-JAVA-IONETTY-1042268
And AWS makes
https://mvnrepository.com/artifact/software.amazon.awssdk/secretsmanager
which pulls in the "stable" version, and provides:
https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/http-configuration-netty.html

It's very messy to have a secrets manager needing special code to mitigate a non ideal default configuration option, that trips a vulnerability in a scanning tool even after the default configuration changes.

Given Q1 2023 is rapidly approaching, any news on when a 5.x with a good default will reach "stable" status so the awssdk developers can have a good default?

@slandelle
Copy link
Contributor

IMO, this SNYK vulnerability ticket is wrong.

Netty is a low level IO framework, brick and mortar. It's not an HTTP client.
Why report this as a Netty vulnerability? Why not report it against javax.net.ssl.SSLEngine instead, as it's where hostname verification is not turned on by default?

IMO, it's the responsibility of the implementors of any Netty-based HTTP client, including AWS SDK's NettyNioAsyncHttpClient, to properly force hostname verification, as documented in Netty's configuration.

@DanS006
Copy link

DanS006 commented Apr 4, 2023

Changing the 5.x default seems to indicate that providing default secure configuration is the right thing to do going forward.

Any information on when 5.x will be stable, since that has been the solution to fixing the concern for a few years?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants