diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index cdb3084a07..115f65591a 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -2400,13 +2400,9 @@ _nm_meta_setting_bond_add_option(NMSetting * setting, char * p; if (!value || !value[0]) { - if (!nm_setting_bond_remove_option(s_bond, name)) { - nm_utils_error_set(error, - NM_UTILS_ERROR_INVALID_ARGUMENT, - _("failed to unset bond option \"%s\""), - name); - return FALSE; - } + /* This call only fails if name is currently not tracked. It's not an + * error to remove an option that is not set. */ + nm_setting_bond_remove_option(s_bond, name); return TRUE; } @@ -2421,7 +2417,7 @@ _nm_meta_setting_bond_add_option(NMSetting * setting, *p = ','; } - if (!nm_setting_bond_add_option(s_bond, name, value)) { + if (!nm_setting_bond_validate_option(name, value)) { nm_utils_error_set(error, NM_UTILS_ERROR_INVALID_ARGUMENT, _("failed to set bond option \"%s\""), @@ -2429,6 +2425,8 @@ _nm_meta_setting_bond_add_option(NMSetting * setting, return FALSE; } + nm_setting_bond_add_option(s_bond, name, value); + if (nm_streq(name, NM_SETTING_BOND_OPTION_ARP_INTERVAL)) { if (_nm_utils_ascii_str_to_int64(value, 10, 0, G_MAXINT, 0) > 0) _nm_setting_bond_remove_options_miimon(s_bond); diff --git a/libnm-core/nm-keyfile/nm-keyfile.c b/libnm-core/nm-keyfile/nm-keyfile.c index 395af1db09..a8aefe6349 100644 --- a/libnm-core/nm-keyfile/nm-keyfile.c +++ b/libnm-core/nm-keyfile/nm-keyfile.c @@ -1141,7 +1141,10 @@ mac_address_parser_INFINIBAND(KeyfileReaderInfo *info, NMSetting *setting, const } static void -read_hash_of_string(GKeyFile *file, NMSetting *setting, const char *key) +read_hash_of_string(KeyfileReaderInfo *info, + GKeyFile * file, + NMSetting * setting, + const char * kf_group) { gs_strfreev char **keys = NULL; const char *const *iter; @@ -1149,10 +1152,10 @@ read_hash_of_string(GKeyFile *file, NMSetting *setting, const char *key) gboolean is_vpn; gsize n_keys; - nm_assert((NM_IS_SETTING_VPN(setting) && nm_streq(key, NM_SETTING_VPN_DATA)) - || (NM_IS_SETTING_VPN(setting) && nm_streq(key, NM_SETTING_VPN_SECRETS)) - || (NM_IS_SETTING_BOND(setting) && nm_streq(key, NM_SETTING_BOND_OPTIONS)) - || (NM_IS_SETTING_USER(setting) && nm_streq(key, NM_SETTING_USER_DATA))); + nm_assert((NM_IS_SETTING_VPN(setting) && nm_streq(kf_group, NM_SETTING_VPN_DATA)) + || (NM_IS_SETTING_VPN(setting) && nm_streq(kf_group, NM_SETTING_VPN_SECRETS)) + || (NM_IS_SETTING_BOND(setting) && nm_streq(kf_group, NM_SETTING_BOND_OPTIONS)) + || (NM_IS_SETTING_USER(setting) && nm_streq(kf_group, NM_SETTING_USER_DATA))); keys = nm_keyfile_plugin_kf_get_keys(file, setting_name, &n_keys, NULL); if (n_keys == 0) @@ -1160,23 +1163,38 @@ read_hash_of_string(GKeyFile *file, NMSetting *setting, const char *key) if ((is_vpn = NM_IS_SETTING_VPN(setting)) || NM_IS_SETTING_BOND(setting)) { for (iter = (const char *const *) keys; *iter; iter++) { + const char * kf_key = *iter; gs_free char *to_free = NULL; gs_free char *value = NULL; const char * name; - value = nm_keyfile_plugin_kf_get_string(file, setting_name, *iter, NULL); + value = nm_keyfile_plugin_kf_get_string(file, setting_name, kf_key, NULL); if (!value) continue; - name = nm_keyfile_key_decode(*iter, &to_free); + name = nm_keyfile_key_decode(kf_key, &to_free); if (is_vpn) { /* Add any item that's not a class property to the data hash */ if (!g_object_class_find_property(G_OBJECT_GET_CLASS(setting), name)) nm_setting_vpn_add_data_item(NM_SETTING_VPN(setting), name, value); } else { - if (!nm_streq(name, "interface-name")) - nm_setting_bond_add_option(NM_SETTING_BOND(setting), name, value); + if (!nm_streq(name, "interface-name")) { + gs_free_error GError *error = NULL; + + if (!_nm_setting_bond_validate_option(name, value, &error)) { + if (!handle_warn(info, + kf_key, + name, + NM_KEYFILE_WARN_SEVERITY_WARN, + _("ignoring invalid bond option %s%s%s = %s%s%s: %s"), + NM_PRINT_FMT_QUOTE_STRING(name), + NM_PRINT_FMT_QUOTE_STRING(value), + error->message)) + return; + } else + nm_setting_bond_add_option(NM_SETTING_BOND(setting), name, value); + } } } openconnect_fix_secret_flags(setting); @@ -3170,7 +3188,7 @@ read_one_setting_value(KeyfileReaderInfo * info, NULL); g_object_set(setting, key, sa, NULL); } else if (type == G_TYPE_HASH_TABLE) { - read_hash_of_string(keyfile, setting, key); + read_hash_of_string(info, keyfile, setting, key); } else if (type == G_TYPE_ARRAY) { read_array_of_uint(keyfile, setting, key); } else if (G_TYPE_IS_FLAGS(type)) { diff --git a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h index 025d684776..ca29eade24 100644 --- a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h +++ b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h @@ -73,6 +73,8 @@ NMBondMode _nm_setting_bond_mode_from_string(const char *str); const char *_nm_setting_bond_mode_to_string(int mode); +gboolean _nm_setting_bond_validate_option(const char *name, const char *value, GError **error); + /*****************************************************************************/ static inline guint32 diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c index f355734bfa..2155815e06 100644 --- a/libnm-core/nm-setting-bond.c +++ b/libnm-core/nm-setting-bond.c @@ -515,8 +515,8 @@ validate_ifname(const char *name, const char *value) return nm_utils_ifname_valid_kernel(value, NULL); } -static gboolean -_setting_bond_validate_option(const char *name, const char *value, GError **error) +gboolean +_nm_setting_bond_validate_option(const char *name, const char *value, GError **error) { const OptionMeta *option_meta; gboolean success; @@ -589,7 +589,7 @@ handle_error: gboolean nm_setting_bond_validate_option(const char *name, const char *value) { - return _setting_bond_validate_option(name, value, NULL); + return _nm_setting_bond_validate_option(name, value, NULL); } /** @@ -608,9 +608,6 @@ nm_setting_bond_get_option_by_name(NMSettingBond *setting, const char *name) { g_return_val_if_fail(NM_IS_SETTING_BOND(setting), NULL); - if (!nm_setting_bond_validate_option(name, NULL)) - return NULL; - return _bond_get_option(setting, name); } @@ -620,16 +617,18 @@ nm_setting_bond_get_option_by_name(NMSettingBond *setting, const char *name) * @name: name for the option * @value: value for the option * - * Add an option to the table. The option is compared to an internal list - * of allowed options. Option names may contain only alphanumeric characters - * (ie [a-zA-Z0-9]). Adding a new name replaces any existing name/value pair + * Add an option to the table. Adding a new name replaces any existing name/value pair * that may already exist. * - * The order of how to set several options is relevant because there are options - * that conflict with each other. + * Returns: returns %FALSE if either @name or @value is %NULL, in that case + * the option is not set. Otherwise, the function does not fail and does not validate + * the arguments. All validation happens via nm_connection_verify() or do basic validation + * yourself with nm_setting_bond_validate_option(). * - * Returns: %TRUE if the option was valid and was added to the internal option - * list, %FALSE if it was not. + * Note: Before 1.30, libnm would perform basic validation of the name and the value + * via nm_setting_bond_validate_option() and reject the request by returning FALSE. + * Since 1.30, libnm no longer rejects any values as the setter is not supposed + * to perform validation. **/ gboolean nm_setting_bond_add_option(NMSettingBond *setting, const char *name, const char *value) @@ -638,16 +637,16 @@ nm_setting_bond_add_option(NMSettingBond *setting, const char *name, const char g_return_val_if_fail(NM_IS_SETTING_BOND(setting), FALSE); - if (!value || !nm_setting_bond_validate_option(name, value)) + if (!name) + return FALSE; + if (!value) return FALSE; priv = NM_SETTING_BOND_GET_PRIVATE(setting); nm_clear_g_free(&priv->options_idx_cache); g_hash_table_insert(priv->options, g_strdup(name), g_strdup(value)); - _notify(setting, PROP_OPTIONS); - return TRUE; } @@ -666,20 +665,17 @@ gboolean nm_setting_bond_remove_option(NMSettingBond *setting, const char *name) { NMSettingBondPrivate *priv; - gboolean found; g_return_val_if_fail(NM_IS_SETTING_BOND(setting), FALSE); - if (!nm_setting_bond_validate_option(name, NULL)) - return FALSE; - priv = NM_SETTING_BOND_GET_PRIVATE(setting); + if (!g_hash_table_remove(priv->options, name)) + return FALSE; + nm_clear_g_free(&priv->options_idx_cache); - found = g_hash_table_remove(priv->options, name); - if (found) - _notify(setting, PROP_OPTIONS); - return found; + _notify(setting, PROP_OPTIONS); + return TRUE; } /** @@ -783,7 +779,7 @@ verify(NMSetting *setting, NMConnection *connection, GError **error) for (i = 0; priv->options_idx_cache[i].name; i++) { n = &priv->options_idx_cache[i]; - if (!n->value_str || !_setting_bond_validate_option(n->name, n->value_str, error)) { + if (!n->value_str || !_nm_setting_bond_validate_option(n->name, n->value_str, error)) { g_prefix_error(error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index a54c9b47c4..43f2a92ef4 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -149,6 +149,7 @@ ignore_option(NMSettingBond *s_bond, const char *option, const char *value) static void update_connection(NMDevice *device, NMConnection *connection) { + NMDeviceBond * self = NM_DEVICE_BOND(device); NMSettingBond *s_bond = nm_connection_get_setting_bond(connection); int ifindex = nm_device_get_ifindex(device); NMBondMode mode = NM_BOND_MODE_UNKNOWN; @@ -197,7 +198,10 @@ update_connection(NMDevice *device, NMConnection *connection) } } - nm_setting_bond_add_option(s_bond, option, value); + if (!_nm_setting_bond_validate_option(option, value, NULL)) + _LOGT(LOGD_BOND, "cannot set invalid bond option '%s' = '%s'", option, value); + else + nm_setting_bond_add_option(s_bond, option, value); } } } diff --git a/src/initrd/nmi-cmdline-reader.c b/src/initrd/nmi-cmdline-reader.c index b9432d4a62..ead9053683 100644 --- a/src/initrd/nmi-cmdline-reader.c +++ b/src/initrd/nmi-cmdline-reader.c @@ -682,8 +682,6 @@ reader_parse_master(Reader *reader, char *argument, const char *type_name, const char * slaves; const char * slave; char * opts; - char * opt; - const char * opt_name; const char * mtu = NULL; master = get_word(&argument, ':'); @@ -705,8 +703,21 @@ reader_parse_master(Reader *reader, char *argument, const char *type_name, const opts = get_word(&argument, ':'); while (opts && *opts) { + gs_free_error GError *error = NULL; + char * opt; + const char * opt_name; + opt = get_word(&opts, ','); opt_name = get_word(&opt, '='); + + if (!_nm_setting_bond_validate_option(opt_name, opt, &error)) { + _LOGW(LOGD_CORE, + "Ignoring invalid bond option: %s%s%s = %s%s%s: %s", + NM_PRINT_FMT_QUOTE_STRING(opt_name), + NM_PRINT_FMT_QUOTE_STRING(opt), + error->message); + continue; + } nm_setting_bond_add_option(s_bond, opt_name, opt); } diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 5a021a8c37..3389b2a0aa 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -5339,24 +5339,31 @@ infiniband_connection_from_ifcfg(const char *file, shvarFile *ifcfg, GError **er static void handle_bond_option(NMSettingBond *s_bond, const char *key, const char *value) { - char * sanitized = NULL, *j; - const char *p = value; + gs_free char *sanitized = NULL; + const char * p = value; /* Remove any quotes or +/- from arp_ip_target */ - if (!g_strcmp0(key, NM_SETTING_BOND_OPTION_ARP_IP_TARGET) && value && value[0]) { + if (nm_streq0(key, NM_SETTING_BOND_OPTION_ARP_IP_TARGET) && value && value[0]) { + char *j; + if (*p == '\'' || *p == '"') p++; - j = sanitized = g_malloc0(strlen(p) + 1); + j = sanitized = g_malloc(strlen(p) + 1); while (*p) { if (*p != '+' && *p != '-' && *p != '\'' && *p != '"') *j++ = *p; p++; } + *j++ = '\0'; + value = sanitized; } - if (!nm_setting_bond_add_option(s_bond, key, sanitized ?: value)) - PARSE_WARNING("invalid bonding option '%s' = %s", key, sanitized ?: value); - g_free(sanitized); + if (!_nm_setting_bond_validate_option(key, value, NULL)) { + PARSE_WARNING("invalid bonding option '%s' = %s", key, value); + return; + } + + nm_setting_bond_add_option(s_bond, key, value); } static NMSetting *