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

GET requests with content-length result in HTTP 400 #44

Closed
kroll-j opened this issue Feb 21, 2021 · 4 comments
Closed

GET requests with content-length result in HTTP 400 #44

kroll-j opened this issue Feb 21, 2021 · 4 comments

Comments

@kroll-j
Copy link

kroll-j commented Feb 21, 2021

I'm running a matrix-synapse server with lighttpd. The matrix-telegram bridge would not start. The lighttpd logs show that the bridge attempts a GET request with a message body, which is rejected by lighttpd:

==> /var/log/lighttpd/error.log <==
2021-02-21 20:21:54: (request.c.407) GET/HEAD with content-length -> 400 
2021-02-21 20:21:54: (connections.c.796) request-header:\nGET /_matrix/client/r0/account/whoami?user_id=@[telegrambot-username]:[host] HTTP/1.1\r\nHost: [host]\r\nAuthorization: Bearer [...]\r\nContent-Type: application/json\r\nAccept: */*\r\nAccept-Encoding: gzip, deflate\r\nUser-Agent: Python/3.7 aiohttp/3.7.3\r\nContent-Length: 2\r\n\r\n 

==> /var/log/lighttpd/access.log <==
1.2.3.4 - - [21/Feb/2021:20:21:54 +0100] "GET /_matrix/client/r0/account/whoami?user_id=@[telegrambot-username]:[host] HTTP/1.1" 400 345 "-" "Python/3.7 aiohttp/3.7.3"

It is possible that other web servers just ignore the message-body and don't throw an error. lighttpd rejects it by design, it is argued that HTTP does not allow it.

The Content-Length of 2 suggests that the body is just an extraneous \r\n or so which got in there somehow, possibly as an oversight. As a "quick fix", I just emptied the content of GET requests in api.py around line 145:

    async def _send(self, method: Method, url: URL, content: Union[bytes, str],
                    query_params: Dict[str, str], headers: Dict[str, str]) -> 'JSON':
        if method == Method.GET:
            content = None  # remove body for GET requests
        while True:
            request = self.session.request(str(method), url, data=content,
                                           params=query_params, headers=headers)

That does fix the problem, the bridge is running now.

Whether that's a good thing to include in mautrix itself I don't know. Maybe it would be better to find all the places in mautrix-telegram (and possibly other mautrix-* bridges) which include a body in a GET request. But then -- GET with body doesn't make much sense, so maybe you want to just zap it in mautrix api and be done with it.

@tulir
Copy link
Member

tulir commented Feb 21, 2021

It's more likely that 2 bytes means {} (https://github.com/tulir/mautrix-python/blob/master/mautrix/api.py#L216), there aren't many \n's and probably no \r's at all.

@kroll-j
Copy link
Author

kroll-j commented Feb 21, 2021

It's more likely that 2 bytes means {} (https://github.com/tulir/mautrix-python/blob/master/mautrix/api.py#L216), there aren't many \n's and probably no \r's at all.

Seems likely!

Maybe send() would be a better place to remove the GET body then?

@gstrauss
Copy link

HTTP GET generally does not contain a request payload and GET request payload can sometimes be abused in request smuggling attacks.

For this (and other) reasons, lighttpd (by default) rejects GET with request payload (400 Bad Request).

However, lighttpd 1.4.54 and later have a configuration option to allow GET with a body, if that is desired:
https://redmine.lighttpd.net/projects/lighttpd/wiki/Server_http-parseoptsDetails
server.http-parseopts += ("method-get-body" => "enable")

@kroll-j
Copy link
Author

kroll-j commented Feb 23, 2021

HTTP GET generally does not contain a request payload and GET request payload can sometimes be abused in request smuggling attacks.

For this (and other) reasons, lighttpd (by default) rejects GET with request payload (400 Bad Request).

However, lighttpd 1.4.54 and later have a configuration option to allow GET with a body, if that is desired:
https://redmine.lighttpd.net/projects/lighttpd/wiki/Server_http-parseoptsDetails
server.http-parseopts += ("method-get-body" => "enable")

Thanks! I'd rather not enable that option for security reasons.

@tulir tulir closed this as completed in 5cce6fd Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants