search

rmyorston / busybox-w32

WIN32 native port of BusyBox.
https://frippery.org/busybox
Other
670 stars 124 forks source link

Cannot download busybox from https://frippery.org/busybox with curl.exe 8.4.0 #388

Closed hemnstill closed 5 months ago

hemnstill commented 5 months ago

Cannot download busybox with curl 8.4.0.

Windows 10 contains native curl.exe v8.4.0. Also it can be download here: https://curl.se/windows/dl-8.4.0_1/curl-8.4.0_1-win64-mingw.zip.

curl --version:

curl 8.4.0 (Windows) libcurl/8.4.0 Schannel WinIDN
Release-Date: 2023-10-11
Protocols: dict file ftp ftps http https imap imaps pop3 pop3s smtp smtps telnet tftp
Features: AsynchDNS HSTS HTTPS-proxy IDN IPv6 Kerberos Largefile NTLM SPNEGO SSL SSPI threadsafe Unicode UnixSockets

curl --location https://frippery.org/files/busybox/busybox64.exe:

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>403 Forbidden</title>
</head><body>
<h1>Forbidden</h1>
<p>You don't have permission to access this resource.</p>
<hr>
<address>Apache/2.4.38 (Debian) Server at frippery.org Port 443</address>
</body></html>

There are no download problems with other versions https://curl.se/windows/dl-8.3.0_1/curl-8.3.0_1-win64-mingw.zip https://curl.se/windows/dl-8.6.0_3/curl-8.6.0_3-win64-mingw.zip

A few days ago everything worked.

rmyorston commented 5 months ago

Should be OK now.

avih commented 5 months ago

Not sure if expected or not, but using this very same binary, wget doesn't seem to succeed in getting it:

$ ./busybox64.exe wget https://frippery.org/files/busybox/busybox64.exe
Connecting to frippery.org (93.93.131.127:443)
ssl_client: TLS error from peer (alert code 80): internal error
wget: error getting response
rmyorston commented 5 months ago

The wget problem is different.

The internal implementation of TLS is buggy (see, for example bug 15679). Since upstream, by default, uses OpenSSL they don't seem to feel much pressure to fix it.

busybox-w32, OTOH, does use the internal TLS and suffers as a result.

avih commented 5 months ago

The internal implementation of TLS is buggy (see, for example bug 15679)

Interesting.

In my own build, using gcc 13.2.0, I get the same error as posted above (TLS error... alert code 80), but when busybox-w32 is built with clang then it works and downloads it successfully.

Also interesting, the solution posted at that link also fixes this issue when built with GCC:

git revert 7fbfb2050f24a457a909ea6bcec85c49a21db83a

So I'm guessing either gcc optimization bug, or that the busybox TLS code relies on some un/implementation-defined behavior (e.g. maybe integer wrap-around), which is not optimized after that commit is reverted, and which clang doesn't optimize.

The fact that the reverting a single commit seems to fix two diffrent symptoms of TLS errors, and that report suggests that earlier version of busybox (with prev debian version) did generally work:

There are two possible fixes: 1) downgrade to older Debian distribution 2) revert commit with compiler optimization

Suggests to me that it's possible that this is the only or main issue with busybox[-w32] TLS.

I think it's worth reverting it in busybox-w32.

EDIT: This revert adds 11K to the binary for me with gcc (651K before the revert, 662k after), which subjectively I think is totally negligible in the context of a Windows binary, and if considering that it fixes at least one TLS issue then it makes it worthwhile. It's also possible that the bigger size is exactly a symptom of over-optimization being skipped.

Also, this revert adds 55K with clang (650k before, 705k after the revert, which is less negligible, but which personally I still think is not interesting in the context of a windows binary).

rmyorston commented 5 months ago

I think I've isolated the problem to some x86_64 assembly language code in the TLS implementation. This only affects x86_64 builds with gcc. Using the equivalent C code seems to fix both the upstream bug and the problem with busybox-w32.

avih commented 5 months ago

Nice. I can confirm it fixes the busybox wget issue here too with gcc.

It's probably worth upstreaming too, as far as I can tell. I doubt anyone maintains the assembly code, and the C code is also apparently smaller (for you. for me it remains identical size in bytes), and it fixes a real issue...

avih commented 5 months ago

It is a bit weird though that the issue is alternatively fixed by using different/less optimizations, because I'd think different optimizations wouldn't affect the assembly code...

So it feels maybe either a gcc optimization issue, maybe something in the interfacing between the C code and the assembly code, or maybe busybox assembly code issue which doesn't take into account how some optimizations may affect the C surrounding code.

Either way, I feel pure C code is safer here regardless, so thanks for disabling the asm.