[SPARK-37266][SQL] View text can only be SELECT queries #34543

beliefer · 2021-11-10T08:56:19Z

What changes were proposed in this pull request?

The current implementation of persistent view is create hive table with view text.
The view text is just a query string, so the hackers may tamper with it through various means.
Such as, select * from tab1 tampered with drop table tab1.

Why are the changes needed?

First, the view text is query string, parser.parsePlan(viewText) occurs more overhead than parser.parseQuery(viewText).
Second, the view text can be tampered by hackers and issue security vulnerabilities.

Does this PR introduce any user-facing change?

'No'. Unless hackers tamper view text, user will not see any change.

How was this patch tested?

New tests.

…ty vulnerabilities caused by sql tampering

SparkQA · 2021-11-10T09:46:48Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49524/

SparkQA · 2021-11-10T10:31:38Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49524/

SparkQA · 2021-11-10T14:03:59Z

Test build #145054 has finished for PR 34543 at commit dc46ba4.

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

SparkQA · 2021-11-10T14:47:57Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49534/

SparkQA · 2021-11-10T15:30:33Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49534/

SparkQA · 2021-11-10T19:02:03Z

Test build #145064 has finished for PR 34543 at commit b3f898c.

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

beliefer · 2021-11-11T09:46:46Z

ping @MaxGekk @gengliangwang
cc @cloud-fan

cloud-fan · 2021-11-11T12:19:01Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala

+        parser.parseQuery(viewText)
+      } catch {
+        case _: ParseException => throw new AnalysisException(
+          s"Invalid view text of view ${metadata.qualifiedName}, it may have been tampered with")


shall we display the view sql text as wel?

and let's move the error to QueryCompilationErrors.scala

cloud-fan · 2021-11-11T12:19:29Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala

+        parser.parseQuery(viewText)
+      } catch {
+        case _: ParseException => throw new AnalysisException(
+          s"Invalid view text of view ${metadata.qualifiedName}, it may have been tampered with")


and let's move the error to QueryCompilationErrors.scala

cloud-fan · 2021-11-11T12:20:50Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveCommandSuite.scala

+
+  test("SPARK-37266: Optimize the analysis for view text of persistent view and" +
+    " fix security vulnerabilities caused by sql tampering") {
+    val table = hiveContext.sessionState.catalog.getTableMetadata(TableIdentifier("parquet_view1"))


can we create a new view to test it instead of altering an existing view?

SparkQA · 2021-11-11T15:59:07Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49583/

SparkQA · 2021-11-11T16:54:54Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49583/

SparkQA · 2021-11-11T16:57:15Z

Test build #145113 has finished for PR 34543 at commit a51a184.

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

SparkQA · 2021-11-12T02:06:51Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49597/

SparkQA · 2021-11-12T02:48:41Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49597/

cloud-fan · 2021-11-12T03:17:17Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveCommandSuite.scala

+      // Simulate the behavior of hackers
+      val tamperedViewText = "drop view parquet_view2"
+      val tamperedTable = table.copy(viewText = Some(tamperedViewText))
+      hiveContext.sessionState.catalog.alterTable(tamperedTable)


Seems like we can run this test in sql/core, as it does not rely on hive. Can we move it to PersistedViewTestSuite?

cloud-fan · 2021-11-12T03:18:06Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveCommandSuite.scala

+
+  test("SPARK-37266: Optimize the analysis for view text of persistent view and" +
+    " fix security vulnerabilities caused by sql tampering") {
+    sql("CREATE VIEW parquet_view2 as select * from parquet_tab4")


can be simpler CREATE VIEW v AS SELECT 1

SparkQA · 2021-11-12T04:57:39Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49608/

cloud-fan · 2021-11-12T05:24:31Z

sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewTestSuite.scala

+  test("SPARK-37266: Optimize the analysis for view text of persistent view and" +
+    " fix security vulnerabilities caused by sql tampering") {
+    sql("CREATE VIEW v AS SELECT 1")
+    val table = spark.sessionState.catalog.getTableMetadata(TableIdentifier("v"))


let's wrap the test with withView

cloud-fan · 2021-11-12T05:33:33Z

sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewTestSuite.scala

  }
+
+  test("SPARK-37266: Optimize the analysis for view text of persistent view and" +
+    " fix security vulnerabilities caused by sql tampering") {


we can shorten the test name: SPARK-37266: view text can only be SELECT queries

SparkQA · 2021-11-12T05:40:33Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49608/

SparkQA · 2021-11-12T06:31:22Z

Test build #145126 has finished for PR 34543 at commit 81f0ba1.

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

SparkQA · 2021-11-12T08:14:18Z

Test build #145137 has finished for PR 34543 at commit 1d8495c.

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

SparkQA · 2021-11-12T17:01:20Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49639/

SparkQA · 2021-11-12T18:02:50Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49639/

SparkQA · 2021-11-12T21:08:32Z

Test build #145169 has finished for PR 34543 at commit 77cbbdf.

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

SparkQA · 2021-11-13T01:59:41Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49647/

SparkQA · 2021-11-13T02:44:50Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49647/

SparkQA · 2021-11-13T06:12:52Z

Test build #145178 has finished for PR 34543 at commit 0f46831.

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

cloud-fan · 2021-11-15T14:47:32Z

thanks, merging to master!

beliefer · 2021-11-15T16:10:12Z

@cloud-fan Thank you for your review.

Optimize the analysis for view text of persistent view and fix securi…

3d9bb6d

…ty vulnerabilities caused by sql tampering

github-actions bot added the SQL label Nov 10, 2021

beliefer added 2 commits November 10, 2021 16:57

Update code

3808638

Update code

dc46ba4

Update code

b3f898c

cloud-fan reviewed Nov 11, 2021

View reviewed changes

cloud-fan approved these changes Nov 11, 2021

View reviewed changes

cloud-fan reviewed Nov 11, 2021

View reviewed changes

Update code

a51a184

Update code

81f0ba1

cloud-fan reviewed Nov 12, 2021

View reviewed changes

Update code

1d8495c

cloud-fan reviewed Nov 12, 2021

View reviewed changes

Update code

77cbbdf

beliefer changed the title ~~[SPARK-37266][SQL] Optimize the analysis for view text of persistent view and fix security vulnerabilities caused by sql tampering~~ [SPARK-37266][SQL] View text can only be SELECT queries Nov 12, 2021

Update code

0f46831

cloud-fan approved these changes Nov 15, 2021

View reviewed changes

cloud-fan closed this in 6391655 Nov 15, 2021

beliefer mentioned this pull request Dec 9, 2021

[SPARK-37589][SQL][TESTS] Improve the performance of PlanParserSuite #34841

Closed

cfmcgrady mentioned this pull request May 10, 2022

[DO-NOT-MERGE][KYUUBI #2619] Add profile spark-3.3 apache/kyuubi#2620

Closed

3 tasks

[SPARK-37266][SQL] View text can only be SELECT queries #34543

[SPARK-37266][SQL] View text can only be SELECT queries #34543

Uh oh!

Conversation

beliefer commented Nov 10, 2021

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Nov 10, 2021

Uh oh!

SparkQA commented Nov 10, 2021

Uh oh!

SparkQA commented Nov 10, 2021

Uh oh!

SparkQA commented Nov 10, 2021

Uh oh!

SparkQA commented Nov 10, 2021

Uh oh!

SparkQA commented Nov 10, 2021

Uh oh!

beliefer commented Nov 11, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 11, 2021

Uh oh!

SparkQA commented Nov 11, 2021

Uh oh!

SparkQA commented Nov 11, 2021

Uh oh!

SparkQA commented Nov 12, 2021

Uh oh!

SparkQA commented Nov 12, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 12, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 12, 2021

Uh oh!

SparkQA commented Nov 12, 2021

Uh oh!

SparkQA commented Nov 12, 2021

Uh oh!

SparkQA commented Nov 12, 2021

Uh oh!

SparkQA commented Nov 12, 2021

Uh oh!

SparkQA commented Nov 12, 2021

Uh oh!

SparkQA commented Nov 13, 2021

Uh oh!

SparkQA commented Nov 13, 2021

Uh oh!

SparkQA commented Nov 13, 2021

Uh oh!

cloud-fan commented Nov 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

beliefer commented Nov 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

cloud-fan commented Nov 15, 2021 •

edited

Loading