Previously most objects were implicitly unexported when they were
destroyed, but since refcounts may make the object live longer than
intended, we should explicitly unexport them when they should no
longer be present on the bus.
This means we can assume that objects will always be un-exported
already when they are destroyed, *except* when quitting where most
objects will live until exit because NM leaves interfaces up and
running on quit.
It is not used externally and its use might be confusing and undesired when we
add plugin aliases. The external users should only use the name when idenfiying
the plugin and nm_vpn_plugin_info_list_find_by_service() when matchin the plugin.
The peer-address (IFA_ADDRESS) can also be all-zero (0.0.0.0).
That is distinct from an usual address without explicit peer-address,
which implicitly has the same peer and local address.
Previously, we treated an all-zero peer_address as having peer and
local address equal. This is especially grave, because the peer is part
of the primary key for an IPv4 address. So we not only get a property of
the address wrong, but we wrongly consider two different addresses as
one and the same.
To properly handle these addresses, we always must explicitly set the peer.
The configuration is going to be applied on the parent interface.
If we didn't do this, an attempt to add a route would result in an assertion
failure:
nm-ip4-config.c:1475:nm_ip4_config_add_route: assertion failed: (priv->ifindex)
A separate instance of the support plugin is spawned for each connection with
a different bus name. The bus name is passed via --bus-name <name> argument.
Plugins that support the feature indicate it with
support-multiple-connections=true key in the [VPN Connection] section.
The bus name is currently generated by adding a .<connection.uuid> to the VPN
service name. It's guarranteed unique, but if it proves to be too long or ugly
it can easily be replaced with something more meaningful (such as the same number
as is used for connection's DBus name).
NMVpnService has been removed and folded into NMVpnConnection. A
NMVpnConnection will spawn a service plugin instance whenever it is activated
and notices the bus name it needs is not provided.
The NMVpnManager no longer needs to keep track of the connections in use apart
for compatibility purposes with plugins that don't support the feature.
The 9b79e6c73 commit moved setting of the MTU from IP4Config to NMDevice, but
VPN connections don't have a NMDevice instance (yet). Set the MTU also from the
VPN connection. Also, copying of the MTU to the IP4Config is no longer needed
as the ip4_config_commit no longer sets the MTU.
Fixes: 9b79e6c732https://bugzilla.gnome.org/show_bug.cgi?id=754781
Always take a reference to the manager when scheaduling an asynchronous
operations. In fact, all callers want to keep the manager alive as long
as there are operations in progress.
Refactor NMFirewallManager to have consistent semantics about
asynchronous calls.
- the callback is always invoked exactly once, but never
synchronously when starting the operation.
- while cancelling the request, the callback is invoked
synchronously with respect to the cancel operation.
- you can cancel a request once (and once only).
- you cannot cancel the request once the callback is invoked.
- when disposing the object with requests pending, the
callbacks are invoked synchronously.
- Add a callback argument to nm_firewall_manager_remove_from_zone().
Otherwise, the user never knows whether a call is still pending and
cancellable (as complete operations cannot be cancelled).
In fact, it makes no sense trying to cancel a call if you don't care
about when it completes.
- get rid of PENDING_CALL_DUMMY. The dummy callback allows cancellation
any number of times. We want to catch wrong usage by asserting that an
operation is only cancelled once.
- nm_firewall_manager_cancel_call() doesn't need the manager argument.
Either the call-id is valid (and has a valid pointer to the manager),
or there is a bug and it is a dangling pointer.
Clone the connection upon activation. This makes it safe for the user
to modify the original connection while it is activated.
This involves several changes:
- NMActiveConnection gets @settings_connection and @applied_connection.
To support add-and-activate, we constructing a NMActiveConnection with
no connection set. Previously, we would set the "connection" field to
a temporary NMConnection. Now NMManager piggybacks this temporary
connection as object-data (TAG_ACTIVE_CONNETION_ADD_AND_ACTIVATE).
- get rid of the functions nm_active_connection_get_connection_type()
and nm_active_connection_get_connection_uuid(). From their names
it is unclear whether this returns the settings or applied connection.
The (few) callers should figure that out themselves.
- rename nm_active_connection_get_id() to
nm_active_connection_get_settings_connection_id(). This function
is only used internally for logging.
- dispatcher calls now get two connections as well. The
applied-connection is used for the connection data, while
the settings-connection is used for the connection path.
- needs special handling for properties that apply immediately
when changed (nm_device_reapply_settings_immediately()).
Co-Authored-By: Thomas Haller <thaller@redhat.com>
https://bugzilla.gnome.org/show_bug.cgi?id=724041
This now gives every logging line of a NMVpnConnection
a fully descriptive prefix.
Especially for non-debug logging, this looks a bit verbose
and repetitive, so we could suppress the prefix in that case.
I still add it because I think the verbose information does help
during debugging.
Refactor agent-manager to always invoke the complete function for
nm_agent_manager_get_secrets().
In general, the complete function is always invoked asnychronously
when starting the operation. On the other hand, when cancelling the
operation or disposing the manager with pending operations, we now
(always) synchronously invoke the callback.
This makes it simpler for the user to reliably cancel the request
and perform potential cleanup.
This behavior bubbles up through NMSettingsConnection and NMActRequest,
and other callers that make directly or indicrectly make use of
nm_agent_manager_get_secrets().
Instead of having the call_id of type guint32, make it an (opaque)
pointer type.
This has the advantage of strong typing and avoids the possiblity
of reusing an invalid integer (or overflow of the call-id counter).
OTOH, it has the disadvantage, that after a call_id is disposed,
it might be reused for future invocations (because malloc might
reuse the memory).
In fact, it is always an error to use a call_id that is already
completed. This commit also adds assertions to the cancel() calls
that the provided call_id is a pending call. Hence, such a bug
will be uncovered by assertions (that only might not tigger in
certain unlikely cases where a call-id got reused).
Note that for NMAgentManager, save_secrets() and delete_secrets()
both returned a call_id. But they didn't also provide a callback when
the operation completes. So the user trying to cancel such a call,
cannot know whether the operation is still in process and he cannot
avoid triggering an assertion.
Fix that by not returning a call-id for these operations. No caller
cared about it anyway.
For NMSettingsConnection, also track the internally scheduled requests
for so that we can cancel them on dispose.
Addresses the clash between the two commits which would cause the parent device
gateway to be overwritten with 0.0.0.0 upon route-based VPN activation:
Fixes: 063677101a
Fixes: 1465c1d326
The new flags are not yet used, so there is no change in functionality.
The flags NM_IP_CONFIG_MERGE_NO_ROUTES and NM_IP_CONFIG_MERGE_NO_DNS go
together with the 'ignore-auto-routes' and 'ignore-auto-dns' setting.
Note that for IPv4, NM_IP_CONFIG_MERGE_NO_DNS also ignores NIS, WINS, and dns-options.
This is different from current other places that handle 'ignore-auto-dns'
and only care about nameservers, domains, and searches.
Use NMVpnPluginInfo to load the plugins in NMVpnManager.
This has the advantage of reusing the code from libnm
to use the same approach to read the plugin config files.
Another advantage is that we now check the file permissions
of the config file.
Create a GDBusProxy for the service to be monitored and use that to
tell whether it is running, rather than using NMDBusManager and the
global NameOwnerChanged signal.
Move D-Bus export/unexport handling into NMExportedObject and remove
type-specific export/get_path methods (export paths are now specified
at the class level, and NMExportedObject handles the counters for all
exported types automatically).
Since all exportable objects now use the same get_path() method, we
can also add some helper methods to simplify get_property()
implementations for object-path and object-path-array properties.
Add NMExportedObject, make it the base class of all D-Bus-exported
types, and move the nm-properties-changed-signal logic into it. (Also,
make NMSettings use the same properties-changed code as everything
else, which it was not previously doing, presumably for historical
reasons).
(This is mostly just shuffling code around at this point, but
NMExportedObject will be more important in the gdbus port, since
gdbus-codegen doesn't do a very good job of supporting objects that
export multiple interfaces [as each NMDevice subclass does, for
example], so we will need more glue/helper code in NMExportedObject
then.)
Rather than randomly including one or more of <glib.h>,
<glib-object.h>, and <gio/gio.h> everywhere (and forgetting to include
"nm-glib-compat.h" most of the time), rename nm-glib-compat.h to
nm-glib.h, include <gio/gio.h> from there, and then change all .c
files in NM to include "nm-glib.h" rather than including the glib
headers directly.
(Public headers files still have to include the real glib headers,
since nm-glib.h isn't installed...)
Also, remove glib includes from header files that are already
including a base object header file (which must itself already include
the glib headers).
#0 0x00007ffff4200a98 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55
#1 0x00007ffff420272a in __GI_abort () at abort.c:89
#2 0x00007ffff4a372a5 in g_assertion_message (domain=domain@entry=0x5555557a0511 "NetworkManager", file=file@entry=0x5555557b201c "nm-ip4-config.c", line=line@entry=1458, func=func@entry=0x5555557b221b "nm_ip4_config_add_route", message=message@entry=0x555555b96a00 "assertion failed: (priv->ifindex)") at gtestutils.c:2356
#3 0x00007ffff4a3733a in g_assertion_message_expr (domain=0x5555557a0511 "NetworkManager", file=0x5555557b201c "nm-ip4-config.c", line=1458, func=0x5555557b221b "nm_ip4_config_add_route", expr=<optimized out>) at gtestutils.c:2371
#4 0x000055555567f414 in nm_ip4_config_add_route (config=0x555555c27f80 [NMIP4Config], new=0x7fffffffd378) at nm-ip4-config.c:1458
#5 0x000055555576b6d6 in add_ip4_vpn_gateway_route (config=0x555555c27f80 [NMIP4Config], parent_device=0x555555afeb80 [NMDeviceEthernet], vpn_gw=4240082129) at vpn-manager/nm-vpn-connection.c:522
#6 0x000055555576b3c3 in apply_parent_device_config (connection=0x7fffdc01a300 [NMVpnConnection]) at vpn-manager/nm-vpn-connection.c:910
#7 0x000055555576b197 in nm_vpn_connection_apply_config (connection=0x7fffdc01a300 [NMVpnConnection]) at vpn-manager/nm-vpn-connection.c:945
#8 0x0000555555769ada in nm_vpn_connection_config_maybe_complete (connection=0x7fffdc01a300 [NMVpnConnection], success=1) at vpn-manager/nm-vpn-connection.c:981
#9 0x000055555576c35f in nm_vpn_connection_ip4_config_get (self=0x7fffdc01a300 [NMVpnConnection], dict=0x555555c10150) at vpn-manager/nm-vpn-connection.c:1285
#10 0x0000555555766e2c in ip4_config_cb (proxy=0x555555acedd0 [GDBusProxy], dict=0x555555c10150, user_data=0x7fffdc01a300) at vpn-manager/nm-vpn-connection.c:1643
#11 0x00007ffff27f2db0 in ffi_call_unix64 () at ../src/x86/unix64.S:76
#12 0x00007ffff27f2818 in ffi_call (cif=cif@entry=0x7fffffffd870, fn=<optimized out>, rvalue=0x7fffffffd7d0, avalue=avalue@entry=0x7fffffffd770) at ../src/x86/ffi64.c:525
#13 0x00007ffff4d114f9 in g_cclosure_marshal_generic (closure=0x555555b67f20, return_gvalue=0x0, n_param_values=<optimized out>, param_values=0x555555a77220, invocation_hint=<optimized out>, marshal_data=0x0) at gclosure.c:1448
#14 0x00005555556c824d in dbus_signal_meta_marshal (closure=0x555555b67f20, return_value=0x0, n_param_values=4, param_values=0x7fffffffdb50, invocation_hint=0x7fffffffdad0, marshal_data=0x555555b8aa60)
at ../libnm-core/nm-dbus-utils.c:95
#18 0x00007ffff4d2b29f in <emit signal ??? on instance 0x555555acedd0 [GDBusProxy]> (instance=instance@entry=0x555555acedd0, signal_id=<optimized out>, detail=detail@entry=0) at gsignal.c:3361
#15 0x00007ffff4d10cd5 in g_closure_invoke (closure=0x555555b67f20, return_value=return_value@entry=0x0, n_param_values=4, param_values=param_values@entry=0x7fffffffdb50, invocation_hint=invocation_hint@entry=0x7fffffffdad0)
at gclosure.c:768
#16 0x00007ffff4d22539 in signal_emit_unlocked_R (node=node@entry=0x555555a46290, detail=detail@entry=0, instance=instance@entry=0x555555acedd0, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffdb50) at gsignal.c:3549
#17 0x00007ffff4d2aef0 in g_signal_emit_valist (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>, var_args=var_args@entry=0x7fffffffdd50) at gsignal.c:3305
#19 0x00007ffff502ebac in on_signal_received (connection=<optimized out>, sender_name=0x7fffe00063e0 ":1.541", object_path=<optimized out>, interface_name=<optimized out>, signal_name=0x7fffe0016f80 "Ip4Config", parameters=0x555555c22330, user_data=0x7fffdc00e850) at gdbusproxy.c:917
#20 0x00007ffff501e8b4 in emit_signal_instance_in_idle_cb (data=0x7fffe0016a60) at gdbusconnection.c:3753
#21 0x00007ffff4a10a8a in g_main_context_dispatch (context=0x555555a23360) at gmain.c:3122
#22 0x00007ffff4a10a8a in g_main_context_dispatch (context=context@entry=0x555555a23360) at gmain.c:3737
#23 0x00007ffff4a10e20 in g_main_context_iterate (context=0x555555a23360, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3808
#24 0x00007ffff4a11142 in g_main_loop_run (loop=0x555555a23420) at gmain.c:4002
#25 0x00005555555b7e7b in main (argc=1, argv=0x7fffffffe3b8) at main.c:484
https://bugzilla.gnome.org/show_bug.cgi?id=752225
It is wrong to only consider internal_gateway of the VPN connection.
Instead, we must first set the gateway of NMIP4Config and then overwrite
it with the connection settings.
For non-tunnel based VPNs (openswan, libreswan), we must
clear the gateway setting. The default route is managed
by NMDefaultRouteManager, and we must not overwrite the
gateway of the parent device.
This fixes a bug if the VPN connection specifies a gateway, it
would have overwritten the gateway of the underlying device.
The gateway property of NMIP4Config/IP6Config determines the next hop
for the default route. That is different from the @external_gw property
of the VPN which is the address of the world-reachable VPN gateway.
It is wrong to set the gateway of the VPN's IP config to the external gateway.
This causes ip4_config_merge_and_apply() to overwrite the gateway of the
underlying device.
Instead, NMDefaultRouteManger gets the gateway directly from the VPN
connection by quering nm_vpn_connection_get_ip4_internal_gateway().
The VPN connection requests secrets a few times; first it retrieves
only system-owned secrets to see if they are sufficient (and thus
doesn't need to bother the user), then it retrieves existing agent
owned secrets (so the user doesn't get a popup), then finally if
those aren't sufficient it asks the user interactively.
But if there was some error retrieving system secrets, or if there
weren't any system secrets at all, don't fail the VPN connection.
Just go on and ask the user for the secrets.
Most nm_platform_*() functions operate on the platform
singleton nm_platform_get(). That made sense because the
NMPlatform instance was mainly to hook fake platform for
testing.
While the implicit argument saved some typing, I think explicit is
better. Especially, because NMPlatform could become a more usable
object then just a hook for testing.
With this change, NMPlatform instances can be used individually, not
only as a singleton instance.
Before this change, the constructor of NMLinuxPlatform could not
call any nm_platform_*() functions because the singleton was not
yet initialized. We could only instantiate an incomplete instance,
register it via nm_platform_setup(), and then complete initialization
via singleton->setup().
With this change, we can create and fully initialize NMPlatform instances
before/without setting them up them as singleton.
Also, currently there is no clear distinction between functions
that operate on the NMPlatform instance, and functions that can
be used stand-alone (e.g. nm_platform_ip4_address_to_string()).
The latter can not be mocked for testing. With this change, the
distinction becomes obvious. That is also useful because it becomes
clearer which functions make use of the platform cache and which not.
Inside nm-linux-platform.c, continue the pattern that the
self instance is named @platform. That makes sense because
its type is NMPlatform, and not NMLinuxPlatform what we
would expect from a paramter named @self.
This is a major diff that causes some pain when rebasing. Try
to rebase to the parent commit of this commit as a first step.
Then rebase on top of this commit using merge-strategy "ours".
Of special note is the new D-Bus rule to allow root to talk to
org.freedesktop.NetworkManager.VPN.Plugin, without which NetworkManager
would not hear signals from the VPN plugins. Oddly, this worked
fine with dbus-glib...
https://bugzilla.gnome.org/show_bug.cgi?id=745307
