Remove validity threshold from SslInfo
Closes gh-44650
This commit is contained in:
@@ -114,8 +114,8 @@ public class InfoContributorAutoConfiguration {
|
||||
@Bean
|
||||
@ConditionalOnMissingBean
|
||||
@ConditionalOnEnabledInfoContributor(value = "ssl", fallback = InfoContributorFallback.DISABLE)
|
||||
SslInfo sslInfo(SslBundles sslBundles, SslHealthIndicatorProperties sslHealthIndicatorProperties) {
|
||||
return new SslInfo(sslBundles, sslHealthIndicatorProperties.getCertificateValidityWarningThreshold());
|
||||
SslInfo sslInfo(SslBundles sslBundles) {
|
||||
return new SslInfo(sslBundles);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -40,14 +40,14 @@ public class SslHealthContributorAutoConfiguration {
|
||||
|
||||
@Bean
|
||||
@ConditionalOnMissingBean(name = "sslHealthIndicator")
|
||||
SslHealthIndicator sslHealthIndicator(SslInfo sslInfo) {
|
||||
return new SslHealthIndicator(sslInfo);
|
||||
SslHealthIndicator sslHealthIndicator(SslInfo sslInfo, SslHealthIndicatorProperties properties) {
|
||||
return new SslHealthIndicator(sslInfo, properties.getCertificateValidityWarningThreshold());
|
||||
}
|
||||
|
||||
@Bean
|
||||
@ConditionalOnMissingBean
|
||||
SslInfo sslInfo(SslBundles sslBundles, SslHealthIndicatorProperties sslHealthIndicatorProperties) {
|
||||
return new SslInfo(sslBundles, sslHealthIndicatorProperties.getCertificateValidityWarningThreshold());
|
||||
SslInfo sslInfo(SslBundles sslBundles) {
|
||||
return new SslInfo(sslBundles);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -52,7 +52,7 @@ public class SslObservabilityAutoConfiguration {
|
||||
@Bean
|
||||
@ConditionalOnMissingBean
|
||||
SslInfo sslInfoProvider(SslBundles sslBundles, SslHealthIndicatorProperties properties) {
|
||||
return new SslInfo(sslBundles, properties.getCertificateValidityWarningThreshold());
|
||||
return new SslInfo(sslBundles);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -301,7 +301,7 @@ class InfoContributorAutoConfigurationTests {
|
||||
|
||||
@Bean
|
||||
SslInfo customSslInfo(SslBundles sslBundles) {
|
||||
return new SslInfo(sslBundles, Duration.ofDays(7));
|
||||
return new SslInfo(sslBundles);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -110,7 +110,7 @@ class SslHealthContributorAutoConfigurationTests {
|
||||
}
|
||||
|
||||
private static void assertDetailsKeys(Health health) {
|
||||
assertThat(health.getDetails()).containsOnlyKeys("validChains", "invalidChains");
|
||||
assertThat(health.getDetails()).containsOnlyKeys("expiringChains", "validChains", "invalidChains");
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
@@ -128,13 +128,13 @@ class SslHealthContributorAutoConfigurationTests {
|
||||
|
||||
@Bean
|
||||
SslInfo customSslInfo(SslBundles sslBundles) {
|
||||
return new SslInfo(sslBundles, Duration.ofDays(7));
|
||||
return new SslInfo(sslBundles);
|
||||
}
|
||||
|
||||
static class CustomSslHealthIndicator extends SslHealthIndicator {
|
||||
|
||||
CustomSslHealthIndicator(SslInfo sslInfo) {
|
||||
super(sslInfo);
|
||||
super(sslInfo, Duration.ofDays(7));
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -92,7 +92,7 @@ class SslMeterBinderTests {
|
||||
}
|
||||
|
||||
private SslInfo createSslInfo(SslBundles sslBundles) {
|
||||
return new SslInfo(sslBundles, Duration.ofDays(7), CLOCK);
|
||||
return new SslInfo(sslBundles);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -16,6 +16,8 @@
|
||||
|
||||
package org.springframework.boot.actuate.ssl;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.time.Instant;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.stream.Stream;
|
||||
@@ -42,20 +44,27 @@ public class SslHealthIndicator extends AbstractHealthIndicator {
|
||||
|
||||
private final SslInfo sslInfo;
|
||||
|
||||
public SslHealthIndicator(SslInfo sslInfo) {
|
||||
private final Duration expiryThreshold;
|
||||
|
||||
public SslHealthIndicator(SslInfo sslInfo, Duration expiryThreshold) {
|
||||
super("SSL health check failed");
|
||||
Assert.notNull(sslInfo, "'sslInfo' must not be null");
|
||||
this.sslInfo = sslInfo;
|
||||
this.expiryThreshold = expiryThreshold;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doHealthCheck(Builder builder) throws Exception {
|
||||
List<CertificateChainInfo> validCertificateChains = new ArrayList<>();
|
||||
List<CertificateChainInfo> invalidCertificateChains = new ArrayList<>();
|
||||
List<CertificateChainInfo> expiringCerificateChains = new ArrayList<>();
|
||||
for (BundleInfo bundle : this.sslInfo.getBundles()) {
|
||||
for (CertificateChainInfo certificateChain : bundle.getCertificateChains()) {
|
||||
if (containsOnlyValidCertificates(certificateChain)) {
|
||||
validCertificateChains.add(certificateChain);
|
||||
if (containsExpiringCertificate(certificateChain)) {
|
||||
expiringCerificateChains.add(certificateChain);
|
||||
}
|
||||
}
|
||||
else if (containsInvalidCertificate(certificateChain)) {
|
||||
invalidCertificateChains.add(certificateChain);
|
||||
@@ -63,8 +72,9 @@ public class SslHealthIndicator extends AbstractHealthIndicator {
|
||||
}
|
||||
}
|
||||
builder.status((invalidCertificateChains.isEmpty()) ? Status.UP : Status.OUT_OF_SERVICE);
|
||||
builder.withDetail("validChains", validCertificateChains);
|
||||
builder.withDetail("expiringChains", expiringCerificateChains);
|
||||
builder.withDetail("invalidChains", invalidCertificateChains);
|
||||
builder.withDetail("validChains", validCertificateChains);
|
||||
}
|
||||
|
||||
private boolean containsOnlyValidCertificates(CertificateChainInfo certificateChain) {
|
||||
@@ -75,6 +85,10 @@ public class SslHealthIndicator extends AbstractHealthIndicator {
|
||||
return validatableCertificates(certificateChain).anyMatch(this::isNotValidCertificate);
|
||||
}
|
||||
|
||||
private boolean containsExpiringCertificate(CertificateChainInfo certificateChain) {
|
||||
return validatableCertificates(certificateChain).anyMatch(this::isExpiringCertificate);
|
||||
}
|
||||
|
||||
private Stream<CertificateInfo> validatableCertificates(CertificateChainInfo certificateChain) {
|
||||
return certificateChain.getCertificates().stream().filter((certificate) -> certificate.getValidity() != null);
|
||||
}
|
||||
@@ -87,4 +101,8 @@ public class SslHealthIndicator extends AbstractHealthIndicator {
|
||||
return !isValidCertificate(certificate);
|
||||
}
|
||||
|
||||
private boolean isExpiringCertificate(CertificateInfo certificate) {
|
||||
return Instant.now().plus(this.expiryThreshold).isAfter(certificate.getValidityEnds());
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -16,8 +16,6 @@
|
||||
|
||||
package org.springframework.boot.actuate.info;
|
||||
|
||||
import java.time.Duration;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import org.springframework.aot.hint.MemberCategory;
|
||||
@@ -42,7 +40,7 @@ class SslInfoContributorTests {
|
||||
@Test
|
||||
void sslInfoShouldBeAdded() {
|
||||
SslBundles sslBundles = new DefaultSslBundleRegistry("test", mock(SslBundle.class));
|
||||
SslInfo sslInfo = new SslInfo(sslBundles, Duration.ofDays(14));
|
||||
SslInfo sslInfo = new SslInfo(sslBundles);
|
||||
SslInfoContributor sslInfoContributor = new SslInfoContributor(sslInfo);
|
||||
Info.Builder builder = new Info.Builder();
|
||||
sslInfoContributor.contribute(builder);
|
||||
|
||||
@@ -16,13 +16,14 @@
|
||||
|
||||
package org.springframework.boot.actuate.ssl;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.time.Instant;
|
||||
import java.util.List;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
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.BundleInfo;
|
||||
@@ -41,26 +42,27 @@ import static org.mockito.Mockito.mock;
|
||||
*/
|
||||
class SslHealthIndicatorTests {
|
||||
|
||||
private HealthIndicator healthIndicator;
|
||||
private final CertificateInfo certificateInfo = mock(CertificateInfo.class);
|
||||
|
||||
private CertificateValidityInfo validity;
|
||||
private final CertificateValidityInfo validity = mock(CertificateValidityInfo.class);
|
||||
|
||||
private SslHealthIndicator healthIndicator;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
SslInfo sslInfo = mock(SslInfo.class);
|
||||
BundleInfo bundle = mock(BundleInfo.class);
|
||||
CertificateChainInfo certificateChain = mock(CertificateChainInfo.class);
|
||||
CertificateInfo certificateInfo = mock(CertificateInfo.class);
|
||||
this.healthIndicator = new SslHealthIndicator(sslInfo);
|
||||
this.validity = mock(CertificateValidityInfo.class);
|
||||
this.healthIndicator = new SslHealthIndicator(sslInfo, Duration.ofDays(7));
|
||||
given(sslInfo.getBundles()).willReturn(List.of(bundle));
|
||||
given(bundle.getCertificateChains()).willReturn(List.of(certificateChain));
|
||||
given(certificateChain.getCertificates()).willReturn(List.of(certificateInfo));
|
||||
given(certificateInfo.getValidity()).willReturn(this.validity);
|
||||
given(certificateChain.getCertificates()).willReturn(List.of(this.certificateInfo));
|
||||
given(this.certificateInfo.getValidity()).willReturn(this.validity);
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldBeUpIfNoSslIssuesDetected() {
|
||||
given(this.certificateInfo.getValidityEnds()).willReturn(Instant.now().plus(Duration.ofDays(365)));
|
||||
given(this.validity.getStatus()).willReturn(CertificateValidityInfo.Status.VALID);
|
||||
Health health = this.healthIndicator.health();
|
||||
assertThat(health.getStatus()).isEqualTo(Status.UP);
|
||||
@@ -101,10 +103,14 @@ class SslHealthIndicatorTests {
|
||||
|
||||
@Test
|
||||
void shouldReportWarningIfACertificateWillExpireSoon() {
|
||||
given(this.validity.getStatus()).willReturn(CertificateValidityInfo.Status.WILL_EXPIRE_SOON);
|
||||
given(this.validity.getStatus()).willReturn(CertificateValidityInfo.Status.VALID);
|
||||
given(this.certificateInfo.getValidityEnds()).willReturn(Instant.now().plus(Duration.ofDays(3)));
|
||||
Health health = this.healthIndicator.health();
|
||||
assertThat(health.getStatus()).isEqualTo(Status.UP);
|
||||
assertDetailsKeys(health);
|
||||
List<CertificateChainInfo> expiring = getExpiringChains(health);
|
||||
assertThat(expiring).hasSize(1);
|
||||
assertThat(expiring.get(0)).isInstanceOf(CertificateChainInfo.class);
|
||||
List<CertificateChainInfo> validChains = getValidChains(health);
|
||||
assertThat(validChains).hasSize(1);
|
||||
assertThat(validChains.get(0)).isInstanceOf(CertificateChainInfo.class);
|
||||
@@ -113,17 +119,24 @@ class SslHealthIndicatorTests {
|
||||
}
|
||||
|
||||
private static void assertDetailsKeys(Health health) {
|
||||
assertThat(health.getDetails()).containsOnlyKeys("validChains", "invalidChains");
|
||||
assertThat(health.getDetails()).containsOnlyKeys("expiringChains", "validChains", "invalidChains");
|
||||
}
|
||||
|
||||
private static List<CertificateChainInfo> getExpiringChains(Health health) {
|
||||
return getChains(health, "expiringChains");
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
private static List<CertificateChainInfo> getInvalidChains(Health health) {
|
||||
return (List<CertificateChainInfo>) health.getDetails().get("invalidChains");
|
||||
return getChains(health, "invalidChains");
|
||||
}
|
||||
|
||||
private static List<CertificateChainInfo> getValidChains(Health health) {
|
||||
return getChains(health, "validChains");
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
private static List<CertificateChainInfo> getValidChains(Health health) {
|
||||
return (List<CertificateChainInfo>) health.getDetails().get("validChains");
|
||||
private static List<CertificateChainInfo> getChains(Health health, String name) {
|
||||
return (List<CertificateChainInfo>) health.getDetails().get(name);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -22,8 +22,6 @@ import java.security.cert.Certificate;
|
||||
import java.security.cert.CertificateExpiredException;
|
||||
import java.security.cert.CertificateNotYetValidException;
|
||||
import java.security.cert.X509Certificate;
|
||||
import java.time.Clock;
|
||||
import java.time.Duration;
|
||||
import java.time.Instant;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
@@ -50,32 +48,13 @@ public class SslInfo {
|
||||
|
||||
private final SslBundles sslBundles;
|
||||
|
||||
private final Duration certificateValidityWarningThreshold;
|
||||
|
||||
private final Clock clock;
|
||||
|
||||
/**
|
||||
* Creates a new instance.
|
||||
* @param sslBundles the {@link SslBundles} to extract the info from
|
||||
* @param certificateValidityWarningThreshold the certificate validity warning
|
||||
* threshold
|
||||
* @param sslBundles the {@link SslBundles} to extract the info from threshold
|
||||
* @since 4.0.0
|
||||
*/
|
||||
public SslInfo(SslBundles sslBundles, Duration certificateValidityWarningThreshold) {
|
||||
this(sslBundles, certificateValidityWarningThreshold, Clock.systemDefaultZone());
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a new instance.
|
||||
* @param sslBundles the {@link SslBundles} to extract the info from
|
||||
* @param certificateValidityWarningThreshold the certificate validity warning
|
||||
* threshold
|
||||
* @param clock the {@link Clock} to use
|
||||
* @since 3.5.0
|
||||
*/
|
||||
public SslInfo(SslBundles sslBundles, Duration certificateValidityWarningThreshold, Clock clock) {
|
||||
public SslInfo(SslBundles sslBundles) {
|
||||
this.sslBundles = sslBundles;
|
||||
this.certificateValidityWarningThreshold = certificateValidityWarningThreshold;
|
||||
this.clock = clock;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -218,12 +197,9 @@ public class SslInfo {
|
||||
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);
|
||||
return CertificateValidityInfo.VALID;
|
||||
}
|
||||
catch (CertificateNotYetValidException ex) {
|
||||
return new CertificateValidityInfo(Status.NOT_YET_VALID, "Not valid before %s", starts);
|
||||
@@ -234,12 +210,6 @@ public class SslInfo {
|
||||
});
|
||||
}
|
||||
|
||||
private boolean isExpiringSoon(X509Certificate certificate, Duration threshold) {
|
||||
Instant shouldBeValidAt = Instant.now(SslInfo.this.clock).plus(threshold);
|
||||
Instant expiresAt = certificate.getNotAfter().toInstant();
|
||||
return shouldBeValidAt.isAfter(expiresAt);
|
||||
}
|
||||
|
||||
private <V, R> R extract(Function<X509Certificate, V> valueExtractor, Function<V, R> resultExtractor) {
|
||||
return extract(valueExtractor.andThen(resultExtractor));
|
||||
}
|
||||
@@ -292,13 +262,7 @@ public class SslInfo {
|
||||
/**
|
||||
* 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);
|
||||
EXPIRED(false);
|
||||
|
||||
private final boolean valid;
|
||||
|
||||
|
||||
@@ -21,7 +21,6 @@ import java.io.IOException;
|
||||
import java.io.InputStreamReader;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.nio.file.Path;
|
||||
import java.time.Duration;
|
||||
import java.util.List;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
@@ -139,32 +138,6 @@ class SslInfoTests {
|
||||
assertThat(cert.getValidity().getMessage()).startsWith("Not valid after");
|
||||
}
|
||||
|
||||
@Test
|
||||
void soonToBeExpiredCertificateShouldProvideSslInfo(@TempDir Path tempDir)
|
||||
throws IOException, InterruptedException {
|
||||
Path keyStore = createKeyStore(tempDir);
|
||||
SslInfo sslInfo = createSslInfo(keyStore.toString());
|
||||
assertThat(sslInfo.getBundles()).hasSize(1);
|
||||
BundleInfo bundle = sslInfo.getBundles().get(0);
|
||||
assertThat(bundle.getName()).isEqualTo("test-0");
|
||||
assertThat(bundle.getCertificateChains()).hasSize(1);
|
||||
CertificateChainInfo certificateChain = bundle.getCertificateChains().get(0);
|
||||
assertThat(certificateChain.getAlias()).isEqualTo("spring-boot");
|
||||
List<CertificateInfo> certs = certificateChain.getCertificates();
|
||||
assertThat(certs).hasSize(1);
|
||||
CertificateInfo cert = certs.get(0);
|
||||
assertThat(cert.getSubject()).isEqualTo("CN=localhost,OU=Spring,O=VMware,L=Palo Alto,ST=California,C=US");
|
||||
assertThat(cert.getIssuer()).isEqualTo(cert.getSubject());
|
||||
assertThat(cert.getSerialNumber()).isNotEmpty();
|
||||
assertThat(cert.getVersion()).isEqualTo("V3");
|
||||
assertThat(cert.getSignatureAlgorithmName()).isNotEmpty();
|
||||
assertThat(cert.getValidityStarts()).isInThePast();
|
||||
assertThat(cert.getValidityEnds()).isInTheFuture();
|
||||
assertThat(cert.getValidity()).isNotNull();
|
||||
assertThat(cert.getValidity().getStatus()).isSameAs(Status.WILL_EXPIRE_SOON);
|
||||
assertThat(cert.getValidity().getMessage()).startsWith("Certificate will expire within threshold");
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithPackageResources({ "test.p12", "test-not-yet-valid.p12", "test-expired.p12" })
|
||||
void multipleBundlesShouldProvideSslInfo(@TempDir Path tempDir) throws IOException, InterruptedException {
|
||||
@@ -208,20 +181,13 @@ 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();
|
||||
assertThat(cert.getValidity()).isNotNull();
|
||||
assertThat(cert.getValidity().getStatus()).isSameAs(Status.WILL_EXPIRE_SOON);
|
||||
assertThat(cert.getValidity().getMessage()).startsWith("Certificate will expire within threshold");
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
void nullKeyStore() {
|
||||
DefaultSslBundleRegistry sslBundleRegistry = new DefaultSslBundleRegistry();
|
||||
sslBundleRegistry.registerBundle("test", SslBundle.of(SslStoreBundle.NONE, SslBundleKey.NONE));
|
||||
SslInfo sslInfo = new SslInfo(sslBundleRegistry, Duration.ofDays(7));
|
||||
SslInfo sslInfo = new SslInfo(sslBundleRegistry);
|
||||
assertThat(sslInfo.getBundles()).hasSize(1);
|
||||
assertThat(sslInfo.getBundles().get(0).getCertificateChains()).isEmpty();
|
||||
}
|
||||
@@ -233,7 +199,7 @@ class SslInfoTests {
|
||||
SslStoreBundle sslStoreBundle = new JksSslStoreBundle(keyStoreDetails, null);
|
||||
sslBundleRegistry.registerBundle("test-%d".formatted(i), SslBundle.of(sslStoreBundle));
|
||||
}
|
||||
return new SslInfo(sslBundleRegistry, Duration.ofDays(7));
|
||||
return new SslInfo(sslBundleRegistry);
|
||||
}
|
||||
|
||||
private Path createKeyStore(Path directory) throws IOException, InterruptedException {
|
||||
|
||||
Reference in New Issue
Block a user