From 97a49aa3bf2380208ff0f1d96806b29fa2934327 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Wed, 29 Apr 2026 08:51:19 -0600 Subject: [PATCH] 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> --- ...ing-security-saml2-service-provider.gradle | 12 +- .../JdbcAssertingPartyMetadataRepository.java | 156 +++++++++++++++++- ...tyMetadataRepositoryBouncyCastleTests.java | 142 ++++++++++++++++ ...AssertingPartyMetadataRepositoryTests.java | 43 +++++ 4 files changed, 343 insertions(+), 10 deletions(-) create mode 100644 saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryBouncyCastleTests.java diff --git a/saml2/saml2-service-provider/spring-security-saml2-service-provider.gradle b/saml2/saml2-service-provider/spring-security-saml2-service-provider.gradle index 140f8a91e7..228548d34f 100644 --- a/saml2/saml2-service-provider/spring-security-saml2-service-provider.gradle +++ b/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 } diff --git a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepository.java b/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepository.java index 23d9c24881..11221a013a 100644 --- a/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepository.java +++ b/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 assertingPartyMetadataRowMapper = new AssertingPartyMetadataRowMapper(); + private RowMapper 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. + * + *

+ * 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 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 { + public static final class AssertingPartyMetadataRowMapper implements RowMapper { - private final Deserializer deserializer = new DefaultDeserializer(); + private Deserializer> deserializer = new Saml2X509CredentialCollectionDeserializer(); @Override public AssertingPartyMetadata mapRow(ResultSet rs, int rowNum) throws SQLException { @@ -162,8 +194,7 @@ public final class JdbcAssertingPartyMetadataRepository implements AssertingPart List algorithms = List.of(rs.getString(COLUMN_NAMES[4]).split(",")); byte[] verificationCredentialsBytes = rs.getBytes(COLUMN_NAMES[5]); byte[] encryptionCredentialsBytes = rs.getBytes(COLUMN_NAMES[6]); - ThrowingFunction> credentials = ( - bytes) -> (Collection) this.deserializer.deserializeFromByteArray(bytes); + ThrowingFunction> credentials = this.deserializer::deserializeFromByteArray; AssertingPartyMetadata.Builder builder = new AssertingPartyDetails.Builder(); Collection verificationCredentials = credentials.apply(verificationCredentialsBytes); Collection 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. + * + *

+ * 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> deserializer) { + Assert.notNull(deserializer, "deserializer cannot be null"); + this.deserializer = deserializer; + } + + /** + * The default deserializer for verification and encryption credentials. + * + *

+ * 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> { + + private static final AllowlistObjectInputFilter ALLOWLIST; + + static { + Set 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 deserialize(InputStream in) throws IOException { + ObjectInputStream oin = new ObjectInputStream(in); + oin.setObjectInputFilter(ALLOWLIST); + try { + Collection credentials = (Collection) 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 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> { - private final Serializer serializer = new DefaultSerializer(); + private Serializer serializer = new DefaultSerializer(); @Override public List apply(AssertingPartyMetadata record) { diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryBouncyCastleTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryBouncyCastleTests.java new file mode 100644 index 0000000000..6600aad224 --- /dev/null +++ b/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()); + } + +} diff --git a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryTests.java b/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryTests.java index 5855462454..7d7a0654ba 100644 --- a/saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/registration/JdbcAssertingPartyMetadataRepositoryTests.java +++ b/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 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); }