From 8dc8fc507d95d86f4ee707e6b5ffda15d5415801 Mon Sep 17 00:00:00 2001
From: Ioannis Kakavas <ikakavas@protonmail.com>
Date: Thu, 14 Mar 2019 18:02:28 +0200
Subject: [PATCH] Handle UTF-8 values in the keystore (#39496)

* Handle UTF8 values in the keystore

Our current implementation uses CharBuffer#array to get the chars
that were decoded from the UTF-8 bytes. The backing array of
CharBuffer is created in CharsetDecoder#decode and gets an initial
length that is the same as the length of the ByteBuffer it decodes,
hence the number of UTF-8 bytes.
This works fine for the first 128 characters where each one needs
one bytes, but for the next UTF-8 characters (other latin alphabets
Greek, Cyrillic etc.) where we need 2 to 4 bytes per character, this
backing char array has a larger size than the number of the actual
chars this CharBuffer contains. Calling `array()` on it will return
a char array that can potentially have extra null chars so the
SecureString we get from the KeystoreWrapper, is not the same as the
one we entered.

This commit changes the behavior to use Arrays#copyOfRange to get
the necessary chars from the CharBuffer and adds a test with
random ( maybe not printable ) UTF-8 strings
---
 .../common/settings/KeyStoreWrapper.java           |  2 +-
 .../settings/AddStringKeyStoreCommandTests.java    | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java
index e017e9e7ca9..e3fbf30a47a 100644
--- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java
+++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java
@@ -532,7 +532,7 @@ public class KeyStoreWrapper implements SecureSettings {
         }
         ByteBuffer byteBuffer = ByteBuffer.wrap(entry.bytes);
         CharBuffer charBuffer = StandardCharsets.UTF_8.decode(byteBuffer);
-        return new SecureString(charBuffer.array());
+        return new SecureString(Arrays.copyOfRange(charBuffer.array(), charBuffer.position(), charBuffer.limit()));
     }
 
     @Override
diff --git a/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java b/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java
index 45b333ded0a..66c9885dcc0 100644
--- a/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java
+++ b/server/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.common.settings;
 
 import java.io.ByteArrayInputStream;
+import java.io.CharArrayWriter;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.Locale;
@@ -133,6 +134,19 @@ public class AddStringKeyStoreCommandTests extends KeyStoreCommandTestCase {
         assertSecureString("foo", "secret value 2");
     }
 
+    public void testAddUtf8String() throws Exception {
+        KeyStoreWrapper.create().save(env.configFile(), new char[0]);
+        final int stringSize = randomIntBetween(8, 16);
+        try (CharArrayWriter secretChars = new CharArrayWriter(stringSize)) {
+            for (int i = 0; i < stringSize; i++) {
+                secretChars.write((char) randomIntBetween(129, 2048));
+            }
+            setInput(secretChars.toString());
+            execute("-x", "foo");
+            assertSecureString("foo", secretChars.toString());
+        }
+    }
+
     public void testMissingSettingName() throws Exception {
         createKeystore("");
         terminal.addTextInput("");