Alexander Wels has uploaded a new change for review. Change subject: userportal,webadmin: xsrf token changes ......................................................................
userportal,webadmin: xsrf token changes - The token was generated in a way that confused people into thinking it did more than it actually did. This patch changes the generation to some random value that is used throughout the session lifetime. Change-Id: Ic028b0d1f8a6fd0cf67863af51d02d892d33f5fb Signed-off-by: Alexander Wels <[email protected]> --- M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/OvirtXsrfTokenServiceServlet.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/XsrfProtectedRpcServlet.java 2 files changed, 57 insertions(+), 72 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/89/31089/1 diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/OvirtXsrfTokenServiceServlet.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/OvirtXsrfTokenServiceServlet.java index 493deff..603c3c3 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/OvirtXsrfTokenServiceServlet.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/OvirtXsrfTokenServiceServlet.java @@ -1,10 +1,13 @@ package org.ovirt.engine.ui.frontend.server.gwt; -import java.nio.charset.Charset; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.SecureRandom; + +import javax.servlet.http.HttpSession; import com.google.gwt.user.client.rpc.XsrfToken; import com.google.gwt.user.server.rpc.XsrfTokenServiceServlet; -import com.google.gwt.util.tools.shared.Md5Utils; import com.google.gwt.util.tools.shared.StringUtils; public class OvirtXsrfTokenServiceServlet extends XsrfTokenServiceServlet { @@ -14,13 +17,57 @@ */ private static final long serialVersionUID = 1854606938563216502L; + /** + * The number of bytes in the token. + */ + private static final int TOKEN_SIZE = 32; + + /** + * The name of the attribute in the {@code HttpSession} that stores the value. + */ + public static final String XSRF_TOKEN = "XSRF_TOKEN"; //$NON-NLS-1$ + + /** + * The random source. + */ + private final SecureRandom random; + + /** + * Constructor that initializes the XSRF token generator. + * @throws NoSuchProviderException + * @throws NoSuchAlgorithmException + */ + OvirtXsrfTokenServiceServlet() throws NoSuchProviderException, NoSuchAlgorithmException { + super(); + random = SecureRandom.getInstance("SHA1PRNG", "SUN"); //$NON-NLS-1$ $NON-NLS-2$ + //Call nextBytes this once to initialize the random implementation. + //We ignore the result, we just called it to initialize the implementation. + byte[] b = new byte[TOKEN_SIZE]; + random.nextBytes(b); + } + @Override public XsrfToken getNewXsrfToken() { return new XsrfToken(generateTokenValueResponse()); } + /** + * Generate the token based on a random value. + * @return A hex based representation of the token value. + */ private String generateTokenValueResponse() { - byte[] tokenBytes = getThreadLocalRequest().getSession().getId().getBytes(Charset.forName("UTF-8")); - return StringUtils.toHexString(Md5Utils.getMd5Digest(tokenBytes)); + byte[] tokenBytes; + HttpSession session = getThreadLocalRequest().getSession(); + synchronized (session) { + if (session.getAttribute(XSRF_TOKEN) == null) { + tokenBytes = new byte[TOKEN_SIZE]; + //nextBytes is thread safe. + random.nextBytes(tokenBytes); + session.setAttribute(XSRF_TOKEN, tokenBytes); + } else { + tokenBytes = (byte[]) session.getAttribute(XSRF_TOKEN); + } + } + return StringUtils.toHexString(tokenBytes); } } diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/XsrfProtectedRpcServlet.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/XsrfProtectedRpcServlet.java index 5dbab56..6a8feca 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/XsrfProtectedRpcServlet.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/XsrfProtectedRpcServlet.java @@ -1,93 +1,31 @@ package org.ovirt.engine.ui.frontend.server.gwt; -import java.io.UnsupportedEncodingException; import java.lang.reflect.Method; -import javax.servlet.ServletException; -import javax.servlet.http.Cookie; +import javax.servlet.http.HttpSession; import com.google.gwt.user.client.rpc.RpcToken; import com.google.gwt.user.client.rpc.RpcTokenException; import com.google.gwt.user.client.rpc.XsrfToken; -import com.google.gwt.user.server.Util; -import com.google.gwt.user.server.rpc.XsrfTokenServiceServlet; -import com.google.gwt.util.tools.shared.Md5Utils; import com.google.gwt.util.tools.shared.StringUtils; public class XsrfProtectedRpcServlet extends AbstractXsrfProtectedRpcServlet { - // Can't use the one from XsrfTokenServiceServlet as it is not public. - static final String COOKIE_NAME_NOT_SET_ERROR_MSG = - "Session cookie name not set! Use context-param to specify session cookie name"; //$NON-NLS-1$ - - String sessionCookieName = null; - /** - * Default constructor. + * Serial version UID. */ - public XsrfProtectedRpcServlet() { - this(null); - } - - /** - * Constructor with session cookie name. - * @param cookieName The session cookie name. - */ - public XsrfProtectedRpcServlet(String cookieName) { - this.sessionCookieName = cookieName; - } - - /** - * Constructor with delegate. - * @param delegate The delegate object - */ - public XsrfProtectedRpcServlet(Object delegate) { - this(delegate, null); - } - - /** - * Constructor with cookie name and delegate. - * @param delegate The delegate object - * @param cookieName The name of the session cookie. - */ - public XsrfProtectedRpcServlet(Object delegate, String cookieName) { - super(delegate); - this.sessionCookieName = cookieName; - } - - @Override - public void init() throws ServletException { - // do not overwrite if value is supplied in constructor - if (sessionCookieName == null) { - // servlet configuration precedes context configuration - sessionCookieName = getServletConfig().getInitParameter(XsrfTokenServiceServlet.COOKIE_NAME_PARAM); - if (sessionCookieName == null) { - sessionCookieName = getServletContext().getInitParameter(XsrfTokenServiceServlet.COOKIE_NAME_PARAM); - } - if (sessionCookieName == null) { - throw new IllegalStateException(COOKIE_NAME_NOT_SET_ERROR_MSG); - } - } - } + private static final long serialVersionUID = 5287411961020889823L; @Override protected void validateXsrfToken(RpcToken token, Method method) { if (token == null) { throw new RpcTokenException("XSRF token missing"); //$NON-NLS-1$ } - Cookie sessionCookie = Util.getCookie(getThreadLocalRequest(), sessionCookieName, false); - if (sessionCookie == null || sessionCookie.getValue() == null - || sessionCookie.getValue().length() == 0) { - throw new RpcTokenException("Session cookie is missing or empty! " + //$NON-NLS-1$ - "Unable to verify XSRF cookie"); //$NON-NLS-1$ - } - String expectedToken; - try { + HttpSession session = getThreadLocalRequest().getSession(); + synchronized (session) { expectedToken = StringUtils.toHexString( - Md5Utils.getMd5Digest(sessionCookie.getValue().getBytes("UTF8"))); //$NON-NLS-1$ - } catch (UnsupportedEncodingException e) { - throw new RpcTokenException("Unable to determine XSRF token from cookie"); //$NON-NLS-1$ + (byte[]) session.getAttribute(OvirtXsrfTokenServiceServlet.XSRF_TOKEN)); } XsrfToken xsrfToken = (XsrfToken) token; -- To view, visit http://gerrit.ovirt.org/31089 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic028b0d1f8a6fd0cf67863af51d02d892d33f5fb Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
