Hide secrets in configuration

[dmlloyd]

Description
We have configuration properties which may contain passwords or other kinds of secrets.  At least one means of viewing or dumping the running configuration does, or will soon, exist (see #7430).  We don't want secrets to appear in the output as this could be a security problem.

We talked about vault-style configuration sources - of which we now have at least one.  The vault config source protects against its contents from being iterated by the simple measure of returning no property names, so querying a vault entry by name directly returns its value but there's no way to know what names are present in the vault. This works to the extent that the vault contents cannot be queried, however, it doesn't prevent the keys from being found if other config sources have the same key, or refer to the key indirectly via a property expression. We can mitigate the latter problem by ensuring that all config viewers suppress expansion (see this example).

However, there is still the basic problem that passwords and secrets are generally encoded simply as strings (which is to say, using a vault is not mandatory), and the user often provides a plain value which is iterable and viewable.

We need some way to "lock" the configuration so that secrets do not appear unless explicitly enabled.

Implementation ideas

Idea 1: forbid clear secrets

We could fail build or startup if a property which is known to be secret contains a value that is not an expression. This could push users towards using a vault.

We cannot prevent users from simply putting the clear value somewhere else in the configuration and then referring to that location via expression though, so this approach is questionable.

This idea requires that the properties which are secret be enumerated during build.

Idea 2: hide the values

We can acknowledge the reality that the configuration can and will contain secrets no matter what we do, and introduce another ConfigSource wrapper which intercepts requests for properties which are known to be secret. It will return a placeholder value like <secret> for these properties unless it is explicitly disabled e.g.:

SecretConfigSource.accessSecretsFor(() -> {
    // this code has cleartext access to secrets
});

We could automatically enable access for our own configuration object population and the MicroProfile spec CDI configuration handling. Beyond that we would require documentation for users who wish to store secret values.

This idea also requires that the properties which are secret be enumerated during build.

Appendix: what are we even doing with strings

We should not use String to hold secret values. I'd recommend using TwoWayPassword ClearPassword (with an associated converter) as the type for all client password values, and OneWayPassword Password (with a converter based on modular crypt) for all server password values. I'd also recommend directly using JCE Key subtypes for public/private/secret keys, if and when we ever spec those, in plain base64 (like SSH) or PEM format depending on the use case and expected key type. Let the converters do the parsing and validation work in a unified way here.

By doing this, we can automatically detect what properties are secret right in the extension loader and automatically generate corresponding build items which can configure the appropriate mechanism. Otherwise we would need a different mechanism (e.g. an annotation) which adds more chance for user error. Using types to carry semantic information about the configuration value is idiomatic for MP Config, so I'd recommend using this approach rather than something else.

[nimo23]

Good idea. Relates to #6510 (and a little to #6034).

Actually, distributing the quarkus app to cloud servers or devices is not secure..

[gastaldi]

Relates to the use case in eclipse/microprofile-config#451

[radcortez]

We just added support for this in SmallRye Config: smallrye/smallrye-config#269

Not released yet, but once done I'll integrate it in Quarkus.

[radcortez]

There is now a way to support this in SmallRye Config latest version, which is also available in Quarkus.

How do we envision this being used in Quarkus? In SmallRye Config, you can list secret keys with the API SmallRyeConfigBuilder#withSecretKeys. Should we came up with a list of our own and auto configure it? Should we provide a configuration property that lists keys that are known to be secrets?

What are your thoughts @dmlloyd ?

[dmlloyd]

Quarkus configuration values which are secrets should have a type which unambiguously identifies the value as a secret, e.g. a Key subtype. The listing of secret properties should thus be automatic based on whether the property has a type which is a secret type.

Another thing to think about is that it's not really possible to list all property names which are secret, because in many cases the property name might be a pattern (part of a Map for example). So the configuration (interceptor?) would have to recognize secret property names by pattern (like we do for default values).

Manually listing property names is not a good solution in any event.

[radcortez]

Sure. I think there are two angles:

• Quarkus exposed configuration.
• User custom configuration.

For Quarkus configuration, since we control it, we could implement the type Key and find patterns to match the secrets that we have.

For user custom configuration, right now, we don't have a good way to set this up. You can only do this in the SmallRyeBuilder which will force users to implement their own ConfigProvider. I'm mostly wondering if we should support a configuration where an end user could list such names or patterns, like we have for profile for instance.

[brentcrammond]

I have written a Secret Config Source (using jasypt library for encryption), with a separate secrets.properties in the config directory, it works with {encrypted} and {plain} prefixes to the property values, {plain} will encrypt the string and dump it to the log (not sure if this is the best option, but works), I like the dynamic decryption mechanism and might look at implementing it:

`
@slf4j
public class SecretConfigProvider implements ConfigSource {
private static final String CIPHER_PREFIX = "{encrypted}";
private static final String PLAIN_PREFIX = "{plain}";

private final StrongTextEncryptor encryptor;
private final Map<String, String> properties;

public SecretConfigProvider() {
    var password = Constants.PROPERTY_PASSWORD;
    encryptor = new StrongTextEncryptor();
    encryptor.setPasswordCharArray(password.toCharArray());

    var props = new Properties();
    try (var is = new FileInputStream(new File("config/secrets.properties"))) {
        props.load(is);
    } catch (FileNotFoundException e) {
        log.error("File config/secrets.properties not found", e);
    } catch (IOException e) {
        log.error("Problem loading file config/secrets.properties", e);
    }
    properties = props.entrySet()
            .stream()
            .collect(Collectors.toMap(e -> e.getKey().toString(), e -> decrypt(e.getKey().toString(), e.getValue().toString())));
}

@Override
public Map<String, String> getProperties() {
    return properties;
}

@Override
public String getValue(String propertyName) {
    return properties.get(propertyName);
}

@Override
public String getName() {
    return "SecretConfigProvider";
}

@Override
public int getOrdinal() {
    return 250;
}

//
// Internal Methods...
//

private String decrypt(String name, String value) {
    if (value == null) {
        return null;
    }
    if (value.startsWith(CIPHER_PREFIX)) {
        var val = value.substring(CIPHER_PREFIX.length());
        try {
            value = encryptor.decrypt(val);
        } catch (Exception e) {
            log.warn(String.format("Cannot decrypt: %s", value), e);
            // Set value to empty to avoid making a password out of the
            // cipher text
            value = "";
        }
    } else if (value.startsWith(PLAIN_PREFIX)) {
        var val = value.substring(PLAIN_PREFIX.length());
        var encValue = encryptor.encrypt(val);
        log.info("To Encrypt: {}={}", name, CIPHER_PREFIX + encValue);
        value = val;
    }
    return value;
}

}
`

[dthommes]

Hi everybody,

we were searching for a solution for encrypting certain configuration properties to obscure their plain text in our client app (native image). The properties should be read from our existing application.properties file.

We came up with the following ConfigSourceInterceptor, that uses a hard-coded AES key/iv to decrypt base64-encoded, secret strings, if they are prefixed with secret. in the application.properties.

Such a ConfigSourceInterceptor needs to be registered via the resource (see documentation):

src/main/resources/META-INF/services/io.smallrye.config.ConfigSourceInterceptor

In this example containing the string

com.example.SecretsConfigInterceptor

Please feel free to use and extend this for your own apps. Note: Currently this interceptor is not thread safe. I investigated via debugger, that its methods are typically only called from Quarkus main thread. If you experience access via different threads, please let me know. This would require a little refactoring for the cipher field.

package com.example;

import io.smallrye.config.ConfigSourceInterceptor;
import io.smallrye.config.ConfigSourceInterceptorContext;
import io.smallrye.config.ConfigValue;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.crypto.Cipher;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;
import java.nio.charset.StandardCharsets;
import java.security.Key;
import java.util.Base64;

/**
 * {@link ConfigSourceInterceptor} to decrypt secret configuration properties on the fly using a hard-coded key/iv.
 */
public class SecretsConfigInterceptor implements ConfigSourceInterceptor {

    private static final Logger logger = LoggerFactory.getLogger(SecretsConfigInterceptor.class);

    private static final String PREFIX = "secret.";
    static final String ALGORITHM = "AES/CBC/PKCS5Padding";
    static final String KEY_ALGORITHM = "AES";

    // TODO enter your IV
    static final byte[] INIT_VECTOR = new byte[]{(byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00,
            (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00,
            (byte) 0x00, (byte) 0x00, (byte) 0x00};

    // TODO enter your KEY
    static final byte[] KEY = new byte[]{(byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00,
            (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00,
            (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00,
            (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00,
            (byte) 0x00, (byte) 0x00, (byte) 0x00};

    private IvParameterSpec ivParameterSpec;
    private Key keySpec;
    private Cipher cipher;

    public SecretsConfigInterceptor() {
        try {
            cipher = Cipher.getInstance(ALGORITHM);
        } catch (Exception e) {
            throw new IllegalStateException("Could not initialize Cipher " + ALGORITHM, e);
        }
        ivParameterSpec = new IvParameterSpec(INIT_VECTOR);
        keySpec = new SecretKeySpec(KEY, KEY_ALGORITHM);
    }

    @Override
    public ConfigValue getValue(ConfigSourceInterceptorContext context, String name) {
        String secretName = PREFIX + name;
        ConfigValue configValue = context.proceed(secretName);
        if (configValue != null) {
            // Found a "secret" value
            return decryptValue(name, configValue);
        }
        // Fall back to standard behavior
        return context.proceed(name);
    }

    /**
     * Decrypts a ConfigValue
     *
     * @param name        the original name to use for the decrypted ConfigValue
     * @param configValue the config value
     * @return the decrypted config value with the original name
     */
    ConfigValue decryptValue(String name, ConfigValue configValue) {
        ConfigValue newConfigValue = configValue;
        String value = configValue.getValue();
        if (value != null && !"".equals(value)) {
            logger.debug("Decrypting {}", configValue.getName());
            try {
                // Secret comes as base64 encrypted string, decoding it
                byte[] decoded = Base64.getDecoder().decode(value);

                // decrypting the decoded secret with the master key and iv
                cipher.init(Cipher.DECRYPT_MODE, keySpec, ivParameterSpec);
                byte[] decrypted = cipher.doFinal(decoded);

                // We will actually feed a String into the properties file, as this is a password
                String newValue = new String(decrypted, StandardCharsets.UTF_8);

                ConfigValue decryptedValue = new ConfigValue.ConfigValueBuilder().
                        withName(name)
                        .withValue(newValue)
                        .withConfigSourceName(configValue.getConfigSourceName())
                        .withConfigSourceOrdinal(configValue.getConfigSourceOrdinal())
                        .withLineNumber(configValue.getLineNumber())
                        .withRawValue(configValue.getRawValue())
                        .build();

                newConfigValue = decryptedValue;

            } catch (Exception e) {
                throw new RuntimeException("Could not decrypt secret " + name + ": ", e);
            }
        }
        return newConfigValue;
    }


}

[radcortez]

@dthommes Thanks. I'll have a look.

[jcunliffe1]

In previous solutions, you seem to be happy with storing the decrypted password to a String, when clearly char[] are preferred to narrow the attack window: https://www.techiedelight.com/why-character-array-preferred-over-string-storing-passwords/

Shouldn't we make org.eclipse.microprofile.config.ConfigValue.java#getValue() generic, with String being the 'default', but others (like char[]) also being ok?

[geoand]

@radcortez am I right to assume that we don't currently integrate with this in Quarkus?

[radcortez]

Right now, not automatically. Users can add their own SecretKeysInterceptor and manually list the secret keys:
https://smallrye.io/smallrye-config/Main/config/secret-keys/. (We changed the docs to reflect this better).

We have another issue open to provide better integration:
#29027

[geoand]

Gotcha, thanks!