Skip to content

Commit

Permalink
Fixes smallrye#269. Support secret keys names.
Browse files Browse the repository at this point in the history
  • Loading branch information
radcortez committed Apr 23, 2020
1 parent 80d546e commit 9211172
Show file tree
Hide file tree
Showing 18 changed files with 454 additions and 44 deletions.
6 changes: 5 additions & 1 deletion doc/modules/ROOT/nav.adoc
Original file line number Diff line number Diff line change
@@ -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]
10 changes: 10 additions & 0 deletions doc/modules/ROOT/pages/config/config.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
:doctype: book
include::../attributes.adoc[]

[[cdi-extensions]]

= Config

* <<secret-keys>>

include::secret-keys.adoc[]
32 changes: 32 additions & 0 deletions doc/modules/ROOT/pages/config/secret-keys.adoc
Original file line number Diff line number Diff line change
@@ -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<T>)`.

[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.
10 changes: 10 additions & 0 deletions doc/modules/ROOT/pages/interceptors/interceptors.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ SmallRye Config provides the following built-in Interceptors:
* <<profile-interceptor>>
* <<relocate-interceptor>>
* <<fallback-interceptor>>
* <<logging-interceptor>>

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
Expand Down Expand Up @@ -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.
11 changes: 11 additions & 0 deletions implementation/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,17 @@
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jboss.weld</groupId>
<artifactId>weld-junit4</artifactId>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>javax.enterprise</groupId>
<artifactId>cdi-api</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
52 changes: 52 additions & 0 deletions implementation/src/main/java/io/smallrye/config/SecretKeys.java
Original file line number Diff line number Diff line change
@@ -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<Boolean> 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> T doUnlocked(Supplier<T> 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> T doLocked(Supplier<T> supplier) {
if (!isLocked()) {
LOCKED.set(true);
try {
return supplier.get();
} finally {
LOCKED.set(false);
}
} else {
return supplier.get();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package io.smallrye.config;

import java.util.Set;

public class SecretKeysConfigSourceInterceptor implements ConfigSourceInterceptor {
private final Set<String> secrets;

public SecretKeysConfigSourceInterceptor(final Set<String> 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -50,6 +52,7 @@ public class SmallRyeConfigBuilder implements ConfigBuilder {
private List<ConfigSource> sources = new ArrayList<>();
private Function<ConfigSource, ConfigSource> sourceWrappers = UnaryOperator.identity();
private Map<Type, ConverterWithPriority> converters = new HashMap<>();
private Set<String> secretKeys = new HashSet<>();
private List<InterceptorWithPriority> interceptors = new ArrayList<>();
private ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
private boolean addDefaultSources = false;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -245,6 +264,10 @@ Map<Type, ConverterWithPriority> getConverters() {
return converters;
}

Set<String> getSecretKeys() {
return secretKeys;
}

List<InterceptorWithPriority> getInterceptors() {
return interceptors;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -37,7 +38,7 @@ public static <T> T getValue(InjectionPoint injectionPoint, Config config) {
}
final SmallRyeConfig src = (SmallRyeConfig) config;
Converter<T> converter = resolveConverter(injectionPoint, src);
String rawValue = src.getRawValue(name);
String rawValue = getRawValue(src, name);
if (rawValue == null) {
rawValue = getDefaultValue(injectionPoint);
}
Expand All @@ -58,6 +59,10 @@ public static <T> T getValue(InjectionPoint injectionPoint, Config config) {
return converted;
}

private static String getRawValue(SmallRyeConfig config, String name) {
return SecretKeys.doUnlocked(() -> config.getRawValue(name));
}

private static <T> Converter<T> resolveConverter(final InjectionPoint injectionPoint, final SmallRyeConfig src) {
return resolveConverter(injectionPoint.getType(), src);
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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();
}
}
Loading

0 comments on commit 9211172

Please sign in to comment.