Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Prefer https #30826

Merged
merged 2 commits into from
Feb 7, 2019
Merged

Prefer https #30826

merged 2 commits into from
Feb 7, 2019

Conversation

PallHaraldsson
Copy link
Contributor

No description provided.

@PallHaraldsson
Copy link
Contributor Author

PallHaraldsson commented Jan 24, 2019

Isn't HTTPS safer in case of man-in-the-middle attacks; and should thus be preferred in all cases I see HTTP used? At least here (confirmed file is available either way).

Some counterpoint I can think of is, if HTTPS breaks, you don't get the file that way (you'll get an exception and know?). I'm not sure it's a good reason here to avoid HTTPS, even with this only for a test. Seems the alternative is the testing infrastructure could be compromised.

@PallHaraldsson
Copy link
Contributor Author

PallHaraldsson commented Jan 24, 2019

I looked for other cases: https://github.com/JuliaLang/julia/search?q=http&unscoped_q=http

Also in Vagrant file (on page 1) and doc/Makefile and at least one .sh file (I do not feel like going through all 10 pages, mostly just for http in comments).

And there are e.g. http://httpbin.org/ip (doesn't seem important).

and for "issue 8278"

target = """71.163.72.113 - - [30/Jul/2014:16:40:55 -0700] "GET emptymind.org/thevacantwall/wp-content/uploads/2013/02/DSC_006421.jpg HTTP/1.1" 200 492513 "http://images.search.yahoo.com/images/view;_ylt=AwrB8py9gdlTGEwADcSjzbkF;_ylu=X3oDMTI2cGZrZTA5BHNlYwNmcC1leHAEc2xrA2V4cARvaWQDNTA3NTRiMzYzY2E5OTEwNjBiMjc2YWJhMjkxMTEzY2MEZ3BvcwM0BGl0A2Jpbmc-?back=http%3A%2F%2Fus.yhs4.search.yahoo.com%2Fyhs%2Fsearch%3Fei%3DUTF-8%26p%3Dapartheid%2Bwall%2Bin%2Bpalestine%26type%3Dgrvydef%26param1%3D1%26param2%3Dsid%253Db01676f9c26355f014f8a9db87545d61%2526b%253DChrome%2526ip%253D71.163.72.113%2526p%253Dgroovorio%2526x%253DAC811262A746D3CD%2526dt%253DS940%2526f%253D7%2526a%253Dgrv_tuto1_14_30%26hsimp%3Dyhs-fullyhosted_003%26hspart%3Dironsource&w=588&h=387&imgurl=occupiedpalestine.files.wordpress.com%2F2012%2F08%2F5-peeking-through-the-wall.jpg%3Fw%3D588%26h%3D387&rurl=http%3A%2F%2Fwww.stopdebezetting.com%2Fwereldpers%2Fcompare-the-berlin-wall-vs-israel-s-apartheid-wall-in-palestine.html&size=49.0KB&name=...+%3Cb%3EApartheid+wall+in+Palestine%3C%2Fb%3E...+%7C+Or+you+go+peeking+through+the+%3Cb%3Ewall%3C%2Fb%3E&p=apartheid+wall+in+palestine&oid=50754b363ca991060b276aba291113cc&fr2=&fr=&tt=...+%3Cb%3EApartheid+wall+in+Palestine%3C%2Fb%3E...+%7C+Or+you+go+peeking+through+the+%3Cb%3Ewall%3C%2Fb%3E&b=0&ni=21&no=4&ts=&tab=organic&sigr=13evdtqdq&sigb=19k7nsjvb&sigi=12o2la1db&sigt=12lia2m0j&sign=12lia2m0j&.crumb=.yUtKgFI6DE&hsimp=yhs-fullyhosted_003&hspart=ironsource" "Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36"""

there the interesting case of the image search for "apartheid wall in palestine"...:

http://images.search.yahoo.com/images/view?back=http%3A%2F%2Fus.yhs4.search.yahoo.com%2Fyhs%2Fsearch%3Fei%3DUTF-8%26p%3Dapartheid%2Bwall%2Bin%2Bpalestine%26type%3Dgrvydef%26param1%3D1%26param2%3Dsid%253Db01676f9c26355f014f8a9db87545d61%2526b%253DChrome%2526ip%253D71.163.72.113%2526p%253Dgroovorio%2526x%253DAC811262A746D3CD%2526dt%253DS940%2526f%253D7%2526a%253Dgrv_tuto1_14_30%26hsimp%3Dyhs-fullyhosted_003%26hspart%3Dironsource&w=588&h=387&imgurl=occupiedpalestine.files.wordpress.com%2F2012%2F08%2F5-peeking-through-the-wall.jpg%3Fw%3D588%26h%3D387&rurl=http%3A%2F%2Fwww.stopdebezetting.com%2Fwereldpers%2Fcompare-the-berlin-wall-vs-israel-s-apartheid-wall-in-palestine.html&size=49.0KB&name=...+%3Cb%3EApartheid+wall+in+Palestine%3C%2Fb%3E...+%7C+Or+you+go+peeking+through+the+%3Cb%3Ewall%3C%2Fb%3E&p=apartheid+wall+in+palestine&oid=50754b363ca991060b276aba291113cc&fr2=&fr=&tt=...+%3Cb%3EApartheid+wall+in+Palestine%3C%2Fb%3E...+%7C+Or+you+go+peeking+through+the+%3Cb%3Ewall%3C%2Fb%3E&b=0&ni=21&no=4&ts=&tab=organic&sigr=13evdtqdq&sigb=19k7nsjvb&sigi=12o2la1db&sigt=12lia2m0j&sign=12lia2m0j&.crumb=.yUtKgFI6DE&hsimp=yhs-fullyhosted_003&hspart=ironsource

@staticfloat
Copy link
Member

Keeping the httpbin stuff to HTTP actually serves a purpose; we want these to be interceptable so that we can debug them when things go wrong. :)

If you want to add changes for the Vagrantfile, I'll be happy to take a look at that too.

@StefanKarpinski
Copy link
Member

Should we have a checksum for the busybox.exe download then? Otherwise we're executing arbitrary code provided by a potential MITM attacker.

@staticfloat
Copy link
Member

That executable can change at any time; it's not versioned. We should probably host our own version, add a checksum, and use that URL instead (with HTTPS).

@PallHaraldsson
Copy link
Contributor Author

PallHaraldsson commented Jan 25, 2019

I changed all three but only confirmed links for other two [now three], not: https://sourceforge.net/projects/msys2/files/Base/x86_64/msys2-base-x86_64-20150916.tar.xz

-s http://mirrors.kernel.org/sourceware/cygwin -g -P $pkg | Where-Object `
[EDIT: wrong copy paste, the striked out seemed to work.]

At least https://mirrors.kernel.org/sourceware/cygwin/ works so all links to that host should also work.

@staticfloat
Copy link
Member

This looks good to me for now, it's clearly an improvement. We should move toward versioned, hash-checked downloads for pretty much everything, I'll open an issue to track that independent of this PR.

@PallHaraldsson
Copy link
Contributor Author

With my seemingly trivial change passing on Linux, I believe the macOS CI fail is a false alarm, for something entirely unrelated and this PR should just be merged:

failed to clone from https://github.com/JuliaLang/Example.jl.git, error: GitError(Code:ERROR, Class:Net, curl error: Could not resolve host: github.com 

[..]

Error in testset Pkg/pkg:
Test Failed at /private/tmp/julia/share/julia/stdlib/v1.2/Pkg/test/pkg.jl:187
  Expression: ((Pkg.API).__installed())[TEST_PKG.name] == old_v
   Evaluated: v"0.5.1" == v"0.3.3"
ERROR: LoadError: Test run finished with errors
in expression starting at /private/tmp/julia/share/julia/test/runtests.jl:61
The command "if [ $(echo "$FILES_CHANGED" | grep -cv '^doc/') -gt 0 ]; then /tmp/julia/bin/julia --check-bounds=yes runtests.jl $TESTSTORUN && /tmp/julia/bin/julia --check-bounds=yes runtests.jl LibGit2/online Pkg/pkg download; fi" exited with 1.

@PallHaraldsson
Copy link
Contributor Author

Bump. Just merge as CI false alarm "Probably a temporary connection problem"?

@staticfloat staticfloat merged commit 0325220 into JuliaLang:master Feb 7, 2019
@staticfloat
Copy link
Member

Yep. Thanks!

@PallHaraldsson PallHaraldsson deleted the patch-24 branch February 13, 2019 14:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants