From 063850c7fb148df981a651c36631ac0597cc983f Mon Sep 17 00:00:00 2001 From: Stefan Allius Date: Thu, 30 May 2024 18:38:05 +0200 Subject: [PATCH 1/5] add allow and block filter for AT+ commands --- app/config/default_config.toml | 5 +++++ app/src/config.py | 13 +++++++++++-- app/src/gen3plus/solarman_v5.py | 24 ++++++++++++++++++++++++ app/tests/test_config.py | 18 +++++++++++------- app/tests/test_solarman.py | 1 + 5 files changed, 52 insertions(+), 9 deletions(-) diff --git a/app/config/default_config.toml b/app/config/default_config.toml index cd95d75..fbe2651 100644 --- a/app/config/default_config.toml +++ b/app/config/default_config.toml @@ -49,3 +49,8 @@ monitor_sn = 2000000000 # The "Monitoring SN:" can be found on a sticker e #pv3 = {type = 'RSM40-8-410M', manufacturer = 'Risen'} # Optional, PV module descr #pv4 = {type = 'RSM40-8-410M', manufacturer = 'Risen'} # Optional, PV module descr +[gen3plus.at_acl] +tsun.allow = ['AT+Z', 'AT+UPURL', 'AT+SUPDATE'] +tsun.block = [] +mqtt.allow = ['AT+'] +mqtt.block = [] diff --git a/app/src/config.py b/app/src/config.py index eef0c86..e1ef749 100644 --- a/app/src/config.py +++ b/app/src/config.py @@ -3,7 +3,7 @@ import shutil import tomllib import logging -from schema import Schema, And, Use, Optional +from schema import Schema, And, Or, Use, Optional class Config(): @@ -38,6 +38,14 @@ class Config(): 'proxy_node_id': Use(str), 'proxy_unique_id': Use(str) }, + 'gen3plus': { + 'at_acl': { + Or('mqtt', 'tsun'): { + 'allow': [str], + Optional('block', default=[]): [str] + } + } + }, 'inverters': { 'allow_all': Use(bool), And(Use(str), lambda s: len(s) == 16): { Optional('monitor_sn', default=0): Use(int), @@ -125,7 +133,8 @@ class Config(): # merge the default and the user config config = def_config.copy() - for key in ['tsun', 'solarman', 'mqtt', 'ha', 'inverters']: + for key in ['tsun', 'solarman', 'mqtt', 'ha', 'inverters', + 'gen3plus']: if key in usr_config: config[key] |= usr_config[key] diff --git a/app/src/gen3plus/solarman_v5.py b/app/src/gen3plus/solarman_v5.py index 1529057..e3ed438 100644 --- a/app/src/gen3plus/solarman_v5.py +++ b/app/src/gen3plus/solarman_v5.py @@ -91,8 +91,13 @@ class SolarmanV5(Message): # MODbus or AT cmd 0x4510: self.msg_command_req, # from server 0x1510: self.msg_command_rsp, # from inverter + # 0x0510: self.msg_command_rsp, # from inverter } self.modbus_elms = 0 # for unit tests + g3p_cnf = Config.get('gen3plus') + + if 'at_acl' in g3p_cnf: + self.at_acl = g3p_cnf['at_acl'] ''' Our puplic methods @@ -320,9 +325,24 @@ class SolarmanV5(Message): return self.mb.build_msg(Modbus.INV_ADDR, func, addr, val, log_lvl) + def at_cmd_forbidden(self, cmd: str, connection: str) -> bool: + return not cmd.startswith(tuple(self.at_acl[connection]['allow'])) or \ + cmd.startswith(tuple(self.at_acl[connection]['block'])) + async def send_at_cmd(self, AT_cmd: str) -> None: if self.state != self.STATE_UP: return + AT_cmd = AT_cmd.strip() + + if self.at_cmd_forbidden(cmd=AT_cmd, connection='mqtt'): + data_json = f'\'{AT_cmd}\' is forbidden' + node_id = self.node_id + key = 'at_resp' + logger.info(f'{key}: {data_json}') + asyncio.ensure_future( + self.publish_mqtt(f'{self.entity_prfx}{node_id}{key}', data_json)) # noqa: E501 + return + self.forward_at_cmd_resp = False self.__build_header(0x4510) self._send_buffer += struct.pack(f' Date: Thu, 30 May 2024 19:32:14 +0200 Subject: [PATCH 2/5] add AT_COMMAND_BLOCKED counter --- app/src/gen3plus/solarman_v5.py | 7 ++++--- app/src/infos.py | 26 ++++++++++++++------------ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/app/src/gen3plus/solarman_v5.py b/app/src/gen3plus/solarman_v5.py index e3ed438..91ee30b 100644 --- a/app/src/gen3plus/solarman_v5.py +++ b/app/src/gen3plus/solarman_v5.py @@ -96,7 +96,7 @@ class SolarmanV5(Message): self.modbus_elms = 0 # for unit tests g3p_cnf = Config.get('gen3plus') - if 'at_acl' in g3p_cnf: + if 'at_acl' in g3p_cnf: # pragma: no cover self.at_acl = g3p_cnf['at_acl'] ''' @@ -450,11 +450,12 @@ class SolarmanV5(Message): result = struct.unpack_from(' Date: Thu, 30 May 2024 19:32:53 +0200 Subject: [PATCH 3/5] add missing testcases --- app/tests/test_infos.py | 4 +- app/tests/test_solarman.py | 78 +++++++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/app/tests/test_infos.py b/app/tests/test_infos.py index c3e6ddf..4b23bfb 100644 --- a/app/tests/test_infos.py +++ b/app/tests/test_infos.py @@ -17,13 +17,13 @@ def test_statistic_counter(): assert val == None or val == 0 i.static_init() # initialize counter - assert json.dumps(i.stat) == json.dumps({"proxy": {"Inverter_Cnt": 0, "Unknown_SNR": 0, "Unknown_Msg": 0, "Invalid_Data_Type": 0, "Internal_Error": 0,"Unknown_Ctrl": 0, "OTA_Start_Msg": 0, "SW_Exception": 0, "Invalid_Msg_Format": 0, "AT_Command": 0, "Modbus_Command": 0}}) + assert json.dumps(i.stat) == json.dumps({"proxy": {"Inverter_Cnt": 0, "Unknown_SNR": 0, "Unknown_Msg": 0, "Invalid_Data_Type": 0, "Internal_Error": 0,"Unknown_Ctrl": 0, "OTA_Start_Msg": 0, "SW_Exception": 0, "Invalid_Msg_Format": 0, "AT_Command": 0, "AT_Command_Blocked": 0, "Modbus_Command": 0}}) val = i.dev_value(Register.INVERTER_CNT) # valid and initiliazed addr assert val == 0 i.inc_counter('Inverter_Cnt') - assert json.dumps(i.stat) == json.dumps({"proxy": {"Inverter_Cnt": 1, "Unknown_SNR": 0, "Unknown_Msg": 0, "Invalid_Data_Type": 0, "Internal_Error": 0,"Unknown_Ctrl": 0, "OTA_Start_Msg": 0, "SW_Exception": 0, "Invalid_Msg_Format": 0, "AT_Command": 0, "Modbus_Command": 0}}) + assert json.dumps(i.stat) == json.dumps({"proxy": {"Inverter_Cnt": 1, "Unknown_SNR": 0, "Unknown_Msg": 0, "Invalid_Data_Type": 0, "Internal_Error": 0,"Unknown_Ctrl": 0, "OTA_Start_Msg": 0, "SW_Exception": 0, "Invalid_Msg_Format": 0, "AT_Command": 0, "AT_Command_Blocked": 0, "Modbus_Command": 0}}) val = i.dev_value(Register.INVERTER_CNT) assert val == 1 diff --git a/app/tests/test_solarman.py b/app/tests/test_solarman.py index 52a66a8..06a57f0 100644 --- a/app/tests/test_solarman.py +++ b/app/tests/test_solarman.py @@ -39,9 +39,10 @@ class MemoryStream(SolarmanV5): self.addr = 'Test: SrvSide' self.db.stat['proxy']['Invalid_Msg_Format'] = 0 self.db.stat['proxy']['AT_Command'] = 0 + self.db.stat['proxy']['AT_Command_Blocked'] = 0 self.test_exception_async_write = False self.entity_prfx = '' - self.at_acl = {'mqtt': {'allow': ['AT+'], 'block': []}, 'tsun': {'allow': ['AT+Z', 'AT+UPURL', 'AT+SUPDATE', 'AT+TIME'], 'block': []}} + self.at_acl = {'mqtt': {'allow': ['AT+'], 'block': ['AT+WEBU']}, 'tsun': {'allow': ['AT+Z', 'AT+UPURL', 'AT+SUPDATE', 'AT+TIME'], 'block': ['AT+WEBU']}} def _timestamp(self): return timestamp @@ -466,6 +467,15 @@ def AtCommandIndMsg(): # 0x4510 msg += b'\x15' return msg +@pytest.fixture +def AtCommandIndMsgBlock(): # 0x4510 + msg = b'\xa5\x17\x00\x10\x45\x03\x02' +get_sn() +b'\x01\x02\x00' + msg += b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' + msg += b'AT+WEBU\r' + msg += correct_checksum(msg) + msg += b'\x15' + return msg + @pytest.fixture def AtCommandRspMsg(): # 0x1510 msg = b'\xa5\x0a\x00\x10\x15\x03\x03' +get_sn() +b'\x01\x01' @@ -1277,11 +1287,49 @@ async def test_AT_cmd(ConfigTsunAllowAll, DeviceIndMsg, DeviceRspMsg, InverterIn assert m.forward_at_cmd_resp == False m.close() -def test_at_command_ind(ConfigTsunInv1, AtCommandIndMsg): +@pytest.mark.asyncio +async def test_AT_cmd_blocked(ConfigTsunAllowAll, DeviceIndMsg, DeviceRspMsg, InverterIndMsg, InverterRspMsg, AtCommandIndMsg): + ConfigTsunAllowAll + m = MemoryStream(DeviceIndMsg, (0,), True) + m.append_msg(InverterIndMsg) + m.read() + assert m.control == 0x4110 + assert str(m.seq) == '01:01' + assert m._recv_buffer==InverterIndMsg # unhandled next message + assert m._send_buffer==DeviceRspMsg + assert m._forward_buffer==DeviceIndMsg + + m._send_buffer = bytearray(0) # clear send buffer for next test + m._forward_buffer = bytearray(0) # clear send buffer for next test + await m.send_at_cmd('AT+WEBU') + assert m._recv_buffer==InverterIndMsg # unhandled next message + assert m._send_buffer==b'' + assert m._forward_buffer==b'' + assert str(m.seq) == '01:01' + + m.read() + assert m.control == 0x4210 + assert str(m.seq) == '02:02' + assert m._recv_buffer==b'' + assert m._send_buffer==InverterRspMsg + assert m._forward_buffer==InverterIndMsg + + m._send_buffer = bytearray(0) # clear send buffer for next test + m._forward_buffer = bytearray(0) # clear send buffer for next test + await m.send_at_cmd('AT+WEBU') + assert m._recv_buffer==b'' + assert m._send_buffer==b'' + assert m._forward_buffer==b'' + assert str(m.seq) == '02:02' + assert m.forward_at_cmd_resp == False + m.close() + +def test_AT_cmd_ind(ConfigTsunInv1, AtCommandIndMsg): ConfigTsunInv1 m = MemoryStream(AtCommandIndMsg, (0,), False) m.db.stat['proxy']['Unknown_Ctrl'] = 0 m.db.stat['proxy']['AT_Command'] = 0 + m.db.stat['proxy']['AT_Command_Blocked'] = 0 m.db.stat['proxy']['Modbus_Command'] = 0 m.read() # read complete msg, and dispatch msg assert not m.header_valid # must be invalid, since msg was handled and buffer flushed @@ -1297,6 +1345,32 @@ def test_at_command_ind(ConfigTsunInv1, AtCommandIndMsg): assert m._forward_buffer==AtCommandIndMsg assert m.db.stat['proxy']['Invalid_Msg_Format'] == 0 assert m.db.stat['proxy']['AT_Command'] == 1 + assert m.db.stat['proxy']['AT_Command_Blocked'] == 0 + assert m.db.stat['proxy']['Modbus_Command'] == 0 + m.close() + +def test_AT_cmd_ind_block(ConfigTsunInv1, AtCommandIndMsgBlock): + ConfigTsunInv1 + m = MemoryStream(AtCommandIndMsgBlock, (0,), False) + m.db.stat['proxy']['Unknown_Ctrl'] = 0 + m.db.stat['proxy']['AT_Command'] = 0 + m.db.stat['proxy']['AT_Command_Blocked'] = 0 + m.db.stat['proxy']['Modbus_Command'] = 0 + m.read() # read complete msg, and dispatch msg + assert not m.header_valid # must be invalid, since msg was handled and buffer flushed + assert m.msg_count == 1 + assert m.header_len==11 + assert m.snr == 2070233889 + # assert m.unique_id == '2070233889' + assert m.control == 0x4510 + assert str(m.seq) == '03:02' + assert m.data_len == 23 + assert m._recv_buffer==b'' + assert m._send_buffer==b'' + assert m._forward_buffer==b'' + assert m.db.stat['proxy']['Invalid_Msg_Format'] == 0 + assert m.db.stat['proxy']['AT_Command'] == 0 + assert m.db.stat['proxy']['AT_Command_Blocked'] == 1 assert m.db.stat['proxy']['Modbus_Command'] == 0 m.close() From 407c1ceb2b59c354a89ca86bb78c2da6eaca8c44 Mon Sep 17 00:00:00 2001 From: Stefan Allius Date: Thu, 30 May 2024 19:40:25 +0200 Subject: [PATCH 4/5] control access via AT commands --- CHANGELOG.md | 1 + README.md | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 830c501..64be905 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] +- add allow and block filter for AT+ commands - catch all OSError errors in the read loop - log Modbus traces with different log levels - add Modbus fifo and timeout handler diff --git a/README.md b/README.md index 27b84c7..89eaba3 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,9 @@ If you use a Pi-hole, you can also store the host entry in the Pi-hole. - `AT-Command` support via MQTT topics (GEN3PLUS only) - Faster DataUp interval sends measurement data to the MQTT broker every minute - Self-sufficient island operation without internet -- Runs in a non-root Docker Container +- Security-Features: + - control access via `AT commands` + - Runs in a non-root Docker Container ## Home Assistant Screenshots From 20f4fd647ca88ed5a5ec6b778069a1e51b698382 Mon Sep 17 00:00:00 2001 From: Stefan Allius Date: Thu, 30 May 2024 19:44:54 +0200 Subject: [PATCH 5/5] update config example --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 89eaba3..ffe9f6d 100644 --- a/README.md +++ b/README.md @@ -170,6 +170,12 @@ pv2 = {type = 'RSM40-8-410M', manufacturer = 'Risen'} # Optional, PV module de pv3 = {type = 'RSM40-8-410M', manufacturer = 'Risen'} # Optional, PV module descr pv4 = {type = 'RSM40-8-410M', manufacturer = 'Risen'} # Optional, PV module descr +[gen3plus.at_acl] +tsun.allow = ['AT+Z', 'AT+UPURL', 'AT+SUPDATE'] # allow this for TSUN access +tsun.block = [] +mqtt.allow = ['AT+'] # allow all via mqtt +mqtt.block = [] + ``` ## Inverter Configuration