In two similar ``if () {} else if () {} else if () {} else {}`` sequences
the latter two {} blocks were unreachable. In the
identity/anonymous-identity case, anonymous-identity is optional,
wpa_supplicant will fall back to identity, so only check that (a likely
privacy issue because no NM or wpa_s documentation explains that the
"secure" identity is also sent in plaintext if anonymous_identity is
missing.)
In the phase2_auth/phase2_autheap case change the message to make it
clear that exactly one of the properties is expected to be present.
Drop the empty string checks because those cases is validated later in
verify() anyway.
However, don't also use the NM_DEPRECATED_IN_1_32 macro, because that
causes annoying compiler warnings.
There is no replacement for the function in libnm, nor is it planned
to add one. So users may still call it, but they are now warned by
documentation that it may not be a good idea.
lgtm.com warns about these uses. They are correct though. Maybe the code should
not use alloca() simply to suppress the warning. Instead, add a comment pointing
out that this is in fact correct.
Setting pause-rx/pause-tx to an explicit value, implies that the user
does not want to enable autoneg. Reject that as invalid value in the
connection profile.
Introducing ethtool PAUSE support with:
* ethtool.pause-autoneg on/off
* ethtool.pause-rx on/off
* ethtool.pause-tx on/off
Limitations:
* When `ethtool.pause-autoneg` is set to true, the `ethtool.pause-rx`
and `ethtool.pause-tx` will be ignored. We don't have warning for
this yet.
Unit test case included.
Signed-off-by: Gris Ge <fge@redhat.com>
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/829
Found by Coverity:
Error: RESOURCE_LEAK (CWE-772):
NetworkManager-1.31.3/src/libnm-core-impl/nm-utils.c:2772: alloc_fn: Storage is returned from allocation function "nm_utils_tc_action_from_str".
NetworkManager-1.31.3/src/libnm-core-impl/nm-utils.c:2772: var_assign: Assigning: "action" = storage returned from "nm_utils_tc_action_from_str(extra_opts, error)".
NetworkManager-1.31.3/src/libnm-core-impl/nm-utils.c:2785: leaked_storage: Variable "action" going out of scope leaks the storage it points to.
# 2783| tfilter = nm_tc_tfilter_new(kind, parent, error);
# 2784| if (!tfilter)
# 2785|-> return NULL;
# 2786|
# 2787| nm_tc_tfilter_set_handle(tfilter, handle);
Fixes: de41c45e61 ('libnm-core: add functionality for dealing with tc-style traffic filter specifiers')
Modern WPA3 authentication methods like SAE and WPA-EAP-SUITE-B-192 need
to have management frame protection set to required according to the
standard. Since the last commit, we enforce this automatically when
key-mgmt is set to 'owe', 'sae' or 'wpa-eap-suite-b-192', so disabling
it manually should not be possible.
Add a check to the pmf property that makes sure it can't be set to
'disabled' or 'optional' when one of those key-mgmt methods is used.
The key-mgmt property of NMSettingWirelessSecurity is slightly confusing
when you know there's also a wpa_supplicant configuration option called
"key_mgmt". Our property is not the same as that supplicant option even
though they do have things in common. NMs key-mgmt is not exactly meant
to configure which AKM suites you want to use, but rather which method
of wifi security is being used (so "wpa2+wpa3 personal", "wpa3 personal
only" or "wpa3 enterprise only").
Try to make this a bit clearer in the documentation of the property by
rewriting it and listing those security methods.
For NetworkManager profiles, "connection.uuid" is the identifier of the
profile. It is supposed to be a UUID, however:
- the UUID was not ensured to be all-lower case. We should make sure
that our UUIDs are in a consistent manner, so that users can rely
on the format of the string.
- the UUID was never actually interpreted as a UUID. It only was some
opaque string, that we use as identifier. We had nm_utils_is_uuid()
which checks that the format is valid, however that did not fully
validate the format, like it would accept "----7daf444dd78741a59e1ef1b3c8b1c0e8"
and "549fac10a25f4bcc912d1ae688c2b4987daf444d" (40 hex characters).
Both invalid UUIDs and non-normalized UUID should be normalized. We
don't want to break existing profiles that use such UUIDs, thus we don't
outright reject them. Let's instead mangle them during
nm_connection_normalize().
This only uses glib's md5 code and has no dependency on our
crypto libraries (gnutls, nss).
Move it to "libnm-glib-aux" so it can be freely used.
Maybe it deserves a better name, but "nm-shared-utils.h" is a heap of
various utility functions, it's hard to name them well.
We use util-linux's libuuid for handling UUIDs. But UUIDs are
really a trivial thing, at least the portion that we use.
Reimplement the parse/unparse/generate_random() methods and drop
the dependency. Note that no other libraries from our dependency chain
was dragging in libuuid, so thereby we really get rid of the dependency.
We still require libuuid for building, because it is used by an example
program. Maybe that should be changed, to avoid the build dependency.
But that can be done at a later time.
A small bug sneaked into commit 3ef3733c81 ('wireless-security: ensure
Mesh networks can't use anything but SAE') during review: Instead of
allowing only "sae" or "none" as key-mgmt, we now disallow "sae" and
"none", but allow anything else. This is obviously not what was
intended, so fix the check.
Also move the valid_key_mgmt check back up to where it was before that
commit, it seems we want to apply that check in all cases.
This patch is introducing the wired setting accept-all-mac-addresses
property. The value corresponds to the kernel flag IFF_PROMISC.
When accept-all-mac-address is enabled, the interface will accept all
the packets without checking the destination mac address.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Review and replace usages of the two nm_connection_to_dbus() flags
marked deprecated in commit 84648e562c98 ('libnm: Refactor
NM_CONNECTION_SERIALIZE_* flags'):
NM_CONNECTION_SERIALIZE_NO_SECRETS and
NM_CONNECTION_SERIALIZE_ONLY_SECRETS.
nm-settings-connection.c has code similar to this in two places:
/* FIXME: improve NMConnection API so we can avoid the overhead of cloning the connection,
* in particular if there are no secrets to begin with. */
connection_cloned = nm_simple_connection_new_clone(new);
/* Clear out unwanted secrets */
_nm_connection_clear_secrets_by_secret_flags(connection_cloned,
NM_SETTING_SECRET_FLAG_NOT_SAVED
| NM_SETTING_SECRET_FLAG_AGENT_OWNED);
secrets = nm_g_variant_ref_sink(
nm_connection_to_dbus(connection_cloned, NM_CONNECTION_SERIALIZE_ONLY_SECRETS));
It seems the secrets filtering can be done by nm_connection_to_dbus() if
the NM_CONNECTION_SERIALIZE_* flags are extended. The current set of
flags contains flags that start with NO, ONLY and WITH prefixes, which
makes it useless for combining the flags because most combinations of
more than one flag don't have a clear interpretation. So they're mostly
useful when used alone, i.e. you'd need to add a new enum value for
each new subset of settings to be serialized.
To get the most flexibility from a small set of flags they should
either all be of the WITH_* type or NO_* type. In the former case they
could be combined to extend the subset of properties serialized, in the
latter case each flag would reduce the subset. After trying both
options I found it's easier to adapt the current set of flags to the
WITH_* schema while keeping binary and source compatibility. This
commit changes the set of flags in the following way:
NM_CONNECTION_SERIALIZE_ALL is kept for compatibility but is equivalent
to a combination of other flags.
NM_CONNECTION_SERIALIZE_WITH_NON_SECRET is added with the same value as
NM_CONNECTION_SERIALIZE_NO_SECRETS, it implies that non-secret
properties are included but doesn't prevent including other properties.
Since it couldn't be meaningfully combined with any other flag this
change shouldn't break compatibility.
Similarly NM_CONNECTION_SERIALIZE_WITH_SECRETS is added with the same
value as existing NM_CONNECTION_SERIALIZE_ONLY_SECRETS with the same
consideration about compatibility.
NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED and the new
NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED and
NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED add only subsets of
secrets and can be combined. For backwards compatibility
NM_CONNECTION_SERIALIZE_ONLY_SECRETS is basically ignored when either of
these three is present, so that the value:
..ONLY_SECRETS | ..AGENT_OWNED works as previously.
initscripts don't support "$VLAN_ID". They actually support "$VID",
which NetworkManager doesn't.
"$VLAN_ID" was introduced by commit 10b32be37b ('ifcfg-rh: various VLAN
cleanups'). It has a comment about "backward compatibility" for the case
where the reader would ignore "$VLAN_ID" if "$DEVICE"'s name contains
a suffix that is parsable as VLAN ID.
That is wrong. If a new feature gets introduce (like NetworkManager
supporting "$VLAN_ID"), then there is no way that an older version of the
tool -- which doesn't know the new feature yet (initscripts) -- supports it.
This is not what backward compatibility means. Backward compatibility
means that if a user has an old ifcfg-file without "$VLAN_ID", then we
continue parsing it as before.
Consider, when a user (or NetworkManager) writes a configuration
DEVICE=vlan9
PHYSDEV=eth0
VLAN_ID=10
then it makes no sense to ignore VLAN_ID=10 and use "9" instead.
Otherwise the user (or NetworkManager) should not have written the
file this way.
Also, NetworkManager profiles support "connection.interface-name=vlan9"
together with "vlan.id=10". Such a configuration is valid and must be
expressible in ifcfg-rh format. The ifcfg-rh writer code did not somehow
restrict the setting of "$VLAN_ID" to account for this odd behavior. Whenever
NetworkManager in the past wrote VLAN_ID variable to file, it really meant
it.
https://bugzilla.redhat.com/show_bug.cgi?id=1907960https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/794
