Port forward interface seems to be broken

[Nixes]

Creating new entries doesn't seem to work correctly, with false positive validation errors.
Also unable to delete existing entries too.

Hardware: DJA0231
Firmware: 20.3.c.0389-MR20-RA
ch-gui-unhide Version: 2022.12.31@16:22

[seud0nym]

Can you check your UPnP port mappings? If there is UPnP mapping using the same port (in this case 8080) you will get that error message.

Can you please check the log for errors when you have a failed delete? I can't reproduce that one.

[Nixes]

I do have UPnP port mappings but none are 8080.

[Nixes]

Hmm where would I find the logs? I don't see anything in the webui logs section that looks like web application logging. And I can't see a nginx log file anywhere.

There are no errors in the browser console.

[seud0nym]

Well, I don't understand it. That error only appears if there is an existing mapping in the port forwards or UPnP rules.

I can't reproduce either of these. Very frustrating.

You can get the nginx log with this command:

logread -e nginx

[Nixes]

Yeah thanks for investigating. I checked logs too, and there are no entries there. It seems like it thinks its working and there might be missing error detection for whatever is happening.

Also these port forwards might have been from the original firmware, before installing tch-gui-unhide if that helps at all.

[Nixes]

On more testing I can always insert a new row if it has a port number higher then the highest UPnP port, but if its lower (but not overlapping or colliding) it won't let me add it.

[Nixes]

I think I found the issue.

On looking at the lua for 'firewall-port-forwarding-modal.lp' it looks like the validPorts function is not really doing the right thing, it seems to be finding the highest port and preventing adding port forwards below that. Which isn't actually doing the required collision detection.

I think it should be calling something like rulesDuplicateCheck instead.

I would make a PR but not really sure what the development process is like for this project. Thanks.

[seud0nym]

Well done. I back-ported the new validation from the Gen 3 and I just assumed (a rookie mistake) that it was working.

I did a quick test and added a PF on port 8080, then 8081 then 8079 (in that order), and they all worked okay. I will add some debugging code and see if I can replicate what you are seeing. I will have to try and dummy up some UPnP entries as well.

PR's welcome. My development process is to make changes using VS Code running in Ubuntu WSL on my Windows 11 PC, run the build script, SCP it up to my test box and run it in, see what's broken and repeat. Sometimes if it is a really difficult to track down an issue, I will do some editing on the test box and try it again without doing a full build, then make the appropriate changes on the real code. Depending on the code I am editing, I may have to restart nginx (usually if it is an include file rather than a modal or card).

[seud0nym]

Are you able to share your /var/upnp.leases file? I don't have anything in my test environment that uses UPnP, and it would be good to test with something I know is causing the issue. Thanks.

[Nixes]

yep sure

UDP:pppoe-wan:62744:*:192.168.0.230:62744:0:Teredo 192.168.0.230:62744->62744 UDP
TCP:pppoe-wan:5262:(null):192.168.0.201:22000:1673409951:NAT-PMP 5262 tcp
TCP:pppoe-wan:12215:*:192.168.0.201:22000:1673409951:syncthing-12215
TCP:pppoe-wan:39478:(null):192.168.0.230:22000:1673410068:NAT-PMP 39478 tcp
TCP:pppoe-wan:33568:*:192.168.0.230:22000:1673410068:syncthing-33568
UDP:pppoe-wan:31498:*:192.168.0.201:31498:1673409332:Parsec
UDP:pppoe-wan:22498:*:192.168.0.201:22498:1673409334:Parsec
UDP:pppoe-wan:22499:*:192.168.0.201:22499:1673409334:Parsec
UDP:pppoe-wan:22500:*:192.168.0.201:22500:1673409334:Parsec
UDP:pppoe-wan:22501:*:192.168.0.201:22501:1673409334:Parsec
UDP:pppoe-wan:30001:*:192.168.0.230:30001:1673409571:Parsec
UDP:pppoe-wan:21001:*:192.168.0.230:21001:1673409600:Parsec
UDP:pppoe-wan:21002:*:192.168.0.230:21002:1673409600:Parsec
UDP:pppoe-wan:21003:*:192.168.0.230:21003:1673409600:Parsec
UDP:pppoe-wan:21004:*:192.168.0.230:21004:1673409600:Parsec

[seud0nym]

Well, that is interesting. I modified your leases to match the subnet of my test device, and started up UPnP. Now I can't delete the test port forwarding entries I had created, plus I get the same existing mapping error if I add one.

At least I have now reproduced it and can see if I can find out what is causing it.

Thanks.

[seud0nym]

Well, I have found the issue with the failed validation. When they build the table of used port ranges from the UPnP entries, they use the WAN port as the range start and the LAN port as the range end. When they are the same, it isn't an issue, but syncthing uses different WAN/LAN ports, and that causes the problem. When I fix it and make the start and end both the UPnP WAN port, it adds the entry correctly.

Now to find out why they won't delete...

[seud0nym]

Now that I have fixed the validation issue, I can't make the deletes fail.

I have done a pre-release if you want to give it a try yourself (this includes the fixed validation). Run this command to apply:

./tch-gui-unhide -u && ./tch-gui-unhide -y

[Nixes]

That seems to have done it, I can't reproduce my problem anymore. Thank you!