[ofbiz-framework] branch trunk updated (ba548f6 -> ba707d4)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[ofbiz-framework] branch trunk updated (ba548f6 -> ba707d4)

jleroux@apache.org
This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a change to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git.


    from ba548f6  Merge branch 'JacquesLeRoux-POC-for-CSRF-Token-OFBIZ-11306' into trunk Because of GitHub message on PR56: This branch cannot be rebased due to conflicts
     new e0f0d52  Fixed: Prevent Host Header Injection (CVE-2019-12425)
     new 05354b7  Improved: prevent [rawtypes] found raw type: List at RequestHandler.java:89
     new ba707d4  Implemented: POC for CSRF Token (OFBIZ-11306)

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../src/main/java/org/apache/ofbiz/base/util/UtilMisc.java  | 13 +++++++++++++
 framework/security/config/security.properties               | 13 ++++++++++---
 .../src/main/java/org/apache/ofbiz/security/CsrfUtil.java   |  2 +-
 .../org/apache/ofbiz/webapp/control/RequestHandler.java     | 11 ++++++++++-
 4 files changed, 34 insertions(+), 5 deletions(-)

Reply | Threaded
Open this post in threaded view
|

[ofbiz-framework] 01/03: Fixed: Prevent Host Header Injection (CVE-2019-12425)

jleroux@apache.org
This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit e0f0d5271a0443fe97fe11368df327e2c734604a
Author: Jacques Le Roux <[hidden email]>
AuthorDate: Sat Apr 4 19:32:02 2020 +0200

    Fixed: Prevent Host Header Injection (CVE-2019-12425)
   
    (OFBIZ-11583)
---
 .../src/main/java/org/apache/ofbiz/base/util/UtilMisc.java  | 13 +++++++++++++
 framework/security/config/security.properties               |  4 ++++
 .../org/apache/ofbiz/webapp/control/RequestHandler.java     |  9 +++++++++
 3 files changed, 26 insertions(+)

diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilMisc.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilMisc.java
index a1b703e..644bcf6 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilMisc.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilMisc.java
@@ -604,6 +604,19 @@ public final class UtilMisc {
         return LocaleHolder.availableLocaleList;
     }
 
+    /** List of domains or IP addresses to be checked to prevent Host Header Injection,
+     * no spaces after commas,no wildcard, can be extended of course...
+     * @return List of domains or IP addresses to be checked to prevent Host Header Injection,
+     */
+    public static List<String> getHostHeadersAllowed() {
+        String hostHeadersAllowedString = UtilProperties.getPropertyValue("security", "host-headers-allowed", "localhost");
+        List<String> hostHeadersAllowed = null;
+        if (UtilValidate.isNotEmpty(hostHeadersAllowedString)) {
+            hostHeadersAllowed = StringUtil.split(hostHeadersAllowedString, ",");
+        }
+        return Collections.unmodifiableList(hostHeadersAllowed);
+    }
+
     /** @deprecated use Thread.sleep() */
     @Deprecated
     public static void staticWait(long timeout) throws InterruptedException {
diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index e019061..d71f7db 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -152,6 +152,10 @@ security.internal.sso.enabled=false
 # -- The secret key for the JWT token signature. Read Passwords and JWT (JSON Web Tokens) usage documentation to choose the way you want to store this key
 security.token.key=security.token.key
 
+# -- List of domains or IP addresses to be checked to prevent Host Header Injection,
+# -- no spaces after commas,no wildcard, can be extended of course...
+host-headers-allowed=localhost,127.0.0.1,demo-trunk.ofbiz.apache.org,demo-stable.ofbiz.apache.org,demo-old.ofbiz.apache.org
+
 # -- By default the SameSite value in SameSiteFilter is strict. This allows to change it to lax if needed  
 SameSiteCookieAttribute=
 
diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
index b18fa8d..a6075a8 100644
--- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
+++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
@@ -86,6 +86,7 @@ public class RequestHandler {
     private final URL controllerConfigURL;
     private final boolean trackServerHit;
     private final boolean trackVisit;
+    private final List hostHeadersAllowed;
     private ControllerConfig ccfg;
 
     public static RequestHandler getRequestHandler(ServletContext servletContext) {
@@ -111,6 +112,9 @@ public class RequestHandler {
 
         this.trackServerHit = !"false".equalsIgnoreCase(context.getInitParameter("track-serverhit"));
         this.trackVisit = !"false".equalsIgnoreCase(context.getInitParameter("track-visit"));
+        
+        hostHeadersAllowed = UtilMisc.getHostHeadersAllowed();
+
     }
 
     public ConfigXMLReader.ControllerConfig getControllerConfig() {
@@ -209,6 +213,11 @@ public class RequestHandler {
     public void doRequest(HttpServletRequest request, HttpServletResponse response, String chain,
             GenericValue userLogin, Delegator delegator) throws RequestHandlerException, RequestHandlerExceptionAllowExternalRequests {
 
+        if (!hostHeadersAllowed.contains(request.getServerName())) {
+            Debug.logError("Domain " + request.getServerName() + " not accepted to prevent host header injection ", module);
+            throw new RequestHandlerException("Domain " + request.getServerName() + " not accepted to prevent host header injection ");
+        }
+                
         final boolean throwRequestHandlerExceptionOnMissingLocalRequest = EntityUtilProperties.propertyValueEqualsIgnoreCase(
                 "requestHandler", "throwRequestHandlerExceptionOnMissingLocalRequest", "Y", delegator);
         long startTime = System.currentTimeMillis();

Reply | Threaded
Open this post in threaded view
|

[ofbiz-framework] 02/03: Improved: prevent [rawtypes] found raw type: List at RequestHandler.java:89

jleroux@apache.org
In reply to this post by jleroux@apache.org
This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit 05354b73b7a0fea5afb734774ee1e79c925e7938
Author: Jacques Le Roux <[hidden email]>
AuthorDate: Sat Apr 4 19:40:26 2020 +0200

    Improved: prevent [rawtypes] found raw type: List at RequestHandler.java:89
---
 .../src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
index a6075a8..0ab73d8 100644
--- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
+++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
@@ -45,7 +45,6 @@ import javax.ws.rs.core.MultivaluedHashMap;
 
 import org.apache.cxf.jaxrs.model.URITemplate;
 import org.apache.ofbiz.base.location.FlexibleLocation;
-import org.apache.ofbiz.security.CsrfUtil;
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.SSLUtil;
 import org.apache.ofbiz.base.util.StringUtil;
@@ -61,6 +60,7 @@ import org.apache.ofbiz.entity.GenericEntityException;
 import org.apache.ofbiz.entity.GenericValue;
 import org.apache.ofbiz.entity.util.EntityQuery;
 import org.apache.ofbiz.entity.util.EntityUtilProperties;
+import org.apache.ofbiz.security.CsrfUtil;
 import org.apache.ofbiz.webapp.OfbizUrlBuilder;
 import org.apache.ofbiz.webapp.control.ConfigXMLReader.ControllerConfig;
 import org.apache.ofbiz.webapp.control.ConfigXMLReader.RequestMap;
@@ -86,7 +86,7 @@ public class RequestHandler {
     private final URL controllerConfigURL;
     private final boolean trackServerHit;
     private final boolean trackVisit;
-    private final List hostHeadersAllowed;
+    private final List<String> hostHeadersAllowed;
     private ControllerConfig ccfg;
 
     public static RequestHandler getRequestHandler(ServletContext servletContext) {

Reply | Threaded
Open this post in threaded view
|

[ofbiz-framework] 03/03: Implemented: POC for CSRF Token (OFBIZ-11306)

jleroux@apache.org
In reply to this post by jleroux@apache.org
This is an automated email from the ASF dual-hosted git repository.

jleroux pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit ba707d45be6b2db77649a5e7695c089c36a0e8c5
Author: Jacques Le Roux <[hidden email]>
AuthorDate: Sun Apr 5 10:48:55 2020 +0200

    Implemented: POC for CSRF Token
    (OFBIZ-11306)
   
    Simple strategy is to rely on SameSite 'strict' value in SameSiteFilter in all
    supported branches. No backport needed with the changes here.
   
    Thanks: James for all the good work we did together :)
---
 framework/security/config/security.properties                    | 9 ++++++---
 .../src/main/java/org/apache/ofbiz/security/CsrfUtil.java        | 2 +-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties
index d71f7db..5e195a3 100644
--- a/framework/security/config/security.properties
+++ b/framework/security/config/security.properties
@@ -156,7 +156,8 @@ security.token.key=security.token.key
 # -- no spaces after commas,no wildcard, can be extended of course...
 host-headers-allowed=localhost,127.0.0.1,demo-trunk.ofbiz.apache.org,demo-stable.ofbiz.apache.org,demo-old.ofbiz.apache.org
 
-# -- By default the SameSite value in SameSiteFilter is strict. This allows to change it to lax if needed  
+# -- By default the SameSite value in SameSiteFilter is 'strict'. This property allows to change to 'lax' if needed
+# -- If you use 'lax' we recommend that you set org.apache.ofbiz.security.CsrfDefenseStrategy for csrf.defense.strategy (see below)
 SameSiteCookieAttribute=
 
 # -- The cache size for the Tokens Maps that stores the CSRF tokens.
@@ -174,6 +175,8 @@ csrf.tokenName.nonAjax=
 # -- Default is 3
 csrf.entity.request.limit=
 
-# csrf defense strategy. Default is org.apache.ofbiz.security.CsrfDefenseStrategy if not specified.
-# use org.apache.ofbiz.security.NoCsrfDefenseStrategy to disable CSRF check totally.
+# -- CSRF defense strategy.
+# -- Because OFBiz OOTB also sets the SameSite attribute to 'strict' for all cookies,
+# -- default is org.apache.ofbiz.security.NoCsrfDefenseStrategy if not specified.
+# -- Use org.apache.ofbiz.security.CsrfDefenseStrategy if you want to use a 'lax' for SameSiteCookieAttribute
 csrf.defense.strategy=
\ No newline at end of file
diff --git a/framework/security/src/main/java/org/apache/ofbiz/security/CsrfUtil.java b/framework/security/src/main/java/org/apache/ofbiz/security/CsrfUtil.java
index 9d400b8..fa31219 100644
--- a/framework/security/src/main/java/org/apache/ofbiz/security/CsrfUtil.java
+++ b/framework/security/src/main/java/org/apache/ofbiz/security/CsrfUtil.java
@@ -61,7 +61,7 @@ public class CsrfUtil {
 
     static {
         try {
-            String className = UtilProperties.getPropertyValue("security", "csrf.defense.strategy", CsrfDefenseStrategy.class.getCanonicalName());
+            String className = UtilProperties.getPropertyValue("security", "csrf.defense.strategy", NoCsrfDefenseStrategy.class.getCanonicalName());
             Class<?> c = Class.forName(className);
             strategy = (ICsrfDefenseStrategy)c.newInstance();
         } catch (Exception e){