From db70aa1a5fc8d1084b03a6e1004d378d272bede9 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Tue, 13 Aug 2024 14:51:11 -0500 Subject: [PATCH] Use Spring Boot PEM parser in SAML2 signing auto-configuration Closes gh-41567 --- ...RelyingPartyRegistrationConfiguration.java | 15 ++++++--- ...ml2RelyingPartyAutoConfigurationTests.java | 33 +++++++++++++++++++ .../boot/ssl/pem/PemContent.java | 8 ++++- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyRegistrationConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyRegistrationConfiguration.java index b9b9185e5b..aabed0af1b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyRegistrationConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyRegistrationConfiguration.java @@ -17,7 +17,7 @@ package org.springframework.boot.autoconfigure.security.saml2; import java.io.InputStream; -import java.security.cert.CertificateFactory; +import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.security.interfaces.RSAPrivateKey; import java.util.Collection; @@ -32,11 +32,11 @@ import org.springframework.boot.autoconfigure.security.saml2.Saml2RelyingPartyPr import org.springframework.boot.autoconfigure.security.saml2.Saml2RelyingPartyProperties.Registration; import org.springframework.boot.autoconfigure.security.saml2.Saml2RelyingPartyProperties.Registration.Signing; import org.springframework.boot.context.properties.PropertyMapper; +import org.springframework.boot.ssl.pem.PemContent; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; import org.springframework.core.io.Resource; -import org.springframework.security.converter.RsaKeyConverters; import org.springframework.security.saml2.core.Saml2X509Credential; import org.springframework.security.saml2.core.Saml2X509Credential.Saml2X509CredentialType; import org.springframework.security.saml2.provider.service.registration.AssertingPartyMetadata; @@ -57,6 +57,7 @@ import org.springframework.util.StringUtils; * @author Moritz Halbritter * @author Lasse Lindqvist * @author Lasse Wulff + * @author Scott Frederick */ @Configuration(proxyBeanMethods = false) @Conditional(RegistrationConfiguredCondition.class) @@ -172,7 +173,11 @@ class Saml2RelyingPartyRegistrationConfiguration { Assert.state(location != null, "No private key location specified"); Assert.state(location.exists(), () -> "Private key location '" + location + "' does not exist"); try (InputStream inputStream = location.getInputStream()) { - return RsaKeyConverters.pkcs8().convert(inputStream); + PemContent pemContent = PemContent.load(inputStream); + PrivateKey privateKey = pemContent.getPrivateKey(); + Assert.isInstanceOf(RSAPrivateKey.class, privateKey, + "PrivateKey in resource '" + location + "' must be an RSAPrivateKey"); + return (RSAPrivateKey) privateKey; } catch (Exception ex) { throw new IllegalArgumentException(ex); @@ -183,7 +188,9 @@ class Saml2RelyingPartyRegistrationConfiguration { Assert.state(location != null, "No certificate location specified"); Assert.state(location.exists(), () -> "Certificate location '" + location + "' does not exist"); try (InputStream inputStream = location.getInputStream()) { - return (X509Certificate) CertificateFactory.getInstance("X.509").generateCertificate(inputStream); + PemContent pemContent = PemContent.load(inputStream); + List certificates = pemContent.getCertificates(); + return certificates.get(0); } catch (Exception ex) { throw new IllegalArgumentException(ex); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyAutoConfigurationTests.java index 1f8bf1a81d..07aab477a6 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/saml2/Saml2RelyingPartyAutoConfigurationTests.java @@ -58,6 +58,7 @@ import static org.mockito.Mockito.mock; * @author Madhura Bhave * @author Moritz Halbritter * @author Lasse Lindqvist + * @author Scott Frederick */ class Saml2RelyingPartyAutoConfigurationTests { @@ -273,6 +274,38 @@ class Saml2RelyingPartyAutoConfigurationTests { } } + @Test + void autoconfigurationWithInvalidPrivateKeyShouldFail() { + this.contextRunner.withPropertyValues( + PREFIX + ".foo.signing.credentials[0].private-key-location=classpath:saml/certificate-location", + PREFIX + ".foo.signing.credentials[0].certificate-location=classpath:saml/certificate-location", + PREFIX + ".foo.assertingparty.singlesignon.url=https://simplesaml-for-spring-saml.cfapps.io/saml2/idp/SSOService.php", + PREFIX + ".foo.assertingparty.singlesignon.binding=post", + PREFIX + ".foo.assertingparty.singlesignon.sign-request=false", + PREFIX + ".foo.assertingparty.entity-id=https://simplesaml-for-spring-saml.cfapps.io/saml2/idp/metadata.php", + PREFIX + ".foo.assertingparty.verification.credentials[0].certificate-location=classpath:saml/certificate-location") + .run((context) -> assertThat(context).hasFailed() + .getFailure() + .rootCause() + .hasMessageContaining("Missing private key or unrecognized format")); + } + + @Test + void autoconfigurationWithInvalidCertificateShouldFail() { + this.contextRunner.withPropertyValues( + PREFIX + ".foo.signing.credentials[0].private-key-location=classpath:saml/private-key-location", + PREFIX + ".foo.signing.credentials[0].certificate-location=classpath:saml/private-key-location", + PREFIX + ".foo.assertingparty.singlesignon.url=https://simplesaml-for-spring-saml.cfapps.io/saml2/idp/SSOService.php", + PREFIX + ".foo.assertingparty.singlesignon.binding=post", + PREFIX + ".foo.assertingparty.singlesignon.sign-request=false", + PREFIX + ".foo.assertingparty.entity-id=https://simplesaml-for-spring-saml.cfapps.io/saml2/idp/metadata.php", + PREFIX + ".foo.assertingparty.verification.credentials[0].certificate-location=classpath:saml/certificate-location") + .run((context) -> assertThat(context).hasFailed() + .getFailure() + .rootCause() + .hasMessageContaining("Missing certificates or unrecognized format")); + } + private void testMultipleProviders(String specifiedEntityId, String expected) throws Exception { try (MockWebServer server = new MockWebServer()) { server.start(); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/PemContent.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/PemContent.java index 3a7e08e43d..ca69817b04 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/PemContent.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/PemContent.java @@ -140,7 +140,13 @@ public final class PemContent { } } - private static PemContent load(InputStream in) throws IOException { + /** + * Load {@link PemContent} from the given {@link InputStream}. + * @param in an input stream to load the content from + * @return the loaded PEM content + * @throws IOException on IO error + */ + public static PemContent load(InputStream in) throws IOException { return of(StreamUtils.copyToString(in, StandardCharsets.UTF_8)); }