UPnP: T4211: T4620 Fix upnp template

[sever-sever]

Change Summary

Fix UPnP jinja2 template, incorrect value rules instead of rule
So rules didn't work at all.
Incorrect logic do disabled rule

Address must be in the format address/mask due to https://github.com/miniupnp/miniupnp/blob/fa42d8f9316bf9c1ca14317e5a6e0d4a21365629/miniupnpd/miniupnpd.conf#L174

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (formatting, renaming)
• Refactoring (no functional changes)
• Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• Other (please describe):

Related Task(s)

• https://phabricator.vyos.net/T4620
• https://phabricator.vyos.net/T4611

Component(s) name

upnp

Proposed changes

How to test

VyOS config:

set service upnp listen '192.0.2.2'
set service upnp rule 10 action 'allow'
set service upnp rule 10 external-port-range '1024-65535'
set service upnp rule 10 internal-port-range '1024-65535'
set service upnp rule 10 ip '10.0.0.1/24'
set service upnp wan-interface 'eth0'
set service upnp  nat-pmp

Expected:

vyos@r1# cat /run/upnp/miniupnp.conf | grep "allow 10"
allow 1024-65535 10.0.0.1/24 1024-65535
[edit]
vyos@r1# 

Checklist:

• I have read the CONTRIBUTING document
• I have linked this PR to one or more Phabricator Task(s)
• I have run the components SMOKETESTS if applicable
• My commit headlines contain a valid Task id
• My change requires a change to the documentation
• I have updated the documentation accordingly

[c-po]

ip-prefix also allows IPv6 addresses. If this is the case, you should use ip-host, too and add ipv6 to the completion helper.

[sever-sever]

Done

[c-po]

I think all 4 options should be listed in the completion help: ipv4, ipv4net, ipv6, ipv6net as the validator also allows everything.

ipv4 shows <x.x.x.x>
ipv4net shows <x.x.x.x/x>
ipv6 shows <h:h:h:h:h:h:h:h>
ipv6net shows <h:h:h:h:h:h:h:h/x>

Also the completion help looks different to the user - we should list all available options - its free for us.

[c-po]

I think all 4 options should be listed in the completion help: ipv4, ipv4net, ipv6, ipv6net as the validator also allows everything.

ipv4 shows <x.x.x.x>
ipv4net shows <x.x.x.x/x>
ipv6 shows <h:h:h:h:h:h:h:h>
ipv6net shows <h:h:h:h:h:h:h:h/x>

Also the completion help looks different to the user - we should list all available options - its free for us.

[sever-sever]

@c-po Documentation says # IP/mask format must be nnn.nnn.nnn.nnn/nn
Why do we need to set x.x.x.x?

[c-po]

@c-po Documentation says # IP/mask format must be nnn.nnn.nnn.nnn/nn
Why do we need to set x.x.x.x?

As you have ipv4-host in your validator list

[sever-sever]

@c-po Documentation says # IP/mask format must be nnn.nnn.nnn.nnn/nn
Why do we need to set x.x.x.x?

As you have ipv4-host in your validator list

There is how it looks like with this 2 commits

vyos@r14# set service upnp rule 10 ip 
Possible completions:
   <x.x.x.x/x>          The IPv4 to which this rule applies
   <h:h:h:h:h:h:h:h/x>  The IPv6 to which this rule applies
                       

Validate x.x.x.x

vyos@r14# set service upnp rule 10 ip 192.0.2.1

  Error: 192.0.2.1 is not a valid IP prefix
  
  Error: 192.0.2.1 is not a valid IP host
  
  
  
  Invalid value
  Value validation failed
  Set failed

[edit]
vyos@r14# 

Validate x.x.x.x/x

vyos@r14# set service upnp rule 10 ip 192.0.2.1/24
[edit]
vyos@r14# set service upnp rule 10 ip 192.0.2.0/24
[edit]
vyos@r14# 

I don't expect x.x.x.x, I'm expecting address or prefix x.x.x.x/x

[sever-sever]

I excluded the IPv6 option as the daemon doesn't like IPv6 for rule x ip (line 98 in generated config)
Also, anchor option was commented out (line 74 in generated config)

vyos@r14# miniupnpd -vv -f /run/upnp/miniupnp.conf 
invalid option in file /run/upnp/miniupnp.conf line 74 : anchor=VyOS
parsing error file /run/upnp/miniupnp.conf line 98 : allow 1024