Replace custom reloadable Key/TrustManager by jkakavas · Pull Request #30509 · elastic/elasticsearch

jkakavas · 2018-05-10T08:25:20Z

In JVMs running in FIPS approved mode, only SunJSSE TrustManagers
and KeyManagers can be used. This commit replaces all the custom
KeyManagers and TrustManagers (ReloadableKeyManager,
ReloadableTrustManager, EmptyKeyManager, EmptyTrustManager) with
instances of X509ExtendedKeyManager and X509ExtendedTrustManager.
Reloadability is now ensured by a volatile instance of SSLContext
in SSLContectHolder.
SSLConfigurationReloaderTests use the reloadable SSLContext to
initialize HTTP Clients and Servers and use these for testing the
key material and trust relations.

In JVMs running in FIPS approved mode, only SunJSSE TrustManagers and KeyManagers can be used. This commit replaces all the custom KeyManagers and TrustManagers (ReloadableKeyManager, ReloadableTrustManager, EmptyKeyManager, EmptyTrustManager) with instances of X509ExtendedKeyManager and X509ExtendedTrustManager. Reloadability is now ensured by a volatile instance of SSLContext in SSLContectHolder. SSLConfigurationReloaderTests use the reloadable SSLContext to initialize HTTP Clients and Servers and use these for testing the key material and trust relations.

elasticmachine · 2018-05-10T08:25:22Z

Pinging @elastic/es-security

jkakavas · 2018-05-10T13:00:29Z

CI fails because of #30507, will rerun tests once it is resolved.

jtibshirani · 2018-05-10T17:29:19Z

@jkakavas I just disabled the failing test, so I think you should be able to re-run CI: 1112fac

jkakavas · 2018-05-10T17:51:35Z

run gradle build tests

jaymode

I left some minor comments. Otherwise LGTM

jaymode · 2018-05-14T18:59:28Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java

-                new ReloadableTrustManager(sslConfiguration.trustConfig().createTrustManager(env), sslConfiguration.trustConfig());
-        ReloadableX509KeyManager keyManager =
-                new ReloadableX509KeyManager(sslConfiguration.keyConfig().createKeyManager(env), sslConfiguration.keyConfig());
+        X509ExtendedTrustManager trustManager =


can you move this statement to a single line now?

jaymode · 2018-05-14T18:59:35Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java

-                new ReloadableX509KeyManager(sslConfiguration.keyConfig().createKeyManager(env), sslConfiguration.keyConfig());
+        X509ExtendedTrustManager trustManager =
+                sslConfiguration.trustConfig().createTrustManager(env);
+        X509ExtendedKeyManager keyManager =


can you move this statement to a single line now?

jaymode · 2018-05-14T19:05:50Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java

+                SSLContext loadedSslContext = SSLContext.getInstance(sslContextAlgorithm(sslConfiguration.supportedProtocols()));
+                loadedSslContext.init(new X509ExtendedKeyManager[]{loadedKeyManager},
+                    new X509ExtendedTrustManager[]{loadedTrustManager}, null);
+                supportedCiphers(loadedSslContext.getSupportedSSLParameters().getCipherSuites(), sslConfiguration.cipherSuites(), true);


I do not think we should log here. This is on the reload of a file and not an update to the ciphers settings

jaymode · 2018-05-14T19:06:28Z

...lugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/SSLConfigurationReloaderTests.java

-                final KeyPair keyPair = CertUtils.generateKeyPair(512);
-                X509Certificate cert = CertUtils.generateSignedCertificate(new X500Principal("CN=testReloadingKeyStore"), null, keyPair,
+        //Load HTTPClient only once. Client uses the same store as a truststore
+        try(CloseableHttpClient client = getSSLClient(keystorePath, "testnode")) {


space between try and (

jkakavas · 2018-05-15T10:59:11Z

run gradle build tests

tvernum

LGTM

Make SSLContext reloadable This commit replaces all customKeyManagers and TrustManagers (ReloadableKeyManager,ReloadableTrustManager, EmptyKeyManager, EmptyTrustManager) with instances of X509ExtendedKeyManager and X509ExtendedTrustManager. This change was triggered by the effort to allow Elasticsearch to run in a FIPS-140 environment. In JVMs running in FIPS approved mode, only SunJSSE TrustManagers and KeyManagers can be used. Reloadability is now ensured by a volatile instance of SSLContext in SSLContectHolder. SSLConfigurationReloaderTests use the reloadable SSLContext to initialize HTTP Clients and Servers and use these for testing the key material and trust relations.

…ngs-to-true * elastic/master: (34 commits) Test: increase search logging for LicensingTests Adjust serialization version in IndicesOptions [TEST] Fix compilation Remove version argument in RangeFieldType (elastic#30411) Remove unused DirectoryUtils class. (elastic#30582) Mitigate date histogram slowdowns with non-fixed timezones. (elastic#30534) Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (elastic#29594) Removes AwaitsFix on IndicesOptionsTests Template upgrades should happen in a system context (elastic#30621) Fix bug in BucketMetrics path traversal (elastic#30632) Fixes IndiceOptionsTests to serialise correctly (elastic#30644) S3 repo plugin populate SettingsFilter (elastic#30652) mute IndicesOptionsTests.testSerialization Rest High Level client: Add List Tasks (elastic#29546) Refactors ClientHelper to combine header logic (elastic#30620) [ML] Wait for ML indices in rolling upgrade tests (elastic#30615) Watcher: Ensure secrets integration tests also run triggered watch (elastic#30478) Move allocation awareness attributes to list setting (elastic#30626) [Docs] Update code snippet in has-child-query.asciidoc (elastic#30510) Replace custom reloadable Key/TrustManager (elastic#30509) ...

…e-dependency and resove conflicts that where introduced by the merge of elastic#30509

Based on the changes to key and trust material reloading that were introduced in elastic#30509. DSA and EC keys are regenerated and the associated certificates are constructed with the correct SAN.

In elastic#30509 we changed the way SSL configuration is reloaded when the content of a file changes. As a consequence of that implementation change the LDAP realm ceased to pick up changes to CA files (or other certificate material) if they changed. This commit repairs the reloading behaviour for LDAP realms, and adds a test for this functionality. Resolves: elastic#36923

In #30509 we changed the way SSL configuration is reloaded when the content of a file changes. As a consequence of that implementation change the LDAP realm ceased to pick up changes to CA files (or other certificate material) if they changed. This commit repairs the reloading behaviour for LDAP realms, and adds a test for this functionality. Resolves: #36923

This change fixes the tests that expect the reload of a SSLConfiguration to fail after changes made in elastic#30509. The tests relied on the behavior that an SSLConfiguration only had reload called on it after the key and trust managers had been created, but that is no longer the case. This change removes the fail call with a wrapped call to the original method and captures the exception and counts down a latch to make these tests consistently tested rather than relying on concurrency to catch a failure. Closes elastic#39260

jkakavas added 3 commits May 10, 2018 10:55

Merge remote-tracking branch 'origin/master' into reloadable-sslcontext

70f7792

Reduce visibility, leftover from test

9234de3

jkakavas added review v7.0.0 :Security/TLS SSL/TLS, Certificates v6.4.0 labels May 10, 2018

jkakavas requested review from albertzaharovits, jaymode and tvernum May 10, 2018 08:25

elasticmachine mentioned this pull request May 10, 2018

Security: FIPS 140-2 Compliance roadmap #29851

Closed

12 tasks

jaymode approved these changes May 14, 2018

View reviewed changes

jkakavas added 2 commits May 15, 2018 08:02

Merge remote-tracking branch 'origin/master' into reloadable-sslcontext

b8f356d

Address feedback

01fc740

tvernum approved these changes May 16, 2018

View reviewed changes

jkakavas merged commit 2b09e90 into elastic:master May 16, 2018

jkakavas added backport pending and removed review labels May 16, 2018

jkakavas mentioned this pull request May 16, 2018

Replace custom reloadable Key/TrustManager (#30509) #30639

Merged

jkakavas removed the backport pending label May 17, 2018

jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request May 17, 2018

Merge remote-tracking branch 'origin/master' into remove-bouncy-castl…

36620c5

…e-dependency and resove conflicts that where introduced by the merge of elastic#30509

lcawl added the >enhancement label Aug 20, 2018

jkakavas mentioned this pull request Sep 11, 2018

Issues with using NSS PKCS#11 Provider in FIPS mode #33459

Closed

jkakavas deleted the reloadable-sslcontext branch September 14, 2018 06:57

tvernum mentioned this pull request Dec 21, 2018

Reload SSL context on file change for LDAP #36937

Merged

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

jaymode mentioned this pull request Feb 26, 2019

Fix SSLConfigurationReloaderTests failure tests #39408

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace custom reloadable Key/TrustManager#30509

Replace custom reloadable Key/TrustManager#30509
jkakavas merged 5 commits intoelastic:masterfrom
jkakavas:reloadable-sslcontext

jkakavas commented May 10, 2018

Uh oh!

elasticmachine commented May 10, 2018

Uh oh!

jkakavas commented May 10, 2018

Uh oh!

jtibshirani commented May 10, 2018 •

edited

Loading

Uh oh!

jkakavas commented May 10, 2018

Uh oh!

jaymode left a comment

Uh oh!

jaymode May 14, 2018

Uh oh!

jaymode May 14, 2018

Uh oh!

jaymode May 14, 2018

Uh oh!

jaymode May 14, 2018

Uh oh!

jkakavas commented May 15, 2018

Uh oh!

tvernum left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

jkakavas commented May 10, 2018

Uh oh!

elasticmachine commented May 10, 2018

Uh oh!

jkakavas commented May 10, 2018

Uh oh!

jtibshirani commented May 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkakavas commented May 10, 2018

Uh oh!

jaymode left a comment

Choose a reason for hiding this comment

Uh oh!

jaymode May 14, 2018

Choose a reason for hiding this comment

Uh oh!

jaymode May 14, 2018

Choose a reason for hiding this comment

Uh oh!

jaymode May 14, 2018

Choose a reason for hiding this comment

Uh oh!

jaymode May 14, 2018

Choose a reason for hiding this comment

Uh oh!

jkakavas commented May 15, 2018

Uh oh!

tvernum left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jtibshirani commented May 10, 2018 •

edited

Loading