maximkulkin / esp-homekit

Apple HomeKit accessory server library for ESP-OPEN-RTOS
MIT License
1.1k stars 168 forks source link

reset http_parser if required #192

Open maccoylton opened 3 years ago

maccoylton commented 3 years ago

If the parser sees a "Connection: Close" message it will not process any subsequent requests. This modification checks for that status and resets the parser. This was first seen when resetting Homebridge on a Raspberry PI, but could equally be exploited by anyone who has access to send http message in the clear to your accessories

rottenhowler commented 3 years ago

Wait, what? If some client sends "Connection: Close", it should affect it's own HTTP parser. Why would you want to reset it or care if it behaves properly? It's that client's own problem. Are you saying that it affects other clients?

maccoylton commented 3 years ago

An accessory using esp-homekit uses http_parser to process request from the IOS device like the pair request. If for whatever reason that request continues "Connection: Closed" then the parser will not process any subsequent requests, and the accessory will be in a not responding state.

In my case for whatever reason a Raspberry PI running homebridge-zigbee-nt was sending this each time homebridge was restarted, and causing accessories to go into a not responding state. Here's an example of the offending message from the PI, in this case the Pi is .33 and the accessory is .54

`>>> HomeKit: [Client 2] Got new client connection from 192.168.1.33

HomeKit: [Client 1] Have existing connection from 192.168.1.43 on_homekit_event: Client connected, Free Heap=19832 on_homekit_event: End, Freep Heap=19832 homekit_client_process: [Client 2] Got 142 incoming data Not encrypted data (142 bytes): "GET /accessories HTTP/1.1\x0D\x0AAccept: application/json, text/plain, /\x0D\x0AUser-Agent: axios/0.21.1\x0D\x0AHost: 192.168.1.54:5556\x0D\x0AConnection: close\x0D\x0A\x0D\x0A" homekit_server_on_message_complete: Unknown endpoint client_sendv: [Client 2] Sending payload: HTTP/1.1 404 Not Found\x0D\x0A\x0D\x0A homekit_client_process: [Client 2] Finished processing `

maximkulkin commented 3 years ago

This log does not demonstrate the problem: some client sends request and gets 404. There's nothing wrong with that.

maccoylton commented 3 years ago

Max, per IM in our slack channel, the issue is NOT the 404 error. The issue is that if ANY message contains "Connection:close" as in the above example then the parser will set an error code and each SUBSEQUENT call to the parser will come straight out ... here is the relevant section in the parser code confirming this behaviour:-

` case s_dead: /* this state is used after a 'Connection: close' message