Embed the XSRF token in the host page again
If the servlet container forces 'HttpOnly' on us for our account
session cookie, we can't use that to prevent cross site forgery.
Instead embed a token that is unique to this session into the host
page, and have the web UI echo that token back on each request.
We'll validate the token matches the session cookie on the server,
and then simply never rotate it within the lifespan of the session.
Change-Id: Ia9678335b7446eab8a6ee7f043e03f928707b1ad
Signed-off-by: Shawn O. Pearce <sop@google.com>
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/HostPageData.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/HostPageData.java
index 66b0c3b..d90d68b 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/HostPageData.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/HostPageData.java
@@ -21,6 +21,7 @@
public class HostPageData {
public Account account;
public AccountDiffPreference accountDiffPref;
+ public String xsrfToken;
public GerritConfig config;
public Theme theme;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
index 43ac009..4b3070f 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
@@ -73,13 +73,13 @@
public static final GerritResources RESOURCES =
GWT.create(GerritResources.class);
public static final SystemInfoService SYSTEM_SVC;
- private static final String SESSION_COOKIE = "GerritAccount";
private static String myHost;
private static GerritConfig myConfig;
private static HostPageData.Theme myTheme;
private static Account myAccount;
private static AccountDiffPreference myAccountDiffPref;
+ private static String xsrfToken;
private static MorphingTabPanel menuLeft;
private static LinkMenuBar menuRight;
@@ -265,10 +265,15 @@
}
static void deleteSessionCookie() {
- Cookies.removeCookie(SESSION_COOKIE);
myAccount = null;
myAccountDiffPref = null;
+ xsrfToken = null;
refreshMenuBar();
+
+ // If the cookie was HttpOnly, this request to delete it will
+ // most likely not be successful. We can try anyway though.
+ //
+ Cookies.removeCookie("GerritAccount");
}
public void onModuleLoad() {
@@ -305,6 +310,7 @@
myTheme = result.theme;
if (result.account != null) {
myAccount = result.account;
+ xsrfToken = result.xsrfToken;
}
if (result.accountDiffPref != null) {
myAccountDiffPref = result.accountDiffPref;
@@ -437,7 +443,7 @@
JsonUtil.setDefaultXsrfManager(new XsrfManager() {
@Override
public String getToken(JsonDefTarget proxy) {
- return Cookies.getCookie(SESSION_COOKIE);
+ return xsrfToken;
}
@Override
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java
index 9cf678a..d2fd6ee 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSession.java
@@ -124,12 +124,14 @@
return val != null;
}
- String getToken() {
- return isSignedIn() ? key.getToken() : null;
+ public String getToken() {
+ return isSignedIn() ? val.getXsrfToken() : null;
}
public boolean isTokenValid(final String inputToken) {
- return isSignedIn() && key.getToken().equals(inputToken);
+ return isSignedIn() //
+ && val.getXsrfToken() != null //
+ && val.getXsrfToken().equals(inputToken);
}
public AccountExternalId.Key getLastLoginExternalId() {
@@ -152,7 +154,7 @@
}
key = manager.createKey(id);
- val = manager.createVal(key, id, rememberMe, identity);
+ val = manager.createVal(key, id, rememberMe, identity, null);
saveCookie();
}
@@ -164,7 +166,7 @@
/** Set the user account for this current request only. */
void setUserAccountId(Account.Id id) {
key = new Key("id:" + id);
- val = new Val(id, 0, false, null);
+ val = new Val(id, 0, false, null, "");
}
public void logout() {
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSessionManager.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSessionManager.java
index 43c99ad..4ae5c30 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSessionManager.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSessionManager.java
@@ -78,12 +78,13 @@
final Account.Id who = val.getAccountId();
final boolean remember = val.isPersistentCookie();
final AccountExternalId.Key lastLogin = val.getExternalId();
+ final String xsrfToken = val.getXsrfToken();
- return createVal(key, who, remember, lastLogin);
+ return createVal(key, who, remember, lastLogin, xsrfToken);
}
Val createVal(final Key key, final Account.Id who, final boolean remember,
- final AccountExternalId.Key lastLogin) {
+ final AccountExternalId.Key lastLogin, String xsrfToken) {
// Refresh the cookie every hour or when it is half-expired.
// This reduces the odds that the user session will be kicked
// early but also avoids us needing to refresh the cookie on
@@ -94,7 +95,17 @@
final long refresh = Math.min(halfAgeRefresh, minRefresh);
final long refreshCookieAt = now() + refresh;
- final Val val = new Val(who, refreshCookieAt, remember, lastLogin);
+ if (xsrfToken == null) {
+ // If we don't yet have a token for this session, establish one.
+ //
+ final int nonceLen = 20;
+ final ByteArrayOutputStream buf;
+ final byte[] rnd = new byte[nonceLen];
+ prng.nextBytes(rnd);
+ xsrfToken = CookieBase64.encode(rnd);
+ }
+
+ Val val = new Val(who, refreshCookieAt, remember, lastLogin, xsrfToken);
self.put(key, val);
return val;
}
@@ -162,13 +173,16 @@
private transient long refreshCookieAt;
private transient boolean persistentCookie;
private transient AccountExternalId.Key externalId;
+ private transient String xsrfToken;
Val(final Account.Id accountId, final long refreshCookieAt,
- final boolean persistentCookie, final AccountExternalId.Key externalId) {
+ final boolean persistentCookie, final AccountExternalId.Key externalId,
+ final String xsrfToken) {
this.accountId = accountId;
this.refreshCookieAt = refreshCookieAt;
this.persistentCookie = persistentCookie;
this.externalId = externalId;
+ this.xsrfToken = xsrfToken;
}
Account.Id getAccountId() {
@@ -187,6 +201,10 @@
return persistentCookie;
}
+ String getXsrfToken() {
+ return xsrfToken;
+ }
+
private void writeObject(final ObjectOutputStream out) throws IOException {
writeVarInt32(out, 1);
writeVarInt32(out, accountId.get());
@@ -202,6 +220,9 @@
writeString(out, externalId.get());
}
+ writeVarInt32(out, 5);
+ writeString(out, xsrfToken);
+
writeVarInt32(out, 0);
}
@@ -223,6 +244,9 @@
case 4:
externalId = new AccountExternalId.Key(readString(in));
continue;
+ case 5:
+ xsrfToken = readString(in);
+ continue;
default:
throw new IOException("Unknown tag found in object: " + tag);
}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/HostPageServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/HostPageServlet.java
index 1c2e5b8..450d757 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/HostPageServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/HostPageServlet.java
@@ -17,6 +17,7 @@
import com.google.gerrit.common.data.GerritConfig;
import com.google.gerrit.common.data.HostPageData;
import com.google.gerrit.httpd.HtmlDomUtil;
+import com.google.gerrit.httpd.WebSession;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.SitePaths;
@@ -61,6 +62,7 @@
private static final String HPD_ID = "gerrit_hostpagedata";
private final Provider<CurrentUser> currentUser;
+ private final Provider<WebSession> session;
private final GerritConfig config;
private final HostPageData.Theme signedOutTheme;
private final HostPageData.Theme signedInTheme;
@@ -71,10 +73,12 @@
private volatile Page page;
@Inject
- HostPageServlet(final Provider<CurrentUser> cu, final SitePaths sp,
- final ThemeFactory themeFactory, final GerritConfig gc,
- final ServletContext servletContext) throws IOException, ServletException {
+ HostPageServlet(final Provider<CurrentUser> cu, final Provider<WebSession> w,
+ final SitePaths sp, final ThemeFactory themeFactory,
+ final GerritConfig gc, final ServletContext servletContext)
+ throws IOException, ServletException {
currentUser = cu;
+ session = w;
config = gc;
signedOutTheme = themeFactory.getSignedOutTheme();
signedInTheme = themeFactory.getSignedInTheme();
@@ -163,6 +167,10 @@
json(((IdentifiedUser) user).getAccount(), w);
w.write(";");
+ w.write(HPD_ID + ".xsrfToken=");
+ json(session.get().getToken(), w);
+ w.write(";");
+
w.write(HPD_ID + ".accountDiffPref=");
json(((IdentifiedUser) user).getAccountDiffPreference(), w);
w.write(";");
