[SPARK-13599] [BUILD] remove transitive groovy dependencies from Hive

[steveloughran]

What changes were proposed in this pull request?

Modifies the dependency declarations of the all the hive artifacts, to explicitly exclude the groovy-all JAR.

This stops the groovy classes and everything else in that uber-JAR from getting into spark-assembly JAR.

How was this patch tested?

1. Pre-patch build was made: mvn clean install -Pyarn,hive,hive-thriftserver
2. spark-assembly expanded, observed to have the org.codehaus.groovy packages and JARs
3. A maven dependency tree was created mvn dependency:tree -Pyarn,hive,hive-thriftserver -Dverbose > target/dependencies.txt
4. This text file examined to confirm that groovy was being imported as a dependency of org.spark-project.hive
5. Patch applied
6. Repeated step1: clean build of project with -Pyarn,hive,hive-thriftserver set
7. Examined created spark-assembly, verified no org.codehaus packages
8. Verified that the maven dependency tree no longer references groovy

Note also that the size of the assembly JAR was 181628646 bytes before this patch, 166318515 after —15MB smaller. That's a good metric of things being excluded

[SparkQA]

Test build #52249 has finished for PR 11449 at commit bc120b4.

• This patch fails build dependency tests.
• This patch merges cleanly.
• This patch adds no public classes.

[JoshRosen]

Mind adding a Maven Enforcer rule so that the build will fail in case this is ever reintroduced?

[JoshRosen]

Enforcer rules are also a nice chance to document why something is being excluded, etc.

[SparkQA]

Test build #52250 has finished for PR 11449 at commit 7bedeec.

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

[steveloughran]

...let me work out how to do enforcer rules

[srowen]

OK by me. I'm still trying to work out whether it belongs in 1.6.2 or not -- thoughts?

[steveloughran]

+1 for 1.6.x.

W.r.t 1.6.2, it'll keep the tar smaller, maybe even load faster. And eliminate the risk of a CVE.

if you set spark.authenticate=true untrusted callers can't submit malicious object streams via kryo, so there's less vulnerability

[SparkQA]

Test build #52320 has finished for PR 11449 at commit 8b6f422.

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

[steveloughran]

Added PR #11473 to cover only the <exclude> bit of the patch, as that is all that applies to 1.6.x

[rxin]

Why would we want to backport this into branch-1.6? We rarely update dependencies, unless there is a security problem.

[srowen]

Yeah, I'm on the fence; it seems low risk if it's really not used, but that also makes it low priority. Steve you're saying there's a potential security risk in Groovy -- is it purely a potential one, or do you have reason to believe there's an actual risk? You mentioned you definitely wanted to back port so I suspected there was a bit more motive in there

[steveloughran]

The risk is deserialization; Groovy CVE-2015-3253 shows how groovy < 2.4.4 makes it straightforward to use a class in Groovy to run arbitrary shell commands on the destination. This has been show on Java ObjectStream and XStream, so assume Kryo is vulnerable too.

[steveloughran]

I should that as well as the org.codehaus.groovy package, there's various shaded things in groovy/ and an unshaded copy of antlr. This may create versioning problems with the antlr.jar pulled in by spark-catalyst, though it'd depend on which version of the antlr classes got pulled in and on antlr's compatibility story.

[steveloughran]

FWIW, We're backporting it in-house.Without it, downstream applications which try to add a secure groovy to their jobs won't know which version is picked up.

[steveloughran]

Groovy and Xstream attack. Assume that you can do the same in Kryo, it just takes someone to sit down and do the work.

[rxin]

OK thanks for the information.

[rxin]

Merging this in master.

[steveloughran]

thx