f3c2851c (port NMAgentManager, etc, to use NMAuthSubject) made it
unnecessary for callers to nm_settings_connection_get_secrets() to
decide whether to filter agents by UID or not, but NMVPNConnection was
still doing it itself anyway.
Tag addresses and routes with their source. We'll use this later to do
(or not do) operations based on where the item came from.
One thing to note is that when synchronizing items with the kernel, all
items are read as source=KERNEL even when they originally came from
NetworkManager, since the kernel has no way of providing this source
information. This requires the source 'priority', which
nm_ip*_config_add_address() and nm_ip*_config_add_route() must respect
to ensure that NM-owned routes don't have their source overwritten
when merging various IP configs in ip*_config_merge_and_apply().
Also of note is that memcmp() can no longer be used to compare
addresses/routes in nm-platform.c, but this had problems before
anyway with ifindex, so that workaround from nm_platform_ip4_route_sync()
can be removed.
https://bugzilla.gnome.org/show_bug.cgi?id=722843https://bugzilla.redhat.com/show_bug.cgi?id=1005416
NMActiveConnection was categorizing all deactivation of master
connections as "failure", and NMActRequest was deactivating all of the
master's slaves with REASON_DEPENDENCY_FAILED no matter what the real
reason was.
In fact, NMActiveConnection only needs to handle the cases where the
master fails before enslaving the device; any failure after that point
will be caught by existing master/slave checks in NMDevice. So update
the code accordingly (and remove the master_failed code from
NMVpnConnection entirely, since no master supports having VPN slaves).
Add IP and DHCP config properties to the D-Bus ActiveConnection
objects.
For device connections, this is redundant with the properties already
on the Device object, but for VPN connections, this information was
not previously available.
Rather than explicitly passing around a UID and a flag saying whether
or not it's relevant.
(This also fixes a bug where the wrong UID was being recorded in
nm-settings-connection.c::auth_start(), which caused problems such as
agent-owned secrets not getting saved because of a perceived UID
mismatch.)
In the migration to NMPlatform, support for ptp/peer addresses was
accidentally dropped. This broke OpenVPN configurations using 'p2p'
topology, which send a different peer address than the local address
for tunX, plus the server may also push routes that use the peer
address as the next hop. NetworkManager was unable to add these
routes, because the kernel had no idea how to talk to the peer,
because the peer's address was not assigned to any interface or
reachable over any routes.
Partly based on a patch from Dan Williams.
These are (most likely) only warnings and not severe bugs.
Some of these changes are mostly made to get a clean run of
Coverity without any warnings.
Error found by running Coverity scan
https://bugzilla.redhat.com/show_bug.cgi?id=1025894
Co-Authored-By: Jiří Klimeš <jklimes@redhat.com>
Signed-off-by: Thomas Haller <thaller@redhat.com>
Both NMActRequest and NMVPNConnection need to track their device's state,
so instead of both subclasses having to do so, consolidate that code into
the superclass.
Eventually the manager will create both NMActRequest and
NMVPNConnection subclasses directly, instead of leaving
NMVPNConnection creation to the VPN manager.
This also ensures that VPN connections get their user_requested
attribute set correctly, which wasn't happening before in the
case of secondary VPN connections.
When we eventually do authorization in the ActiveConnection itself,
we want to make sure the AC doesn't get exported until everything
is authorized. Thus let the manager handle exporting the objects
since it knows when the AC will be authorized or not.
The errors were documented as org.freedesktop.NetworkManager.VPN.Error.*,
but the actual values were org.freedesktop.NetworkManager.VPN.Plugin.*
Also update the errors documentation.
The ConnectInteractive() -> Connect() fallback code doesn't work, because
_connect_internal() changes the state to NM_VPN_SERVICE_STATE_STARTING before
checking if it can implement ConnectInteractive(), and then when the Connect()
call comes in, the VPN is not in STOPPED or INIT, so it returns an error.
The commit moves setting state to STARTING after the ConnectInteractive() check
availability, in the plugin. We introduce new plugin error and set it when the
the plugin does not implement ConnectInteractive(). NetworkManager uses this
error for ConnectInteractive() -> Connect() fallback.
https://bugzilla.gnome.org/show_bug.cgi?id=708255
When a VPN wanted to add some routes (like the host route for the
VPN gateway) it would add them itself and listen for parent device
events and re-add them if necessary. That's pretty fragile, plus
the platform blows away routes that aren't part of the IP config
that's getting applied.
So we might as well just have the VPN connection tell the parent
what the routes are, and have the parent device handle updating
the routing. The routes are through the parent device anyway,
and so are "owned" by the parent too.
If all agents can handle VPN hints, then we'll try to use
ConnectInteractive() to let the VPN plugin ask for secrets
interactively via the SecretsRequired signal. These hints
are then passed to agents during the connection process if
the plugin needs more secrets or different secrets, and when
the new secrets are returned, they are passed back to the VPN
plugin.
If at least one agent does not have the VPN hints capability,
we can't use ConnectInteractive(), but fall back to the old
Connect call, because that agent won't be able to send the
hints to the VPN plugin's authentication dialog, and thus
we won't get back the secrets the VPN plugin is looking for.
So, for interactive secrets to work correctly, you need:
1) A VPN plugin updated for interactive secrets requests
2) NM updated for interactive secrets requests
3) all agents to set the VPN_HINTS capability when
registering with NetworkManager and to pass hints
along to the VPN authentication dialog
4) a VPN authentication dialog updated to look for hints
and only return secrets corresponding to the hints
requested by the plugin
Note that this patch doesn't effectively change any code.
Functions moved from nm-system:
* nm_system_apply_ip?_config → nm_ip?_config_commit
* ip?_dest_in_same_subnet → nm_ip?_config_destination_is_direct
Functions moved from NetworkManagerUtils:
* nm_utils_merge_ip?_config → nm_ip?_config_merge_setting
Functions renamed (and moved down to form one group):
* nm_ip?_config_new_for_interface → nm_ip?_config_capture
(The rationale for the rename is that from the achitectural point of
view it doesn't matter whether the function creates a new object or
updates an existing one. After the rename, it's obvious that
nm_ip?_config_capture() and nm_ip?_config_commit() are counterparts of
each other.)
nm_platform_*_sync() functions check the cached kernel configuration
items (addresses, routes) before adding addresses to the kernel.
Therefore we don't need to be so careful about pushing NetworkManager
configuration to the kernel.
This patch helps to avoid having to compare nm_ip[46]_config objects,
which should only be created when a configuration change is being
performed.
We need to register NMVpnConnection's properties with the
PropertiesChanged signal (which it inherits from NMActiveConnection)
or it will ignore them.
https://bugzilla.gnome.org/show_bug.cgi?id=701713
Although having different parts of NM in different subdirectories
keeps the source tree neat, it has made the build messy, particularly
because of cross-dependencies between the subdirs.
Reorganize to build all of the pieces of the NetworkManager binary
from src/Makefile, and only use recursive make for test programs,
helper binaries, and plugins.
As part of this, get rid of all the per-directory convenience
libraries, and switch to building a single top-level
libNetworkManager.la, containing everything except main.c, which all
of the test programs can then link against.
GObject creation cannot normally fail, except for types that implement
GInitable and take a GError in their _new() method. Some NM types
override constructor() and return NULL in some cases, but these
generally only happen in the case of programmer error (eg, failing to
set a mandatory property), and so crashing is reasonable (and most
likely inevitable anyway).
So, remove all NULL checks after calls to g_object_new() and its
myriad wrappers.
https://bugzilla.gnome.org/show_bug.cgi?id=693678
No need to copy the list when (a) we never care if it gets modified
in-place (since the loops break when the connection is found) and
(b) we never modify it in place anyway. Reduces the possibility of
leaking the list due to programming errors too.
