From 890b4ede7a2be1e00b48a9e902156425c01958bf Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 19 Aug 2024 15:08:18 -0700 Subject: [PATCH] Further polish SslInfoContributor and SslHealthIndicator See gh-41205 --- ...althContributorAutoConfigurationTests.java | 18 +- .../boot/actuate/ssl/SslHealthIndicator.java | 61 +++-- .../actuate/ssl/SslHealthIndicatorTests.java | 56 ++-- .../springframework/boot/info/SslInfo.java | 247 ++++++++++-------- .../boot/ssl/DefaultSslBundleRegistry.java | 16 +- .../springframework/boot/ssl/SslBundles.java | 16 +- .../boot/info/SslInfoTests.java | 37 ++- .../ssl/DefaultSslBundleRegistryTests.java | 6 +- 8 files changed, 237 insertions(+), 220 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/ssl/SslHealthContributorAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/ssl/SslHealthContributorAutoConfigurationTests.java index 4895cb3e1b..d122d5041a 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/ssl/SslHealthContributorAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/ssl/SslHealthContributorAutoConfigurationTests.java @@ -29,7 +29,7 @@ import org.springframework.boot.actuate.ssl.SslHealthIndicator; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.ssl.SslAutoConfiguration; import org.springframework.boot.info.SslInfo; -import org.springframework.boot.info.SslInfo.CertificateChain; +import org.springframework.boot.info.SslInfo.CertificateChainInfo; import org.springframework.boot.ssl.SslBundles; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.context.annotation.Bean; @@ -65,9 +65,9 @@ class SslHealthContributorAutoConfigurationTests { Health health = context.getBean(SslHealthIndicator.class).health(); assertThat(health.getStatus()).isSameAs(Status.OUT_OF_SERVICE); assertDetailsKeys(health); - List invalidChains = getInvalidChains(health); + List invalidChains = getInvalidChains(health); assertThat(invalidChains).hasSize(1); - assertThat(invalidChains).first().isInstanceOf(CertificateChain.class); + assertThat(invalidChains).first().isInstanceOf(CertificateChainInfo.class); }); } @@ -84,9 +84,9 @@ class SslHealthContributorAutoConfigurationTests { Health health = context.getBean(SslHealthIndicator.class).health(); assertThat(health.getStatus()).isSameAs(Status.OUT_OF_SERVICE); assertDetailsKeys(health); - List invalidChains = getInvalidChains(health); + List invalidChains = getInvalidChains(health); assertThat(invalidChains).hasSize(1); - assertThat(invalidChains).first().isInstanceOf(CertificateChain.class); + assertThat(invalidChains).first().isInstanceOf(CertificateChainInfo.class); }); } @@ -101,9 +101,9 @@ class SslHealthContributorAutoConfigurationTests { Health health = context.getBean(SslHealthIndicator.class).health(); assertThat(health.getStatus()).isSameAs(Status.OUT_OF_SERVICE); assertDetailsKeys(health); - List invalidChains = getInvalidChains(health); + List invalidChains = getInvalidChains(health); assertThat(invalidChains).hasSize(1); - assertThat(invalidChains).first().isInstanceOf(CertificateChain.class); + assertThat(invalidChains).first().isInstanceOf(CertificateChainInfo.class); }); } @@ -112,8 +112,8 @@ class SslHealthContributorAutoConfigurationTests { } @SuppressWarnings("unchecked") - private static List getInvalidChains(Health health) { - return (List) health.getDetails().get("invalidChains"); + private static List getInvalidChains(Health health) { + return (List) health.getDetails().get("invalidChains"); } @Configuration(proxyBeanMethods = false) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/ssl/SslHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/ssl/SslHealthIndicator.java index 17eaf153d9..18d8f2e193 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/ssl/SslHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/ssl/SslHealthIndicator.java @@ -16,14 +16,18 @@ package org.springframework.boot.actuate.ssl; +import java.util.ArrayList; import java.util.List; +import java.util.stream.Stream; import org.springframework.boot.actuate.health.AbstractHealthIndicator; import org.springframework.boot.actuate.health.Health.Builder; import org.springframework.boot.actuate.health.HealthIndicator; import org.springframework.boot.actuate.health.Status; import org.springframework.boot.info.SslInfo; -import org.springframework.boot.info.SslInfo.CertificateChain; +import org.springframework.boot.info.SslInfo.BundleInfo; +import org.springframework.boot.info.SslInfo.CertificateChainInfo; +import org.springframework.boot.info.SslInfo.CertificateInfo; /** * {@link HealthIndicator} that checks the certificates the application uses and reports @@ -42,38 +46,41 @@ public class SslHealthIndicator extends AbstractHealthIndicator { @Override protected void doHealthCheck(Builder builder) throws Exception { - List certificateChains = this.sslInfo.getBundles() - .stream() - .flatMap((bundle) -> bundle.getCertificateChains().stream()) - .toList(); - List validCertificateChains = certificateChains.stream() - .filter(this::containsOnlyValidCertificates) - .toList(); - List invalidCertificateChains = certificateChains.stream() - .filter(this::containsInvalidCertificate) - .toList(); + List validCertificateChains = new ArrayList<>(); + List invalidCertificateChains = new ArrayList<>(); + for (BundleInfo bundle : this.sslInfo.getBundles()) { + for (CertificateChainInfo certificateChain : bundle.getCertificateChains()) { + if (containsOnlyValidCertificates(certificateChain)) { + validCertificateChains.add(certificateChain); + } + else if (containsInvalidCertificate(certificateChain)) { + invalidCertificateChains.add(certificateChain); + } + } + } + builder.status((invalidCertificateChains.isEmpty()) ? Status.UP : Status.OUT_OF_SERVICE); builder.withDetail("validChains", validCertificateChains); builder.withDetail("invalidChains", invalidCertificateChains); - if (invalidCertificateChains.isEmpty()) { - builder.status(Status.UP); - } - else { - builder.status(Status.OUT_OF_SERVICE); - } } - private boolean containsOnlyValidCertificates(CertificateChain certificateChain) { - return certificateChain.getCertificates() - .stream() - .filter((certificate) -> certificate.getValidity() != null) - .allMatch((certificate) -> certificate.getValidity().getStatus().isValid()); + private boolean containsOnlyValidCertificates(CertificateChainInfo certificateChain) { + return validatableCertificates(certificateChain).allMatch(this::isValidCertificate); } - private boolean containsInvalidCertificate(CertificateChain certificateChain) { - return certificateChain.getCertificates() - .stream() - .filter((certificate) -> certificate.getValidity() != null) - .anyMatch((certificate) -> !certificate.getValidity().getStatus().isValid()); + private boolean containsInvalidCertificate(CertificateChainInfo certificateChain) { + return validatableCertificates(certificateChain).anyMatch(this::isNotValidCertificate); + } + + private Stream validatableCertificates(CertificateChainInfo certificateChain) { + return certificateChain.getCertificates().stream().filter((certificate) -> certificate.getValidity() != null); + } + + private boolean isValidCertificate(CertificateInfo certificate) { + return certificate.getValidity().getStatus().isValid(); + } + + private boolean isNotValidCertificate(CertificateInfo certificate) { + return !isValidCertificate(certificate); } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/ssl/SslHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/ssl/SslHealthIndicatorTests.java index 2318eaa299..74475c5ebf 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/ssl/SslHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/ssl/SslHealthIndicatorTests.java @@ -25,10 +25,10 @@ import org.springframework.boot.actuate.health.Health; import org.springframework.boot.actuate.health.HealthIndicator; import org.springframework.boot.actuate.health.Status; import org.springframework.boot.info.SslInfo; -import org.springframework.boot.info.SslInfo.Bundle; -import org.springframework.boot.info.SslInfo.CertificateChain; +import org.springframework.boot.info.SslInfo.BundleInfo; +import org.springframework.boot.info.SslInfo.CertificateChainInfo; import org.springframework.boot.info.SslInfo.CertificateInfo; -import org.springframework.boot.info.SslInfo.CertificateInfo.Validity; +import org.springframework.boot.info.SslInfo.CertificateValidityInfo; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; @@ -43,18 +43,16 @@ class SslHealthIndicatorTests { private HealthIndicator healthIndicator; - private Validity validity; + private CertificateValidityInfo validity; @BeforeEach void setUp() { SslInfo sslInfo = mock(SslInfo.class); - Bundle bundle = mock(Bundle.class); - CertificateChain certificateChain = mock(CertificateChain.class); + BundleInfo bundle = mock(BundleInfo.class); + CertificateChainInfo certificateChain = mock(CertificateChainInfo.class); CertificateInfo certificateInfo = mock(CertificateInfo.class); - this.healthIndicator = new SslHealthIndicator(sslInfo); - this.validity = mock(Validity.class); - + this.validity = mock(CertificateValidityInfo.class); given(sslInfo.getBundles()).willReturn(List.of(bundle)); given(bundle.getCertificateChains()).willReturn(List.of(certificateChain)); given(certificateChain.getCertificates()).willReturn(List.of(certificateInfo)); @@ -63,54 +61,54 @@ class SslHealthIndicatorTests { @Test void shouldBeUpIfNoSslIssuesDetected() { - given(this.validity.getStatus()).willReturn(Validity.Status.VALID); + given(this.validity.getStatus()).willReturn(CertificateValidityInfo.Status.VALID); Health health = this.healthIndicator.health(); assertThat(health.getStatus()).isEqualTo(Status.UP); assertDetailsKeys(health); - List validChains = getValidChains(health); + List validChains = getValidChains(health); assertThat(validChains).hasSize(1); - assertThat(validChains.get(0)).isInstanceOf(CertificateChain.class); - List invalidChains = getInvalidChains(health); + assertThat(validChains.get(0)).isInstanceOf(CertificateChainInfo.class); + List invalidChains = getInvalidChains(health); assertThat(invalidChains).isEmpty(); } @Test void shouldBeOutOfServiceIfACertificateIsExpired() { - given(this.validity.getStatus()).willReturn(Validity.Status.EXPIRED); + given(this.validity.getStatus()).willReturn(CertificateValidityInfo.Status.EXPIRED); Health health = this.healthIndicator.health(); assertThat(health.getStatus()).isEqualTo(Status.OUT_OF_SERVICE); assertDetailsKeys(health); - List validChains = getValidChains(health); + List validChains = getValidChains(health); assertThat(validChains).isEmpty(); - List invalidChains = getInvalidChains(health); + List invalidChains = getInvalidChains(health); assertThat(invalidChains).hasSize(1); - assertThat(invalidChains.get(0)).isInstanceOf(CertificateChain.class); + assertThat(invalidChains.get(0)).isInstanceOf(CertificateChainInfo.class); } @Test void shouldBeOutOfServiceIfACertificateIsNotYetValid() { - given(this.validity.getStatus()).willReturn(Validity.Status.NOT_YET_VALID); + given(this.validity.getStatus()).willReturn(CertificateValidityInfo.Status.NOT_YET_VALID); Health health = this.healthIndicator.health(); assertThat(health.getStatus()).isEqualTo(Status.OUT_OF_SERVICE); assertDetailsKeys(health); - List validChains = getValidChains(health); + List validChains = getValidChains(health); assertThat(validChains).isEmpty(); - List invalidChains = getInvalidChains(health); + List invalidChains = getInvalidChains(health); assertThat(invalidChains).hasSize(1); - assertThat(invalidChains.get(0)).isInstanceOf(CertificateChain.class); + assertThat(invalidChains.get(0)).isInstanceOf(CertificateChainInfo.class); } @Test void shouldReportWarningIfACertificateWillExpireSoon() { - given(this.validity.getStatus()).willReturn(Validity.Status.WILL_EXPIRE_SOON); + given(this.validity.getStatus()).willReturn(CertificateValidityInfo.Status.WILL_EXPIRE_SOON); Health health = this.healthIndicator.health(); assertThat(health.getStatus()).isEqualTo(Status.UP); assertDetailsKeys(health); - List validChains = getValidChains(health); + List validChains = getValidChains(health); assertThat(validChains).hasSize(1); - assertThat(validChains.get(0)).isInstanceOf(CertificateChain.class); - List invalidChains = getInvalidChains(health); + assertThat(validChains.get(0)).isInstanceOf(CertificateChainInfo.class); + List invalidChains = getInvalidChains(health); assertThat(invalidChains).isEmpty(); } @@ -119,13 +117,13 @@ class SslHealthIndicatorTests { } @SuppressWarnings("unchecked") - private static List getInvalidChains(Health health) { - return (List) health.getDetails().get("invalidChains"); + private static List getInvalidChains(Health health) { + return (List) health.getDetails().get("invalidChains"); } @SuppressWarnings("unchecked") - private static List getValidChains(Health health) { - return (List) health.getDetails().get("validChains"); + private static List getValidChains(Health health) { + return (List) health.getDetails().get("validChains"); } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/info/SslInfo.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/info/SslInfo.java index e0c5e533a5..484167a981 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/info/SslInfo.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/info/SslInfo.java @@ -24,12 +24,18 @@ import java.security.cert.CertificateNotYetValidException; import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; +import java.util.Arrays; import java.util.Collections; +import java.util.Date; import java.util.List; +import java.util.function.Function; -import org.springframework.boot.info.SslInfo.CertificateInfo.Validity.Status; +import javax.security.auth.x500.X500Principal; + +import org.springframework.boot.info.SslInfo.CertificateValidityInfo.Status; import org.springframework.boot.ssl.SslBundle; import org.springframework.boot.ssl.SslBundles; +import org.springframework.util.ObjectUtils; /** * Information about the certificates that the application uses. @@ -48,38 +54,32 @@ public class SslInfo { this.certificateValidityWarningThreshold = certificateValidityWarningThreshold; } - public List getBundles() { - return this.sslBundles.getBundles() - .entrySet() + public List getBundles() { + return this.sslBundles.getBundleNames() .stream() - .map((entry) -> new Bundle(entry.getKey(), entry.getValue())) + .map((name) -> new BundleInfo(name, this.sslBundles.getBundle(name))) .toList(); } - public final class Bundle { + /** + * Info about a single {@link SslBundle}. + */ + public final class BundleInfo { private final String name; - private final List certificateChains; + private final List certificateChains; - private Bundle(String name, SslBundle sslBundle) { + private BundleInfo(String name, SslBundle sslBundle) { this.name = name; - this.certificateChains = createCertificateChains(sslBundle.getStores().getKeyStore()); + this.certificateChains = extractCertificateChains(sslBundle.getStores().getKeyStore()); } - public String getName() { - return this.name; - } - - public List getCertificateChains() { - return this.certificateChains; - } - - private List createCertificateChains(KeyStore keyStore) { + private List extractCertificateChains(KeyStore keyStore) { try { return Collections.list(keyStore.aliases()) .stream() - .map((alias) -> new CertificateChain(alias, getCertificates(alias, keyStore))) + .map((alias) -> new CertificateChainInfo(keyStore, alias)) .toList(); } catch (KeyStoreException ex) { @@ -87,27 +87,39 @@ public class SslInfo { } } - private List getCertificates(String alias, KeyStore keyStore) { - try { - Certificate[] certificateChain = keyStore.getCertificateChain(alias); - return (certificateChain != null) ? List.of(certificateChain) : Collections.emptyList(); - } - catch (KeyStoreException ex) { - return Collections.emptyList(); - } + public String getName() { + return this.name; + } + + public List getCertificateChains() { + return this.certificateChains; } } - public final class CertificateChain { + /** + * Info about a single certificate chain. + */ + public final class CertificateChainInfo { private final String alias; private final List certificates; - CertificateChain(String alias, List certificates) { + CertificateChainInfo(KeyStore keyStore, String alias) { this.alias = alias; - this.certificates = certificates.stream().map(CertificateInfo::new).toList(); + this.certificates = extractCertificates(keyStore, alias); + } + + private List extractCertificates(KeyStore keyStore, String alias) { + try { + Certificate[] certificates = keyStore.getCertificateChain(alias); + return (!ObjectUtils.isEmpty(certificates)) + ? Arrays.stream(certificates).map(CertificateInfo::new).toList() : Collections.emptyList(); + } + catch (KeyStoreException ex) { + return Collections.emptyList(); + } } public String getAlias() { @@ -120,130 +132,139 @@ public class SslInfo { } + /** + * Info about a certificate. + */ public final class CertificateInfo { private final X509Certificate certificate; private CertificateInfo(Certificate certificate) { - if (certificate instanceof X509Certificate x509Certificate) { - this.certificate = x509Certificate; - } - else { - this.certificate = null; - } + this.certificate = (certificate instanceof X509Certificate x509Certificate) ? x509Certificate : null; } public String getSubject() { - return (this.certificate != null) ? this.certificate.getSubjectX500Principal().getName() : null; + return extract(X509Certificate::getSubjectX500Principal, X500Principal::getName); } public String getIssuer() { - return (this.certificate != null) ? this.certificate.getIssuerX500Principal().getName() : null; + return extract(X509Certificate::getIssuerX500Principal, X500Principal::getName); } public String getSerialNumber() { - return (this.certificate != null) ? this.certificate.getSerialNumber().toString(16) : null; + return extract(X509Certificate::getSerialNumber, (serial) -> serial.toString(16)); } public String getVersion() { - return (this.certificate != null) ? "V" + this.certificate.getVersion() : null; + return extract((certificate) -> "V" + certificate.getVersion()); } public String getSignatureAlgorithmName() { - return (this.certificate != null) ? this.certificate.getSigAlgName() : null; + return extract(X509Certificate::getSigAlgName); } public Instant getValidityStarts() { - return (this.certificate != null) ? this.certificate.getNotBefore().toInstant() : null; + return extract(X509Certificate::getNotBefore, Date::toInstant); } public Instant getValidityEnds() { - return (this.certificate != null) ? this.certificate.getNotAfter().toInstant() : null; + return extract(X509Certificate::getNotAfter, Date::toInstant); } - public Validity getValidity() { - try { - if (this.certificate != null) { - this.certificate.checkValidity(); - if (isCloseToBeExpired(this.certificate, SslInfo.this.certificateValidityWarningThreshold)) { - return new Validity(Status.WILL_EXPIRE_SOON, - "Certificate will expire within threshold (%s) at %s".formatted( - SslInfo.this.certificateValidityWarningThreshold, this.getValidityEnds())); - } - else { - return new Validity(Status.VALID, null); - } + public CertificateValidityInfo getValidity() { + return extract((certificate) -> { + Instant starts = getValidityStarts(); + Instant ends = getValidityEnds(); + Duration threshold = SslInfo.this.certificateValidityWarningThreshold; + try { + certificate.checkValidity(); + return (!isExpiringSoon(certificate, threshold)) ? CertificateValidityInfo.VALID + : new CertificateValidityInfo(Status.WILL_EXPIRE_SOON, + "Certificate will expire within threshold (%s) at %s", threshold, ends); } - else { - return null; + catch (CertificateNotYetValidException ex) { + return new CertificateValidityInfo(Status.NOT_YET_VALID, "Not valid before %s", starts); } - } - catch (CertificateNotYetValidException exception) { - return new Validity(Status.NOT_YET_VALID, "Not valid before %s".formatted(this.getValidityStarts())); - } - catch (CertificateExpiredException exception) { - return new Validity(Status.EXPIRED, "Not valid after %s".formatted(this.getValidityEnds())); - } + catch (CertificateExpiredException ex) { + return new CertificateValidityInfo(Status.EXPIRED, "Not valid after %s", ends); + } + }); } - private boolean isCloseToBeExpired(X509Certificate certificate, Duration certificateValidityThreshold) { - Instant shouldBeValidAt = Instant.now().plus(certificateValidityThreshold); + private boolean isExpiringSoon(X509Certificate certificate, Duration threshold) { + Instant shouldBeValidAt = Instant.now().plus(threshold); Instant expiresAt = certificate.getNotAfter().toInstant(); return shouldBeValidAt.isAfter(expiresAt); } - public static class Validity { + private R extract(Function valueExtractor, Function resultExtractor) { + return extract(valueExtractor.andThen(resultExtractor)); + } - private final Status status; + private R extract(Function extractor) { + return (this.certificate != null) ? extractor.apply(this.certificate) : null; + } - private final String message; + } - Validity(Status status, String message) { - this.status = status; - this.message = message; + /** + * Certificate validity info. + */ + public static class CertificateValidityInfo { + + static final CertificateValidityInfo VALID = new CertificateValidityInfo(Status.VALID, null); + + private final Status status; + + private final String message; + + CertificateValidityInfo(Status status, String message, Object... messageArgs) { + this.status = status; + this.message = (message != null) ? message.formatted(messageArgs) : null; + } + + public Status getStatus() { + return this.status; + } + + public String getMessage() { + return this.message; + } + + /** + * Validity Status. + */ + public enum Status { + + /** + * The certificate is valid. + */ + VALID(true), + + /** + * The certificate's validity date range is in the future. + */ + NOT_YET_VALID(false), + + /** + * The certificate's validity date range is in the past. + */ + EXPIRED(false), + + /** + * The certificate is still valid, but the end of its validity date range is + * within the defined threshold. + */ + WILL_EXPIRE_SOON(true); + + private final boolean valid; + + Status(boolean valid) { + this.valid = valid; } - public Status getStatus() { - return this.status; - } - - public String getMessage() { - return this.message; - } - - public enum Status { - - /** - * The certificate is valid. - */ - VALID(true), - - /** - * The certificate's validity date range is in the future. - */ - NOT_YET_VALID(false), - - /** - * The certificate's validity date range is in the past. - */ - EXPIRED(false), - - /** - * The certificate is still valid, but the end of its validity date range - * is within the defined threshold. - */ - WILL_EXPIRE_SOON(true); - - private final boolean valid; - - Status(boolean valid) { - this.valid = valid; - } - - public boolean isValid() { - return this.valid; - } - + public boolean isValid() { + return this.valid; } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/DefaultSslBundleRegistry.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/DefaultSslBundleRegistry.java index 8cef5d4e65..0a886a323f 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/DefaultSslBundleRegistry.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/DefaultSslBundleRegistry.java @@ -16,13 +16,13 @@ package org.springframework.boot.ssl; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Consumer; -import java.util.stream.Collectors; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -71,15 +71,15 @@ public class DefaultSslBundleRegistry implements SslBundleRegistry, SslBundles { } @Override - public Map getBundles() { - return this.registeredBundles.entrySet() - .stream() - .collect(Collectors.toUnmodifiableMap(Entry::getKey, (entry) -> entry.getValue().getBundle())); + public void addBundleUpdateHandler(String name, Consumer updateHandler) throws NoSuchSslBundleException { + getRegistered(name).addUpdateHandler(updateHandler); } @Override - public void addBundleUpdateHandler(String name, Consumer updateHandler) throws NoSuchSslBundleException { - getRegistered(name).addUpdateHandler(updateHandler); + public List getBundleNames() { + List names = new ArrayList<>(this.registeredBundles.keySet()); + Collections.sort(names); + return Collections.unmodifiableList(names); } private RegisteredSslBundle getRegistered(String name) throws NoSuchSslBundleException { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundles.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundles.java index d6c5270662..91bce68fdc 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundles.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/ssl/SslBundles.java @@ -16,7 +16,7 @@ package org.springframework.boot.ssl; -import java.util.Map; +import java.util.List; import java.util.function.Consumer; /** @@ -37,13 +37,6 @@ public interface SslBundles { */ SslBundle getBundle(String name) throws NoSuchSslBundleException; - /** - * Return all the {@link SslBundle SslBundles} by name. - * @return the bundles - * @since 3.4.0 - */ - Map getBundles(); - /** * Add a handler that will be called each time the named bundle is updated. * @param name the bundle name @@ -53,4 +46,11 @@ public interface SslBundles { */ void addBundleUpdateHandler(String name, Consumer updateHandler) throws NoSuchSslBundleException; + /** + * Return the names of all bundles managed by this instance. + * @return the bundle names + * @since 3.4.0 + */ + List getBundleNames(); + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/info/SslInfoTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/info/SslInfoTests.java index 8bf7fe8579..b2412a7356 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/info/SslInfoTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/info/SslInfoTests.java @@ -28,10 +28,10 @@ import java.util.stream.Collectors; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import org.springframework.boot.info.SslInfo.Bundle; -import org.springframework.boot.info.SslInfo.CertificateChain; +import org.springframework.boot.info.SslInfo.BundleInfo; +import org.springframework.boot.info.SslInfo.CertificateChainInfo; import org.springframework.boot.info.SslInfo.CertificateInfo; -import org.springframework.boot.info.SslInfo.CertificateInfo.Validity.Status; +import org.springframework.boot.info.SslInfo.CertificateValidityInfo.Status; import org.springframework.boot.ssl.DefaultSslBundleRegistry; import org.springframework.boot.ssl.SslBundle; import org.springframework.boot.ssl.SslStoreBundle; @@ -51,7 +51,7 @@ class SslInfoTests { void validCertificatesShouldProvideSslInfo() { SslInfo sslInfo = createSslInfo("classpath:test.p12"); assertThat(sslInfo.getBundles()).hasSize(1); - Bundle bundle = sslInfo.getBundles().get(0); + BundleInfo bundle = sslInfo.getBundles().get(0); assertThat(bundle.getName()).isEqualTo("test-0"); assertThat(bundle.getCertificateChains()).hasSize(4); assertThat(bundle.getCertificateChains().get(0).getAlias()).isEqualTo("spring-boot"); @@ -62,7 +62,6 @@ class SslInfoTests { assertThat(bundle.getCertificateChains().get(2).getCertificates()).isEmpty(); assertThat(bundle.getCertificateChains().get(3).getAlias()).isEqualTo("test-alias-cert"); assertThat(bundle.getCertificateChains().get(3).getCertificates()).isEmpty(); - CertificateInfo cert1 = bundle.getCertificateChains().get(0).getCertificates().get(0); assertThat(cert1.getSubject()).isEqualTo("CN=localhost,OU=Spring,O=VMware,L=Palo Alto,ST=California,C=US"); assertThat(cert1.getIssuer()).isEqualTo(cert1.getSubject()); @@ -74,7 +73,6 @@ class SslInfoTests { assertThat(cert1.getValidity()).isNotNull(); assertThat(cert1.getValidity().getStatus()).isSameAs(Status.VALID); assertThat(cert1.getValidity().getMessage()).isNull(); - CertificateInfo cert2 = bundle.getCertificateChains().get(1).getCertificates().get(0); assertThat(cert2.getSubject()).isEqualTo("CN=localhost,OU=Spring,O=VMware,L=Palo Alto,ST=California,C=US"); assertThat(cert2.getIssuer()).isEqualTo(cert2.getSubject()); @@ -92,10 +90,10 @@ class SslInfoTests { void notYetValidCertificateShouldProvideSslInfo() { SslInfo sslInfo = createSslInfo("classpath:test-not-yet-valid.p12"); assertThat(sslInfo.getBundles()).hasSize(1); - Bundle bundle = sslInfo.getBundles().get(0); + BundleInfo bundle = sslInfo.getBundles().get(0); assertThat(bundle.getName()).isEqualTo("test-0"); assertThat(bundle.getCertificateChains()).hasSize(1); - CertificateChain certificateChain = bundle.getCertificateChains().get(0); + CertificateChainInfo certificateChain = bundle.getCertificateChains().get(0); assertThat(certificateChain.getAlias()).isEqualTo("spring-boot"); List certs = certificateChain.getCertificates(); assertThat(certs).hasSize(1); @@ -116,10 +114,10 @@ class SslInfoTests { void expiredCertificateShouldProvideSslInfo() { SslInfo sslInfo = createSslInfo("classpath:test-expired.p12"); assertThat(sslInfo.getBundles()).hasSize(1); - Bundle bundle = sslInfo.getBundles().get(0); + BundleInfo bundle = sslInfo.getBundles().get(0); assertThat(bundle.getName()).isEqualTo("test-0"); assertThat(bundle.getCertificateChains()).hasSize(1); - CertificateChain certificateChain = bundle.getCertificateChains().get(0); + CertificateChainInfo certificateChain = bundle.getCertificateChains().get(0); assertThat(certificateChain.getAlias()).isEqualTo("spring-boot"); List certs = certificateChain.getCertificates(); assertThat(certs).hasSize(1); @@ -142,10 +140,10 @@ class SslInfoTests { Path keyStore = createKeyStore(tempDir); SslInfo sslInfo = createSslInfo(keyStore.toString()); assertThat(sslInfo.getBundles()).hasSize(1); - Bundle bundle = sslInfo.getBundles().get(0); + BundleInfo bundle = sslInfo.getBundles().get(0); assertThat(bundle.getName()).isEqualTo("test-0"); assertThat(bundle.getCertificateChains()).hasSize(1); - CertificateChain certificateChain = bundle.getCertificateChains().get(0); + CertificateChainInfo certificateChain = bundle.getCertificateChains().get(0); assertThat(certificateChain.getAlias()).isEqualTo("spring-boot"); List certs = certificateChain.getCertificates(); assertThat(certs).hasSize(1); @@ -169,13 +167,11 @@ class SslInfoTests { "classpath:test-expired.p12", keyStore.toString()); assertThat(sslInfo.getBundles()).hasSize(4); assertThat(sslInfo.getBundles()).allSatisfy((bundle) -> assertThat(bundle.getName()).startsWith("test-")); - List certs = sslInfo.getBundles() .stream() .flatMap((bundle) -> bundle.getCertificateChains().stream()) .flatMap((certificateChain) -> certificateChain.getCertificates().stream()) .toList(); - assertThat(certs).hasSize(5); assertThat(certs).allSatisfy((cert) -> { assertThat(cert.getSubject()).isEqualTo("CN=localhost,OU=Spring,O=VMware,L=Palo Alto,ST=California,C=US"); @@ -185,7 +181,6 @@ class SslInfoTests { assertThat(cert.getSignatureAlgorithmName()).isNotEmpty(); assertThat(cert.getValidity()).isNotNull(); }); - assertThat(certs).anySatisfy((cert) -> { assertThat(cert.getValidityStarts()).isInThePast(); assertThat(cert.getValidityEnds()).isInTheFuture(); @@ -193,7 +188,6 @@ class SslInfoTests { assertThat(cert.getValidity().getStatus()).isSameAs(Status.VALID); assertThat(cert.getValidity().getMessage()).isNull(); }); - assertThat(certs).satisfiesOnlyOnce((cert) -> { assertThat(cert.getValidityStarts()).isInTheFuture(); assertThat(cert.getValidityEnds()).isInTheFuture(); @@ -201,7 +195,6 @@ class SslInfoTests { assertThat(cert.getValidity().getStatus()).isSameAs(Status.NOT_YET_VALID); assertThat(cert.getValidity().getMessage()).startsWith("Not valid before"); }); - assertThat(certs).satisfiesOnlyOnce((cert) -> { assertThat(cert.getValidityStarts()).isInThePast(); assertThat(cert.getValidityEnds()).isInThePast(); @@ -209,7 +202,6 @@ class SslInfoTests { assertThat(cert.getValidity().getStatus()).isSameAs(Status.EXPIRED); assertThat(cert.getValidity().getMessage()).startsWith("Not valid after"); }); - assertThat(certs).satisfiesOnlyOnce((cert) -> { assertThat(cert.getValidityStarts()).isInThePast(); assertThat(cert.getValidityEnds()).isInTheFuture(); @@ -234,10 +226,11 @@ class SslInfoTests { Process process = createProcessBuilder(keyStore).start(); int exitCode = process.waitFor(); if (exitCode != 0) { - String out = new BufferedReader(new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8)) - .lines() - .collect(Collectors.joining("\n")); - throw new RuntimeException("Unexpected exit code from keytool: %d\n%s".formatted(exitCode, out)); + try (BufferedReader reader = new BufferedReader( + new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8))) { + String out = reader.lines().collect(Collectors.joining("\n")); + throw new RuntimeException("Unexpected exit code from keytool: %d\n%s".formatted(exitCode, out)); + } } return keyStore; } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslBundleRegistryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslBundleRegistryTests.java index b6f7bc655f..d140a1db5d 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslBundleRegistryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/ssl/DefaultSslBundleRegistryTests.java @@ -106,12 +106,10 @@ class DefaultSslBundleRegistryTests { } @Test - void getBundlesReturnsBundles() { + void getBundleNamesReturnsNames() { this.registry.registerBundle("test1", this.bundle1); this.registry.registerBundle("test2", this.bundle2); - assertThat(this.registry.getBundles()).hasSize(2) - .containsEntry("test1", this.bundle1) - .containsEntry("test2", this.bundle2); + assertThat(this.registry.getBundleNames()).containsExactly("test1", "test2"); } @Test