[SPARK-10589] [WEBUI] Add defense against external site framing

[srowen]

Set X-Frame-Options: SAMEORIGIN to protect against frame-related vulnerability

[SparkQA]

Test build #42425 has finished for PR 8745 at commit 70da85b.

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

[holdenk]

This seems like it could cause issues for manager type programs that want to subframe the Spark UI (e.g. databricks cloud & microsofts hosted spark solution), maybe make it configurable?

[srowen]

Hm, that's a good point -- would they be on the same domain though? I think this setting allows that. There is another value it can take on to allow framing from a specific other domain. That could be configurable ... though I'd love to avoid another config. I am not sure how common this attack is though I hesitate to ignore it too.

[holdenk]

I'm definitively not a front-end security person so I'm not sure how common/bad an attack like this could be. Adding a config to allow the framing from a specific other domain seems like it would solve the problem. I'm not super sure if the framing is done on the same domain or different domain (no longer have an account either of those systems to check).

[srowen]

I can slap in a config option here in any event, and leave it undocumented (?) for now to both allow for adjusting it but not propagate the little extra complexity for everyone else.

[falaki]

+1 for making it configurable.

[srowen]

@falaki @holdenk what do you think about this? I'm not super excited about plumbing through SparkConf several methods but it's not crazy. The default is SAMEORIGIN but can be configured to ALLOW-FROM [uri].

Another option is to have this off by default, which is most compatible. I'm on the fence; it's a legitimate issue, but for the Spark UIs, the worst case is ... someone tricking you into killing your jobs? Trying to figure out what the right default is.

[SparkQA]

Test build #42482 has finished for PR 8745 at commit 530f1a9.

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

[JoshRosen]

@holdenk @falaki @ahirreddy, this should not be an issue for DBC since the framed UI should already be served from the same origin.

[srowen]

I'm pretty on the fence on the kind-of-abstract security issue vs possibly inconveniencing some deployments. Certainly it's possible to configure it to still work with framing, securely, after this change. If there's no clear use case that this would break, I would propose proceeding with this to close the possible security issue, for 1.6.

[rxin]

LGTM.

[alexmnyc]

Now I am not able to embed it on my grafana dashboard... That should be a configuration parameter