Support PKCS#11 tokens as keystores and truststores #34063

jkakavas · 2018-09-25T21:30:03Z

This enables Elasticsearch to use the JVM-wide configured
PKCS#11 token as a keystore or a truststore for its TLS configuration.
The JVM is assumed to be configured accordingly with the appropriate
Security Provider implementation that supports PKCS#11 tokens.
For the PKCS#11 token to be used as a keystore or a truststore for an
SSLConfiguration, the .keystore.type or .truststore.type must be
explicitly set to pkcs11 in the configuration.
The fact that the PKCS#11 token configuration is JVM wide implies that
there is only one available keystore and truststore that can be used by TLS
configurations in Elasticsearch.
The PIN for the PKCS#11 token can be set as a truststore parameter in
Elasticsearch or as a JVM parameter ( -Djavax.net.ssl.trustStorePassword).

The basic goal of enabling PKCS#11 token support is to allow PKCS#11-NSS in
FIPS mode to be used as a FIPS 140-2 enabled Security Provider.

This commit enables Elasticsearch to use the JVM-wide configured PKCS#11 token as a keystore or a truststore for its TLS configuration. The JVM is assumed to be configured accordingly with the appropriate Security Provider implementation that supports PKCS#11 tokens. For the PKCS#11 token to be used as a keystore or a truststore for an SSLConfiguration, the keystore.type or truststore.type must be explicitly set to pkcs11 accordingly in the configuration.

elasticmachine · 2018-09-25T21:30:05Z

Pinging @elastic/es-security

jkakavas · 2018-09-25T21:40:38Z

Not sure if the scope of this makes it a good candidate for 6.4.2, WDYT @jaymode ?

jkakavas · 2018-09-25T21:44:35Z

docs/reference/settings/security-settings.asciidoc

@@ -400,8 +400,9 @@ The path to the Java Keystore file that contains a private key and certificate.
 `ssl.key` and `ssl.keystore.path` may not be used at the same time.

 `ssl.keystore.type`::
-The format of the keystore file. Should be either `jks` to use the Java


The reason for calling out PKCS11 in the settings is that the implementation now includes specific handling if this is set as the keystore.type ( As opposed to for instance BCFKS which can be used with the BouncyCastle FIPS Security Provider if configured, but handled transparently )

jaymode

I left a request for a simple test. Otherwise this LGTM. IMO this is a bugfix and should go to 6.4.x

jaymode · 2018-09-26T19:45:39Z

docs/reference/settings/security-settings.asciidoc

@@ -1316,6 +1320,29 @@ a PKCS#12 container includes trusted certificate ("anchor") entries look for
 `openssl pkcs12 -info` output, or `trustedCertEntry` in the
 `keytool -list` output.

+===== PKCS#11 tokens
+
+When using a PKCS#11 cryptographic token, which contain the


s/contain/contains ?

jaymode · 2018-09-26T19:50:41Z

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

+            ks.load(null, storePassword.getChars());
+            return ks;
+        }
+        throw new IllegalArgumentException("keystore.path or truststore.path can only be empty when using a PKCS#11 token");


can you add unit tests for this method? At least one with an expectThrows for this IAE.

Done, thanks

- Adds unit test for TrustConfig#getStore - Fix typo - Do not return the certificates in the PKCS#11 token when using the SSL Certificate API

jkakavas · 2018-09-27T09:51:55Z

Failed to get resource: HEAD. [HTTP HTTP/1.1 504 Gateway Time-out: https://plugins.gradle.org/m2/org/apache/apache/13/apache-13.pom]

jkakavas · 2018-09-27T14:44:40Z

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

-                final Certificate certificate = chain[i];
-                if (certificate instanceof X509Certificate) {
-                    certificates.add(new CertificateInfo(keyStorePath, keyStoreType, alias, i == 0, (X509Certificate) certificate));
+        // Do not return certificates from the PKCS#11 token, as it is also the JRE default store


@jaymode I wanted to point out this change ( to not return the certificates of the PKCS#11 token in the get certificates API responses ) since I introduced it after your LGTM.

hmm, this seems odd since the idea is to be able to use this API to alert about certificate expiration. Wouldn't we want this if the cert in the PKCS11 token is going to expire?

jkakavas · 2018-09-28T13:00:23Z

@jaymode Can you please take a look at the additional changes?

docs/reference/settings/security-settings.asciidoc

jaymode · 2018-09-28T14:30:49Z

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

@@ -78,11 +80,11 @@ public int hashCode() {
     * @param trustConfig the trust configuration to merge with
     * @return a {@link TrustConfig} that represents a combination of both trust configurations
     */
-    static TrustConfig merge(TrustConfig trustConfig) {
+    static TrustConfig merge(TrustConfig trustConfig, SecureString trustStorePassword) {


can you add the parameter to the javadocs and include that this password is for the default jdk store?

jaymode · 2018-09-28T14:32:20Z

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

@@ -96,7 +98,10 @@ static TrustConfig merge(TrustConfig trustConfig) {
    private KeyStore getSystemTrustStore() throws KeyStoreException, CertificateException, NoSuchAlgorithmException, IOException {
        if (System.getProperty("javax.net.ssl.trustStoreType", "").equalsIgnoreCase("PKCS11")) {
            KeyStore keyStore = KeyStore.getInstance("PKCS11");
-            keyStore.load(null, System.getProperty("javax.net.ssl.trustStorePassword", "").toCharArray());
+            if (trustStorePassword.length() == 0) {


just to be paranoid add a null check?

jaymode · 2018-09-28T14:34:41Z

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

-                final Certificate certificate = chain[i];
-                if (certificate instanceof X509Certificate) {
-                    certificates.add(new CertificateInfo(keyStorePath, keyStoreType, alias, i == 0, (X509Certificate) certificate));
+        // Do not return certificates from the PKCS#11 token, as it is also the JRE default store


hmm, this seems odd since the idea is to be able to use this API to alert about certificate expiration. Wouldn't we want this if the cert in the PKCS11 token is going to expire?

jaymode · 2018-09-28T14:35:10Z

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

-        throw new IllegalArgumentException("the keystore [" + keyStorePath + "] does not contain a private key entry");
+        final String message = null != keyStorePath ?
+            "the keystore [" + keyStorePath + "] does not contain a private key entry" :
+            "the system wide configured PKCS#11 token does not contain a private key entry";


remove system wide?

jaymode · 2018-09-28T14:36:10Z

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ssl/TestsSSLService.java

+
+    public SSLContext sslContext(String context) {
+        return sslContextHolder(super.getSSLConfiguration(context)).sslContext();
+


nit: extra line

jkakavas · 2018-09-28T16:57:11Z

hmm, this seems odd since the idea is to be able to use this API to alert about certificate expiration. Wouldn't we want this if the cert in the PKCS11 token is going to expire?

You have a point. I got carried away by the fact that we used to filter all certs from the JDK default truststore. Returning everything will provide with all sorts of CA certs that the JDK ships with and/or irrelevant to Elasticsearch certificates that happened to be in the token so there is a chance of false positives. I guess we can return them all and adjust the documentation for the get certificates API ( https://www.elastic.co/guide/en/elasticsearch/reference/current/security-api-ssl.html ) so that users are aware and can filter the results as they see fit.

jaymode · 2018-09-28T18:04:36Z

I guess we can return them all and adjust the documentation for the get certificates API

That works for me since this is only for PKCS#11

- Return all certificates when using a PKCS11 token and adjust documentation to reflect that - Ensure that the truststore password defined in a system property takes precedence over the one defined in Elasticsearch config

jkakavas · 2018-10-01T10:13:59Z

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

+        }
+    }
+
+    private static SecureString getDefaultTrustStorePassword(Settings settings) {


Resolve the deafult JDK TrustStore password here for the case of PKCS11 tokens, so that we don't need to pass the trustStorePassword to the DefaultJDKTrustConfig's constructor for all other truststore types.

…TrustConfig

jaymode

LGTM

jaymode · 2018-10-02T16:52:15Z

docs/reference/settings/security-settings.asciidoc

+fallback to use he appropriate JVM setting (`-Djavax.net.ssl.trustStorePassword`)
+if that s set.
+Since there can only be one PKCS#11 token configured, only one keystore and
+truststore will be usable for configuration in Elasticsearch. This in turn means


s/Elasticsearch/{es}

jkakavas · 2018-10-03T14:02:36Z

Thanks @jaymode , I'm running a final CI run with the master merged for due diligence and will merge this on green

This enables Elasticsearch to use the JVM-wide configured PKCS#11 token as a keystore or a truststore for its TLS configuration. The JVM is assumed to be configured accordingly with the appropriate Security Provider implementation that supports PKCS#11 tokens. For the PKCS#11 token to be used as a keystore or a truststore for an SSLConfiguration, the .keystore.type or .truststore.type must be explicitly set to pkcs11 in the configuration. The fact that the PKCS#11 token configuration is JVM wide implies that there is only one available keystore and truststore that can be used by TLS configurations in Elasticsearch. The PIN for the PKCS#11 token can be set as a truststore parameter in Elasticsearch or as a JVM parameter ( -Djavax.net.ssl.trustStorePassword). The basic goal of enabling PKCS#11 token support is to allow PKCS#11-NSS in FIPS mode to be used as a FIPS 140-2 enabled Security Provider.

* master: (25 commits) HLRC: ML Adding get datafeed stats API (elastic#34271) Small fixes to the HLRC watcher documentation. (elastic#34306) Tasks: Document that status is not semvered (elastic#34270) HLRC: ML Add preview datafeed api (elastic#34284) [CI] Fix bogus ScheduleWithFixedDelayTests.testRunnableRunsAtMostOnceAfterCancellation Fix error in documentation for activete watch SCRIPTING: Terms set query expression (elastic#33856) Logging: Drop remaining Settings log ctor (elastic#34149) [ML] Remove unused last_data_time member from Job (elastic#34262) Docs: Allow skipping response assertions (elastic#34240) HLRC: Add activate watch action (elastic#33988) [Security] Multi Index Expression alias wildcard exclusion (elastic#34144) [ML] Label anomalies with multi_bucket_impact (elastic#34233) Document smtp.ssl.trust configuration option (elastic#34275) Support PKCS#11 tokens as keystores and truststores (elastic#34063) Fix sporadic failure in NestedObjectMapperTests [Authz] Allow update settings action for system user (elastic#34030) Replace version with reader cache key in IndicesRequestCache (elastic#34189) [TESTS] Set SO_LINGER and SO_REUSEADDR on the mock socket (elastic#34211) Security: upgrade unboundid ldapsdk to 4.0.8 (elastic#34247) ...

* rename-ccr-stats: (25 commits) HLRC: ML Adding get datafeed stats API (elastic#34271) Small fixes to the HLRC watcher documentation. (elastic#34306) Tasks: Document that status is not semvered (elastic#34270) HLRC: ML Add preview datafeed api (elastic#34284) [CI] Fix bogus ScheduleWithFixedDelayTests.testRunnableRunsAtMostOnceAfterCancellation Fix error in documentation for activete watch SCRIPTING: Terms set query expression (elastic#33856) Logging: Drop remaining Settings log ctor (elastic#34149) [ML] Remove unused last_data_time member from Job (elastic#34262) Docs: Allow skipping response assertions (elastic#34240) HLRC: Add activate watch action (elastic#33988) [Security] Multi Index Expression alias wildcard exclusion (elastic#34144) [ML] Label anomalies with multi_bucket_impact (elastic#34233) Document smtp.ssl.trust configuration option (elastic#34275) Support PKCS#11 tokens as keystores and truststores (elastic#34063) Fix sporadic failure in NestedObjectMapperTests [Authz] Allow update settings action for system user (elastic#34030) Replace version with reader cache key in IndicesRequestCache (elastic#34189) [TESTS] Set SO_LINGER and SO_REUSEADDR on the mock socket (elastic#34211) Security: upgrade unboundid ldapsdk to 4.0.8 (elastic#34247) ...

This enables Elasticsearch to use the JVM-wide configured PKCS#11 token as a keystore or a truststore for its TLS configuration. The JVM is assumed to be configured accordingly with the appropriate Security Provider implementation that supports PKCS#11 tokens. For the PKCS#11 token to be used as a keystore or a truststore for an SSLConfiguration, the .keystore.type or .truststore.type must be explicitly set to pkcs11 in the configuration. The fact that the PKCS#11 token configuration is JVM wide implies that there is only one available keystore and truststore that can be used by TLS configurations in Elasticsearch. The PIN for the PKCS#11 token can be set as a truststore parameter in Elasticsearch or as a JVM parameter ( -Djavax.net.ssl.trustStorePassword). The basic goal of enabling PKCS#11 token support is to allow PKCS#11-NSS in FIPS mode to be used as a FIPS 140-2 enabled Security Provider.

jkakavas added 6 commits September 25, 2018 20:57

Merge remote-tracking branch 'origin/master' into pkcs11-stores

6f6eb3a

Updated javadoc

430b998

Typo and extra new line

8a0b64e

cleanup imports

365d675

Add initial documentation

92248a1

jkakavas added >bug review v7.0.0 :Security/TLS SSL/TLS, Certificates v6.5.0 v6.4.2 labels Sep 25, 2018

jkakavas requested a review from jaymode September 25, 2018 21:30

jkakavas commented Sep 25, 2018

View reviewed changes

jkakavas mentioned this pull request Sep 25, 2018

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

Closed

jaymode approved these changes Sep 26, 2018

View reviewed changes

Address feedback

5fda332

- Adds unit test for TrustConfig#getStore - Fix typo - Do not return the certificates in the PKCS#11 token when using the SSL Certificate API

Merge remote-tracking branch 'origin/master' into pkcs11-stores

6dde2f7

jkakavas commented Sep 27, 2018

View reviewed changes

jkakavas added 3 commits September 28, 2018 01:38

Allow PKCS11 token password to be set in ES

2df67f4

Remove redundant modifier

c37c5e3

Fix watcher tests

efe542c

jaymode reviewed Sep 28, 2018

View reviewed changes

Address feedback

60da02b

- Return all certificates when using a PKCS11 token and adjust documentation to reflect that - Ensure that the truststore password defined in a system property takes precedence over the one defined in Elasticsearch config

Merge remote-tracking branch 'origin/master' into pkcs11-stores

777554b

jkakavas commented Oct 1, 2018

View reviewed changes

jkakavas added 3 commits October 1, 2018 13:15

Fix typo

41c74ac

Do not close the SecureString as it is needed for reloading the Store…

4ec50c6

…TrustConfig

Merge remote-tracking branch 'origin/master' into pkcs11-stores

ba90235

jaymode approved these changes Oct 2, 2018

View reviewed changes

jkakavas added 2 commits October 3, 2018 17:01

address feedback

88f80c3

Merge remote-tracking branch 'origin/master' into pkcs11-stores

6604aa1

jkakavas merged commit 2c82b80 into elastic:master Oct 4, 2018

jkakavas added v6.4.3 backport pending and removed v6.4.2 review labels Oct 4, 2018

jkakavas removed the backport pending label Oct 4, 2018

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support PKCS#11 tokens as keystores and truststores #34063

Support PKCS#11 tokens as keystores and truststores #34063

jkakavas commented Sep 25, 2018 •

edited

Loading

elasticmachine commented Sep 25, 2018

jkakavas commented Sep 25, 2018

jkakavas Sep 25, 2018

jaymode left a comment

jaymode Sep 26, 2018

jaymode Sep 26, 2018

jkakavas Sep 27, 2018

jkakavas commented Sep 27, 2018

jkakavas Sep 27, 2018

jaymode Sep 28, 2018

jkakavas commented Sep 28, 2018

jaymode Sep 28, 2018

jaymode Sep 28, 2018

jaymode Sep 28, 2018

jaymode Sep 28, 2018

jaymode Sep 28, 2018

jkakavas commented Sep 28, 2018

jaymode commented Sep 28, 2018

jkakavas Oct 1, 2018

jaymode left a comment

jaymode Oct 2, 2018

jkakavas commented Oct 3, 2018


		public SSLContext sslContext(String context) {
		return sslContextHolder(super.getSSLConfiguration(context)).sslContext();

Support PKCS#11 tokens as keystores and truststores #34063

Support PKCS#11 tokens as keystores and truststores #34063

Conversation

jkakavas commented Sep 25, 2018 • edited Loading

elasticmachine commented Sep 25, 2018

jkakavas commented Sep 25, 2018

Choose a reason for hiding this comment

jaymode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkakavas commented Sep 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkakavas commented Sep 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkakavas commented Sep 28, 2018

jaymode commented Sep 28, 2018

Choose a reason for hiding this comment

jaymode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkakavas commented Oct 3, 2018

jkakavas commented Sep 25, 2018 •

edited

Loading