From a89ae3fbee647a8d83578c2317544c12a3391869 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 4 Sep 2024 11:30:28 +0100 Subject: [PATCH] Improve laziness of Pem and JKS SSL store bundles Fixes gh-42119 --- .../boot/ssl/jks/JksSslStoreBundle.java | 20 +++++---- .../boot/ssl/pem/LoadedPemSslStore.java | 12 +++++- .../boot/ssl/pem/PemSslStoreBundle.java | 29 +++++++------ .../boot/ssl/jks/JksSslStoreBundleTests.java | 42 ++++++++++--------- .../boot/ssl/pem/LoadedPemSslStoreTests.java | 18 +++++++- .../boot/ssl/pem/PemSslStoreBundleTests.java | 18 +++++++- 6 files changed, 97 insertions(+), 42 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/jks/JksSslStoreBundle.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/jks/JksSslStoreBundle.java index 8f475f1c8337..adcffa654e63 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/jks/JksSslStoreBundle.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/jks/JksSslStoreBundle.java @@ -24,12 +24,14 @@ import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; import java.security.cert.CertificateException; +import java.util.function.Supplier; import org.springframework.boot.ssl.SslStoreBundle; import org.springframework.core.style.ToStringCreator; import org.springframework.util.Assert; import org.springframework.util.ResourceUtils; import org.springframework.util.StringUtils; +import org.springframework.util.function.SingletonSupplier; /** * {@link SslStoreBundle} backed by a Java keystore. @@ -43,9 +45,9 @@ public class JksSslStoreBundle implements SslStoreBundle { private final JksSslStoreDetails keyStoreDetails; - private final KeyStore keyStore; + private final Supplier keyStore; - private final KeyStore trustStore; + private final Supplier trustStore; /** * Create a new {@link JksSslStoreBundle} instance. @@ -54,13 +56,13 @@ public class JksSslStoreBundle implements SslStoreBundle { */ public JksSslStoreBundle(JksSslStoreDetails keyStoreDetails, JksSslStoreDetails trustStoreDetails) { this.keyStoreDetails = keyStoreDetails; - this.keyStore = createKeyStore("key", this.keyStoreDetails); - this.trustStore = createKeyStore("trust", trustStoreDetails); + this.keyStore = SingletonSupplier.of(() -> createKeyStore("key", this.keyStoreDetails)); + this.trustStore = SingletonSupplier.of(() -> createKeyStore("trust", trustStoreDetails)); } @Override public KeyStore getKeyStore() { - return this.keyStore; + return this.keyStore.get(); } @Override @@ -70,7 +72,7 @@ public String getKeyStorePassword() { @Override public KeyStore getTrustStore() { - return this.trustStore; + return this.trustStore.get(); } private KeyStore createKeyStore(String name, JksSslStoreDetails details) { @@ -127,10 +129,12 @@ private void loadKeyStore(KeyStore store, String location, char[] password) { @Override public String toString() { ToStringCreator creator = new ToStringCreator(this); - creator.append("keyStore.type", (this.keyStore != null) ? this.keyStore.getType() : "none"); + KeyStore keyStore = this.keyStore.get(); + creator.append("keyStore.type", (keyStore != null) ? keyStore.getType() : "none"); String keyStorePassword = getKeyStorePassword(); creator.append("keyStorePassword", (keyStorePassword != null) ? "******" : null); - creator.append("trustStore.type", (this.trustStore != null) ? this.trustStore.getType() : "none"); + KeyStore trustStore = this.trustStore.get(); + creator.append("trustStore.type", (trustStore != null) ? trustStore.getType() : "none"); return creator.toString(); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/LoadedPemSslStore.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/LoadedPemSslStore.java index 5edacd360e61..5f001421f57b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/LoadedPemSslStore.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/LoadedPemSslStore.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -97,4 +97,14 @@ public PrivateKey privateKey() { return this.privateKeySupplier.get(); } + @Override + public PemSslStore withAlias(String alias) { + return new LoadedPemSslStore(this.details.withAlias(alias)); + } + + @Override + public PemSslStore withPassword(String password) { + return new LoadedPemSslStore(this.details.withPassword(password)); + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/PemSslStoreBundle.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/PemSslStoreBundle.java index f8f5eda84ef5..76f8731f8a8e 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/PemSslStoreBundle.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/pem/PemSslStoreBundle.java @@ -24,11 +24,13 @@ import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.util.List; +import java.util.function.Supplier; import org.springframework.boot.ssl.SslStoreBundle; import org.springframework.core.style.ToStringCreator; import org.springframework.util.Assert; import org.springframework.util.StringUtils; +import org.springframework.util.function.SingletonSupplier; /** * {@link SslStoreBundle} backed by PEM-encoded certificates and private keys. @@ -42,9 +44,9 @@ public class PemSslStoreBundle implements SslStoreBundle { private static final String DEFAULT_ALIAS = "ssl"; - private final KeyStore keyStore; + private final Supplier keyStore; - private final KeyStore trustStore; + private final Supplier trustStore; /** * Create a new {@link PemSslStoreBundle} instance. @@ -66,8 +68,9 @@ public PemSslStoreBundle(PemSslStoreDetails keyStoreDetails, PemSslStoreDetails */ @Deprecated(since = "3.2.0", forRemoval = true) public PemSslStoreBundle(PemSslStoreDetails keyStoreDetails, PemSslStoreDetails trustStoreDetails, String alias) { - this.keyStore = createKeyStore("key", PemSslStore.load(keyStoreDetails), alias); - this.trustStore = createKeyStore("trust", PemSslStore.load(trustStoreDetails), alias); + this.keyStore = SingletonSupplier.of(() -> createKeyStore("key", PemSslStore.load(keyStoreDetails), alias)); + this.trustStore = SingletonSupplier + .of(() -> createKeyStore("trust", PemSslStore.load(trustStoreDetails), alias)); } /** @@ -81,13 +84,13 @@ public PemSslStoreBundle(PemSslStore pemKeyStore, PemSslStore pemTrustStore) { } private PemSslStoreBundle(PemSslStore pemKeyStore, PemSslStore pemTrustStore, String alias) { - this.keyStore = createKeyStore("key", pemKeyStore, alias); - this.trustStore = createKeyStore("trust", pemTrustStore, alias); + this.keyStore = SingletonSupplier.of(() -> createKeyStore("key", pemKeyStore, alias)); + this.trustStore = SingletonSupplier.of(() -> createKeyStore("trust", pemTrustStore, alias)); } @Override public KeyStore getKeyStore() { - return this.keyStore; + return this.keyStore.get(); } @Override @@ -97,7 +100,7 @@ public String getKeyStorePassword() { @Override public KeyStore getTrustStore() { - return this.trustStore; + return this.trustStore.get(); } private static KeyStore createKeyStore(String name, PemSslStore pemSslStore, String alias) { @@ -105,11 +108,11 @@ private static KeyStore createKeyStore(String name, PemSslStore pemSslStore, Str return null; } try { - Assert.notEmpty(pemSslStore.certificates(), "Certificates must not be empty"); + List certificates = pemSslStore.certificates(); + Assert.notEmpty(certificates, "Certificates must not be empty"); alias = (pemSslStore.alias() != null) ? pemSslStore.alias() : alias; alias = (alias != null) ? alias : DEFAULT_ALIAS; KeyStore store = createKeyStore(pemSslStore.type()); - List certificates = pemSslStore.certificates(); PrivateKey privateKey = pemSslStore.privateKey(); if (privateKey != null) { addPrivateKey(store, privateKey, alias, pemSslStore.password(), certificates); @@ -149,9 +152,11 @@ private static void addCertificates(KeyStore keyStore, List cer @Override public String toString() { ToStringCreator creator = new ToStringCreator(this); - creator.append("keyStore.type", (this.keyStore != null) ? this.keyStore.getType() : "none"); + KeyStore keyStore = this.keyStore.get(); + KeyStore trustStore = this.trustStore.get(); + creator.append("keyStore.type", (keyStore != null) ? keyStore.getType() : "none"); creator.append("keyStorePassword", null); - creator.append("trustStore.type", (this.trustStore != null) ? this.trustStore.getType() : "none"); + creator.append("trustStore.type", (trustStore != null) ? trustStore.getType() : "none"); return creator.toString(); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/jks/JksSslStoreBundleTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/jks/JksSslStoreBundleTests.java index 159e98c45654..bb57ec607cb7 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/jks/JksSslStoreBundleTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/jks/JksSslStoreBundleTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -58,12 +58,10 @@ void whenStoresHaveNoValues() { } @Test - void whenTypePKCS11AndLocationThrowsException() { - assertThatIllegalStateException().isThrownBy(() -> { - JksSslStoreDetails keyStoreDetails = new JksSslStoreDetails("PKCS11", null, "test.jks", null); - JksSslStoreDetails trustStoreDetails = null; - new JksSslStoreBundle(keyStoreDetails, trustStoreDetails); - }) + void whenTypePKCS11AndLocationGetKeyStoreThrowsException() { + JksSslStoreDetails keyStoreDetails = new JksSslStoreDetails("PKCS11", null, "test.jks", null); + JksSslStoreBundle jksSslStoreBundle = new JksSslStoreBundle(keyStoreDetails, null); + assertThatIllegalStateException().isThrownBy(jksSslStoreBundle::getKeyStore) .withMessageContaining( "Unable to create key store: Location is 'test.jks', but must be empty or null for PKCS11 hardware key stores"); } @@ -104,22 +102,28 @@ void whenHasTrustStoreType() { @Test void whenHasKeyStoreProvider() { - assertThatIllegalStateException().isThrownBy(() -> { - JksSslStoreDetails keyStoreDetails = new JksSslStoreDetails(null, "com.example.KeyStoreProvider", - "classpath:test.jks", "secret"); - JksSslStoreDetails trustStoreDetails = null; - new JksSslStoreBundle(keyStoreDetails, trustStoreDetails); - }).withMessageContaining("com.example.KeyStoreProvider"); + JksSslStoreDetails keyStoreDetails = new JksSslStoreDetails(null, "com.example.KeyStoreProvider", + "classpath:test.jks", "secret"); + JksSslStoreBundle jksSslStoreBundle = new JksSslStoreBundle(keyStoreDetails, null); + assertThatIllegalStateException().isThrownBy(jksSslStoreBundle::getKeyStore) + .withMessageContaining("com.example.KeyStoreProvider"); } @Test void whenHasTrustStoreProvider() { - assertThatIllegalStateException().isThrownBy(() -> { - JksSslStoreDetails keyStoreDetails = null; - JksSslStoreDetails trustStoreDetails = new JksSslStoreDetails(null, "com.example.KeyStoreProvider", - "classpath:test.jks", "secret"); - new JksSslStoreBundle(keyStoreDetails, trustStoreDetails); - }).withMessageContaining("com.example.KeyStoreProvider"); + JksSslStoreDetails trustStoreDetails = new JksSslStoreDetails(null, "com.example.KeyStoreProvider", + "classpath:test.jks", "secret"); + JksSslStoreBundle jksSslStoreBundle = new JksSslStoreBundle(null, trustStoreDetails); + assertThatIllegalStateException().isThrownBy(jksSslStoreBundle::getTrustStore) + .withMessageContaining("com.example.KeyStoreProvider"); + } + + @Test + void storeCreationIsLazy() { + JksSslStoreDetails details = new JksSslStoreDetails(null, null, "does-not-exist", null); + JksSslStoreBundle bundle = new JksSslStoreBundle(details, details); + assertThatIllegalStateException().isThrownBy(bundle::getKeyStore); + assertThatIllegalStateException().isThrownBy(bundle::getTrustStore); } private Consumer storeContainingCertAndKey(String keyAlias, String keyPassword) { diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/pem/LoadedPemSslStoreTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/pem/LoadedPemSslStoreTests.java index b7bccce838a5..f78b777921cf 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/pem/LoadedPemSslStoreTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/pem/LoadedPemSslStoreTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -45,4 +45,20 @@ void privateKeyIsLoadedLazily() { assertThatExceptionOfType(UncheckedIOException.class).isThrownBy(store::privateKey); } + @Test + void withAliasIsLazy() { + PemSslStoreDetails details = PemSslStoreDetails.forCertificate("classpath:missing-test-cert.pem") + .withPrivateKey("classpath:test-key.pem"); + PemSslStore store = new LoadedPemSslStore(details).withAlias("alias"); + assertThatExceptionOfType(UncheckedIOException.class).isThrownBy(store::certificates); + } + + @Test + void withPasswordIsLazy() { + PemSslStoreDetails details = PemSslStoreDetails.forCertificate("classpath:missing-test-cert.pem") + .withPrivateKey("classpath:test-key.pem"); + PemSslStore store = new LoadedPemSslStore(details).withPassword("password"); + assertThatExceptionOfType(UncheckedIOException.class).isThrownBy(store::certificates); + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/pem/PemSslStoreBundleTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/pem/PemSslStoreBundleTests.java index b34901931062..fd78a1e711e6 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/pem/PemSslStoreBundleTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/pem/PemSslStoreBundleTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -27,6 +27,10 @@ import org.springframework.util.function.ThrowingConsumer; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.then; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; /** * Tests for {@link PemSslStoreBundle}. @@ -215,6 +219,18 @@ void createWithPemSslStoreCreatesInstance() { assertThat(bundle.getTrustStore()).satisfies(storeContainingCertAndKey("ssl")); } + @Test + void storeCreationIsLazy() { + PemSslStore pemSslStore = mock(PemSslStore.class); + PemSslStoreBundle bundle = new PemSslStoreBundle(pemSslStore, pemSslStore); + given(pemSslStore.certificates()).willReturn(PemContent.of(CERTIFICATE).getCertificates()); + then(pemSslStore).shouldHaveNoInteractions(); + bundle.getKeyStore(); + then(pemSslStore).should().certificates(); + bundle.getTrustStore(); + then(pemSslStore).should(times(2)).certificates(); + } + private Consumer storeContainingCert(String keyAlias) { return storeContainingCert(KeyStore.getDefaultType(), keyAlias); }