openwisp/openwrt-openwisp-monitoring

[bug] ULA prefix check is bugged

ChristianDresel opened this issue · 6 comments

Hello

i tested the openwisp monitoring on openwrt first time today.

Now i get only a error in logread:

Sat May 14 10:13:31 2022 daemon.err openwisp-monitoring: Data not sent successfully. Response code is "400". Error message is ""Invalid data in \"#/\", validator says:\n\nNone is not of type 'object'"" Run with verbose mode to find more.

if i run manual the /usr/sbin/netjson-monitoring i get this:

root@TestDevice1:/tmp# /usr/sbin/netjson-monitoring --dump *
lua: /usr/lib/lua/openwisp-monitoring/utils.lua:7: attempt to index local 'str' (a nil value)
stack traceback:
	/usr/lib/lua/openwisp-monitoring/utils.lua:7: in function 'split'
	/usr/lib/lua/openwisp-monitoring/interfaces.lua:132: in function 'get_addresses'
	/usr/libexec/netjson-monitoring:165: in main chunk
	[C]: ?
root@TestDevice1:/tmp# /usr/sbin/netjson-monitoring --dump eth0
lua: /usr/lib/lua/openwisp-monitoring/utils.lua:7: attempt to index local 'str' (a nil value)
stack traceback:
	/usr/lib/lua/openwisp-monitoring/utils.lua:7: in function 'split'
	/usr/lib/lua/openwisp-monitoring/interfaces.lua:132: in function 'get_addresses'
	/usr/libexec/netjson-monitoring:165: in main chunk
	[C]: ?

any ideas what here fails? With verbose i don't see more errors, i think the probleme is the lua error.

The probleme is here:

https://github.com/openwisp/openwrt-openwisp-monitoring/blob/master/openwisp-monitoring/files/lib/openwisp-monitoring/interfaces.lua#L132

i have a IP address without :: (e.g. 2001:db8:aa:bb:be9f:f3ff:fecd:c646/64 ).

Thanks for reporting @ChristianDresel, looks like a bug.

The check to find out whether the IPv6 matches the ula prefix or not is very fragile and seem to fail if the ula prefix is nil.

@ChristianDresel can you try the following command on your openwrt device?

uci show network.globals.ula_prefix

If you get uci: Entry not found, try adding the following to /etc/config/network as a temporary workaround until we fix this bug:

config globals 'globals'
	option ula_prefix 'fd2e:b527:b236::/48'

A possible easy fix (without having to implement the parsing of the ipv6 address) would be:

diff --git a/openwisp-monitoring/files/lib/openwisp-monitoring/interfaces.lua b/openwisp-monitoring/files/lib/openwisp-monitoring/interfaces.lua
index 273fb13..ceb894c 100644
--- a/openwisp-monitoring/files/lib/openwisp-monitoring/interfaces.lua
+++ b/openwisp-monitoring/files/lib/openwisp-monitoring/interfaces.lua
@@ -128,8 +128,10 @@ function interfaces.get_addresses(name)
           if utils.starts_with(addr, 'fe80') then
             proto = 'static'
           else
-            local ula = uci_cursor.get('network', 'globals', 'ula_prefix')
-            local ula_prefix = utils.split(ula, '::')[1]
+            local ula_prefix = uci_cursor.get('network', 'globals', 'ula_prefix') or ''
+            if ula_prefix then
+              ula_prefix = ula_prefix.sub(0, 13)
+            end
             if utils.starts_with(addr, ula_prefix) then
               proto = 'static'
             else

@devkapilbansal any suggestion on how we could write a failing test for this while maintaining the rest of the test suite unaltered?

Looking into it 👀

@ChristianDresel should have been fixed by #96, let us know if you have any other problem! Thanks for reporting!