GitLab

Fixes testing on 32-bit arches:

/core/general/test_setting_compare_timestamp:
(./test-general:29331): GLib-GObject-WARNING **: g_object_set_valist: object class `NMSettingConnection' has no property named `$?\xff\xff\x89t$0\x89|$4\xe8\u001c\x98\xff\xff\x85\xc0tM\x83\xf8\xfft3\x8dT$(\xc7D$\u0008'
/bin/sh: line 5: 29331 Trace/breakpoint trap   ${dir}$tst
FAIL: test-general

Fixes: 093a3c88

for ignoring timestamp when comparing settings or connections.

Although libnm filters out properties received from the daemon that it
doesn't understand, there may be other clients that do not. In
particular, a client might call GetSettings() on a connection, update
the ipv4.addresses property in the returned dictionary, and then pass
the dictionary to Update(). In that case, the updated dictionary would
contain ipv4.address-data, but it would not reflect the changes the
client intended to make.

Fix this by changing the daemon side to prefer the legacy properties
to the new ones if both are set, and changing the client side to not
send the legacy properties (since we don't support new clients talking
to old servers anyway).

config.h should be included from every .c file, and it should be
included before any other include. Fix that.

(As a side effect of how I did this, this also changes us to
consistently use "config.h" rather than <config.h>. To the extent that
it matters [which is not much], quotes are more correct anyway, since
we're talking about a file in our own build tree, not a system
include.)

Make the type return GBytes since most in-tree users want that.

Allow the function to accept many more formats as valid hex, including
bytes delimited by ':' and a leading '0x'.

https://bugzilla.gnome.org/show_bug.cgi?id=735512
https://bugzilla.redhat.com/show_bug.cgi?id=663730Signed-off-by: Thomas Haller <thaller@redhat.com>

If no address in an NMSettingIP4Config has a label, then don't bother
serializing an array of empty strings.

Add AddressData and RouteData properties to NMSettingIPConfig and
NMIP[46]Config. These are like the existing "addresses" and "routes"
properties, but using strings and containing additional attributes,
like NMIPAddress and NMIPRoute.

This only affects the D-Bus representations; there are no API changes
to NMSettingIP{,4,6}Config or NMIP{4,6}Config as a result of this; the
additional information is just added to the existing 'addresses' and
'routes' properties.

NMSettingIP4Config and NMSettingIP6Config now always generate both
old-style data ('addresses', 'address-labels', 'routes') and new-style
data ('address-data', 'gateway', 'route-data') when serializing to
D-Bus, for backward compatibility. When deserializing, they will fill
in the 'addresses' and 'routes' properties from the new-style data if
it is present (ignoring the old-style data), or from the old-style
data if the new-style isn't present.

The daemon-side NMIP4Config and NMIP6Config always emit changes for
both 'Addresses'/'Routes' and 'AddressData'/'RouteData'. The
libnm-side classes initially listen for changes on both properties,
but start ignoring the 'Addresses' and 'Routes' properties once they
know the daemon is also providing 'AddressData' and 'RouteData'.

The gateway is a global property of the IPv4/IPv6 configuration, not
an attribute of any particular address. So represent it as such in the
API; remove the gateway from NMIPAddress, and add it to
NMSettingIPConfig.

Behind the scenes, the gateway is still serialized along with the
first address in NMSettingIPConfig:addresses, and is deserialized from
that if the settings dictionary doesn't contain a 'gateway' key.

Adjust nmcli's interactive mode to prompt for IP addresses and gateway
separately. (Patch partly from Jirka Klimeš.)

Split a base NMSettingIPConfig class out of NMSettingIP4Config and
NMSettingIP6Config, and update things accordingly.

Further simplifications of now-redundant IPv4-vs-IPv6 code are
possible, and should happen in the future.

Add key-value attributes to NMIPAddress and NMIPRoute, and use them to
store IPv4 address labels. Demote NMSettingIP4Config:address-labels to
a D-Bus-only property, and arrange for :addresses setter to read the
labels out of that property when creating the addresses.

Merge NMIP4Address and NMIP6Address into NMIPAddress, and NMIP4Route
and NMIP6Route into NMIPRoute. The new types represent IP addresses as
strings, rather than in binary, and so are address-family agnostic.

We disabled -Wstrict-prototypes in commit
db9b1df0 .
Fix compiler warnings.
Signed-off-by: Thomas Haller <thaller@redhat.com>

nm_setting_verify() took a GSList of other NMSettings, but really it
would just be simpler all around to pass the NMConnection instead...

This means that several formerly NMSetting-branded functions that
operated on lists-of-settings now get replaced with
NMConnection-branded functions instead.

Add nm-core-types.h, typedefing all of the GObject types in
libnm-core; this is needed so that nm-setting.h can reference
NMConnection in addition to nm-connection.h referencing NMSetting.

Removing the cross-includes from the various headers causes lots of
fallout elsewhere. (In particular, nm-utils.h used to include
nm-connection.h, which included every setting header, so any file that
included nm-utils.h automatically got most of the rest of libnm-core
without needing to pay attention to specifics.) Fix this up by
including nm-core-internal.h from those files that are now missing
includes.

Convert hardware addresses to canonical form (uppercase, leading
zeros, colons) when setting them on/adding them to NMSetting
properties.

Add a helper function for converting hardware addresses to canonical
form.

Each setting type was defining its own error type, but most of them
had exactly the same three errors ("unknown", "missing property", and
"invalid property"), and none of the other values was of much use
programmatically anyway.

So, this commit merges NMSettingError, NMSettingAdslError, etc, all
into NMConnectionError. (The reason for merging into NMConnectionError
rather than NMSettingError is that we also already have
"NMSettingsError", for errors related to the settings service, so
"NMConnectionError" is a less-confusable name for settings/connection
errors than "NMSettingError".)

Also, make sure that all of the affected error messages are localized,
and (where appropriate) prefix them with the relevant property name.

Renamed error codes:

NM_SETTING_ERROR_PROPERTY_NOT_FOUND -> NM_CONNECTION_ERROR_PROPERTY_NOT_FOUND
NM_SETTING_ERROR_PROPERTY_NOT_SECRET -> NM_CONNECTION_ERROR_PROPERTY_NOT_SECRET

Remapped error codes:

NM_SETTING_*_ERROR_MISSING_PROPERTY -> NM_CONNECTION_ERROR_MISSING_PROPERTY
NM_SETTING_*_ERROR_INVALID_PROPERTY -> NM_CONNECTION_ERROR_INVALID_PROPERTY
NM_SETTING_ERROR_PROPERTY_TYPE_MISMATCH -> NM_CONNECTION_ERROR_INVALID_PROPERTY
NM_SETTING_BLUETOOTH_ERROR_TYPE_SETTING_NOT_FOUND -> NM_CONNECTION_ERROR_INVALID_SETTING
NM_SETTING_BOND_ERROR_INVALID_OPTION -> NM_CONNECTION_ERROR_INVALID_PROPERTY
NM_SETTING_BOND_ERROR_MISSING_OPTION -> NM_CONNECTION_ERROR_MISSING_PROPERTY
NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND -> NM_CONNECTION_ERROR_MISSING_SETTING
NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND -> NM_CONNECTION_ERROR_MISSING_SETTING
NM_SETTING_IP4_CONFIG_ERROR_NOT_ALLOWED_FOR_METHOD -> NM_CONNECTION_ERROR_INVALID_PROPERTY
NM_SETTING_IP6_CONFIG_ERROR_NOT_ALLOWED_FOR_METHOD -> NM_CONNECTION_ERROR_INVALID_PROPERTY
NM_SETTING_VLAN_ERROR_INVALID_PARENT -> NM_CONNECTION_ERROR_INVALID_PROPERTY
NM_SETTING_WIRELESS_SECURITY_ERROR_MISSING_802_1X_SETTING -> NM_CONNECTION_ERROR_MISSING_SETTING
NM_SETTING_WIRELESS_SECURITY_ERROR_LEAP_REQUIRES_802_1X -> NM_CONNECTION_ERROR_INVALID_PROPERTY
NM_SETTING_WIRELESS_SECURITY_ERROR_LEAP_REQUIRES_USERNAME -> NM_CONNECTION_ERROR_MISSING_PROPERTY
NM_SETTING_WIRELESS_SECURITY_ERROR_SHARED_KEY_REQUIRES_WEP -> NM_CONNECTION_ERROR_INVALID_PROPERTY
NM_SETTING_WIRELESS_ERROR_CHANNEL_REQUIRES_BAND -> NM_CONNECTION_ERROR_MISSING_PROPERTY

Dropped error codes (were previously defined but unused):

NM_SETTING_CDMA_ERROR_MISSING_SERIAL_SETTING
NM_SETTING_CONNECTION_ERROR_IP_CONFIG_NOT_ALLOWED
NM_SETTING_GSM_ERROR_MISSING_SERIAL_SETTING
NM_SETTING_PPP_ERROR_REQUIRE_MPPE_NOT_ALLOWED
NM_SETTING_PPPOE_ERROR_MISSING_PPP_SETTING
NM_SETTING_SERIAL_ERROR_MISSING_PPP_SETTING
NM_SETTING_WIRELESS_ERROR_MISSING_SECURITY_SETTING

Rename NM_CONNECTION_ERROR_UNKNOWN to NM_CONNECTION_ERROR_FAILED,
following GError best practices.

Replace NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND ("no
NMSettingConnection") with a more generic
NM_CONNECTION_ERROR_MISSING_SETTING. Use that new code in a few places
that had previously been using NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
which was supposed to mean "the setting that you asked about doesn't
exist", not "the connection is invalid because it's missing a required
setting".

Clarify that NM_CONNECTION_ERROR_INVALID_SETTING can be used for any
"invalid or inappropriate NMSetting", not just a "conflicting" one.
(But fix a case in nm_connection_update_secrets() that was returning
INVALID_SETTING when it should have been return-if-failing instead.)

For both MISSING_SETTING and INVALID_SETTING, always prefix the error
message with "setting-name: ", just like we do with the various
NMSetting MISSING_PROPERTY and INVALID_PROPERTY errors. And make sure
that the error message is marked for localization.

Drop NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID, which is pretty
pointless; it was only used in the case where connection.type was the
name of a valid setting type that is not a base setting type. Instead,
just return NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY for
connection.type in this case (which is what the code already did when
connection.type was completely unrecognized).

The autoconnect priority has only any relevance, if the connection
is autoconnect too.

The priority defaults to zero, with higher numbers meaning preferred.
The valid range is limited to [-999,999].
Signed-off-by: Thomas Haller <thaller@redhat.com>

NMSettingSerial:parity was defined as a char-typed property that could
have the (case-sensitive!) values 'n', 'E', or 'o'. This is zany. Add
an NMSettingSerialParity enum, and use that instead.

Make enum- and flags-valued properties use GParamSpecEnum and
GParamSpecFlags, for better introspectability/bindability.

This requires no changes outside libnm-core/libnm since the expected
data size is still the same with g_object_get()/g_object_set(), and
GLib will internally convert between int/uint and enum/flags GValues
when using g_object_get_property()/g_object_set_property().

libnm-core: fix crash in update-secrets when trying to clear all settings without providing a connection variant

When the caller wants to clear all settings (thus providing
@setting_name NULL), a NM_VARIANT_TYPE_CONNECTION variant is
expected. This would lead to a crash when constructing the
@Error literal due to uninitialized @key.

Clang also warns:

    Making all in .
    make[4]: Entering directory `./NetworkManager/libnm-core'
      CC       nm-connection.lo
    ../libnm-core/nm-connection.c:1016:25: error: variable 'key' is uninitialized when used here [-Werror,-Wuninitialized]
                                                 key);
                                                 ^~~
    ../libnm-core/nm-connection.c:962:17: note: initialize the variable 'key' to silence this warning
            const char *key;
                           ^
                            = NULL
    1 error generated.

Fixes: acf86f68Signed-off-by: Thomas Haller <thaller@redhat.com>

Use nmtst_variant_editor() to rewrite the libnm-core tests to work
entirely in connection dictionaries, not connection hash tables.

In preparation for porting to GDBus, make nm_connection_to_dbus(),
etc, represent connections as GVariants of type 'a{sa{sv}}' rather
than as GHashTables-of-GHashTables-of-GValues.

This means we're constantly converting back and forth internally, but
this is just a stepping stone on the way to the full GDBus port, and
all of that code will go away again later.

libnm-util's connection deserializing/copying/replacing functions have
odd semantics where sometimes they both modify a connection AND return
an error. libnm-core tried to improve things by guaranteeing that the
connection would not be modified if the new settings were invalid, but
this ended up breaking a bunch of places that needed to be able to
work with invalid connections. So re-fix the functions by reverting
back to the old semantics, but having return values that clearly
distinguish whether the connection was modified or not.

For comparison:

  - nm_connection_new_from_hash() / nm_simple_connection_new_from_dbus():

      - libnm-util: returns a valid connection or NULL.

      - OLD libnm-core: returned a valid connection or NULL.

      - NEW libnm-core: returns a valid connection or NULL.

  - nm_connection_duplicate() / nm_simple_connection_new_clone():

      - libnm-util: always succeeds, whether or not the connection is
        valid.

      - OLD libnm-core: returned a valid connection or NULL

      - NEW libnm-core: always succeeds, whether or not the connection
        is valid.

    - nm_connection_replace_settings_from_connection():

      - libnm-util: always replaces the settings, but returns FALSE if
        the connection is now invalid.

      - OLD libnm-core: either replaced the settings and returned TRUE
        (if the settings were valid), or else left the connection
        unchanged and returned FALSE (if not).

      - NEW libnm-core: always replaces the settings, and has no
        return value. (The modified connection is valid if and only if
        the replaced-from connection was valid; just like with the
        libnm-util version.)

    - nm_connection_replace_settings():

      - libnm-util: returns TRUE if the new settings are valid, or
        FALSE if either (a) the new settings could not be deserialized
        and the connection is unchanged, or (b) the new settings were
        deserialized, and the connection was updated, but is now not
        valid.

      - OLD libnm-core: either replaced the settings and returned TRUE
        (if the settings were valid), or else left the connection
        unchanged and returned FALSE (if not).

      - NEW libnm-core: returns TRUE if the connection was updated
        (whether or not it is valid), or FALSE if the new settings
        could not be deserialized and the connection is unchanged.

It needs to be possible to deserialize a connection hash into an
invalid NMConnection; in particular, AddAndActivateConnection()
explicitly allows this.

Previously, the SetFunc and NotSetFunc passed to
_nm_setting_class_override_property() could return a verification
error immediately, but this functionality has to go away if we're
going to be able to deserialize invalid connections.

That functionality was only used in the handling of invalid virtual
interface names; reorganize how that code works so that
NMSettingConnection does all of the verification itself. (The code to
make sure that it returned the "correct" error domain in that case
turned out to be irrelevant, since the setting error domains don't get
serialized over D-Bus correctly anyway.)

Before, _nm_setting_to_dbus() would return NULL instead of an empty
hash. This would be the case, if all properties are default.

When exporting connections via DBUS, we eventually call
_nm_setting_to_dbus() to convert the connection into a hash of hashes.
By _nm_setting_to_hash() converting empty hashes to NULL, the setting
is missing. Not returning empty hashes means that to_dbus() and
new_from_dbus() don't make a valid round-trip conversion.

Fix that by always returning a hash from _nm_setting_to_dbus()

https://bugzilla.gnome.org/show_bug.cgi?id=735255

See-also: 4d326182Signed-off-by: Thomas Haller <thaller@redhat.com>

Change all DBUS_TYPE_G_UCHAR_ARRAY properties to G_TYPE_BYTES, and
update corresponding APIs. Notably, this means they are now refcounted
rather than being copied.

Update the rest of NM for the changes. The daemon still converts SSIDs
to GByteArrays internally, because changing it to use GBytes has lots
of trickle-down effects. It can possibly be changed later.

APIs that take arbitrary data should take it in the form of a pointer
and length, not a GByteArray, so that you can use them regardless of
what format you have the data in (GByteArray, GBytes, plain array,
etc).

Make the :addresses and :routes properties be GPtrArrays of
NMIP4Address, etc, rather than just reflecting the D-Bus data.

Make the :dns properties be arrays of strings rather than arrays of
binary IP addresses (and update the corresponding APIs as well).

Change all DBUS_TYPE_G_LIST_OF_STRING and DBUS_TYPE_G_ARRAY_OF_STRING
properties to G_TYPE_STRV, and update everything accordingly.

(This doesn't actually require using
_nm_setting_class_transform_property(); dbus-glib is happy to transform
between 'as' and G_TYPE_STRV.)

Make all mac-address properties (including NMSettingBluetooth:bdaddr,
NMSettingOlpcMesh:dhcp-anycast-addr, and NMSettingWireless:bssid) be
strings, using _nm_setting_class_transform_property() to handle
translating to/from binary form when dealing with D-Bus.

Update everything accordingly for the change, and also add a test for
transformed setting properties to test-general.

NMParamSpecSpecialized existed basically to provide a version of
GParamSpecBoxed that could compare dbus-glib-valued properties
correctly.

However, g_param_values_cmp() was only used by NM directly in one
place (NMSetting's compare_property()), and we don't actually need to
indirect through GParamSpec there; we could just call
NMParamSpecSpecialized's value-comparison function directly.

So, change all NMParamSpecSpecialized properties to GParamSpecBoxed,
rename the _gvalues_compare() function it used to
"nm_property_compare()", and use that from NMSetting.

(g_param_values_cmp() also gets used internally by
g_param_value_defaults(), but all NMParamSpecSpecialized properties
have a default value of NULL, so GParamSpecBoxed's pointer-equality
check will do the job just fine there.)

Make the formerly-nm-param-spec-specialized test compile (fix use of
inet_pton), and pass (include the mandatory "gateway" element in the
IPv6 addresses), make it use gtestutils and g_assert (so it actually
fails when it fails), and test a few more cases.

Remove the virtual :interface-name properties and their getters, and
use property overrides to do backward-compat handling when
serializing/deserializing.

Now when constructing an NMConnection from a hash, if the virtual
property is set and the NMSettingConnection property isn't, then the
override for NMSettingConnection:interface-name will set that property
to the value of the virtual interface-name. And when converting an
NMConnection to a hash, the overrides for the virtual properties will
return the value of NMSettingConnection:interface-name.

The virtual :interface-name properties (eg,
NMDeviceBond:interface-name) are deprecated in favor of
NMSettingConnection:interface-name, and nm_connection_verify() ensures
that their values are kept in sync. So (a) there is no need to set
those properties when we can just set
NMSettingConnection:interface-name instead, and (b) we can replace any
calls to the setting-specific get_interface_name() methods with
nm_connection_get_interface_name() or
nm_setting_connection_get_interface_name().

Drop the NMSetting properties that were marked deprecated in
libnm-util in 0.9.10, but use nm_setting_class_add_dbus_property() to
deal with them appropriately when serializing/deserializing.

Add _nm_setting_class_add_dbus_only_property(), for declaring
properties that appear in the D-Bus serialization, but which don't
correspond to GObject properties.

Since some property overrides will require examining settings other
than the setting that they are on (eg, the value of
802-11-wireless.security depends on whether an
NMSettingWirelessSecurity setting is present, and
NMSettingConnection:interface-name might sometimes be set from, eg,
bond.interface-name), we also update _nm_setting_to_dbus() to take the
full NMConnection as an argument, and _nm_setting_new_from_dbus() to
take the full connection hash.

Additionally, with some deprecated properties, we'll want to validate
them on construction, but we don't need to keep the value around after
that. So allow _nm_setting_new_from_dbus() to return a verification
error directly, so we don't need to store the value until the verify()
call.