[SPARK-5682][Core] Add encrypted shuffle in spark by winningsix · Pull Request #8880 · apache/spark

winningsix · 2015-09-23T07:05:21Z

This patch is using Apache Commons Crypto library to enable shuffle encryption support.

vanzin · 2015-09-23T23:49:59Z

ok to test

SparkQA · 2015-09-23T23:58:43Z

Test build #42941 has finished for PR 8880 at commit 4a95c73.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- abstract class AesCtrCryptoCodec extends CryptoCodec
- case class CipherSuite(name: String, algoBlockSize: Int)
- abstract case class CryptoCodec()
- class CryptoInputStream(in: InputStream, codecVal: CryptoCodec,
- class CryptoOutputStream(out: OutputStream, codecVal: CryptoCodec, bufferSizeVal: Int,
- trait Decryptor
- trait Encryptor
- class JceAesCtrCryptoCodec(conf: SparkConf) extends AesCtrCryptoCodec with Logging
- class JceAesCtrCipher(mode: Int, provider: String) extends Encryptor with Decryptor

rxin · 2015-09-24T01:10:07Z

Can you please add some high level documentation about this change?

winningsix · 2015-09-24T03:11:13Z

@rxin, the design document is available in https://issues.apache.org/jira/secure/attachment/12730704/Design%20Document%20of%20Encrypted%20Spark%20Shuffle_20150506.docx which is an attachment of SPARK-5682. This PR is trying to add the basic shuffle encryption workflow and JCE key provider support. Next step we will add openssl crypto codec to improve the encryption performance (~17x expected with AES-NI enabled).

SparkQA · 2015-09-24T06:56:34Z

Test build #42953 timed out for PR 8880 at commit 67b391e after a configured wait of 250m.

vanzin · 2015-09-28T19:41:42Z

no need to import Byte

SparkQA · 2016-08-25T10:12:53Z

Test build #64408 has finished for PR 8880 at commit 9f958a4.

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

vanzin · 2016-08-25T18:25:02Z

 if (master == "yarn" && deployMode == "client") System.setProperty("SPARK_YARN_MODE", "true")
+ if (_conf.get(IO_ENCRYPTION_ENABLED) && !SparkHadoopUtil.get.isYarnMode()) {
+ throw new SparkException("IO encryption is only supported in YARN mode, please disable it " +
+ "by setting spark.io.encryption.enabled to false")


nit: use ${IO_ENCRYPTION_ENABLED.key} instead of the hardcoded key name.

zsxwing · 2016-08-25T20:30:11Z

+ def toCryptoConf(
+ conf: SparkConf,
+ sparkPrefix: String,
+ cryptoPrefix: String): Properties = {


nit: you don't need sparkPrefix and cryptoPrefix any more.

zsxwing · 2016-08-25T21:45:16Z

Looks pretty overall. Just a high level question: Why needs to generate a new key for IO encryption? Can we just use SecurityManager.getSecretKey?

vanzin · 2016-08-25T21:53:23Z

Why needs to generate a new key for IO encryption?

They could be different sizes (different config options control that). We could change it so both use the same configs / same code to generate the keys, but in general if they're used for different things I prefer to have different configs, at least.

zsxwing · 2016-08-25T21:55:03Z

+ <td><code>spark.io.encryption.enabled</code></td>
+ <td>false</td>
+ <td>
+ Enable IO encryption.


Please say it only supports Yarn mode here.

zsxwing · 2016-08-25T22:10:18Z

They could be different sizes (different config options control that). We could change it so both use the same configs / same code to generate the keys, but in general if they're used for different things I prefer to have different configs, at least.

Sounds good to me

SparkQA · 2016-08-26T05:34:01Z

Test build #64451 has finished for PR 8880 at commit a9a05c5.

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

vanzin · 2016-08-26T17:02:06Z

+ <td><code>spark.io.encryption.enabled</code></td>
+ <td>false</td>
+ <td>
+ Enable IO encryption. It only supports YARN mode.


"Only supported in YARN mode."

vanzin · 2016-08-29T18:18:30Z

+
+ val encryptedBytes = outputStream.toByteArray
+ val encryptedStr = new String(encryptedBytes)
+ assert (plainStr !== encryptedStr)


nit: no space before (

vanzin · 2016-08-29T18:19:38Z

LGTM. Any remaining comments @zsxwing ?

SparkQA · 2016-08-29T19:40:03Z

Test build #64582 has finished for PR 8880 at commit a811bf4.

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

zsxwing · 2016-08-30T00:01:21Z

LGTM

SparkQA · 2016-08-30T04:10:54Z

Test build #64614 has finished for PR 8880 at commit 928a59b.

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

vanzin · 2016-08-30T16:15:04Z

Merging to master.

This patch is using Apache Commons Crypto library to enable shuffle encryption support. Author: Ferdinand Xu <cheng.a.xu@intel.com> Author: kellyzly <kellyzly@126.com> Closesapache#8880 from winningsix/SPARK-10771.

winningsix mentioned this pull request Sep 23, 2015
[Spark-5682] Add spark encrypted shuffle by using chimera lib #5307
Closed

winningsix force-pushed the SPARK-10771 branch from 4a95c73 to 67b391eCompare September 24, 2015 02:40

vanzin reviewed Sep 28, 2015
View reviewed changes

Comment threadcore/src/main/scala/org/apache/spark/crypto/AesCtrCryptoCodec.scala Outdated
vanzinSep 28, 2015
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to import Byte

vanzin reviewed Aug 25, 2016
View reviewed changes

Fix some further comments
a9a05c5

zsxwing reviewed Aug 25, 2016
View reviewed changes

vanzin reviewed Aug 26, 2016
View reviewed changes

Ferdinand Xu added 2 commits August 29, 2016 08:48

Update test cases
a811bf4

Address further comments
928a59b

vanzin reviewed Aug 29, 2016
View reviewed changes

asfgit closed this in 4b4e329Aug 30, 2016

Uh oh!

Conversation

winningsix commented Sep 23, 2015• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vanzin commented Sep 23, 2015

Uh oh!

SparkQA commented Sep 23, 2015

Uh oh!

rxin commented Sep 24, 2015

Uh oh!

winningsix commented Sep 24, 2015

Uh oh!

SparkQA commented Sep 24, 2015

Uh oh!

vanzinSep 28, 2015

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 25, 2016

Uh oh!

vanzinAug 25, 2016

Choose a reason for hiding this comment

Uh oh!

zsxwingAug 25, 2016

Choose a reason for hiding this comment

Uh oh!

zsxwing commented Aug 25, 2016

Uh oh!

vanzin commented Aug 25, 2016

Uh oh!

zsxwingAug 25, 2016

Choose a reason for hiding this comment

Uh oh!

zsxwing commented Aug 25, 2016

Uh oh!

SparkQA commented Aug 26, 2016

Uh oh!

vanzinAug 26, 2016

Choose a reason for hiding this comment

Uh oh!

vanzinAug 29, 2016

Choose a reason for hiding this comment

Uh oh!

vanzin commented Aug 29, 2016

Uh oh!

SparkQA commented Aug 29, 2016

Uh oh!

zsxwing commented Aug 30, 2016

Uh oh!

SparkQA commented Aug 30, 2016

Uh oh!

vanzin commented Aug 30, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

winningsix commented Sep 23, 2015•
edited
Loading