When a connection should be assumed and the generated connection did not
contain a wired setting, the connection did not match due to S390 properties.
Such a connection should be allowed to match to a connection with a wired
setting with default (empty) S390 properties.
This can happen when there is a VLAN profile configured that contains a wired
setting in it and NetworkManager is (re)started.
Example/reproducer:
$ nmcli con add type vlan con-name vlan-test autoconnect no dev em1 id 44
$ nmcli con mod vlan-test eth.mtu 1450 (modify the connection, so that it has a wired setting)
$ nmcli con up vlan-test (activate the connection)
$ sudo systemctl restart NetworkManager
$ nmcli device
check that 'vlan-test' connection is active on em1.44 device
(and not the auto-generated em1.44)
https://bugzilla.redhat.com/show_bug.cgi?id=1276343
RFC7217 introduces an alternative mechanism for creating addresses during
stateless IPv6 address configuration. It's supposed to create addresses whose
host part stays stable in a particular network but changes when the hosts
enters another network to mitigate possibility of tracking the host movement.
It can be used alongside RFC 4941 privacy extensions (temporary addresses)
and replaces the use of RFC 4862 interface identifiers.
The address creation mode is controlld by ip6.addr_gen_mode property
(ADDR_GEN_MODE in ifcfg-rh), with values of "stable-privacy" and "eui-64",
defaulting to "eui-64" if unspecified.
The host part of an address is computed by hashing a system-specific secret
salted with various stable values that identify the connection with a secure
hash algorithm:
RID = F(Prefix, Net_Iface, Network_ID, DAD_Counter, secret_key)
For NetworkManager we use these parameters:
* F()
SHA256 hash function.
* Prefix
This is a network part of the /64 address
* Net_Iface
We use the interface name (e.g. "eth0"). This ensures the address won't
change with the change of interface hardware.
* Network_ID
We use the connection UUID here. This ensures the salt is different for
wireless networks with a different SSID as suggested by RFC7217.
* DAD_Counter
A per-address counter that increases with each DAD failure.
* secret_key
We store the secret key in /var/lib/NetworkManager/secret_key. If it's
shorter than 128 bits then it's rejected. If the file is not present we
initialize it by fetching 256 pseudo-random bits from /dev/urandom on
first use.
Duplicate address detection uses IDGEN_RETRIES = 3 and does not utilize the
IDGEN_DELAY delay (despite it SHOULD). This is for ease of implementation
and may change in future. Neither parameter is currently configurable.
Having a static string buffer for convenience is useful not only
for platform. Define the string buffer in NetworkManagerUtils.h,
so that all to-string functions can reuse *one* buffer.
Of course, this has the potential danger, that different
to-string method might reuse the same buffer. Hence, low-level
library functions are adviced to use their own buffer, because
an upper level might already use the global buffer for another
string.
Support a new configuration option
[.config]
enable=<ENABLED>
for configuration snippets.
This new [.config] section is only relevant within the snippet itself
and it is not merged into the combined configuration.
Currently only the "enable" key is supported. If the "enable" key is
missing, it obviously defaults to being enabled. It allows snippets
to be skipped from loading. The main configuration "NetworkManager.conf"
cannot be skipped.
<ENABLED> can be a boolean value (false), to skip a configuration
snippet from loading.
It can also be a string to match against the NetworkManager version,
like "enable=nm-version-min:1.1,nm-version-min:1.0.6"
There are several motivations for this:
- the user can disable an entire configuration snippet by toggeling
one entry.
This generalizes the functionality of the global-dns.enable
setting, but in a way that applies to configuration on a per-file
basis.
- for developing, we often switch between different versions of
NetworkManager. Thus, we might want to use different configuration.
E.g. before global-dns options, I want to use "dns=none" and manage
resolv.conf myself. Now, I can use global-dns setting to do that.
That can be achieved with something like the following (not exactly,
it's an example only):
[.config]
enable=nm-version-min:1.1
[main]
dns=default
[global-dns-domain-*]
nameserver=127.0.0.1
Arguably, this would be more awesome, if we would bump our micro devel
version (1.1.0) more often while developing 1.2.0 (*hint*).
- in principle, packages could drop configuration snippets and enable
them based on the NetworkManager version.
- with the "env:" spec, you can enable/disable snippets by configuring
an environment variable. Again, useful for testing and developing.
This is effectively the same as nm_config_parse_boolean(). The difference is,
that "nm-config.c" is not available to the interface-helper, thus any
code used by interface-helper (like "NetworkManager.c") cannot use this
function.
Still don't drop nm_config_parse_boolean() entirely, because it's better
to have the explicit notion of parsing a string in the config-context.
I ended up not using the function. But I'd still keep this patch.
It is not uncommon to see first a "disown" message before "disposing".
That alone does not yet indicate anything wrong or a leak. Remove that
overly alarming part of the logging message.
Port remaining bits to gdbus and remove stray dbus-glib references
Drop the dbus-glib version check from configure, since nothing depends
on new dbus-glib any more.
Move nm-dbus-glib-types.h and nm-gvaluearray-compat.h from include/ to
libnm-util/ since they are now only used by libnm-util and libnm-glib.
The localization headers are now included via "nm-default.h".
Also fixes several places, where we wrongly included <glib/gi18n-lib.h>
instead of <glib/gi18n.h>. For example under "clients/" directory.
Previously, the order of destructing singleton instances
was undefined. Now, have singleton instances register their
destruction via nm_singleton_instance_register().
Objects that are registered later, will be destructed earlier. IOW,
they will be destroyed in reverse order of construction.
This is only a crude method to get the lifetime of singleton instances
right by default. Having singletons ref other singletons to keep them
alive gives more control over the lifetimes of singletons. This change
of having a defined order of destruction does not conflict with taking
references to singletons (and thus extending their lifetime).
Note that previously, NMPlatform was not registered for destruction.
We don't change that yet and intenionally leak a reference.
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.
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).
We have a special implemenation nm_match_spec_split() to split
a string. We also need the reverse operation to be able to convert
a list of specs to string without loss.
kill(pid,sig) can return success for zombie processes. This
caused nm_utils_kill_process_sync() to hang indefinitely.
Fix it by also checking the process state.
Change nm_platform_link_get() to return the cached NMPlatformLink
instance. Now what all our implementations (fake and linux) have such a
cache internal object, let's just expose it directly.
Note that the lifetime of the exposed link object is possibly quite
short. A caller must copy the returned value if he intends to preserve
it for later.
Also add nm_platform_link_get_by_ifname() and modify nm_platform_link_get_by_address()
to return the instance.
Certain functions, such as nm_platform_link_get_name(),
nm_platform_link_get_ifindex(), etc. are solely implemented based
on looking at the returned NMPlatformLink object. No longer implement
them as virtual functions but instead implement them in the base class
(nm-platform.c).
This removes code and eliminates the redundancy of the exposed
NMPlatformLink instance and the nm_platform_link_get_*() accessors.
Thereby also fix a bug in NMFakePlatform that tracked the link address
in a separate "address" field, instead of using "link.addr". That was
a case where the redundancy actually led to a bug in fake platform.
Also remove some stub implementations in NMFakePlatform that just
bail out. Instead allow for a missing virtual functions and perform
the "default" action in the accessor.
An example for that is nm_platform_link_get_permanent_address().
nm_utils_get_shared_wifi_permission() that is called for each AddAndActivate
uses this and triggers an assertion failure for InifnibandConnections which
don't have IPv4 configuration:
NetworkManager[54006]: nm_utils_get_ip_config_method: assertion 's_ip4 != NULL' failed
#2 0x000055555562b833 in nm_utils_get_ip_config_method (connection=connection@entry=0x5555559c3b60, ip_setting_type=93824996202304) at NetworkManagerUtils.c:1329
#3 0x000055555562b914 in nm_utils_get_shared_wifi_permission (connection=0x5555559c3b60) at NetworkManagerUtils.c:1105
#4 0x00005555555fc012 in nm_active_connection_authorize (self=0x5555559b30a0, result_func=0x55555561b980 <_add_and_activate_auth_done>, user_data1=0x555555957210, user_data2=0x555555a055f0)
at nm-active-connection.c:683
#5 0x0000555555621b21 in impl_manager_add_and_activate_connection (self=0x555555957210, settings=<optimized out>, device_path=<optimized out>, specific_object_path=0x0, context=0x555555a055f0)
at nm-manager.c:3495
We don't want error logging for nm_platform_link_add() which
tries to load the bonding module. Later we will run tests as non-root,
where modprobe will fail. Logging an error would break the tests.
Code that is testable often needs special hooks to work
both for unit-tests and production.
Add a function nm_utils_get_testing() that returns whether
the code is run as part of a unit-test.
For non-testing mode, nm_utils_get_testing() will return
zero (NM_UTILS_TEST_NONE). For unit tests, the test should call
_nm_utils_set_testing() to configure tested functions.
By specifing the @flags attribute, the test can enable/disable
specific behaviors.
https://bugzilla.gnome.org/show_bug.cgi?id=701112
prevents:
NetworkManager[2775]: <error> [1429889302.113450] [NetworkManagerUtils.c:576]
nm_utils_kill_child_async(): kill child process 'teamd' (3025): failed due
to unexpected return value -1 by waitpid (No child processes, 10) after
sending SIGTERM (15)
We do the same for the original MAC address.
A device enslaved to a bond it inherits the bond's MAC address. When
NetworkManager tries to assume a connection the generated cloned-mac property
causes a mismatch with the connection that originally brought up the device,
causing the generated connection to be used instead:
NetworkManager[14190]: <debug> [1424355817.112154] [NetworkManagerUtils.c:1641]
nm_utils_match_connection(): Connection 'eth2' differs from candidate
'bond-slave-eth2' in 802-3-ethernet.cloned-mac-address
https://bugzilla.gnome.org/show_bug.cgi?id=744812
nm_ethernet_address_is_valid() did not check whether @addr was a valid
address in the first place. It only checked whether the address was not
equal to a few notorious MAC addresses.
At the same time, be more forgiving and accept %NULL as argument.
This fixes an assertion nm_ap_match_in_hash().
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".
No need to parse the subchannels if the spec list is empty.
This isn't actually an issue, because nm_match_spec_s390_subchannels()
was never called with empty spec list.
Some device types (s390 OSA and ipvlan) can use the same link-layer address
for multiple virtual interfaces, and the kernel used the dev_id property
to differentiate these devices when constructing the IID. NM should do
this too to prevent IID clashes.
https://bugzilla.redhat.com/show_bug.cgi?id=1101809
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
