Hi Jakub, On Wed, 2020-01-15 at 18:08 +0100, Jakub Witowski wrote: > During sending messages with high speed, the 'cached' value may have exceeded the 24-bit range > and this value will save into storage. > > The current implementation protects us against exceeding max sequence number (0xFFFFFF). > > Jakub Witowski (1): > mesh: sequence number out of range fix > > mesh/mesh-config-json.c | 4 ++++ > mesh/net.c | 3 +++ > 2 files changed, 7 insertions(+) > This is a very dangerous course of action. The suggested patch might potentially cause a node to re-use a sequence number more than once, which would cause a "dirty nonce" condition, and allow unauthorized entities to derive the encrypted data without the keys. The Mesh working group put a lot of work into determining what should be range of Sequence Numbers and IV Indexes, and came up with the 24 bits of sequence numbers, which can be reset to 0 with an IV Index update every 192 hours. This will allow a node to send 24 packets (every 42ms) a second constantly without running out of sequence numbers. While this is technically possible, especially with a daemon that might be looping back messages internally without ever using the OTA interface, it is not really possible in an actual BT driven real life system. So here is the solution I would suggest: Beacuse we store sequence numbers internally with a u32 sized data type, we should *let* the value go over the max legal sequence nunmbver of 0xFFFFFF (perhaps capping it at 0x01000000 to prevent "super overflows"). Then we *reject* all send requests with a sequence number > 0xFFFFFF. Assuming the IV Index update logic is working correctly (triggered internally when the Sequence number goes over 0x00800000... half the max) the node will work just fine as long as it never tries to send more than one packet every 42ms for 192 hours without rest... Just as the specification intended. If it does, then the specification and code will disallow it. BR, Brian
