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

[jruby] support a truststore option #2849

Merged
merged 7 commits into from
Apr 9, 2022
Merged

Conversation

kares
Copy link
Contributor

@kares kares commented Mar 30, 2022

which might be a completely different file than the keystore ...

due backwards compatibility we assume truststore = keystore
(truststore_pass = keystore_pass)

Description

Since the SSL implementations are different on MRI vs JRuby and the JRuby versions follows Java convention, in terms of using a keystore, this PR takes the convention used in Java one step further to have a separate trust-store (with trusted certificates) and key-store which is expected to hold the private key related material.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

which might be a completely different file than keystore ...

due backwards compatibility we assume `truststore = keystore`
(`truststore_pass = keystore_pass`)
Rakefile Show resolved Hide resolved
Copy link
Member

@nateberkopec nateberkopec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concerned that test would be green on master - do we have a way of specifically testing a cert that would fail unless the trust store was set?

@nateberkopec nateberkopec added the waiting-for-changes Waiting on changes from the requestor label Apr 2, 2022
@nateberkopec nateberkopec added waiting-for-review Waiting on review from anyone and removed waiting-for-changes Waiting on changes from the requestor labels Apr 6, 2022
@nateberkopec nateberkopec merged commit ceb4c56 into puma:master Apr 9, 2022
@nateberkopec
Copy link
Member

Thanks @kares!

kares added a commit to kares/puma that referenced this pull request May 23, 2022
MSP-Greg pushed a commit that referenced this pull request May 30, 2022
* [jruby] refactor - only keep peer cert around

* [jruby] make miss an error not to be caught!

* [test] follow-up proper testing of GH-2849

* [jruby] support truststore = :default

* [jruby] sync dsl/context-builder with new props
@MSP-Greg MSP-Greg added ssl jruby and removed waiting-for-review Waiting on review from anyone labels Jun 1, 2022
@MSP-Greg MSP-Greg mentioned this pull request Aug 26, 2022
JuanitoFatas pushed a commit to JuanitoFatas/puma that referenced this pull request Sep 9, 2022
* [jruby] support a truststore option

which might be a completely different file than keystore ...

due backwards compatibility we assume `truststore = keystore`
(`truststore_pass = keystore_pass`)

* [jruby] actually use truststore on initialize

* [jruby] add keystore_type and truststore_type

* [jruby] dry and simplify native bits

* [jruby] setup SSLError in native (like C part)

* [jruby] map to SSLError from native exception

* [jruby] provide peercert even if hand-shake fails
JuanitoFatas pushed a commit to JuanitoFatas/puma that referenced this pull request Sep 9, 2022
* [jruby] refactor - only keep peer cert around

* [jruby] make miss an error not to be caught!

* [test] follow-up proper testing of pumaGH-2849

* [jruby] support truststore = :default

* [jruby] sync dsl/context-builder with new props
@ahorek ahorek mentioned this pull request Mar 29, 2023
7 tasks
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.

None yet

3 participants