summaryrefslogtreecommitdiff
path: root/src/vpn
Commit message (Collapse)AuthorAgeFilesLines
* all: drop emacs file variables from source filesThomas Haller2019-06-114-4/+0
| | | | | | | | | | | | | | | | | | | | | | We no longer add these. If you use Emacs, configure it yourself. Also, due to our "smart-tab" usage the editor anyway does a subpar job handling our tabs. However, on the upside every user can choose whatever tab-width he/she prefers. If "smart-tabs" are used properly (like we do), every tab-width will work. No manual changes, just ran commands: F=($(git grep -l -e '-\*-')) sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}" sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}" Check remaining lines with: git grep -e '-\*-' The ultimate purpose of this is to cleanup our files and eventually use SPDX license identifiers. For that, first get rid of the boilerplate lines.
* dispatcher: replace guint call-id by opaque NMDispatcherCallIdThomas Haller2019-05-271-9/+13
| | | | | | | | | A guint value can wrap, so we would need to check that we don't allocate duplicate IDs (which we currently don't, and it's likely never to actually hit). Just expose the (opaque) pointer of the call-id. We still keep a "request_id", but that is only for logging purpose.
* pacrunner: refactor pacrunner to use GDBusConnectionThomas Haller2019-05-131-19/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - use GDBusConnection instead of GDBusProxy. - rename "call-id" to "conf-id". It's really not a "call" but configuration that gets added and NMPacrunnerManager ensures that the configuration is send to pacrunner. - let "conf-id" keep a reference to NMPacrunnerManager. For one, when we remove configurations we need to call DestroyProxyConfiguration to remove it again. We cannot just abort the requests but must linger around until our configuration is properly cleaned up. Hence, we anyway cannot destroy the NMPacrunnerManager earlier. With respect to fixing shutdown not to leak anything, this merely means that we must wait (and iterate the main loop) as long as NMPacrunnerManager singleton still exits (that is anyway the plan how to fix shutdown). With these considerations it's also clear that our D-Bus calls must have a stricter timeout: NM_SHUTDOWN_TIMEOUT_MS. This is also nice because nm_pacrunner_manager_remove() no longer needs a manager parameter, it can just rely on having a reference to the manager. - for logging the configuration IDs, don't log pointer values. Logging pointer values should be avoided as it defeats ASLR. Instead, give them a "log_id" number. - pacrunner is a D-Bus activatable service. D-Bus activatable services needs special care. We don't want to start it over and over again. Instead, we only try to "StartServiceByName" if - we have any configuration to add - if pacrunner is currently confirmed not to be running (by watching name owner changes) - we didn't try to start it already. That means, only start it at the beginning and afterwards set a flag to block it. When we see pacrunner appear on D-Bus we always clear that flag, that means if pacrunner drops of, we will try to restart it (once).
* core: use nm_connection_get_setting_ip_config() helperThomas Haller2019-03-051-5/+1
| | | | (cherry picked from commit 2be022ad68158acd544704244a7d3193aa6545a7)
* policy: treat WireGuard devices as VPN for DNSThomas Haller2019-02-141-2/+0
| | | | | | | WireGuard devices are (will be) regular NMDevice implementations, but NMDnsManager should treat them like VPN. For that, reuse the device's type and nm_device_get_route_metric_default().
* all: drop unnecessary includes of <errno.h> and <string.h>Thomas Haller2019-02-122-4/+0
| | | | | "nm-macros-interal.h" already includes <errno.h> and <string.h>. No need to include it everywhere else too.
* vpn: add route to vpn gw when parent has a default device routeBeniamino Galvani2019-02-041-12/+28
| | | | | | | | | | | When the parent device has a device default route (i.e. without gateway) and we establish a VPN on top of it, 'ip route get' for the VPN gateway returns a device route, which is the same result we get for an unreachable VPN gateway. However it is necessary to add the route to the gateway or otherwise it will possibly become unreachable once the VPN gets activated. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/93
* all/trivial: rename NM_UTILS_LOOKUP_STR() to have "_A" suffixThomas Haller2019-01-151-7/+10
| | | | | | | | | | | | | | | | | NM_UTILS_LOOKUP_STR() uses alloca(). Partly to avoid the overhead of malloc(), but more important because it's convenient to use. It does not require to declare a varible to manage the lifetime of the heap allocation. It's quite safe, because the stack allocation is of a fixed size of only a few bytes. Overall, I think the convenience that we get (resulting in simpler code) outweighs the danger of stack allocation in this case. It's still worth it. However, as it uses alloca(), it still must not be used inside a (unbound) loop and it is obviously a macro. Rename the macros to have a _A() suffix. This should make the peculiarities more apparent.
* platform: merge NMPlatformError with nm-errorThomas Haller2018-12-271-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | Platform had it's own scheme for reporting errors: NMPlatformError. Before, NMPlatformError indicated success via zero, negative integer values are numbers from <errno.h>, and positive integer values are platform specific codes. This changes now according to nm-error: success is still zero. Negative values indicate a failure, where the numeric value is either from <errno.h> or one of our error codes. The meaning of positive values depends on the functions. Most functions can only report an error reason (negative) and success (zero). For such functions, positive values should never be returned (but the caller should anticipate them). For some functions, positive values could mean additional information (but still success). That depends. This is also what systemd does, except that systemd only returns (negative) integers from <errno.h>, while we merge our own error codes into the range of <errno.h>. The advantage is to get rid of one way how to signal errors. The other advantage is, that these error codes are compatible with all other nm-errno values. For example, previously negative values indicated error codes from <errno.h>, but it did not entail error codes from netlink.
* all: don't use static buffer for nm_utils_inet*_ntop()Thomas Haller2018-12-191-15/+16
| | | | | | | | | | While nm_utils_inet*_ntop() accepts a %NULL buffer to fallback to a static buffer, don't do that. I find the possibility of using a static buffer here error prone and something that should be avoided. There is of course the downside, that in some cases it requires an additional line of code to allocate the buffer on the stack as auto-variable.
* core: improve and fix keeping connection active based on ↵Thomas Haller2018-12-092-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "connection.permissions" By setting "connection.permissions", a profile is restricted to a particular user. That means for example, that another user cannot see, modify, delete, activate or deactivate the profile. It also means, that the profile will only autoconnect when the user is logged in (has a session). Note that root is always able to activate the profile. Likewise, the user is also allowed to manually activate the own profile, even if no session currently exists (which can easily happen with `sudo`). When the user logs out (the session goes away), we want do disconnect the profile, however there are conflicting goals here: 1) if the profile was activate by root user, then logging out the user should not disconnect the profile. The patch fixes that by not binding the activation to the connection, if the activation is done by the root user. 2) if the profile was activated by the owner when it had no session, then it should stay alive until the user logs in (once) and logs out again. This is already handled by the previous commit. Yes, this point is odd. If you first do $ sudo -u $OTHER_USER nmcli connection up $PROFILE the profile activates despite not having a session. If you then $ ssh guest@localhost nmcli device you'll still see the profile active. However, the moment the SSH session ends, a session closes and the profile disconnects. It's unclear, how to solve that any better. I think, a user who cares about this, should not activate the profile without having a session in the first place. There are quite some special cases, in particular with internal activations. In those cases we need to decide whether to bind the activation to the profile's visibility. Also, expose the "bind" setting in the D-Bus API. Note, that in the future this flag may be modified via D-Bus API. Like we may also add related API that allows to tweak the lifetime of the activation. Also, I think we broke handling of connection visiblity with 37e8c53eeed "core: Introduce helper class to track connection keep alive". This should be fixed now too, with improved behavior. Fixes: 37e8c53eeed579fe34a68819cd12f3295d581394 https://bugzilla.redhat.com/show_bug.cgi?id=1530977
* core: add checks on connection default propertiesBeniamino Galvani2018-12-011-1/+1
| | | | | | | | | | Add a new CON_DEFAULT() macro that places a property name into a special section used at runtime to check whether it is a supported connection default. Unfortunately, this mechanism doesn't work for plugins so we have to enumerate the connection defaults from plugins in the daemon using another CON_DEFAULT_NOP() macro.
* vpn: disconnect signal handlers from proxy in NMVpnConnection::dispose()Thomas Haller2018-09-141-0/+3
| | | | | | | | We cannot be sure who holds a reference to the proxy, and who is gonna call us back after the VPN connection instance is destroyed. (cherry picked from commit 6ebb9091d272c7af4e1eaab4a110f7de37fb2b4d)
* vpn: fix assertion during "SecretsRequired" in unexpected stateThomas Haller2018-09-141-3/+7
| | | | | | | | | | | | | | | | | | Got this assertion: NetworkManager[12939]: <debug> [1536917977.4868] active-connection[0x563d8fd34540]: set state deactivated (was deactivating) ... NetworkManager[12939]: nm-openvpn[1106] <info> openvpn[1132]: send SIGTERM NetworkManager[12939]: nm-openvpn[1106] <info> wait for 1 openvpn processes to terminate... NetworkManager[12939]: nm-openvpn[1106] <warn> openvpn[1132] exited with error code 1 NetworkManager[12939]: <info> [1536917977.5035] vpn-connection[0x563d8fd34540,2fdeaea3-975f-4325-8305-83ebca5eaa26,"my-openvpn-Red-Hat",0]: VPN plugin: requested secrets; state disconnected (9) NetworkManager[12939]: plugin_interactive_secrets_required: assertion 'priv->vpn_state == STATE_CONNECT || priv->vpn_state == STATE_NEED_AUTH' failed Meaning. We should either ensure that secrets_required_cb() signal callback is disconnected from proxy's signal, or we gracefully handle callbacks at unexpected moments. Do the latter. (cherry picked from commit 92344dd0848f2353b81eab5b0b294eb92aaf59c0)
* core: add support for connection.llmnrBeniamino Galvani2018-09-061-0/+1
|
* core: add nm_config_data_get_connection_default_int64()Beniamino Galvani2018-09-061-7/+4
|
* settings: use delegation instead of inheritance for NMSettingsConnection and ↵Thomas Haller2018-08-281-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | NMConnection NMConnection is an interface, which is implemented by the types NMSimpleConnection (libnm-core), NMSettingsConnection (src) and NMRemoteConnection (libnm). NMSettingsConnection does a lot of things already: 1) it "is-a" NMDBusObject and exports the API of a connection profile on D-Bus 2) it interacts with NMSettings and contains functionality for tracking the profiles. 3) it is the base-class of types like NMSKeyfileConnection and NMIfcfgConnection. These handle how the profile is persisted on disk. 4) it implements NMConnection interface, to itself track the settings of the profile. 3) and 4) would be better implemented via delegation than inheritance. Address 4) and don't let NMSettingsConnection implemente the NMConnection interface. Instead, a settings-connection references now a NMSimpleConnection instance, to which it delegates for keeping the actual profiles. Advantages: - by delegating, there is a clearer separation of what NMSettingsConnection does. For example, in C we often required casts from NMSettingsConnection to NMConnection. NMConnection is a very trivial object with very little logic. When we have a NMConnection instance at hand, it's good to know that it is *only* that simple instead of also being an entire NMSettingsConnection instance. The main purpose of this patch is to simplify the code by separating the NMConnection from the NMSettingsConnection. We should generally be aware whether we handle a NMSettingsConnection or a trivial NMConnection instance. Now, because NMSettingsConnection no longer "is-a" NMConnection, this distinction is apparent. - NMConnection is implemented as an interface and we create NMSimpleConnection instances whenever we need a real instance. In GLib, interfaces have a performance overhead, that we needlessly pay all the time. With this change, we no longer require NMConnection to be an interface. Thus, in the future we could compile a version of libnm-core for the daemon, where NMConnection is not an interface but a GObject implementation akin to NMSimpleConnection. - In the previous implementation, we cannot treat NMConnection immutable and copy-on-write. For example, when NMDevice needs a snapshot of the activated profile as applied-connection, all it can do is clone the entire NMSettingsConnection as a NMSimpleConnection. Likewise, when we get a NMConnection instance and want to keep a reference to it, we cannot do that, because we never know who also references and modifies the instance. By separating NMSettingsConnection we could in the future have NMConnection immutable and copy-on-write, to avoid all unnecessary clones.
* all: don't use gchar/gshort/gint/glong but C typesThomas Haller2018-07-111-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We commonly don't use the glib typedefs for char/short/int/long, but their C types directly. $ git grep '\<g\(char\|short\|int\|long\|float\|double\)\>' | wc -l 587 $ git grep '\<\(char\|short\|int\|long\|float\|double\)\>' | wc -l 21114 One could argue that using the glib typedefs is preferable in public API (of our glib based libnm library) or where it clearly is related to glib, like during g_object_set (obj, PROPERTY, (gint) value, NULL); However, that argument does not seem strong, because in practice we don't follow that argument today, and seldomly use the glib typedefs. Also, the style guide for this would be hard to formalize, because "using them where clearly related to a glib" is a very loose suggestion. Also note that glib typedefs will always just be typedefs of the underlying C types. There is no danger of glib changing the meaning of these typedefs (because that would be a major API break of glib). A simple style guide is instead: don't use these typedefs. No manual actions, I only ran the bash script: FILES=($(git ls-files '*.[hc]')) sed -i \ -e 's/\<g\(char\|short\|int\|long\|float\|double\)\>\( [^ ]\)/\1\2/g' \ -e 's/\<g\(char\|short\|int\|long\|float\|double\)\> /\1 /g' \ -e 's/\<g\(char\|short\|int\|long\|float\|double\)\>/\1/g' \ "${FILES[@]}"
* all: use the elvis operator wherever possibleLubomir Rintel2018-05-101-3/+3
| | | | | | | | | | | | | | | | | | | | | Coccinelle: @@ expression a, b; @@ -a ? a : b +a ?: b Applied with: spatch --sp-file ternary.cocci --in-place --smpl-spacing --dir . With some manual adjustments on spots that Cocci didn't catch for reasons unknown. Thanks to the marvelous effort of the GNU compiler developer we can now spare a couple of bits that could be used for more important things, like this commit message. Standards commitees yet have to catch up.
* all: remove consecutive empty linesBeniamino Galvani2018-04-301-1/+0
| | | | | | | Normalize coding style by removing consecutive empty lines from C sources and headers. https://github.com/NetworkManager/NetworkManager/pull/108
* manager: ensure valid specific_object path is passed to _new_active_connection()Thomas Haller2018-04-181-0/+1
| | | | | | From the D-Bus layer, no specific-object is represented by "/". We should early on normalize such values to NULL, and not expect or handle them later (like during _new_active_connection()).
* core: specify an activation reason for active connectionsBeniamino Galvani2018-04-082-0/+3
| | | | | | Specify a reason when creating active connections. The reason will be used in the next commit to tell whether slaves must be reconnected or not if a master has autoconnect-slaves=yes.
* core/dbus: rework D-Bus implementation to use lower layer GDBusConnection APIThomas Haller2018-03-122-42/+48
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we used the generated GDBusInterfaceSkeleton types and glued them via the NMExportedObject base class to our NM types. We also used GDBusObjectManagerServer. Don't do that anymore. The resulting code was more complicated despite (or because?) using generated classes. It was hard to understand, complex, had ordering-issues, and had a runtime and memory overhead. This patch refactors this entirely and uses the lower layer API GDBusConnection directly. It replaces the generated code, GDBusInterfaceSkeleton, and GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager and static descriptor instances of type GDBusInterfaceInfo. This adds a net plus of more then 1300 lines of hand written code. I claim that this implementation is easier to understand. Note that previously we also required extensive and complex glue code to bind our objects to the generated skeleton objects. Instead, now glue our objects directly to GDBusConnection. The result is more immediate and gets rid of layers of code in between. Now that the D-Bus glue us more under our control, we can address issus and bottlenecks better, instead of adding code to bend the generated skeletons to our needs. Note that the current implementation now only supports one D-Bus connection. That was effectively the case already, although there were places (and still are) where the code pretends it could also support connections from a private socket. We dropped private socket support mainly because it was unused, untested and buggy, but also because GDBusObjectManagerServer could not export the same objects on multiple connections. Now, it would be rather straight forward to fix that and re-introduce ObjectManager on each private connection. But this commit doesn't do that yet, and the new code intentionally supports only one D-Bus connection. Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start() succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough for the moment. It could be easily extended later, for example with polling whether the system bus appears (like was done previously). Also, restart of D-Bus daemon isn't supported either -- just like before. Note how NMDBusManager now implements the ObjectManager D-Bus interface directly. Also, this fixes race issues in the server, by no longer delaying PropertiesChanged signals. NMExportedObject would collect changed properties and send the signal out in idle_emit_properties_changed() on idle. This messes up the ordering of change events w.r.t. other signals and events on the bus. Note that not only NMExportedObject messed up the ordering. Also the generated code would hook into notify() and process change events in and idle handle, exhibiting the same ordering issue too. No longer do that. PropertiesChanged signals will be sent right away by hooking into dispatch_properties_changed(). This means, changing a property in quick succession will no longer be combined and is guaranteed to emit signals for each individual state. Quite possibly we emit now more PropertiesChanged signals then before. However, we are now able to group a set of changes by using standard g_object_freeze_notify()/g_object_thaw_notify(). We probably should make more use of that. Also, now that our signals are all handled in the right order, we might find places where we still emit them in the wrong order. But that is then due to the order in which our GObjects emit signals, not due to an ill behavior of the D-Bus glue. Possibly we need to identify such ordering issues and fix them. Numbers (for contrib/rpm --without debug on x86_64): - the patch changes the code size of NetworkManager by - 2809360 bytes + 2537528 bytes (-9.7%) - Runtime measurements are harder because there is a large variance during testing. In other words, the numbers are not reproducible. Currently, the implementation performs no caching of GVariants at all, but it would be rather simple to add it, if that turns out to be useful. Anyway, without strong claim, it seems that the new form tends to perform slightly better. That would be no surprise. $ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done) - real 1m39.355s + real 1m37.432s $ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done) - real 0m26.843s + real 0m25.281s - Regarding RSS size, just looking at the processes in similar conditions, doesn't give a large difference. On my system they consume about 19MB RSS. It seems that the new version has a slightly smaller RSS size. - 19356 RSS + 18660 RSS
* core: implement setting MDNS setting for systemdThomas Haller2018-01-091-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | The connection.mdns setting is a per-connection setting, so one might expect that one activated device can only have one MDNS setting at a time. However, with certain VPN plugins (those that don't have their own IP interface, like libreswan), the VPN configuration is merged into the configuration of the device. So, in this case, there might be multiple settings for one device that must be merged. We already have a mechanism for that. It's NMIP4Config. Let NMIP4Config track this piece of information. Although, stricitly speaking this is not tied to IPv4, the alternative would be to introduce a new object to track such data, which would be a tremendous effort and more complicated then this. Luckily, NMDnsManager and NMDnsPlugin are already equipped to handle multiple NMIPConfig instances per device (IPv4 vs. IPv6, and Device vs. VPN). Also make "connection.mdns" configurable via global defaults in NetworkManager.conf.
* all: get rid of a handful of unused-but-set variablesLubomir Rintel2017-12-181-2/+0
|
* core: refactor NMSettingsConnectionCallId typedef not to be a pointer to structThomas Haller2017-11-271-2/+2
| | | | | Typedefs to structs are fine, but a typedef for a pointer seems confusing to me. Let's avoid it.
* core/vpn: mark secret hints as constThomas Haller2017-11-231-14/+18
|
* vpn: avoid adding unneeded routes when ipvx.ignore-auto-routes=yesBeniamino Galvani2017-11-171-6/+10
| | | | | | Instead of adding routes and then let nm_ipx_config_merge_setting() remove them, don't add them in the first place when ipvx.ignore-auto-routes=yes.
* vpn: avoid coverity warning in print_vpn_config()Thomas Haller2017-11-151-2/+2
| | | | | coverity thinks that @address4 might be NULL. Maybe it can. We have an nm_assert(), but to be sure, check the value.
* vpn: consider the never-default connection propertyBeniamino Galvani2017-10-251-5/+11
| | | | | | | | | | | After commit 5c299454b49b ("core: rework tracking of gateway/default-route in ip-config") NM set a default route for VPNs only based on the "never-default" option reported by the plugin. It should also consider the connection setting. Fixes: 5c299454b49b165f645c25fd3e083c0bb747ad91 https://bugzilla.redhat.com/show_bug.cgi?id=1505886
* core,clients: use our own string hashing function nm_str_hash()Thomas Haller2017-10-181-1/+1
| | | | | | | | | | | | | | | | | | | | Replace the usage of g_str_hash() with our own nm_str_hash(). GLib's g_str_hash() uses djb2 hashing function, just like we do at the moment. The only difference is, that we use a diffrent seed value. Note, that we initialize the hash seed with random data (by calling getrandom() or reading /dev/urandom). That is a change compared to before. This change of the hashing function and accessing the random pool might be undesired for libnm/libnm-core. Hence, the change is not done there as it possibly changes behavior for public API. Maybe we should do that later though. At this point, there isn't much of a change. This patch becomes interesting, if we decide to use a different hashing algorithm.
* core: rework tracking of gateway/default-route in ip-configThomas Haller2017-10-101-32/+19
| | | | | | | | | | | | | | | | | | | | | | Instead of having 3 properties @gateway, @never_default and @has_gateway on NMIP4Config/NMIP6Config that determine the default-route, track the default-route as a regular route. The gateway setting is the configuration knob for the default-route. Since an NMIP4Config/NMIP6Config instance only has one gateway property, it cannot track more then one default-routes (see related bug rh#1445417). Especially with policy routing, it might be interesting to configure a default-route in multiple tables. Also, later it might be interesting to allow adding default-routes as regular static routes in a connection, so that the user can configure additional route parameters for the default-route or add default-routes in multiple tables. With this patch, default-routes now have a rt_source property according to their origin. Also, the previous commits of this branch broke handling of the default-route :) . That should be working now again.
* core: don't track route MSS in ip-configThomas Haller2017-10-091-6/+6
| | | | | | | | | | | | | | | | | The MSS is only set for VPN connections (by merging it in the respective NMIP4Config/NMIP6Config). It is also only used when setting the MSS of the default route. Don't track that property in NMIP4Config/NMIP6Config, instead, set the mss of the route directly in nm_vpn_connection_ip4_config_get() and nm_vpn_connection_ip6_config_get(). There is a potential change in behavior here: NMDevice also consisdered the MSS for the default route, but that would only be set if the MSS gets merged from an vpn-ip-config. Which at most is the case for iterface-less VPN types (libreswan). But even in that case, it doesn't seem right to me to use the VPN's MSS for the device's default-route.
* core: use ipv6.route-table setting for other IPv6 routesThomas Haller2017-10-091-4/+10
| | | | Including device-routes, default-route, SLAAC.
* core: use ipv4.route-table setting for other IPv4 routesThomas Haller2017-10-091-3/+9
| | | | Including device-routes, default-route, DHCPv4, IPv4LL.
* all: rework configuring route table support by adding "route-table" settingThomas Haller2017-10-091-11/+17
| | | | | | | | | | | | | | | | | | | | | | | | | We added "ipv4.route-table-sync" and "ipv6.route-table-sync" to not change behavior for users that configured policy routing outside of NetworkManager, for example, via a dispatcher script. Users had to explicitly opt-in for NetworkManager to fully manage all routing tables. These settings were awkward. Replace them with new settings "ipv4.route-table" and "ipv6.route-table". Note that this commit breaks API/ABI on the unstable development branch by removing recently added API. As before, a connection will have no route-table set by default. This has the meaning that policy-routing is not enabled and only the main table will be fully synced. Once the user sets a table, we recognize that and NetworkManager manages all routing tables. The new route-table setting has other important uses: analog to "ipv4.route-metric", it is the default that applies to all routes. Currently it only works for static routes, not DHCP, SLAAC, default-route, etc. That will be implemented later. For static routes, each route still can explicitly set a table, and overwrite the per-connection setting in "ipv4.route-table" and "ipv6.route-table".
* device/trivial: rename nm_device_get_ip_route_metric() to ↵Thomas Haller2017-10-061-2/+2
| | | | | | nm_device_get_route_metric() Brevity!
* device: remove wrappers for nm_device_get_ip_route_metric()Thomas Haller2017-10-061-2/+2
|
* libnm,core: add TABLE attribute for routes settingsThomas Haller2017-09-261-1/+31
| | | | https://bugzilla.redhat.com/show_bug.cgi?id=1436531
* core: fix handling IPv6 device-route and use correct route metricThomas Haller2017-09-191-0/+3
| | | | | | | | | | | | | | | | Before commit 6698bf58bb53fb07838c52ca67293dd5352ec31c, we would rely on kernel to add the device-route for manual IPv6 routes. We broke that and now kernel would still add the device-route, however nm_platform_ip_route_sync() would delete it immediately after. That is because previously nm_platform_ip_route_sync() would ignore routes with rtm_protocol RTPRO_KERNEL. Now, it will sync and delete those too. Fix that by adding the device-route like we do it for IPv4. This also fixes an actual issue where the automatically added route always had route-metric 256. Instead, we now use the metric from ipv6.route-metric setting. Fixes: 6698bf58bb53fb07838c52ca67293dd5352ec31c
* core: workaround configuring IPv6 routes with "src" (RTA_PREFSRC)Thomas Haller2017-09-151-1/+2
| | | | | | | | | | | | | | | | | | | | | Kernel does not allow to add IPv6 routes with "src", as long as the corresponding address is still tentative (related bug rh#1457196). The workaround for this is cumbersome. First, when we fail to add such a route with "pref_src", we guess that it happend due to this issue. In that case, nm_ip6_config_commit() returns the list of routes that could not be added for the moment (but hopefully can be added later). We track this list in NMDevice, and keep trying to merge the routes back into ip6_config. In order to not try indefinitely, keep track of a timestamp when we tried to add this route for the first time. Another uglyness is that pending tentative routes don't explicitly block activation. In practice they may do, because for these routes we also have an IPv6 address that is still doing DAD, so the IP configuration is still pending due to that. https://bugzilla.redhat.com/show_bug.cgi?id=1452684
* core: track IPv4 device routes in NMIP4ConfigThomas Haller2017-09-131-2/+14
| | | | | | | For IPv6, we create device routes when processing the RA and add it to NMIP6Config like any other route. For IPv4 we didn't do that. Instead we created the list of device routes during nm_ip4_config_commit() and passed it to nm_platform_ip_route_sync().
* core: rework handling of default-routes and drop NMDefaultRouteManagerth/platform-route-pt4Thomas Haller2017-09-081-7/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove NMDefaultRouteManager. Instead, add the default-route to the NMIP4Config/NMIP6Config instance. This basically reverts commit e8824f6a5205ffcf761abd3e0897a22b254c7797. We added NMDefaultRouteManager because we used the corresponding to `ip route replace` when configuring routes. That would replace default-routes on other interfaces so we needed a central manager to coordinate routes. Now, we use the corresponding of `ip route append` to configure routes, and each interface can configure routes indepdentently. In NMDevice, when creating the default-route, ignore @auto_method for external devices. We shall not touch these devices. Especially the code in NMPolicy regarding selection of the best-device seems wrong. It probably needs further adjustments in the future. Especially get_best_ip_config() should be replaced, because this distinction VPN vs. devices seems wrong to me. Thereby, remove the @ignore_never_default argument. It was added by commit bb750260045239ab85574366bae8102eff8058cc, I don't think it's needed anymore. This brings another change. Now that we track default-routes in NMIP4Config/NMIP6Config, they are also exposed on D-Bus like regular routes. I think that makes sense, but it is a change in behavior, as previously such routes were not exposed there.
* core: return new route from _nm_ip_config_add_obj()Thomas Haller2017-09-081-8/+8
| | | | | Later we will need the exact instance that we just added (or the previously existing one, if the new route is already tracked).
* vpn: only rely on `ip route get` to resolve route to external VPN gatewayThomas Haller2017-09-071-43/+23
| | | | | | | | | | Until recently, we would only consier the IP config of the parent device to determine the route to the external VPN gateway. We changed that, to additionally improve the guess by letting kernel resolve the route. Now, drop checking the parent's config entirely. The only thing that matters is the here and now runtime configuraion on the parent device. And for that we ask kernel to resolve the route.
* vpn: improve resolving route to external VPN gateway by restricting oifThomas Haller2017-09-071-8/+34
| | | | | | | | | | | | | Previously, we would try to resolve the route in general (unrestricted to a certain ifindex), and reject it the result wasn't on the parent device. Now, use the oif argument, and resolve the route only on the parent device. The problem is that kernel would pretend that the destination is onlink, if there is no route to it. Hence, hack around that by only accepting an onlink route, if the VPN gateway itself is site-local. Yes, there are scenarios where this will still lead to a wrong guess. See related bug rh#1489343 for kernel.
* platform: add oif argument to nm_platform_ip_route_get()Thomas Haller2017-09-071-0/+2
| | | | Analog to `ip route get $DST oif $IFACE`.
* vpn: apply parent config in nm_vpn_connection_apply_config() firstThomas Haller2017-09-071-2/+2
| | | | | | | | | In practice, it shouldn't matter much, because NM may frequently reapply the IP config. Hence, it anyway must cope with the fact that IP config from a previous iteration is already applied on the VPN device, before applying it to the parent device. Anyway, it makes a bit more sense to apply it first the the parent device.
* vpn: fix ifindex of parent IP config in apply_parent_device_config()Thomas Haller2017-09-071-25/+16
| | | | | | | | | | | | | | | When creating the NMIP4Config/NMIP6Config instance, we must always use the right ifindex. That is the ifindex, on which we want to apply the config. It also means, that for device-based VPNs (those with priv->ip_ifindex set, like OpenVPN), the parent's config must have the ip-ifindex of the parent device. Not of the VPN's device. One effect of this bug is that in add_ip4_vpn_gateway_route() we resolve the route to the external gateway and only accept it if it's on the parent device. But since the ifindex of the config was wrong, we would accept route on the wrong interface. https://bugzilla.gnome.org/show_bug.cgi?id=787370
* vpn: resolve route to external VPN gateway from kernelThomas Haller2017-08-241-43/+86
| | | | | | | | | | | | | | | | | | When activating a route, we commonly need to add a route to the external VPN gateway, so that the (encrypted) data is not sent over the VPN itself. Currently, our VPN connections are rather strongly tied to their parent device. Maybe the shouldn't be, as VPN may happily support changing the route from one device/IP address to another. Anyway, our previous way to determine the gateway for the route was not great. Instead, ask kernel how to reach the gateway via (something like) `ip route get`. If kernel would route the packets to the gateway via our parent device, we take that gateway. If not, we keep our previous heuristic (which is probably wrong in this case).
View Lorry Controller Status