Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ZEPPELIN-2775] Strict-Transport-Security and X-XSS-Protection Headers #2492

Closed
wants to merge 4 commits into from

Conversation

krishna-pandey
Copy link
Contributor

What is this PR for?

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.

What type of PR is it?

[Bug Fix | Improvement ]

What is the Jira issue?

How should this be tested?

Make a curl call to Zeppelin? Go to Chrome Browser and select "More Tools" -> "Developer Tools" from the right-side menu. Under Network Section, select any request and check for "Response Headers". You should see below headers along with existing ones.

strict-transport-security:max-age=631138519
x-xss-protection:1; mode=block

screen shot 2017-07-14 at 8 19 14 pm

Questions:

  • Does this needs documentation?

@krishna-pandey
Copy link
Contributor Author

@Leemoonsoo, @felixcheung, @jongyoul, @prabhjyotsingh Please help review this.

Note: Chrome Browser seems to be ignoring "X-XSS-Protection" header when value is set to 1. Ideally, it should be set to "X-XSS-Protection:1; mode=block". Difference being when later value is set, the browser will prevent rendering of the page if an attack is detected rather than sanitizing the page as in previous case.

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably would be a good idea to document this.
should this go to zeppelin/docs/setup/ or zeppelin/docs/security?

ZEPPELIN_SERVER_JETTY_NAME("zeppelin.server.jetty.name", null);
ZEPPELIN_SERVER_JETTY_NAME("zeppelin.server.jetty.name", null),
ZEPPELIN_SERVER_STRICT_TRANSPORT("zeppelin.server.strict.transport", "max-age=631138519"),
ZEPPELIN_SERVER_X_XSS_PROTECTION("zeppelin.server.xxss.protection", 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mode=block is not in the value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was fixed in separate commit.

@prabhjyotsingh
Copy link
Contributor

Tested on local, works as expected. LGTM!

@@ -681,7 +681,7 @@ public String getStrictTransport() {
ZEPPELIN_SERVER_XFRAME_OPTIONS("zeppelin.server.xframe.options", "SAMEORIGIN"),
ZEPPELIN_SERVER_JETTY_NAME("zeppelin.server.jetty.name", null),
ZEPPELIN_SERVER_STRICT_TRANSPORT("zeppelin.server.strict.transport", "max-age=631138519"),
ZEPPELIN_SERVER_X_XSS_PROTECTION("zeppelin.server.xxss.protection", 1);
ZEPPELIN_SERVER_X_XSS_PROTECTION("zeppelin.server.xxss.protection", "1");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixcheung That's intentional. When "X-XSS-Protection" header when value is set to "1", it enables XSS filtering (usually default in browsers). If a cross-site scripting attack is detected, the browser will sanitize the page (remove the unsafe parts).

Ideally, it should be set to "X-XSS-Protection:1; mode=block". With this value, rather than sanitizing the page, the browser will prevent rendering of the page if an attack is detected.

Hence, the default value is set to "1" so that it should not become strict. Those who want this security feature can enable it from Config, where putting "1" doesn't change anything and hence the ideal value "1; mode=block". If you feel, I should change the config value for sake of conformity across project, do let me know.

<!--
<property>
<name>zeppelin.server.xxss.protection</name>
<value>1; mode=block</value>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see - I understand the differences actually, but was wondering why the default value in the template was different from the default value in the code.

I feel either they should be the same, or they should be explained like you have in this PR. For the later approach, you could explain 1; mode=block as recommended for stricter safety in the description, but that the default value in the absence of a value in zeppelin-site.xml is simply 1 which means ... etc.

@krishna-pandey
Copy link
Contributor Author

@felixcheung Made the change as suggested.
Also provided documentation for all HTTP Security Headers support we added recently (tested it locally). Let me know if I am still missing anything. Thanks for the review.

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very cool, thanks
a few more comments

<property>
<name>zeppelin.server.strict.transport</name>
<value>max-age=631138519</value>
<description>The HTTP Strict-Transport-Security response header is a security feature that lets a web site tell browsers that it should only be communicated with using HTTPS, instead of using HTTP. Enable this when Zeppelin is running on HTTPS.Set value is in Seconds equivalent to 20 years.</description>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a space "HTTPS.Set "

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this say "Set value is in Seconds equivalent to 20 years"
-> "Value is in Seconds, the default value is equivalent to 20 years"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on this part.

<property>
<name>zeppelin.server.strict.transport</name>
<value>max-age=631138519</value>
<description>The HTTP Strict-Transport-Security response header is a security feature that lets a web site tell browsers that it should only be communicated with using HTTPS, instead of using HTTP. Enable this when Zeppelin is running on HTTPS.Set value is in Seconds equivalent to 20 years.</description>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto here "HTTPS.Set "

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this with above suggestion.


The HTTP X-XSS-Protection response header is a feature of Internet Explorer, Chrome and Safari Web browsers that initiates configured action when they detect reflected cross-site scripting (XSS) attacks.

The following properties needs to be updated in the zeppelin-site.xml in order to set X-XSS-PROTECTION header.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be "following property needs"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed all the occurrences as suggested.


## Setting up X-Frame-Options Header

The X-Frame-Options HTTP response header can indicate browser to avoid clickjacking attacks, by ensuring that their content is not embedded into other sites in a \<frame>,\<iframe> or \<object>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should you perhaps put the html tags in backtick? <frame>?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks more clean now.Thanks for suggestion.


## Setting up Server Header

Security conscious organisations does not want to reveal the Application Server name and version to prevent Script-kiddies from finding the information easily when fingerprinting the Application. The exact version number can tell an Attacker if the current Application Server is patched for or vulnerable to certain publicly known CVE associated to it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we should avoid the term Script-kiddies?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixcheung Can we use word "Attacker"?

@krishna-pandey
Copy link
Contributor Author

@felixcheung Updated the documentation as per review. Let me know if any other changes are required. Thanks.

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
anyone else want to review or comment?

@felixcheung
Copy link
Member

merging if no more comment

@asfgit asfgit closed this in 780f0eb Aug 3, 2017
prabhjyotsingh added a commit to prabhjyotsingh/zeppelin that referenced this pull request Aug 15, 2017
### What is this PR for?
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.

### What type of PR is it?
[Bug Fix | Improvement ]

### What is the Jira issue?
* [ZEPPELIN-2775](https://issues.apache.org/jira/browse/ZEPPELIN-2775)

### How should this be tested?
Make a curl call to Zeppelin? Go to Chrome Browser and select "More Tools" -> "Developer Tools" from the right-side menu. Under Network Section, select any request and check for "Response Headers". You should see below headers along with existing ones.

> strict-transport-security:max-age=631138519
> x-xss-protection:1; mode=block

<img width="1436" alt="screen shot 2017-07-14 at 8 19 14 pm" src="https://user-images.githubusercontent.com/6433184/28217231-16ce6cee-68d2-11e7-91aa-77ad083612c7.png">

### Questions:
* Does this needs documentation?

Author: krishna-pandey <krish.pandey21@gmail.com>

Closes apache#2492 from krishna-pandey/ZEPPELIN-2775 and squashes the following commits:

7d9978e [krishna-pandey] Modified Documentation as per review.
6733289 [krishna-pandey] Adding documentation for HTTP Security Headers
754d2d7 [krishna-pandey] Supplying String instead of Int (required for Response Header)
468231c [krishna-pandey] Added configurable Strict-Transport-Security and X-XSS-Protection Headers
prabhjyotsingh pushed a commit to prabhjyotsingh/zeppelin that referenced this pull request Aug 15, 2017
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.

[Bug Fix | Improvement ]

* [ZEPPELIN-2775](https://issues.apache.org/jira/browse/ZEPPELIN-2775)

Make a curl call to Zeppelin? Go to Chrome Browser and select "More Tools" -> "Developer Tools" from the right-side menu. Under Network Section, select any request and check for "Response Headers". You should see below headers along with existing ones.

> strict-transport-security:max-age=631138519
> x-xss-protection:1; mode=block

<img width="1436" alt="screen shot 2017-07-14 at 8 19 14 pm" src="https://user-images.githubusercontent.com/6433184/28217231-16ce6cee-68d2-11e7-91aa-77ad083612c7.png">

* Does this needs documentation?

Author: krishna-pandey <krish.pandey21@gmail.com>

Closes apache#2492 from krishna-pandey/ZEPPELIN-2775 and squashes the following commits:

7d9978e [krishna-pandey] Modified Documentation as per review.
6733289 [krishna-pandey] Adding documentation for HTTP Security Headers
754d2d7 [krishna-pandey] Supplying String instead of Int (required for Response Header)
468231c [krishna-pandey] Added configurable Strict-Transport-Security and X-XSS-Protection Headers
prabhjyotsingh added a commit to prabhjyotsingh/zeppelin that referenced this pull request Sep 1, 2017
### What is this PR for?
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.

### What type of PR is it?
[Bug Fix | Improvement ]

### What is the Jira issue?
* [ZEPPELIN-2775](https://issues.apache.org/jira/browse/ZEPPELIN-2775)

### How should this be tested?
Make a curl call to Zeppelin? Go to Chrome Browser and select "More Tools" -> "Developer Tools" from the right-side menu. Under Network Section, select any request and check for "Response Headers". You should see below headers along with existing ones.

> strict-transport-security:max-age=631138519
> x-xss-protection:1; mode=block

<img width="1436" alt="screen shot 2017-07-14 at 8 19 14 pm" src="https://user-images.githubusercontent.com/6433184/28217231-16ce6cee-68d2-11e7-91aa-77ad083612c7.png">

### Questions:
* Does this needs documentation?

Author: krishna-pandey <krish.pandey21@gmail.com>

Closes apache#2492 from krishna-pandey/ZEPPELIN-2775 and squashes the following commits:

7d9978e [krishna-pandey] Modified Documentation as per review.
6733289 [krishna-pandey] Adding documentation for HTTP Security Headers
754d2d7 [krishna-pandey] Supplying String instead of Int (required for Response Header)
468231c [krishna-pandey] Added configurable Strict-Transport-Security and X-XSS-Protection Headers
@1ambda
Copy link
Member

1ambda commented Sep 4, 2017

@krishna-pandey Hi, I am getting this error in browser console. Could you check this?

image

@krishna-pandey
Copy link
Contributor Author

@1ambda What's the value you are providing for "zeppelin.server.xxss.protection" property. It can take three possible values "0", "1" or "1; mode=block".

@1ambda
Copy link
Member

1ambda commented Sep 4, 2017

@krishna-pandey
Copy link
Contributor Author

krishna-pandey commented Sep 4, 2017

@1ambda I am able to reproduce the issue, seems like the value is getting repeated. It turns out that all Headers are being set multiple times. I have created an issue ZEPPELIN-2896 for that.

@1ambda
Copy link
Member

1ambda commented Sep 4, 2017

Thanks.

prabhjyotsingh added a commit to prabhjyotsingh/zeppelin that referenced this pull request Oct 23, 2017
### What is this PR for?
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.

### What type of PR is it?
[Bug Fix | Improvement ]

### What is the Jira issue?
* [ZEPPELIN-2775](https://issues.apache.org/jira/browse/ZEPPELIN-2775)

### How should this be tested?
Make a curl call to Zeppelin? Go to Chrome Browser and select "More Tools" -> "Developer Tools" from the right-side menu. Under Network Section, select any request and check for "Response Headers". You should see below headers along with existing ones.

> strict-transport-security:max-age=631138519
> x-xss-protection:1; mode=block

<img width="1436" alt="screen shot 2017-07-14 at 8 19 14 pm" src="https://user-images.githubusercontent.com/6433184/28217231-16ce6cee-68d2-11e7-91aa-77ad083612c7.png">

### Questions:
* Does this needs documentation?

Author: krishna-pandey <krish.pandey21@gmail.com>

Closes apache#2492 from krishna-pandey/ZEPPELIN-2775 and squashes the following commits:

7d9978e [krishna-pandey] Modified Documentation as per review.
6733289 [krishna-pandey] Adding documentation for HTTP Security Headers
754d2d7 [krishna-pandey] Supplying String instead of Int (required for Response Header)
468231c [krishna-pandey] Added configurable Strict-Transport-Security and X-XSS-Protection Headers
prabhjyotsingh pushed a commit to prabhjyotsingh/zeppelin that referenced this pull request Dec 9, 2017
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.

[Bug Fix | Improvement ]

* [ZEPPELIN-2775](https://issues.apache.org/jira/browse/ZEPPELIN-2775)

Make a curl call to Zeppelin? Go to Chrome Browser and select "More Tools" -> "Developer Tools" from the right-side menu. Under Network Section, select any request and check for "Response Headers". You should see below headers along with existing ones.

> strict-transport-security:max-age=631138519
> x-xss-protection:1; mode=block

<img width="1436" alt="screen shot 2017-07-14 at 8 19 14 pm" src="https://user-images.githubusercontent.com/6433184/28217231-16ce6cee-68d2-11e7-91aa-77ad083612c7.png">

* Does this needs documentation?

Author: krishna-pandey <krish.pandey21@gmail.com>

Closes apache#2492 from krishna-pandey/ZEPPELIN-2775 and squashes the following commits:

7d9978e [krishna-pandey] Modified Documentation as per review.
6733289 [krishna-pandey] Adding documentation for HTTP Security Headers
754d2d7 [krishna-pandey] Supplying String instead of Int (required for Response Header)
468231c [krishna-pandey] Added configurable Strict-Transport-Security and X-XSS-Protection Headers

Change-Id: Ic4865871aeb2aad46228396cc6c08db9d7a90f93
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants