From ed8d7ecc95e9fc27006c499c2884545fd4c6c011 Mon Sep 17 00:00:00 2001 From: jaymode Date: Wed, 2 May 2018 13:45:07 -0600 Subject: [PATCH 1/4] Security: fix TokenMetaData equals and hashcode The TokenMetaData equals method compared byte arrays using `.equals` on the arrays themselves, which is the equivalent of an `==` check. This means that a seperate byte[] with the same contents would not be considered equivalent to the existing one, even though it should be. The method has been updated to use `Array#equals` and similarly the hashcode method has been updated to call `Arrays#hashCode` instead of calling hashcode on the array itself. --- .../org/elasticsearch/test/ESTestCase.java | 8 +++ .../core/security/authc/TokenMetaData.java | 5 +- .../security/authc/TokenMetaDataTests.java | 52 +++++++++++++++++++ .../xpack/security/authc/BytesKey.java | 2 +- 4 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/TokenMetaDataTests.java diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index dfd3713333543..c46b8b7f6679a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -519,6 +519,14 @@ public static byte randomByte() { return (byte) random().nextInt(); } + public static byte[] randomByteArrayOfLength(int size) { + byte[] bytes = new byte[size]; + for (int i = 0; i < size; i++) { + bytes[i] = randomByte(); + } + return bytes; + } + public static short randomShort() { return (short) random().nextInt(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/TokenMetaData.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/TokenMetaData.java index 3b8ea2910d13d..6bd6228f2efe1 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/TokenMetaData.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/TokenMetaData.java @@ -14,6 +14,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import java.io.IOException; +import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -74,13 +75,13 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; TokenMetaData that = (TokenMetaData)o; - return keys.equals(that.keys) && currentKeyHash.equals(that.currentKeyHash); + return keys.equals(that.keys) && Arrays.equals(currentKeyHash, that.currentKeyHash); } @Override public int hashCode() { int result = keys.hashCode(); - result = 31 * result + currentKeyHash.hashCode(); + result = 31 * result + Arrays.hashCode(currentKeyHash); return result; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/TokenMetaDataTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/TokenMetaDataTests.java new file mode 100644 index 0000000000000..77f7c4dd3ad04 --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/TokenMetaDataTests.java @@ -0,0 +1,52 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.security.authc; + +import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.EqualsHashCodeTestUtils; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +public class TokenMetaDataTests extends ESTestCase { + + public void testEqualsAndHashCode() { + final int numKeyAndTimestamps = scaledRandomIntBetween(1, 8); + final List keyAndTimestampList = generateKeyAndTimestampListOfSize(numKeyAndTimestamps); + final byte[] currentKeyHash = randomByteArrayOfLength(8); + final TokenMetaData original = new TokenMetaData(keyAndTimestampList, currentKeyHash); + + EqualsHashCodeTestUtils.checkEqualsAndHashCode(original, tokenMetaData -> { + final List copiedList = new ArrayList<>(keyAndTimestampList); + final byte[] copyKeyHash = Arrays.copyOf(currentKeyHash, currentKeyHash.length); + return new TokenMetaData(copiedList, copyKeyHash); + }, tokenMetaData -> { + final List modifiedList = generateKeyAndTimestampListOfSize(numKeyAndTimestamps); + return new TokenMetaData(modifiedList, currentKeyHash); + }); + + EqualsHashCodeTestUtils.checkEqualsAndHashCode(original, tokenMetaData -> { + BytesStreamOutput out = new BytesStreamOutput(); + tokenMetaData.writeTo(out); + return new TokenMetaData(out.bytes().streamInput()); + }, tokenMetaData -> { + final byte[] modifiedKeyHash = randomByteArrayOfLength(8); + return new TokenMetaData(keyAndTimestampList, modifiedKeyHash); + }); + } + + private List generateKeyAndTimestampListOfSize(int size) { + final List keyAndTimestampList = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + keyAndTimestampList.add( + new KeyAndTimestamp(new SecureString(randomAlphaOfLengthBetween(1, 12).toCharArray()), randomNonNegativeLong())); + } + return keyAndTimestampList; + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/BytesKey.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/BytesKey.java index 1534b78899f8b..0ead753a4461c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/BytesKey.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/BytesKey.java @@ -14,7 +14,7 @@ * Simple wrapper around bytes so that it can be used as a cache key. The hashCode is computed * once upon creation and cached. */ -public class BytesKey { +public final class BytesKey { final byte[] bytes; private final int hashCode; From 445d890eebd8b027effafb57cb5dec10d9bb7206 Mon Sep 17 00:00:00 2001 From: jaymode Date: Fri, 4 May 2018 11:57:24 -0600 Subject: [PATCH 2/4] add javadoc --- .../src/main/java/org/elasticsearch/test/ESTestCase.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index c46b8b7f6679a..3d96fa95a7e5a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -519,6 +519,11 @@ public static byte randomByte() { return (byte) random().nextInt(); } + /** + * Helper method to create a byte array of a given length populated with random byte values + * + * @see #randomByte() + */ public static byte[] randomByteArrayOfLength(int size) { byte[] bytes = new byte[size]; for (int i = 0; i < size; i++) { From 0ce047976ecc08f6d3524289d88b633484605f86 Mon Sep 17 00:00:00 2001 From: jaymode Date: Fri, 4 May 2018 12:38:21 -0600 Subject: [PATCH 3/4] changelog --- docs/CHANGELOG.asciidoc | 2 ++ .../src/main/java/org/elasticsearch/test/ESTestCase.java | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/CHANGELOG.asciidoc b/docs/CHANGELOG.asciidoc index f4ecaf44a6c5a..14d45dd622d02 100644 --- a/docs/CHANGELOG.asciidoc +++ b/docs/CHANGELOG.asciidoc @@ -207,6 +207,8 @@ coming[6.3.1] Reduce the number of object allocations made by {security} when resolving the indices and aliases for a request ({pull}30180[#30180]) +Fix equality check for TokenMetaData to prevent unnecessary transmission during cluster state updates ({pull}30347[#30347]) + //[float] //=== Regressions diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 3d96fa95a7e5a..0a51fbdb8bd9b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -521,7 +521,7 @@ public static byte randomByte() { /** * Helper method to create a byte array of a given length populated with random byte values - * + * * @see #randomByte() */ public static byte[] randomByteArrayOfLength(int size) { From 3239f211bf212e2cad1c578b46397271be6f13f2 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 10 May 2018 07:27:12 -0600 Subject: [PATCH 4/4] remove changelog entry --- docs/CHANGELOG.asciidoc | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/CHANGELOG.asciidoc b/docs/CHANGELOG.asciidoc index e94d36c39bcec..5f7ed63cdd8ad 100644 --- a/docs/CHANGELOG.asciidoc +++ b/docs/CHANGELOG.asciidoc @@ -239,7 +239,6 @@ coming[6.3.1] Reduce the number of object allocations made by {security} when resolving the indices and aliases for a request ({pull}30180[#30180]) -Fix equality check for TokenMetaData to prevent unnecessary transmission during cluster state updates ({pull}30347[#30347]) Respect accept header on requests with no handler ({pull}30383[#30383]) //[float]