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

TLS: Introduce *-alias to select specific key to use (or cert to trust) #17884

Merged
merged 1 commit into from
Jun 16, 2021
Merged

TLS: Introduce *-alias to select specific key to use (or cert to trust) #17884

merged 1 commit into from
Jun 16, 2021

Conversation

famod
Copy link
Member

@famod famod commented Jun 13, 2021

Resolves #17424

Notes on store modifications in integration-tests/vertx-http:

  • added key pair to server-keystore.jks with alias "anotherserver" (which comes before the existing "server" pair)
  • added key pair to new client-keystore-2.jks and added its cert chain to server-truststore.jks with alias "mykey-2"
  • renamed existing "mykey" to "mykey-1" in server-truststore.jks
  • renamed client-keystore.jks to client-keystore-1.jks

More notes:

  • I unified the creation of KeyStoreOptions in the recorder
  • I went with the parameter naming suggestion but maybe those names are a bit misleading because those are aliases for entries in stores, not aliases of stores, but then again I might be splitting hairs here

@quarkus-bot
Copy link

quarkus-bot bot commented Jun 14, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 68bedec

Status Name Step Test failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 16 Build Test failures Logs Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 16 #

📦 extensions/vertx-http/deployment

io.quarkus.vertx.http.testrunner.TestChangeTrackingWhenStartFailsTestCase.testChangeTrackingOnStartupFailure line 42 - More details - Source on GitHub

@famod
Copy link
Member Author

famod commented Jun 14, 2021

I had a talk with my colleague @mickroll and I think this parameter naming question becomes more relevant:
In JBoss/WildFly, Spring Boot etc. you can also specify a password for the key (not the store!) and I'm planning on requesting this in vert.x (and adding it to Quarkus once it has been released).
This would lead to a conflict with key-store-password if key-store-alias stays like this.

Maybe it would be better to have:

  • key-store-key-alias
  • key-store-key-password

?
Or even a "sub-section":

  • key-store-key.alias
  • key-store-key.password

?
Another approach would be to not use key-store- prefix at all for this...

WDYT?

@sberyozkin
Copy link
Member

@famod

    key-store-key-alias
    key-store-key-password

are definitely better, and for this PR in particular, key-store-key-alias as it is exactly what it is, thanks

@famod
Copy link
Member Author

famod commented Jun 15, 2021

Remarks have been addressed. Btw, I went with trust-store-cert-alias for the trust store param.

Resolves #17424

Notes on store modifications in integration-tests/vertx-http:
- added key pair to server-keystore.jks with alias "anotherserver" (which comes before the existing "server" pair)
- added key pair to new client-keystore-2.jks and added its cert chain to server-truststore.jks with alias "mykey-2"
- renamed existing "mykey" to "mykey-1" in server-truststore.jks
- renamed client-keystore.jks to client-keystore-1.jks
@quarkus-bot
Copy link

quarkus-bot bot commented Jun 15, 2021

This workflow status is outdated as a new workflow run has been triggered.

🚫 This workflow run has been cancelled.

Failing Jobs - Building c06b35d

⚠️ Artifacts of the workflow run were not available thus the report misses some details.

Status Name Step Test failures Logs Raw logs
Initial JDK 11 Build Build ⚠️ Check → Logs Raw logs

@famod famod changed the title TLS: Introduce *-store-alias to select specific key to use (or cert to trust) TLS: Introduce *-alias to select specific key to use (or cert to trust) Jun 15, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Jun 15, 2021

Failing Jobs - Building 0f68fa1

Status Name Step Test failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 11 Windows Build Test failures Logs Raw logs
✔️ JVM Tests - JDK 16

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 integration-tests/resteasy-reactive-rest-client

io.quarkus.it.rest.client.BasicTest.shouldMakeJsonRequest line 40 - More details - Source on GitHub

@sberyozkin
Copy link
Member

Looks it is OK to merge, Clement @cescoffier has also approved, the failing test on Windows is not related to this PR

@sberyozkin sberyozkin merged commit 6a7c162 into quarkusio:main Jun 16, 2021
@quarkus-bot quarkus-bot bot added this to the 2.1 - main milestone Jun 16, 2021
@famod famod deleted the store-aliases branch June 16, 2021 10:42
@gsmet gsmet modified the milestones: 2.1 - main, 2.0.0.Final Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to support selection of SSL key in JKS keystore using an alias property
4 participants