abique / hefur

Standalone C++ BitTorrent tracker
MIT License
190 stars 30 forks source link

Broken parsing of HTTP headers #39

Open abouvier opened 2 years ago

abouvier commented 2 years ago

The parsing of HTTP headers is broken for certain value of Accept-Encoding when Hefur is behind a reverse proxy.

Apache config:

<VirtualHost *:443>
        ServerName tracker.domain.com
        (...)
        <Location />
                ProxyPass http://localhost:6969/
                ProxyPassReverse http://localhost:6969/
                ProxyPreserveHost On
        </Location>
</VirtualHost>

Hefur is launched like this: $ hefurd -torrent-dir /srv/tracker -bind-addr 127.0.0.1 -reverse-proxy-from 127.0.0.1 -reverse-proxy-header X-Forwarded-For

The problem arise here: https://github.com/abique/hefur/blob/5136250a567a290f41507f607d2c590f1a14eb2f/hefur/log-handler.cc#L24-L25

With qBittorrent, Apache sends the following request to Hefur:

Host: tracker.domain.com
User-Agent: qBittorrent/4.3.9
Accept-Encoding: gzip
X-Forwarded-For: 42.42.42.42
(...)

request.unparsedHeaders() returns all headers and X-Forwarded-For is found.

With Transmission, Apache sends the following request to Hefur:

Host: tracker.domain.com
User-Agent: Transmission/3.00
Accept: */*
Accept-Encoding: deflate, gzip, br, zstd
X-Forwarded-For: 42.42.42.42
(...)

request.unparsedHeaders() returns only Accept: */*

If I remove Accept-Encoding from the request sent by Apache (with RequestHeader unset Accept-Encoding in <Location />), then the request is parsed correctly and the real IP is found, but Hefur crash soon after with:

hefurd[28911]: hefurd: hefur/address.cc:133: hefur::Address& hefur::Address::operator=(const sockaddr*): Assertion 'false' failed.

So an announce request from Transmission seems totally broken: the real IP is not found, the response is bad (wrong values of seeders/leechers) and Transmission try to announce again every 8 seconds in a loop. Everything is working fine with qBittorrent.

OS: Arch Linux Hefur: 1.0 Apache: 2.4.51 qBittorrent: 4.3.9 Transmission: 3.00

abique commented 2 years ago

Thank you very much! I'll look into it soon.

batkom commented 2 years ago

I think the problem in https://github.com/abique/mimosa/blob/42c0041b570b55a24c606a7da79c70c9933c07d4/mimosa/http/request-parser.y#L98 abouvier, can you test it please?

--- request-parser.y    2021-02-01 20:32:00.000000000 +0300
+++ request-parser.y    2022-06-07 13:22:20.739899488 +0300
@@ -95,8 +95,8 @@
 kvs: kv kvs | /* epsilon */ ;

 kv:
-  KEY VALUE { rq.addHeader(*$1, *$2); delete $1; delete $2; }
-| KEY_ACCEPT_ENCODING accept_encodings
+  KEY_ACCEPT_ENCODING accept_encodings
+| KEY VALUE { rq.addHeader(*$1, *$2); delete $1; delete $2; }
 | KEY_CONNECTION VALUE_CONNECTION { rq.setKeepAlive($2); }
 | KEY_COOKIE cookies
 | KEY_CONTENT_LENGTH VAL64 { rq.setContentLength($2); }
abique commented 2 years ago

KEY_ACCEPT_ENCODING shows up first in the lexer, but does not hurt to move it down. Then maybe I could move the KEY VALUE option at the end.

abique commented 2 years ago

I've added a unit test with your example, and updated mimosa. @abouvier does it fixes it for you? PS: sorry for the long delay...

abouvier commented 2 years ago

The new commit of mimosa submodule is non existent. Did you forget to push something on the mimosa repository?

abique commented 2 years ago

Super strange I get timeout to push and pull to mimosa...

abique commented 2 years ago

You can try again now.

abouvier commented 2 years ago

hefur now crashes immediately when I try to load https://tracker.domain.com/stat, with the same error:

hefurd: hefur/address.cc:133: hefur::Address& hefur::Address::operator=(const sockaddr*): Assertion 'false' failed.

abique commented 2 years ago

I've pushed some changes, that assertion wasn't correct.

abouvier commented 2 years ago

It's better. The client's public address is correctly registered in hefur, but the response sent by hefur to Transmission is still bogus, with nonsensical values. File sharing with other peers works though.

abique commented 2 years ago

Yeah something must have gone wrong, because it is a strange error to have a inet addr that isn't ipv4 or ipv6. I'll have to take a deeper look I suppose.

abouvier commented 2 years ago

It may be because hefur and Transmission are on the same host?

abique commented 2 years ago

Still it should be using ipv4/6