luci icon indicating copy to clipboard operation
luci copied to clipboard

luci-app-upnp: Fix to get the correct Description upnp rule from Leases

Open alexwbaule opened this issue 2 years ago • 7 comments

TL:DR

To get the description, you need to look in all lines from the leases. Currently, its read only a line following the iptables command. If the iptables return 2 lines, and the leases file have 5 lines, the match does not occour. Because the read of leases follow the read of iptables command. Read one line of iptables, read one line of leases.

This PR fixes that, reading all leases for every line from iptables command, doing the match.

alexwbaule avatar Apr 22 '22 19:04 alexwbaule

Can you please paste a lease file example here exhibiting the problem in the original code?

jow- avatar Apr 22 '22 22:04 jow-

Shure !!

This is the output of iptables command.

Chain MINIUPNPD (2 references)
num      pkts      bytes target     prot opt in     out     source               destination
1          35     2146 DNAT       tcp  --  *      *       0.0.0.0/0            0.0.0.0/0            tcp dpt:16901 to:192.168.250.7:32400
2           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:39703 to:192.168.250.135:39703
3           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:52715 to:192.168.250.129:52715
4           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:38933 to:192.168.250.117:38933
5           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:60785 to:192.168.250.117:60785

And this is the leases content file.

TCP:39757:192.168.250.36:9010:1650923593:IPC_CMD
UDP:59318:192.168.250.117:59318:1651149581:Minecraft
UDP:58277:192.168.250.117:58277:1651154491:Minecraft	
UDP:55609:192.168.250.117:55609:1651159167:Minecraft
UDP:37712:192.168.250.155:37712:1651227604:Minecraft	
UDP:59488:192.168.250.129:59488:1651228961:Minecraft
UDP:39703:192.168.250.135:39703:1651232027:Minecraft
UDP:52715:192.168.250.129:52715:1651253104:Minecraft	
UDP:38933:192.168.250.117:38933:1651256691:Minecraft
UDP:60785:192.168.250.117:60785:1651257127:Minecraft	
TCP:16901:192.168.250.7:32400:1651259189:Plex Media Server

My config to UPNP:

config upnpd 'config'
	option enabled '1'
	option download '1024'
	option upload '512'
	option internal_iface 'lan'
	option port '5000'
	option upnp_lease_file '/var/run/miniupnpd.leases'
	option uuid 'c5b4d518-2213-4254-a86b-568b906d83b2'
	option serial_number 'v1'
	option presentation_url 'http://openwrt'
	option model_number 'TP-Link Archer C2600'
	option log_output '1'
	option notify_interval '25'
	option clean_ruleset_threshold '30'
	option clean_ruleset_interval '300'
	option igdv1 '1'

config perm_rule
	option action 'allow'
	option ext_ports '1024-65535'
	option int_addr '0.0.0.0/0'
	option int_ports '1024-65535'
	option comment 'Allow high ports'

config perm_rule
	option action 'deny'
	option ext_ports '0-65535'
	option int_addr '0.0.0.0/0'
	option int_ports '0-65535'
	option comment 'Default deny'

alexwbaule avatar Apr 23 '22 01:04 alexwbaule

I dont know if the "clean_ruleset_*" config, clean some rules and the lease file remains with the line that was removed by the cleanup rule. I realize the error when i got my router running for a several days and sometimes in the "Overview" tab, i remove some upnp rules that don't make sense anymore. The iptables inserted rules are synced with the leases ?

alexwbaule avatar Apr 23 '22 01:04 alexwbaule

Looking into iptables and leases file, i think thats 2 files are not synced.

root@openwrt:~# iptables --line-numbers -t nat -xnvL MINIUPNPD 2>/dev/null
Chain MINIUPNPD (2 references)
num      pkts      bytes target     prot opt in     out     source               destination         
1           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:28926 to:192.168.250.117:28926
2           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:54586 to:192.168.250.211:54586
3          26     1563 DNAT       tcp  --  *      *       0.0.0.0/0            0.0.0.0/0            tcp dpt:16901 to:192.168.250.7:32400
4           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:52770 to:192.168.250.211:52770
root@openwrt:~# cat /tmp/run/miniupnpd.leases 
UDP:28926:192.168.250.117:28926:1651771713:Minecraft
UDP:54586:192.168.250.211:54586:1651794681:Minecraft
UDP:52770:192.168.250.211:52770:1651851375:Minecraft
TCP:16901:192.168.250.7:32400:1651862627:Plex Media Server

alexwbaule avatar Apr 29 '22 19:04 alexwbaule

Hello. I Fixed it the commit messages to pass in the CI rules.

alexwbaule avatar Jul 13 '22 22:07 alexwbaule

@jow-

The leases and the iptables are not synced.

The "delete" has the same problem, erases the line that is not correspondent to the iptables line.

Do you want to test ? Its simple.

Enable UPNP and open minecraft in some device (tablet, pc, phone).

Open, start a game to create a upnp rule, close and do it a couple times...

Now, delete some rule, and voila... missing description too in delete.

I will update this PR to fix the delete too.

alexwbaule avatar Jul 14 '22 18:07 alexwbaule

The order is not followed between iptables and leases.

root@openwrt:~# cat /var/run/miniupnpd.leases
UDP:43348:192.168.250.134:43348:1658432168:Minecraft
UDP:42049:192.168.250.145:42049:1658432288:Minecraft
UDP:44872:192.168.250.134:44872:1658432352:Minecraft
TCP:16901:192.168.250.7:32400:1658432382:Plex Media Server
UDP:47120:192.168.250.134:47120:1658433944:Minecraft
UDP:37261:192.168.250.134:37261:1658433987:Minecraft
root@openwrt:~# iptables --line-numbers -t nat -xnvL MINIUPNPD
Chain MINIUPNPD (2 references)
num      pkts      bytes target     prot opt in     out     source               destination         
1           0        0 DNAT       tcp  --  *      *       0.0.0.0/0            0.0.0.0/0            tcp dpt:16901 to:192.168.250.7:32400
2           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:43348 to:192.168.250.134:43348
3           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:42049 to:192.168.250.145:42049
4           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:44872 to:192.168.250.134:44872
5           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:47120 to:192.168.250.134:47120
6           0        0 DNAT       udp  --  *      *       0.0.0.0/0            0.0.0.0/0            udp dpt:37261 to:192.168.250.134:37261

alexwbaule avatar Jul 14 '22 20:07 alexwbaule