search

openwrt / packages

Community maintained packages for OpenWrt. Documentation for submitting pull requests is in CONTRIBUTING.md
GNU General Public License v2.0
3.98k stars 3.47k forks source link

mwan3: ip6tables v1.8.3 (legacy): host/network `[redacted-ipv4-address]' not found #13003

Closed brianjmurrell closed 3 years ago

brianjmurrell commented 4 years ago

Maintainer: @feckert Environment: x86, 19.07.3, mwan3-2.8.12-1

Description: When restarting mwan3 with /etc/init.d/mwan3 restart I get the following output:

ip6tables v1.8.3 (legacy): host/network `[redacted-ipv4-address]' not found
Try `ip6tables -h' or 'ip6tables --help' for more information.
iptables v1.8.3 (legacy): invalid mask `127' specified
Try `iptables -h' or 'iptables --help' for more information.
ipset v7.3: The set with the given name does not exist
ip6tables v1.8.3 (legacy): host/network `[redacted-ipv4-address]' not found
Try `ip6tables -h' or 'ip6tables --help' for more information.
iptables v1.8.3 (legacy): invalid mask `127' specified
Try `iptables -h' or 'iptables --help' for more information.
ip6tables v1.8.3 (legacy): host/network `[redacted-ipv4-address]' not found
Try `ip6tables -h' or 'ip6tables --help' for more information.
iptables v1.8.3 (legacy): invalid mask `127' specified
Try `iptables -h' or 'iptables --help' for more information.
ip6tables v1.8.3 (legacy): host/network `[redacted-ipv4-address]' not found
Try `ip6tables -h' or 'ip6tables --help' for more information.
iptables v1.8.3 (legacy): invalid mask `127' specified
Try `iptables -h' or 'iptables --help' for more information.

Clearly these are IPv4 addresses that are trying to be applied with IPv6 iptables rules.

At least in the ip6tables errors case, that would be from this rule:

config rule '[redacted]'
    option dest_port '[redacted]'
    option proto 'tcp'
    option sticky '0'
    option use_policy 'wan1_only'
    option dest_ip '[redacted-ipv4-address]/31'

I'm not sure what in the above rule is supposed to tell mwan3 to only try to apply this with iptables and not ip6tables but whatever it is, clearly it's not working.

The other errors about the 127 mask, this appears to be the rule:

config rule '[redacted]'
    option dest_ip '[redacted-ipv6-address]/127'
    option dest_port '[redacted]'
    option proto 'tcp'
    option sticky '0'
    option use_policy 'wan1_only'

and mwan3 is trying to apply that with IPv4 iptables

jamesmacwhite commented 4 years ago

@brianjmurrell If you explicitly specify option family 'ipv4' and option family 'ipv6' on these rules does the error still happen? By default I believe without the family option mwan3 will try and apply to both IPv4 and IPv6 chains, regardless if it is valid or not.

For rules, the default family option value would be all, so your IPv4 rule is being applied to the IPv6 chain and the IPv6 rule is also being applied to IPv4 without specifying the family directly.

https://openwrt.org/docs/guide-user/network/wan/multiwan/mwan3#rule_configuration

You may be seeing this because of the lack of the family option. Previously I think in older mwan3 versions the iptables error output was being masked by /dev/null so this might have been happening for a while but mwan3 would have masked the errors. More recently to help with debugging, iptables errors are no longer redirected to /dev/null and will be shown when interacting with mwan3 i.e. mwan3 restart.

https://github.com/openwrt/packages/commit/702a104f9c516fdddd5e71207d1ad91eb70f9a41#diff-ccaf6dffccf2fc134d63b5c33d5ee8e1

Also, slightly unrelated but you don't need to specify option sticky '0' for rules as it is the default, so you can save a line on your rules!

Hope this helps!

jamesmacwhite commented 4 years ago

I think the general recommendation should be when using options like src_ip/dest_ip, you should always set the family explicitly as you'll be using a specific address family at this point but you don't always need to do it e.g. the default https sticky rule as that concerns dest_port only and would be valid to be applied to both chains with the default all value.

I think it probably doesn't help is the mwan3 documentation has been mostly written from a IPv4 only perspective. None of the example rules on the wiki set the family value when using IPv4 only address values. More recently I've updated some of the examples and information but it could perhaps be clearer. There's also a lot of information dating back to OpenWrt 12.09, which probably doesn't need to be on there anymore.

brianjmurrell commented 4 years ago

FWIW, the luci page for mwan3 rules doesn't have a family option.

But surely the family can be detected by the address/mask, etc. when it does need to be applied to one of iptables or ip6tables.

feckert commented 4 years ago

@brianjmurrell Please try my staged P/R for family selection on rules.

If we use it this way we have to change the destination address and the source address so that depending on the family the correct help text or input check is performed.

feckert commented 4 years ago

So that we can implement this correctly in the LuCI I would suggest that we also need to extend mwan3 so that we have src_ip/src_ip6 or dest_ip/dest_ip6.

@aaronjg What's your opinion? Up to now all IPv6 did not work at all and we did a lot of changes recently in IPv6. LuCI up to now supports only IPv4 because we have no family option added in the LuCI

aaronjg commented 4 years ago

Please try my staged P/R for family selection on rules.

If you are doing a new version of the luci package, we should also add the source interface option. I had this staged on my local branch. I should have made a P/R earlier ... https://github.com/openwrt/luci/commit/ba91b5ea7c443b6099808a380533cc1111c9957f

So that we can implement this correctly in the LuCI I would suggest that we also need to extend mwan3 so that we have src_ip/src_ip6 or dest_ip/dest_ip6.

I think it would be better to keep the mwan3 config simpler and not add additional options if possible. Why does luci need to have separate fields?

I haven't tried to add rules based on ipv6 IP yet, but I think it should be possible to do it on luci if you just pick an ipv6 address? Until the family option is added, would still through an error when it tried to add it to the ipv4 firewall, but it should go through on ipv6. Also, (undocumented) feature is that rather than an ip address, you can pass a hostname to mwan3 and then iptables will resolve it to an ip address when it adds the rule, either ipv4 with iptables or ipv6 with ip6tables.

Another thought: it seems like often when people are adding rules based on ip address, they want to add the same policy to several ip addresses, and end up having to create a separate rule for each. I wonder if we can convert the src and dest ip address to a config list, and then when multiple are specified, mwan3 could create an ipset for those rules and use the ipset in creating the firewall rule.

jamesmacwhite commented 4 years ago

@brianjmurrell Ah good point, I personally didn't think about LuCI really. My bad.

If it helps, here's an example of IPv6 rules as shown by LuCI. I would have set them by editing the mwan3 config directly, but really the only missing piece is the family option, this is how LuCI parses them:

image

image

image

image

feckert commented 4 years ago

Please try my staged P/R for family selection on rules.

If you are doing a new version of the luci package, we should also add the source interface option. I had this staged on my local branch. I should have made a P/R earlier ... openwrt/luci@ba91b5e

I will cherrypick them to my pullrequest.

So that we can implement this correctly in the LuCI I would suggest that we also need to extend mwan3 so that we have src_ip/src_ip6 or dest_ip/dest_ip6.

I think it would be better to keep the mwan3 config simpler and not add additional options if possible. Why does luci need to have separate fields?

From my point of view the handling would be easier and you could work with depends. So that depending on the family option the right fields are displayed. Additional the value check is easier to handle and the address comment for ipv4 or ipv6 is then individual.

Another thought: it seems like often when people are adding rules based on ip address, they want to add the same policy to several ip addresses, and end up having to create a separate rule for each. I wonder if we can convert the src and dest ip address to a config list, and then when multiple are specified, mwan3 could create an ipset for those rules and use the ipset in creating the firewall rule.

We can later convert this into a list element. But we also have to adapt the mwan3 to create an ipset.

aaronjg commented 4 years ago

I will cherrypick them to my pullrequest.

Thank you.

From my point of view the handling would be easier and you could work with depends.

That makes sense. We already sort of have this issue with specifying port and protocol, where port doesn't work with protocol 'any'.

I think it should be possible with one ip field though, and not needing to make any change to mwan3. If 'any' we can hide/disable the ip field. Then with ipv4/ipv6 we dynamically choose the validation.

We can later convert this into a list element. But we also have to adapt the mwan3 to create an ipset.

Sure, it will require some mwan3 changes. I can create a feature request issue for this, and then you can assign it to me. If we do this though, then we can have a rule with family 'any' and then it could have both ipv6 and ipv4 addresses in the ipset.

aaronjg commented 4 years ago

@jamesmacwhite pointed out here (#13088) that mwan3 already supports multiple ip addresses when they are separated by commas. This is because iptables will parse them and create separate rules for each. So it seems like it would be difficult to to have a luci validator for the ip addresses without breaking current configuration, since the 'ipaddr' datatype will not work out of the box.

I think the 'correct' way to do this would be to change src and dst ip to list elements, then update the luci code to handle the config option and do the proper validation, and finally write a migration script to put in /etc/uci-defaults so that peoples configurations are not broken when they upgrade.

FWIW, it is not totally unprecedented to jam multiple options into a single uci config option. The firewall zone config does this with a space separated list for network.

jamesmacwhite commented 4 years ago

@aaronjg Yes, this is a good point to be aware of. I have rules using the comma method on src_ip and dest_ip as well as dest_port so any changes here need to be careful not to break existing configurations here. It's not something widely documented I don't think, but if you know iptables supports it, I'm sure others are doing it as well, as it cuts down rules.

I'm not sure src_ipv6 and dest_ipv6 is the ideal way if I'm honest. Right now you can still use the single src_ip and dest_ip fields/config value, you just need to define a family value. If that can be improved on while keeping the single src_ip and dest_ip fields, that makes more sense.

I would consider having to do src_ipv6 and dest_ipv6 even more complex than it is now, but that's just my thoughts.

feckert commented 4 years ago

@aaronjg Yes, this is a good point to be aware of. I have rules using the comma method on src_ip and dest_ip as well as dest_port so any changes here need to be careful not to break existing configurations here. It's not something widely documented I don't think, but if you know iptables supports it, I'm sure others are doing it as well, as it cuts down rules.

If I look at it that way from the LuCI's point of view, it is of course bad. The ipv4 and ipv6 validation check does not work anymore if I write a comma separated value here. If this is a use case, then we have to change the option to a list element. To make it work again with an existing configuration. If so we have to write an upgrade script that converts the value from a csv to a uci list element.

I'm not sure src_ipv6 and dest_ipv6 is the ideal way if I'm honest. Right now you can still use the single src_ip and dest_ip fields/config value, you just need to define a family value. If that can be improved on while keeping the single src_ip and dest_ip fields, that makes more sense.

I would consider having to do src_ipv6 and dest_ipv6 even more complex than it is now, but that's just my thoughts.

Usually we can choose IPv4, IPv6 or any for the family. If we choose IPv4 then there are the options src_ip and dest_ip as well as src_port and dest_port and the same is true for IPv6

With any the rule applies to both IPv4 and IPv6. So there is no src_ip and no dest_ip but src_port and dest_port. So that this fits better into the LuCI and we can work with dependencies here, I suggest that we still use src_ip6 and dest_ip6, this would simplify things from my point of view in the LuCI There is no use case to mix IPv6 and IPv4 src_ip and dst_ip?

jamesmacwhite commented 4 years ago

@feckert Fair points there. If the introduction of src_ipv6 and dest_ipv6 is made, I'm happy to update the documentation here:

https://openwrt.org/docs/guide-user/network/wan/multiwan/mwan3?s[]=mwan3#rule_configuration

As otherwise it will not be entirely accurate going forward.

I guess it's just finding the balance between LuCI and directly writing rules by editing /etc/config/mwan3 without LuCI and making it all work, which I appreciate requires careful consideration.

aaronjg commented 4 years ago

There is no use case to mix IPv6 and IPv4 src_ip and dst_ip?

I am not currently using it in my setup, but I could imagine a use case. If there is a dual stack setup, and the user wants the same rule to apply for a hosts IPv4 and IPv6 address.

we must also hide the src_port and dest_port if the proto is not set to udp or tcp.

We actually should probably change this to be a multiselect like the luci-firewall app does, and allow a user to select 'udp' and 'tcp' and then hide the port if any other protocols (or no protocol) is selected.

My preference would be to try to be to have the mwan3 config be similar to the firewall config (both on the luci side, and in the config files), since users will likely be more familiar with this, and it can soften the learning curve somewhat.

feckert commented 4 years ago

My preference would be to try to be to have the mwan3 config be similar to the firewall config (both on the luci side, and in the config files), since users will likely be more familiar with this, and it can soften the learning curve somewhat.

good point :+1: @aaronjg I just see that if you set the address family to ipv4 and ipv6 in the firewall, the source ip option and the destination ip option will not disappear! Does this make sense? @jow- Or is this a bug in the luci-app-firewall?

brianjmurrell commented 4 years ago

@brianjmurrell If you explicitly specify option family 'ipv4' and option family 'ipv6' on these rules does the error still happen?

No. Other issues in this ticket about UI exposure of the family setting, etc. are still relevant though, of course.

jamesmacwhite commented 4 years ago

Yep. I think this issue triggered a few PRs and identified inconsistencies with LuCI. There should be a fix coming for LuCI with the family option: https://github.com/openwrt/luci/pull/4349

There's also a PR for changing some of the default in LuCI that are mismatched with mwan3track.

brianjmurrell commented 3 years ago

I'm still seeing this on 19.07.7. Is there no intention to fix this on that branch?

feckert commented 3 years ago

@brianjmurrell As already mentioned, this is because a src_ip option or a dest_ip option is usually set and this is then set for both ipv4 and ipv6. This has always been the case, except that this log message is no longer redirected to /dev/null.

In the master or next stable release 21.02, I no longer worked on the distinction for time reasons. But I will have another look at it next week.