From ed3a07f5d9b83511b6a685a0c98696b7781832f8 Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Thu, 10 May 2018 13:12:11 -0600 Subject: [PATCH] Security: fix TokenMetaData equals and hashcode (#30347) 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 | 13 +++++ .../core/security/authc/TokenMetaData.java | 5 +- .../security/authc/TokenMetaDataTests.java | 52 +++++++++++++++++++ .../xpack/security/authc/BytesKey.java | 2 +- 4 files changed, 69 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 7a278aeadaede..4717fc7c1ba31 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,19 @@ 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++) { + 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;