[SPARK-7756] [CORE] More robust SSL options processing.

[tellison]

Subset the enabled algorithms in an SSLOptions to the elements that are supported by the protocol provider.

Update the list of ciphers in the sample config to include modern algorithms, and specify both Oracle and IBM names. In practice the user would either specify their own chosen cipher suites, or specify none, and delegate the decision to the provider.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[SparkQA]

Test build #35858 has started for PR 7043 at commit 4b5c89f.

[srowen]

I find this confusing but maybe I'm missing something. You're allocating a default here to fall back to, only if there is an exception in the try block? Is that clearer to set in the exception handling path then? and can this ever be None, so, why Option?

[tellison]

All you are missing is my inexperience in Scala! So how about something like this?

    var context: SSLContext = null
    try {
      context = SSLContext.getInstance(protocol.orNull)
      context.init(null, null, null)
    } catch {
      case npe: NullPointerException =>
        logDebug("No SSL protocol specified")
        context = SSLContext.getDefault
      case nsa: NoSuchAlgorithmException =>
        logDebug(s"No support for requested SSL protocol ${protocol.get}")
        context = SSLContext.getDefault
    }

    val providerAlgorithms = context.getServerSocketFactory.getSupportedCipherSuites.toSet

[srowen]

I think that's more straightforward yes. When do you get the NPE? is it more straightforward to just check if protocol is set rather than turn it into null and then wait for something to barf? or is that not the logic.

So the change here is mostly to remove unsupported algos from the list of enabled ones? that makes sense

[tellison]

I have removed the use of Option, but code to check if protocol is empty would be bulky IMHO. Go ahead and change it as you see fit.

Yes, the change is to only use supported algorithms. This is logic already done by Jetty (see SslContextFactory.java lines 1212 on) so is arguably unnecessary for the Spark HttpServer, but there is no such logic in Netty and specifying unsupported ciphers causes the handshake failure via Akka etc..

As I mentioned earlier, use of explicit protocols and cipher suites is subject to change as the definition of 'secure' changes. SSLv3 is widely deprecated since POODLE, so not a good choice for the sample config. Restricting the protocol to use specific ciphers can ensure you only use secure algorithms, but typically Java vendors are constantly amending the default list in light of known vulnerabilities, and will update the negotiation set accordingly. Since Spark's SSLSampleConfigs are explicit, they should be kept under review too.

[SparkQA]

Test build #35858 has finished for PR 7043 at commit 4b5c89f.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Merged build finished. Test PASSed.

[srowen]

Nit but I think the java imports should be grouped and ordered by package

[tellison]

Fixed

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[SparkQA]

Test build #35938 has started for PR 7043 at commit 3797f8b.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[SparkQA]

Test build #35939 has started for PR 7043 at commit 034efa5.

[SparkQA]

Test build #35938 has finished for PR 7043 at commit 3797f8b.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Merged build finished. Test FAILed.

[SparkQA]

Test build #35939 has finished for PR 7043 at commit 034efa5.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Merged build finished. Test PASSed.