From 4a66435d38777eb3ddf92f8248a24eb7213b7bed Mon Sep 17 00:00:00 2001 From: Jasper Aarts Date: Sat, 22 Dec 2018 07:05:57 +1100 Subject: [PATCH 1/2] Allow overriding encrypted properties with unencrypted properties (#459) --- .../EnvironmentDecryptApplicationInitializer.java | 5 ++++- ...EnvironmentDecryptApplicationInitializerTests.java | 11 +++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java index af49fdc1..6e8d6bc6 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java @@ -217,10 +217,13 @@ public class EnvironmentDecryptApplicationInitializer implements } } else if (COLLECTION_PROPERTY.matcher(key).matches()) { - // put non-ecrypted properties so merging of index properties + // put non-encrypted properties so merging of index properties // happens correctly otherCollectionProperties.put(key, value); } + else { + overrides.remove(key); + } } } // copy all indexed properties even if not encrypted diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java index 60889642..007b0b29 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java @@ -75,6 +75,17 @@ public class EnvironmentDecryptApplicationInitializerTests { assertEquals("spam", context.getEnvironment().getProperty("foo")); } + @Test + public void propertySourcesOrderedCorrectlyWithUnencryptedOverrides() { + ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(); + TestPropertyValues.of("foo: {cipher}bar").applyTo(context); + context.getEnvironment().getPropertySources() + .addFirst(new MapPropertySource("test_override", + Collections.singletonMap("foo", "spam"))); + this.listener.initialize(context); + assertEquals("spam", context.getEnvironment().getProperty("foo")); + } + @Test(expected = IllegalStateException.class) public void errorOnDecrypt() { this.listener = new EnvironmentDecryptApplicationInitializer( From 59798f52ed54766099d76f970640093954359d49 Mon Sep 17 00:00:00 2001 From: Jasper Aarts Date: Tue, 8 Jan 2019 04:33:09 +1100 Subject: [PATCH 2/2] Only attempt to decrypt properties that are not overridden (#462) --- ...ironmentDecryptApplicationInitializer.java | 65 +++++++++++-------- ...entDecryptApplicationInitializerTests.java | 21 ++++++ 2 files changed, 60 insertions(+), 26 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java index 6e8d6bc6..17f15aa6 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializer.java @@ -163,21 +163,25 @@ public class EnvironmentDecryptApplicationInitializer implements sources.add(0, source); } for (PropertySource source : sources) { - decrypt(source, overrides); + collectEncryptedProperties(source, overrides); } + + doDecrypt(overrides); return overrides; } private Map decrypt(PropertySource source) { Map overrides = new LinkedHashMap<>(); - decrypt(source, overrides); + collectEncryptedProperties(source, overrides); + doDecrypt(overrides); return overrides; } private static final Pattern COLLECTION_PROPERTY = Pattern .compile("(\\S+)?\\[(\\d+)\\](\\.\\S+)?"); - private void decrypt(PropertySource source, Map overrides) { + private void collectEncryptedProperties(PropertySource source, + Map overrides) { if (source instanceof EnumerablePropertySource) { Map otherCollectionProperties = new LinkedHashMap<>(); @@ -189,28 +193,6 @@ public class EnvironmentDecryptApplicationInitializer implements if (property != null) { String value = property.toString(); if (value.startsWith("{cipher}")) { - value = value.substring("{cipher}".length()); - try { - value = this.encryptor.decrypt(value); - if (logger.isDebugEnabled()) { - logger.debug("Decrypted: key=" + key); - } - } - catch (Exception e) { - String message = "Cannot decrypt: key=" + key; - if (this.failOnError) { - throw new IllegalStateException(message, e); - } - if (logger.isDebugEnabled()) { - logger.warn(message, e); - } - else { - logger.warn(message); - } - // Set value to empty to avoid making a password out of the - // cipher text - value = ""; - } overrides.put(key, value); if (COLLECTION_PROPERTY.matcher(key).matches()) { sourceHasDecryptedCollection = true; @@ -236,11 +218,42 @@ public class EnvironmentDecryptApplicationInitializer implements for (PropertySource nested : ((CompositePropertySource) source) .getPropertySources()) { - decrypt(nested, overrides); + collectEncryptedProperties(nested, overrides); } } } + private void doDecrypt(Map overrides) { + for (String key : overrides.keySet()) { + String value = overrides.get(key).toString(); + if (value.startsWith("{cipher}")) { + value = value.substring("{cipher}".length()); + try { + value = this.encryptor.decrypt(value); + if (logger.isDebugEnabled()) { + logger.debug("Decrypted: key=" + key); + } + } + catch (Exception e) { + String message = "Cannot decrypt: key=" + key; + if (this.failOnError) { + throw new IllegalStateException(message, e); + } + if (logger.isDebugEnabled()) { + logger.warn(message, e); + } + else { + logger.warn(message); + } + // Set value to empty to avoid making a password out of the + // cipher text + value = ""; + } + overrides.put(key, value); + } + } + } + } diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java index 007b0b29..2e2fd3d7 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/encrypt/EnvironmentDecryptApplicationInitializerTests.java @@ -30,11 +30,14 @@ import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; import org.springframework.security.crypto.encrypt.Encryptors; +import org.springframework.security.crypto.encrypt.TextEncryptor; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static org.springframework.cloud.bootstrap.encrypt.EnvironmentDecryptApplicationInitializer.DECRYPTED_PROPERTY_SOURCE_NAME; @@ -161,4 +164,22 @@ public class EnvironmentDecryptApplicationInitializerTests { assertEquals("value", ctx.getEnvironment().getProperty("key")); } + @Test + public void testOnlyDecryptIfNotOverridden() { + ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(); + TextEncryptor encryptor = mock(TextEncryptor.class); + when(encryptor.decrypt("bar2")).thenReturn("bar2"); + EnvironmentDecryptApplicationInitializer initializer = new EnvironmentDecryptApplicationInitializer( + encryptor); + TestPropertyValues.of("foo: {cipher}bar", "foo2: {cipher}bar2").applyTo(context); + context.getEnvironment().getPropertySources() + .addFirst(new MapPropertySource("test_override", + Collections.singletonMap("foo", "spam"))); + initializer.initialize(context); + assertEquals("spam", context.getEnvironment().getProperty("foo")); + assertEquals("bar2", context.getEnvironment().getProperty("foo2")); + verify(encryptor).decrypt("bar2"); + verifyNoMoreInteractions(encryptor); + } + }