svn commit: r1845418 - in /ofbiz/ofbiz-framework/trunk/framework: base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java

Taher Alkhateeb

2018-11-01 12:01:32 UTC

it is usually not good practice to add commented-out code to the code
base. I recommend removing it. I also think the comments might benefit
from better formatting and structuring instead of this sporadic "//"
sprinkled all over the place

Author: jleroux
Date: Thu Nov 1 09:36:35 2018
New Revision: 1845418
URL: http://svn.apache.org/viewvc?rev=1845418&view=rev
Fixed: Missing Security and Cache Headers in CMS Events
(OFBIZ-10597)
While rendering the view through the controller request we set the important
security headers like x-frame-options, strict-transport-security,
x-content-type-options, X-XSS-Protection and Referrer-Policy etc. in the
response object. (Please see the 'rendervView' method of RequestHandler class.)
In the similar line, we set the cache related headers like Expires,
Last-Modified, Cache-Control, Pragma.
But these security headers are missing in the pages rendered through CMS.
(Please visit the CmsEvents class).
These headers are very crucial for the security of the application as they help
to prevent various security threats like cross-site scripting,
cross-site request forgery, clickjacking etc.
Thanks: Deepak Nigam for initial patch and review
ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java?rev=1845418&r1=1845417&r2=1845418&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java Thu Nov 1 09:36:35 2018
@@ -57,6 +57,7 @@ import org.apache.http.impl.client.Close
import org.apache.http.impl.client.HttpClients;
import org.apache.http.ssl.SSLContexts;
import org.apache.ofbiz.entity.util.EntityUtilProperties;
+import org.apache.ofbiz.webapp.control.ConfigXMLReader;
import org.apache.ofbiz.widget.renderer.VisualTheme;
import org.apache.oro.text.regex.MalformedPatternException;
import org.apache.oro.text.regex.Pattern;
@@ -980,6 +981,58 @@ public final class UtilHttp {
response.setHeader("Cache-Control", "no-store, no-cache, must-revalidate, private"); // HTTP/1.1
response.setHeader("Pragma", "no-cache"); // HTTP/1.0
}
+
+ public static void setResponseBrowserDefaultSecurityHeaders(HttpServletResponse resp, ConfigXMLReader.ViewMap viewMap) {
+ // See https://cwiki.apache.org/confluence/display/OFBIZ/How+to+Secure+HTTP+Headers for details and how to test
+ String xFrameOption = null;
+ if (viewMap != null) {
+ xFrameOption = viewMap.xFrameOption;
+ }
+ // default to sameorigin
+ if (UtilValidate.isNotEmpty(xFrameOption)) {
+ if(!"none".equals(xFrameOption)) {
+ resp.addHeader("x-frame-options", xFrameOption);
+ }
+ } else {
+ resp.addHeader("x-frame-options", "sameorigin");
+ }
+
+ String strictTransportSecurity = null;
+ if (viewMap != null) {
+ strictTransportSecurity = viewMap.strictTransportSecurity;
+ }
+ // default to "max-age=31536000; includeSubDomains" 31536000 secs = 1 year
+ if (UtilValidate.isNotEmpty(strictTransportSecurity)) {
+ if (!"none".equals(strictTransportSecurity)) {
+ resp.addHeader("strict-transport-security", strictTransportSecurity);
+ }
+ } else {
+ if (EntityUtilProperties.getPropertyAsBoolean("requestHandler", "strict-transport-security", true)) { // FIXME later pass req.getAttribute("delegator") as last argument
+ resp.addHeader("strict-transport-security", "max-age=31536000; includeSubDomains");
+ }
+ }
+
+ //The only x-content-type-options defined value, "nosniff", prevents Internet Explorer from MIME-sniffing a response away from the declared content-type.
+ // This also applies to Google Chrome, when downloading extensions.
+ resp.addHeader("x-content-type-options", "nosniff");
+
+ // This header enables the Cross-site scripting (XSS) filter built into most recent web browsers.
+ // It's usually enabled by default anyway, so the role of this header is to re-enable the filter for this particular website if it was disabled by the user.
+ // This header is supported in IE 8+, and in Chrome (not sure which versions). The anti-XSS filter was added in Chrome 4. Its unknown if that version honored this header.
+ // FireFox has still an open bug entry and "offers" only the noscript plugin
+ // https://wiki.mozilla.org/Security/Features/XSS_Filter
+ // https://bugzilla.mozilla.org/show_bug.cgi?id=528661
+ resp.addHeader("X-XSS-Protection","1; mode=block");
+
+ resp.setHeader("Referrer-Policy", "no-referrer-when-downgrade"); // This is the default (in Firefox at least)
+
+ //resp.setHeader("Content-Security-Policy", "default-src 'self'");
+ //resp.setHeader("Content-Security-Policy-Report-Only", "default-src 'self'; report-uri webtools/control/ContentSecurityPolicyReporter");
+ resp.setHeader("Content-Security-Policy-Report-Only", "default-src 'self'");
+
+ // TODO in custom project. Public-Key-Pins-Report-Only is interesting but can't be used OOTB because of demos (the letsencrypt certificate is renewed every 3 months)
+ }
+
public static String getContentTypeByFileName(String fileName) {
FileNameMap mime = URLConnection.getFileNameMap();
Modified: ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java?rev=1845418&r1=1845417&r2=1845418&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java Thu Nov 1 09:36:35 2018
@@ -1008,58 +1008,16 @@ public class RequestHandler {
if (Debug.verboseOn()) Debug.logVerbose("The ContentType for the " + view + " view is: " + contentType, module);
+ //Cache Headers
boolean viewNoCache = viewMap.noCache;
if (viewNoCache) {
UtilHttp.setResponseBrowserProxyNoCache(resp);
if (Debug.verboseOn()) Debug.logVerbose("Sending no-cache headers for view [" + nextPage + "]", module);
}
- // Security headers vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
- // See https://cwiki.apache.org/confluence/display/OFBIZ/How+to+Secure+HTTP+Headers
- String xFrameOption = viewMap.xFrameOption;
- // default to sameorigin
- if (UtilValidate.isNotEmpty(xFrameOption)) {
- if(!"none".equals(xFrameOption)) {
- resp.addHeader("x-frame-options", xFrameOption);
- }
- } else {
- resp.addHeader("x-frame-options", "sameorigin");
- }
-
- String strictTransportSecurity = viewMap.strictTransportSecurity;
- // default to "max-age=31536000; includeSubDomains" 31536000 secs = 1 year
- if (UtilValidate.isNotEmpty(strictTransportSecurity)) {
- if (!"none".equals(strictTransportSecurity)) {
- resp.addHeader("strict-transport-security", strictTransportSecurity);
- }
- } else {
- if (EntityUtilProperties.getPropertyAsBoolean("requestHandler", "strict-transport-security", true)) { // FIXME later pass req.getAttribute("delegator") as last argument
- resp.addHeader("strict-transport-security", "max-age=31536000; includeSubDomains");
- }
- }
-
- //The only x-content-type-options defined value, "nosniff", prevents Internet Explorer from MIME-sniffing a response away from the declared content-type.
- // This also applies to Google Chrome, when downloading extensions.
- resp.addHeader("x-content-type-options", "nosniff");
-
- // This header enables the Cross-site scripting (XSS) filter built into most recent web browsers.
- // It's usually enabled by default anyway, so the role of this header is to re-enable the filter for this particular website if it was disabled by the user.
- // This header is supported in IE 8+, and in Chrome (not sure which versions). The anti-XSS filter was added in Chrome 4. Its unknown if that version honored this header.
- // FireFox has still an open bug entry and "offers" only the noscript plugin
- // https://wiki.mozilla.org/Security/Features/XSS_Filter
- // https://bugzilla.mozilla.org/show_bug.cgi?id=528661
- resp.addHeader("X-XSS-Protection","1; mode=block");
+ //Security Headers
+ UtilHttp.setResponseBrowserDefaultSecurityHeaders(resp, viewMap);
- resp.setHeader("Referrer-Policy", "no-referrer-when-downgrade"); // This is the default (in Firefox at least)
-
- //resp.setHeader("Content-Security-Policy", "default-src 'self'");
- //resp.setHeader("Content-Security-Policy-Report-Only", "default-src 'self'; report-uri webtools/control/ContentSecurityPolicyReporter");
- resp.setHeader("Content-Security-Policy-Report-Only", "default-src 'self'");
-
- // TODO in custom project. Public-Key-Pins-Report-Only is interesting but can't be used OOTB because of demos (the letsencrypt certificate is renewed every 3 months)
-
- // Security headers ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
try {
if (Debug.verboseOn()) Debug.logVerbose("Rendering view [" + nextPage + "] of type [" + viewMap.type + "]", module);
ViewHandler vh = viewFactory.getViewHandler(viewMap.type);

Jacques Le Roux

2018-11-01 12:58:28 UTC

Permalink

Right, and there is enough information in the link provided in the top of the method

Done in trunk, R17 and R16

Jacques

Post by Taher Alkhateeb
it is usually not good practice to add commented-out code to the code
base. I recommend removing it. I also think the comments might benefit
from better formatting and structuring instead of this sporadic "//"
sprinkled all over the place