esp-homekit icon indicating copy to clipboard operation
esp-homekit copied to clipboard

reset http_parser if required

Open maccoylton opened this issue 3 years ago • 4 comments

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

maccoylton avatar Mar 29 '21 21:03 maccoylton

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?

rottenhowler avatar Mar 30 '21 03:03 rottenhowler

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 `

maccoylton avatar Mar 30 '21 08:03 maccoylton

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

maximkulkin avatar Mar 31 '21 17:03 maximkulkin

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 * the parser will error out if it reads another message */

maccoylton avatar Mar 31 '21 18:03 maccoylton