Skip to content

Commit c83611e

Browse files
author
Marcelo Vanzin
committed
[SPARK-23538][core] Remove custom configuration for SSL client.
These options were used to configure the built-in JRE SSL libraries when downloading files from HTTPS servers. But because they were also used to set up the now (long) removed internal HTTPS file server, their default configuration chose convenience over security by having overly lenient settings. This change removes the configuration options that affect the JRE SSL libraries. The JRE trust store can still be configured via system properties (or globally in the JRE security config). The only lost functionality is not being able to disable the default hostname verifier when using spark-submit, which should be fine since Spark itself is not using https for any internal functionality anymore. I also removed the HTTP-related code from the REPL class loader, since we haven't had a HTTP server for REPL-generated classes for a while.
1 parent 3a4d15e commit c83611e

File tree

6 files changed

+7
-223
lines changed

6 files changed

+7
-223
lines changed

core/src/main/scala/org/apache/spark/SecurityManager.scala

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -256,51 +256,6 @@ private[spark] class SecurityManager(
256256
// the default SSL configuration - it will be used by all communication layers unless overwritten
257257
private val defaultSSLOptions = SSLOptions.parse(sparkConf, "spark.ssl", defaults = None)
258258

259-
// SSL configuration for the file server. This is used by Utils.setupSecureURLConnection().
260-
val fileServerSSLOptions = getSSLOptions("fs")
261-
val (sslSocketFactory, hostnameVerifier) = if (fileServerSSLOptions.enabled) {
262-
val trustStoreManagers =
263-
for (trustStore <- fileServerSSLOptions.trustStore) yield {
264-
val input = Files.asByteSource(fileServerSSLOptions.trustStore.get).openStream()
265-
266-
try {
267-
val ks = KeyStore.getInstance(KeyStore.getDefaultType)
268-
ks.load(input, fileServerSSLOptions.trustStorePassword.get.toCharArray)
269-
270-
val tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm)
271-
tmf.init(ks)
272-
tmf.getTrustManagers
273-
} finally {
274-
input.close()
275-
}
276-
}
277-
278-
lazy val credulousTrustStoreManagers = Array({
279-
logWarning("Using 'accept-all' trust manager for SSL connections.")
280-
new X509TrustManager {
281-
override def getAcceptedIssuers: Array[X509Certificate] = null
282-
283-
override def checkClientTrusted(x509Certificates: Array[X509Certificate], s: String) {}
284-
285-
override def checkServerTrusted(x509Certificates: Array[X509Certificate], s: String) {}
286-
}: TrustManager
287-
})
288-
289-
require(fileServerSSLOptions.protocol.isDefined,
290-
"spark.ssl.protocol is required when enabling SSL connections.")
291-
292-
val sslContext = SSLContext.getInstance(fileServerSSLOptions.protocol.get)
293-
sslContext.init(null, trustStoreManagers.getOrElse(credulousTrustStoreManagers), null)
294-
295-
val hostVerifier = new HostnameVerifier {
296-
override def verify(s: String, sslSession: SSLSession): Boolean = true
297-
}
298-
299-
(Some(sslContext.getSocketFactory), Some(hostVerifier))
300-
} else {
301-
(None, None)
302-
}
303-
304259
def getSSLOptions(module: String): SSLOptions = {
305260
val opts = SSLOptions.parse(sparkConf, s"spark.ssl.$module", Some(defaultSSLOptions))
306261
logDebug(s"Created SSL options for $module: $opts")

core/src/main/scala/org/apache/spark/util/Utils.scala

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -673,7 +673,6 @@ private[spark] object Utils extends Logging {
673673
logDebug("fetchFile not using security")
674674
uc = new URL(url).openConnection()
675675
}
676-
Utils.setupSecureURLConnection(uc, securityMgr)
677676

678677
val timeoutMs =
679678
conf.getTimeAsSeconds("spark.files.fetchTimeout", "60s").toInt * 1000
@@ -2363,20 +2362,6 @@ private[spark] object Utils extends Logging {
23632362
PropertyConfigurator.configure(pro)
23642363
}
23652364

2366-
/**
2367-
* If the given URL connection is HttpsURLConnection, it sets the SSL socket factory and
2368-
* the host verifier from the given security manager.
2369-
*/
2370-
def setupSecureURLConnection(urlConnection: URLConnection, sm: SecurityManager): URLConnection = {
2371-
urlConnection match {
2372-
case https: HttpsURLConnection =>
2373-
sm.sslSocketFactory.foreach(https.setSSLSocketFactory)
2374-
sm.hostnameVerifier.foreach(https.setHostnameVerifier)
2375-
https
2376-
case connection => connection
2377-
}
2378-
}
2379-
23802365
def invoke(
23812366
clazz: Class[_],
23822367
obj: AnyRef,

core/src/test/scala/org/apache/spark/SSLSampleConfigs.scala

Lines changed: 0 additions & 68 deletions
This file was deleted.

core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -370,51 +370,6 @@ class SecurityManagerSuite extends SparkFunSuite with ResetSystemProperties {
370370
assert(securityManager.checkModifyPermissions("user1") === false)
371371
}
372372

373-
test("ssl on setup") {
374-
val conf = SSLSampleConfigs.sparkSSLConfig()
375-
val expectedAlgorithms = Set(
376-
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384",
377-
"TLS_RSA_WITH_AES_256_CBC_SHA256",
378-
"TLS_DHE_RSA_WITH_AES_256_CBC_SHA256",
379-
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
380-
"TLS_DHE_RSA_WITH_AES_128_CBC_SHA256",
381-
"SSL_ECDHE_RSA_WITH_AES_256_CBC_SHA384",
382-
"SSL_RSA_WITH_AES_256_CBC_SHA256",
383-
"SSL_DHE_RSA_WITH_AES_256_CBC_SHA256",
384-
"SSL_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
385-
"SSL_DHE_RSA_WITH_AES_128_CBC_SHA256")
386-
387-
val securityManager = new SecurityManager(conf)
388-
389-
assert(securityManager.fileServerSSLOptions.enabled === true)
390-
391-
assert(securityManager.sslSocketFactory.isDefined === true)
392-
assert(securityManager.hostnameVerifier.isDefined === true)
393-
394-
assert(securityManager.fileServerSSLOptions.trustStore.isDefined === true)
395-
assert(securityManager.fileServerSSLOptions.trustStore.get.getName === "truststore")
396-
assert(securityManager.fileServerSSLOptions.keyStore.isDefined === true)
397-
assert(securityManager.fileServerSSLOptions.keyStore.get.getName === "keystore")
398-
assert(securityManager.fileServerSSLOptions.trustStorePassword === Some("password"))
399-
assert(securityManager.fileServerSSLOptions.keyStorePassword === Some("password"))
400-
assert(securityManager.fileServerSSLOptions.keyPassword === Some("password"))
401-
assert(securityManager.fileServerSSLOptions.protocol === Some("TLSv1.2"))
402-
assert(securityManager.fileServerSSLOptions.enabledAlgorithms === expectedAlgorithms)
403-
}
404-
405-
test("ssl off setup") {
406-
val file = File.createTempFile("SSLOptionsSuite", "conf", Utils.createTempDir())
407-
408-
System.setProperty("spark.ssl.configFile", file.getAbsolutePath)
409-
val conf = new SparkConf()
410-
411-
val securityManager = new SecurityManager(conf)
412-
413-
assert(securityManager.fileServerSSLOptions.enabled === false)
414-
assert(securityManager.sslSocketFactory.isDefined === false)
415-
assert(securityManager.hostnameVerifier.isDefined === false)
416-
}
417-
418373
test("missing secret authentication key") {
419374
val conf = new SparkConf().set("spark.authenticate", "true")
420375
val mgr = new SecurityManager(conf)

docs/security.md

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,6 @@ component-specific configuration namespaces used to override the default setting
4444
<th>Config Namespace</th>
4545
<th>Component</th>
4646
</tr>
47-
<tr>
48-
<td><code>spark.ssl.fs</code></td>
49-
<td>File download client (used to download jars and files from HTTPS-enabled servers).</td>
50-
</tr>
5147
<tr>
5248
<td><code>spark.ssl.ui</code></td>
5349
<td>Spark application Web UI</td>

repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala

Lines changed: 7 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,24 @@
1717

1818
package org.apache.spark.repl
1919

20-
import java.io.{ByteArrayOutputStream, FileNotFoundException, FilterInputStream, InputStream, IOException}
21-
import java.net.{HttpURLConnection, URI, URL, URLEncoder}
20+
import java.io.{ByteArrayOutputStream, FileNotFoundException, FilterInputStream, InputStream}
21+
import java.net.{URI, URL, URLEncoder}
2222
import java.nio.channels.Channels
2323

24-
import scala.util.control.NonFatal
25-
2624
import org.apache.hadoop.fs.{FileSystem, Path}
2725
import org.apache.xbean.asm5._
2826
import org.apache.xbean.asm5.Opcodes._
2927

3028
import org.apache.spark.{SparkConf, SparkEnv}
3129
import org.apache.spark.deploy.SparkHadoopUtil
3230
import org.apache.spark.internal.Logging
33-
import org.apache.spark.util.{ParentClassLoader, Utils}
31+
import org.apache.spark.util.ParentClassLoader
3432

3533
/**
36-
* A ClassLoader that reads classes from a Hadoop FileSystem or HTTP URI, used to load classes
37-
* defined by the interpreter when the REPL is used. Allows the user to specify if user class path
38-
* should be first. This class loader delegates getting/finding resources to parent loader, which
39-
* makes sense until REPL never provide resource dynamically.
34+
* A ClassLoader that reads classes from a Hadoop FileSystem or Spark RPC endpoint, used to load
35+
* classes defined by the interpreter when the REPL is used. Allows the user to specify if user
36+
* class path should be first. This class loader delegates getting/finding resources to parent
37+
* loader, which makes sense until REPL never provide resource dynamically.
4038
*
4139
* Note: [[ClassLoader]] will preferentially load class from parent. Only when parent is null or
4240
* the load failed, that it will call the overridden `findClass` function. To avoid the potential
@@ -60,7 +58,6 @@ class ExecutorClassLoader(
6058

6159
private val fetchFn: (String) => InputStream = uri.getScheme() match {
6260
case "spark" => getClassFileInputStreamFromSparkRPC
63-
case "http" | "https" | "ftp" => getClassFileInputStreamFromHttpServer
6461
case _ =>
6562
val fileSystem = FileSystem.get(uri, SparkHadoopUtil.get.newConfiguration(conf))
6663
getClassFileInputStreamFromFileSystem(fileSystem)
@@ -113,42 +110,6 @@ class ExecutorClassLoader(
113110
}
114111
}
115112

116-
private def getClassFileInputStreamFromHttpServer(pathInDirectory: String): InputStream = {
117-
val url = if (SparkEnv.get.securityManager.isAuthenticationEnabled()) {
118-
val uri = new URI(classUri + "/" + urlEncode(pathInDirectory))
119-
val newuri = Utils.constructURIForAuthentication(uri, SparkEnv.get.securityManager)
120-
newuri.toURL
121-
} else {
122-
new URL(classUri + "/" + urlEncode(pathInDirectory))
123-
}
124-
val connection: HttpURLConnection = Utils.setupSecureURLConnection(url.openConnection(),
125-
SparkEnv.get.securityManager).asInstanceOf[HttpURLConnection]
126-
// Set the connection timeouts (for testing purposes)
127-
if (httpUrlConnectionTimeoutMillis != -1) {
128-
connection.setConnectTimeout(httpUrlConnectionTimeoutMillis)
129-
connection.setReadTimeout(httpUrlConnectionTimeoutMillis)
130-
}
131-
connection.connect()
132-
try {
133-
if (connection.getResponseCode != 200) {
134-
// Close the error stream so that the connection is eligible for re-use
135-
try {
136-
connection.getErrorStream.close()
137-
} catch {
138-
case ioe: IOException =>
139-
logError("Exception while closing error stream", ioe)
140-
}
141-
throw new ClassNotFoundException(s"Class file not found at URL $url")
142-
} else {
143-
connection.getInputStream
144-
}
145-
} catch {
146-
case NonFatal(e) if !e.isInstanceOf[ClassNotFoundException] =>
147-
connection.disconnect()
148-
throw e
149-
}
150-
}
151-
152113
private def getClassFileInputStreamFromFileSystem(fileSystem: FileSystem)(
153114
pathInDirectory: String): InputStream = {
154115
val path = new Path(directory, pathInDirectory)

0 commit comments

Comments
 (0)