[BUG]Opensearch won't start if there are expired certificates in the CA bundle

[reshippie]

What is the bug?
The security plugin now verifies every certificate in the CA bundle and prevents Opensearch from starting if any of them are expired.

How can one reproduce the bug?
Steps to reproduce the behavior:

1. Create a CA bundle with at least 1 expired certificate
2. Copy the CA bundle to Opensearch's configuration directory
3. Set the copied bundle as plugins.security.ssl.http.pemtrustedcas_filepath and or plugins.security.ssl.transport.pemtrustedcas_filepath
4. Try to start Opensearch

What is the expected behavior?
I would expect that the service will start, just like it did up through 2.17.1

What is your host/environment?

• OS: Debian 11
• Version 2.18
• Plugins: Security

Do you have any screenshots?

java.lang.IllegalStateException: failed to load plugin class [org.opensearch.security.OpenSearchSecurityPlugin]
        at org.opensearch.plugins.PluginsService.loadPlugin(PluginsService.java:805) ~[opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.plugins.PluginsService.loadBundle(PluginsService.java:744) ~[opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.plugins.PluginsService.loadBundles(PluginsService.java:545) ~[opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.plugins.PluginsService.<init>(PluginsService.java:197) ~[opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.node.Node.<init>(Node.java:523) ~[opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.node.Node.<init>(Node.java:450) ~[opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.bootstrap.Bootstrap$5.<init>(Bootstrap.java:242) ~[opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.bootstrap.Bootstrap.setup(Bootstrap.java:242) ~[opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.bootstrap.Bootstrap.init(Bootstrap.java:404) [opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.bootstrap.OpenSearch.init(OpenSearch.java:181) [opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.bootstrap.OpenSearch.execute(OpenSearch.java:172) [opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:104) [opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.cli.Command.mainWithoutErrorHandling(Command.java:138) [opensearch-cli-2.18.0.jar:2.18.0]
        at org.opensearch.cli.Command.main(Command.java:101) [opensearch-cli-2.18.0.jar:2.18.0]
        at org.opensearch.bootstrap.OpenSearch.main(OpenSearch.java:138) [opensearch-2.18.0.jar:2.18.0]
        at org.opensearch.bootstrap.OpenSearch.main(OpenSearch.java:104) [opensearch-2.18.0.jar:2.18.0]
Caused by: java.lang.reflect.InvocationTargetException
        at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:74) ~[?:?]
        at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502) ~[?:?]
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:486) ~[?:?]
        at org.opensearch.plugins.PluginsService.loadPlugin(PluginsService.java:796) ~[opensearch-2.18.0.jar:2.18.0]
        ... 15 more
Caused by: org.opensearch.OpenSearchException: Invalid certificates
        at org.opensearch.security.ssl.config.KeyStoreUtils.validateKeyStoreCertificates(KeyStoreUtils.java:161) ~[?:?]
        at org.opensearch.security.ssl.config.TrustStoreConfiguration.createTrustManagerFactory(TrustStoreConfiguration.java:61) ~[?:?]
        at org.opensearch.security.ssl.SslConfiguration.lambda$buildServerSslContext$0(SslConfiguration.java:84) ~[?:?]
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:571) ~[?:?]
        at org.opensearch.security.ssl.SslConfiguration.buildServerSslContext(SslConfiguration.java:73) ~[?:?]
        at org.opensearch.security.ssl.SslContextHandler.<init>(SslContextHandler.java:42) ~[?:?]
        at org.opensearch.security.ssl.SslContextHandler.<init>(SslContextHandler.java:38) ~[?:?]
        at org.opensearch.security.ssl.SslSettingsManager.lambda$buildSslContexts$0(SslSettingsManager.java:96) ~[?:?]
        at java.base/java.util.Optional.ifPresentOrElse(Optional.java:196) ~[?:?]
        at org.opensearch.security.ssl.SslSettingsManager.buildSslContexts(SslSettingsManager.java:95) ~[?:?]
        at org.opensearch.security.ssl.SslSettingsManager.<init>(SslSettingsManager.java:80) ~[?:?]
        at org.opensearch.security.ssl.OpenSearchSecuritySSLPlugin.<init>(OpenSearchSecuritySSLPlugin.java:249) ~[?:?]
        at org.opensearch.security.OpenSearchSecurityPlugin.<init>(OpenSearchSecurityPlugin.java:318) ~[?:?]
        at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62) ~[?:?]
        at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502) ~[?:?]
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:486) ~[?:?]
        at org.opensearch.plugins.PluginsService.loadPlugin(PluginsService.java:796) ~[opensearch-2.18.0.jar:2.18.0]
        ... 15 more
Caused by: java.security.cert.CertificateExpiredException: NotAfter: Wed Dec 15 08:00:00 UTC 2021
        at java.base/sun.security.x509.CertificateValidity.valid(CertificateValidity.java:182) ~[?:?]
        at java.base/sun.security.x509.X509CertImpl.checkValidity(X509CertImpl.java:534) ~[?:?]
        at java.base/sun.security.x509.X509CertImpl.checkValidity(X509CertImpl.java:507) ~[?:?]
        at org.opensearch.security.ssl.config.KeyStoreUtils.validateKeyStoreCertificates(KeyStoreUtils.java:147) ~[?:?]
        at org.opensearch.security.ssl.config.TrustStoreConfiguration.createTrustManagerFactory(TrustStoreConfiguration.java:61) ~[?:?]
        at org.opensearch.security.ssl.SslConfiguration.lambda$buildServerSslContext$0(SslConfiguration.java:84) ~[?:?]
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:571) ~[?:?]
        at org.opensearch.security.ssl.SslConfiguration.buildServerSslContext(SslConfiguration.java:73) ~[?:?]
        at org.opensearch.security.ssl.SslContextHandler.<init>(SslContextHandler.java:42) ~[?:?]
        at org.opensearch.security.ssl.SslContextHandler.<init>(SslContextHandler.java:38) ~[?:?]
        at org.opensearch.security.ssl.SslSettingsManager.lambda$buildSslContexts$0(SslSettingsManager.java:96) ~[?:?]
        at java.base/java.util.Optional.ifPresentOrElse(Optional.java:196) ~[?:?]
        at org.opensearch.security.ssl.SslSettingsManager.buildSslContexts(SslSettingsManager.java:95) ~[?:?]
        at org.opensearch.security.ssl.SslSettingsManager.<init>(SslSettingsManager.java:80) ~[?:?]
        at org.opensearch.security.ssl.OpenSearchSecuritySSLPlugin.<init>(OpenSearchSecuritySSLPlugin.java:249) ~[?:?]
        at org.opensearch.security.OpenSearchSecurityPlugin.<init>(OpenSearchSecurityPlugin.java:318) ~[?:?]
        at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62) ~[?:?]
        at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502) ~[?:?]
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:486) ~[?:?]
        at org.opensearch.plugins.PluginsService.loadPlugin(PluginsService.java:796) ~[opensearch-2.18.0.jar:2.18.0]
        ... 15 more

Do you have any additional context?
I believe this problem was introduced by #4837 This problem was not present in 2.17.1, but is in 2.18. A workaround is to identify the expired certificates in the CA bundle and exclude them, but even the current stable distribution of Debian includes at least 1 expired certificate.
An option to ignore this check for CA certificates would be extremely helpful, especially since it's not immediately obvious from the above error that the problem is with the CA bundle.

[kumargu]

@cwperks could you please assign this to @udabhas

[cwperks]

[Triage] Thank you for filing this issue @reshippie. I see a couple possible solutions:

1. Introduce a setting where root ca validation can be disabled
2. Only verify certificates in the chain of the node certificate
3. Ensure all certs in the bundle remain up-to-date

[MenschNestor]

Isn't this a ticking time bomb as well? Won't any installation that uses a certificate from a truststore like the JDK default one just fail to start as soon as any certificate in it expires? Or is the recommendation to always create your own truststore with only the required certificate path(s)?

[kumargu]

we are evaluating a solution and a timeline to address this.

[shubhmkr-amazon]

I believe we should not check certificate validity of the truststore bundle that is coming from either PEM file or JKS file for the following reasons:

1. The truststore can be obtained from external sources and is not always under our control.
2. Since we are already validating leaf certificate validity, this implicitly ensures that no signer CA can have an expiry date earlier than the signed certificate, as a certificate can't be created with a longer validity period than its signer.

We need to update checks in two places:

1. During plugin loading:
   https://github.com/opensearch-project/security/blob/2.19/src/main/java/org/opensearch/security/ssl/config/TrustStoreConfiguration.java#L60-L62

2. During certificate reload calls:
   https://github.com/opensearch-project/security/blob/2.19/src/main/java/org/opensearch/security/ssl/SslContextHandler.java#L99

As @cwperks has already started working on the PR, I think we should also include the hot-reload part in this change.

@willyborankin, what are your thoughts on this approach? Do you see any potential issues by not validating expiry on truststore bundle?

[kumargu]

As @cwperks has already started working on the PR, I think we should also include the hot-reload part in this change.

which PR are you referring here?

[shubhmkr-amazon]

which PR are you referring here?

#4979

[kumargu]

@willyborankin: pinging back for your feedback. thanks!

[willyborankin]

I believe we should not check certificate validity of the truststore bundle that is coming from either PEM file or JKS file for the following reasons:

1. The truststore can be obtained from external sources and is not always under our control.

2. Since we are already validating leaf certificate validity, this implicitly ensures that no signer CA can have an expiry date earlier than the signed certificate, as a certificate can't be created with a longer validity period than its signer.

We need to update checks in two places:

1. During plugin loading:
   https://github.com/opensearch-project/security/blob/2.19/src/main/java/org/opensearch/security/ssl/config/TrustStoreConfiguration.java#L60-L62

2. During certificate reload calls:
   https://github.com/opensearch-project/security/blob/2.19/src/main/java/org/opensearch/security/ssl/SslContextHandler.java#L99

As @cwperks has already started working on the PR, I think we should also include the hot-reload part in this change.

@willyborankin, what are your thoughts on this approach? Do you see any potential issues by not validating expiry on truststore bundle?

Agree that @cwperks' solution is the way to go. And reload should use the same logic we use to verify SSL certs

[cwperks]

Since we are already validating leaf certificate validity, this implicitly ensures that no signer CA can have an expiry date earlier than the signed certificate, as a certificate can't be created with a longer validity period than its signer.

This is not necessarily the case. See https://security.stackexchange.com/a/120471