Commit Graph

20294 Commits

Author SHA1 Message Date
Thomas Haller
644aa42f68 dns: change main.rc-manager=file behavior to always follow symlink
With "main.rc-manager=file", if /etc/resolv.conf is a symlink, NetworkManager
would follow the symlink and update the file instead.

However, note that realpath() only returns a target, if the file actually
exists. That means, if /etc/resolv.conf is a dangling symlink, NetworkManager
would replace the symlink with a file.

This was the only case in which NetworkManager would every change a symlink
resolv.conf to a file. I think this is undesired behavior.

This is a change in long established behavior. Although note that there were several
changes regarding rc-manager settings in the past. See for example commit [1] and [2].

Now, first still try using realpath() as before. Only if that fails, try
to resolve /etc/resolv.conf as a symlink with readlink().

Following the dangling symlink is likely not a problem for the user, it
probably is even desired. The part that most likely can cause problems
is if the destination file is not writable. That happens for example, if
the destination's parent directories are missing. In this case, NetworkManager
will now fail to write resolv.conf and log a warning. This has the potential of
breaking existing setups, but it really is a mis-configuration from the user's
side.

This fixes for example the problem, if the user configures
/etc/resolv.conf as symlink to /tmp/my-resolv.conf. At boot, the file
would not exist, and NetworkManager would previously always replace the
link with a plain file. Instead, it should follow the symlink and create
the file.

[1] 718fd22436
[2] 15177a34be

https://github.com/NetworkManager/NetworkManager/pull/127
2018-06-05 16:21:10 +02:00
Beniamino Galvani
92ebd16cee cli: fix array out-of-bounds access on command timeout
Fixes: 4b3297271e

https://bugzilla.redhat.com/show_bug.cgi?id=1573839
2018-06-05 14:49:24 +02:00
Beniamino Galvani
7696e6c1fa manager: fix failed assertion on user activations
We can't use g_steal_pointer(&active) in the argument list if another
argument uses @active because the order of evaluation is not defined.

This fixes the following bug:

 src/nm-manager.c:511:_async_op_complete_ac_auth_cb: assertion failed: (active == async_op_data->ac_auth.active)

Fixes: f4fc62bad8

https://bugzilla.redhat.com/show_bug.cgi?id=1585494
2018-06-04 18:06:47 +02:00
Beniamino Galvani
3fb4eed3ef settings: let connections keep NMSettings alive
The NMSettings instance can't be disposed while there is any exported
connection. Ideally we should unexport all connections on NMSettings'
disposal, but for now leak @self on termination when there are
connections alive.

This fixes the following bug on shutdown:

 assertion failed: (c_list_is_empty (&priv->connections_lst_head))
 #0  raise () from target:/lib64/libc.so.6
 #1  abort () from target:/lib64/libc.so.6
 #2  g_assertion_message (domain=0x66cab2 "NetworkManager", file=0x6a5e48 "src/settings/nm-settings.c", line=1929)
 #3  g_assertion_message_expr () at gtestutils.c:2555
 #4  finalize (object=0x1dab170) at src/settings/nm-settings.c:1929
 #5  g_object_unref (_object=0x1dab170) at gobject.c:3340
 #6  dispose (object=0x1de50b0) at src/nm-manager.c:7139
 #7  g_object_unref (_object=0x1de50b0) at gobject.c:3303
 #8  _nm_singleton_instance_destroy () at src/nm-core-utils.c:138
 #9  _dl_fini () from target:/lib64/ld-linux-x86-64.so.2
 #10 __run_exit_handlers () from target:/lib64/libc.so.6
 #11 exit () from target:/lib64/libc.so.6
 #12 main (argc=<optimized out>, argv=<optimized out>) at src/main.c:460

https://bugzilla.redhat.com/show_bug.cgi?id=1579858
2018-06-03 16:46:48 +02:00
Beniamino Galvani
bd63d39252 dhcp: fix handling of failure events
DHCPv4 can fail for two reasons:

 (a) the client failed to contact server and to get an initial lease

 (b) the client failed to renew the lease after it was successfully
     acquired

For (a) the client generates a TIMEOUT event, for (b) an EXPIRED
event.  Currently we fail the IP method immediately after (a), but
this doesn't work well when the carrier flickers and we restart the
client because if the server goes temporarily down, the IP method
fails and DHCP is never restarted.

Let's change this, and determine whether to fail IP configuration only
by looking at the current IP state: when it's IP_CONF then we are
getting the initial lease and a failure means that IP configuration
must fail; otherwise any other state means that the lease expired or
could not be renewed and thus we keep the client running for the grace
period.

https://bugzilla.redhat.com/show_bug.cgi?id=1573780
2018-06-02 10:50:18 +02:00
Beniamino Galvani
0aa2d252bf cli: fix active connections color
Fixes: a1b25a47b0
2018-06-01 18:10:03 +02:00
Beniamino Galvani
e86ea0240f device: don't try to change MTU on a disconnected device
ip_config_merge_and_apply() can be called without an applied
connection, but then it calls nm_device_set_ip_config() and tries to
retrieve the configured MTU, throwing an assertion if the applied
connection is NULL.

src/devices/nm-device.c: line 8080 (nm_device_get_configured_mtu_for_wired): should not be reached

Since it doesn't make sense apply a MTU from the connection when there
is no connection, add a check against this.
2018-06-01 17:02:23 +02:00
Thomas Haller
357717c2aa cli: merge branch 'th/cli-connection-handling-2'
https://github.com/NetworkManager/NetworkManager/pull/125
2018-06-01 16:03:32 +02:00
Thomas Haller
5d716defc1 cli/trivial: move code 2018-06-01 16:03:23 +02:00
Thomas Haller
68fa68b3ed cli: rework printing of general active-connection properties
use nmc_print() for the job.

Also, localize non-terse output.

Also, fix bug with

  $ nmcli c s /org/freedesktop/NetworkManager/ActiveConnection/1

if active connection #1 is invisible to the user.

Also, previously, fill_output_active_connection() wrongly tries to
write to a field that doesn't exist:

  set_val_strc (arr, 13-idx_start, s_con ? nm_setting_connection_get_slave_type (s_con) : NULL);
2018-06-01 16:03:23 +02:00
Thomas Haller
b990cee00c cli: sort active-connection for nmcli connection show $PROFILE output
There might be multiple active connections. Output them in a defined order.
2018-06-01 16:03:23 +02:00
Thomas Haller
a1b25a47b0 cli: rework printing of nmcli connection for multiple active connections
The output of `nmcli connection show` contains also information about
whether the profile is currently active, for example the device and
the current (activation) state.

Even when a profile can be activated only once (without supporting
mutiple activations at the same time), there are moments when a
connection is activating and still deactivating on another device.
NetworkManager ensures in the case with single activations that
a profile cannot be in state "activated" multiple times. But that
doesn't mean, that one profile cannot have multiple active connection
which reference it. That was already handled wrongly before, because
`nmcli connection show` would only search the first matching
active-connection. That is, it would arbitrarily pick an active
connection in case there were multiple and only show activation
state about one.
Furthermore, we will soon also add the possibility, that a profile can be
active multiple times (at the same time). Especially then, we need to
extend the output format to show all the devices on which the profile is
currently active.

Rework printing the connection list to use nmc_print(), and fix various
issues.

- as discussed, a profile may have multiple active connections at each time.
  There are only two possibilities: if a profile is active multiple
  times, show a line for each activation, or otherwise, show the
  information about multiple activations combined in one line, e.g. by
  printing "DEVICE eth0,eth1". This patch, does the former.
  We will now print a line for each active connection, to show
  all the devices and activation states in multiple lines.
  Yes, this may result in the same profile being printed multiple times.
  That is a change in behavior, and inconvenient if you do something
  like

     for UUID in $(nmcli connection show | awk '{print$2}'); do ...

  However, above is anyway wrong because it assumes that there are no
  spaces in the connection name. The proper way to do this is like

     for UUID in $(nmcli -g UUID connection show); do ...

  In the latter case, whenever a user selects a subset of fields
  (--fields, --get) which don't print information about active connections,
  these multiple lines are combined. So, above still works as expected,
  never returning duplicate UUIDs.

- if a user has no permissions to see a connection, we previously
  would print "<invisible> $NAME". No longer do this but just print
  the ID was it is reported by the active-connection. If the goal
  of this was to prevent users from accidentally access the non-existing
  connection by $NAME, then this was a bad solution, because a script
  would instead try to access "<invisible> $NAME". This is now solved
  better by hiding the active connection if the user selects "-g NAME".

- the --order option now sorts according to how the fields are shown.
  For example, with --terse mode, it will evaluate type "802-11-wireless"
  but with pretty mode it will consider "wifi". This may change the
  ordering in which connections are shown. Also, for sorting the name,
  we use g_utf8_collate() because it's unicode.
2018-06-01 16:03:23 +02:00
Thomas Haller
3690f8bcd5 clients/tests: add test for showing connection's active state 2018-06-01 16:03:23 +02:00
Thomas Haller
3645be6484 clients/tests: add test for showing invisible connection in nmcli
It also shows how we handle invisible connections wrongly, when we have
multiple active-connections that reference them.
2018-06-01 16:03:23 +02:00
Thomas Haller
46b7d52109 clients/tests: add tests for output of nmcli con show 2018-06-01 16:03:23 +02:00
Thomas Haller
b18c9e44ed shared/trivial: add code comment to explain NMUtilsEnumValueInfo consistency rules 2018-06-01 12:26:07 +02:00
Beniamino Galvani
a1f1b13f4f settings: fix plugins loading
Since load_plugin() modifies the list, we must pass its address.

Fixes: fd86a1aebb
2018-06-01 10:26:01 +02:00
Thomas Haller
bc28a2b164 man: clarify main.rc-manager=file behavior for resolv.conf as dangling symlink
It's not clear whether this was desired behavior. However, it was
behavior for a long time, so we probably should not change it.

Just document what happens with dangling symlinks.
2018-06-01 09:05:38 +02:00
Thomas Haller
5485dbea21 dns: move local variables to inner scope in update_resolv_conf() 2018-06-01 08:26:29 +02:00
Lubomir Rintel
960632ee7b release: bump version to 1.11.4 (development) 2018-05-31 16:58:10 +02:00
Lubomir Rintel
0317fad966 release: update NEWS for a development snapshot 2018-05-31 16:57:49 +02:00
Lubomir Rintel
ad7b700d6a test: don't assert on the tun link being up to date prior to upping it
Fixes the test run with:

  NMTST_SEED_RAND=502735495 src/platform/tests/test-link-linux \
      -p /link/software/detect/tun/external
2018-05-31 16:54:35 +02:00
Thomas Haller
cc472f52fb all: merge branch 'th/build-cleanup-defines'
https://github.com/NetworkManager/NetworkManager/pull/126
2018-05-31 15:59:47 +02:00
Thomas Haller
f445128af4 build/meson: fix meson build for shared files
The files in shared/nm-utils are not compiled as one static library,
instead each subproject that needs (parts of) them, re-compiles the
files individually.

The major reason for that is, because we might have different compile
flags, depending on whether we build libnm-core or
libnm-util/libnm-glib. Actually, I think that is not really the case,
and maybe this should be refactored, to indeed build them all as a
static library first.

Anyway, libnm-util, libnm-glib, clients' common lib, they all need a
different set of shared files that they should compile. Refactor
"shared/meson.build" to account for that and handle it like autotools
does.

Another change is, that "shared_c_siphash_dep" no longer advertises
"include_directories: include_directories('c-siphash/src')". We don't
put c-siphash.h into the include search path. Users who need it, should
include it via "#include <c-siphash/src/c-siphash.h>". The only exception
is when building shared_n_acd library, which is not under our control.
2018-05-31 15:59:38 +02:00
Thomas Haller
b8b6100c78 all: replace systemd's siphash24 with c-siphash
Originally, we used "nm-utils/siphash24.c", which was copied
from systemd's source tree. It was both used by our own NetworkManager
code, and by our internal systemd fork.

Then, we added "shared/c-siphash" as a dependency for n-acd.

Now, drop systemd's implementation and use c-siphash also
for our internal purpose. Also, let systemd code use c-siphash,
by patching "src/systemd/src/basic/siphash24.h".
2018-05-31 15:59:38 +02:00
Thomas Haller
b7426e91db build: use default NM_BUILD_* defines for tests
Use two common defines NM_BUILD_SRCDIR and NM_BUILD_BUILDDIR
for specifying the location of srcdir and builddir.

Note that this is only relevant for tests, as they expect
a certain layout of the directories, to find files that concern
them.
2018-05-31 15:59:38 +02:00
Thomas Haller
7fcf33908b build: define NM_BUILD_SRCDIR and NM_BUILD_BUILDDIR 2018-05-31 15:59:38 +02:00
Thomas Haller
d63cf1ef2f build: use common locale directory for building nmtui
All other places use $(nmlocaledir) variable.
2018-05-31 15:59:38 +02:00
Thomas Haller
e5d1a71396 build: unifiy specifying locale directory define 2018-05-31 15:59:38 +02:00
Thomas Haller
82b088ab5f build: don't add shared/nm-utils directory to include search path
All users are supposed to include files from nm-utils by fully specifying
the path. -I.*shared/nm-utils is wrong.

Only, systemd code likes to include "siphash24.h" directly. Instead of
adding "-Ishared/nm-utils" to the search path, add an intermediary
header to sd-adapt. Note, that in the meantime we anyway should rework
siphash24 to use shared/c-siphash instead.

This also fixes build for meson, which was broken recently.
2018-05-31 15:59:38 +02:00
Alfonso Sánchez-Beato
cdbd99c5d5 platform/wifi: do not double-free nl_msg
In some places, there was an unneeded call to nlmsg_free () for
messages declared with the nm_auto_nlmsg macro.
2018-05-31 11:53:26 +02:00
Lubomir Rintel
581ce347fb merge: branch 'lr/setting-plugin-props-cleanup'
https://github.com/NetworkManager/NetworkManager/pull/119
2018-05-31 11:52:06 +02:00
Lubomir Rintel
0a3f1ab1a4 settings-plugin: drop all properties
They're not useful and just add extra noise.
2018-05-31 11:50:02 +02:00
Lubomir Rintel
0112253064 settings: do away with plugin capabilities
There's exactly one and not too useful -- only used only in one spot
where we can do hapilly without it.
2018-05-31 11:50:02 +02:00
Lubomir Rintel
8b6c998a94 settings: don't use the name property to disambiguate plugins
Use the path instead. This drop an useless use of the "name" property,
which is, coincidentally also wrong. (We use "ibft" in the plugin path
whereas the property is set to "iBFT".)
2018-05-31 11:50:02 +02:00
Lubomir Rintel
159cb0302c settings: simplify the settings plugin loading log line
It's actually annoying, useless and wraps over even on wide displays.
Let's make it consistent with the log line we use for device plugins.

Also, this drops the last use of the "info" property and one useless use
of the "name" property.
2018-05-31 11:50:02 +02:00
Lubomir Rintel
fd86a1aebb settings: refactor load_plugins() to remote a harmful use of goto
Turn the plugin loading logic between load_plugin: and next: into a
subroutine.
2018-05-31 11:50:02 +02:00
Beniamino Galvani
851d89dc6a platform: sort known IPv6 addresses by scope before a sync
The order we want to enforce is only among addresses with the same
scope, as the kernel always keeps addresses sorted by
scope. Therefore, apply the same sorting to known addresses, so that
we don't try to unnecessary change the order of addresses with
different scopes.

https://bugzilla.redhat.com/show_bug.cgi?id=1578668
2018-05-30 17:59:57 +02:00
Thomas Haller
54356ac8c7 shared: don't use nm_str_hash() in "nm-enum-utils.c"
This was only used for some extra assertions. It' is not essential.
If this would be for real usage, we should add a dependancy so that
nm-utils/nm-enum-utils.c requires nm-hash-utils.h. But as it is,
this is not necessary.

This fixes build for meson, which wrongly tries to build nm-enum-utils.c
for libnm-util, but then fails to include nm-hash-utils.c. That should
be fixed independently.

Fixes: 84a6eff106
2018-05-30 11:57:10 +02:00
Thomas Haller
84a6eff106 shared: don't allow aliases re-numbering in _nm_utils_enum_from_str_full()
For _nm_utils_enum_to_str_full(), we always first look whether we have
an alias/nick for the numeric value, and preferably use that. That makes a
lot of sense, as it allows the caller to provide better names (aliases),
which are preferred over the name from the GLib type. It renames the
numeric value.

For the reverse conversion, this makes less sense. A name should have a
unique numeric value. That is, we should not use one name that maps to
a different numeric value based on value_infos and GLib type. IOW, we
should not re-number names.

Add an assertion that we don't provide such a value_infos parameter,
that conflicts with names from GLib type.

Also, although the case where GLib type and value_infos disagree is now
forbidden by an assert, reorder the statements in _nm_utils_enum_from_str_full()
too. There is no difference in practice, but it mirros what we do in the
to-str case.
2018-05-29 16:28:09 +02:00
Thomas Haller
3021a8cf6b shared/trivial: rename local variable class
There is no real problem with using "class" in C. However, it would
be a keyword in C++. Just avoid it, and use "klass" instead.
2018-05-29 15:18:54 +02:00
Thomas Haller
7bde4bd492 libnm/tests: fix crash in tests
Fixes: daf4ba43da
2018-05-29 14:33:52 +02:00
Thomas Haller
3c6bd6769b shared: fix parsing aliases for flags in _nm_utils_enum_from_str_full()
Otherwise, the last alias overwrites previous values.

Fixes: b9fa0e0a19
2018-05-29 13:14:01 +02:00
Thomas Haller
daf4ba43da shared/tests: extend tests for nm_utils_enum_from_str() 2018-05-29 13:14:01 +02:00
Beniamino Galvani
33e87bd6ec n-acd: merge branch 'bg/n-acd-fixes-rh1578675'
https://bugzilla.redhat.com/show_bug.cgi?id=1578675
2018-05-29 11:21:53 +02:00
Beniamino Galvani
39b5691847 device: vlan: restart ARP announcement after we change MAC
After the parent MAC address changes and we update the VLAN MAC, also
restart ARP announcement to notify neighbors of the new address mapping.
2018-05-29 11:18:30 +02:00
Beniamino Galvani
b6b158e310 core: vlan: avoid unneeded casts 2018-05-29 11:18:30 +02:00
Beniamino Galvani
7f6a19b1ad n-acd: slightly improve logging
If timeout is 0 we don't really do a probe. Also, log the timeout.
2018-05-29 11:18:30 +02:00
Beniamino Galvani
d082af6b5c n-acd: better handle interfaces going temporarily down
NM sometimes brings an interface temporarily down (for example to
change a VLAN MAC to align it to the parent interface's one). When
this happens, any recv() or send() in n-acd fails, the n-acd instance
is reset to the initial state and a DOWN event is reported to the
manager, which currently does not handle it. The result is an
inconsistent state.

There is no simple way of dealing with the DOWN event in the
manager. What we can do instead is to:

 - ignore errors during recv() because there is really nothing we can
   do, except for waiting timeouts to expire;

 - during probe, ignore errors during send() so that we don't exceed
   the probe timeout;

 - during announcement, retry after a send() error to ensure we send
   all 3 announcements.

https://bugzilla.redhat.com/show_bug.cgi?id=1578675
2018-05-29 11:18:30 +02:00
Beniamino Galvani
2f4b3392d5 n-acd: use RFC 5227 timeout for announcements
When doing announcements, use the the timeout specified by RFC
5227. Note that timeout_multiplier might be 0.

This aligns behavior to upstream version of n-acd.
2018-05-29 11:18:30 +02:00