Switch to GH-Actions #634
Merged
Switch to GH-Actions #634
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tests are failing due to SSL errors, which are caused by invalid config and/or incorrectly generated certificates. As we do some hackety hackery to mimic different ssl weirdiness, it might be the problem of misconfiguration, but I can't figure out what exactly isn't correct. |
|
I tend to disable failing specs temporarily and address those as a separate pr (help is super welcomed). @tarcieri wdyt? |
tarcieri
approved these changes
Dec 20, 2020
* Use GitHub Actions for CI * Update Coveralls integration: generate lcov report with SimpleCov and send it after the test suite using coveralls GitHub Actions plugin * Update and cleanup RSpec config * Cleanup Rakefile * Remove active_model dependency (certificate_authority was fixed) PS: GH Actions syntax is ugly. Should we switch to Cirlce CI or GitLab CI? XD Resolves: #633
446845d to
ddaddd2
Compare
Specs are failing due to some misconfiguration caused by new OpenSSL. TODO: #627
ddaddd2 to
2ae6a71
Compare
We were not testing those on Travis-CI, thus to simplify migration I've deicded to disable those. Once everything is fixed and stabilized we will add those too.
Just to make sure we're using expected ones
- use stub_const - consistently normalize URIs
/cc @tarcieri Probably we should just add dependency on gem itself?
This reverts commit f8dfb38.
jRuby is using outdated openssl gem bundled in, which don't have validate_hostname getter on SSLContext.
tarcieri
reviewed
Dec 28, 2020
| @@ -36,8 +36,9 @@ def start_tls(host, ssl_socket_class, ssl_context) | |||
| connect_ssl | |||
|
|
|||
| return unless ssl_context.verify_mode == OpenSSL::SSL::VERIFY_PEER | |||
| return unless ssl_context.respond_to?(:verify_hostname) && ssl_context.verify_hostname | |||
There was a problem hiding this comment.
In hindsight, this should probably have been:
return if ssl_context.respond_to?(:verify_hostname) && !ssl_context.verify_hostname...so as to only support explicitly disabling hostname verification. As is, it seems hostname verification won't be performed on JRuby at all.
There was a problem hiding this comment.
Opened #636 as a follow-up to this comment.
tarcieri
added a commit
that referenced
this pull request
Dec 28, 2020
Following up on this comment: #634 (review) The previous logic skipped hostname verification entirely if the `verify_hostname` method is not defined for `OpenSSL::SSL::SSLContext`, which is currently the case for JRuby. This commit changes the logic so if that method is undefined, hostname verification is still performed. Otherwise, hostname verification would always be skipped on Rubies which don't define a `verify_hostname` method. Note that this was *just* introduced in #634 which was merged 10 hours ago, so I think this was caught quickly enough simply correcting it suffices and there isn't additional security-related followup here (e.g. CVE)
tarcieri
added a commit
that referenced
this pull request
Dec 28, 2020
Following up on this comment: #634 (review) The previous logic skipped hostname verification entirely if the `verify_hostname` method is not defined for `OpenSSL::SSL::SSLContext`, which is currently the case for JRuby. This commit changes the logic so if that method is undefined, hostname verification is still performed. Otherwise, hostname verification would always be skipped on Rubies which don't define a `verify_hostname` method. Note that this was *just* introduced in #634 which was merged 10 hours ago, so I think this was caught quickly enough simply correcting it suffices and there isn't additional security-related followup here (e.g. CVE)
|
Thanks for the fix |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: #633