You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@spark.apache.org by krishna-pandey <gi...@git.apache.org> on 2017/10/03 13:23:03 UTC

[GitHub] spark pull request #19419: Adding security headers for preventing XSS, MitM ...

GitHub user krishna-pandey opened a pull request:

    https://github.com/apache/spark/pull/19419

    Adding security headers for preventing XSS, MitM and MIME sniffing

    ## What changes were proposed in this pull request?
    
    The HTTP Strict-Transport-Security response header (often abbreviated as HSTS) is a security feature that lets a web site tell browsers that it should only be communicated with using HTTPS, instead of using HTTP.
    
    Note: The Strict-Transport-Security header is ignored by the browser when your site is accessed using HTTP; this is because an attacker may intercept HTTP connections and inject the header or remove it. When your site is accessed over HTTPS with no certificate errors, the browser knows your site is HTTPS capable and will honor the Strict-Transport-Security header.
    
    The HTTP X-XSS-Protection response header is a feature of Internet Explorer, Chrome and Safari that stops pages from loading when they detect reflected cross-site scripting (XSS) attacks.
    
    The HTTP X-Content-Type-Options response header is used to protect against MIME sniffing vulnerabilities.
    
    ## How was this patch tested?
    Checked on my system locally.
    
    <img width="750" alt="screen shot 2017-10-03 at 6 49 20 pm" src="https://user-images.githubusercontent.com/6433184/31127234-eadf7c0c-a86b-11e7-8e5d-f6ea3f97b210.png">
    
    
    Please review http://spark.apache.org/contributing.html before opening a pull request.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/krishna-pandey/spark SPARK-22188

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/19419.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #19419
    
----
commit abb081df1f93fea38b611bcbfe563606783420fd
Author: krishna-pandey <kr...@gmail.com>
Date:   2017-10-03T13:13:27Z

    Adding security headers for preventing XSS, MitM and MIME sniffing

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by tgravescs <gi...@git.apache.org>.

Github user tgravescs commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    overall I think the headers are fine, more security the better.  I'm not exactly sure the attack vector with the spark UI though.  Normally I would expect your UI to be on a corporate network and you vpn in, but I guess maybe if you are running in AWS or similar public cloud and you go somewhere to access, but I'm not sure what data they can get there or why you would be using http in the first place but there are lots of setups.  
    
    @krishna-pandey  do you have specific use case/attack vector in mind here?
    
    I was wondering if there was a more generic way to allow user to specify desired headers without having a config for each one. Downside to that is its not as obvious though too so I need to think about that a bit more.



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82795 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82795/testReport)** for PR 19419 at commit [`acfd227`](https://github.com/apache/spark/commit/acfd227a550fd4e40fd74001e1b48880fdea60bc).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82878 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82878/testReport)** for PR 19419 at commit [`b6d4885`](https://github.com/apache/spark/commit/b6d4885e9ad9a03a40b3c28df41d7b263b89369f).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    The downside of additional header traffic is trivial. I suppose it is not needed in most contexts so it was conservative to leave it off by default. That said I am not aware of any particular negative to enabling it. I would be OK with leaving it on by default if there's no particular downside. 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by asfgit <gi...@git.apache.org>.

Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/19419


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144204531
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -25,3 +25,10 @@
     # spark.serializer                 org.apache.spark.serializer.KryoSerializer
     # spark.driver.memory              5g
     # spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +# spark.ui.allowFramingFrom         https://www.example.com/
    +# spark.ui.xXssProtection           1; mode=block
    +# spark.ui.xContentType.options     nosniff
    +
    +# Enable below only when Spark is running on HTTPS
    +# spark.ui.strictTransportSecurity  max-age=31536000
    --- End diff --
    
    The REQUIRED "max-age" directive specifies the number of seconds, after the reception of the STS header field, during which the UA regards the host (from whom the message was received) as a Known HSTS Host. Here the value is equal to 365 days. More at https://tools.ietf.org/html/rfc6797#section-6.1.1


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    @vanzin @tgravescs @ajbozarth  what is your opinion on this PR? Is it a necessary fix for Spark? 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82807 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82807/testReport)** for PR 19419 at commit [`1cc34e9`](https://github.com/apache/spark/commit/1cc34e912763e92d8bd65075ce1e8020cb93dc39).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142442356
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -19,9 +19,16 @@
     # This is useful for setting default environmental settings.
     
     # Example:
    -# spark.master                     spark://master:7077
    -# spark.eventLog.enabled           true
    -# spark.eventLog.dir               hdfs://namenode:8021/directory
    -# spark.serializer                 org.apache.spark.serializer.KryoSerializer
    -# spark.driver.memory              5g
    -# spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +# spark.master                          spark://master:7077
    +# spark.eventLog.enabled                true
    +# spark.eventLog.dir                    hdfs://namenode:8021/directory
    +# spark.serializer                      org.apache.spark.serializer.KryoSerializer
    +# spark.driver.memory                   5g
    +# spark.executor.extraJavaOptions       -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +#spark.ui.allowFramingFrom              https://example.com/
    +#spark.ui.xXssProtection.enabled        1; mode=block
    --- End diff --
    
    Possible values can be one from the below set, default one mentioned in template is commonly-used and provides effective protection.
    X-XSS-Protection: { 0 | 1 |1; mode=block | 1; report=<reporting-uri> }



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    @srowen @tgravescs @dongjoon-hyun @jerryshao 
    Please review the PR. I have incorporated all changes as suggested.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by dongjoon-hyun <gi...@git.apache.org>.

Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r143113850
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -25,3 +25,10 @@
     # spark.serializer                 org.apache.spark.serializer.KryoSerializer
     # spark.driver.memory              5g
     # spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +#spark.ui.allowFramingFrom         https://www.example.com/
    +#spark.ui.xXssProtection           1; mode=block
    +#spark.ui.xContentType.options     nosniff
    +
    +#Enable below only when Spark is running on HTTPS
    +#spark.ui.strictTransportSecurity  max-age=31536000
    --- End diff --
    
    Could you add a space after `#` and an empty line at the end of file?
    ```
    # spark.ui.allowFramingFrom         https://www.example.com/
    # spark.ui.xXssProtection           1; mode=block
    # spark.ui.xContentType.options     nosniff
    
    # Enable below only when Spark is running on HTTPS
    # spark.ui.strictTransportSecurity  max-age=31536000
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Merged to master


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144565082
  
    --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
    @@ -420,6 +420,25 @@ package object config {
           .toSequence
           .createWithDefault(Nil)
     
    +
    +  private[spark] val UI_X_XSS_PROTECTION =
    +    ConfigBuilder("spark.ui.xXssProtection")
    +      .doc("Value for HTTP X-XSS-Protection response header")
    +      .stringConf
    +      .createOptional
    +
    +  private[spark] val UI_X_CONTENT_TYPE_OPTIONS =
    +    ConfigBuilder("spark.ui.xContentTypeOptions.enabled")
    +      .doc("Set to 'true' for setting X-Content-Type-Options HTTP response header to 'nosniff'")
    +      .stringConf
    --- End diff --
    
    Making it to Boolean.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142578622
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +92,9 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            response.setHeader("X-XSS-Protection", xXssProtectionValue.get)
    --- End diff --
    
    @srowen Added the check for if Option exists then set and tested locally. Thanks for the review.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144262264
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -25,3 +25,10 @@
     # spark.serializer                 org.apache.spark.serializer.KryoSerializer
     # spark.driver.memory              5g
     # spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +# spark.ui.allowFramingFrom         https://www.example.com/
    --- End diff --
    
    @srowen , @jerryshao Removed the entries from the config template and put it to configuration doc as suggested.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144689872
  
    --- Diff: docs/configuration.md ---
    @@ -2013,7 +2013,6 @@ Apart from these, the following properties are also available, and may be useful
         </tr>
     </table>
     
    -
    --- End diff --
    
    If you have to change the pull request again, I'd revert this, but no need to change it only for this


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82741 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82741/testReport)** for PR 19419 at commit [`1e61484`](https://github.com/apache/spark/commit/1e61484cbd6dba28ed19e0c6463e2d36e1b4e809).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144186220
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -25,3 +25,10 @@
     # spark.serializer                 org.apache.spark.serializer.KryoSerializer
     # spark.driver.memory              5g
     # spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +# spark.ui.allowFramingFrom         https://www.example.com/
    +# spark.ui.xXssProtection           1; mode=block
    +# spark.ui.xContentType.options     nosniff
    +
    +# Enable below only when Spark is running on HTTPS
    +# spark.ui.strictTransportSecurity  max-age=31536000
    --- End diff --
    
    What's the meaning of this specific number "31536000"?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82741/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82795/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by dongjoon-hyun <gi...@git.apache.org>.

Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r143043339
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -19,9 +19,16 @@
     # This is useful for setting default environmental settings.
     
     # Example:
    -# spark.master                     spark://master:7077
    -# spark.eventLog.enabled           true
    -# spark.eventLog.dir               hdfs://namenode:8021/directory
    -# spark.serializer                 org.apache.spark.serializer.KryoSerializer
    -# spark.driver.memory              5g
    -# spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +# spark.master                          spark://master:7077
    +# spark.eventLog.enabled                true
    +# spark.eventLog.dir                    hdfs://namenode:8021/directory
    +# spark.serializer                      org.apache.spark.serializer.KryoSerializer
    +# spark.driver.memory                   5g
    +# spark.executor.extraJavaOptions       -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    --- End diff --
    
    Hi, @krishna-pandey .
    It seems that you can revert the change on line 22 ~ 27.
    Could you re-adjust the spacing?



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82737 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82737/testReport)** for PR 19419 at commit [`10f6f30`](https://github.com/apache/spark/commit/10f6f302b8e6b58b5ba6e14b29ee979599d5f1ea).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    ok to test.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142634158
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -19,9 +19,16 @@
     # This is useful for setting default environmental settings.
     
     # Example:
    -# spark.master                     spark://master:7077
    -# spark.eventLog.enabled           true
    -# spark.eventLog.dir               hdfs://namenode:8021/directory
    -# spark.serializer                 org.apache.spark.serializer.KryoSerializer
    -# spark.driver.memory              5g
    -# spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +# spark.master                          spark://master:7077
    +# spark.eventLog.enabled                true
    +# spark.eventLog.dir                    hdfs://namenode:8021/directory
    +# spark.serializer                      org.apache.spark.serializer.KryoSerializer
    +# spark.driver.memory                   5g
    +# spark.executor.extraJavaOptions       -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +#spark.ui.allowFramingFrom              https://example.com/
    +#spark.ui.xXssProtection.enabled        1; mode=block
    --- End diff --
    
    I would remove '.enabled' from the name then


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82807/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144483070
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +90,14 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            conf.get(UI_X_XSS_PROTECTION).foreach(response.setHeader("X-XSS-Protection", _))
    +            if (conf.get(UI_X_CONTENT_TYPE_OPTIONS.key).equals("true")) {
    --- End diff --
    
    At the moment this would fail if unset


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144281816
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +92,9 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            xXssProtectionValue.foreach(response.setHeader("X-XSS-Protection", _))
    +            xContentTypeOptionsValue.foreach(response.setHeader("X-Content-Type-Options", _))
    +            strictTransportSecurityValue.foreach(response.setHeader("Strict-Transport-Security", _))
    --- End diff --
    
    @jerryshao I added a check to set the STS header, only if SSL is enabled. However, STS header was not stopping Browser from rendering the content even when it was set and Spark was running on plain HTTP. Need to get expected behaviour here.
    <img width="631" alt="screen shot 2017-10-12 at 6 22 18 pm" src="https://user-images.githubusercontent.com/6433184/31497151-ba7f7dac-af7b-11e7-99ea-88b64c778b51.png">



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82876 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82876/testReport)** for PR 19419 at commit [`de54313`](https://github.com/apache/spark/commit/de54313479383be54de6bb075afe228617c244f2).
     * This patch passes all tests.
     * This patch **does not merge cleanly**.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144689866
  
    --- Diff: docs/security.md ---
    @@ -186,7 +186,52 @@ configure those ports.
       </tr>
     </table>
     
    +### HTTP Security Headers
    +
    +Apache Spark can be configured to include HTTP Headers which aids in preventing Cross 
    +Site Scripting (XSS), Cross-Frame Scripting (XFS), MIME-Sniffing and also enforces HTTP 
    +Strict Transport Security.
    +
    +<table class="table">
    +    <tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
    +    <tr>
    +        <td><code>spark.ui.xXssProtection</code></td>
    +        <td>None</td>
    +        <td>
    +            Value for HTTP X-XSS-Protection response header. You can choose appropriate value 
    +            from below:
    +            <br />
    --- End diff --
    
    Why not just leave this as a bulleted list? Not a big deal I guess just less conventional for HTML


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82742 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82742/testReport)** for PR 19419 at commit [`5c76b91`](https://github.com/apache/spark/commit/5c76b914ecbd7fd82276496151f7ed89fe519025).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144286844
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -79,6 +79,9 @@ private[spark] object JettyUtils extends Logging {
         val allowFramingFrom = conf.getOption("spark.ui.allowFramingFrom")
         val xFrameOptionsValue =
           allowFramingFrom.map(uri => s"ALLOW-FROM $uri").getOrElse("SAMEORIGIN")
    +    val xXssProtectionValue = conf.getOption("spark.ui.xXssProtection")
    --- End diff --
    
    It follows new code convention, newly added configurations is better to change to that way.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    @dongjoon-hyun Made the changes as suggested.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    >/home/jenkins/workspace/SparkPullRequestBuilder@2/core/src/main/scala/org/apache/spark/internal/config/package.scala:440:0: Whitespace at end of line
    
    Please fix the style issue.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82791 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82791/testReport)** for PR 19419 at commit [`85cb02b`](https://github.com/apache/spark/commit/85cb02b84d549ef5b824db527f4df2e45465cd41).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by dongjoon-hyun <gi...@git.apache.org>.

Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    I can see them from the browser.
    LGTM except two minor comments, @krishna-pandey .


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    @tgravescs These generic headers are about providing available client-side protection for the application. I also think even if there is no sensitive data to formulate an attack by itself here, the information can be used in conjunction to target other ecosystem components. Also, in future we may add an interface for data access. Now is the time to think of Security First. Cross-site Scripting is one of the most prevalent attack vector and has been an OWASP Top 10 risk for web applications for decades. As the effort to have these in place here is minimal, IMHO we should set these. 
    
    As you rightly mentioned, deployment on cloud can expand the attack surface pretty wide in absence of right firewall policy. Also let's not forget insider threat inside corporate networks.
    
    Going forward may be we will have enough insight to choose which headers are needed to be enabled by default and enforce them from application side and not leave it to Users.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82742/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Can one of the admins verify this patch?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82791 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82791/testReport)** for PR 19419 at commit [`85cb02b`](https://github.com/apache/spark/commit/85cb02b84d549ef5b824db527f4df2e45465cd41).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142409795
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -19,9 +19,16 @@
     # This is useful for setting default environmental settings.
     
     # Example:
    -# spark.master                     spark://master:7077
    -# spark.eventLog.enabled           true
    -# spark.eventLog.dir               hdfs://namenode:8021/directory
    -# spark.serializer                 org.apache.spark.serializer.KryoSerializer
    -# spark.driver.memory              5g
    -# spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +# spark.master                          spark://master:7077
    +# spark.eventLog.enabled                true
    +# spark.eventLog.dir                    hdfs://namenode:8021/directory
    +# spark.serializer                      org.apache.spark.serializer.KryoSerializer
    +# spark.driver.memory                   5g
    +# spark.executor.extraJavaOptions       -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +#spark.ui.allowFramingFrom              https://example.com/
    +#spark.ui.xXssProtection.enabled        1; mode=block
    +#spark.ui.xContentType.options          nosniff
    +
    +#Enable below only when Spark is running on HTTPS
    +#spark.ui.strictTransportSecurity.age   max-age=31536000
    --- End diff --
    
    On a similar note does max-age need to be part of the user-supplied string or just the age? in what units?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144787904
  
    --- Diff: docs/security.md ---
    @@ -186,7 +186,54 @@ configure those ports.
       </tr>
     </table>
     
    +### HTTP Security Headers
    +
    +Apache Spark can be configured to include HTTP Headers which aids in preventing Cross 
    +Site Scripting (XSS), Cross-Frame Scripting (XFS), MIME-Sniffing and also enforces HTTP 
    +Strict Transport Security.
    +
    +<table class="table">
    +    <tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
    --- End diff --
    
    @jerryshao Fixed the indentation issue as per other Spark docs. Thanks for pointing that out.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142701588
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -19,9 +19,16 @@
     # This is useful for setting default environmental settings.
     
     # Example:
    -# spark.master                     spark://master:7077
    -# spark.eventLog.enabled           true
    -# spark.eventLog.dir               hdfs://namenode:8021/directory
    -# spark.serializer                 org.apache.spark.serializer.KryoSerializer
    -# spark.driver.memory              5g
    -# spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +# spark.master                          spark://master:7077
    +# spark.eventLog.enabled                true
    +# spark.eventLog.dir                    hdfs://namenode:8021/directory
    +# spark.serializer                      org.apache.spark.serializer.KryoSerializer
    +# spark.driver.memory                   5g
    +# spark.executor.extraJavaOptions       -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +#spark.ui.allowFramingFrom              https://example.com/
    +#spark.ui.xXssProtection.enabled        1; mode=block
    +#spark.ui.xContentType.options          nosniff
    +
    +#Enable below only when Spark is running on HTTPS
    +#spark.ui.strictTransportSecurity.age   max-age=31536000
    --- End diff --
    
    I understand that the "max-age" part is common across all values but don't want to tamper the value part for ease and any future compatibility. I will rather remove ".age" to avoid confusion arising out of this.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82736 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82736/testReport)** for PR 19419 at commit [`10f6f30`](https://github.com/apache/spark/commit/10f6f302b8e6b58b5ba6e14b29ee979599d5f1ea).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82878 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82878/testReport)** for PR 19419 at commit [`b6d4885`](https://github.com/apache/spark/commit/b6d4885e9ad9a03a40b3c28df41d7b263b89369f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144262853
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -79,6 +79,9 @@ private[spark] object JettyUtils extends Logging {
         val allowFramingFrom = conf.getOption("spark.ui.allowFramingFrom")
         val xFrameOptionsValue =
           allowFramingFrom.map(uri => s"ALLOW-FROM $uri").getOrElse("SAMEORIGIN")
    +    val xXssProtectionValue = conf.getOption("spark.ui.xXssProtection")
    --- End diff --
    
    @jerryshao I am not sure if that will add much value in this context apart from following best practices.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144483296
  
    --- Diff: docs/configuration.md ---
    @@ -2013,7 +2013,62 @@ Apart from these, the following properties are also available, and may be useful
         </tr>
     </table>
     
    +### HTTP Security Headers
     
    +Apache Spark can be configured to include HTTP Headers which aids in preventing Cross 
    +Site Scripting (XSS), Cross-Frame Scripting (XFS), MIME-Sniffing and also enforces HTTP 
    +Strict Transport Security.
    +
    +<table class="table">
    +    <tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
    +    <tr>
    +        <td><code>spark.ui.xXssProtection</code></td>
    +        <td>None</td>
    +        <td>
    +            Value for HTTP X-XSS-Protection response header. You can choose appropriate value 
    +            from below:
    +            <ul>
    +                <li>  0 (Disables XSS filtering)
    --- End diff --
    
    Markdown won't work here? I think it won't. Close the `<li>` tags though. It's not clear below what the values the users sets though. The values are `0`, `1`, or `1; mode=block` and that should be clear.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    For the content type header, does the UI seem to work normally with it on? I don't think we should have any problem with that restriction or else need to fix it .  You could move the defaults to on then.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144495051
  
    --- Diff: docs/configuration.md ---
    @@ -2013,7 +2013,62 @@ Apart from these, the following properties are also available, and may be useful
         </tr>
     </table>
     
    +### HTTP Security Headers
     
    +Apache Spark can be configured to include HTTP Headers which aids in preventing Cross 
    +Site Scripting (XSS), Cross-Frame Scripting (XFS), MIME-Sniffing and also enforces HTTP 
    +Strict Transport Security.
    +
    +<table class="table">
    +    <tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
    +    <tr>
    +        <td><code>spark.ui.xXssProtection</code></td>
    +        <td>None</td>
    +        <td>
    +            Value for HTTP X-XSS-Protection response header. You can choose appropriate value 
    +            from below:
    +            <ul>
    +                <li>  0 (Disables XSS filtering)
    --- End diff --
    
    Removed the lists, however it is being used in many places in existing configuration.md


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144488504
  
    --- Diff: docs/configuration.md ---
    @@ -2013,7 +2013,62 @@ Apart from these, the following properties are also available, and may be useful
         </tr>
     </table>
     
    +### HTTP Security Headers
    --- End diff --
    
    I think this could move to `security.md` as a security related advanced configurations. 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144880059
  
    --- Diff: docs/security.md ---
    @@ -186,7 +186,54 @@ configure those ports.
       </tr>
     </table>
     
    +### HTTP Security Headers
    +
    +Apache Spark can be configured to include HTTP Headers which aids in preventing Cross 
    +Site Scripting (XSS), Cross-Frame Scripting (XFS), MIME-Sniffing and also enforces HTTP 
    +Strict Transport Security.
    +
    +<table class="table">
    +<tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
    +<tr>
    +  <td><code>spark.ui.xXssProtection</code></td>
    +  <td>None</td>
    +  <td>
    +    Value for HTTP X-XSS-Protection response header. You can choose appropriate value 
    +    from below:
    +    <ul>
    +      <li> 0 </li> (Disables XSS filtering) 
    --- End diff --
    
    @srowen Thanks for the excellent tip. It looks more readable now.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #3947 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3947/testReport)** for PR 19419 at commit [`5c76b91`](https://github.com/apache/spark/commit/5c76b914ecbd7fd82276496151f7ed89fe519025).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144495102
  
    --- Diff: docs/configuration.md ---
    @@ -2013,7 +2013,62 @@ Apart from these, the following properties are also available, and may be useful
         </tr>
     </table>
     
    +### HTTP Security Headers
     
    +Apache Spark can be configured to include HTTP Headers which aids in preventing Cross 
    +Site Scripting (XSS), Cross-Frame Scripting (XFS), MIME-Sniffing and also enforces HTTP 
    +Strict Transport Security.
    +
    +<table class="table">
    +    <tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
    +    <tr>
    +        <td><code>spark.ui.xXssProtection</code></td>
    +        <td>None</td>
    +        <td>
    +            Value for HTTP X-XSS-Protection response header. You can choose appropriate value 
    +            from below:
    +            <ul>
    +                <li>  0 (Disables XSS filtering)
    +                <li>  1 (Enables XSS filtering. If a cross-site scripting attack is detected, 
    +                        the browser will sanitize the page.)
    +                <li>  1; mode=block (Enables XSS filtering. The browser will prevent rendering 
    +                        of the page if an attack is detected.)
    +            </ul> 
    +        </td>
    +    </tr>
    +    <tr>
    +        <td><code>spark.ui.allowFramingFrom</code></td>
    +        <td>SAMEORIGIN</td>
    +        <td>
    +            Value for X-Frame-Options HTTP response header
    +            <br />You can provide the "website uri" which can only be displayed in a frame on 
    +                the specified origin. 
    +            <br />
    --- End diff --
    
    Done.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    I think the change is OK, to give a mechanism to set these headers if desired. There are still several comments to address though.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82878/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r143427428
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +92,9 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            xXssProtectionValue.foreach(response.setHeader("X-XSS-Protection", _))
    +            xContentTypeOptionsValue.foreach(response.setHeader("X-Content-Type-Options", _))
    +            strictTransportSecurityValue.foreach(response.setHeader("Strict-Transport-Security", _))
    --- End diff --
    
    @jerryshao Out of these three, Strict Transport Security header makes sense, when SSL/TLS is enabled.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144482977
  
    --- Diff: core/src/main/scala/org/apache/spark/internal/config/package.scala ---
    @@ -420,6 +420,25 @@ package object config {
           .toSequence
           .createWithDefault(Nil)
     
    +
    +  private[spark] val UI_X_XSS_PROTECTION =
    +    ConfigBuilder("spark.ui.xXssProtection")
    +      .doc("Value for HTTP X-XSS-Protection response header")
    +      .stringConf
    +      .createOptional
    +
    +  private[spark] val UI_X_CONTENT_TYPE_OPTIONS =
    +    ConfigBuilder("spark.ui.xContentTypeOptions.enabled")
    +      .doc("Set to 'true' for setting X-Content-Type-Options HTTP response header to 'nosniff'")
    +      .stringConf
    --- End diff --
    
    This should be a boolean option then?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144283398
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +92,13 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            xXssProtectionValue.foreach(response.setHeader("X-XSS-Protection", _))
    +            if (xContentTypeOptionsValue.get.equalsIgnoreCase("true")) {
    +              response.setHeader("X-Content-Type-Options", "nosniff")
    +            }
    +            if (conf.get("spark.ssl.enabled").equalsIgnoreCase("true")) {
    --- End diff --
    
    I think you can check `request.scheme` if it is "https" or not?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142409896
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +92,9 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            response.setHeader("X-XSS-Protection", xXssProtectionValue.get)
    --- End diff --
    
    This will fail if these aren't set. Only set the headers if the Option exists


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by dongjoon-hyun <gi...@git.apache.org>.

Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r143048963
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +92,15 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            if (xXssProtectionValue.isDefined) {
    +              response.setHeader("X-XSS-Protection", xXssProtectionValue.get)
    +            }
    +            if (xContentTypeOptionsValue.isDefined) {
    +              response.setHeader("X-Content-Type-Options", xContentTypeOptionsValue.get)
    +            }
    +            if (strictTransportSecurityValue.isDefined) {
    +              response.setHeader("Strict-Transport-Security", strictTransportSecurityValue.get)
    +            }
    --- End diff --
    
    Can we simplify line 95~103 into the following?
    ```scala
    xXssProtectionValue.foreach(response.setHeader("X-XSS-Protection", _))
    xContentTypeOptionsValue.foreach(response.setHeader("X-Content-Type-Options", _))
    strictTransportSecurityValue.foreach(response.setHeader("Strict-Transport-Security", _))
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82736 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82736/testReport)** for PR 19419 at commit [`10f6f30`](https://github.com/apache/spark/commit/10f6f302b8e6b58b5ba6e14b29ee979599d5f1ea).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #3944 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3944/testReport)** for PR 19419 at commit [`d00a1dc`](https://github.com/apache/spark/commit/d00a1dc59ea43943e1b19761ef76d746541b6b84).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142634239
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -19,9 +19,16 @@
     # This is useful for setting default environmental settings.
     
     # Example:
    -# spark.master                     spark://master:7077
    -# spark.eventLog.enabled           true
    -# spark.eventLog.dir               hdfs://namenode:8021/directory
    -# spark.serializer                 org.apache.spark.serializer.KryoSerializer
    -# spark.driver.memory              5g
    -# spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +# spark.master                          spark://master:7077
    +# spark.eventLog.enabled                true
    +# spark.eventLog.dir                    hdfs://namenode:8021/directory
    +# spark.serializer                      org.apache.spark.serializer.KryoSerializer
    +# spark.driver.memory                   5g
    +# spark.executor.extraJavaOptions       -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +#spark.ui.allowFramingFrom              https://example.com/
    +#spark.ui.xXssProtection.enabled        1; mode=block
    +#spark.ui.xContentType.options          nosniff
    +
    +#Enable below only when Spark is running on HTTPS
    +#spark.ui.strictTransportSecurity.age   max-age=31536000
    --- End diff --
    
    I'd just remove '.age' then, likewise. It's the very value of the STS header


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    @srowen @rxin Made changes to enable the X-Content-Type-Options and X-XSS-Protection values by default. Please review.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    LGTM.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82737 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82737/testReport)** for PR 19419 at commit [`10f6f30`](https://github.com/apache/spark/commit/10f6f302b8e6b58b5ba6e14b29ee979599d5f1ea).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82737/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82876/testReport)** for PR 19419 at commit [`de54313`](https://github.com/apache/spark/commit/de54313479383be54de6bb075afe228617c244f2).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142409684
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -19,9 +19,16 @@
     # This is useful for setting default environmental settings.
     
     # Example:
    -# spark.master                     spark://master:7077
    -# spark.eventLog.enabled           true
    -# spark.eventLog.dir               hdfs://namenode:8021/directory
    -# spark.serializer                 org.apache.spark.serializer.KryoSerializer
    -# spark.driver.memory              5g
    -# spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +# spark.master                          spark://master:7077
    +# spark.eventLog.enabled                true
    +# spark.eventLog.dir                    hdfs://namenode:8021/directory
    +# spark.serializer                      org.apache.spark.serializer.KryoSerializer
    +# spark.driver.memory                   5g
    +# spark.executor.extraJavaOptions       -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +#spark.ui.allowFramingFrom              https://example.com/
    +#spark.ui.xXssProtection.enabled        1; mode=block
    --- End diff --
    
    This sounds like a boolean flag but its value isn't boolean; if it has only one reasonable value then it can be a boolean that sets that value. Otherwise maybe call the property something else


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #3947 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3947/testReport)** for PR 19419 at commit [`5c76b91`](https://github.com/apache/spark/commit/5c76b914ecbd7fd82276496151f7ed89fe519025).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144348004
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +92,13 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            xXssProtectionValue.foreach(response.setHeader("X-XSS-Protection", _))
    +            if (xContentTypeOptionsValue.get.equalsIgnoreCase("true")) {
    +              response.setHeader("X-Content-Type-Options", "nosniff")
    +            }
    +            if (conf.get("spark.ssl.enabled").equalsIgnoreCase("true")) {
    --- End diff --
    
    @jerryshao Thanks for the tip. I will do that.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82795 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82795/testReport)** for PR 19419 at commit [`acfd227`](https://github.com/apache/spark/commit/acfd227a550fd4e40fd74001e1b48880fdea60bc).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r143349235
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -25,3 +25,10 @@
     # spark.serializer                 org.apache.spark.serializer.KryoSerializer
     # spark.driver.memory              5g
     # spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +# spark.ui.allowFramingFrom         https://www.example.com/
    --- End diff --
    
    Hm, my last thought on this is that these are undocumented options, and maybe rightly so as they're kind of advanced options. But if so they don't really have a place in the default props template; what do they mean to a user?
    
    I think they should actually be removed here, on second glance, but, wouldn't be a big deal to document them in the configuration doc as they're arguably non-internal, important functionality for some users.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82807 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82807/testReport)** for PR 19419 at commit [`1cc34e9`](https://github.com/apache/spark/commit/1cc34e912763e92d8bd65075ce1e8020cb93dc39).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142708896
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -19,9 +19,16 @@
     # This is useful for setting default environmental settings.
     
     # Example:
    -# spark.master                     spark://master:7077
    -# spark.eventLog.enabled           true
    -# spark.eventLog.dir               hdfs://namenode:8021/directory
    -# spark.serializer                 org.apache.spark.serializer.KryoSerializer
    -# spark.driver.memory              5g
    -# spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +# spark.master                          spark://master:7077
    +# spark.eventLog.enabled                true
    +# spark.eventLog.dir                    hdfs://namenode:8021/directory
    +# spark.serializer                      org.apache.spark.serializer.KryoSerializer
    +# spark.driver.memory                   5g
    +# spark.executor.extraJavaOptions       -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +#spark.ui.allowFramingFrom              https://example.com/
    +#spark.ui.xXssProtection.enabled        1; mode=block
    --- End diff --
    
    @srowen renamed the keys as suggested. Thanks again for the review.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82736/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142446016
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +92,9 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            response.setHeader("X-XSS-Protection", xXssProtectionValue.get)
    --- End diff --
    
    Yes, I will add a check for that.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    @rxin , @srowen I think we can enable X-XSS-Protection and X-Content-Type-Options response header by default. STS Header can be left configurable or enabled by default when Spark UI is running on HTTPS.
    
    **Word of caution**: When X-Content-Type-Options response HTTP header is set to "nosniff", it will block a request if the requested type is  "style" and the MIME type is not "text/css", or when requested type is "script" and the MIME type is not a JavaScript MIME type.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by rxin <gi...@git.apache.org>.

Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Is there a reason why this cannot be always enabled?



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82791/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #3944 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3944/testReport)** for PR 19419 at commit [`d00a1dc`](https://github.com/apache/spark/commit/d00a1dc59ea43943e1b19761ef76d746541b6b84).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144775941
  
    --- Diff: docs/security.md ---
    @@ -186,7 +186,54 @@ configure those ports.
       </tr>
     </table>
     
    +### HTTP Security Headers
    +
    +Apache Spark can be configured to include HTTP Headers which aids in preventing Cross 
    +Site Scripting (XSS), Cross-Frame Scripting (XFS), MIME-Sniffing and also enforces HTTP 
    +Strict Transport Security.
    +
    +<table class="table">
    +    <tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
    --- End diff --
    
    I think in Spark we follow 2 space indent for html code. You could refer to other docs.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144768513
  
    --- Diff: docs/configuration.md ---
    @@ -2013,7 +2013,6 @@ Apart from these, the following properties are also available, and may be useful
         </tr>
     </table>
     
    -
    --- End diff --
    
    Restored the change.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    @jerryshao removed Whitespace at end of line 440 in package.scala. ok to test.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    @dongjoon-hyun Thanks for the review. Made the changes as suggested.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r143377740
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -25,3 +25,10 @@
     # spark.serializer                 org.apache.spark.serializer.KryoSerializer
     # spark.driver.memory              5g
     # spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +# spark.ui.allowFramingFrom         https://www.example.com/
    --- End diff --
    
    Agree with @srowen , we should remove the configurations here in template, since they're not common configurations. Also add them to `docs/configuration.md`.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144483342
  
    --- Diff: docs/configuration.md ---
    @@ -2013,7 +2013,62 @@ Apart from these, the following properties are also available, and may be useful
         </tr>
     </table>
     
    +### HTTP Security Headers
     
    +Apache Spark can be configured to include HTTP Headers which aids in preventing Cross 
    +Site Scripting (XSS), Cross-Frame Scripting (XFS), MIME-Sniffing and also enforces HTTP 
    +Strict Transport Security.
    +
    +<table class="table">
    +    <tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
    +    <tr>
    +        <td><code>spark.ui.xXssProtection</code></td>
    +        <td>None</td>
    +        <td>
    +            Value for HTTP X-XSS-Protection response header. You can choose appropriate value 
    +            from below:
    +            <ul>
    +                <li>  0 (Disables XSS filtering)
    +                <li>  1 (Enables XSS filtering. If a cross-site scripting attack is detected, 
    +                        the browser will sanitize the page.)
    +                <li>  1; mode=block (Enables XSS filtering. The browser will prevent rendering 
    +                        of the page if an attack is detected.)
    +            </ul> 
    +        </td>
    +    </tr>
    +    <tr>
    +        <td><code>spark.ui.allowFramingFrom</code></td>
    +        <td>SAMEORIGIN</td>
    +        <td>
    +            Value for X-Frame-Options HTTP response header
    +            <br />You can provide the "website uri" which can only be displayed in a frame on 
    +                the specified origin. 
    +            <br />
    --- End diff --
    
    Remove this


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144768206
  
    --- Diff: docs/security.md ---
    @@ -186,7 +186,52 @@ configure those ports.
       </tr>
     </table>
     
    +### HTTP Security Headers
    +
    +Apache Spark can be configured to include HTTP Headers which aids in preventing Cross 
    +Site Scripting (XSS), Cross-Frame Scripting (XFS), MIME-Sniffing and also enforces HTTP 
    +Strict Transport Security.
    +
    +<table class="table">
    +    <tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
    +    <tr>
    +        <td><code>spark.ui.xXssProtection</code></td>
    +        <td>None</td>
    +        <td>
    +            Value for HTTP X-XSS-Protection response header. You can choose appropriate value 
    +            from below:
    +            <br />
    --- End diff --
    
    @srowen Made them bulleted list and closed the <li> tag as well.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82876/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r143377794
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -79,6 +79,9 @@ private[spark] object JettyUtils extends Logging {
         val allowFramingFrom = conf.getOption("spark.ui.allowFramingFrom")
         val xFrameOptionsValue =
           allowFramingFrom.map(uri => s"ALLOW-FROM $uri").getOrElse("SAMEORIGIN")
    +    val xXssProtectionValue = conf.getOption("spark.ui.xXssProtection")
    --- End diff --
    
    Please use `ConfigEntry` for newly added configurations, you could refer to `org.apache.spark.internal.config`.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by rxin <gi...@git.apache.org>.

Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    Yea in general for security features it seems like it's good to turn on them by default.



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142578623
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +92,9 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            response.setHeader("X-XSS-Protection", xXssProtectionValue.get)
    --- End diff --
    
    @srowen Added the check for if Option exists then set and tested locally. Thanks for the review.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82742 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82742/testReport)** for PR 19419 at commit [`5c76b91`](https://github.com/apache/spark/commit/5c76b914ecbd7fd82276496151f7ed89fe519025).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by krishna-pandey <gi...@git.apache.org>.

Github user krishna-pandey commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r142445649
  
    --- Diff: conf/spark-defaults.conf.template ---
    @@ -19,9 +19,16 @@
     # This is useful for setting default environmental settings.
     
     # Example:
    -# spark.master                     spark://master:7077
    -# spark.eventLog.enabled           true
    -# spark.eventLog.dir               hdfs://namenode:8021/directory
    -# spark.serializer                 org.apache.spark.serializer.KryoSerializer
    -# spark.driver.memory              5g
    -# spark.executor.extraJavaOptions  -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +# spark.master                          spark://master:7077
    +# spark.eventLog.enabled                true
    +# spark.eventLog.dir                    hdfs://namenode:8021/directory
    +# spark.serializer                      org.apache.spark.serializer.KryoSerializer
    +# spark.driver.memory                   5g
    +# spark.executor.extraJavaOptions       -XX:+PrintGCDetails -Dkey=value -Dnumbers="one two three"
    +
    +#spark.ui.allowFramingFrom              https://example.com/
    +#spark.ui.xXssProtection.enabled        1; mode=block
    +#spark.ui.xContentType.options          nosniff
    +
    +#Enable below only when Spark is running on HTTPS
    +#spark.ui.strictTransportSecurity.age   max-age=31536000
    --- End diff --
    
    Yes, "max-age" need to be part of user-supplied string. Possible values can be one from the below set, default one mentioned in template is for optimum secure value commonly used.
    
    Strict-Transport-Security: {max-age=\<expire-time\> | max-age=\<expire-time\>; includeSubDomains | max-age=\<expire-time\>; preload}
    
    
    Value is in delta-seconds. More here https://tools.ietf.org/html/rfc6797#section-6.1.1


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by srowen <gi...@git.apache.org>.

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r144816893
  
    --- Diff: docs/security.md ---
    @@ -186,7 +186,54 @@ configure those ports.
       </tr>
     </table>
     
    +### HTTP Security Headers
    +
    +Apache Spark can be configured to include HTTP Headers which aids in preventing Cross 
    +Site Scripting (XSS), Cross-Frame Scripting (XFS), MIME-Sniffing and also enforces HTTP 
    +Strict Transport Security.
    +
    +<table class="table">
    +<tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
    +<tr>
    +  <td><code>spark.ui.xXssProtection</code></td>
    +  <td>None</td>
    +  <td>
    +    Value for HTTP X-XSS-Protection response header. You can choose appropriate value 
    +    from below:
    +    <ul>
    +      <li> 0 </li> (Disables XSS filtering) 
    --- End diff --
    
    Now you have text outside the list item. This won't render correctly. This should be for example:
    ```
    <li><code>0</code> (Disables XSS filtering)</li>
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark pull request #19419: [SPARK-22188] [CORE] Adding security headers for ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19419#discussion_r143377976
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -89,6 +92,9 @@ private[spark] object JettyUtils extends Logging {
                 val result = servletParams.responder(request)
                 response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate")
                 response.setHeader("X-Frame-Options", xFrameOptionsValue)
    +            xXssProtectionValue.foreach(response.setHeader("X-XSS-Protection", _))
    +            xContentTypeOptionsValue.foreach(response.setHeader("X-Content-Type-Options", _))
    +            strictTransportSecurityValue.foreach(response.setHeader("Strict-Transport-Security", _))
    --- End diff --
    
    The changes here will also affect HTTP request, is that OK?
    
    Also is it enough to only change the GET request?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #19419: [SPARK-22188] [CORE] Adding security headers for prevent...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/19419
  
    **[Test build #82741 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82741/testReport)** for PR 19419 at commit [`1e61484`](https://github.com/apache/spark/commit/1e61484cbd6dba28ed19e0c6463e2d36e1b4e809).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org