Implement blacklisted interface monitor to prevent IPv6 default routes on them #29
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LaakkonenJussi
commented
May 12, 2022
|
If this should be always active I think this ofono bit could be even a separate plugin, to always block those default routes set via RA for the additional |
abranson
approved these changes
May 13, 2022
LaakkonenJussi
changed the title
|
But now it doesn't anymore. I think the move to the plugin broke something. |
[service] Add wrappers for network and index lookup, getter for ipconfig. JB#58105 Add getter for ipconfig method to get IPv4 or IPv6 method. Report CONNMAN_IPCONFIG_TYPE_UNDEFINED for any other request type. Add wrappers for service lookup by index and getting network struct for plugins to use.
LaakkonenJussi
force-pushed
the
jb58105
branch
2 times, most recently
from
9899e8e
to
2b5886a
Compare
I think this is now fixed. And it would be good to get some eyes on the code as well. |
LaakkonenJussi
force-pushed
the
jb58105
branch
3 times, most recently
from
0082b1f
to
d1c9f38
Compare
monich
reviewed
Jun 27, 2022
monich
reviewed
Jun 27, 2022
monich
reviewed
Jun 27, 2022
monich
reviewed
Jun 27, 2022
monich
reviewed
Jun 27, 2022
monich
reviewed
Jun 27, 2022
monich
reviewed
Jun 27, 2022
spiiroin
approved these changes
Jun 28, 2022
mkosola
approved these changes
Jun 29, 2022
Tomin1
approved these changes
Jun 29, 2022
[inet] Support setting metric value with IPv6 network route. JB#58105
Add support for metric value when managing IPv6 network routes. By
default the value was 1 (for VPNs) but in some cases the value may need
to be defined, e.g., when removing a specific external route. This is
achieved with connman_inet_{add,del}_ipv6_network_route_with_metric()
functions and that are eventually called by the pre-existing
network|host route adding functions.
Also add a wrapper to __connman_inet_is_any_addr() for plugins.
[inet] Do not add RTF_HOST for IPv6 default route. JB#58105 When adding a default route for an IPv6 interface it is not necessary to set the RTF_HOST flag. When adding a default route the gateway should be present and then RTF_GATEWAY is set.
[rtnl] Extend notify and better support IPv6 routes set via RA. JB#58105 Extend the notify to support IPv6 better when gateways are added and removed. These use the destination as well as metric value for more details about the routes. Metric for IPv6 is retrieved from RTA_PRIORITY as it is used by kernel, see, e.g., https://github.com/torvalds/linux/blob/master/net/ipv6/route.c Add toggle to pass also the routes set by RA (protocol type RTPROT_RA) to the functions that do request it. For this an additional field to the notify and a function to enable RTPROT_RA support was added. This could be later re-designed to support the different user-specific protocol types defined in linux/rtnetnlink.h but as of now includes only RA-types. This change adds two callbacks to the struct connman_rtnl while keeping rest of the content intact. Each callback gets also the rtm_protocol info so the responsibility of processing decision is on the component implementing the callback. After the user-specific protocol types can be added this proves to be quite useful in filtering incoming IPv6 netlink messages. Also support setting address family for the rtnl request. Keep AF_INET for the internal rtnl.c functions but support also AF_INET6 and AF_UNSPEC. The value is set only via internal/plugin calls when needed. In some cases it is also useful to have route request available for the plugins if there is a plugin monitoring a more complex network configuration where, for example, additional not-managed interfaces (blacklisted) are getting a default route assigned by kernel via route advertisements and should ensure that the route is not used by default when ConnMan assigned route is to be preferred.
[ipconfig] Add wrapper to conf type getter for plugins. JB#58105
[device] Add wrapper to check if interface is blacklisted for plugins. JB#58105
…ed ifs [blacklist monitor] Plugin to prevent IPv6 default routes on blacklisted ifs. JB#58105 Introduce a new plugin to prevent non-monitored and managed interfaces set in blacklist configuration from enabling any IPv6 default routes when ConnMan has a service connected. The routes may be added by RA for IPv6 through kernel, or an external service can add these for the blacklisted interfaces, like DHCP, which is not supported yet. The same RTPROT_BOOT and RTPROT_KERNEL messages types in addition to RTPROT_RA are supported. Plugin requests rtnl.c to pass RTPROT_RA type messages to be processed. The basic mechanism is by monitor via new rtnl feature the exact IPv6 default routes that are added and removed for any blacklisted interface. These are log them into a list that is used for excess default route removal when there is a change on the default service or its IPv6 configuration. The list contains only one default route per index and metric pair, gateway is changeable. If there are changes on default service or its IPv6 configuration the list of the default routes is cleared. If there is a default service already connected new default routes from these blacklisted interfaces are prevented unless the connected default service index matches the blacklisted interface. This allows to keep the default routes of an service that is mainly managed via plugin. Furthermore, the blacklist monitor requests IPv6 routes when the default service changes. If the default service change and previous default service was using a blacklisted interface it would have a potentially conflicting IPv6 route set before the new default service's route. By requesting the routes, and the index has changed as default service changes this plugin can make it sure that the services using a blacklisted, i.e., non-managed interface, will not conflict with the current service connection. As a result in the case when, e.g., cellular that has support for both IPv4 and IPv6 remains active when a WLAN is connected. ConnMan does not manage IPv6 and relies on kernel so in kernel routing table there can be a route left on cellular if the interface is blacklisted, and hence the interface name usually has lower ASCII index than a 'w' the cellular interface (e.g., rmnet_data) remains on top of the wlan interface. Thus, traffic would go on IPv4 over WLAN and IPv6 would use mobile data. Worse case is when WLAN does not have IPv6 at all but only IPv4 exists. That will cause some confusion on user. In such case the existing route information is requested from kernel and the IPv6 routes that are on blacklisted interfaces are removed from routing table. If the ipconfig is set as manual or fixed the routes are kept in a restore list, which is utilized when the interface is used by a default service. This is imperative because when a network with either of those ipconfig methods have their routes removed when, e.g., WLAN is connected they are not restored from anywhere after WLAN is dropped out. This change ensures that they are added back once the service has become the default service from which the route was removed earlier. Therefore, resuming connection on SLAAC or DHCPv6 enabled connection remains unaffected.
This contains basic unit tests and two simulations of the process where blacklisted is default, then manages is default and then blacklisted is default again, latter case disconnects blacklisted before coming back as default.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a new plugin to monitor IPv6 default routes on interfaces that are set as blacklisted in ConnMan. When there are excess routes, i.e., on interfaces that are not active as a default route they will be removed and if the service using the interface is connected at least, the routes are retained in a list to be restored when the service comes as the default.
Rtnl side required some changes to facilitate support for this by including RTPROT_RA messages to be handled, which will be improved later on to support any user-specificed protocol ignored by kernel by default. Those are information for the daemons only, which is just what this case requires.
All in all this does fix the issue of multiple blacklisted interfaces getting default route being set by kernel or route advertisements which would conflict the current active network setup maintained by ConnMan. Usually the blacklisted interface names have lower ascii value to begin with than, for example, WLAN, thus those routes apparently, with the same metric would be used first by kernel making network in some cases completely inoperable on IPv6.