Thanks Jacques :) Done in trunk r1846632 R17.12 r1846633 R16.11 r1846634
Thanks and Regards, *Aditya Sharma* | Enterprise Software Engineer HotWax Commerce <http://www.hotwax.co/> by HotWax Systems <http://www.hotwaxsystems.com/> [image: https://www.linkedin.com/in/aditya-p-sharma/] <https://www.linkedin.com/in/aditya-p-sharma/> On Sun, Nov 11, 2018 at 4:28 PM Jacques Le Roux < [email protected]> wrote: > Hi Aditya, > > Yes indeed, that's a good idea. Please feel free to refactor. > > Thanks > > Jacques > > > Le 10/11/2018 à 16:58, Aditya Sharma a écrit : > > Hi Jacques, > > > > Just a suggestion, instead of having multiple null checks for > > viewMap separately for each security header we can have a single combined > > null check at the top in setResponseBrowserDefaultSecurityHeaders() > method. > > > > + String xFrameOption = null; > > + if (viewMap != null) { > > + xFrameOption = viewMap.xFrameOption; > > + } > > > > + String strictTransportSecurity = null; > > + if (viewMap != null) { > > + strictTransportSecurity = viewMap.strictTransportSecurity; > > + } > > > > can be changed to > > > > + String xFrameOption = null; > > + String strictTransportSecurity = null; > > + if (viewMap != null) { > > + xFrameOption = viewMap.xFrameOption; > > + strictTransportSecurity = viewMap.strictTransportSecurity; > > + } > > > > With increasing security headers these may become expensive. > > > > WDYT? > > > > Thanks and Regards, > > > > *Aditya Sharma* | Enterprise Software Engineer > > HotWax Commerce <http://www.hotwax.co/> by HotWax Systems > > <http://www.hotwaxsystems.com/> > > [image: https://www.linkedin.com/in/aditya-p-sharma/] > > <https://www.linkedin.com/in/aditya-p-sharma/> > > > > > > On Thu, Nov 1, 2018 at 3:07 PM <[email protected]> wrote: > > > >> Author: jleroux > >> Date: Thu Nov 1 09:37:06 2018 > >> New Revision: 1845419 > >> > >> URL: http://svn.apache.org/viewvc?rev=1845419&view=rev > >> Log: > >> "Applied fix from trunk for revision: 1845418" > >> ------------------------------------------------------------------------ > >> r1845418 | jleroux | 2018-11-01 10:36:35 +0100 (jeu. 01 nov. 2018) | 20 > >> lignes > >> > >> 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/branches/release17.12/ (props changed) > >> > >> > ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java > >> > >> > ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java > >> > >> Propchange: ofbiz/ofbiz-framework/branches/release17.12/ > >> > >> > ------------------------------------------------------------------------------ > >> --- svn:mergeinfo (original) > >> +++ svn:mergeinfo Thu Nov 1 09:37:06 2018 > >> @@ -10,4 +10,4 @@ > >> /ofbiz/branches/json-integration-refactoring:1634077-1635900 > >> /ofbiz/branches/multitenant20100310:921280-927264 > >> /ofbiz/branches/release13.07:1547657 > >> > >> > -/ofbiz/ofbiz-framework/trunk:1819499,1819598,1819800,1819805,1819811,1820038,1820262,1820374-1820375,1820441,1820457,1820644,1820658,1820790,1820823,1820949,1820966,1821012,1821036,1821112,1821115,1821144,1821186,1821219,1821226,1821230,1821386,1821613,1821628,1821965,1822125,1822310,1822377,1822383,1822393,1823467,1823562,1823876,1824314,1824316,1824732,1824803,1824847,1824855,1825192,1825211,1825216,1825233,1825450,1826374,1826502,1826592,1826671,1826674,1826805,1826938,1826997,1827439,1828255,1828316,1828346,1828424,1828512,1828514,1829690,1830936,1831074,1831078,1831234,1831608,1831831,1832577,1832662,1832756,1832800,1832944,1833173,1833211,1834181,1834191,1834736,1835235,1835887,1835891,1835953,1835964,1836144,1836871,1837857,1838032,1838256,1838381,1840189,1840199,1840828,1841657,1841662,1842372,1842921,1843225,1843893,1844943 > >> > >> > +/ofbiz/ofbiz-framework/trunk:1819499,1819598,1819800,1819805,1819811,1820038,1820262,1820374-1820375,1820441,1820457,1820644,1820658,1820790,1820823,1820949,1820966,1821012,1821036,1821112,1821115,1821144,1821186,1821219,1821226,1821230,1821386,1821613,1821628,1821965,1822125,1822310,1822377,1822383,1822393,1823467,1823562,1823876,1824314,1824316,1824732,1824803,1824847,1824855,1825192,1825211,1825216,1825233,1825450,1826374,1826502,1826592,1826671,1826674,1826805,1826938,1826997,1827439,1828255,1828316,1828346,1828424,1828512,1828514,1829690,1830936,1831074,1831078,1831234,1831608,1831831,1832577,1832662,1832756,1832800,1832944,1833173,1833211,1834181,1834191,1834736,1835235,1835887,1835891,1835953,1835964,1836144,1836871,1837857,1838032,1838256,1838381,1840189,1840199,1840828,1841657,1841662,1842372,1842921,1843225,1843893,1844943,1845418 > >> > >> Modified: > >> > ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java > >> URL: > >> > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java?rev=1845419&r1=1845418&r2=1845419&view=diff > >> > >> > ============================================================================== > >> --- > >> > ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java > >> (original) > >> +++ > >> > ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java > >> Thu Nov 1 09:37:06 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; > >> @@ -981,6 +982,58 @@ public final class UtilHttp { > >> response.addHeader("Cache-Control", "post-check=0, > pre-check=0, > >> false"); > >> 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/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java > >> URL: > >> > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java?rev=1845419&r1=1845418&r2=1845419&view=diff > >> > >> > ============================================================================== > >> --- > >> > ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java > >> (original) > >> +++ > >> > ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java > >> Thu Nov 1 09:37:06 2018 > >> @@ -941,58 +941,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); > >> > >> > >> > >
