svn commit: r1845419 - in /ofbiz/ofbiz-framework/branches/release17.12: ./ framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java

Discussion:

Aditya Sharma

2018-11-10 15:58:22 UTC

Permalink

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/>

Author: jleroux
Date: Thu Nov 1 09:37:06 2018
New Revision: 1845419
URL: http://svn.apache.org/viewvc?rev=1845419&view=rev
"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
(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/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
ofbiz/ofbiz-framework/branches/release17.12/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java
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();
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
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);

Jacques Le Roux

2018-11-11 10:56:52 UTC

Permalink

Hi Aditya,

Yes indeed, that's a good idea. Please feel free to refactor.

Thanks

Jacques

Post by Aditya Sharma
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/>

Aditya Sharma

2018-11-15 10:54:21 UTC

Permalink

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 <

Post by Jacques Le Roux
Hi Aditya,
Yes indeed, that's a good idea. Please feel free to refactor.
Thanks
Jacques

method.

Post by Aditya Sharma
+ 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/>

CMS.