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

PAYARA-2109 Protect Admin Console Web Pages against Clickjacking/UI Redress attacks #2097

Merged
merged 4 commits into from Nov 3, 2017

Conversation

MeroRai
Copy link
Member

@MeroRai MeroRai commented Oct 25, 2017

No description provided.

@MeroRai MeroRai requested a review from fturizo October 25, 2017 15:00
@MeroRai MeroRai self-assigned this Oct 25, 2017
@MeroRai MeroRai added this to the Payara 4.174 milestone Oct 25, 2017
Copy link
Contributor

@smillidge smillidge left a comment

Choose a reason for hiding this comment

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

XFrame Options has 3 values therefore should be selectable from the 3 rather than a boolean true or false

@@ -300,6 +300,8 @@ http.XpoweredBy=XPowered By:
http.XpoweredByHelp=Include X-Powered-By: Servlet/3.0 in servlet-generated HTTP response headers
http.ServerHeader=Server Header:
http.ServerHeaderHelp=Include Server Header: Servlet/3.0 in servlet-generated HTTP response headers
http.XframeOptions=XFrame Options:
http.XframeOptionsHelp=Include X-Frame-Options: Servlet/3.0 in servlet-generated HTTP response headers
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should say "Default X-Frame-Options value to be included in all responses"

httpHeader.getHeaders().removeHeader(Header.Server);
}

if (xFrameOptions == null && httpHeader.containsHeader(xFrameOptionsHeader)){
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you should remove an XFrameOptions Header if it is already present. XPoweredBy and Server are removed when configured to be removed because they leak security information. However XFrame-Options could be set by the application developer for legitimate reasons so should be honoured if present.

@MeroRai
Copy link
Member Author

MeroRai commented Nov 3, 2017

Jenkins test please

@payara-ci
Copy link
Contributor

Quick build and test passed!

@payara payara deleted a comment from payara-ci Nov 3, 2017
@payara payara deleted a comment from payara-ci Nov 3, 2017
@arjantijms arjantijms merged commit 8480750 into payara:master Nov 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants