Re: svn commit: r1845418 - in /ofbiz/ofbiz-framework/trunk/f
Locked
 
|
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 On Thu, Nov 1, 2018 at 12:36 PM <[hidden email]> wrote: > > Author: jleroux > Date: Thu Nov 1 09:36:35 2018 > New Revision: 1845418 > > URL: http://svn.apache.org/viewvc?rev=1845418&view=rev > Log: > Fixed: Missing Security and Cache Headers in CMS Events > Fixed: > (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 > > Modified: > 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); > > |
Re: svn commit: r1845418 - in /ofbiz/ofbiz-framework/trunk/f
Administrator
|
Right, and there is enough information in the link provided in the top of the method
Done in trunk, R17 and R16 Jacques Le 01/11/2018 à 13:01, Taher Alkhateeb a écrit : > 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 > On Thu, Nov 1, 2018 at 12:36 PM <[hidden email]> wrote: >> Author: jleroux >> Date: Thu Nov 1 09:36:35 2018 >> New Revision: 1845418 >> >> URL: http://svn.apache.org/viewvc?rev=1845418&view=rev >> Log: >> Fixed: Missing Security and Cache Headers in CMS Events >> Fixed: >> (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 >> >> Modified: >> 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); >> >> |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |