kennedyshead / aioasuswrt

MIT License
24 stars 24 forks source link

Library leaks SSH connections as is not Coroutine-Safe #90

Open danielskowronski opened 8 months ago

danielskowronski commented 8 months ago

Issue discovered when debugging https://github.com/home-assistant/core/issues/103230 and should have been solved by https://github.com/kennedyshead/aioasuswrt/pull/68 long time ago

aioasuswrt does not always close SSH connections, making remote system solely responsible for freeing up resources, which is a bad assumption, especially on AsusWRT routers, where "Idle Timeout" setting for SSH server applies only to interactive sessions, not to SSH connections with all channels closed (dropbear is always started without -I so it never times out). This behaviour quite often makes high-end routers with 1GB of memory crash in less than a week.

Points to solve:

1) SshConnection.async_connect() should check if a connection exists before creating a new one, so it can be closed if needed; this code may be called when a connection object exists, but some error was handled and reconnect is needed

2) entire SshConnection.async_connect() should be guarded by some form of lock, for example asyncio.Lock() so it's Coroutine-Safe; otherwise bad timing of await can create and overwrite multiple connections we no longer have access to, yet they still consume resources on a server