fw4: do not allow setting ports for non-TCP or -UDP rules and redirects

[tk154]

Currently, it is possible to set source and destination ports for proto like all or icmp, although only tcp and udp ports are converted into the nftables ruleset.

While this might not be a problem for a protocol like ICMP, there is a security risk when the user specifies all, because then the rule/redirect applies to all L4 ports, including TCP and UDP, regardless if the user sets specific ports.

This commit checks for rules and redirects that if any port was set the protocol must be TCP and/or UDP. If not, the user will be informed via a warning, and the rule/redirect won't be applied. Should the protocol be all, it is automatically corrected
to TCP and UDP by the ucode function ensure_tcpudp.

Here are two examples for an input and a DNAT rule:

config rule
        option name             'Rule-Test'
        option src              'wan'
        option dest_port        '22'
        option proto            'all'
        option target           'ACCEPT'

config redirect
        option name             'DNAT-Test'
        option src              'wan'
        option src_dport        '22222'
        option dest             'lan'
        option dest_ip          '192.168.1.2'
        option dest_port        '22'
        option proto            'all'
        option target           'DNAT'

Resulting into the following nftables rules:

chain input_wan {
	counter packets 0 bytes 0 accept comment "!fw4: Rule-Test"
	ct status dnat accept comment "!fw4: Accept port redirections"
	jump reject_from_wan
}

chain dstnat_wan {
	meta nfproto ipv4 counter packets 0 bytes 0 dnat ip to 192.168.1.2 comment "!fw4: DNAT-Test"
}

The input rule ignores the destination port and, therefore, accepts any incoming traffic, while the DNAT rule redirects any IPv4 traffic to the given destination IP 192.168.1.2, ignoring any port settings.

[brada4]

hmm nice catch ... What about "assume tcp udp when no protocol is set"?
Can you enter this rule via LuCI?

[tk154]

hmm nice catch ... What about "assume tcp udp when no protocol is set"?

Thanks for the hint! I just noticed there is already a convenient ucode function ensure_tcpudp, that corrects the protocol to TCP and UDP if the proto is set to all.

Can you enter this rule via LuCI?

Doesn't seem to be possible, if I enter Any or ICMP the port fields disappear.

[brada4]

1. @jow- what do you think how to handle uci-forced rules?
2. OK, "generic user" is safe at least

[brada4]

Original accounts portless protos.
https://github.com/openwrt/firewall3/blob/1aef9791a21e3d15d4357060f09a7bb9ed3d6e4e/rules.c#L581

[brada4]

Emits nothing on malformed rule which s much safer tham replicating identical danger slide in fw3