search

kristapsdz / kcgi

minimal CGI and FastCGI library for C/C++
https://kristaps.bsd.lv/kcgi
ISC License
271 stars 40 forks source link

newer libcurl versions brings up issues with HTTP-Version not being correctly handled #72

Closed xse closed 4 years ago

xse commented 4 years ago

TLDR: #73 fix the issue but the issue should be fixed differently.

This first post can be entirely skipped i leave it there for context.

libcurl now requires explicitly opting in to HTTP/0.9 to get the fcgi regress tests working. This post is kinda long 'cause i edited it many times while figuring out the issue.

Hello,

Both systems were tests are failing are archlinux. I've noted some similarities with #39 bmake regress fails on test-fcgi-bigfile.

"By return value" the following tests are failing:

test-abort-validator and test-fcgi-abort-validator are both generating a coredump.

On armv6, note about the sandbox

Platform: Linux rpi 4.19.79-1-ARCH #1 SMP PREEMPT Sat Oct 12 17:02:53 UTC 2019 armv6l GNU/Linux

Looking into the sandbox stuff, -DSANDBOX_SECCOMP_DEBUG did not output anything at all but for the two validator tests strace shown two syscalls that were apparently blocked by it: gettid:

[pid 23412] gettid( <unfinished ...>
[pid 23412] --- SIGSYS {si_signo=SIGSYS, si_code=SYS_SECCOMP, si_call_addr=0x76d0931c, si_syscall=__NR_gettid, si_arch=AUDIT_ARCH_ARM} ---

and tgkill:

[pid 24039] tgkill(24039, 24039, SIGABRT <unfinished ...>
[pid 24039] --- SIGSYS {si_signo=SIGSYS, si_code=SYS_SECCOMP, si_call_addr=0x76cf8330, si_syscall=__NR_tgkill, si_arch=AUDIT_ARCH_ARM} ---

After adding those two to the preauth_ctrl and preauth_work arrays in sandbox-seccomp-filter.c, the test now executes gettid and tgkill, then it calls rt_sigprocmask which was already authorized and it then receive a SIGABRT. (Before those changes it was a SIGSYS triggering the dump) That doesn't change much but i guess abort is intended while the seccomp block isn't.

To try and fix the fcgi tests, i've also tried building everything with -fsigned-char since i'm getting a signedness warning when building on x86, with no success.

On x86_64, same fcgi tests are failing. Only noted on arch linux

Platform: Linux krkrkr.org 5.3.7-arch1-1-ARCH #1 SMP PREEMPT Fri Oct 18 00:17:03 UTC 2019 x86_64 GNU/Linux

The same exact tests are returning != 0.

I did not notice any errors on debian, might be archlinux related. I just tested versions as far back as 0_10_3 which was working well back then, make regress failing there too.

EDIT: aight just verified on a fresh x86 debian, no issues here. Seems arch related.

19/10/18 update

Noted a sandbox message on arch, only saw that once so far tho so to me it seems totally unrelated:

ssh_sandbox_violation_control: unexpected system call (control) (arch:0xc000003e,syscall:219 @ 0x7f9245ad79b7)
ssh_sandbox_violation_worker: unexpected system call (worker) (arch:0xc000003e,syscall:219 @ 0x7f9245ad79b7)

On debian, this test executes and exits, on arch, it stays open in the background expecting something Really does not look like to be a sandbox issue to me the logs don't show anything related to what was there for gettid and tgkill on arm. Few strace logs debian vs arch are available here.

This looks to me like a socket issue.

libcurl

Trying to understand that, i've modified the fcgi-bigfile test, adding: curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L); The outputs now differs quite a bit from arch & debian.

The main issue seems to be: (see also)

* Received HTTP/0.9 when not allowed

* Closing connection 0
write: Connection reset by peer

And indeed adding : curl_easy_setopt(curl, CURLOPT_HTTP09_ALLOWED, 1L); to the fcgi bigfile test makes it pass on archlinux.

Have a good day!

xse commented 4 years ago

Alright changes in #73 fix the libcurl issue.

EDIT; i did not ask myself why those were HTTP/0.9, kcgi's manual states that

 +o    HTTP response headers are standardised in RFC 2616, "HTTP/1.1" and further in RFC 4229, "HTTP Header Field Registrations".

It might be worth looking into that.

Ok i've looked into that a bit, the thing is:

The version of an HTTP message is indicated by an HTTP-Version field in the first line of the message. If the protocol version is not specified, the recipient must assume that the message is in the simple HTTP/0.9 format

To have a look at the responses and requests i used CURLOPT_DEBUGFUNCTION, copying the included example to several different regress tests.

libcurl sends as headers 0000: 47 45 54 20 2f 20 48 54 54 50 2f 31 2e 31 0d 0a GET / HTTP/1.1 In some cases (those failing tests), kcgi sends back something like that (when HTTP/0.9 is added as option, otherwise they fail):

<= Recv data, 0000000016 bytes (0x00000010)
0000: fcgi.c:677: fcgi_waitread: exit request
53 74 61 74 75 73 3a 20 32 30 30 20 4f 4b 0d 0a Status: 200 OK..
<= Recv data, 0000000027 bytes (0x0000001b)
0000: 43 6f 6e 74 65 6e 74 2d 54 79 70 65 3a 20 74 65 Content-Type: te
0010: 78 74 2f 68 74 6d 6c 0d 0a 0d 0a                xt/html....
Status: 200 OK
Content-Type: text/html

In the case of the test that are not failing, kcgi sends back something like that

<= Recv header, 0000000017 bytes (0x00000011)
0000: 48 54 54 50 2f 31 2e 31 20 32 30 30 20 4f 4b 0d HTTP/1.1 200 OK.
0010: 0a                                              .
<= Recv header, 0000000025 bytes (0x00000019)
........

So i feel like my PR is more or less a dirty solution, kcgi should send the HTTP version, otherwise clients have to assume that the response is HTTP/0.9.

Why this difference like why some requests are answered by "Status: %s" while others get the proper thingy. Is it only happening for the tests ? Why is this only happening in some tests ? I'm still trying to figure out how that works, the dochild_cgi function, it's fcgi counterpart and pretty much all those files are confusing to me. Should that even be considered as a problem ? After all as long as curl accept HTTP/0.9 responses, it works.

Does that make sense ?

kristapsdz commented 4 years ago

This has been fixed and will be in the next version. Thank you!