Add setRowMapper

This commit makes so that the strategy for reading an
AssertingPartyMetadata can be configured.

Signed-off-by: Josh Cummings <3627351+jzheaux@users.noreply.github.com>

saml2/saml2-service-provider/spring-security-saml2-service-provider.gradle

@@ -137,6 +137,14 @@ tasks.register("opensaml5Test", Test) {
 	classpath = sourceSets.opensaml5Test.output + sourceSets.opensaml5Test.runtimeClasspath
 }
 
-tasks.named("test") {
-	dependsOn opensaml5Test
+tasks.register("bouncyCastleTest", Test) {
+	useJUnitPlatform()
+	testClassesDirs = sourceSets.test.output.classesDirs
+	classpath = sourceSets.test.runtimeClasspath
+	include "**/JdbcAssertingPartyMetadataRepositoryBouncyCastleTests.class"
+}
+
+tasks.named("test") {
+	exclude "**/JdbcAssertingPartyMetadataRepositoryBouncyCastleTests.class"
+	dependsOn opensaml5Test, bouncyCastleTest
 }

saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepository.java

@@ -16,6 +16,11 @@
 
 package org.springframework.security.saml2.provider.service.registration;
 
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.ObjectInputFilter;
+import java.io.ObjectInputStream;
+import java.security.Security;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Types;
@@ -23,11 +28,16 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.function.Function;
 
-import org.springframework.core.serializer.DefaultDeserializer;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 import org.springframework.core.serializer.DefaultSerializer;
 import org.springframework.core.serializer.Deserializer;
 import org.springframework.core.serializer.Serializer;
@@ -37,6 +47,7 @@ import org.springframework.jdbc.core.PreparedStatementSetter;
 import org.springframework.jdbc.core.RowMapper;
 import org.springframework.jdbc.core.SqlParameterValue;
 import org.springframework.security.saml2.core.Saml2X509Credential;
+import org.springframework.security.saml2.provider.service.registration.JdbcAssertingPartyMetadataRepository.AssertingPartyMetadataRowMapper.Saml2X509CredentialCollectionDeserializer;
 import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration.AssertingPartyDetails;
 import org.springframework.util.Assert;
 import org.springframework.util.function.ThrowingFunction;
@@ -51,7 +62,7 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart
 
 	private final JdbcOperations jdbcOperations;
 
-	private final RowMapper<AssertingPartyMetadata> assertingPartyMetadataRowMapper = new AssertingPartyMetadataRowMapper();
+	private RowMapper<AssertingPartyMetadata> assertingPartyMetadataRowMapper = new AssertingPartyMetadataRowMapper();
 
 	private final AssertingPartyMetadataParametersMapper assertingPartyMetadataParametersMapper = new AssertingPartyMetadataParametersMapper();
 
@@ -145,13 +156,34 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart
 		return this.jdbcOperations.update(UPDATE_CREDENTIAL_RECORD_SQL, parameters.toArray());
 	}
 
+	/**
+	 * Set the {@link RowMapper} to use for reading {@link AssertingPartyMetadata} records
+	 * from the database. By default, {@link AssertingPartyMetadataRowMapper} is used.
+	 *
+	 * <p>
+	 * Note that the default row mapper protects against insecure deserialization of
+	 * credentials by way of {@link Saml2X509CredentialCollectionDeserializer}, which uses
+	 * an allowlist of classes that can be deserialized. If a custom row mapper is used,
+	 * it is the responsibility of the developer to ensure that any deserialization of
+	 * credentials is done securely.
+	 * @param rowMapper - the rowMapper to use
+	 * @since 7.0.5
+	 * @see AssertingPartyMetadataRowMapper
+	 */
+	public void setRowMapper(RowMapper<AssertingPartyMetadata> rowMapper) {
+		Assert.notNull(rowMapper, "rowMapper cannot be null");
+		this.assertingPartyMetadataRowMapper = rowMapper;
+	}
+
 	/**
 	 * The default {@link RowMapper} that maps the current row in
 	 * {@code java.sql.ResultSet} to {@link AssertingPartyMetadata}.
+	 *
+	 * @since 7.0.5
 	 */
-	private static final class AssertingPartyMetadataRowMapper implements RowMapper<AssertingPartyMetadata> {
+	public static final class AssertingPartyMetadataRowMapper implements RowMapper<AssertingPartyMetadata> {
 
-		private final Deserializer<Object> deserializer = new DefaultDeserializer();
+		private Deserializer<Collection<Saml2X509Credential>> deserializer = new Saml2X509CredentialCollectionDeserializer();
 
 		@Override
 		public AssertingPartyMetadata mapRow(ResultSet rs, int rowNum) throws SQLException {
@@ -162,8 +194,7 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart
 			List<String> algorithms = List.of(rs.getString(COLUMN_NAMES[4]).split(","));
 			byte[] verificationCredentialsBytes = rs.getBytes(COLUMN_NAMES[5]);
 			byte[] encryptionCredentialsBytes = rs.getBytes(COLUMN_NAMES[6]);
-			ThrowingFunction<byte[], Collection<Saml2X509Credential>> credentials = (
-					bytes) -> (Collection<Saml2X509Credential>) this.deserializer.deserializeFromByteArray(bytes);
+			ThrowingFunction<byte[], Collection<Saml2X509Credential>> credentials = this.deserializer::deserializeFromByteArray;
 			AssertingPartyMetadata.Builder<?> builder = new AssertingPartyDetails.Builder();
 			Collection<Saml2X509Credential> verificationCredentials = credentials.apply(verificationCredentialsBytes);
 			Collection<Saml2X509Credential> encryptionCredentials = (encryptionCredentialsBytes != null)
@@ -185,12 +216,121 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart
 			return builder.build();
 		}
 
+		/**
+		 * Set the {@link Deserializer} to use for reading the credential collection from
+		 * the database. By default, {@link Saml2X509CredentialCollectionDeserializer} is
+		 * used, which uses Java Object serialization.
+		 *
+		 * <p>
+		 * If a custom deserializer is used, it is the responsibility of the developer to
+		 * ensure that any deserialization of credentials is done securely.
+		 * @param deserializer the deserializer to use
+		 */
+		public void setCredentialsDeserializer(Deserializer<Collection<Saml2X509Credential>> deserializer) {
+			Assert.notNull(deserializer, "deserializer cannot be null");
+			this.deserializer = deserializer;
+		}
+
+		/**
+		 * The default deserializer for verification and encryption credentials.
+		 *
+		 * <p>
+		 * This is equipped with an allowlist of classes that can be deserialized. If you
+		 * implement your own, you are responsible for to protecte against insecure
+		 * deserialization.
+		 *
+		 * @since 7.0.5
+		 */
+		public static final class Saml2X509CredentialCollectionDeserializer
+				implements Deserializer<Collection<Saml2X509Credential>> {
+
+			private static final AllowlistObjectInputFilter ALLOWLIST;
+
+			static {
+				Set<String> classes = new LinkedHashSet<>();
+				classes.add(Saml2X509Credential.class.getName());
+				classes.add(Saml2X509Credential.Saml2X509CredentialType.class.getName());
+				classes.add(Enum.class.getName());
+				classes.add("java.security.cert.Certificate$CertificateRep");
+				classes.add("sun.security.x509.X509CertImpl");
+				classes.add(LinkedHashSet.class.getName());
+				classes.add(HashSet.class.getName());
+				classes.add("java.util.Map$Entry");
+				if (Security.getProvider("BC") != null) {
+					classes.add("org.bouncycastle.jcajce.provider.asymmetric.x509.X509CertificateObject");
+				}
+				ALLOWLIST = new AllowlistObjectInputFilter(classes);
+			}
+
+			@Override
+			@SuppressWarnings("unchecked")
+			public Collection<Saml2X509Credential> deserialize(InputStream in) throws IOException {
+				ObjectInputStream oin = new ObjectInputStream(in);
+				oin.setObjectInputFilter(ALLOWLIST);
+				try {
+					Collection<Saml2X509Credential> credentials = (Collection<Saml2X509Credential>) oin.readObject();
+					for (Object credential : credentials) {
+						Assert.isInstanceOf(Saml2X509Credential.class, credential,
+								"Deserialized object is not of type Saml2X509Credential");
+					}
+					return credentials;
+				}
+				catch (ClassNotFoundException ex) {
+					throw new IOException("Failed to deserialize asserting party credential collection", ex);
+				}
+			}
+
+			private static final class AllowlistObjectInputFilter implements ObjectInputFilter {
+
+				private static final Log logger = LogFactory.getLog(JdbcAssertingPartyMetadataRepository.class);
+
+				private static final int MAX_DEPTH = 20;
+
+				private static final int MAX_REFS = 1000;
+
+				private static final int MAX_ARRAY = 16384;
+
+				private static final int MAX_BYTES = 1_048_576;
+
+				private final ObjectInputFilter delegate;
+
+				private AllowlistObjectInputFilter(Set<String> allowlist) {
+					ObjectInputFilter pattern = ObjectInputFilter.Config
+						.createFilter(String.join(";", allowlist) + ";!*" + ";maxdepth=" + MAX_DEPTH + ";maxrefs="
+								+ MAX_REFS + ";maxarray=" + MAX_ARRAY + ";maxbytes=" + MAX_BYTES);
+					ObjectInputFilter global = ObjectInputFilter.Config.getSerialFilter();
+					this.delegate = (global != null) ? ObjectInputFilter.merge(pattern, global) : pattern;
+				}
+
+				@Override
+				public Status checkInput(FilterInfo info) {
+					Status status = this.delegate.checkInput(info);
+					if (status != Status.REJECTED) {
+						return status;
+					}
+					Class<?> c = info.serialClass();
+					if (c != null) {
+						logger.trace("Failed to deserialize due to [" + c.getName() + "] not being in the allowlist");
+					}
+					else {
+						logger.trace("Failed to deserialize due to exceeding one the following limits: " + "depth=["
+								+ info.depth() + " < " + MAX_DEPTH + "]" + ", refs=[" + info.references() + " < "
+								+ MAX_REFS + "]" + ", bytes=[" + info.streamBytes() + " < " + MAX_BYTES + "]"
+								+ ", arrayLength=[" + info.arrayLength() + " < " + MAX_ARRAY + "]");
+					}
+					return Status.REJECTED;
+				}
+
+			}
+
+		}
+
 	}
 
-	private static class AssertingPartyMetadataParametersMapper
+	private class AssertingPartyMetadataParametersMapper
 			implements Function<AssertingPartyMetadata, List<SqlParameterValue>> {
 
-		private final Serializer<Object> serializer = new DefaultSerializer();
+		private Serializer<Object> serializer = new DefaultSerializer();
 
 		@Override
 		public List<SqlParameterValue> apply(AssertingPartyMetadata record) {

saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryBouncyCastleTests.java

@@ -0,0 +1,142 @@
+/*
+ * Copyright 2004-present the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.security.saml2.provider.service.registration;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InvalidClassException;
+import java.io.ObjectOutputStream;
+import java.security.Security;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.jdbc.core.JdbcOperations;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+
+/**
+ * Tests for {@link JdbcAssertingPartyMetadataRepository} when Bouncy Castle is the
+ * preferred JCA provider. Run in their own forked JVM (see the {@code bouncyCastleTest}
+ * Gradle task) so that the production code's allowlist is computed with BC registered and
+ * so this provider change does not leak into other tests.
+ */
+class JdbcAssertingPartyMetadataRepositoryBouncyCastleTests {
+
+	private static final String SCHEMA_SQL_RESOURCE = "org/springframework/security/saml2/saml2-asserting-party-metadata-schema.sql";
+
+	static {
+		Security.insertProviderAt(new BouncyCastleProvider(), 1);
+	}
+
+	private EmbeddedDatabase db;
+
+	private JdbcAssertingPartyMetadataRepository repository;
+
+	private JdbcOperations jdbcOperations;
+
+	private final AssertingPartyMetadata metadata = TestRelyingPartyRegistrations.full()
+		.build()
+		.getAssertingPartyMetadata();
+
+	@BeforeEach
+	void setUp() {
+		this.db = createDb();
+		this.jdbcOperations = new JdbcTemplate(this.db);
+		this.repository = new JdbcAssertingPartyMetadataRepository(this.jdbcOperations);
+	}
+
+	@AfterEach
+	void tearDown() {
+		this.db.shutdown();
+	}
+
+	@Test
+	void findByEntityIdWhenEntityPresentThenReturns() {
+		this.repository.save(this.metadata);
+
+		AssertingPartyMetadata found = this.repository.findByEntityId(this.metadata.getEntityId());
+
+		assertAssertingPartyEquals(found, this.metadata);
+	}
+
+	@Test
+	void saveWhenExistingThenUpdates() {
+		this.repository.save(this.metadata);
+		boolean existing = this.metadata.getWantAuthnRequestsSigned();
+		this.repository.save(this.metadata.mutate().wantAuthnRequestsSigned(!existing).build());
+		boolean updated = this.repository.findByEntityId(this.metadata.getEntityId()).getWantAuthnRequestsSigned();
+		assertThat(existing).isNotEqualTo(updated);
+	}
+
+	@Test
+	void findByEntityIdWhenSerializedTypeNotInAllowlistThenFailsDeserialization() throws Exception {
+		this.repository.save(this.metadata);
+		byte[] notAllowed = serialize(new HashMap<>(Map.of("not", "allowed")));
+		this.jdbcOperations.update(
+				"UPDATE saml2_asserting_party_metadata SET verification_credentials = ? WHERE entity_id = ?",
+				notAllowed, this.metadata.getEntityId());
+
+		assertThatExceptionOfType(RuntimeException.class)
+			.isThrownBy(() -> this.repository.findByEntityId(this.metadata.getEntityId()))
+			.withRootCauseInstanceOf(InvalidClassException.class);
+	}
+
+	private static byte[] serialize(Object value) throws IOException {
+		ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+		try (ObjectOutputStream oos = new ObjectOutputStream(bytes)) {
+			oos.writeObject(value);
+		}
+		return bytes.toByteArray();
+	}
+
+	private static EmbeddedDatabase createDb() {
+		// @formatter:off
+		return new EmbeddedDatabaseBuilder()
+				.generateUniqueName(true)
+				.setType(EmbeddedDatabaseType.HSQL)
+				.setScriptEncoding("UTF-8")
+				.addScript(SCHEMA_SQL_RESOURCE)
+				.build();
+		// @formatter:on
+	}
+
+	private void assertAssertingPartyEquals(AssertingPartyMetadata found, AssertingPartyMetadata expected) {
+		assertThat(found).isNotNull();
+		assertThat(found.getEntityId()).isEqualTo(expected.getEntityId());
+		assertThat(found.getSingleSignOnServiceLocation()).isEqualTo(expected.getSingleSignOnServiceLocation());
+		assertThat(found.getSingleSignOnServiceBinding()).isEqualTo(expected.getSingleSignOnServiceBinding());
+		assertThat(found.getWantAuthnRequestsSigned()).isEqualTo(expected.getWantAuthnRequestsSigned());
+		assertThat(found.getSingleLogoutServiceLocation()).isEqualTo(expected.getSingleLogoutServiceLocation());
+		assertThat(found.getSingleLogoutServiceResponseLocation())
+			.isEqualTo(expected.getSingleLogoutServiceResponseLocation());
+		assertThat(found.getSingleLogoutServiceBinding()).isEqualTo(expected.getSingleLogoutServiceBinding());
+		assertThat(found.getSigningAlgorithms()).containsAll(expected.getSigningAlgorithms());
+		assertThat(found.getVerificationX509Credentials()).containsAll(expected.getVerificationX509Credentials());
+		assertThat(found.getEncryptionX509Credentials()).containsAll(expected.getEncryptionX509Credentials());
+	}
+
+}

saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryTests.java

@@ -16,7 +16,13 @@
 
 package org.springframework.security.saml2.provider.service.registration;
 
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InvalidClassException;
+import java.io.ObjectOutputStream;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.Map;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -24,12 +30,19 @@ import org.junit.jupiter.api.Test;
 
 import org.springframework.jdbc.core.JdbcOperations;
 import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.jdbc.core.RowMapper;
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType;
+import org.springframework.security.saml2.provider.service.registration.JdbcAssertingPartyMetadataRepository.AssertingPartyMetadataRowMapper;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 
 /**
  * Tests for {@link JdbcAssertingPartyMetadataRepository}
@@ -115,6 +128,36 @@ class JdbcAssertingPartyMetadataRepositoryTests {
 		assertThat(existing).isNotEqualTo(updated);
 	}
 
+	@Test
+	void saveWhenCustomRowMapperThenUses() throws Exception {
+		RowMapper<AssertingPartyMetadata> rowMapper = spy(new AssertingPartyMetadataRowMapper());
+		this.repository.setRowMapper(rowMapper);
+		this.repository.save(this.metadata);
+		this.repository.findByEntityId(this.metadata.getEntityId());
+		verify(rowMapper).mapRow(any(), eq(0));
+	}
+
+	@Test
+	void findByEntityIdWhenSerializedTypeNotInAllowlistThenFailsDeserialization() throws Exception {
+		this.repository.save(this.metadata);
+		byte[] notAllowed = serialize(new HashMap<>(Map.of("not", "allowed")));
+		this.jdbcOperations.update(
+				"UPDATE saml2_asserting_party_metadata SET verification_credentials = ? WHERE entity_id = ?",
+				notAllowed, this.metadata.getEntityId());
+
+		assertThatExceptionOfType(RuntimeException.class)
+			.isThrownBy(() -> this.repository.findByEntityId(this.metadata.getEntityId()))
+			.withRootCauseInstanceOf(InvalidClassException.class);
+	}
+
+	private static byte[] serialize(Object value) throws IOException {
+		ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+		try (ObjectOutputStream oos = new ObjectOutputStream(bytes)) {
+			oos.writeObject(value);
+		}
+		return bytes.toByteArray();
+	}
+
 	private static EmbeddedDatabase createDb() {
 		return createDb(SCHEMA_SQL_RESOURCE);
 	}