fix/remove wrong host test, failing with OpenSSL #139
Comments
|
@mkitti, can you provide details of how to repro the conda-forge test failure and what the exact output is? |
|
Sure. I'm just copying this over from conda-forge/julia-feedstock#119 (comment). The build.sh and meta.yaml can be viewed via the corresponding hyperlinks. Two of the key build parameters are To reproduce locally, clone https://github.com/conda-forge/julia-feedstock.git at b02be7bb62fde90656e6dab3c4b002f2aebb056e and run The full build log is available here: Click here to see a summary of the test errors.The errors are due to 818cd12 which is a backport of #114 for the Downloads.jl 1.4 branch (Julia 1.6.2). Specifically, the test failures occur around lines 387, 395, and 405 of Downloads.jl 1.4.1 which tests a mechanism to override host verification. This is due to only modification of only |
StefanKarpinski
added a commit
that referenced
this issue
Aug 19, 2021
Since #114, we only turn off peer verification, not host verification when the `SSL_NO_VERIFY` variables are set. This means that the last set of tests in the "SSL no verify override" testset *should* fail for `wrong.host.badssl.com`. That is not what I was seeing, however — the test was still passing — which I found puzzling but just moved on with my life at the time. It turns out that the test *does* fail if libcurl is build with OpenSSL. Since whether the test passes or not for that host depends on how things are built, this change simply skips the test (by popping the URL from the set of tested URLS for that testset).
StefanKarpinski
added a commit
that referenced
this issue
Aug 19, 2021
Since #114, we only turn off peer verification, not host verification when the `SSL_NO_VERIFY` variables are set. This means that the last set of tests in the "SSL no verify override" testset *should* fail for `wrong.host.badssl.com`. That is not what I was seeing, however — the test was still passing — which I found puzzling but just moved on with my life at the time. It turns out that the test *does* fail if libcurl is build with OpenSSL. Since whether the test passes or not for that host depends on how things are built, this change simply skips the test (by popping the URL from the set of tested URLS for that testset). The tests above that which use the easy hook mechanism are fixed in a different way: for those I made the hook disable both host and peer verification, which should fix the tests for any bad host including when the server sends the wrong host name.
|
Thanks for the debug info. I've made a change that will hopefully fix the issue. I think it could also be backported to Julia 1.6 but that will take a bunch of additional PRs. Is it possible for you to test against Julia master? I can make a Pr to update Downloads on master. |
|
Yes, I can change the build script to pull from git rather than a tarball. |
|
Just for testing. You should probably be building a real release, which we can handle once we've verified that this actually fixes the issue. |
5 tasks
ericphanson
pushed a commit
to ericphanson/Downloads.jl
that referenced
this issue
Jan 26, 2022
…liaLang#140) Since JuliaLang#114, we only turn off peer verification, not host verification when the `SSL_NO_VERIFY` variables are set. This means that the last set of tests in the "SSL no verify override" testset *should* fail for `wrong.host.badssl.com`. That is not what I was seeing, however — the test was still passing — which I found puzzling but just moved on with my life at the time. It turns out that the test *does* fail if libcurl is build with OpenSSL. Since whether the test passes or not for that host depends on how things are built, this change simply skips the test (by popping the URL from the set of tested URLS for that testset). The tests above that which use the easy hook mechanism are fixed in a different way: for those I made the hook disable both host and peer verification, which should fix the tests for any bad host including when the server sends the wrong host name.
ericphanson
pushed a commit
to ericphanson/Downloads.jl
that referenced
this issue
Jan 27, 2022
…liaLang#140) Since JuliaLang#114, we only turn off peer verification, not host verification when the `SSL_NO_VERIFY` variables are set. This means that the last set of tests in the "SSL no verify override" testset *should* fail for `wrong.host.badssl.com`. That is not what I was seeing, however — the test was still passing — which I found puzzling but just moved on with my life at the time. It turns out that the test *does* fail if libcurl is build with OpenSSL. Since whether the test passes or not for that host depends on how things are built, this change simply skips the test (by popping the URL from the set of tested URLS for that testset). The tests above that which use the easy hook mechanism are fixed in a different way: for those I made the hook disable both host and peer verification, which should fix the tests for any bad host including when the server sends the wrong host name. (cherry picked from commit e22219f)
DilumAluthge
added a commit
that referenced
this issue
Mar 3, 2022
* Before building and testing the package, make sure that the UUID has not been edited (#128) (cherry picked from commit 21843d0) * CI: Standardize the workflow for testing and changing the UUID (#129) (cherry picked from commit cd002c3) * fix #131 and add test (#132) (cherry picked from commit adbb974) * Improve inferability of download() (#133) (cherry picked from commit 848d374) * fix ci badge (#137) (cherry picked from commit 3870614) * Fix a handful of invalidations in expression-checking (#138) ChainRulesCore defines `==(a, b::AbstractThunk)` and its converse, and this invalidates a couple of poorly-typed Symbol checks. This more "SSA-like" way of writing the code is easier to infer. (cherry picked from commit 25f7af3) * tests: skip wrong host test for SSL_NO_VERIFY (fix #139) (#140) Since #114, we only turn off peer verification, not host verification when the `SSL_NO_VERIFY` variables are set. This means that the last set of tests in the "SSL no verify override" testset *should* fail for `wrong.host.badssl.com`. That is not what I was seeing, however — the test was still passing — which I found puzzling but just moved on with my life at the time. It turns out that the test *does* fail if libcurl is build with OpenSSL. Since whether the test passes or not for that host depends on how things are built, this change simply skips the test (by popping the URL from the set of tested URLS for that testset). The tests above that which use the easy hook mechanism are fixed in a different way: for those I made the hook disable both host and peer verification, which should fix the tests for any bad host including when the server sends the wrong host name. (cherry picked from commit e22219f) * Fix input body size detection for IOBuffer(codeunits(str)) (#143) Somewhat surprisingly, the type of this is not IOBuffer, but a related type (Base.GenericIOBuffer{Base.CodeUnits{UInt8, String}}). (cherry picked from commit 470b7f0) * Typo fix: indiation -> indication (#144) (cherry picked from commit 5f1509d) * use Timer instead of libuv timer API (cherry picked from commit 11493ff) * use FDWatcher instead of libuv poll API (cherry picked from commit 4c1d2af) * fix wrong definition of curl_socket_t on Windows (cherry picked from commit 2eb0491) * Revert "stop using raw libuv API" (#156) (cherry picked from commit c91876a) * Revert "Revert "stop using raw libuv API" (#156)" This reverts commit c91876a. (cherry picked from commit 69acc13) * add missing locks during Timer callbacks (cherry picked from commit 43a3484) * fix Timer usage (#158) (cherry picked from commit 62b497e) * Workaround for missing isopen check in FDWatcher (#161) (possible multithread race with this still needs to be fixed) (cherry picked from commit 7f91b8a) * Check for timer isopen correctly (#162) (cherry picked from commit 4250b35) * remove trailing whitespace (cherry picked from commit d8c626b) * Avoid infinite recursion in `timer_callback` (#164) Fixes #163 (cherry picked from commit a55825b) * should also look into headers for input_size (#167) If no content length is set while uploading some contents, Curl defaults to use chunked transfer encoding. In some cases we want to prevent that because the server may not support chunked transfers. With this change, the request method will also look at the headers while determining the input size and if found call `set_upload_size` as usual. So to switch off chunked transfers, one must also know and set the content length header while invoking `download` or `request` methods. (cherry picked from commit ab628ab) * rename: singularize add_{upload,seek}_callback These only add one callback so having them be plural is weird. (cherry picked from commit 5bd0826) * add support for setting a debug callback (cherry picked from commit 55a0c39) * end-to-end tests for #167 This adds end-to-end tests for the changes introduced in #167. Verbose mode is switched off for these tests, but switching it on would show that not setting content-length headers results in chunked transfer encoding while setting it prevents that. Both tests should pass. (cherry picked from commit 911368d) * tests: use debug option to test for non/chunked uploads This combines the functionality from the previous two commits to not only trigger both chunked and non-chunked uploads, but also test for that difference by capturing and inspecting the debug events. (cherry picked from commit 4e0408a) * bump patch Co-authored-by: Dilum Aluthge <[email protected]> Co-authored-by: Jakob Nybo Nissen <[email protected]> Co-authored-by: Yuto Horikawa <[email protected]> Co-authored-by: Tim Holy <[email protected]> Co-authored-by: Stefan Karpinski <[email protected]> Co-authored-by: Chris Foster <[email protected]> Co-authored-by: Benoît Legat <[email protected]> Co-authored-by: Jameson Nash <[email protected]> Co-authored-by: Tanmay Mohapatra <[email protected]>
Merged
StefanKarpinski
added a commit
that referenced
this issue
Mar 24, 2022
Since #114, we only turn off peer verification, not host verification when the `SSL_NO_VERIFY` variables are set. This means that the last set of tests in the "SSL no verify override" testset *should* fail for `wrong.host.badssl.com`. That is not what I was seeing, however — the test was still passing — which I found puzzling but just moved on with my life at the time. It turns out that the test *does* fail if libcurl is build with OpenSSL. Since whether the test passes or not for that host depends on how things are built, this change simply skips the test (by popping the URL from the set of tested URLS for that testset). The tests above that which use the easy hook mechanism are fixed in a different way: for those I made the hook disable both host and peer verification, which should fix the tests for any bad host including when the server sends the wrong host name. (cherry picked from commit e22219f)
StefanKarpinski
added a commit
that referenced
this issue
Mar 24, 2022
Since #114, we only turn off peer verification, not host verification when the `SSL_NO_VERIFY` variables are set. This means that the last set of tests in the "SSL no verify override" testset *should* fail for `wrong.host.badssl.com`. That is not what I was seeing, however — the test was still passing — which I found puzzling but just moved on with my life at the time. It turns out that the test *does* fail if libcurl is build with OpenSSL. Since whether the test passes or not for that host depends on how things are built, this change simply skips the test (by popping the URL from the set of tested URLS for that testset). The tests above that which use the easy hook mechanism are fixed in a different way: for those I made the hook disable both host and peer verification, which should fix the tests for any bad host including when the server sends the wrong host name. (cherry picked from commit e22219f)
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
See #114 (comment). Synopsis:
CURLOPT_SSL_VERIFYHOSTon as we do now causes the wrong host test to fail — this is what we would expect since we have told libcurl to allow hosts that it cannot verify the identity of with a root cert but not to allow connections to hosts that claim to be the wrong host entirelySimplest fix seems to be to disable that test, but it would also be good to file an upstream issue with libcurl.
The text was updated successfully, but these errors were encountered: