From ca1252be9410487bcce550c85ebac4ced08f73e3 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Wed, 10 Jun 2020 10:08:31 -0500 Subject: [PATCH] Replace whitelist with allowlist Issue gh-8676 --- build.gradle | 6 +++- .../jackson2/SecurityJackson2Modules.java | 32 +++++++++---------- .../SecurityJackson2ModulesTests.java | 32 +++++++++---------- .../reactive/oauth2/resource-server.adoc | 2 +- .../servlet/oauth2/oauth2-resourceserver.adoc | 4 +-- .../{whitelist.lines => allowlist.lines} | 0 ...wtIssuerAuthenticationManagerResolver.java | 10 +++--- ...ReactiveAuthenticationManagerResolver.java | 10 +++--- .../web/firewall/StrictHttpFirewall.java | 2 +- 9 files changed, 51 insertions(+), 47 deletions(-) rename etc/nohttp/{whitelist.lines => allowlist.lines} (100%) diff --git a/build.gradle b/build.gradle index 70125633cb..67b6e77640 100644 --- a/build.gradle +++ b/build.gradle @@ -2,7 +2,7 @@ buildscript { dependencies { classpath 'io.spring.gradle:spring-build-conventions:0.0.32.RELEASE' classpath "org.springframework.boot:spring-boot-gradle-plugin:$springBootVersion" - classpath 'io.spring.nohttp:nohttp-gradle:0.0.2.RELEASE' + classpath 'io.spring.nohttp:nohttp-gradle:0.0.5.RELEASE' classpath "io.freefair.gradle:aspectj-plugin:5.0.1" classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion" } @@ -39,3 +39,7 @@ subprojects { options.encoding = "UTF-8" } } + +nohttp { + allowlistFile = project.file("etc/nohttp/allowlist.lines") +} diff --git a/core/src/main/java/org/springframework/security/jackson2/SecurityJackson2Modules.java b/core/src/main/java/org/springframework/security/jackson2/SecurityJackson2Modules.java index f10fefa8fc..011ee95b1a 100644 --- a/core/src/main/java/org/springframework/security/jackson2/SecurityJackson2Modules.java +++ b/core/src/main/java/org/springframework/security/jackson2/SecurityJackson2Modules.java @@ -90,7 +90,7 @@ public final class SecurityJackson2Modules { if (mapper != null) { TypeResolverBuilder typeBuilder = mapper.getDeserializationConfig().getDefaultTyper(null); if (typeBuilder == null) { - mapper.setDefaultTyping(createWhitelistedDefaultTyping()); + mapper.setDefaultTyping(createAllowlistedDefaultTyping()); } } } @@ -148,11 +148,11 @@ public final class SecurityJackson2Modules { } /** - * Creates a TypeResolverBuilder that performs whitelisting. - * @return a TypeResolverBuilder that performs whitelisting. + * Creates a TypeResolverBuilder that restricts allowed types. + * @return a TypeResolverBuilder that restricts allowed types. */ - private static TypeResolverBuilder createWhitelistedDefaultTyping() { - TypeResolverBuilder result = new WhitelistTypeResolverBuilder(ObjectMapper.DefaultTyping.NON_FINAL); + private static TypeResolverBuilder createAllowlistedDefaultTyping() { + TypeResolverBuilder result = new AllowlistTypeResolverBuilder(ObjectMapper.DefaultTyping.NON_FINAL); result = result.init(JsonTypeInfo.Id.CLASS, null); result = result.inclusion(JsonTypeInfo.As.PROPERTY); return result; @@ -164,9 +164,9 @@ public final class SecurityJackson2Modules { * and overrides the {@code TypeIdResolver} * @author Rob Winch */ - static class WhitelistTypeResolverBuilder extends ObjectMapper.DefaultTypeResolverBuilder { + static class AllowlistTypeResolverBuilder extends ObjectMapper.DefaultTypeResolverBuilder { - WhitelistTypeResolverBuilder(ObjectMapper.DefaultTyping defaultTyping) { + AllowlistTypeResolverBuilder(ObjectMapper.DefaultTyping defaultTyping) { super( defaultTyping, //we do explicit validation in the TypeIdResolver @@ -182,17 +182,17 @@ public final class SecurityJackson2Modules { PolymorphicTypeValidator subtypeValidator, Collection subtypes, boolean forSer, boolean forDeser) { TypeIdResolver result = super.idResolver(config, baseType, subtypeValidator, subtypes, forSer, forDeser); - return new WhitelistTypeIdResolver(result); + return new AllowlistTypeIdResolver(result); } } /** * A {@link TypeIdResolver} that delegates to an existing implementation and throws an IllegalStateException if the - * class being looked up is not whitelisted, does not provide an explicit mixin, and is not annotated with Jackson + * class being looked up is not in the allowlist, does not provide an explicit mixin, and is not annotated with Jackson * mappings. See https://github.com/spring-projects/spring-security/issues/4370 */ - static class WhitelistTypeIdResolver implements TypeIdResolver { - private static final Set WHITELIST_CLASS_NAMES = Collections.unmodifiableSet(new HashSet(Arrays.asList( + static class AllowlistTypeIdResolver implements TypeIdResolver { + private static final Set ALLOWLIST_CLASS_NAMES = Collections.unmodifiableSet(new HashSet(Arrays.asList( "java.util.ArrayList", "java.util.Collections$EmptyList", "java.util.Collections$EmptyMap", @@ -209,7 +209,7 @@ public final class SecurityJackson2Modules { private final TypeIdResolver delegate; - WhitelistTypeIdResolver(TypeIdResolver delegate) { + AllowlistTypeIdResolver(TypeIdResolver delegate) { this.delegate = delegate; } @@ -238,7 +238,7 @@ public final class SecurityJackson2Modules { DeserializationConfig config = (DeserializationConfig) context.getConfig(); JavaType result = delegate.typeFromId(context, id); String className = result.getRawClass().getName(); - if (isWhitelisted(className)) { + if (isInAllowlist(className)) { return result; } boolean isExplicitMixin = config.findMixInClassFor(result.getRawClass()) != null; @@ -249,14 +249,14 @@ public final class SecurityJackson2Modules { if (jacksonAnnotation != null) { return result; } - throw new IllegalArgumentException("The class with " + id + " and name of " + className + " is not whitelisted. " + + throw new IllegalArgumentException("The class with " + id + " and name of " + className + " is not in the allowlist. " + "If you believe this class is safe to deserialize, please provide an explicit mapping using Jackson annotations or by providing a Mixin. " + "If the serialization is only done by a trusted source, you can also enable default typing. " + "See https://github.com/spring-projects/spring-security/issues/4370 for details"); } - private boolean isWhitelisted(String id) { - return WHITELIST_CLASS_NAMES.contains(id); + private boolean isInAllowlist(String id) { + return ALLOWLIST_CLASS_NAMES.contains(id); } @Override diff --git a/core/src/test/java/org/springframework/security/jackson2/SecurityJackson2ModulesTests.java b/core/src/test/java/org/springframework/security/jackson2/SecurityJackson2ModulesTests.java index da7aac86f0..e28aaf0ffd 100644 --- a/core/src/test/java/org/springframework/security/jackson2/SecurityJackson2ModulesTests.java +++ b/core/src/test/java/org/springframework/security/jackson2/SecurityJackson2ModulesTests.java @@ -44,20 +44,20 @@ public class SecurityJackson2ModulesTests { } @Test - public void readValueWhenNotWhitelistedOrMappedThenThrowsException() { - String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotWhitelisted\",\"property\":\"bar\"}"; + public void readValueWhenNotAllowedOrMappedThenThrowsException() { + String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotAllowlisted\",\"property\":\"bar\"}"; assertThatThrownBy(() -> { mapper.readValue(content, Object.class); } - ).hasStackTraceContaining("whitelisted"); + ).hasStackTraceContaining("allowlist"); } @Test public void readValueWhenExplicitDefaultTypingAfterSecuritySetupThenReadsAsSpecificType() throws Exception { mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL, JsonTypeInfo.As.PROPERTY); - String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotWhitelisted\",\"property\":\"bar\"}"; + String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotAllowlisted\",\"property\":\"bar\"}"; - assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotWhitelisted.class); + assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotAllowlisted.class); } @Test @@ -65,29 +65,29 @@ public class SecurityJackson2ModulesTests { mapper = new ObjectMapper(); mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL, JsonTypeInfo.As.PROPERTY); SecurityJackson2Modules.enableDefaultTyping(mapper); - String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotWhitelisted\",\"property\":\"bar\"}"; + String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotAllowlisted\",\"property\":\"bar\"}"; - assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotWhitelisted.class); + assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotAllowlisted.class); } @Test public void readValueWhenAnnotatedThenReadsAsSpecificType() throws Exception { - String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotWhitelistedButAnnotated\",\"property\":\"bar\"}"; + String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotAllowlistedButAnnotated\",\"property\":\"bar\"}"; - assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotWhitelistedButAnnotated.class); + assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotAllowlistedButAnnotated.class); } @Test public void readValueWhenMixinProvidedThenReadsAsSpecificType() throws Exception { - mapper.addMixIn(NotWhitelisted.class, NotWhitelistedMixin.class); - String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotWhitelisted\",\"property\":\"bar\"}"; + mapper.addMixIn(NotAllowlisted.class, NotAllowlistedMixin.class); + String content = "{\"@class\":\"org.springframework.security.jackson2.SecurityJackson2ModulesTests$NotAllowlisted\",\"property\":\"bar\"}"; - assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotWhitelisted.class); + assertThat(mapper.readValue(content, Object.class)).isInstanceOf(NotAllowlisted.class); } @Test public void readValueWhenHashMapThenReadsAsSpecificType() throws Exception { - mapper.addMixIn(NotWhitelisted.class, NotWhitelistedMixin.class); + mapper.addMixIn(NotAllowlisted.class, NotAllowlistedMixin.class); String content = "{\"@class\":\"java.util.HashMap\"}"; assertThat(mapper.readValue(content, Object.class)).isInstanceOf(HashMap.class); @@ -99,7 +99,7 @@ public class SecurityJackson2ModulesTests { public @interface NotJacksonAnnotation {} @NotJacksonAnnotation - static class NotWhitelisted { + static class NotAllowlisted { private String property = "bar"; public String getProperty() { @@ -111,7 +111,7 @@ public class SecurityJackson2ModulesTests { } @JsonIgnoreType(false) - static class NotWhitelistedButAnnotated { + static class NotAllowlistedButAnnotated { private String property = "bar"; public String getProperty() { @@ -126,7 +126,7 @@ public class SecurityJackson2ModulesTests { @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE, isGetterVisibility = JsonAutoDetect.Visibility.NONE) @JsonIgnoreProperties(ignoreUnknown = true) - abstract class NotWhitelistedMixin { + abstract class NotAllowlistedMixin { } } diff --git a/docs/manual/src/docs/asciidoc/_includes/reactive/oauth2/resource-server.adoc b/docs/manual/src/docs/asciidoc/_includes/reactive/oauth2/resource-server.adoc index cfb4f85fcb..16413cd94f 100644 --- a/docs/manual/src/docs/asciidoc/_includes/reactive/oauth2/resource-server.adoc +++ b/docs/manual/src/docs/asciidoc/_includes/reactive/oauth2/resource-server.adoc @@ -1082,7 +1082,7 @@ In this case, you construct `JwtIssuerReactiveAuthenticationManagerResolver` wit This approach allows us to add and remove elements from the repository (shown as a `Map` in the snippet) at runtime. NOTE: It would be unsafe to simply take any issuer and construct an `ReactiveAuthenticationManager` from it. -The issuer should be one that the code can verify from a trusted source like a whitelist. +The issuer should be one that the code can verify from a trusted source like an allowed list of issuers. [[webflux-oauth2resourceserver-bearertoken-resolver]] == Bearer Token Resolution diff --git a/docs/manual/src/docs/asciidoc/_includes/servlet/oauth2/oauth2-resourceserver.adoc b/docs/manual/src/docs/asciidoc/_includes/servlet/oauth2/oauth2-resourceserver.adoc index 0ff9c92939..f3844ed84a 100644 --- a/docs/manual/src/docs/asciidoc/_includes/servlet/oauth2/oauth2-resourceserver.adoc +++ b/docs/manual/src/docs/asciidoc/_includes/servlet/oauth2/oauth2-resourceserver.adoc @@ -1857,7 +1857,7 @@ In this case, you construct `JwtIssuerAuthenticationManagerResolver` with a stra This approach allows us to add and remove elements from the repository (shown as a `Map` in the snippet) at runtime. NOTE: It would be unsafe to simply take any issuer and construct an `AuthenticationManager` from it. -The issuer should be one that the code can verify from a trusted source like a whitelist. +The issuer should be one that the code can verify from a trusted source like a list of allowed issuers. ===== Parsing the Claim Only Once @@ -1907,7 +1907,7 @@ public class TenantJWSKeySelector ---- <1> A hypothetical source for tenant information <2> A cache for `JWKKeySelector`s, keyed by tenant identifier -<3> Looking up the tenant is more secure than simply calculating the JWK Set endpoint on the fly - the lookup acts as a tenant whitelist +<3> Looking up the tenant is more secure than simply calculating the JWK Set endpoint on the fly - the lookup acts as a list of allowed tenants <4> Create a `JWSKeySelector` via the types of keys that come back from the JWK Set endpoint - the lazy lookup here means that you don't need to configure all tenants at startup The above key selector is a composition of many key selectors. diff --git a/etc/nohttp/whitelist.lines b/etc/nohttp/allowlist.lines similarity index 100% rename from etc/nohttp/whitelist.lines rename to etc/nohttp/allowlist.lines diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerAuthenticationManagerResolver.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerAuthenticationManagerResolver.java index 3f132fbe2c..97cc3d3fe9 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerAuthenticationManagerResolver.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerAuthenticationManagerResolver.java @@ -45,7 +45,7 @@ import org.springframework.util.Assert; * * To use, this class must be able to determine whether or not the `iss` claim is trusted. Recall that * anyone can stand up an authorization server and issue valid tokens to a resource server. The simplest way - * to achieve this is to supply a whitelist of trusted issuers in the constructor. + * to achieve this is to supply a list of trusted issuers in the constructor. * * This class derives the Issuer from the `iss` claim found in the {@link HttpServletRequest}'s * Bearer Token. @@ -60,7 +60,7 @@ public final class JwtIssuerAuthenticationManagerResolver implements Authenticat /** * Construct a {@link JwtIssuerAuthenticationManagerResolver} using the provided parameters * - * @param trustedIssuers a whitelist of trusted issuers + * @param trustedIssuers a list of trusted issuers */ public JwtIssuerAuthenticationManagerResolver(String... trustedIssuers) { this(Arrays.asList(trustedIssuers)); @@ -69,7 +69,7 @@ public final class JwtIssuerAuthenticationManagerResolver implements Authenticat /** * Construct a {@link JwtIssuerAuthenticationManagerResolver} using the provided parameters * - * @param trustedIssuers a whitelist of trusted issuers + * @param trustedIssuers a list of trusted issuers */ public JwtIssuerAuthenticationManagerResolver(Collection trustedIssuers) { Assert.notEmpty(trustedIssuers, "trustedIssuers cannot be empty"); @@ -82,7 +82,7 @@ public final class JwtIssuerAuthenticationManagerResolver implements Authenticat * Construct a {@link JwtIssuerAuthenticationManagerResolver} using the provided parameters * * Note that the {@link AuthenticationManagerResolver} provided in this constructor will need to - * verify that the issuer is trusted. This should be done via a whitelist. + * verify that the issuer is trusted. This should be done via an allowlist. * * One way to achieve this is with a {@link Map} where the keys are the known issuers: * 
@@ -93,7 +93,7 @@ public final class JwtIssuerAuthenticationManagerResolver implements Authenticat
 	 *     	(authenticationManagers::get);
 	 *
* - * The keys in the {@link Map} are the whitelist. + * The keys in the {@link Map} are the allowed issuers. * * @param issuerAuthenticationManagerResolver a strategy for resolving the {@link AuthenticationManager} by the issuer */ diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerReactiveAuthenticationManagerResolver.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerReactiveAuthenticationManagerResolver.java index c2f415378b..0328d5ae3b 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerReactiveAuthenticationManagerResolver.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtIssuerReactiveAuthenticationManagerResolver.java @@ -48,7 +48,7 @@ import org.springframework.web.server.ServerWebExchange; * * To use, this class must be able to determine whether or not the `iss` claim is trusted. Recall that * anyone can stand up an authorization server and issue valid tokens to a resource server. The simplest way - * to achieve this is to supply a whitelist of trusted issuers in the constructor. + * to achieve this is to supply a list of trusted issuers in the constructor. * * This class derives the Issuer from the `iss` claim found in the {@link ServerWebExchange}'s * Bearer Token. @@ -66,7 +66,7 @@ public final class JwtIssuerReactiveAuthenticationManagerResolver /** * Construct a {@link JwtIssuerReactiveAuthenticationManagerResolver} using the provided parameters * - * @param trustedIssuers a whitelist of trusted issuers + * @param trustedIssuers a list of trusted issuers */ public JwtIssuerReactiveAuthenticationManagerResolver(String... trustedIssuers) { this(Arrays.asList(trustedIssuers)); @@ -75,7 +75,7 @@ public final class JwtIssuerReactiveAuthenticationManagerResolver /** * Construct a {@link JwtIssuerReactiveAuthenticationManagerResolver} using the provided parameters * - * @param trustedIssuers a whitelist of trusted issuers + * @param trustedIssuers a collection of trusted issuers */ public JwtIssuerReactiveAuthenticationManagerResolver(Collection trustedIssuers) { Assert.notEmpty(trustedIssuers, "trustedIssuers cannot be empty"); @@ -87,7 +87,7 @@ public final class JwtIssuerReactiveAuthenticationManagerResolver * Construct a {@link JwtIssuerReactiveAuthenticationManagerResolver} using the provided parameters * * Note that the {@link ReactiveAuthenticationManagerResolver} provided in this constructor will need to - * verify that the issuer is trusted. This should be done via a whitelist. + * verify that the issuer is trusted. This should be done via an allowed list of issuers. * * One way to achieve this is with a {@link Map} where the keys are the known issuers: * 
@@ -98,7 +98,7 @@ public final class JwtIssuerReactiveAuthenticationManagerResolver
 	 *     	(issuer -> Mono.justOrEmpty(authenticationManagers.get(issuer));
 	 *
* - * The keys in the {@link Map} are the whitelist. + * The keys in the {@link Map} are the trusted issuers. * * @param issuerAuthenticationManagerResolver a strategy for resolving the {@link ReactiveAuthenticationManager} * by the issuer diff --git a/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java b/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java index 25dfe2b3a1..b7c752445b 100644 --- a/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java +++ b/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java @@ -358,7 +358,7 @@ public class StrictHttpFirewall implements HttpFirewall { if (!this.allowedHttpMethods.contains(request.getMethod())) { throw new RequestRejectedException("The request was rejected because the HTTP method \"" + request.getMethod() + - "\" was not included within the whitelist " + + "\" was not included within the list of allowed HTTP methods " + this.allowedHttpMethods); } }