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

Correctly handle PKCS#11 tokens for system keystore #33460

Merged
merged 4 commits into from Sep 10, 2018

Conversation

@jkakavas
Copy link
Contributor

jkakavas commented Sep 6, 2018

  • Ensure we handle the NONE keyword for system keystores correctly

    As defined in the PKCS#11 reference guide
    https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html
    PKCS#11 tokens can be used as the JSSE keystore and truststore and
    the way to indicate this is to set javax.net.ssl.keyStore and
    javax.net.ssl.trustStore to NONE (case sensitive).
    This commits ensures that we honor this convention and do not
    attempt to load the keystore or truststore if the system property is
    set to NONE.

  • Ensure we handle password protected PKCS#11 tokens used as
    system truststores correctly.

    When a PKCS#11 token is used as the system truststore, we need to
    pass a password when loading it, even if only for reading
    certificate entries. This commit ensures that if
    javax.net.ssl.trustStoreType is set to PKCS#11 (as it would
    when a PKCS#11 token is in use) the password specified in
    javax.net.ssl.trustStorePassword is passed when attempting to
    load the truststore.

Relates #33459

As defined in the PKCS#11 reference guide
https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html
PKCS#11 tokens can be used as the JSSE keystore and truststore and
the way to indicate this is to set `javax.net.ssl.keyStore` and
`javax.net.ssl.trustStore` to `NONE` (case sensitive).

This commits ensures that we honor this convention and do not
attempt to load the keystore or truststore if the system property is
set to NONE.

Relates #33459
@elasticmachine

This comment has been minimized.

Copy link
Collaborator

elasticmachine commented Sep 6, 2018

@jaymode
jaymode approved these changes Sep 6, 2018
Copy link
Member

jaymode left a comment

LGTM

When a PKCS#11 token is used as the system truststore, we need to
pass a password when loading it, even if only for reading
certificate entries. This commit ensures that if
`javax.net.ssl.trustStoreType` is set to `PKCS#11` (as it would
when a PKCS#11 token is in use) the password specified in
`javax.net.ssl.trustStorePassword` is passed when attempting to
load the truststore.
@jkakavas jkakavas changed the title Correctly handle NONE keyword for system keystore Correctly handle PKCS#11 tokens for system keystore Sep 6, 2018
@jkakavas

This comment has been minimized.

Copy link
Contributor Author

jkakavas commented Sep 6, 2018

@jaymode I widened the scope of this PR slightly as the changes fitted well together, could you take another look please ?

@jaymode
jaymode approved these changes Sep 6, 2018
Copy link
Member

jaymode left a comment

LGTM

@jkakavas

This comment has been minimized.

Copy link
Contributor Author

jkakavas commented Sep 6, 2018

CI failed with

FAILURE 0.03s J1 | DateTimeUnitTests.testConversion <<< FAILURES!
   > Throwable #1: java.lang.AssertionError: 
   > Expected: is <783512977353L>
   >      but: was <783509377353L>
   > 	at __randomizedtesting.SeedInfo.seed([A473697370F6B495:A27BEC495D39EB01]:0)
   > 	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
   > 	at org.elasticsearch.common.rounding.DateTimeUnitTests.testConversion(DateTimeUnitTests.java:79)
   > 	at java.lang.Thread.run(Thread.java:748)
Completed [958/1128] on J1 in 0.04s, 2 tests, 1 failure <<< FAILURES!

This has never failed in the past , this looks unrelated to this PR and possibly related to #32641 (ping @spinscale for visibility), although it doesn't reproduce locally.

./gradlew :server:test -Dtests.seed=A473697370F6B495 -Dtests.class=org.elasticsearch.common.rounding.DateTimeUnitTests -Dtests.method="testConversion" -Dtests.security.manager=true -Dtests.locale=en-NZ -Dtests.timezone=Pacific/Honolulu -Dcompiler.java=10 -Druntime.java=8
@jkakavas

This comment has been minimized.

Copy link
Contributor Author

jkakavas commented Sep 6, 2018

Jenkins test this please

@jkakavas jkakavas merged commit 77aeeda into elastic:master Sep 10, 2018
4 checks passed
4 checks passed
CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details
jkakavas added a commit that referenced this pull request Sep 10, 2018
* Correctly handle NONE keyword for system keystore

As defined in the PKCS#11 reference guide
https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html
PKCS#11 tokens can be used as the JSSE keystore and truststore and
the way to indicate this is to set `javax.net.ssl.keyStore` and
`javax.net.ssl.trustStore` to `NONE` (case sensitive).

This commits ensures that we honor this convention and do not
attempt to load the keystore or truststore if the system property is
set to NONE.

* Handle password protected system truststore

When a PKCS#11 token is used as the system truststore, we need to
pass a password when loading it, even if only for reading
certificate entries. This commit ensures that if
`javax.net.ssl.trustStoreType` is set to `PKCS#11` (as it would
when a PKCS#11 token is in use) the password specified in
`javax.net.ssl.trustStorePassword` is passed when attempting to
load the truststore.

Relates #33459
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 11, 2018
* master:
  Add full cluster restart base class (elastic#33577)
  Validate list values for settings (elastic#33503)
  Copy and validatie soft-deletes setting on resize (elastic#33517)
  Test: Fix package name
  SQL: Fix result column names for arithmetic functions (elastic#33500)
  Upgrade to latest Lucene snapshot (elastic#33505)
  Enable not wiping cluster settings after REST test (elastic#33575)
  MINOR: Remove Dead Code in SearchScript (elastic#33569)
  [Test] Remove duplicate method in TestShardRouting (elastic#32815)
  mute test on windows
  Update beats template to include apm-server metrics (elastic#33286)
  Fix typos (elastic#33499)
  [CCR] Delay auto follow license check (elastic#33557)
  [CCR] Add create_follow_index privilege (elastic#33559)
  Strengthen FilterRoutingTests (elastic#33149)
  Correctly handle PKCS#11 tokens for system keystore (elastic#33460)
  Remove some duplicate request conversion methods. (elastic#33538)
@jkakavas jkakavas deleted the jkakavas:pkcs11-fix branch Sep 14, 2018
@jkakavas jkakavas added v6.4.3 and removed v6.4.2 labels Oct 5, 2018
jkakavas added a commit that referenced this pull request Oct 5, 2018
* Correctly handle NONE keyword for system keystore

As defined in the PKCS#11 reference guide
https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html
PKCS#11 tokens can be used as the JSSE keystore and truststore and
the way to indicate this is to set `javax.net.ssl.keyStore` and
`javax.net.ssl.trustStore` to `NONE` (case sensitive).

This commits ensures that we honor this convention and do not
attempt to load the keystore or truststore if the system property is
set to NONE.

* Handle password protected system truststore

When a PKCS#11 token is used as the system truststore, we need to
pass a password when loading it, even if only for reading
certificate entries. This commit ensures that if
`javax.net.ssl.trustStoreType` is set to `PKCS#11` (as it would
when a PKCS#11 token is in use) the password specified in
`javax.net.ssl.trustStorePassword` is passed when attempting to
load the truststore.

Relates #33459
@jimczi jimczi added the v7.0.0-beta1 label Feb 7, 2019
@jimczi jimczi removed the v7.0.0 label Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.