Browse Source

Make hashed token ids url safe (#42651)

This commit changes the way token ids are hashed so that the output is
url safe without requiring encoding. This follows the pattern that we
use for document ids that are autogenerated, see UUIDs and the
associated classes for additional details.
Jay Modi 6 years ago
parent
commit
8d0bae6792

+ 2 - 2
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/Hasher.java

@@ -360,14 +360,14 @@ public enum Hasher {
         public char[] hash(SecureString text) {
             MessageDigest md = MessageDigests.sha256();
             md.update(CharArrays.toUtf8Bytes(text.getChars()));
-            return Base64.getEncoder().encodeToString(md.digest()).toCharArray();
+            return Base64.getUrlEncoder().withoutPadding().encodeToString(md.digest()).toCharArray();
         }
 
         @Override
         public boolean verify(SecureString text, char[] hash) {
             MessageDigest md = MessageDigests.sha256();
             md.update(CharArrays.toUtf8Bytes(text.getChars()));
-            return CharArrays.constantTimeEquals(Base64.getEncoder().encodeToString(md.digest()).toCharArray(), hash);
+            return CharArrays.constantTimeEquals(Base64.getUrlEncoder().withoutPadding().encodeToString(md.digest()).toCharArray(), hash);
         }
     },
 

+ 1 - 1
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java

@@ -181,7 +181,7 @@ public final class TokenService {
             TimeValue.MINUS_ONE, Property.NodeScope);
 
     static final String TOKEN_DOC_TYPE = "token";
-    private static final int HASHED_TOKEN_LENGTH = 44;
+    private static final int HASHED_TOKEN_LENGTH = 43;
     // UUIDs are 16 bytes encoded base64 without padding, therefore the length is (16 / 3) * 4 + ((16 % 3) * 8 + 5) / 6 chars
     private static final int TOKEN_LENGTH = 22;
     private static final String TOKEN_DOC_ID_PREFIX = TOKEN_DOC_TYPE + "_";

+ 9 - 3
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java

@@ -53,14 +53,17 @@ import org.elasticsearch.xpack.core.security.authc.support.TokensInvalidationRes
 import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.core.watcher.watch.ClockMock;
 import org.elasticsearch.xpack.security.support.SecurityIndexManager;
-import org.junit.After;
 import org.elasticsearch.xpack.security.test.SecurityMocks;
 import org.hamcrest.Matchers;
+import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 
+import javax.crypto.SecretKey;
 import java.io.IOException;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
 import java.time.Clock;
 import java.time.Instant;
@@ -70,8 +73,6 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
-import javax.crypto.SecretKey;
-
 import static java.time.Clock.systemUTC;
 import static org.elasticsearch.repositories.ESBlobStoreTestCase.randomBytes;
 import static org.elasticsearch.test.ClusterServiceUtils.setState;
@@ -721,6 +722,11 @@ public class TokenServiceTests extends ESTestCase {
         assertThat(authToken, Matchers.nullValue());
     }
 
+    public void testHashedTokenIsUrlSafe() {
+        final String hashedId = TokenService.hashTokenString(UUIDs.randomBase64UUID());
+        assertEquals(hashedId, URLEncoder.encode(hashedId, StandardCharsets.UTF_8));
+    }
+
     private TokenService createTokenService(Settings settings, Clock clock) throws GeneralSecurityException {
         return new TokenService(settings, clock, client, licenseState, securityMainIndex, securityTokensIndex, clusterService);
     }

+ 2 - 0
x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java

@@ -7,6 +7,7 @@ package org.elasticsearch.upgrades;
 
 import org.apache.http.HttpHeaders;
 import org.apache.http.HttpHost;
+import org.apache.lucene.util.LuceneTestCase.AwaitsFix;
 import org.elasticsearch.Version;
 import org.elasticsearch.client.Request;
 import org.elasticsearch.client.RequestOptions;
@@ -25,6 +26,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+@AwaitsFix(bugUrl = "need to backport #42651")
 public class TokenBackwardsCompatibilityIT extends AbstractUpgradeTestCase {
 
     private Collection<RestClient> twoClients = null;

+ 6 - 0
x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/50_token_auth.yml

@@ -2,6 +2,8 @@
 "Get the indexed token and use if to authenticate":
   - skip:
       features: headers
+      version: " - 7.99.99"
+      reason: "Need to backport PR #42651"
 
   - do:
       cluster.health:
@@ -59,6 +61,8 @@
 "Get the indexed refreshed access token and use if to authenticate":
   - skip:
       features: headers
+      version: " - 7.99.99"
+      reason: "Need to backport PR #42651"
 
   - do:
       get:
@@ -111,6 +115,8 @@
 "Get the indexed refresh token and use it to get another access token and authenticate":
   - skip:
       features: headers
+      version: " - 7.99.99"
+      reason: "Need to backport PR #42651"
 
   - do:
       get:

+ 4 - 0
x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml

@@ -2,6 +2,8 @@
 "Get the indexed token and use if to authenticate":
   - skip:
       features: headers
+      version: " - 8.0.0"
+      reason: "Need to backport PR #42651"
 
   - do:
       cluster.health:
@@ -49,6 +51,8 @@
 "Get the indexed refresh token and use if to get another access token and authenticate":
   - skip:
       features: headers
+      version: " - 8.0.0"
+      reason: "Need to backport PR #42651"
 
   - do:
       get: