svn commit: r1823324 - in /ofbiz/ofbiz-framework/trunk/f
Locked
svn commit: r1823324 - in /ofbiz/ofbiz-framework/trunk/f
 
|
Author: jleroux
Date: Tue Feb 6 13:17:57 2018 New Revision: 1823324 URL: http://svn.apache.org/viewvc?rev=1823324&view=rev Log: 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/trunk/framework/common/groovyScripts/ExternalServerName.groovy Modified: ofbiz/ofbiz-framework/trunk/framework/security/config/security.properties ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java Modified: ofbiz/ofbiz-framework/trunk/framework/security/config/security.properties URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/security/config/security.properties?rev=1823324&r1=1823323&r2=1823324&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/security/config/security.properties (original) +++ ofbiz/ofbiz-framework/trunk/framework/security/config/security.properties Tue Feb 6 13:17:57 2018 @@ -136,10 +136,8 @@ security.login.externalLoginKey.enabled= ### With this example, the external-server-query must be /catalog/control/ # -- 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) ex: demo-trunk.ofbiz.apache.org where the port is not needed -external-server-name=demo-trunk.ofbiz.apache.org -# -- Query part of the URL to use -external-server-query=/catalog/control/ +use-external-server=Y +# -- 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/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java?rev=1823324&r1=1823323&r2=1823324&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java Tue Feb 6 13:17:57 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/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java?rev=1823324&r1=1823323&r2=1823324&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ExternalLoginKeysManager.java Tue Feb 6 13:17:57 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/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java?rev=1823324&r1=1823323&r2=1823324&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java Tue Feb 6 13:17:57 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"); |
«
Return to OFBiz - Commits
|
1 view|%1 views
| Free forum by Nabble | Edit this page |