Allow Spring Security's RSA key converters to be used when binding configuration properties

[jzheaux]

Spring Security ships with converters for reading RSA public and private key files. These are applied to the application context's ConversionService through a BeanFactoryPostProcessor.

This allows an application to do things like:

@ConfigurationProperties("jwt")
public class Jwt {
    private RSAPublicKey key;
}

to retrieve keys from configuration.

This doesn't work, though, if a Spring Boot application includes auto-configuration that includes a @ConfigurationPropertiesBinding for another set of properties. It appears this may change the loading order such that Spring Security's RsaKeyConversionServicePostProcessor doesn't get applied to Boot's conversion service.

I believe the correct enhancement is for Spring Boot to add @ConfigurationPropertiesBinding @Beans to Security's auto configuration like so:

@Bean
@ConfigurationPropertiesBinding
Converter<String, RSAPrivateKey> privateKeys() {
    return new ResourceKeyConverterAdapter<>(RsaKeyConverters.pkcs8());
}

@Bean
@ConfigurationPropertiesBinding
Converter<String, RSAPublicKey> publicKeys() {
    return new ResourceKeyConverterAdapter<>(RsaKeyConverters.x509());
}

[wilkinsona]

ResourceKeyConverterAdapter was added in this commit and isn't yet in a release.

[wilkinsona]

I'm struggling to reproduce this in a test. With a @ConfigurationPropertiesBinding converter defined and the binder, therefore, using its own ApplicationConversionService, I still see the converters configured by Spring Security being used. This happens because the binder copies over the editors that have been registered with the bean factory.

I think we need to look again at the sample that accompanied the security issue to figure out exactly where the Security-registered converters will and will not be used.

[wilkinsona]

I've figured out the difference in behaviour. It's to do with the way in which Spring Security registers the converters and how the binder copies things over.

ConfigurationPropertiesBinder copies over the property editors using the bean factory's copyRegisteredEditorsTo method:

spring-boot/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBinder.java

Lines 187 to 192 in 713c0fc

	private Consumer<PropertyEditorRegistry> getPropertyEditorInitializer() {
	if (this.applicationContext instanceof ConfigurableApplicationContext) {
	return ((ConfigurableApplicationContext) this.applicationContext).getBeanFactory()::copyRegisteredEditorsTo;
	}
	return null;
	}

Spring Security registers the converters differently depending on whether or not the bean factory has a conversion service:

ConversionService service = beanFactory.getConversionService();
if (service instanceof ConverterRegistry) {
	ConverterRegistry registry = (ConverterRegistry) service;
	registry.addConverter(String.class, RSAPrivateKey.class, this.pkcs8);
	registry.addConverter(String.class, RSAPublicKey.class, this.x509);
}
else {
	beanFactory.addPropertyEditorRegistrar((registry) -> {
		registry.registerCustomEditor(RSAPublicKey.class, new ConverterPropertyEditorAdapter<>(this.x509));
		registry.registerCustomEditor(RSAPrivateKey.class, new ConverterPropertyEditorAdapter<>(this.pkcs8));
	});
}

The bean factory in a typical Spring Boot application will have a conversion service configured. This causes the converters to be registered directly with the conversion service rather than via a property editor registrar. As a result, there's no editor to be copied over and the converters are lost to the binder.

When I tried to recreate the problem in a test, I used ApplicationContextRunner. This omits any configuration typically performed by SpringApplication so the context's bean factory did not have a conversion service configured. This resulted in Spring Security adding a property editor registrar for the RSA key converters which would then be copied over to the binder's conversion service. The problem can be reproduced with an ApplicationContextRunner-based test using an application context initializer to set the conversion service on the context's bean factory.

Here is a fix for the problem. The code itself isn't too bad, but I'm not entirely happy with it for a couple of reasons:

1. The whole arrangement feels rather brittle due to the different behaviour when there is and is not a @ConfigurationPropertiesBinding converter and when the bean factory does and does not have a conversion service configured
2. ResourceKeyConverterAdapter needs to be sub-classed to provide sufficient generic type information for the converter registration to work. Without the sub-classes the converters are both registered as String -> java.security.Key as the type constraint on the adapter is T extends Key

I've opened spring-projects/spring-security#9626 for 2. I'm not sure what, if anything, we can do about 1.

[philwebb]

2 is a problem I've faced before. It's really a weakness with our registration logic. I've opened #26034 to see if we can improve that.

1 is a bit more tricky. I feel like a typical Boot application wouldn't want the RsaKeyConversionServicePostProcessor to run. AFAICT there's no way to opt-out of it if you're using @EnableWebSecurity. Perhaps we can somehow remove the post-processor bean before it's used?