From 7fdc7fe16347a512faf6a7139ff9d568b56f034f Mon Sep 17 00:00:00 2001 From: Stefan Allius Date: Sun, 22 Dec 2024 11:51:14 +0100 Subject: [PATCH 1/4] avoid forwarding to a private (lokal) IP addr --- app/src/inverter_base.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/app/src/inverter_base.py b/app/src/inverter_base.py index 580490c..9daa55b 100644 --- a/app/src/inverter_base.py +++ b/app/src/inverter_base.py @@ -6,6 +6,7 @@ import gc from aiomqtt import MqttCodeError from asyncio import StreamReader, StreamWriter +from ipaddress import ip_address from inverter_ifc import InverterIfc from proxy import Proxy @@ -101,6 +102,20 @@ async def create_remote(self) -> None: logging.info(f'[{stream.node_id}] Connect to {addr}') connect = asyncio.open_connection(host, port) reader, writer = await connect + r_addr = writer.get_extra_info('peername') + if r_addr is not None: + (ip, _) = r_addr + if ip_address(ip).is_private: + logging.error( + f"""resolve {host} to {ip}, which is a private IP! +\u001B[31m Check your DNS settings and use a public DNS resolver! + + To prevent a possible loop, forwarding to local IP addresses is + not supported and is deactivated for subsequent connections +\u001B[0m +""") + Config.act_config[self.config_id]['enabled'] = False + ifc = AsyncStreamClient( reader, writer, self.local, self.__del_remote) From c8692226772485c47a29dc86b465fbe527f8756a Mon Sep 17 00:00:00 2001 From: Stefan Allius Date: Sun, 22 Dec 2024 12:42:46 +0100 Subject: [PATCH 2/4] test DNS resolver issues --- app/tests/test_inverter_base.py | 76 ++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/app/tests/test_inverter_base.py b/app/tests/test_inverter_base.py index 7293ead..f480165 100644 --- a/app/tests/test_inverter_base.py +++ b/app/tests/test_inverter_base.py @@ -54,11 +54,12 @@ def feed_eof(self): class FakeWriter(): + peer = ('47.1.2.3', 10000) def write(self, buf: bytes): return def get_extra_info(self, sel: str): if sel == 'peername': - return ('47.1.2.3', 10000) + return self.peer elif sel == 'sockname': return 'sock:1234' assert False @@ -241,6 +242,79 @@ async def test_remote_conn(config_conn, patch_open_connection): cnt += 1 assert cnt == 0 +@pytest.mark.asyncio +async def test_remote_conn_to_private(config_conn, patch_open_connection): + _ = config_conn + _ = patch_open_connection + assert asyncio.get_running_loop() + InverterBase._registry.clear() + reader = FakeReader() + writer = FakeWriter() + FakeWriter.peer = ("192.168.0.1", 10000) + + with InverterBase(reader, writer, 'tsun', Talent) as inverter: + assert inverter.local.stream + assert inverter.local.ifc + await inverter.create_remote() + await asyncio.sleep(0) + assert not Config.act_config['tsun']['enabled'] + assert inverter.remote.stream + assert inverter.remote.ifc + assert inverter.local.ifc.healthy() + + # outside context manager the unhealth AsyncStream is released + FakeWriter.peer = ("47.1.2.3", 10000) + cnt = 0 + for inv in InverterBase: + assert inv.healthy() # inverter is healthy again (without the unhealty AsyncStream) + cnt += 1 + del inv + assert cnt == 1 + + del inverter + cnt = 0 + for inv in InverterBase: + print(f'InverterBase refs:{gc.get_referrers(inv)}') + cnt += 1 + assert cnt == 0 + + +@pytest.mark.asyncio +async def test_remote_conn_to_loopback(config_conn, patch_open_connection): + _ = config_conn + _ = patch_open_connection + assert asyncio.get_running_loop() + InverterBase._registry.clear() + reader = FakeReader() + writer = FakeWriter() + FakeWriter.peer = ("127.0.0.1", 10000) + + with InverterBase(reader, writer, 'tsun', Talent) as inverter: + assert inverter.local.stream + assert inverter.local.ifc + await inverter.create_remote() + await asyncio.sleep(0) + assert not Config.act_config['tsun']['enabled'] + assert inverter.remote.stream + assert inverter.remote.ifc + assert inverter.local.ifc.healthy() + + # outside context manager the unhealth AsyncStream is released + FakeWriter.peer = ("47.1.2.3", 10000) + cnt = 0 + for inv in InverterBase: + assert inv.healthy() # inverter is healthy again (without the unhealty AsyncStream) + cnt += 1 + del inv + assert cnt == 1 + + del inverter + cnt = 0 + for inv in InverterBase: + print(f'InverterBase refs:{gc.get_referrers(inv)}') + cnt += 1 + assert cnt == 0 + @pytest.mark.asyncio async def test_unhealthy_remote(config_conn, patch_open_connection, patch_unhealthy_remote): _ = config_conn From 1a3e67486d2069facd2ca26949c3db98f7c83773 Mon Sep 17 00:00:00 2001 From: Stefan Allius Date: Sun, 22 Dec 2024 12:57:35 +0100 Subject: [PATCH 3/4] increase test coverage --- app/tests/test_inverter_base.py | 39 +++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/app/tests/test_inverter_base.py b/app/tests/test_inverter_base.py index f480165..15b14bf 100644 --- a/app/tests/test_inverter_base.py +++ b/app/tests/test_inverter_base.py @@ -244,6 +244,7 @@ async def test_remote_conn(config_conn, patch_open_connection): @pytest.mark.asyncio async def test_remote_conn_to_private(config_conn, patch_open_connection): + '''check DNS resolving of the TSUN FQDN to a local address''' _ = config_conn _ = patch_open_connection assert asyncio.get_running_loop() @@ -281,6 +282,7 @@ async def test_remote_conn_to_private(config_conn, patch_open_connection): @pytest.mark.asyncio async def test_remote_conn_to_loopback(config_conn, patch_open_connection): + '''check DNS resolving of the TSUN FQDN to the loopback address''' _ = config_conn _ = patch_open_connection assert asyncio.get_running_loop() @@ -315,6 +317,43 @@ async def test_remote_conn_to_loopback(config_conn, patch_open_connection): cnt += 1 assert cnt == 0 +@pytest.mark.asyncio +async def test_remote_conn_to_None(config_conn, patch_open_connection): + '''check if get_extra_info() return None in case of an error''' + _ = config_conn + _ = patch_open_connection + assert asyncio.get_running_loop() + InverterBase._registry.clear() + reader = FakeReader() + writer = FakeWriter() + FakeWriter.peer = None + + with InverterBase(reader, writer, 'tsun', Talent) as inverter: + assert inverter.local.stream + assert inverter.local.ifc + await inverter.create_remote() + await asyncio.sleep(0) + assert Config.act_config['tsun']['enabled'] + assert inverter.remote.stream + assert inverter.remote.ifc + assert inverter.local.ifc.healthy() + + # outside context manager the unhealth AsyncStream is released + FakeWriter.peer = ("47.1.2.3", 10000) + cnt = 0 + for inv in InverterBase: + assert inv.healthy() # inverter is healthy again (without the unhealty AsyncStream) + cnt += 1 + del inv + assert cnt == 1 + + del inverter + cnt = 0 + for inv in InverterBase: + print(f'InverterBase refs:{gc.get_referrers(inv)}') + cnt += 1 + assert cnt == 0 + @pytest.mark.asyncio async def test_unhealthy_remote(config_conn, patch_open_connection, patch_unhealthy_remote): _ = config_conn From d8cc31206834883f9f11ef512080211ef32fb901 Mon Sep 17 00:00:00 2001 From: Stefan Allius Date: Sun, 22 Dec 2024 12:57:58 +0100 Subject: [PATCH 4/4] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c4fb45..21c7b5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [unreleased] +- detect usage of a local DNS resolver [#37](https://github.com/s-allius/tsun-gen3-proxy/issues/37) - path for logs is now configurable by cli args - configure the number of keeped logfiles by cli args - add DOCS.md for add-ons