The name of the boot file can be either in option 67 or in a field of
the DHCP header. Add an accessor for the one from the DHCP header.
(cherry picked from commit 6a1349c5fb)
Add an internal option that specifies the boot file name from the DHCP
header. The option name 'filename' is the same as exposed by dhclient.
(cherry picked from commit 0994a444e5)
Control characters (DEL (=127) or those below 32) could cause undesired
effects when a client displays or parses DHCP options. Escape them.
(cherry picked from commit 0c10e4f8b6)
This mode was added to network-legacy in [1]. NetworkManager anyway always
does DHCP in parallel, so this is basically an alias for "dhcp".
Note that network-legacy's "single-dhcp" will stop waiting for DHCP
once the first device gets an address. NetworkManager currently cannot
do that. While it runs DHCP in parallel, all devices need to settle
and there is no concept where completing one device makes the overall
"startup complete" process finish early. That could however be added.
Anyway, while not being exactly the same, it's still more useful to do
something similar instead of not working at all.
See-also: https://github.com/dracutdevs/dracut/pull/853
See-also: https://github.com/dracutdevs/dracut/pull/961
See-also: https://github.com/dracutdevs/dracut/pull/1048
[1] 4026cd3b01
(cherry picked from commit bf7530ccc1)
Due to something that really should be fixed, NetworkManager merges the routes
that it wants to configure, with the routes that are configured externally.
This includes a subtract and merge dance, which is wrong.
Anyway. If we are in nm_platform_ip_route_sync(), then we never want to
actively configure a route, that we only have in the list because it is
(or was) present on the interface.
Otherwise we have a problem. Note that we make a plan which
routes/addresses to add/remove before starting. So, if we start with an
IPv4 address configured in kernel, then there is also a corresponding
local route. We would track that local route as external.
During sync, we first remove the IP address, and kernel automatically
also removes the local route. However, as we already made the plan to
keep that route, NetworkManager would wrongly configure it again.
This should fix that bug. It is anyway wrong to even try to explicitly
configure a route, that is purely in the list as being external.
https://bugzilla.redhat.com/show_bug.cgi?id=1979192#c11
(cherry picked from commit 13d749942f)
We will need to track whether a route is externally added or not.
We maybe could use rt_source for that, but instead add a boolean flag.
(cherry picked from commit dc0ac73780)
On architectures where "char" is signed, the check "ch < ' '" is also
TRUE for characters greater than 127 (that is, UTF-8 characters).
Let's preserve valid UTF-8 characters and don't clear them.
Also note that already before we filtered out invalid UTF-8 sequences,
so if we encounter here a character > 127, it is part of a valid UTF-8
sequence.
(cherry picked from commit fb3e6cb0dc)
Previously, once in_parent was TRUE it was never reset, thus the
remainder of the string was cleared. That was most likely not intended.
If the intent really was to clear all the remainder, then the code could
have simply truncated the string at the first '('.
(cherry picked from commit 17bdd3a40d)
This is like using nm_ascii_is_ctrl_or_del() instead of
nm_ascii_is_ctrl() in the previous version of the patch.
We thus now always will switch to ANSIC escaping if we see
a ASCII DEL character. That is probable desirable, but either
way should not make a big difference (because we can parse
the DEL character both in regular quotation and in ANSIC quotation).
The patch is however larger, to also take the opportunity to only check
for nm_ascii_is_regular() in the "fast path". The behavior is the same
as changing nm_ascii_is_ctrl() to nm_ascii_is_ctrl_or_del().
(cherry picked from commit fc2f758af5)
On architectures where "char" is signed, the check "ch < ' '" is
also TRUE for non-ASCII characters greater than 127. This is an
easy mistake to make. Fix it by using nm_ascii_is_control() which
gets this right.
It's a bug, but possibly not too bad because unnecesarily escaping
a UTF-8 characters is not a severe problem, because the user anyway must
be prepared to unescape the string.
(cherry picked from commit 83f888054b)
These functions have overlap with g_ascii_is*() functions.
However g_ascii_is*() (and the is* functions from <ctype.h>) are
always confusing to me, in the sense that it's not clearly stated
which characters qualify for a certain category. And review is not
easy either, because they are implemented via a table lookup.
E.g. were you aware that 127 is considered g_ascii_iscntrl()? Probably
you were, but it's not clear to see that anywhere.
The main point of our own functions is to have is easier to see how
characters get categorized, by using comparison instead of table lookup.
Also, several existing code did in fact not use the g_ascii_is*()
macros, possibly because of the (perceived) difficulty to understand
their exact meaning. As a consequence, several checks got wrong.
For example, (ch < ' ') is not a valid check for testing whether
the character is a ASCII control character, for two reasons:
- if char is a signed type (as likely it is), then this also evaluates
to TRUE for all non-ASCII, UTF-8 characters that are greater than
127.
- it does not consider DEL character (127) a control character.
(cherry picked from commit 5b6005d06e)
Problems of this patch:
- the code does not differentiate between an ifcfg file and an alias
file. Different shell variables are honored however depending on the
context and the warning should reflect that.
- there are no warnings about /etc/sysconfig/network. The main problem
is that we read this file for every ifcfg file we parse, and we would
need to ratelimit the number of warnings. Another problem is that
the file likely contains keys that we intentionally don't support.
We would need a new way to omit warnings about those lines.
Example:
TYPE=Ethernet
PROXY_METHOD=none
BROWSER_ONLY=no
BOOTPROTO=dhcp
DEFROUTE=yes
STABLE_ID=$'xxx\xF4yy'
IPV4_FAILURE_FATAL=no
IPV6INIT=yes
XX=foo
XX1=foo'
'
IPV6_AUTOCONF=yes xxxx
IPV6_DEFROUTE=yesx
IPV6_DEFROUTE=yes
IPV6_FAILURE_FATAL=no
IPV6_ADDR_GEN_MODE=stable-privacy
NAME=xxx
UUID=9d8ed7ff-3cdd-4336-9e26-3e978dc87102
ONBOOT=no
<warn> [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:6: key STABLE_ID does not contain valid UTF-8 and is treated as ""
<debug> [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:9: key XX is unknown and ignored
<warn> [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:10: key XX1 is badly quoted and is treated as ""
<warn> [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:11: invalid line ignored
<warn> [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:12: key IPV6_AUTOCONF is badly quoted and is treated as ""
<warn> [...] ifcfg-rh: ifcfg,/etc/sysconfig/network-scripts/ifcfg-xxx:13: key IPV6_DEFROUTE is duplicated and the early occurrence ignored
https://bugzilla.redhat.com/show_bug.cgi?id=1959656
(cherry picked from commit 41be0c8fde)
ifcfg files are a text format. It makes no sense to ever accept
non-UTF-8 blobs. If binary data is to be encoded in a ifcfg file, then
the upper layers must escape/encode it in valid UTF-8.
Let svUnescape() silently reject any binary "text". This will lead to treat such
strings as empty strings "". This is no different than some invalid
quoting: the string is not parsable as (UTF-8) text and will be treated
as such.
This is potentially a breaking change. But the benefit is that all the
upper layers can rely on only getting valid UTF-8 strings. For example,
a non-UTF-8 string cannot be converted to a "s" GVariant (of course not,
it's not a string). But our nm_connection_verify() commonly does not
check that all strings are in fact valid UTF-8. So a user who edits
an ifcfg file could inject non-valid strings, and cause assertion
failures later on.
It's actually easy to provoke a crash (or at least an assertion failure)
by writing an ifcfg file with certain keys as binary.
Note that you can either reproduce the binary files by writing non-UTF-8
"strings" dirctly, or by using \x, \u, or \U escape sequences.
Note that also '\0' gets rejected and renders the string as invalid
(i.e. as empty). Before the returned string would have been simply
truncated and the rest ignored. Such NUL bytes can only be produced
using the escape sequences, because the ifcfg reader already (silently)
truncates the file on the first binary NUL.
(cherry picked from commit 7c9b0d68e4)
Note that previously the check
if (s[slen] < ' ') {
...
return (*to_free = _escape_ansic(s));
}
would be TRUE for all UTF-8 characters if `char` is signed. That means,
depending on the compiler, we would always ANSI escape all UTF-8
characters. With this patch, we no longer do that!
Instead, valid unicode gets now preserved (albeit quoted).
On the other hand, always ANSIC escape invalid UTF-8 (regardless of the
compiler). ifcfg-rh is really a text based format. If a caller wants to store
binary data, they need to escape it first, for example with some own escaping
scheme, base64 or bin2hexstr.
A caller passing a non-text to svEscape() is likely a bug already and
they should have not done that.
Still, let svEscape() handle that by using ANSIC escaping. That works
as far as escaping is concerned, but likely later will be a problem
during unescaping, when the reader expects a valid UTF-8 string.
svEscape() is in no place to signal a sensible error, so proceed the
best it can, by escaping.
(cherry picked from commit 5877928b46)
src/libnm-glib-aux/nm-random-utils.c:112:12: error: ignoring return value of 'getrandom' declared with attribute 'warn_unused_result' [-Werror=unused-result]
Fixes: 18597e33cb ('glib-aux: also use getrandom() for seeding pseudo random generator')
(cherry picked from commit 11d59f81cf)
During nm_device_unrealize(), we first clear the device's ifindex. Then
we call _set_state_full(NM_DEVICE_STATE_UNMANAGED).
NMVpnConnection are subclasses of NMActiveConnection, it is that way
connected to NM_DEVICE_STATE_CHANGED signal. And this leads to a call
to _set_vpn_state(), which then calls nm_device_replace_vpn6_config()
to unregister the config. Thereby an assertion fails because the
ifindex no longer matches.
Fix that by relaxing the assertion. Also, don't apply the IP
configuration in unexpected device states.
https://bugzilla.redhat.com/show_bug.cgi?id=1912423https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/927
(cherry picked from commit 63a248ecb2)
NMDeviceModem has priv->modem set from construction to dispose, and
the NM_MODEM_IP4_CONFIG_RESULT/NM_MODEM_IP6_CONFIG_RESULT signals
connected all the time.
On the other hand, NMModem may hook up to NMPPPManager's
NM_PPP_MANAGER_SIGNAL_IP{4,6}_CONFIG signals, which emit the
config-results signals. And PPP manager emits the config signals
from impl_ppp_manager_set_ip{4,6}_config().
That means, at any moment can be a D-Bus calls, which leads to emitting
those signals and calling into modem_ip4_config_result() and
modem_ip6_config_result().
At least, it's not clear from review what would prevent that from
happening. If you cannot easily verify that certain conditions are
satisfied, then this is not the place to assert, but to handle the case
as something that can happen regularly.
Handle signals in the unexpected state by ignoring them.
https://bugzilla.redhat.com/show_bug.cgi?id=1916192https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/926
(cherry picked from commit fa786e1f46)
- the return value of getrandom() is ssize_t.
- handle EAGAIN to indicate low entropy.
- treat a return value of zero the same as any other
low "n", by falling back to bad random bytes.
(cherry picked from commit 416f97c5a9)
We make an effort to get a better fallback case with
_bad_random_bytes().
Also make an effort to get good randomness in the first place. Even if
we compile against libc headers that don't provide getrandom(). Also,
this isn't really ugly, because for a long time glibc was reluctant to
add getrandom() wrapper and using syscall() was the way to go.
(cherry picked from commit 05a6936bef)
nm_utils_random_bytes() tries to get good randomness. If it fails, we still
try our own approach, but also signal that the returned numbers are bad.
In practice, none of the callers cares about the return value, because they
wouldn't know what to do in case of bad randomness (abort() is not an
option and retry is not expected to help and sending an email to the
admin isn't gonna help either). So the fallback case really should try
its best.
The fallback case depends on a good random seed and a good pseudorandom
number generator.
Getting a good seed is in reality impossible, after kernel let us down.
That is part of the problem, but we try our best.
The other part is to use a cryptographic pseudorandom number generator.
GRand uses a Mersenne Twister, so that is not good enough. In this low
level code we also cannot call gnutls/nss, because usually we don't have
that dependency. Maybe we could copy&paste the chacha20 implementation,
it's simple enough and a compatible license. That might be good, but
instead cock our own by adding some sha256 into the mix. This is
fallback code after all, and we want to try hard, but not *that* hard to
add chacha20 to NetworkManager.
So, what we do is to use a well seeded GRand instance, and XOR that
output with a sha256 digest of the state. It's probably slow, but
performance is not the issue in this code path.
(cherry picked from commit c22c3ce9f9)
