Change XsrfTokenManager to support new HMAC token format

This follows up on Brian's work to transition not just to a new format
with an empty scope value, but instead to replace the existing format
entirely with a new one that:

  1) includes a version number to support future format migrations
  2) doesn't include a field for the scope at all, since scoping the
     tokens adds no real security benefit and just makes verification
     more difficult
  3) replaces the raw SHA-256 hash with a SHA-256 HMAC instead, as a
     best practice to avoid length-extension attacks [1], even though
     in our particular case they would only be able to extend the
     timestamp and would thus be relatively innocuous

The new format will be produced by calling generateToken(), and the
scope-accepting version is renamed to generateLegacyToken() in addition
to its existing deprecation, for maximum clarity.

I changed the validateToken() logic to stop accepting a scope entirely;
when validating a legacy-style token, we'll test it against the two
existing legacy scope values ("admin" and "console") and accept it if
it matches either one.

Note that this means the xsrfScope parameter in @Action is now wholly
obsolete; I'll remove it in a follow-up to avoid bringing extra files
into this CL.

After this CL hits production, the next one will replace all calls to
generateLegacyToken() with generateToken().  Once that CL is deployed,
the last step will be removing the legacy fallback in validateToken().

[1] See https://en.wikipedia.org/wiki/Length_extension_attack

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=148936805

java/google/registry/loadtest/LoadTestAction.java

@@ -185,7 +185,7 @@ public class LoadTestAction implements Runnable {
     xmlHostCreateTmpl = loadXml("host_create");
     xmlHostCreateFail = xmlHostCreateTmpl.replace("%host%", EXISTING_HOST);
     xmlHostInfo = loadXml("host_info").replace("%host%", EXISTING_HOST);
-    xsrfToken = xsrfTokenManager.generateToken("admin", "");
+    xsrfToken = xsrfTokenManager.generateLegacyToken("admin", "");
   }
 
   @Override

java/google/registry/request/RequestHandler.java

@@ -164,9 +164,7 @@ public class RequestHandler<C> {
       return;
     }
     if (route.get().shouldXsrfProtect(method)
-        && !xsrfTokenManager.validateToken(
+        && !xsrfTokenManager.validateToken(nullToEmpty(req.getHeader(X_CSRF_TOKEN)))) {
-                nullToEmpty(req.getHeader(X_CSRF_TOKEN)),
-                route.get().action().xsrfScope())) {
       rsp.sendError(SC_FORBIDDEN, "Invalid " + X_CSRF_TOKEN);
       return;
     }

java/google/registry/request/auth/LegacyAuthenticationMechanism.java

@@ -53,9 +53,7 @@ public class LegacyAuthenticationMechanism implements AuthenticationMechanism {
     }
 
     if (!SAFE_METHODS.contains(request.getMethod())
-        && !xsrfTokenManager.validateToken(
+        && !xsrfTokenManager.validateToken(nullToEmpty(request.getHeader(X_CSRF_TOKEN)))) {
-            nullToEmpty(request.getHeader(X_CSRF_TOKEN)),
-            "console")) { // hard-coded for now; in the long run, this will be removed
       return AuthResult.create(NONE);
     }
 

java/google/registry/security/XsrfTokenManager.java

@@ -14,18 +14,21 @@
 
 package google.registry.security;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.io.BaseEncoding.base64Url;
+import static google.registry.util.PreconditionsUtils.checkArgumentNotNull;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.joda.time.DateTimeZone.UTC;
 
 import com.google.appengine.api.users.UserService;
 import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.hash.Hashing;
 import google.registry.model.server.ServerSecret;
 import google.registry.util.Clock;
 import google.registry.util.FormattingLogger;
 import java.util.List;
-import javax.annotation.Nullable;
 import javax.inject.Inject;
 import org.joda.time.DateTime;
 import org.joda.time.Duration;
@@ -33,13 +36,18 @@ import org.joda.time.Duration;
 /** Helper class for generating and validate XSRF tokens. */
 public final class XsrfTokenManager {
 
-  // TODO(b/35388772): remove the scope parameter
-
   /** HTTP header used for transmitting XSRF tokens. */
   public static final String X_CSRF_TOKEN = "X-CSRF-Token";
 
+  /** Maximum age of an acceptable XSRF token. */
   private static final Duration XSRF_VALIDITY = Duration.standardDays(1);
 
+  /** Token version identifier for version 1. */
+  private static final String VERSION_1 = "1";
+
+  /** Legacy scope values that will be supported during the scope removal process. */
+  private static final ImmutableSet<String> LEGACY_SCOPES = ImmutableSet.of("admin", "console");
+
   private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
 
   private final Clock clock;
@@ -51,18 +59,39 @@ public final class XsrfTokenManager {
     this.userService = userService;
   }
 
+  /** Generates an XSRF token for a given user based on email address. */
+  public String generateToken(String email) {
+    checkArgumentNotNull(email);
+    long timestampMillis = clock.nowUtc().getMillis();
+    return encodeToken(ServerSecret.get().asBytes(), email, timestampMillis);
+  }
+
   /**
-   * Encode a token.
+   * Returns an XSRF token for the given server secret, user email, and timestamp.
    *
-   * <p>The token is a Base64-encoded SHA-256 hash of a string containing the secret, email, scope
+   * <p>The token format consists of three colon-delimited fields: the version number (currently 1),
-   * and creation time, separated by tabs. If the scope is null, the string is secret, email,
+   * the timestamp in milliseconds since the epoch, and the Base64url-encoded SHA-256 HMAC (using
-   * creation time. In the future, the scope option will be removed.
+   * the given secret key) of the user email and the timestamp millis separated by a tab character.
+   *
+   * <p>We use HMAC instead of a plain SHA-256 hash to avoid length-extension vulnerabilities.
    */
-  private static String encodeToken(long creationTime, @Nullable String scope, String userEmail) {
+  private static String encodeToken(byte[] secret, String email, long timestampMillis) {
+    String payload = Joiner.on('\t').skipNulls().join(email, timestampMillis);
+    String hmac =
+        base64Url().encode(Hashing.hmacSha256(secret).hashString(payload, UTF_8).asBytes());
+    return Joiner.on(':').join(VERSION_1, timestampMillis, hmac);
+  }
+
+  /**
+   * Computes the hash payload portion of a legacy-style XSRF token.
+   *
+   * <p>The result is a Base64-encoded SHA-256 hash of a string containing the secret, email, scope
+   * and creation time, separated by tabs.
+   */
+  private static String computeLegacyHash(long creationTime, String scope, String userEmail) {
+    checkArgument(LEGACY_SCOPES.contains(scope), "Invalid scope value: %s", scope);
     String token =
-        Joiner.on('\t')
+        Joiner.on('\t').join(ServerSecret.get().asUuid(), userEmail, scope, creationTime);
-            .skipNulls()
-            .join(ServerSecret.get().asUuid(), userEmail, scope, creationTime);
     return base64Url().encode(Hashing.sha256()
         .newHasher(token.length())
         .putString(token, UTF_8)
@@ -71,86 +100,77 @@ public final class XsrfTokenManager {
   }
 
   /**
-   * Generate an xsrf token for a given scope and user.
+   * Generates a legacy-style XSRF token for a given scope and user.
    *
    * <p>If there is no user (email is an empty string), the entire xsrf check becomes basically a
    * no-op, but that's ok because any callback that doesn't have a user shouldn't be able to access
    * any per-user resources anyways.
    *
-   * <p>The scope (or lack thereof) is passed to {@link #encodeToken}. Use of a scope in xsrf tokens
+   * <p>The scope is passed to {@link #computeLegacyHash}. Use of a scope in xsrf tokens is
-   * is deprecated; instead, use the no-argument version.
+   * deprecated; instead, use {@link #generateToken}.
    */
+  // TODO(b/35388772): remove this in favor of generateToken()
   @Deprecated
-  public String generateToken(@Nullable String scope, String email) {
+  public String generateLegacyToken(String scope, String email) {
+    checkArgumentNotNull(scope);
+    checkArgumentNotNull(email);
     long now = clock.nowUtc().getMillis();
-    return Joiner.on(':').join(encodeToken(now, scope, email), now);
+    return Joiner.on(':').join(computeLegacyHash(now, scope, email), now);
-  }
-
-  /** Generate an xsrf token for a given user. */
-  public String generateToken(String email) {
-    return generateToken(null, email);
-  }
-
-  private String getLoggedInEmailOrEmpty() {
-    return userService.isUserLoggedIn() ? userService.getCurrentUser().getEmail() : "";
   }
 
   /**
-   * Validate an xsrf token, given the scope it was used for.
+   * Validates an XSRF token against the current logged-in user.
    *
-   * <p>We plan to remove the scope parameter. As a first step, the method first checks for the
+   * This accepts both legacy-style and new-style XSRF tokens.  For legacy-style tokens, it will
-   * existence of a token with no scope. If that is not found, it then looks for the existence of a
+   * accept tokens generated with any scope from {@link #LEGACY_SCOPES}.
-   * token with the specified scope. Our next step will be to have clients pass in a null scope.
-   * Finally, we will remove scopes from this code altogether.
-   */
-  @Deprecated
-  public boolean validateToken(String token, @Nullable String scope) {
-    return validateTokenSub(token, scope);
-  }
-
-  /**
-   * Validate an xsrf token.
-   *
-   * <p>This is the non-scoped version to which we will transition in the future.
    */
   public boolean validateToken(String token) {
-    return validateTokenSub(token, null);
+    checkArgumentNotNull(token);
-  }
-
-  private boolean validateTokenSub(String token, @Nullable String scope) {
     List<String> tokenParts = Splitter.on(':').splitToList(token);
-    if (tokenParts.size() != 2) {
+    if (tokenParts.size() < 2) {
       logger.warningfmt("Malformed XSRF token: %s", token);
       return false;
     }
-    String encodedPart = tokenParts.get(0);
     String timePart = tokenParts.get(1);
-    long creationTime;
+    long timestampMillis;
     try {
-      creationTime = Long.parseLong(timePart);
+      timestampMillis = Long.parseLong(timePart);
     } catch (NumberFormatException e) {
       logger.warningfmt("Bad timestamp in XSRF token: %s", token);
       return false;
     }
-    if (new DateTime(creationTime).plus(XSRF_VALIDITY).isBefore(clock.nowUtc())) {
+    if (new DateTime(timestampMillis, UTC).plus(XSRF_VALIDITY).isBefore(clock.nowUtc())) {
       logger.infofmt("Expired timestamp in XSRF token: %s", token);
       return false;
     }
-    // First, check for a scopeless token, because that's the token of the future.
+    String currentUserEmail =
-    String reconstructedToken = encodeToken(creationTime, null, getLoggedInEmailOrEmpty());
+        userService.isUserLoggedIn() ? userService.getCurrentUser().getEmail() : "";
-    if (reconstructedToken.equals(encodedPart)) {
-      return true;
-    }
 
-    // If we don't find one, look for one with the specified scope.
+    // Reconstruct the token to verify validity, using version 1 format if detected.
-    if (scope != null) {
+    if (tokenParts.get(0).equals(VERSION_1)) {
-      reconstructedToken = encodeToken(creationTime, scope, getLoggedInEmailOrEmpty());
+      String reconstructedToken =
-      if (reconstructedToken.equals(encodedPart)) {
+          encodeToken(ServerSecret.get().asBytes(), currentUserEmail, timestampMillis);
-        return true;
+      if (!token.equals(reconstructedToken)) {
-      }
+        logger.warningfmt(
-    }
+            "Reconstructed XSRF mismatch (got != expected): %s != %s", token, reconstructedToken);
-
-    logger.warningfmt("Reconstructed XSRF mismatch: %s ≠ %s", encodedPart, reconstructedToken);
         return false;
       }
+      return true;
+    } else {
+      // Fall back to the legacy format, and try the few possible scopes.
+      String hash = tokenParts.get(0);
+      ImmutableSet.Builder<String> reconstructedTokenCandidates = new ImmutableSet.Builder<>();
+      for (String scope : LEGACY_SCOPES) {
+        String reconstructedHash = computeLegacyHash(timestampMillis, scope, currentUserEmail);
+        reconstructedTokenCandidates.add(reconstructedHash);
+        if (hash.equals(reconstructedHash)) {
+          return true;
+        }
+      }
+      logger.warningfmt(
+          "Reconstructed XSRF mismatch: %s matches none of %s",
+          hash, reconstructedTokenCandidates.build());
+      return false;
+    }
+  }
 }

java/google/registry/tools/AppEngineConnection.java

@@ -66,7 +66,7 @@ class AppEngineConnection implements Connection {
       memoize(new Supplier<String>() {
         @Override
         public String get() {
-          return xsrfTokenManager.generateToken("admin", getUserId());
+          return xsrfTokenManager.generateLegacyToken("admin", getUserId());
         }});
 
   @Override

java/google/registry/ui/server/registrar/ConsoleUiAction.java

@@ -107,7 +107,7 @@ public final class ConsoleUiAction implements Runnable {
     Registrar registrar = Registrar.loadByClientId(sessionUtils.getRegistrarClientId(req));
     data.put(
         "xsrfToken",
-        xsrfTokenManager.generateToken(
+        xsrfTokenManager.generateLegacyToken(
             EppConsoleAction.XSRF_SCOPE, userService.getCurrentUser().getEmail()));
     data.put("clientId", registrar.getClientId());
     data.put("showPaymentLink", registrar.getBillingMethod() == Registrar.BillingMethod.BRAINTREE);

javatests/google/registry/request/RequestHandlerTest.java

@@ -383,18 +383,18 @@ public final class RequestHandlerTest {
     userService.setUser(testUser,  false);
     when(req.getMethod()).thenReturn("POST");
     when(req.getHeader("X-CSRF-Token"))
-        .thenReturn(xsrfTokenManager.generateToken("vampire", testUser.getEmail()));
+        .thenReturn(xsrfTokenManager.generateLegacyToken("admin", testUser.getEmail()));
     when(req.getRequestURI()).thenReturn("/safe-sloth");
     handler.handleRequest(req, rsp);
     verify(safeSlothTask).run();
   }
 
   @Test
-  public void testXsrfProtection_tokenWithInvalidScopeProvided_returns403() throws Exception {
+  public void testXsrfProtection_tokenWithInvalidUserProvided_returns403() throws Exception {
     userService.setUser(testUser,  false);
     when(req.getMethod()).thenReturn("POST");
     when(req.getHeader("X-CSRF-Token"))
-        .thenReturn(xsrfTokenManager.generateToken("blood", testUser.getEmail()));
+        .thenReturn(xsrfTokenManager.generateLegacyToken("admin", "wrong@example.com"));
     when(req.getRequestURI()).thenReturn("/safe-sloth");
     handler.handleRequest(req, rsp);
     verify(rsp).sendError(403, "Invalid X-CSRF-Token");

javatests/google/registry/request/auth/RequestAuthenticatorTest.java

@@ -220,7 +220,7 @@ public class RequestAuthenticatorTest {
   public void testAnyUserAnyMethod_success() throws Exception {
     fakeUserService.setUser(testUser, false /* isAdmin */);
     when(req.getHeader(XsrfTokenManager.X_CSRF_TOKEN))
-        .thenReturn(xsrfTokenManager.generateToken("console", testUser.getEmail()));
+        .thenReturn(xsrfTokenManager.generateLegacyToken("console", testUser.getEmail()));
 
     Optional<AuthResult> authResult = runTest(fakeUserService, AuthAnyUserAnyMethod.class);
 
@@ -275,7 +275,7 @@ public class RequestAuthenticatorTest {
   public void testAdminUserAnyMethod_success() throws Exception {
     fakeUserService.setUser(testUser, true /* isAdmin */);
     when(req.getHeader(XsrfTokenManager.X_CSRF_TOKEN))
-        .thenReturn(xsrfTokenManager.generateToken("console", testUser.getEmail()));
+        .thenReturn(xsrfTokenManager.generateLegacyToken("console", testUser.getEmail()));
 
     Optional<AuthResult> authResult = runTest(fakeUserService, AuthAdminUserAnyMethod.class);
 

javatests/google/registry/security/XsrfTokenManagerTest.java

@@ -16,6 +16,7 @@ package google.registry.security;
 
 import static com.google.common.truth.Truth.assertThat;
 import static google.registry.util.DateTimeUtils.START_OF_TIME;
+import static org.junit.Assert.fail;
 
 import com.google.appengine.api.users.User;
 import com.google.common.base.Splitter;
@@ -23,6 +24,7 @@ import google.registry.testing.AppEngineRule;
 import google.registry.testing.FakeClock;
 import google.registry.testing.FakeUserService;
 import google.registry.testing.InjectRule;
+import org.joda.time.Duration;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -46,65 +48,102 @@ public class XsrfTokenManagerTest {
   private final FakeUserService userService = new FakeUserService();
   private final XsrfTokenManager xsrfTokenManager = new XsrfTokenManager(clock, userService);
 
-  String realToken;
+  private String token;
+  private String legacyToken;
 
   @Before
   public void init() {
     userService.setUser(testUser, false);
-    realToken = xsrfTokenManager.generateToken("console", testUser.getEmail());
+    token = xsrfTokenManager.generateToken(testUser.getEmail());
+    legacyToken = xsrfTokenManager.generateLegacyToken("console", testUser.getEmail());
   }
 
   @Test
-  public void testSuccess() {
+  public void testGenerateLegacyToken_invalidScope() {
-    assertThat(xsrfTokenManager.validateToken(realToken, "console")).isTrue();
+    try {
+      xsrfTokenManager.generateLegacyToken("foo", testUser.getEmail());
+      fail("Expected IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().isEqualTo("Invalid scope value: foo");
+    }
   }
 
   @Test
-  public void testNoTimestamp() {
+  public void testValidate_token() {
-    assertThat(xsrfTokenManager.validateToken("foo", "console")).isFalse();
+    assertThat(xsrfTokenManager.validateToken(token)).isTrue();
   }
 
   @Test
-  public void testBadNumberTimestamp() {
+  public void testValidate_legacyToken() {
-    assertThat(xsrfTokenManager.validateToken("foo:bar", "console")).isFalse();
+    assertThat(xsrfTokenManager.validateToken(legacyToken)).isTrue();
   }
 
   @Test
-  public void testExpired() {
+  public void testValidate_token_missingParts() {
-    clock.setTo(START_OF_TIME.plusDays(2));
+    assertThat(xsrfTokenManager.validateToken("foo")).isFalse();
-    assertThat(xsrfTokenManager.validateToken(realToken, "console")).isFalse();
   }
 
   @Test
-  public void testTimestampTamperedWith() {
+  public void testValidate_token_badNumberTimestamp() {
-    String encodedPart = Splitter.on(':').splitToList(realToken).get(0);
+    assertThat(xsrfTokenManager.validateToken("1:notanumber:base64")).isFalse();
+  }
+
+  @Test
+  public void testValidate_legacyToken_badNumberTimestamp() {
+    assertThat(xsrfTokenManager.validateToken("base64:notanumber")).isFalse();
+  }
+
+  @Test
+  public void testValidate_token_expiresAfterOneDay() {
+    clock.advanceBy(Duration.standardDays(1));
+    assertThat(xsrfTokenManager.validateToken(token)).isTrue();
+    clock.advanceOneMilli();
+    assertThat(xsrfTokenManager.validateToken(token)).isFalse();
+  }
+
+  @Test
+  public void testValidate_legacyToken_expiresAfterOneDay() {
+    clock.advanceBy(Duration.standardDays(1));
+    assertThat(xsrfTokenManager.validateToken(legacyToken)).isTrue();
+    clock.advanceOneMilli();
+    assertThat(xsrfTokenManager.validateToken(legacyToken)).isFalse();
+  }
+
+  @Test
+  public void testValidate_token_timestampTamperedWith() {
+    String encodedPart = Splitter.on(':').splitToList(token).get(2);
+    long fakeTimestamp = clock.nowUtc().plusMillis(1).getMillis();
+    assertThat(xsrfTokenManager.validateToken("1:" + fakeTimestamp + ":" + encodedPart)).isFalse();
+  }
+
+  @Test
+  public void testValidate_legacyToken_timestampTamperedWith() {
+    String encodedPart = Splitter.on(':').splitToList(legacyToken).get(0);
     long tamperedTimestamp = clock.nowUtc().plusMillis(1).getMillis();
-    assertThat(xsrfTokenManager.validateToken(encodedPart + ":" + tamperedTimestamp, "console"))
+    assertThat(xsrfTokenManager.validateToken(encodedPart + ":" + tamperedTimestamp)).isFalse();
-        .isFalse();
   }
 
   @Test
-  public void testDifferentUser() {
+  public void testValidate_token_differentUser() {
-    assertThat(xsrfTokenManager
+    String otherToken = xsrfTokenManager.generateToken("eve@example.com");
-        .validateToken(xsrfTokenManager.generateToken("console", "eve@example.com"), "console"))
+    assertThat(xsrfTokenManager.validateToken(otherToken)).isFalse();
-            .isFalse();
   }
 
   @Test
-  public void testDifferentScope() {
+  public void testValidate_legacyToken_differentUser() {
-    assertThat(xsrfTokenManager.validateToken(realToken, "foobar")).isFalse();
+    String otherToken = xsrfTokenManager.generateLegacyToken("console", "eve@example.com");
+    assertThat(xsrfTokenManager.validateToken(otherToken)).isFalse();
   }
 
   @Test
-  public void testNullScope() {
+  public void testValidate_legacyToken_adminScope() {
-    String tokenWithNullScope = xsrfTokenManager.generateToken(null, testUser.getEmail());
+    String adminToken = xsrfTokenManager.generateLegacyToken("admin", testUser.getEmail());
-    assertThat(xsrfTokenManager.validateToken(tokenWithNullScope, null)).isTrue();
+    assertThat(xsrfTokenManager.validateToken(adminToken)).isTrue();
   }
 
-  // This test checks that the server side will pass when we switch the client to use a null scope.
   @Test
-  public void testNullScopePassesWhenTestedWithNonNullScope() {
+  public void testValidate_legacyToken_consoleScope() {
-    String tokenWithNullScope = xsrfTokenManager.generateToken(null, testUser.getEmail());
+    String consoleToken = xsrfTokenManager.generateLegacyToken("console", testUser.getEmail());
-    assertThat(xsrfTokenManager.validateToken(tokenWithNullScope, "console")).isTrue();
+    assertThat(xsrfTokenManager.validateToken(consoleToken)).isTrue();
   }
 }