TextEncryptor not resolved from bootstrapContext in DecryptEnvironmentPostProcessor

[ninj]

Version Information

spring-cloud-context: 3.0.1

Details

Pre-Ilford, it was possible to inject a custom TextEncryptor into the framework to override the default implementation of decrypting a secret. (In my case, I am using GCP KMS to decrypt the secrets.)

The TextEncryptorBindHandler introduced in #872 allows for a custom TextEncryptor implementations by resolving from the bootstrap context: TextEncryptorConfigBootstrapper:L70

registry.registerIfAbsent(BindHandler.class, context -> {
    TextEncryptor textEncryptor = context.get(TextEncryptor.class);
    if (textEncryptor != null) {
        KeyProperties keyProperties = context.get(KeyProperties.class);
        return new TextEncryptorBindHandler(textEncryptor, keyProperties);
    }
    return null;
});

However, DecryptEnvironmentPostProcessor prefers to create it's own TextEncryptor: DecryptEnvironmentPostProcessor:L78

protected TextEncryptor getTextEncryptor(ConfigurableEnvironment environment) {
    Binder binder = Binder.get(environment);
    KeyProperties keyProperties = binder.bind(KeyProperties.PREFIX, KeyProperties.class)
            .orElseGet(KeyProperties::new);
    if (keysConfigured(keyProperties)) {
        setFailOnError(keyProperties.isFailOnError());
        if (ClassUtils.isPresent("org.springframework.security.rsa.crypto.RsaSecretEncryptor", null)) {
            RsaProperties rsaProperties = binder.bind(RsaProperties.PREFIX, RsaProperties.class)
                    .orElseGet(RsaProperties::new);
            return EncryptionBootstrapConfiguration.createTextEncryptor(keyProperties, rsaProperties);
        }
        return new EncryptorFactory(keyProperties.getSalt()).create(keyProperties.getKey());
    }
    // no keys configured
    return new FailsafeTextEncryptor();
}

Workaround

To enable custom decryption of properties in the spring environment:

• Subclass DecryptEnvironmentPostProcessor (with higher priority) to:
  • decrypt properties using custom TextEncryptor by overriding getTextEncryptor to create a new instance of the custom TextEncryptor.
  • inject a custom PropertySource to enable (for example) spring.config.use-legacy-processing to prevent DecryptEnvironmentPostProcessor from processing encrypted properties (*).
• Because the standard DecryptEnvironmentPostProcessor executes at lowest priority, use a high-priority ApplicationContextInitializer to remove the custom PropertySource before any other ApplicationContextInitializer implementations might execute and use the temporary value.

In addition to the above, to support decrypting properties with TextEncryptorBindHandler:

• Add a custom EnvironmentPostProcessor to register a TextEncryptor supplier that creates a new instance of the custom TextEncryptor.

(*) If DecryptEnvironmentPostProcessor executes (because neither spring cloud bootstrap nor legacy processing is enabled), this results in a FailsafeTextEncryptor being created (**). An exception gets thrown when DecryptEnvironmentPostProcessor also tries to decrypt any values that have been encrypted.

(**) Because the properties to configure a normal TextEncryptor are not defined, as we are intending to use a custom implementation instead.

Questions

• Should people be allowed to customise TextEncryptor during bootstrap to decrypt ConfigData values?
• If not, should people simply use a different prefix for encrypted values and clone the code for DecryptEnvironmentPostProcessor and AbstractEnvironmentDecrypt?
• Or alternatively, people could implement a ConfigData version of what is described in Spring Boot Configuration with Jasypt. However, this means moving away from using the {cipher} prefix and so away from how spring-cloud denotes encrypted values.

[spencergibb]

How did you get the TextEncryptor from SpringApplication?

[ninj]

How did you get the TextEncryptor from SpringApplication?

Sorry, I should have mentioned that I am trying to register a different TextEncryptor by using a higher-priority implementation of org.springframework.boot.Bootstrapper to register a TextEncryptor before TextEncryptorConfigBootstrapper does.

Basically I'd like to change the method of encryption/decryption for {cipher}-prefixed values by providing my own implementation of TextEncryptor.

[spencergibb]

I understand that, I specifically wanted to know how you got that TextEncryptor in your subclass of DecryptEnvironmentPostProcessor

[ninj]

I understand that, I specifically wanted to know how you got that TextEncryptor in your subclass of DecryptEnvironmentPostProcessor

Argh, I think I need a code sample for this - sorry.
I found some bugs in my first implementation as I was also trying to handle configuring the TextEncryptor via properties.
I'll update this ticket when I've got a sample ready.

update: erk, so reading over my own code still makes my head spin! I'll update the description of the workaround in the meantime, but this looks like it might be a little while to assemble the sample.

[spencergibb]

thanks!

[ninj]

So I've managed to cobble together an app that uses custom decryption of environment variables - custom decryption support for Binder to come later.

Current demo code is here: https://github.com/ninj/demo-custom-textencryptor

I think implementing this has given me some insights on what is going on when decrypting environment properties (though I have also thought that several times before 😭 )

• DecryptEnvironmentPostProcessor creating a fresh TextEncryptor by reading configuration from the latest Environment seems like the correct thing to do.
• Using a previously-cached TextEncryptor could be prone to problems where a TextEncryptor is cached but then properties to configure the TextEncryptor are then added to the Environment later on.
• DecryptEnvironmentPostProcessor should therefore not source a TextEncryptor from the bootstrapContext, as a cached instance might not have been configured with the latest values available in the environment.
• Potentially a factory method could be registered into the bootstrapContext, e.g. Function<ConfigurableEnvironment, TextEncryptor> that could be used by DecryptEnvironmentPostProcessor to create fresh TextEncryptor instance using latest environment values.

A word of caution - TextEncryptorConfigBootstrapper is actually only for supporting TextEncryptorBindHandler, and doesn't have anything to do with a TextEncryptor that might be used for DecryptEnvironmentPostProcessor.

Oh - I've just noticed #927, this will make the workaround to disable DecryptEnvironmentPostProcessor less onerous.