Conflicting rules for vulnerable use of Java's SSLContext

Clean Code Report False-positive / False-negative...
,
1

Sonarqube Version 6.7.5 (build 38563) (community edition I think) has contradicting rules:
https://t.co/OMXZ34eSje from “FindBugs Security Audit” and https://t.co/OMXZ34eSje from “Sonar way”.

SSLContext.getInstance("TLS"); is ok for the former, but not the latter.
The latter is wrong BTW.

RSPEC-4423 is misleading. Selection of SSLContext does not determine the protocol version that is negotiated. In JDK8 the SSLContext “TLS” is an alias for “TLSv1.2”. The only way to constrain the negotiated protocol version is to set the enabled cipher suites via the SSLEngine.

As an example:

    clientSslContext = SSLContext.getInstance("TLSv1.2");
    serverSslContext = SSLContext.getInstance("TLS");
    serverEngine = serverSslContext.createSSLEngine();
    serverEngine.setEnabledProtocols(new String[] { "TLSv1" });

Given the above client/server SSL configs, the client will send a TLSv1.2 hello message. The server will respond with a TLSv1.0 hello, and the two will end up using TLSv1.0 along with its weaker cipher suites.

Changing the client to use SSLContext.getInstance(“TLS”) has no impact on the handshake, since “TLS” is an alias for “TLSv1.2” in JDK8.

This rule is actually quite dangerous because the suggested fix leads to a false sense of increased security.

2

Hi Jean Noël,

As said on Twitter, thank you for your feedback, and thank you for reposting it here.

I agree that the java version of RSPEC-4423 needs to be improved. As JDK 11 will add support for TLSv1.3 it is more future-proof to use SSLContext.getInstance("TLS"), thus it will be recommended. However the rule will still enable the use of precise secure versions (i.e. DTLSv1.2, TLSv1.2 and TLSv1.3) as they are valid and forbidding it would create unnecessary issues. The rule will just explain that it doesn’t prevent protocol version negotiation.

I will also add in the rule a few examples showing how to set “enabled protocols” on SSLEngine, SSLSocket and SSLServerSocket.

Please note that the second rule you mentioned, i.e. the one from “FindBugs Security Audit”, comes from the Sonar-Findbugs plugin. It is not maintained by SonarSource. There can be duplicates and differences between its rules and the ones defined in the SonarJava plugin (maintained by SonarSource).

Don’t hesitate to share additional comments regarding this rule or any other.

Best regards

3

Thanks for all of your answers.