Author: jleroux
Date: Tue Feb 6 13:21:34 2018
New Revision: 1823325
URL: http://svn.apache.org/viewvc?rev=1823325&view=rev
Log:
"Applied fix from trunk for revision: 1823324 "
------------------------------------------------------------------------
r1823324 | jleroux | 2018-02-06 14:17:57 +0100 (mar., 06 févr. 2018) | 26 lines
Fixed: Security issue in Token Based Authentication
(OFBIZ-10206)
The version I commitetd so far in OFBIZ-9833 has a small security issue.
See the Jira description for all details
To test I have attached a OFBIZ-10206-external-server-test-example.patch to
the Jira
This removes the external-server-query property now useless
In ContextFilter the getHeader (wrapper) now uses an autoLoginCookie to get
the userLoginId passed in the JWT instead of externalServerUserLogin parameter.
A sourceServerWebappName parameter must be passed from the client request to
allow reading the autoLoginCookie.
This userLoginId is then retrieved on the target server from the JWT in the
externalServerLoginCheck which is simplified.
In LoginWorker
getAutoLoginCookieName() has now 2 versions to allow to pass a webappname
A new autoLogoutFromAllBackendSessions() method has been added but for now
commented out. Decommenting it out will be submitted as a patch in OFBIZ-4959.
Thanks: Leila Mekika for reporting the security issue directly to me
------------------------------------------------------------------------
Removed:
ofbiz/ofbiz-framework/branches/release17.12/framework/common/groovyScripts/ExternalServerName.groovy
Modified:
ofbiz/ofbiz-framework/branches/release17.12/ (props changed)
ofbiz/ofbiz-framework/branches/release17.12/framework/security/config/security.properties
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java
Propchange: ofbiz/ofbiz-framework/branches/release17.12/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Feb 6 13:21:34 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,1821600,1821613,1821628,1821965,1822125,1822310,1822377,1822383,1822393,1822882
+/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,1821600,1821613,1821628,1821965,1822125,1822310,1822377,1822383,1822393,1822882,1823324
Modified:
ofbiz/ofbiz-framework/branches/release17.12/framework/security/config/security.properties
URL:
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release17.12/framework/security/config/security.properties?rev=1823325&r1=1823324&r2=1823325&view=diff
==============================================================================
---
ofbiz/ofbiz-framework/branches/release17.12/framework/security/config/security.properties
(original)
+++
ofbiz/ofbiz-framework/branches/release17.12/framework/security/config/security.properties
Tue Feb 6 13:21:34 2018
@@ -137,9 +137,7 @@ security.login.externalLoginKey.enabled=
# -- If true, then it's possible to connect to another webapp on another
server w/o signing in
# -- This needs to be changed on both the source server and the target server
use-external-server=N
-# -- Name of the external server (DNS)
-external-server-name=demo-trunk.ofbiz.apache.org
-# -- Query part of the URL to use
-external-server-query=/catalog/control/
+# -- Name of the external server (DNS) ex: demo-trunk.ofbiz.apache.org where
the port is not needed, or localhost:8443 (default) for local tests (not using
the same webapp)
+external-server-name=localhost:8443
# -- Time To Live of the token send to the external server in seconds
external-server-token-duration=30
Modified:
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
URL:
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java?rev=1823325&r1=1823324&r2=1823325&view=diff
==============================================================================
---
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
(original)
+++
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
Tue Feb 6 13:21:34 2018
@@ -191,19 +191,23 @@ public class ContextFilter implements Fi
HttpServletRequestWrapper wrapper = new
HttpServletRequestWrapper(httpRequest) {
@Override
public String getHeader(String name) {
- String externalServerUserLoginId =
request.getParameter(ExternalLoginKeysManager.EXTERNAL_SERVER_LOGIN_KEY);
+ String sourceWebappName =
request.getParameter(ExternalLoginKeysManager.SOURCE_SERVER_WEBAPP_NAME);
String value = null;
- if (externalServerUserLoginId != null) {
- // ExternalLoginKeysManager .createJwt() arguments in
order:
- // id an Id, I suggest userLoginId
- // issuer is who/what issued the token. I suggest the
server DNS
- // subject is the subject of the token. I suggest the
destination webapp
- // timeToLive is the token maximum duration
- String webAppName =
UtilHttp.getApplicationName(httpRequest);
- String dnsName =
ExternalLoginKeysManager.getExternalServerName(httpRequest);
- long timeToLive =
ExternalLoginKeysManager.getJwtTokenTimeToLive(httpRequest);
- // We would need a Bearer token (in Authorization request
header) if we were using Oauth2, here we don't, so no Bearer
- value =
ExternalLoginKeysManager.createJwt(externalServerUserLoginId, dnsName,
webAppName , timeToLive);
+ if (sourceWebappName != null) {
+ HttpServletRequest httpRequest = (HttpServletRequest)
request;
+ String userLoginId =
LoginWorker.getAutoUserLoginId(httpRequest, sourceWebappName);
+ if (userLoginId != null) { // At this stage the user must
be logged in. But safer to check because we can't grab it from the session here.
+ // ExternalLoginKeysManager.createJwt() arguments
in order:
+ // id an Id, here userLoginId
+ // issuer is who/what issued the token, here the
server URL
+ // subject is the subject of the token, here the
target webapp
+ // timeToLive is the token maximum duration,
default 30 seconds
+ String targetWebAppName =
UtilHttp.getApplicationName(httpRequest);
+ String targetServerUrl =
ExternalLoginKeysManager.getTargetServerUrl(httpRequest);
+ long timeToLive =
ExternalLoginKeysManager.getJwtTokenTimeToLive(httpRequest);
+ // We would need a Bearer token (in Authorization
request header) if we were using Oauth2, here we don't, so no Bearer
+ value =
ExternalLoginKeysManager.createJwt(userLoginId, targetServerUrl,
targetWebAppName , timeToLive);
+ }
}
if (value != null) return value;
return super.getHeader(name);
Modified:
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java
URL:
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java?rev=1823325&r1=1823324&r2=1823325&view=diff
==============================================================================
---
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java
(original)
+++
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java
Tue Feb 6 13:21:34 2018
@@ -43,9 +43,13 @@ import org.apache.ofbiz.service.LocalDis
import org.apache.ofbiz.webapp.WebAppUtil;
import io.jsonwebtoken.Claims;
+import io.jsonwebtoken.ExpiredJwtException;
import io.jsonwebtoken.JwtBuilder;
import io.jsonwebtoken.Jwts;
+import io.jsonwebtoken.MalformedJwtException;
import io.jsonwebtoken.SignatureAlgorithm;
+import io.jsonwebtoken.SignatureException;
+import io.jsonwebtoken.UnsupportedJwtException;
/**
* This class manages the authentication tokens that provide single sign-on
authentication to the OFBiz applications.
@@ -55,7 +59,7 @@ public class ExternalLoginKeysManager {
private static final String EXTERNAL_LOGIN_KEY_ATTR = "externalLoginKey";
// This Map is keyed by the randomly generated externalLoginKey and the
value is a UserLogin GenericValue object
private static final Map<String, GenericValue> externalLoginKeys = new
ConcurrentHashMap<>();
- public static final String EXTERNAL_SERVER_LOGIN_KEY =
"externalServerLoginKey";
+ public static final String SOURCE_SERVER_WEBAPP_NAME =
"sourceServerWebappName";
// This works the same way than externalLoginKey but between 2 servers,
not 2 webapps on the same server.
// The Single Sign On (SSO) is ensured by a JWT token, then all is handled
as normal by a session on the reached server.
// The servers may or may not share a database but the 2 loginUserIds must
be the same.
@@ -175,23 +179,37 @@ public class ExternalLoginKeysManager {
}
public static String externalServerLoginCheck(HttpServletRequest request,
HttpServletResponse response) {
-
Delegator delegator = (Delegator) request.getAttribute("delegator");
HttpSession session = request.getSession();
- String externalServerUserLoginId =
request.getParameter(EXTERNAL_SERVER_LOGIN_KEY);
- if (externalServerUserLoginId == null) return "success"; // Nothing to
do here
- if (!"Y".equals(EntityUtilProperties.getPropertyValue("security",
"use-external-server", "N", delegator))) return "success"; // The target server
does not allow external login by default
-
- GenericValue currentUserLogin = (GenericValue)
session.getAttribute("userLogin");
+ // The target server does not allow external login by default
+ boolean useExternalServer =
"Y".equals(EntityUtilProperties.getPropertyValue("security",
"use-external-server", "N", delegator));
+ String sourceWebappName =
request.getParameter(SOURCE_SERVER_WEBAPP_NAME);
+ if (!useExternalServer || sourceWebappName == null) return "success";
// Nothing to do here
try {
- GenericValue userLogin =
EntityQuery.use(delegator).from("UserLogin").where("userLoginId",
externalServerUserLoginId).queryOne();
+ String userLoginId = null;
+ String authorizationHeader = request.getHeader("Authorization");
+ if (authorizationHeader != null) {
+ Claims claims = returnsClaims(authorizationHeader);
+ userLoginId = getSourceUserLoginId(claims );
+ boolean jwtOK = checkJwt(authorizationHeader, userLoginId,
getTargetServerUrl(request), UtilHttp.getApplicationName(request));
+ if (!jwtOK) {
+ // Something unexpected happened here
+ Debug.logWarning("*** There was a problem with the JWT
token, not signin in the user login " + userLoginId, module);
+ return "success";
+ }
+ } else {
+ // Nothing to do here
+ return "success";
+ }
+
+
+ GenericValue userLogin =
EntityQuery.use(delegator).from("UserLogin").where("userLoginId",
userLoginId).queryOne();
if (userLogin != null) {
- //to check it's the right tenant
- //in case username and password are the same in different
tenants
+ // Check it's the right tenant in case username and password
are the same in different tenants
+ // TODO : not sure this is really useful in the case of
external server, should not hurt anyway
LocalDispatcher dispatcher = (LocalDispatcher)
request.getAttribute("dispatcher");
- delegator = (Delegator) request.getAttribute("delegator");
String oldDelegatorName = delegator.getDelegatorName();
ServletContext servletContext = session.getServletContext();
if
(!oldDelegatorName.equals(userLogin.getDelegator().getDelegatorName())) {
@@ -199,44 +217,15 @@ public class ExternalLoginKeysManager {
dispatcher =
WebAppUtil.makeWebappDispatcher(servletContext, delegator);
LoginWorker.setWebContextObjects(request, response,
delegator, dispatcher);
}
-
- String authorizationHeader =
request.getHeader("Authorization");
- if (authorizationHeader != null) {
- boolean jwtOK = checkJwt(authorizationHeader,
userLogin.getString("userLoginId"), getExternalServerName(request),
UtilHttp.getApplicationName(request));
- if (!jwtOK) {
- Debug.logWarning("*** There was a problem with the JWT
token, loging out the current user: " + externalServerUserLoginId, module);
- LoginWorker.logout(request, response);
- return "success";
- }
- } else {
- // Something weird happened here => logout current user
- Debug.logWarning("*** There was a problem with the JWT
token, loging out the current user: " + externalServerUserLoginId, module);
- LoginWorker.logout(request, response);
- return "success";
- }
-
- // if the user is already logged in and the login is
different, logout the other user
- if (currentUserLogin != null) {
- if
(currentUserLogin.getString("userLoginId").equals(userLogin.getString("userLoginId")))
{
- // is the same user, just carry on...
- return "success";
- }
-
- // logout the current user and login the new user...
- LoginWorker.logout(request, response);
- // ignore the return value; even if the operation failed
we want to set the new UserLogin
- }
-
- //connect
String enabled = userLogin.getString("enabled");
if (enabled == null || "Y".equals(enabled)) {
userLogin.set("hasLoggedOut", "N");
userLogin.store();
}
- LoginWorker.doBasicLogin(userLogin, request);
} else {
- Debug.logWarning("Could not find userLogin for external login
key: " + externalServerUserLoginId, module);
+ Debug.logWarning("*** There was a problem with the JWT token.
Could not find userLogin " + userLoginId, module);
}
+ LoginWorker.doBasicLogin(userLogin, request);
} catch (GenericEntityException e) {
Debug.logError(e, "Cannot get autoUserLogin information: " +
e.getMessage(), module);
}
@@ -264,11 +253,11 @@ public class ExternalLoginKeysManager {
Key signingKey = new SecretKeySpec(apiKeySecretBytes,
signatureAlgorithm.getJcaName());
//Let's set the JWT Claims
JwtBuilder builder = Jwts.builder().setId(id)
- .setIssuedAt(now)
- .setSubject(subject)
- .setIssuer(issuer)
- .setIssuedAt(now)
- .signWith(signatureAlgorithm, signingKey);
+ .setIssuedAt(now)
+ .setSubject(subject)
+ .setIssuer(issuer)
+ .setIssuedAt(now)
+ .signWith(signatureAlgorithm, signingKey);
//if it has been specified, let's add the expiration date, this should
always be true
if (ttlMillis >= 0) {
@@ -292,6 +281,28 @@ public class ExternalLoginKeysManager {
*/
private static boolean checkJwt(String jwt, String id, String issuer,
String subject) {
//The JWT signature algorithm is using this to sign the token
+ Claims claims = returnsClaims(jwt);
+
+ long nowMillis = System.currentTimeMillis();
+ Date now = new Date(nowMillis);
+
+ return claims.getId().equals(id)
+ && claims.getIssuer().equals(issuer)
+ && claims.getSubject().equals(subject)
+ && claims.getExpiration().after(now);
+ }
+
+ /**
+ * @param jwt a JWT token
+ * @return claims the claims
+ * @throws ExpiredJwtException
+ * @throws UnsupportedJwtException
+ * @throws MalformedJwtException
+ * @throws SignatureException
+ * @throws IllegalArgumentException
+ */
+ private static Claims returnsClaims(String jwt) throws
ExpiredJwtException, UnsupportedJwtException,
+ MalformedJwtException, SignatureException,
IllegalArgumentException {
SignatureAlgorithm signatureAlgorithm = SignatureAlgorithm.HS512;
byte[] apiKeySecretBytes =
DatatypeConverter.parseBase64Binary(ExternalServerJwtMasterSecretKey);
@@ -301,25 +312,21 @@ public class ExternalLoginKeysManager {
Claims claims = Jwts.parser()
.setSigningKey(signingKey)
.parseClaimsJws(jwt).getBody();
-
- long nowMillis = System.currentTimeMillis();
- Date now = new Date(nowMillis);
-
- return claims.getId().equals(id)
- && claims.getIssuer().equals(issuer)
- && claims.getSubject().equals(subject)
- && claims.getExpiration().after(now);
+ return claims;
}
- public static String getExternalServerName(HttpServletRequest request) {
- String reportingServerName = "";
+ private static String getSourceUserLoginId(Claims claims) {
+ return claims.getId();
+ }
+
+ public static String getTargetServerUrl(HttpServletRequest request) {
+ String targetServerUrl = "";
Delegator delegator = (Delegator) request.getAttribute("delegator");
if (delegator != null &&
"Y".equals(EntityUtilProperties.getPropertyValue("security",
"use-external-server", "N", delegator))) {
- reportingServerName =
EntityUtilProperties.getPropertyValue("security", "external-server-name",
"localhost:8443", delegator);
- String reportingServerQuery =
EntityUtilProperties.getPropertyValue("security", "external-server-query",
"/catalog/control/", delegator);
- reportingServerName = "https://"; + reportingServerName +
reportingServerQuery;
+ targetServerUrl =
EntityUtilProperties.getPropertyValue("security", "external-server-name",
"localhost:8443", delegator);
+ targetServerUrl = "https://"; + targetServerUrl;
}
- return reportingServerName;
+ return targetServerUrl;
}
public static long getJwtTokenTimeToLive(HttpServletRequest request) {
Modified:
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java
URL:
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java?rev=1823325&r1=1823324&r2=1823325&view=diff
==============================================================================
---
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java
(original)
+++
ofbiz/ofbiz-framework/branches/release17.12/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java
Tue Feb 6 13:21:34 2018
@@ -622,12 +622,13 @@ public class LoginWorker {
RequestHandler rh =
RequestHandler.getRequestHandler(request.getSession().getServletContext());
rh.runBeforeLogoutEvents(request, response);
-
// invalidate the security group list cache
GenericValue userLogin = (GenericValue)
request.getSession().getAttribute("userLogin");
doBasicLogout(userLogin, request, response);
-
+
+ //autoLogoutFromAllBackendSessions(userLogin, request, response);
+ // TODO check why, seems no sense
if (request.getAttribute("_AUTO_LOGIN_LOGOUT_") == null) {
return autoLoginCheck(request, response);
}
@@ -718,6 +719,14 @@ public class LoginWorker {
return UtilHttp.getApplicationName(request) + ".autoUserLoginId";
}
+ protected static String getAutoLoginCookieName(String webappName) {
+ return webappName + ".autoUserLoginId";
+ }
+
+ /**
+ * @deprecated Moved to {@link
org.apache.ofbiz.webapp.control.LoginWorker#getAutoUserLoginId(HttpServletRequest
request, String webappName) String}
+ */
+ @Deprecated
public static String getAutoUserLoginId(HttpServletRequest request) {
String autoUserLoginId = null;
Cookie[] cookies = request.getCookies();
@@ -734,12 +743,31 @@ public class LoginWorker {
}
return autoUserLoginId;
}
+
+ public static String getAutoUserLoginId(HttpServletRequest request, String
webappName) {
+ String autoUserLoginId = null;
+ Cookie[] cookies = request.getCookies();
+ if (Debug.verboseOn()) {
+ Debug.logVerbose("Cookies: " + Arrays.toString(cookies), module);
+ }
+ if (cookies != null) {
+ for (Cookie cookie: cookies) {
+ String cookieName = (webappName != null) ?
getAutoLoginCookieName(webappName) : getAutoLoginCookieName(request);
+ if (cookie.getName().equals(cookieName)) {
+ autoUserLoginId = cookie.getValue();
+ break;
+ }
+ }
+ }
+ return autoUserLoginId;
+ }
+
public static String autoLoginCheck(HttpServletRequest request,
HttpServletResponse response) {
Delegator delegator = (Delegator) request.getAttribute("delegator");
HttpSession session = request.getSession();
- return autoLoginCheck(delegator, session, getAutoUserLoginId(request));
+ return autoLoginCheck(delegator, session, getAutoUserLoginId(request,
null));
}
private static String autoLoginCheck(Delegator delegator, HttpSession
session, String autoUserLoginId) {
@@ -794,6 +822,34 @@ public class LoginWorker {
return "success";
}
+ public static String autoLogoutFromAllBackendSessions(GenericValue
userLogin, HttpServletRequest request, HttpServletResponse response) {
+ HttpSession session = request.getSession();
+
+ // remove all the autoLoginCookies but if in ecommerce/ecomseo and
webpos (it's done manually there, not sure for webpos TODO: check)
+ Cookie[] cookies = request.getCookies();
+ if (Debug.verboseOn()) {
+ Debug.logVerbose("Cookies: " + Arrays.toString(cookies), module);
+ }
+ if (cookies != null && userLogin != null) {
+ for (Cookie autoLoginCookie: cookies) {
+ if (autoLoginCookie.getName().contains("autoUserLoginId")
+ && !(autoLoginCookie.getName().contains("ecommerce")
+ || autoLoginCookie.getName().contains("ecomseo")
+ || autoLoginCookie.getName().contains("webpos")))
+ autoLoginCookie.setMaxAge(0);
+ autoLoginCookie.setPath("/");
+ response.addCookie(autoLoginCookie);
+ }
+ }
+
+ // remove the session attributes
+ session.removeAttribute("autoUserLogin");
+ session.removeAttribute("autoName");
+
+ request.setAttribute("_AUTO_LOGIN_LOGOUT_", Boolean.TRUE); // TODO
check it's useful
+ return "success";
+ }
+
public static boolean isUserLoggedIn(HttpServletRequest request) {
HttpSession session = request.getSession();
GenericValue currentUserLogin = (GenericValue)
session.getAttribute("userLogin");
