Commit graph

2599 commits

Author SHA1 Message Date
Aleksander Morgado
6ed21e8342 settings,gsm: deprecate and stop using 'number' property
The 'number' property in GSM settings is a legacy thing that comes
from when ModemManager used user-provided numbers, if any, to connect
3GPP modems.

Since ModemManager 1.0, this property is completely unused for 3GPP
modems, and so it doesn't make sense to use it in the NetworkManager
settings. Ofono does not use it either.

For AT+PPP-based 3GPP modems, the 'number' to call to establish the
data connection is decided by ModemManager itself, e.g. for standard
GSM/UMTS/LTE modems it will connect a given predefined PDP context,
and for other modems like Iridium it will have the number to call
hardcoded in the plugin itself.

https://github.com/NetworkManager/NetworkManager/pull/261
2018-12-19 08:54:50 +01:00
Thomas Haller
f877ba8c04 core: avoid calling platform code with invalid ifindex (5)
https://bugzilla.redhat.com/show_bug.cgi?id=1659790

Fixes: 945c904f95
2018-12-16 20:34:45 +01:00
Aleksander Morgado
90e9695af5 wwan: rework when settings/device are blocked for autoconnection
The reasons to block autoconnection at settings level are not the same
as the ones to block autoconnection at device level.

E.g. if the SIM-PIN is wrong, you may want to block autoconnection
both at settings level (as the PIN configured in settings is wrong)
and at device level (so that no other setting is tried automatically).

For some other reasons, you may want to block autoconnection only at
setting level (e.g. wrong APN).

And for some other reasons you may want to block autoconnection at
device level only (e.g. SIM missing), so that the autoconnection
blocking is removed when the device goes away. This is especially
important with SIM hotplug events processed by ModemManager, as a
device without SIM will be removed from MM when a new SIM is
inserted, so that a completely new object is exposed in MM with the
newly detected SIM.

https://github.com/NetworkManager/NetworkManager/pull/259
2018-12-14 14:25:36 +01:00
Thomas Haller
672852c4d3 dhcp: support generating DHCP client-id/duid for infiniband
https://bugzilla.redhat.com/show_bug.cgi?id=1658057
2018-12-14 13:49:54 +01:00
Aleksander Morgado
87bed48974 devices,bluetooth: fix default CDMA number setting
https://github.com/NetworkManager/NetworkManager/pull/260

Fixes: 215306f5a1
2018-12-13 16:58:05 +01:00
Thomas Haller
b1082aa9a7 device: disable rp_filter handling
Don't let NetworkManager change rp_filter sysctl [1]. By default, various
distributions set rp_filter rather strict. That works badly, in common cases
where the user connects multiple interfaces to the same IP network (for
example, using Wi-Fi and ethernet in your home network). It also confuses
connectivity checking. For that reason, NetworkManager would loosen the
rp_filter setting.

However, that was not configurable and users who really wanted a strict
setting could not prevent NetworkManager from doing it ([2], [3], [4]).

Hence it was decided, that a better solution is for NetworkManager not
to do anything about rp_filter. Instead, distibutions should not enable
it strictly (or at least, only for certain setups where it makes sense
-- if it ever makes sense) ([5], [6]);

Disble this behavior for the moment. In the future, the code will be
removed entirely.

[1] https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1492472
[4] https://bugzilla.redhat.com/show_bug.cgi?id=1593194
[3] https://bugzilla.redhat.com/show_bug.cgi?id=1651097
[5] https://bugzilla.redhat.com/show_bug.cgi?id=1653824
[6] https://github.com/systemd/systemd/pull/10971

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1651097
2018-12-13 14:23:07 +01:00
Thomas Haller
b16e09a707 core: use streq() instead of strcmp() for comparing ip-config methods
Refactor some code to use nm_streq() and NM_IN_STRSET() instead of
strcmp().

Note that nm_utils_get_ip_config_method() never returns %NULL (not even
with g_return*() assertion failures). nm_streq() is sufficent.
2018-12-13 09:16:32 +01:00
Thomas Haller
589063db3b core: use addr-family argument for nm_utils_get_ip_config_method()
Recently, more and more code was refactored to use an addr_family
integer to distinguish between IPv4 and IPv6.

Refactor nm_utils_get_ip_config_method() and nm_device_get_effective_ip_config_method()
to do that too. If we use different identifiers, we need to translate from one to
another and its inconsistent. Also, accessing a GType is an unnecessary function call,
instead of a plain constant.
2018-12-13 09:16:32 +01:00
Benjamin Berg
adbb9eb246 core: allow devices to modify the meaning of the AUTO IP config method
For P2P wifi we need to do DHCP if we are a peer or provide DHCP if we
are the group owner. This may only be decided while establishing the
connection, making the meaning of the AUTO method dynamic.

This adds a way for the device subclass to override the meaning of AUTO.

Patch cherry picked early from [1].

[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/24
2018-12-13 09:14:27 +01:00
Beniamino Galvani
e01a7c1154 core: use NMTernary for SR-IOV autoprobe-drivers 2018-12-12 14:38:18 +01:00
Beniamino Galvani
529533a50c device: reset SR-IOV VFs on deactivation
If the connection has a sriov setting we configure SR-IOV VFs on
activation. We should also clear resources when the connection
deactivates.
2018-12-12 14:18:53 +01:00
Beniamino Galvani
75024e11b3 device: configure static number of VFs in unavailable state
Don't configure the static number of VFs when the device is realized
because the device could still be unmanaged. Instead, do it when the
device becomes managed.
2018-12-12 14:18:53 +01:00
Thomas Haller
a68d027ba4 core: never fail reading host-id timestamp and never change it
The timestamp of the host-id is the timestamp of the secret_key file.
Under normal circumstances, reading the timestamp should never fail,
and reading it multiple times should always yield the same result.

If we unexpectedly fail to read the timestamp from the file we want:

- log a warning, so that the user can find out what's wrong. But
  do so only once.

- we don't want to handle errors or fail operation due to a missing
  timestamp. Remember, it's not supposed to ever fail, and if it does,
  just log a warning and proceed with a fake timestamp instead. In
  that case something is wrong, but using a non-stable, fake timestamp
  is the least of the problems here.
  We already have a stable identifier (the host-id) which we can use to
  generate a fake timestamp. Use it.

In case the user would replace the secret_key file, we also don't want
that accessing nm_utils_host_id_get_timestamp*() yields different
results. It's not implemented (nor necessary) to support reloading a
different timestamp. Hence, nm_utils_host_id_get_timestamp() should
memoize the value and ensure that it never changes.
2018-12-12 14:03:35 +01:00
Thomas Haller
d693e03a74 core/trivial: rename nm_utils_get_boot_id_*()
Rename to nm_utils_boot_id_*(), it matches nm_utils_machine_id_*()
and nm_utils_host_id_get().
2018-12-12 12:52:55 +01:00
Thomas Haller
6ffcd26317 core/trivial: rename secret-key to host-key
Now that the secret-key is hashed with the machine-id, the name is
no longer best.

Sure, part of the key are persisted in /var/lib/NetworkManager/secret_key
file, which the user is well advised to keep secret.

But what nm_utils_secret_key_get() returns is first and foremost a binary
key that is per-host and used for hashing a per-host component. It's
really the "host-id". Compare that to what we also have, the
"machine-id" and the "boot-id".

Rename.
2018-12-12 12:52:55 +01:00
Thomas Haller
1bff602c46 device: fix matching device-spec for DHCP plugin
https://bugzilla.redhat.com/show_bug.cgi?id=1658057

Fixes: b9eb264efe
2018-12-11 12:43:39 +01:00
Thomas Haller
54ebe32f68 connectivity: consider default route for global connectivity state
When we agregate the connectivity state, only devices that
have the best default route should be considered.

Since we do connectivity checking per-device, the per-device check
does not care whether traffic to the internet is really routed via this
device.

But when talking about the global connectivity state, we care mostly
about the (best) default route. So, we should not allow a device with
worse or now default route, to contribute its connectivity state.

Fixes: 6b7e9f9b22
2018-12-11 09:23:47 +01:00
Thomas Haller
128aec5c2e connectivity: issue connectivity check right away on configuration reload 2018-12-11 09:23:47 +01:00
Thomas Haller
1fac911cf0 connectivity: log address family for device's connectivity checks 2018-12-11 09:23:47 +01:00
Thomas Haller
ade753d06f connectivity: fix determining the global connectivity state
Since we determine the connectivity state of each device individually,
the global connectivity state is an aggregate of all these states.

I am not sure about considering here devices that don't have the (best)
default route for their respective address family. But anyway.

When we aggregate the best connectivity, we chose the numerical largest
value. That is wrong, because PORTAL is numerically smaller than
LIMITED.

That means, if you have two devices, one with connectivity LIMITED and
one with connectivity PORTAL, then LIMITED wrongly wins.

Fixes: 6b7e9f9b22

https://bugzilla.redhat.com/show_bug.cgi?id=1619873
2018-12-11 09:23:47 +01:00
Thomas Haller
8dfa903a9d core: avoid calling platform code with invalid ifindex (4)
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/95

Fixes: 945c904f95
2018-12-10 16:43:23 +01:00
Beniamino Galvani
29e8f6d5a1 device: always take device down when changing MAC for wifi devices
If the device is not taken down wpa_supplicant does not detect that
the MAC changed and the authentication can fail due to wrong key
derivation.

Fixes: e206a34732

https://bugzilla.redhat.com/show_bug.cgi?id=1656157
2018-12-10 11:10:04 +01:00
Thomas Haller
8c744b66cc core: avoid calling platform code with invalid ifindex (3)
Fixes: 945c904f95
2018-12-10 08:29:49 +01:00
Thomas Haller
b635b4d419 core: improve and fix keeping connection active based on "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 37e8c53eee
"core: Introduce helper class to track connection keep alive". This
should be fixed now too, with improved behavior.

Fixes: 37e8c53eee

https://bugzilla.redhat.com/show_bug.cgi?id=1530977
2018-12-09 14:47:32 +01:00
Thomas Haller
a4bdb161eb device: arm keep-alive instance when queuing active-connection for activation
Now that the keep-alive instance defaults to ALIVE by default, we can
always arm it when starting to activate the active-connection.

The keep-alive instance may have been armed earlier already:
for example, when binding its lifetime to a D-Bus name or
when watching the connection's visible state.

However, at the moment when we queue the active-connection for
activation, we also want to make sure that the keep-alive instance is
armed. It is nicer for consistancy reasons.

Note, that nm_keep_alive_arm() has no effect if nm_keep_alive_disarm()
was called earlier already. Also note, that NMActiveConnection will
disarm the keep-alive instance, when changing to a state greater than
ACTIVATED. So, all works together nicely.

Also, no longer arm the keep-alive instance in the constructor of
NMActiveConnection. It would essentially mean, that the instances
is aremd very early.

Also, as alternative point of interest, arm the keep-alive instance
when registering the signal handler in "nm-policy.c".
2018-12-09 14:47:32 +01:00
Thomas Haller
e1b0451d68 device: always disconnect in nm_device_disconnect_active_connection()
Previously, if @active referenced a device but was not currently queued
or the current activation request, nothing was done.

Now, in such a case still call nm_active_connection_set_state_fail().
Note that nm_active_connection_set_state_fail() has no effects on
active-connections that are already in disconnected state (which
we would expect by such an active connection). Likely there is no
visible change here, but it feels more correct to ensure the active
connection is always failed.
2018-12-09 14:47:31 +01:00
Thomas Haller
71a090c920 device: use correct active-connection's state-change reason in nm_device_disconnect_active_connection()
It just makes more sense, to let the caller decide on the reason.
2018-12-09 14:47:31 +01:00
Thomas Haller
8f36019731 device: pass active-connection's state-change reason to nm_device_disconnect_active_connection()
No change in behavior, yet.
2018-12-09 14:47:31 +01:00
Thomas Haller
fe5f5f7a0e device: pass active-connection's state-change reason to _clear_queued_act_request()
No change in behavior, yet.
2018-12-09 14:47:31 +01:00
Thomas Haller
461bf7aa0c device: add state-change reason argument to nm_device_disconnect_active_connection()
nm_device_disconnect_active_connection() is generally useful and a prefered
form to fail an active connection. The device's state-change reason is important,
so it needs to be injected.
2018-12-09 14:47:31 +01:00
Beniamino Galvani
84f9c9489b device: avoid platform assertion failure
Avoid the following:

   nmp_cache_lookup_entry_link: assertion 'ifindex > 0' failed
2018-12-06 11:12:03 +01:00
Thomas Haller
6ba9f47c94 core: avoid calling platform code with invalid ifindex (2)
Fixes: 945c904f95
2018-12-04 13:13:34 +01:00
Thomas Haller
d45eed4437 core: avoid calling platform code with invalid ifindex
Since commit 945c904f95 "platform: assert against valid ifindex and
remove duplicate assertions", it is no longer allowed to call certain
platform functions with invalid ifindex.

These trigger now an assertion. Note that the assertion is merely a
g_return_val_if_fail(), hence in non-debug mode, this does not lead to
a crash.

Fixes: 945c904f95
2018-12-03 13:47:42 +01:00
Thomas Haller
1a2e767f1f device/shared: set ANDROID_METERED option 43 for shared connections
The problem is that updating the metered value of a shared connection is
not implemented. The user needs to fully reactivate the profile for changes
to take effect.

That is unfortunate, especially because reapplying the route metric
works in other other cases.
2018-12-03 12:28:45 +01:00
Thomas Haller
140a5e3316 all: make use of NM_MAKE_STRV() macro 2018-12-01 15:16:48 +01:00
Beniamino Galvani
446e5b27d6 core: add checks on connection default properties
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.
2018-12-01 15:16:48 +01:00
Beniamino Galvani
218d7687a0 device: fix wrong connection default property
Fixes: 96cabbcbb8
2018-12-01 15:16:48 +01:00
Lubomir Rintel
b385ad0159 all: say Wi-Fi instead of "wifi" or "WiFi"
Correct the spelling across the *entire* tree, including translations,
comments, etc. It's easier that way.

Even the places where it's not exposed to the user, such as tests, so
that we learn how is it spelled correctly.
2018-11-29 17:53:35 +01:00
Lubomir Rintel
3a999475ef wifi/olpc-mesh: allow autoconnect
There's no reason the mesh shouldn't autoconnect. Almost.

The mesh and regular Wi-Fi shares the same radio. There, in the first
place, probably shouldn't have been separate NMDevices. Not sure whether
we can fix it at this point, but we can surely avoid unnecessary
competition between the two devices: give the regular Wi-Fi priority and
only connect mesh if the regular companion stays disconnected.

For the record; connections shipped on XO-1 laptops all have
autoconnect=off and thus are not affected by this.
2018-11-29 17:50:00 +01:00
Lubomir Rintel
d64e577272 wifi: do not persist the mac address on AddAndActivate
If the client wants to pinpoint the connection to a particular device
they can just add an appropriate property.

That said, MAC address probably even doesn't count as appropriate; an
interface name is supposed to stay stable and could be used in such
cases.

This fixes the case where "nmcli d wifi connect ..." ends up with a
connection tied to a rather random device that happened to be around
even without the "ifname" argument.
2018-11-29 17:50:00 +01:00
Thomas Haller
e206a34732 device: avoid taking down link to change MAC address
A lot of drivers actually support changing the MAC address of a link
without taking it down.

Taking down a link is very bad, because kernel will remove routes
and IPv6 addresses.

For example, if the user used a dispatcher script to add routes,
these will be lost. Note that we may change the MAC address of a
device any time. For example, a VLAN device watches the parent's
MAC address and configures it (with a logging message "parent hardware
address changed to ...").

Try first whether we can change the MAC address without taking the
link down. Only if that fails, retry with taking it down first.

https://bugzilla.redhat.com/show_bug.cgi?id=1639274
2018-11-29 13:50:10 +01:00
Beniamino Galvani
94ea3a5e1b team: make teamd log to syslog
When teamd is started in foreground it logs to stderr and its output
appears in the journal as coming from NM, which is quite confusing.

Use the new TEAM_LOG_OUTPUT environment variable [1] to tell teamd to
log through syslog, unless NetworkManager is in debug mode.

[1] https://github.com/jpirko/libteam/commit/e47d5db53873

https://bugzilla.redhat.com/show_bug.cgi?id=1596917
2018-11-23 13:59:37 +01:00
Beniamino Galvani
e909778710 lldp: fix parsing of vlan-name attribute
We used to read 3 bytes after the TLV, fix this.

Also, check that string length is at most 32 bytes as specified in
figure E.3 of IEEE 802.1AB-2009.

Fixes: 18133ea142

https://bugzilla.redhat.com/show_bug.cgi?id=1652210
2018-11-22 09:08:00 +01:00
Thomas Haller
aae6846d20 device: ignore "carrier-wait" when device is already activated
nm_device_has_pending_action_reason() marks the device as busy, which in
turn delays "startup-complete" and NetworkManager-wait-online.service.

A device which has no carrier but is otherwise in activated state, is
clearly ready. I didn't test this, but I presume that can easily be the
case with static IP configuration (which can activate without the device
having carrier).
2018-11-15 15:36:49 +01:00
Thomas Haller
8861ac2976 dhcp: add "ipv4.dhcp-client-id=duid" setting
Add a new mode for the DHCPv4 client identifier.

"duid" is what the internal (systemd) DHCP client already does by
default. It is also the same as used by systemd-networkd's
"ClientIdentifier=duid" setting. What we still lack (compared to
networkd) are a way to overwrite IAID and the DUID.

Previously, this mode was used by the internal DHCP plugin
by default. However, it could not be explicitly configured.
In general, our default values should also be explicitly selectable.
Now the "duid" client identifier can also be used with the "dhclient"
plugin.
2018-11-13 19:09:34 +01:00
Thomas Haller
7ffbf71276 all: add "${MAC}" substituion for "connection.stable-id"
We already had "${DEVICE}" which uses the interface name.
In times of predictable interface naming, that works well.
It allows the user to generate IDs per device which don't
change when the hardware is replaced.

"${MAC}" is similar, except that is uses the permanent MAC
address of the device. The substitution results in the empty
word, if the device has no permanent MAC address (like software
devices).

The per-device substitutions "${DEVICE}" and "${MAC}" are especially
interesting with "connection.multi-connect=multiple".
2018-11-13 19:09:34 +01:00
Thomas Haller
50121ee028 core: cleanup generating DUID in nm-device.c
- use NMUuid type where appropriate.

- no error handling for generate_duid_from_machine_id().
  It cannot fail anymore.

- add thread-safety to generate_duid_from_machine_id() with
  double-checked locking.

- use unions for converting the sha256 digest to the target
  type.
2018-11-13 19:09:33 +01:00
Thomas Haller
c51e63feb6 core: pass boot-id to nm_utils_stable_id_parse()
For testing purpose, it's bad to let nm_utils_stable_id_parse()
directly access nm_utils_get_boot_id_str(). Instead, the function
should have no side-effects.

Since the boot-id is anyway cached, accessing it is cheap. Even
if it likely won't be needed.
2018-11-13 19:09:31 +01:00
Thomas Haller
8308311264 core: refactor loading machine-id and cache it
Previously, whenever we needed /etc/machine-id we would re-load it
from file. The are 3 downsides of that:

 - the smallest downside is the runtime overhead of repeatedly
   reading the file and parse it.

 - as we read it multiple times, it may change anytime. Most
   code in NetworkManager does not expect or handle a change of
   the machine-id.
   Generally, the admin should make sure that the machine-id is properly
   initialized before NetworkManager starts, and not change it. As such,
   a change of the machine-id should never happen in practice.
   But if it would change, we would get odd behaviors. Note for example
   how generate_duid_from_machine_id() already cached the generated DUID
   and only read it once.
   It's better to pick the machine-id once, and rely to use the same
   one for the remainder of the program.
   If the admin wants to change the machine-id, NetworkManager must be
   restarted as well (in case the admin cares).
   Also, as we now only load it once, it makes sense to log an error
   (once) when we fail to read the machine-id.

 - previously, loading the machine-id could fail each time. And we
   have to somehow handle that error. It seems, the best thing what we
   anyway can do, is to log an error once and continue with a fake
   machine-id. Here we add a fake machine-id based on the secret-key
   or the boot-id. Now obtaining a machine-id can no longer fail
   and error handling is no longer necessary.

Also, ensure that a machine-id of all zeros is not valid.

Technically, a machine-id is not an RFC 4122 UUID. But it's
the same size, so we also use NMUuid data structure for it.

While at it, also refactor caching of the boot-id and the secret
key. In particular, fix the thread-safety of the double-checked
locking implementations.
2018-11-13 19:04:34 +01:00
Thomas Haller
37e47fbdab build: avoid header conflict for <linux/if.h> and <net/if.h> with "nm-platform.h"
In the past, the headers "linux/if.h" and "net/if.h" were incompatible.
That means, we can either include one or the other, but not both.
This is fixed in the meantime, however the issue still exists when
building against older kernel/glibc.

That means, including one of these headers from a header file
is problematic. In particular if it's a header like "nm-platform.h",
which itself is dragged in by many other headers.

Avoid that by not including these headers from "platform.h", but instead
from the source files where needed (or possibly from less popular header
files).

Currently there is no problem. However, this allows an unknowing user to
include <net/if.h> at the same time with "nm-platform.h", which is easy
to get wrong.
2018-11-12 16:02:35 +01:00