Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace custom reloadable Key/TrustManager #30509

Merged
merged 5 commits into from May 16, 2018

Conversation

Projects
None yet
7 participants
@jkakavas
Copy link
Contributor

commented May 10, 2018

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.

jkakavas added some commits May 10, 2018

Make SSLContext reloadable
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

This comment has been minimized.

Copy link

commented May 10, 2018

@elasticmachine elasticmachine referenced this pull request May 10, 2018

Closed

Security: FIPS 140-2 Compliance roadmap #29851

12 of 12 tasks complete
@jkakavas

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2018

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

@jtibshirani

This comment has been minimized.

Copy link
Member

commented May 10, 2018

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

@jkakavas

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2018

run gradle build tests

@jaymode
Copy link
Member

left a comment

I left some minor comments. Otherwise LGTM

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

This comment has been minimized.

Copy link
@jaymode

jaymode May 14, 2018

Member

can you move this statement to a single line now?

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

This comment has been minimized.

Copy link
@jaymode

jaymode May 14, 2018

Member

can you move this statement to a single line now?

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

This comment has been minimized.

Copy link
@jaymode

jaymode May 14, 2018

Member

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

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")) {

This comment has been minimized.

Copy link
@jaymode

jaymode May 14, 2018

Member

space between try and (

@jkakavas

This comment has been minimized.

Copy link
Contributor Author

commented May 15, 2018

run gradle build tests

@tvernum
Copy link
Contributor

left a comment

LGTM

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

3 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details

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

jkakavas added a commit that referenced this pull request May 16, 2018

Replace custom reloadable Key/TrustManager (#30509) (#30639)
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.

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 16, 2018

Merge remote-tracking branch 'elastic/master' into default-copy-setti…
…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)
  ...

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…
…e-dependency

and resove conflicts that where introduced by the
merge of elastic#30509

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

Refactor SSLConfigurationReloaderTests
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.

@lcawl lcawl added the >enhancement label Aug 20, 2018

@jkakavas jkakavas deleted the jkakavas:reloadable-sslcontext branch Sep 14, 2018

tvernum added a commit to tvernum/elasticsearch that referenced this pull request Dec 21, 2018

Reload SSL context on file change for LDAP
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

tvernum added a commit that referenced this pull request Dec 28, 2018

Reload SSL context on file change for LDAP (#36937)
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

original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Dec 28, 2018

Reload SSL context on file change for LDAP (elastic#36937)
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

tvernum added a commit that referenced this pull request Jan 4, 2019

Reload SSL context on file change for LDAP (#36937)
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

tvernum added a commit that referenced this pull request Jan 4, 2019

Reload SSL context on file change for LDAP (#36937)
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

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

jaymode added a commit to jaymode/elasticsearch that referenced this pull request Feb 26, 2019

Fix SSLConfigurationReloaderTests failure tests
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.