The in_buffer is initialized with a NULL buffer. If we never receive and
data, the buffer might still be NULL.
Maybe it actually can never happen, but it's not clear that this is
always the case. To be sure, ensure we don't return a NULL buffer on
success.
Setting
wifi.cloned-mac-address="stable-ssid"
should generate the same SSID as
connection.stable-id="${NETWORK_SSID}"
wifi.cloned-mac-address="stable"
For that to work correctly, we need to post-process the generated stable
id.
Fixes: d210923c0f ('wifi: add "wifi.cloned-mac-address=stable-ssid"')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1813
Add a new "stable-ssid" mode that generates the MAC address based on the
Wi-Fi's SSID.
Note that this gives the same MAC address as setting
connection.stable-id="${NETWORK_SSID}"
wifi.cloned-mac-address="stable"
The difference is that changing the stable ID of a profile also affects
"ipv6.addr-gen-mode=stable-privacy" and other settings.
For Wi-Fi profiles, this will encode the SSID in the stable-id.
For other profiles, this encodes the connection UUID (but the SSID and
the UUID will always result in distinct stable IDs).
Also escape the SSID, so that the generated stable-id is always valid
UTF-8.
Comparing integers of different signedness gives often unexpected
results. Adjust usages of MIN()/MAX() to ensure that the arguments agree
in signedness.
nm_hash_siphash42() uses a randomized seed like nm_hash*(). In this case,
we want to always generate the same fake timestamp, based on the host-id.
In practice, it doesn't really matter, because this is only the fallback
path for something gone horribly wrong already.
Parse the access point announced bandwidth in MHz. This is considering
both HT and VHT. Please notice that for VHT 80+80 MHz we are representing it
as 160 MHz.
Struct allow named arguments, which seems easier to maintain instead of
a function with many arguments. Also, adding a new parameter does not
require changes to most of the callers.
The real advantage of this is that we encode all the search parameters
in one argument. And we can add that argument to
_match_section_infos_lookup(), alongside lookup by NMDevice or
NMPlatformLink.
All callers eventually want a boolean instead of a NMMatchSpecMatchType.
I think the NMMatchSpecMatchType enum still has value at the lower
layers, where the enum values are clearer (when reading the code). So
don't drop NMMatchSpecMatchType entirely.
However, let's add nm_match_spec_match_type_to_bool() to convert the
match-type to a boolean to avoid duplicating the code.
The "connection.stable-id" supports placeholders like "${CONNECTION}" or
"${DEVICE}".
The stable-id can also be specified in global connection defaults in
NetworkManager.conf, by leaving it unset in the profile. Global
connection defaults always follow the pattern, that they correspond to a
per-profile property, and only when the per-profile value indicates a
special default/unset value, the global connection default is consulted.
Finally, if the global connection default is also not configured in
NetworkManager.conf, a built-in default is used (which may not be
constant either, for example ipv6.ip6-privacy's built-in default depends
on a sysctl value).
In any case, every possible configuration that can be achieved should be
configurable both per-profile and via global connection default. That
was not given for the stable-id, because the built-in default generated
an ID in a way that could not be explicitly expressed otherwise.
So you could not:
- explicitly set the per-profile value to the built-in default, to avoid
that the global-connection-default overwrites it.
- explicitly set the global-connection-default to the built-in default,
to avoid that a lower priority [connection*] section overwrites the
stable-id again.
Fix that inconsistency to make it possible to explicitly set the
built-in default.
Change behavior for literally "default${CONNECTION}" and make it behave
as the built-in default. Also document that the built-in default has that
value.
It's unlikely that this breaks an existing configuration, but of course,
if any user configured "connection.stable-id=default${CONNECTION}", then
the behavior changes for them.
We did not initialize "child_stderr". If that were necessary, we would need
to add it too. However, it is clearly not necessary to initialize those fields.
It is wrong trying to send the signal still. Just error out.
Note that ECHILD indicates that the process is either not a child
or was already reaped. In both cases, that is a bug of the caller
who must keep accurate track of the child's process ID.
It's easy enough to know how many bytes are needed. Just allocate the
right size (+1, because NMStrBuf really likes to reserve that extra byte
for the trailing NUL, even if it's not needed in this case).
Currently the only way to return an error code from the daemon helper
is via the process exit code, but that is not enough to fully describe
an error from getaddrinfo(); in fact, the function returns a EAI_*
error code and when the value is EAI_SYSTEM, the error code is
returned in errno.
At the moment, any messages printed to stderr by the helper goes to NM
stderr; instead, we want to capture it and pass it through the logging
mechanism of NM, so that it can be filtered according to level and
domain.
g_random_*() is based on GRand, which is not a CSPRNG. Instead, rely on
kernel to give us good random numbers, which is what nm_random_*() does.
Note that nm_random_*() calls getrandom() (or reads /dev/urandom), which
most likely is slower than GRand. It doesn't matter for our uses though.
It is cumbersome to review all uses of g_rand_*() whether their usage of
a non-cryptographically secure generator is appropriate. Instead, just
always use an appropriate function, thereby avoiding this question. Even
glib documentation refers to reading "/dev/urandom" as alternative. Which
is what nm_random_*() does. These days, it seems unnecessary to not use
the best random generator available, unless it's not fast enough or you
need a stable/seedable stream of random numbers.
In particular in nmcli, we used g_random_int_range() to generate
passwords. That is not appropriate. Sure, it's *only* for the hotspot,
but still.
nm_utils_get_ipv6_interface_identifier() has non-obvious requirements on
the hardware address. If the caller passes a wrong length, it will
trigger an assertion or even cause out of bound read. This would mean
that the caller needs to carefully check the length. Such requirements
on the caller are wrong.
Also, in practice the hardware length comes from platform/kernel. We
don't want to trust that what kernel tells us always has the required
address length, so the caller would always have to double check before
calling the function.
Instead, handle unexpected address lengths.
Fixes: e2270040c0 ('core: use Interface Identifiers for IPv6 SLAAC addresses')
Fixes: 1d396e9972 ('core-utils: use 64-bit WPAN address for a 6LoWPAN IID')
This is the version shipped in Fedora 37. As Fedora 37 is now out, the
core developers switch to it. Our gitlab-ci will also use that as base
image for the check-{patch.tree} tests and to generate the pages. There
is a need that everybody agrees on which clang-format version to use,
and that version should be the one of the currently used Fedora release.
Also update the used Fedora image in "contrib/scripts/nm-code-format-container.sh"
script.
The gitlab-ci still needs update in the following commit. The change
in isolation will break the "check-tree" test.
Clang 15 now (correctly) warns about this:
../src/libnm-core-impl/nm-vpn-plugin-info.c:201:40: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
_nm_vpn_plugin_info_get_default_dir_etc()
^
void
../src/libnm-core-impl/nm-vpn-plugin-info.c:213:40: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
_nm_vpn_plugin_info_get_default_dir_lib()
^
void
../src/libnm-core-impl/nm-vpn-plugin-info.c:226:41: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
_nm_vpn_plugin_info_get_default_dir_user()
^
void
../src/libnm-core-impl/nm-vpn-plugin-info.c:315:29: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
nm_vpn_plugin_info_list_load()
^
void
These variants provide additional nm_assert() checks, and are thus
preferable.
Note that we cannot just blindly replace &g_array_index() with
&nm_g_array_index(), because the latter would not allow getting a
pointer at index [arr->len]. That might be a valid (though uncommon)
usecase. The correct replacement of &g_array_index() is thus
nm_g_array_index_p().
I checked the code manually and replaced uses of nm_g_array_index_p()
with &nm_g_array_index(), if that was a safe thing to do. The latter
seems preferable, because it is familar to &g_array_index().
Add nm_g_array_index() as a replacement for g_array_index(). The value
of nm_g_array_index(), nm_g_array_index_p(), nm_g_array_first() and
nm_g_array_last() is that they add nm_assert() checks for valid
parameters.
nm_g_array_{first,last}() now returns an lvalue and not a pointer.
As such, they are just shorthands for nm_g_array_index() at index
0 and len-1, respectively.
`nm_g_array_index_p(arr, Type, idx)` is almost the same as
`&nm_g_array_index(arr, Type, idx)`. The only difference (and why the
former variant exists), is that nm_g_array_index_p() allows to get a
pointer one after the end.
This means, this is correct and valid to do:
// arr->len might be zero
arr = nm_g_array_index_p(arr, Type, 0);
for (i = 0; i < arr->len; i++, arr++)
...
ptr = nm_g_array_index_p(arr, Type, 0);
end = nm_g_array_index_p(arr, Type, arr->len);
for (; ptr < end; ptr++)
...
This would not be valid to do with nm_g_array_{index,first,last}().
Also fix supporting "const GArray *arr" parameter. Of course, the function
casts the constness away. Technically, that matches the fact that arr->data
is also not a const pointer. In practice, we might want to propagate the
constness of the container to the constness of the element lookup. While
doable, that is not implemented.
- name things related to `in_addr_t`, `struct in6_addr`, `NMIPAddr` as
`nm_ip4_addr_*()`, `nm_ip6_addr_*()`, `nm_ip_addr_*()`, respectively.
- we have a wrapper `nm_inet_ntop()` for `inet_ntop()`. This name
of our wrapper is chosen to be familiar with the libc underlying
function. With this, also name functions that are about string
representations of addresses `nm_inet_*()`, `nm_inet4_*()`,
`nm_inet6_*()`. For example, `nm_inet_parse_str()`,
`nm_inet_is_normalized()`.
<<<<
R() {
git grep -l "$1" | xargs sed -i "s/\<$1\>/$2/g"
}
R NM_CMP_DIRECT_IN4ADDR_SAME_PREFIX NM_CMP_DIRECT_IP4_ADDR_SAME_PREFIX
R NM_CMP_DIRECT_IN6ADDR_SAME_PREFIX NM_CMP_DIRECT_IP6_ADDR_SAME_PREFIX
R NM_UTILS_INET_ADDRSTRLEN NM_INET_ADDRSTRLEN
R _nm_utils_inet4_ntop nm_inet4_ntop
R _nm_utils_inet6_ntop nm_inet6_ntop
R _nm_utils_ip4_get_default_prefix nm_ip4_addr_get_default_prefix
R _nm_utils_ip4_get_default_prefix0 nm_ip4_addr_get_default_prefix0
R _nm_utils_ip4_netmask_to_prefix nm_ip4_addr_netmask_to_prefix
R _nm_utils_ip4_prefix_to_netmask nm_ip4_addr_netmask_from_prefix
R nm_utils_inet4_ntop_dup nm_inet4_ntop_dup
R nm_utils_inet6_ntop_dup nm_inet6_ntop_dup
R nm_utils_inet_ntop nm_inet_ntop
R nm_utils_inet_ntop_dup nm_inet_ntop_dup
R nm_utils_ip4_address_clear_host_address nm_ip4_addr_clear_host_address
R nm_utils_ip4_address_is_link_local nm_ip4_addr_is_link_local
R nm_utils_ip4_address_is_loopback nm_ip4_addr_is_loopback
R nm_utils_ip4_address_is_zeronet nm_ip4_addr_is_zeronet
R nm_utils_ip4_address_same_prefix nm_ip4_addr_same_prefix
R nm_utils_ip4_address_same_prefix_cmp nm_ip4_addr_same_prefix_cmp
R nm_utils_ip6_address_clear_host_address nm_ip6_addr_clear_host_address
R nm_utils_ip6_address_same_prefix nm_ip6_addr_same_prefix
R nm_utils_ip6_address_same_prefix_cmp nm_ip6_addr_same_prefix_cmp
R nm_utils_ip6_is_ula nm_ip6_addr_is_ula
R nm_utils_ip_address_same_prefix nm_ip_addr_same_prefix
R nm_utils_ip_address_same_prefix_cmp nm_ip_addr_same_prefix_cmp
R nm_utils_ip_is_site_local nm_ip_addr_is_site_local
R nm_utils_ipaddr_is_normalized nm_inet_is_normalized
R nm_utils_ipaddr_is_valid nm_inet_is_valid
R nm_utils_ipx_address_clear_host_address nm_ip_addr_clear_host_address
R nm_utils_parse_inaddr nm_inet_parse_str
R nm_utils_parse_inaddr_bin nm_inet_parse_bin
R nm_utils_parse_inaddr_bin_full nm_inet_parse_bin_full
R nm_utils_parse_inaddr_prefix nm_inet_parse_with_prefix_str
R nm_utils_parse_inaddr_prefix_bin nm_inet_parse_with_prefix_bin
R test_nm_utils_ip6_address_same_prefix test_nm_ip_addr_same_prefix
./contrib/scripts/nm-code-format.sh -F
_host_id_read() is the only place where we really care to have good
random numbers, because that is the secret key that we persist to disk.
Previously, we tried only nm_random_get_bytes_full(), which is a best
effort to get strong random numbers. If it fails to generate those,
it would simply remember the generated key in memory and proceed, but not
persist it to disk.
nm_random_get_bytes_full() does not block waiting for good numbers.
Change that. Now, first call nm_random_get_crypto_bytes(), which would
block and try hard to get good random numbers. Only if that fails,
fallback to nm_random_get_bytes_full() as before. The difference is of
course only in early boot, when we might not yet have entropy. In that
case, I think it's better for NetworkManager to block.
Heavily inspired by systemd ([1]).
We now also have nm_random_get_bytes{,_full}() and
nm_random_get_crypto_bytes(), like systemd's random_bytes()
and crypto_random_bytes(), respectively.
Differences:
- instead of systemd's random_bytes(), our nm_random_get_bytes_full()
also estimates whether the output is of high quality. The caller
may find that interesting. Due to that, we will first try to call
getrandom(GRND_NONBLOCK) before getrandom(GRND_INSECURE). That is
reversed from systemd's random_bytes(), because we want to find
out whether we can get good random numbers. In most cases, kernel
should have entropy already, and it makes no difference.
Otherwise, heavily rework the code. It should be easy to understand
and correct.
There is also a major bugfix here. Previously, if getrandom() failed
with ENOSYS and we fell back to /dev/urandom, we would assume that we
have high quality random numbers. That assumption is not warranted.
Now instead poll on /dev/random to find out.
[1] a268e7f402/src/basic/random-util.c (L81)
We read /proc/cmdline for "match.kernel-command-line". But next we will
also honor "nm.debug" on the kernel command line, to enable debug
logging. For "nm.debug" it makes sense that it overwrites the debug
options from the command line and from "NetworkManager.conf". That
means, if you set "nm.debug", then verbose logging will be enabled. It
can only be turned off again at runtime (via D-Bus), otherwise, it's
hard to avoid.
It still can make sense to overrule this setting once again. Support
that, by honoring a file "/run/NetworkManager/proc-cmdline" to be used
instead of "/proc/cmdline".
This option is mainly for debugging and testing, but it might be useful
in production too, if you had "nm.debug" enabled during boot, but later
want to disable it until next reboot. Then you could do:
sed 's/ *\<nm\.debug\> */ /g' /proc/cmdline > /run/NetworkManager/proc-cmdline
nmcli general logging level DEFAULT domains DEFAULT
NM_STR_BUF_INIT() and nm_str_buf_init() were pretty much redundant. Drop one of
them.
Usually our pattern is that we don't have functions that return structs.
But NM_STR_BUF_INIT() returns a struct, because it's convenient to use
with
nm_auto_str_buf NMStrBuf strbuf = NM_STR_BUF_INIT(...);
So use that variant instead.
On glibc, HOST_NAME_MAX is defined as 64. Also, Linux'
sethostname() enforces that limit (__NEW_UTS_LEN). Also,
`man gethostname` comments that HOST_NAME_MAX on Linux is
64.
However, when building against musl, HOST_NAME_MAX is defined as 255.
That seems wrong. We use this limit to validate the hostname, and that
should not depend on the libc or on the compilation.
Hardcode the value to 64.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1197
