diff --git a/java/google/registry/loadtest/LoadTestAction.java b/java/google/registry/loadtest/LoadTestAction.java index c059d91a7..c777563ed 100644 --- a/java/google/registry/loadtest/LoadTestAction.java +++ b/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 diff --git a/java/google/registry/request/RequestHandler.java b/java/google/registry/request/RequestHandler.java index df3576cdb..e671ae9f2 100644 --- a/java/google/registry/request/RequestHandler.java +++ b/java/google/registry/request/RequestHandler.java @@ -164,9 +164,7 @@ public class RequestHandler { return; } if (route.get().shouldXsrfProtect(method) - && !xsrfTokenManager.validateToken( - nullToEmpty(req.getHeader(X_CSRF_TOKEN)), - route.get().action().xsrfScope())) { + && !xsrfTokenManager.validateToken(nullToEmpty(req.getHeader(X_CSRF_TOKEN)))) { rsp.sendError(SC_FORBIDDEN, "Invalid " + X_CSRF_TOKEN); return; } diff --git a/java/google/registry/request/auth/LegacyAuthenticationMechanism.java b/java/google/registry/request/auth/LegacyAuthenticationMechanism.java index e93924b6d..6b1d3f50d 100644 --- a/java/google/registry/request/auth/LegacyAuthenticationMechanism.java +++ b/java/google/registry/request/auth/LegacyAuthenticationMechanism.java @@ -53,9 +53,7 @@ public class LegacyAuthenticationMechanism implements AuthenticationMechanism { } if (!SAFE_METHODS.contains(request.getMethod()) - && !xsrfTokenManager.validateToken( - nullToEmpty(request.getHeader(X_CSRF_TOKEN)), - "console")) { // hard-coded for now; in the long run, this will be removed + && !xsrfTokenManager.validateToken(nullToEmpty(request.getHeader(X_CSRF_TOKEN)))) { return AuthResult.create(NONE); } diff --git a/java/google/registry/security/XsrfTokenManager.java b/java/google/registry/security/XsrfTokenManager.java index 0e0418a88..778ab7cc4 100644 --- a/java/google/registry/security/XsrfTokenManager.java +++ b/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 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. * - *

The token is a Base64-encoded SHA-256 hash of a string containing the secret, email, scope - * and creation time, separated by tabs. If the scope is null, the string is secret, email, - * creation time. In the future, the scope option will be removed. + *

The token format consists of three colon-delimited fields: the version number (currently 1), + * the timestamp in milliseconds since the epoch, and the Base64url-encoded SHA-256 HMAC (using + * the given secret key) of the user email and the timestamp millis separated by a tab character. + * + *

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. + * + *

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') - .skipNulls() - .join(ServerSecret.get().asUuid(), userEmail, scope, creationTime); + Joiner.on('\t').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. * *

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. * - *

The scope (or lack thereof) is passed to {@link #encodeToken}. Use of a scope in xsrf tokens - * is deprecated; instead, use the no-argument version. + *

The scope is passed to {@link #computeLegacyHash}. Use of a scope in xsrf tokens is + * 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); - } - - /** 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() : ""; + return Joiner.on(':').join(computeLegacyHash(now, scope, email), now); } /** - * Validate an xsrf token, given the scope it was used for. + * Validates an XSRF token against the current logged-in user. * - *

We plan to remove the scope parameter. As a first step, the method first checks for the - * existence of a token with no scope. If that is not found, it then looks for the existence of a - * 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. - * - *

This is the non-scoped version to which we will transition in the future. + * This accepts both legacy-style and new-style XSRF tokens. For legacy-style tokens, it will + * accept tokens generated with any scope from {@link #LEGACY_SCOPES}. */ public boolean validateToken(String token) { - return validateTokenSub(token, null); - } - - private boolean validateTokenSub(String token, @Nullable String scope) { + checkArgumentNotNull(token); List 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 reconstructedToken = encodeToken(creationTime, null, getLoggedInEmailOrEmpty()); - if (reconstructedToken.equals(encodedPart)) { - return true; - } + String currentUserEmail = + userService.isUserLoggedIn() ? userService.getCurrentUser().getEmail() : ""; - // If we don't find one, look for one with the specified scope. - if (scope != null) { - reconstructedToken = encodeToken(creationTime, scope, getLoggedInEmailOrEmpty()); - if (reconstructedToken.equals(encodedPart)) { - return true; + // Reconstruct the token to verify validity, using version 1 format if detected. + if (tokenParts.get(0).equals(VERSION_1)) { + String reconstructedToken = + encodeToken(ServerSecret.get().asBytes(), currentUserEmail, timestampMillis); + if (!token.equals(reconstructedToken)) { + logger.warningfmt( + "Reconstructed XSRF mismatch (got != expected): %s != %s", token, 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 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; } - - logger.warningfmt("Reconstructed XSRF mismatch: %s ≠ %s", encodedPart, reconstructedToken); - return false; } } diff --git a/java/google/registry/tools/AppEngineConnection.java b/java/google/registry/tools/AppEngineConnection.java index 5024787ee..6cfa4b857 100644 --- a/java/google/registry/tools/AppEngineConnection.java +++ b/java/google/registry/tools/AppEngineConnection.java @@ -66,7 +66,7 @@ class AppEngineConnection implements Connection { memoize(new Supplier() { @Override public String get() { - return xsrfTokenManager.generateToken("admin", getUserId()); + return xsrfTokenManager.generateLegacyToken("admin", getUserId()); }}); @Override diff --git a/java/google/registry/ui/server/registrar/ConsoleUiAction.java b/java/google/registry/ui/server/registrar/ConsoleUiAction.java index 3f0d3d14b..c9c92355b 100644 --- a/java/google/registry/ui/server/registrar/ConsoleUiAction.java +++ b/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); diff --git a/javatests/google/registry/request/RequestHandlerTest.java b/javatests/google/registry/request/RequestHandlerTest.java index cb5ff6ffe..5520b3103 100644 --- a/javatests/google/registry/request/RequestHandlerTest.java +++ b/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"); diff --git a/javatests/google/registry/request/auth/RequestAuthenticatorTest.java b/javatests/google/registry/request/auth/RequestAuthenticatorTest.java index da05cb5e7..366e25617 100644 --- a/javatests/google/registry/request/auth/RequestAuthenticatorTest.java +++ b/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 = 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 = runTest(fakeUserService, AuthAdminUserAnyMethod.class); diff --git a/javatests/google/registry/security/XsrfTokenManagerTest.java b/javatests/google/registry/security/XsrfTokenManagerTest.java index b9660f520..9ce4862e2 100644 --- a/javatests/google/registry/security/XsrfTokenManagerTest.java +++ b/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() { - assertThat(xsrfTokenManager.validateToken(realToken, "console")).isTrue(); + public void testGenerateLegacyToken_invalidScope() { + try { + xsrfTokenManager.generateLegacyToken("foo", testUser.getEmail()); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessageThat().isEqualTo("Invalid scope value: foo"); + } } @Test - public void testNoTimestamp() { - assertThat(xsrfTokenManager.validateToken("foo", "console")).isFalse(); + public void testValidate_token() { + assertThat(xsrfTokenManager.validateToken(token)).isTrue(); } @Test - public void testBadNumberTimestamp() { - assertThat(xsrfTokenManager.validateToken("foo:bar", "console")).isFalse(); + public void testValidate_legacyToken() { + assertThat(xsrfTokenManager.validateToken(legacyToken)).isTrue(); } @Test - public void testExpired() { - clock.setTo(START_OF_TIME.plusDays(2)); - assertThat(xsrfTokenManager.validateToken(realToken, "console")).isFalse(); + public void testValidate_token_missingParts() { + assertThat(xsrfTokenManager.validateToken("foo")).isFalse(); } @Test - public void testTimestampTamperedWith() { - String encodedPart = Splitter.on(':').splitToList(realToken).get(0); + public void testValidate_token_badNumberTimestamp() { + 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")) - .isFalse(); + assertThat(xsrfTokenManager.validateToken(encodedPart + ":" + tamperedTimestamp)).isFalse(); } @Test - public void testDifferentUser() { - assertThat(xsrfTokenManager - .validateToken(xsrfTokenManager.generateToken("console", "eve@example.com"), "console")) - .isFalse(); + public void testValidate_token_differentUser() { + String otherToken = xsrfTokenManager.generateToken("eve@example.com"); + assertThat(xsrfTokenManager.validateToken(otherToken)).isFalse(); } @Test - public void testDifferentScope() { - assertThat(xsrfTokenManager.validateToken(realToken, "foobar")).isFalse(); + public void testValidate_legacyToken_differentUser() { + String otherToken = xsrfTokenManager.generateLegacyToken("console", "eve@example.com"); + assertThat(xsrfTokenManager.validateToken(otherToken)).isFalse(); } @Test - public void testNullScope() { - String tokenWithNullScope = xsrfTokenManager.generateToken(null, testUser.getEmail()); - assertThat(xsrfTokenManager.validateToken(tokenWithNullScope, null)).isTrue(); + public void testValidate_legacyToken_adminScope() { + String adminToken = xsrfTokenManager.generateLegacyToken("admin", testUser.getEmail()); + 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() { - String tokenWithNullScope = xsrfTokenManager.generateToken(null, testUser.getEmail()); - assertThat(xsrfTokenManager.validateToken(tokenWithNullScope, "console")).isTrue(); + public void testValidate_legacyToken_consoleScope() { + String consoleToken = xsrfTokenManager.generateLegacyToken("console", testUser.getEmail()); + assertThat(xsrfTokenManager.validateToken(consoleToken)).isTrue(); } }