Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Broken parsing of HTTP headers #39

Open
abouvier opened this issue Dec 27, 2021 · 13 comments
Open

Broken parsing of HTTP headers #39

abouvier opened this issue Dec 27, 2021 · 13 comments
Labels

Comments

@abouvier
Copy link

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:

auto headers = request.unparsedHeaders();
auto header = headers.find(REVERSE_PROXY_HEADER);

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 abique added the bug label Dec 27, 2021
@abique
Copy link
Owner

abique commented Dec 27, 2021

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

@batkom
Copy link

batkom commented Jun 7, 2022

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
Copy link
Owner

abique commented Jun 7, 2022

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
Copy link
Owner

abique commented Jun 7, 2022

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
Copy link
Author

abouvier commented Jun 7, 2022

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

@abique
Copy link
Owner

abique commented Jun 7, 2022

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

@abique
Copy link
Owner

abique commented Jun 7, 2022

You can try again now.

@abouvier
Copy link
Author

abouvier commented Jun 7, 2022

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
Copy link
Owner

abique commented Jun 7, 2022

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

@abouvier
Copy link
Author

abouvier commented Jun 8, 2022

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
Copy link
Owner

abique commented Jun 8, 2022

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
Copy link
Author

abouvier commented Jun 8, 2022

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

@abique
Copy link
Owner

abique commented Jun 8, 2022

Still it should be using ipv4/6

@github-staff github-staff deleted a comment from maulanaayub May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants