diff --git a/doc/modules/ROOT/nav.adoc b/doc/modules/ROOT/nav.adoc index 8dba7339b..f9b223af0 100644 --- a/doc/modules/ROOT/nav.adoc +++ b/doc/modules/ROOT/nav.adoc @@ -1,6 +1,10 @@ * xref:index.adoc[Index] + +* xref:config/config.adoc[Config] + +* xref:interceptors/interceptors.adoc[Interceptors] + * Extensions ** xref:config-sources/config-sources.adoc[Config Sources] ** xref:converters/converters.adoc[Converters] -** xref:interceptors/interceptors.adoc[Interceptors] ** xref:cdi/cdi.adoc[CDI] diff --git a/doc/modules/ROOT/pages/config/config.adoc b/doc/modules/ROOT/pages/config/config.adoc new file mode 100644 index 000000000..feee071c0 --- /dev/null +++ b/doc/modules/ROOT/pages/config/config.adoc @@ -0,0 +1,10 @@ +:doctype: book +include::../attributes.adoc[] + +[[cdi-extensions]] + += Config + +* <> + +include::secret-keys.adoc[] diff --git a/doc/modules/ROOT/pages/config/secret-keys.adoc b/doc/modules/ROOT/pages/config/secret-keys.adoc new file mode 100644 index 000000000..6d8a43864 --- /dev/null +++ b/doc/modules/ROOT/pages/config/secret-keys.adoc @@ -0,0 +1,32 @@ +[[secret-keys]] +== Secret Keys + +When configuration properties contain passwords or other kinds of secrets, Smallrye Config can hide them to +prevent accidental exposure of such values. + +**This is no way a replacement for securing secrets. ** Proper security mechanisms must still be used to secure +secrets. However, there is still the basic problem that passwords and secrets are generally encoded simply as +strings. + +Secret Keys provides a way to "lock" the configuration so that secrets do not appear unless explicitly enabled. + +=== Configuration + +Secret Keys requires the list of Config property names that must be hidden. This can be supplied in +`SmallRyeConfigBuilder.withSecretKeys` and initialized with `SmallRyeConfigFactory`. From this point forward, any +config name retrieved from the `Config` instance that matches the Secret Keys will throw a `SecurityException`. + +=== Unlock Keys + +Access to the Secret Keys, is available via the APIs `io.smallrye.config.SecretKeys.doUnlocked(java.lang.Runnable)` and +`io.smallrye.config.SecretKeys.doUnlocked(java.util.function.Supplier)`. + +[source,java] +---- +String secretValue = SecretKeys.doUnlocked(() -> { + config.getValue("secret", String.class); +}); +---- + +Secret Keyes are only unlocked in the context of `doUnlocked`. Once the execution completes, the secrets become +locked again. diff --git a/doc/modules/ROOT/pages/interceptors/interceptors.adoc b/doc/modules/ROOT/pages/interceptors/interceptors.adoc index c1a9b40b2..18650df88 100644 --- a/doc/modules/ROOT/pages/interceptors/interceptors.adoc +++ b/doc/modules/ROOT/pages/interceptors/interceptors.adoc @@ -54,6 +54,7 @@ SmallRye Config provides the following built-in Interceptors: * <> * <> * <> +* <> None of the interceptors is registered by default. Registration needs to happen via the ServiceLoader mechanism, via the Programmatic API or by calling `SmallRyeConfigBuilder.addDefaultInterceptors`, which adds the @@ -154,3 +155,12 @@ new FallbackConfigSourceInterceptor( name.replaceAll("microprofile\\.config", "smallrye.config") : name)); ---- + +[[logging-interceptor]] +=== LoggingConfigSourceInterceptor + +The `LoggingConfigSourceInterceptor` logs lookups of configuration names in the provided logging platform. The log +information includes config name and value, the config source origing and location if exists. + +The log is done as `debug`, so the debug threshold must be set to `debug` for the `io.smallrye.config` appender to +display the logs. diff --git a/implementation/pom.xml b/implementation/pom.xml index f88134629..c6f87cd28 100644 --- a/implementation/pom.xml +++ b/implementation/pom.xml @@ -76,6 +76,17 @@ junit test + + org.jboss.weld + weld-junit4 + test + + + javax.enterprise + cdi-api + + + diff --git a/implementation/src/main/java/io/smallrye/config/ConfigLogging.java b/implementation/src/main/java/io/smallrye/config/ConfigLogging.java index 36fe57118..23ca0d0bc 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigLogging.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigLogging.java @@ -14,4 +14,12 @@ public interface ConfigLogging extends BasicLogger { @LogMessage(level = Logger.Level.WARN) @Message(id = 1000, value = "Unable to get context classloader instance") void failedToRetrieveClassloader(@Cause Throwable cause); + + @LogMessage(level = Logger.Level.DEBUG) + @Message(id = 1001, value = "The config %s was loaded from %s with the value %s") + void lookup(String name, String source, String value); + + @LogMessage(level = Logger.Level.DEBUG) + @Message(id = 1002, value = "The config %s was not found") + void notFound(String name); } diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMessages.java b/implementation/src/main/java/io/smallrye/config/ConfigMessages.java index c4cb7627b..f7bc87c48 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigMessages.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigMessages.java @@ -85,4 +85,7 @@ interface ConfigMessages { @Message(id = 23, value = "Array type being converted is unknown") IllegalArgumentException unknownArrayType(); + + @Message(id = 24, value = "Not allowed to access secret key %s") + SecurityException notAllowed(String name); } diff --git a/implementation/src/main/java/io/smallrye/config/LoggingConfigSourceInterceptor.java b/implementation/src/main/java/io/smallrye/config/LoggingConfigSourceInterceptor.java new file mode 100644 index 000000000..cf17e7022 --- /dev/null +++ b/implementation/src/main/java/io/smallrye/config/LoggingConfigSourceInterceptor.java @@ -0,0 +1,33 @@ +package io.smallrye.config; + +import static io.smallrye.config.SecretKeys.doLocked; + +public class LoggingConfigSourceInterceptor implements ConfigSourceInterceptor { + @Override + public ConfigValue getValue(final ConfigSourceInterceptorContext context, final String name) { + try { + // Unlocked keys will run here. + ConfigValue configValue = doLocked(() -> context.proceed(name)); + if (configValue != null) + ConfigLogging.log.lookup(configValue.getName(), getLocation(configValue), configValue.getValue()); + else + ConfigLogging.log.notFound(name); + return configValue; + } catch (SecurityException e) { + // Handled next, to omit the values to log from the secret. + } + + // Locked keys here. + final ConfigValue secret = context.proceed(name); + if (secret != null) + ConfigLogging.log.lookup(secret.getName(), "secret", "secret"); + else + ConfigLogging.log.notFound(name); + return secret; + } + + private String getLocation(final ConfigValue configValue) { + return configValue.getLineNumber() != -1 ? configValue.getConfigSourceName() + ":" + configValue.getLineNumber() + : configValue.getConfigSourceName(); + } +} diff --git a/implementation/src/main/java/io/smallrye/config/SecretKeys.java b/implementation/src/main/java/io/smallrye/config/SecretKeys.java new file mode 100644 index 000000000..6680a95fc --- /dev/null +++ b/implementation/src/main/java/io/smallrye/config/SecretKeys.java @@ -0,0 +1,52 @@ +package io.smallrye.config; + +import java.io.Serializable; +import java.util.function.Supplier; + +public class SecretKeys implements Serializable { + private static final ThreadLocal LOCKED = ThreadLocal.withInitial(() -> Boolean.TRUE); + + public static boolean isLocked() { + return LOCKED.get(); + } + + public static void doUnlocked(Runnable runnable) { + doUnlocked(() -> { + runnable.run(); + return null; + }); + } + + public static T doUnlocked(Supplier supplier) { + if (isLocked()) { + LOCKED.set(false); + try { + return supplier.get(); + } finally { + LOCKED.set(true); + } + } else { + return supplier.get(); + } + } + + public static void doLocked(Runnable runnable) { + doLocked(() -> { + runnable.run(); + return null; + }); + } + + public static T doLocked(Supplier supplier) { + if (!isLocked()) { + LOCKED.set(true); + try { + return supplier.get(); + } finally { + LOCKED.set(false); + } + } else { + return supplier.get(); + } + } +} diff --git a/implementation/src/main/java/io/smallrye/config/SecretKeysConfigSourceInterceptor.java b/implementation/src/main/java/io/smallrye/config/SecretKeysConfigSourceInterceptor.java new file mode 100644 index 000000000..2e103b394 --- /dev/null +++ b/implementation/src/main/java/io/smallrye/config/SecretKeysConfigSourceInterceptor.java @@ -0,0 +1,23 @@ +package io.smallrye.config; + +import java.util.Set; + +public class SecretKeysConfigSourceInterceptor implements ConfigSourceInterceptor { + private final Set secrets; + + public SecretKeysConfigSourceInterceptor(final Set secrets) { + this.secrets = secrets; + } + + @Override + public ConfigValue getValue(final ConfigSourceInterceptorContext context, final String name) { + if (SecretKeys.isLocked() && isSecret(name)) { + throw ConfigMessages.msg.notAllowed(name); + } + return context.proceed(name); + } + + private boolean isSecret(final String name) { + return secrets.contains(name); + } +} diff --git a/implementation/src/main/java/io/smallrye/config/SmallRyeConfigBuilder.java b/implementation/src/main/java/io/smallrye/config/SmallRyeConfigBuilder.java index 0c3342dfa..f2ac0b50e 100644 --- a/implementation/src/main/java/io/smallrye/config/SmallRyeConfigBuilder.java +++ b/implementation/src/main/java/io/smallrye/config/SmallRyeConfigBuilder.java @@ -21,11 +21,13 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.OptionalInt; import java.util.ServiceLoader; +import java.util.Set; import java.util.function.Function; import java.util.function.UnaryOperator; import java.util.stream.Collectors; @@ -50,6 +52,7 @@ public class SmallRyeConfigBuilder implements ConfigBuilder { private List sources = new ArrayList<>(); private Function sourceWrappers = UnaryOperator.identity(); private Map converters = new HashMap<>(); + private Set secretKeys = new HashSet<>(); private List interceptors = new ArrayList<>(); private ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); private boolean addDefaultSources = false; @@ -150,6 +153,17 @@ public OptionalInt getPriority() { return OptionalInt.of(600); } })); + interceptors.add(new InterceptorWithPriority(new ConfigSourceInterceptorFactory() { + @Override + public ConfigSourceInterceptor getInterceptor(final ConfigSourceInterceptorContext context) { + return new SecretKeysConfigSourceInterceptor(secretKeys); + } + + @Override + public OptionalInt getPriority() { + return OptionalInt.of(-Integer.MAX_VALUE); + } + })); return interceptors; } @@ -185,6 +199,11 @@ public SmallRyeConfigBuilder withInterceptorFactories(ConfigSourceInterceptorFac return this; } + public SmallRyeConfigBuilder withSecretKeys(String... keys) { + secretKeys.addAll(Stream.of(keys).collect(Collectors.toSet())); + return this; + } + @Override public SmallRyeConfigBuilder withConverters(Converter[] converters) { for (Converter converter : converters) { @@ -245,6 +264,10 @@ Map getConverters() { return converters; } + Set getSecretKeys() { + return secretKeys; + } + List getInterceptors() { return interceptors; } diff --git a/implementation/src/main/java/io/smallrye/config/inject/ConfigProducerUtil.java b/implementation/src/main/java/io/smallrye/config/inject/ConfigProducerUtil.java index da75cb544..22165ebed 100644 --- a/implementation/src/main/java/io/smallrye/config/inject/ConfigProducerUtil.java +++ b/implementation/src/main/java/io/smallrye/config/inject/ConfigProducerUtil.java @@ -18,6 +18,7 @@ import org.eclipse.microprofile.config.spi.Converter; import io.smallrye.config.Converters; +import io.smallrye.config.SecretKeys; import io.smallrye.config.SmallRyeConfig; /** @@ -37,7 +38,7 @@ public static T getValue(InjectionPoint injectionPoint, Config config) { } final SmallRyeConfig src = (SmallRyeConfig) config; Converter converter = resolveConverter(injectionPoint, src); - String rawValue = src.getRawValue(name); + String rawValue = getRawValue(src, name); if (rawValue == null) { rawValue = getDefaultValue(injectionPoint); } @@ -58,6 +59,10 @@ public static T getValue(InjectionPoint injectionPoint, Config config) { return converted; } + private static String getRawValue(SmallRyeConfig config, String name) { + return SecretKeys.doUnlocked(() -> config.getRawValue(name)); + } + private static Converter resolveConverter(final InjectionPoint injectionPoint, final SmallRyeConfig src) { return resolveConverter(injectionPoint.getType(), src); } diff --git a/implementation/src/test/java/io/smallrye/config/ConfigSourceLoggingInterceptorTest.java b/implementation/src/test/java/io/smallrye/config/ConfigSourceLoggingInterceptorTest.java deleted file mode 100644 index 657cbe770..000000000 --- a/implementation/src/test/java/io/smallrye/config/ConfigSourceLoggingInterceptorTest.java +++ /dev/null @@ -1,41 +0,0 @@ -package io.smallrye.config; - -import org.eclipse.microprofile.config.Config; -import org.jboss.logging.Logger; -import org.junit.Assert; -import org.junit.Test; - -public class ConfigSourceLoggingInterceptorTest { - @Test - public void interceptor() { - SmallRyeConfig config = (SmallRyeConfig) buildConfig("my.prop", "1234"); - - final String value = config.getValue("my.prop", String.class); - Assert.assertEquals("1234", value); - } - - private static Config buildConfig(String... keyValues) { - return new SmallRyeConfigBuilder() - .addDefaultSources() - .withSources(KeyValuesConfigSource.config(keyValues)) - .withInterceptors(new LoggingConfigSourceInterceptor()) - .build(); - } - - private static class LoggingConfigSourceInterceptor implements ConfigSourceInterceptor { - private static final Logger LOG = Logger.getLogger("io.smallrye.config"); - - @Override - public ConfigValue getValue(final ConfigSourceInterceptorContext context, final String name) { - final ConfigValue configValue = context.proceed(name); - - final String key = configValue.getName(); - final String value = configValue.getValue(); - final String configSource = configValue.getConfigSourceName(); - - LOG.infov("The key {0} was loaded from {1} with the value {2}", key, configSource, value); - - return configValue; - } - } -} diff --git a/implementation/src/test/java/io/smallrye/config/LoggingConfigSourceInterceptorTest.java b/implementation/src/test/java/io/smallrye/config/LoggingConfigSourceInterceptorTest.java new file mode 100644 index 000000000..1bc6369ad --- /dev/null +++ b/implementation/src/test/java/io/smallrye/config/LoggingConfigSourceInterceptorTest.java @@ -0,0 +1,34 @@ +package io.smallrye.config; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import java.util.NoSuchElementException; + +import org.eclipse.microprofile.config.Config; +import org.junit.Test; + +public class LoggingConfigSourceInterceptorTest { + @Test + public void interceptor() throws Exception { + Config config = buildConfig(); + + assertEquals("abc", config.getValue("my.prop", String.class)); + assertThrows(SecurityException.class, () -> config.getValue("secret", String.class)); + assertThrows(NoSuchElementException.class, () -> config.getValue("not.found", String.class)); + + // This should not log the secret value: + assertEquals("12345678", SecretKeys.doUnlocked(() -> config.getValue("secret", String.class))); + } + + private static Config buildConfig() throws Exception { + return new SmallRyeConfigBuilder() + .addDefaultSources() + .addDefaultInterceptors() + .withSources(new ConfigValuePropertiesConfigSource( + LoggingConfigSourceInterceptorTest.class.getResource("/config-values.properties"))) + .withInterceptors(new LoggingConfigSourceInterceptor()) + .withSecretKeys("secret") + .build(); + } +} diff --git a/implementation/src/test/java/io/smallrye/config/SecretKeysTest.java b/implementation/src/test/java/io/smallrye/config/SecretKeysTest.java new file mode 100644 index 000000000..3f91c75c2 --- /dev/null +++ b/implementation/src/test/java/io/smallrye/config/SecretKeysTest.java @@ -0,0 +1,66 @@ +package io.smallrye.config; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import org.eclipse.microprofile.config.Config; +import org.junit.Test; + +public class SecretKeysTest { + @Test + public void lock() { + final Config config = buildConfig("secret", "12345678", "not.secret", "value"); + + assertThrows("Not allowed to access secret key secret", SecurityException.class, + () -> config.getValue("secret", String.class)); + assertEquals("value", config.getValue("not.secret", String.class)); + } + + @Test + public void unlock() { + final Config config = buildConfig("secret", "12345678", "not.secret", "value"); + + SecretKeys.doUnlocked(() -> assertEquals("12345678", config.getValue("secret", String.class))); + assertEquals("12345678", SecretKeys.doUnlocked(() -> config.getValue("secret", String.class))); + + assertThrows("Not allowed to access secret key secret", SecurityException.class, + () -> config.getValue("secret", String.class)); + } + + @Test + public void unlockAndLock() { + final Config config = buildConfig("secret", "12345678", "not.secret", "value"); + + SecretKeys.doUnlocked(() -> { + assertEquals("12345678", config.getValue("secret", String.class)); + + SecretKeys.doLocked(() -> { + assertThrows("Not allowed to access secret key secret", SecurityException.class, + () -> config.getValue("secret", String.class)); + }); + }); + + assertEquals("12345678", SecretKeys.doUnlocked(() -> config.getValue("secret", String.class))); + } + + @Test + public void lockAndUnlock() { + final Config config = buildConfig("secret", "12345678", "not.secret", "value"); + + SecretKeys.doLocked(() -> { + assertThrows("Not allowed to access secret key secret", SecurityException.class, + () -> config.getValue("secret", String.class)); + + SecretKeys.doUnlocked(() -> assertEquals("12345678", config.getValue("secret", String.class))); + }); + } + + private static Config buildConfig(String... keyValues) { + return new SmallRyeConfigBuilder() + .addDefaultSources() + .addDefaultInterceptors() + .withSources(KeyValuesConfigSource.config(keyValues)) + .withSecretKeys("secret") + .build(); + } +} diff --git a/implementation/src/test/java/io/smallrye/config/inject/ConfigInjectionTest.java b/implementation/src/test/java/io/smallrye/config/inject/ConfigInjectionTest.java new file mode 100644 index 000000000..097674205 --- /dev/null +++ b/implementation/src/test/java/io/smallrye/config/inject/ConfigInjectionTest.java @@ -0,0 +1,118 @@ +package io.smallrye.config.inject; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.net.URLClassLoader; +import java.net.URLConnection; +import java.net.URLStreamHandler; + +import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; + +import org.eclipse.microprofile.config.Config; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.jboss.weld.junit4.WeldInitiator; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; + +public class ConfigInjectionTest { + @BeforeClass + public static void beforeClass() throws Exception { + final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); + final URLClassLoader urlClassLoader = new URLClassLoader(new URL[] { + new URL("memory", null, 0, "/", + new InMemoryStreamHandler("io.smallrye.config.inject.ConfigInjectionTestConfigFactory")) + }, contextClassLoader); + Thread.currentThread().setContextClassLoader(urlClassLoader); + } + + @After + public void afterClass() { + final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader(contextClassLoader.getParent()); + } + + @Rule + public WeldInitiator weld = WeldInitiator.from(ConfigProducer.class, ConfigBean.class) + .addBeans() + .activate(ApplicationScoped.class) + .inject(this) + .build(); + + @Inject + private ConfigBean configBean; + + @Test + public void inject() { + assertEquals("1234", configBean.getMyProp()); + assertEquals("1234", configBean.getExpansion()); + assertEquals("12345678", configBean.getSecret()); + + assertThrows("Not allowed to access secret key secret", SecurityException.class, + () -> configBean.getConfig().getValue("secret", String.class)); + } + + @ApplicationScoped + public static class ConfigBean { + @Inject + @ConfigProperty(name = "my.prop") + private String myProp; + @Inject + @ConfigProperty(name = "expansion") + private String expansion; + @Inject + @ConfigProperty(name = "secret") + private String secret; + @Inject + private Config config; + + String getMyProp() { + return myProp; + } + + String getExpansion() { + return expansion; + } + + String getSecret() { + return secret; + } + + Config getConfig() { + return config; + } + } + + public static class InMemoryStreamHandler extends URLStreamHandler { + final byte[] contents; + + public InMemoryStreamHandler(final String contents) { + this.contents = contents.getBytes(); + } + + @Override + protected URLConnection openConnection(final URL u) throws IOException { + if (!u.getFile().endsWith("SmallRyeConfigFactory")) { + return null; + } + + return new URLConnection(u) { + @Override + public void connect() throws IOException { + } + + @Override + public InputStream getInputStream() throws IOException { + return new ByteArrayInputStream(contents); + } + }; + } + } +} diff --git a/implementation/src/test/java/io/smallrye/config/inject/ConfigInjectionTestConfigFactory.java b/implementation/src/test/java/io/smallrye/config/inject/ConfigInjectionTestConfigFactory.java new file mode 100644 index 000000000..1f67884b8 --- /dev/null +++ b/implementation/src/test/java/io/smallrye/config/inject/ConfigInjectionTestConfigFactory.java @@ -0,0 +1,19 @@ +package io.smallrye.config.inject; + +import io.smallrye.config.KeyValuesConfigSource; +import io.smallrye.config.SmallRyeConfig; +import io.smallrye.config.SmallRyeConfigFactory; +import io.smallrye.config.SmallRyeConfigProviderResolver; + +public class ConfigInjectionTestConfigFactory extends SmallRyeConfigFactory { + @Override + public SmallRyeConfig getConfigFor( + final SmallRyeConfigProviderResolver configProviderResolver, final ClassLoader classLoader) { + return configProviderResolver.getBuilder().forClassLoader(classLoader) + .addDefaultSources() + .addDefaultInterceptors() + .withSources(KeyValuesConfigSource.config("my.prop", "1234", "expansion", "${my.prop}", "secret", "12345678")) + .withSecretKeys("secret") + .build(); + } +} diff --git a/implementation/src/test/resources/config-values.properties b/implementation/src/test/resources/config-values.properties index f22753f97..41549918a 100644 --- a/implementation/src/test/resources/config-values.properties +++ b/implementation/src/test/resources/config-values.properties @@ -7,7 +7,7 @@ my.prop=abc - +secret=12345678