From ba959cfa375693a5ebbd9409abb29f44a241e5f5 Mon Sep 17 00:00:00 2001 From: jaymode Date: Tue, 28 May 2019 13:37:26 -0600 Subject: [PATCH 1/4] Make hashed token ids url safe 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. --- .../xpack/core/security/authc/support/Hasher.java | 4 ++-- .../xpack/security/authc/TokenServiceTests.java | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/Hasher.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/Hasher.java index 5413a38bd6288..1b5b65e60c258 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/Hasher.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/Hasher.java @@ -360,14 +360,14 @@ public boolean verify(SecureString text, char[] hash) { 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); } }, diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java index 49796333098ff..8153748932fe9 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java @@ -53,14 +53,17 @@ 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.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; @@ -722,6 +723,11 @@ public void testCannotValidateTokenIfLicenseDoesNotAllowTokens() throws Exceptio 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); } From 0c5fe327a3932bcb1f6423b6f177df959a66ac66 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 29 May 2019 08:22:36 +0300 Subject: [PATCH 2/4] Adjust expected length of hashed refresh tokens --- .../org/elasticsearch/xpack/security/authc/TokenService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index a8f68870556e6..6472220482966 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/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 + "_"; From 02c6a430b7b9ff15d401ec8a5289d89c3cb88bfa Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 30 May 2019 06:15:06 -0600 Subject: [PATCH 3/4] mute bwc tests until change is backported --- .../upgrades/TokenBackwardsCompatibilityIT.java | 2 ++ .../rest-api-spec/test/mixed_cluster/50_token_auth.yml | 6 ++++++ .../rest-api-spec/test/upgraded_cluster/50_token_auth.yml | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java index 69c515d80a3d2..4a0639050d522 100644 --- a/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java +++ b/x-pack/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java @@ -7,6 +7,7 @@ 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.List; import java.util.Map; +@AwaitsFix(bugUrl = "need to backport #42651") public class TokenBackwardsCompatibilityIT extends AbstractUpgradeTestCase { private Collection twoClients = null; diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/50_token_auth.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/50_token_auth.yml index f426d9b2525b4..a34128579f3f8 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/50_token_auth.yml +++ b/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: diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml index 430f94c1064d6..3d8f8dbf3712a 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml +++ b/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: " - 7.99.99" + 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: " - 7.99.99" + reason: "Need to backport PR #42651" - do: get: From d9fe7636d349083e4ebab60ad9830150df07ea77 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 30 May 2019 08:22:17 -0600 Subject: [PATCH 4/4] use right version in skip --- .../rest-api-spec/test/upgraded_cluster/50_token_auth.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml index 3d8f8dbf3712a..64897707c15d3 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml @@ -2,7 +2,7 @@ "Get the indexed token and use if to authenticate": - skip: features: headers - version: " - 7.99.99" + version: " - 8.0.0" reason: "Need to backport PR #42651" - do: @@ -51,7 +51,7 @@ "Get the indexed refresh token and use if to get another access token and authenticate": - skip: features: headers - version: " - 7.99.99" + version: " - 8.0.0" reason: "Need to backport PR #42651" - do: