It is safer to enable send-sci by default because, at the cost of
8-byte overhead, it makes MACsec work over bridges (note that kernel
also enables it by default). While at it, also make the option
configurable.
https://bugzilla.redhat.com/show_bug.cgi?id=1588041
We set the metric to the routes as we receive them from the PPP plugin. We
ought to let the modem know before it starts IPv4 configuration, not right
before the commit.
https://bugzilla.redhat.com/show_bug.cgi?id=1585611
For better or worse, the logging done for ipv4.dhcp-client-id
is prefixed with ipv4.dhcp-client-id. Let ipv6.dhcp-duid follow
that pattern.
Also, generate_duid_from_machine_id() would log at two places,
it should use the same logging prefix.
Also, it logs the value of "duid" variable. Ensure, that "duid"
is not %NULL at that point.
Also, fix leak of nm_dhcp_utils_duid_to_string() value during logging.
Previously, there were two blocks
if (NM_IN_SET (duid, "ll", "llt")
preprocess_hwaddr()
else if (NM_IN_SET (duid, "stable-ll", "stable-llt", "stable-uuid"))
preprecess_stable_id()
if (nm_streq (duid, "ll")
generate_ll()
else if (nm_streq (duid, "llt"))
generate_llt()
else if (nm_streq (duid, "stable-ll")
generate_stable_ll()
...
That is, the latter block depends on the execution of the previous
block, while the previous block is guarded by a particular condition,
slighlty different than the condition in the later block.
It is confusing to follow. Instead, check for our cases one by one, and
when we determined a particular DUID type, process it within the same block
of code. Now the code consists of individual blocks, that all end with a "goto
out*". That means, it's easier to understand the flow of the code.
Also, don't initialize duid_error variable and separate between
"out_error" and "out_good". This allows that the compiler gives
a warning if we missed ot initialize duid_error.
dhcp6_get_duid() already handles failure to generate the DUID in a
sensible manner. No reason to duplicate the error handling in
generate_duid_from_machine_id().
Especially, because generate_duid_from_machine_id() used to cache the
random DUID in memory and reuse it from then on. There is no reason to do
that, /etc/machine-id must be available to NetworkManager. We still
handle such a grave error gracefully by generating a random DUID.
First of all, generating the client-id is not expected to fail. If it fails,
something is already very wrong. Maybe, a failure to generate a client-id
should result in failing the activation. However, let's not go full
measure in this question.
Instead:
- ensure that we log a warning and a reason why the client-id could not
be generated.
- fallback to a random client id. Clearly, we were unable to generate
the requested client-id, hence, we should fallback to a default value
which does not make the host easily identifyable. Of course, that means
that the generated DHCP client-id is not at all stable. But note that
something is already very wrong, so when handling the error we should do
something conservative (that is, protecting the users privacy).
This is also what happens for a failure to generate the ipv6.dhcp-duid.
In practice, there should be no difference between peeking into
the platform cache, or using the cached value from nm_device_get_hw_address().
Prefer the hardware address from the platform, because:
- we also pass the current MAC address to nm_dhcp_manager_start_ip4().
For not particularly strong reason, it uses the MAC address obtained
from platform. At the least, it makes sense that we use the same
addresses for the client-id as well.
- ipv6.dhcp-duid also gets the address from platform. Again,
no strong reason either way, but they should behave similar
in this regard.
They're intended to be used via update-alternatives(8) as compatibility
shims for Red Hat systems without the legacy network control scripts.
While they're not strictly parts of the settings plugin, they're best
just installed along with it, since they're supposed to be available on
systems that use the ifcfg files.
This allows implementing some convenience features in nmcli -- listing
the backing store for the connection in "nmcli c show", and using the
filename for specifying connection in "nmcli c up/down".
Eventually, paired with ReloadConnections(), this could be used to
implement something similar to what "systemctl edit" does for units
(though we'd need to pick another command name as we aready use
"nmcli c edit" for something different).
When the used client is dhclient we were used to search for DUID not
only in the specific lease files generated by NetworkManager, but also
in the global lease file generated outside NetworkManager.
Keep this capability but allow to just search in the NM lease files if
a value different from the default one is specified in dhcp-duid.
This commit centralizes the DUID generation in nm-device.c.
As a consequence, a DUID is always provided when starting a
DHCPv6 client. The DHCP client can override the passed DUID
with the value contained in the client-specific lease file.
allow to specify the DUID to be used int the DHCPv6 client identifier
option: the dhcp-duid property accepts either a hex string or the
special values "lease", "llt", "ll", "stable-llt", "stable-ll" and
"stable-uuid".
"lease": give priority to the DUID available in the lease file if any,
otherwise fallback to a global default dependant on the dhcp
client used. This is the default and reflects how the DUID
was managed previously.
"ll": enforce generation and use of LL type DUID based on the current
hardware address.
"llt": enforce generation and use of LLT type DUID based on the current
hardware address and a stable time field.
"stable-ll": enforce generation and use of LL type DUID based on a
link layer address derived from the stable id.
"stable-llt": enforce generation and use of LLT type DUID based on
a link layer address and a timestamp both derived from the
stable id.
"stable-uuid": enforce generation and use of a UUID type DUID based on a
uuid generated from the stable id.
We will soon introduce a property to set a custom DUID and we want
to enforce that the provided value is used.
Note that this commit does not cause any change in behavior in current
code.
The nm_dhcp_dhclient_save_duid() function will save a newly generated
DUID to a previously existing lease file. The function will only save
the DUID if not present in the lease file: in this case, should preserve
the other contents of the lease file.
A dhclient lease file for IPv6 generated by NetworkManager will always
add the DUID as a first item: so in practice finding a lease file
without DUID will never happen.
This has hidden a bug in the function: the loop that is meant to append
the non-duid lines in the lease file would strip all the newlines,
mangling the lease file.
Fix the function allowing to keep the original lines and add a test to
check this functionality is kept well functioning.
FIXME: the new test and the other duid ones already there store the file
in the current working-directory. Tests should not do that.
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] 15177a34behttps://github.com/NetworkManager/NetworkManager/pull/127
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: f4fc62bad8https://bugzilla.redhat.com/show_bug.cgi?id=1585494
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
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
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.
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.
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".
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.
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.
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".)
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.
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
If the active connection is deactivated because the device is gone,
don't block autoconnection. Otherwise, whenever the device comes
back (e.g. maybe it was reset in the middle of a connection attempt),
the autoconnection logic won't be triggered, as the settings are still
blocked.
I'm able to reproduce this by performing a WWAN modem reset in the
middle of a connection attempt.
https://github.com/NetworkManager/NetworkManager/pull/121
Add new stable-id specifier "${DEVICE}" to explicitly declare that the
connection's identity differs per-device.
Note that for settings like "ipv6.addr-gen-mode=stable" we already hash
the interface's name. So, in combination with addr-gen-mode, using this
specifier has no real use. But for example, we don't do that for
"ipv4.dhcp-client-id=stable".
Point being, in various context we possibly already include a per-device
token into the generation algorithm. But that is not the case for all
contexts and uses.
Especially the DHCPv4 client identifier is supposed to differ between interfaces
(according to RFC). We don't do that by default with "ipv4.dhcp-client-id=stable",
but with "${DEVICE}" can can now be configured by the user.
Note that the fact that the client-id is the same accross interfaces, is not a
common problem, because profiles are usually restricted to one device via
connection.interface-name.
Otherwise, the generated client-id depends purely on the profile's
stable-id. It means, the same profile (that is, either the same UUID
or same stable-id) on different hosts will result in identical client-ids.
That is clearly not desired. Hash a per-host secret-key as well.
Note, that we don't hash the interface name. So, activating the
profile on different interfaces, will still yield the same client-id.
But also note, that commonly a profile is restricted to one device,
via "connection.interface-name".
Note that this is a change in behavior. However, "ipv4.dhcp-client-id=stable"
was only added recently and not yet released.
Fixes: 62a7863979
and add nm_utils_secret_key_get() to cache the secret-key, to only
obtain it once.
nm_utils_secret_key_read() is not expected to fail. However, in case
of an unexpected error, don't propagate the error to the caller,
but instead handle it internally.
That means, in case of error:
- log a warning within nm_utils_secret_key_read() itself.
- always return a generated secret-key. In case of error, the
key won't be persisted (obviously). But the caller can ignore
the error and just proceed with an in-memory key.
Hence, also add nm_utils_secret_key_get() to cache the key. This way,
we only try to actually generate/read the secret-key once. Since that
might fail and return an in-memory key, we must for future invocations
return the same key, without generating a new one.
