Skip to content

Commit

Permalink
Move CharArrays to core lib (#32851)
Browse files Browse the repository at this point in the history
This change cleans up some methods in the CharArrays class from x-pack, which
includes the unification of char[] to utf8 and utf8 to char[] conversions that
intentionally do not use strings. There was previously an implementation in
x-pack and in the reloading of secure settings. The method from the reloading
of secure settings was adopted as it handled more scenarios related to the
backing byte and char buffers that were used to perform the conversions. The
cleaned up class is moved into libs/core to allow it to be used by requests
that will be migrated to the high level rest client.

Relates #32332
  • Loading branch information
jaymode authored Aug 15, 2018
1 parent 364ccc3 commit 1a45b27
Show file tree
Hide file tree
Showing 18 changed files with 257 additions and 178 deletions.
150 changes: 150 additions & 0 deletions libs/core/src/main/java/org/elasticsearch/common/CharArrays.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.common;

import java.nio.ByteBuffer;
import java.nio.CharBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Objects;

/**
* Helper class similar to Arrays to handle conversions for Char arrays
*/
public final class CharArrays {

private CharArrays() {}

/**
* Decodes the provided byte[] to a UTF-8 char[]. This is done while avoiding
* conversions to String. The provided byte[] is not modified by this method, so
* the caller needs to take care of clearing the value if it is sensitive.
*/
public static char[] utf8BytesToChars(byte[] utf8Bytes) {
final ByteBuffer byteBuffer = ByteBuffer.wrap(utf8Bytes);
final CharBuffer charBuffer = StandardCharsets.UTF_8.decode(byteBuffer);
final char[] chars;
if (charBuffer.hasArray()) {
// there is no guarantee that the char buffers backing array is the right size
// so we need to make a copy
chars = Arrays.copyOfRange(charBuffer.array(), charBuffer.position(), charBuffer.limit());
Arrays.fill(charBuffer.array(), (char) 0); // clear sensitive data
} else {
final int length = charBuffer.limit() - charBuffer.position();
chars = new char[length];
charBuffer.get(chars);
// if the buffer is not read only we can reset and fill with 0's
if (charBuffer.isReadOnly() == false) {
charBuffer.clear(); // reset
for (int i = 0; i < charBuffer.limit(); i++) {
charBuffer.put((char) 0);
}
}
}
return chars;
}

/**
* Encodes the provided char[] to a UTF-8 byte[]. This is done while avoiding
* conversions to String. The provided char[] is not modified by this method, so
* the caller needs to take care of clearing the value if it is sensitive.
*/
public static byte[] toUtf8Bytes(char[] chars) {
final CharBuffer charBuffer = CharBuffer.wrap(chars);
final ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(charBuffer);
final byte[] bytes;
if (byteBuffer.hasArray()) {
// there is no guarantee that the byte buffers backing array is the right size
// so we need to make a copy
bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit());
Arrays.fill(byteBuffer.array(), (byte) 0); // clear sensitive data
} else {
final int length = byteBuffer.limit() - byteBuffer.position();
bytes = new byte[length];
byteBuffer.get(bytes);
// if the buffer is not read only we can reset and fill with 0's
if (byteBuffer.isReadOnly() == false) {
byteBuffer.clear(); // reset
for (int i = 0; i < byteBuffer.limit(); i++) {
byteBuffer.put((byte) 0);
}
}
}
return bytes;
}

/**
* Tests if a char[] contains a sequence of characters that match the prefix. This is like
* {@link String#startsWith(String)} but does not require conversion of the char[] to a string.
*/
public static boolean charsBeginsWith(String prefix, char[] chars) {
if (chars == null || prefix == null) {
return false;
}

if (prefix.length() > chars.length) {
return false;
}

for (int i = 0; i < prefix.length(); i++) {
if (chars[i] != prefix.charAt(i)) {
return false;
}
}

return true;
}

/**
* Constant time equality check of char arrays to avoid potential timing attacks.
*/
public static boolean constantTimeEquals(char[] a, char[] b) {
Objects.requireNonNull(a, "char arrays must not be null for constantTimeEquals");
Objects.requireNonNull(b, "char arrays must not be null for constantTimeEquals");
if (a.length != b.length) {
return false;
}

int equals = 0;
for (int i = 0; i < a.length; i++) {
equals |= a[i] ^ b[i];
}

return equals == 0;
}

/**
* Constant time equality check of strings to avoid potential timing attacks.
*/
public static boolean constantTimeEquals(String a, String b) {
Objects.requireNonNull(a, "strings must not be null for constantTimeEquals");
Objects.requireNonNull(b, "strings must not be null for constantTimeEquals");
if (a.length() != b.length()) {
return false;
}

int equals = 0;
for (int i = 0; i < a.length(); i++) {
equals |= a.charAt(i) ^ b.charAt(i);
}

return equals == 0;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.common;

import org.elasticsearch.test.ESTestCase;

import java.nio.charset.StandardCharsets;

public class CharArraysTests extends ESTestCase {

public void testCharsToBytes() {
final String originalValue = randomUnicodeOfCodepointLengthBetween(0, 32);
final byte[] expectedBytes = originalValue.getBytes(StandardCharsets.UTF_8);
final char[] valueChars = originalValue.toCharArray();

final byte[] convertedBytes = CharArrays.toUtf8Bytes(valueChars);
assertArrayEquals(expectedBytes, convertedBytes);
}

public void testBytesToUtf8Chars() {
final String originalValue = randomUnicodeOfCodepointLengthBetween(0, 32);
final byte[] bytes = originalValue.getBytes(StandardCharsets.UTF_8);
final char[] expectedChars = originalValue.toCharArray();

final char[] convertedChars = CharArrays.utf8BytesToChars(bytes);
assertArrayEquals(expectedChars, convertedChars);
}

public void testCharsBeginsWith() {
assertFalse(CharArrays.charsBeginsWith(randomAlphaOfLength(4), null));
assertFalse(CharArrays.charsBeginsWith(null, null));
assertFalse(CharArrays.charsBeginsWith(null, randomAlphaOfLength(4).toCharArray()));
assertFalse(CharArrays.charsBeginsWith(randomAlphaOfLength(2), randomAlphaOfLengthBetween(3, 8).toCharArray()));

final String prefix = randomAlphaOfLengthBetween(2, 4);
assertTrue(CharArrays.charsBeginsWith(prefix, prefix.toCharArray()));
final char[] prefixedValue = prefix.concat(randomAlphaOfLengthBetween(1, 12)).toCharArray();
assertTrue(CharArrays.charsBeginsWith(prefix, prefixedValue));

final String modifiedPrefix = randomBoolean() ? prefix.substring(1) : prefix.substring(0, prefix.length() - 1);
final char[] nonMatchingValue = modifiedPrefix.concat(randomAlphaOfLengthBetween(0, 12)).toCharArray();
assertFalse(CharArrays.charsBeginsWith(prefix, nonMatchingValue));
assertTrue(CharArrays.charsBeginsWith(modifiedPrefix, nonMatchingValue));
}

public void testConstantTimeEquals() {
final String value = randomAlphaOfLengthBetween(0, 32);
assertTrue(CharArrays.constantTimeEquals(value, value));
assertTrue(CharArrays.constantTimeEquals(value.toCharArray(), value.toCharArray()));

final String other = randomAlphaOfLengthBetween(1, 32);
assertFalse(CharArrays.constantTimeEquals(value, other));
assertFalse(CharArrays.constantTimeEquals(value.toCharArray(), other.toCharArray()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@

import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.nodes.BaseNodesRequest;
import org.elasticsearch.common.CharArrays;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.SecureString;

import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.CharBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;

import static org.elasticsearch.action.ValidateActions.addValidationError;
Expand Down Expand Up @@ -83,7 +81,7 @@ public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
final byte[] passwordBytes = in.readByteArray();
try {
this.secureSettingsPassword = new SecureString(utf8BytesToChars(passwordBytes));
this.secureSettingsPassword = new SecureString(CharArrays.utf8BytesToChars(passwordBytes));
} finally {
Arrays.fill(passwordBytes, (byte) 0);
}
Expand All @@ -92,69 +90,11 @@ public void readFrom(StreamInput in) throws IOException {
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
final byte[] passwordBytes = charsToUtf8Bytes(this.secureSettingsPassword.getChars());
final byte[] passwordBytes = CharArrays.toUtf8Bytes(this.secureSettingsPassword.getChars());
try {
out.writeByteArray(passwordBytes);
} finally {
Arrays.fill(passwordBytes, (byte) 0);
}
}

/**
* Encodes the provided char[] to a UTF-8 byte[]. This is done while avoiding
* conversions to String. The provided char[] is not modified by this method, so
* the caller needs to take care of clearing the value if it is sensitive.
*/
private static byte[] charsToUtf8Bytes(char[] chars) {
final CharBuffer charBuffer = CharBuffer.wrap(chars);
final ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(charBuffer);
final byte[] bytes;
if (byteBuffer.hasArray()) {
// there is no guarantee that the byte buffers backing array is the right size
// so we need to make a copy
bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit());
Arrays.fill(byteBuffer.array(), (byte) 0); // clear sensitive data
} else {
final int length = byteBuffer.limit() - byteBuffer.position();
bytes = new byte[length];
byteBuffer.get(bytes);
// if the buffer is not read only we can reset and fill with 0's
if (byteBuffer.isReadOnly() == false) {
byteBuffer.clear(); // reset
for (int i = 0; i < byteBuffer.limit(); i++) {
byteBuffer.put((byte) 0);
}
}
}
return bytes;
}

/**
* Decodes the provided byte[] to a UTF-8 char[]. This is done while avoiding
* conversions to String. The provided byte[] is not modified by this method, so
* the caller needs to take care of clearing the value if it is sensitive.
*/
public static char[] utf8BytesToChars(byte[] utf8Bytes) {
final ByteBuffer byteBuffer = ByteBuffer.wrap(utf8Bytes);
final CharBuffer charBuffer = StandardCharsets.UTF_8.decode(byteBuffer);
final char[] chars;
if (charBuffer.hasArray()) {
// there is no guarantee that the char buffers backing array is the right size
// so we need to make a copy
chars = Arrays.copyOfRange(charBuffer.array(), charBuffer.position(), charBuffer.limit());
Arrays.fill(charBuffer.array(), (char) 0); // clear sensitive data
} else {
final int length = charBuffer.limit() - charBuffer.position();
chars = new char[length];
charBuffer.get(chars);
// if the buffer is not read only we can reset and fill with 0's
if (charBuffer.isReadOnly() == false) {
charBuffer.clear(); // reset
for (int i = 0; i < charBuffer.limit(); i++) {
charBuffer.put((char) 0);
}
}
}
return chars;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.xpack.core.security.authc.support.CharArrays;
import org.elasticsearch.common.CharArrays;

import java.io.IOException;
import java.util.Arrays;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.xpack.core.security.authc.support.CharArrays;
import org.elasticsearch.common.CharArrays;

import java.io.IOException;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.common.CharArrays;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.xpack.core.security.authc.support.CharArrays;

import java.io.IOException;
import java.util.Map;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

import org.elasticsearch.common.CharArrays;
import org.elasticsearch.common.settings.SecureString;

import java.security.SecureRandom;
Expand Down Expand Up @@ -54,7 +55,7 @@
* String stronger_salt = BCrypt.gensalt(12)<br>
* </code>
* <p>
* The amount of work increases exponentially (2**log_rounds), so
* The amount of work increases exponentially (2**log_rounds), so
* each increment is twice as much work. The default log_rounds is
* 10, and the valid range is 4 to 30.
*
Expand Down Expand Up @@ -689,7 +690,11 @@ public static String hashpw(SecureString password, String salt) {

// the next lines are the SecureString replacement for the above commented-out section
if (minor >= 'a') {
try (SecureString secureString = new SecureString(CharArrays.concat(password.getChars(), "\000".toCharArray()))) {
final char[] suffix = "\000".toCharArray();
final char[] result = new char[password.length() + suffix.length];
System.arraycopy(password.getChars(), 0, result, 0, password.length());
System.arraycopy(suffix, 0, result, password.length(), suffix.length);
try (SecureString secureString = new SecureString(result)) {
passwordb = CharArrays.toUtf8Bytes(secureString.getChars());
}
} else {
Expand Down
Loading

0 comments on commit 1a45b27

Please sign in to comment.