From f7379086e06c493398cd05b097e412c53f34033b Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Mon, 6 Jan 2020 13:09:26 +0100 Subject: [PATCH] KEYCLOAK-12619 Improve mapped byte buffer cleanup --- .../java/org/keycloak/vault/DefaultVaultRawSecret.java | 9 +++++++-- .../org/keycloak/vault/PlainTextVaultProviderTest.java | 6 +++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/services/src/main/java/org/keycloak/vault/DefaultVaultRawSecret.java b/services/src/main/java/org/keycloak/vault/DefaultVaultRawSecret.java index 0310ea84a2..9c5798a758 100644 --- a/services/src/main/java/org/keycloak/vault/DefaultVaultRawSecret.java +++ b/services/src/main/java/org/keycloak/vault/DefaultVaultRawSecret.java @@ -26,6 +26,8 @@ import java.util.concurrent.ThreadLocalRandom; */ public class DefaultVaultRawSecret implements VaultRawSecret { + private static final ByteBuffer EMPTY_BUFFER = ByteBuffer.allocate(0); + private static final VaultRawSecret EMPTY_VAULT_SECRET = new VaultRawSecret() { @Override public Optional get() { @@ -42,7 +44,7 @@ public class DefaultVaultRawSecret implements VaultRawSecret { } }; - private final ByteBuffer rawSecret; + private ByteBuffer rawSecret; private byte[] secretArray; @@ -80,9 +82,12 @@ public class DefaultVaultRawSecret implements VaultRawSecret { public void close() { if (rawSecret.hasArray()) { ThreadLocalRandom.current().nextBytes(rawSecret.array()); - } else if (this.secretArray != null) { + } + if (this.secretArray != null) { ThreadLocalRandom.current().nextBytes(this.secretArray); + this.secretArray = null; // dispose of secretArray } rawSecret.clear(); + rawSecret = EMPTY_BUFFER; } } diff --git a/services/src/test/java/org/keycloak/vault/PlainTextVaultProviderTest.java b/services/src/test/java/org/keycloak/vault/PlainTextVaultProviderTest.java index 051a8caefc..33cb3676d0 100644 --- a/services/src/test/java/org/keycloak/vault/PlainTextVaultProviderTest.java +++ b/services/src/test/java/org/keycloak/vault/PlainTextVaultProviderTest.java @@ -5,10 +5,9 @@ import org.junit.Test; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.ArrayList; import java.util.Arrays; -import java.util.List; +import static org.hamcrest.CoreMatchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -154,11 +153,12 @@ public class PlainTextVaultProviderTest { //when VaultRawSecret secretAfterFirstRead = provider.obtainSecret(secretName); + assertThat(secretAfterFirstRead, secretContains("secret")); secretAfterFirstRead.close(); VaultRawSecret secretAfterSecondRead = provider.obtainSecret(secretName); //then - assertThat(secretAfterFirstRead, secretContains("secret")); + assertThat(secretAfterFirstRead, not(secretContains("secret"))); assertThat(secretAfterSecondRead, secretContains("secret")); } } \ No newline at end of file