Rework parsing of nmcli's passwd-file.
1) support backslash escaping of secrets.
- only the secret can be backslash escaped, the property and setting
name cannot.
This is a change in behavior for passwd-files with secrets that contain
a backslash.
2) strip the white space around the secret. This is a change in behavior
for secrets that had leading or trailing spaces. Note that you can
backslash escape spaces in secrets.
3) strip white space around the setting.property key. This is also a
change in behavior, but such keys would never have been valid
previously (or the caller would have performed the same kind of
stripping).
4) accept '=' as alternative delimiter beside ':'. The ':' feels really
odd and unexpected. Also accept '='. This is a change in behavior if
keys would contain '=', which they really shouldn't.
5) reject non-UTF-8 secrets and keys. For keys, that is not an issue,
because such keys were never valid. For secrets, it probably didn't
work anyway to specify non-UTF-8 secrets, because most (if not all)
secrets are transmitted via D-Bus as strings where arbitrary binary
is not allowed.
6) ignore empty lines and lines starting with '#'.
7) ensure we don't leak any secrets in memory.
1) to 4) are changes in behavior. 3) and 4) seem less severe, as they
only concern unusual setting.property keys, which really shouldn't be
used (although, VPN secrets can have almost arbitrary names *sigh*).
1) and 2) is more dangerous, as it changes behavior for secrets that
contain backslashes or leading/trailing white space.
When parsing user input if is often convenient to allow stripping whitespace.
Especially with escaped strings, the user could still escape the whitespace,
if the space should be taken literally.
Add support for that to nm_utils_buf_utf8safe_unescape().
Note that this is not the same as calling g_strstrip() before/after
unescape. That is, because nm_utils_buf_utf8safe_unescape() correctly
preserves escaped whitespace. If you call g_strstrip() before/after
the unescape, you don't know whether the whitespace is escaped.
We want to use the function to unescape (compress) secrets. As such, we want
to be sure that no secrets are leaked in memory due to growing the buffer with
realloc. In fact, reallocation should never happen. Assert for that.
As reallocation cannot happen, we could directly fill a buffer with
API like nm_utils_strbuf_*(). But NMStrBuf has low overhead even in this
case.
nm-device now applies ethtool coalesce settings during stage 2 "device
config" of the connection activation.
Coalesce settings will be then restored (according to what the state
was before the connection got activated on the device) when the
connection is deactivated during the device cleanup.
One thing to be noted is that unset coalesce settings (in the profile)
will not be touched at all by NetworkManager so that if the NIC driver
sets some default values these will be preserved unless specifically
overridden by the connection profile.
https://bugzilla.redhat.com/show_bug.cgi?id=1614700
Callbacks might reference the main loop when destroying the NMClient
instance. Unref the main loop later.
# G_DEBUG=fatal-warnings valgrind --num-callers=100 nmcli device wifi connect home
^C
Error: nmcli terminated by signal Interrupt (2)
Error: Connection activation failed: (0) No reason given.
==11050== Invalid read of size 4
==11050== at 0x4C90D3D: g_main_loop_quit (in /usr/lib64/libglib-2.0.so.0.6200.6)
==11050== by 0x431435: quit (devices.c:934)
==11050== by 0x43272C: connected_state_cb (devices.c:1919)
==11050== by 0x4BF6741: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4C0A603: ??? (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4C133AD: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4C139D2: g_signal_emit (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4BFB1C3: ??? (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4BFAAEC: ??? (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x4BFD86A: g_object_thaw_notify (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x48BA040: _nm_client_notify_event_emit (nm-client.c:937)
==11050== by 0x48CA01F: _dbus_handle_changes_commit (nm-client.c:2850)
==11050== by 0x48CC221: _dbus_handle_changes (nm-client.c:2864)
==11050== by 0x48CC833: _init_release_all (nm-client.c:6969)
==11050== by 0x48D2818: dispose (nm-client.c:7826)
==11050== by 0x4BFBC27: g_object_unref (in /usr/lib64/libgobject-2.0.so.0.6200.6)
==11050== by 0x43FF93: nmc_cleanup (nmcli.c:941)
==11050== by 0x4410AD: main (nmcli.c:1005)
==11050== Address 0x54738fc is 12 bytes inside a block of size 16 free'd
==11050== at 0x4839A0C: free (vg_replace_malloc.c:540)
==11050== by 0x4C9649C: g_free (in /usr/lib64/libglib-2.0.so.0.6200.6)
==11050== by 0x4410A3: main (nmcli.c:1004)
==11050== Block was alloc'd at
==11050== at 0x483AB1A: calloc (vg_replace_malloc.c:762)
==11050== by 0x4C96400: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6200.6)
==11050== by 0x4C90A45: g_main_loop_new (in /usr/lib64/libglib-2.0.so.0.6200.6)
==11050== by 0x441020: main (nmcli.c:987)
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/501
NM_SET_OUT() macro already has an "if" condition to only do
anything if the output pointer is not NULL.
As such, we don't need to check first. In practice, this only reorders
the checks, which the compiler may anyway do.
Note that above the checks are still relevant, because we want to
avoid the more expensive parsing, if we don't care about the result.
Often it is useful to not accept %NULL as input argument, to catch bugs.
For simple functions like nm_ethtool_id_get_by_name(), such limitations
are more annoying than helpful. Gracefully accept %NULL and treat is
like an invalid ethtool option.
_ASSERT_data() checks static, immutable data. Even with more asserts enabled,
there is no need to do that every time. Use NM_MORE_ASSERT_ONCE().
Note that NM_MORE_ASSERT_ONCE() will return constant FALSE, when build
without a sufficiently high assertion level. That means, the compiler
will just optimize the rest away.
When doing a release, we should care about the checksum of the tarball.
Log all of them... also, because fedpkg uses sha512, ftpadmin@gnome uses
sha256, etc.
We have nm_str_not_empty() which is the inverse of that. The purpose
of nm_str_not_empty() is to normalize a string to either return
%NULL or a non-empty string, like
const char *
get_name (Object *obj)
{
return nm_str_not_empty (obj->name);
}
Sometimes, we however want to check whether a string is not empty.
So, we previously had two choices:
1) use a temporary variable:
const char *tmp;
tmp = get_string ();
if (tmp && tmp[0])
...
The problem with this variant is that it's more verbose (by requiring a
temporary variable). Another downside is that there are multiple ways
how to check for an empty string (!tmp[0], tmp[0] == '\0', !strlen (tmp),
strlen (tmp) == 0), and sure enough they are all in use.
2) use !nm_str_not_empty(). But this double negation looks really odd
and confusing.
Add nm_str_is_empty() instead.
