Fail if reading from closed KeyStoreWrapper

server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java

@@ -157,6 +157,7 @@ private static class Entry {
 
     /** The decrypted secret data. See {@link #decrypt(char[])}. */
     private final SetOnce<Map<String, Entry>> entries = new SetOnce<>();
+    private volatile boolean closed;
 
     private KeyStoreWrapper(int formatVersion, boolean hasPassword, byte[] dataBytes) {
         this.formatVersion = formatVersion;
@@ -274,7 +275,7 @@ public static void upgrade(KeyStoreWrapper wrapper, Path configDir, char[] passw
 
     @Override
     public boolean isLoaded() {
-        return entries.get() != null;
+        return entries.get() != null && closed == false;
     }
 
     /** Return true iff calling {@link #decrypt(char[])} requires a non-empty password. */
@@ -500,16 +501,22 @@ public void save(Path configDir, char[] password) throws Exception {
         }
     }
 
+    /**
+     * It is possible to retrieve the setting names even if the keystore is closed.
+     * This allows {@link SecureSetting} to correctly determine that a entry exists even though it cannot be read. Thus attempting to
+     * read a secure setting after the keystore is closed will generate a "keystore is closed" exception rather than using the fallback
+     * setting.
+     */
     @Override
     public Set<String> getSettingNames() {
-        assert isLoaded();
+        assert entries.get() != null : "Keystore is not loaded";
         return entries.get().keySet();
     }
 
     // TODO: make settings accessible only to code that registered the setting
     @Override
     public SecureString getString(String setting) {
-        assert isLoaded();
+        ensureOpen();
         Entry entry = entries.get().get(setting);
         if (entry == null || entry.type != EntryType.STRING) {
             throw new IllegalArgumentException("Secret setting " + setting + " is not a string");
@@ -521,12 +528,11 @@ public SecureString getString(String setting) {
 
     @Override
     public InputStream getFile(String setting) {
-        assert isLoaded();
+        ensureOpen();
         Entry entry = entries.get().get(setting);
         if (entry == null || entry.type != EntryType.FILE) {
             throw new IllegalArgumentException("Secret setting " + setting + " is not a file");
         }
-
         return new ByteArrayInputStream(entry.bytes);
     }
 
@@ -544,7 +550,7 @@ public static void validateSettingName(String setting) {
 
     /** Set a string setting. */
     void setString(String setting, char[] value) {
-        assert isLoaded();
+        ensureOpen();
         validateSettingName(setting);
 
         ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(CharBuffer.wrap(value));
@@ -557,7 +563,7 @@ void setString(String setting, char[] value) {
 
     /** Set a file setting. */
     void setFile(String setting, byte[] bytes) {
-        assert isLoaded();
+        ensureOpen();
         validateSettingName(setting);
 
         Entry oldEntry = entries.get().put(setting, new Entry(EntryType.FILE, Arrays.copyOf(bytes, bytes.length)));
@@ -568,15 +574,23 @@ void setFile(String setting, byte[] bytes) {
 
     /** Remove the given setting from the keystore. */
     void remove(String setting) {
-        assert isLoaded();
+        ensureOpen();
         Entry oldEntry = entries.get().remove(setting);
         if (oldEntry != null) {
             Arrays.fill(oldEntry.bytes, (byte)0);
         }
     }
 
+    private void ensureOpen() {
+        if (closed) {
+            throw new IllegalStateException("Keystore is closed");
+        }
+        assert isLoaded() : "Keystore is not loaded";
+    }
+
     @Override
     public void close() {
+        this.closed = true;
         for (Entry entry : entries.get().values()) {
             Arrays.fill(entry.bytes, (byte)0);
         }

server/src/test/java/org/elasticsearch/common/settings/KeyStoreWrapperTests.java

@@ -25,30 +25,27 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.nio.CharBuffer;
-import java.nio.charset.CharsetEncoder;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.FileSystem;
 import java.nio.file.Path;
 import java.security.KeyStore;
 import java.util.ArrayList;
 import java.util.Base64;
 import java.util.List;
-import java.util.Map;
-import java.util.stream.Collectors;
 
 import org.apache.lucene.codecs.CodecUtil;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexOutput;
 import org.apache.lucene.store.SimpleFSDirectory;
 import org.elasticsearch.core.internal.io.IOUtils;
-import org.elasticsearch.bootstrap.BootstrapSettings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ESTestCase;
+import org.hamcrest.Matchers;
 import org.junit.After;
 import org.junit.Before;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.notNullValue;
 
 public class KeyStoreWrapperTests extends ESTestCase {
 
@@ -92,6 +89,19 @@ public void testCreate() throws Exception {
         assertTrue(keystore.getSettingNames().contains(KeyStoreWrapper.SEED_SETTING.getKey()));
     }
 
+    public void testCannotReadStringFromClosedKeystore() throws Exception {
+        KeyStoreWrapper keystore = KeyStoreWrapper.create();
+        assertThat(keystore.getSettingNames(), Matchers.hasItem(KeyStoreWrapper.SEED_SETTING.getKey()));
+        assertThat(keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey()), notNullValue());
+
+        keystore.close();
+
+        assertThat(keystore.getSettingNames(), Matchers.hasItem(KeyStoreWrapper.SEED_SETTING.getKey()));
+        final IllegalStateException exception = expectThrows(IllegalStateException.class,
+            () -> keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey()));
+        assertThat(exception.getMessage(), containsString("closed"));
+    }
+
     public void testUpgradeNoop() throws Exception {
         KeyStoreWrapper keystore = KeyStoreWrapper.create();
         SecureString seed = keystore.getString(KeyStoreWrapper.SEED_SETTING.getKey());