From 32f78ae6c3baea7f2a90664fc676e73b236a4372 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Sep 2016 10:30:50 +0200 Subject: [PATCH 1/3] libnm: expose nm_utils_is_json_object() utility function Since we possibly already link against libjansson, we can also expose some helper utils which allows nmcli to do basic validation of JSON without requiring to duplicate the effort of using libjansson. Also, tighten up the cecks to ensure that we have a JSON object at hand. We are really interested in that and not of arrays or literals. --- libnm-core/nm-core-internal.h | 1 - libnm-core/nm-keyfile-reader.c | 2 +- libnm-core/nm-setting-team-port.c | 2 +- libnm-core/nm-setting-team.c | 2 +- libnm-core/nm-utils.c | 69 +++++++++++++++++++++++--- libnm-core/nm-utils.h | 3 ++ libnm-core/tests/test-general.c | 2 +- libnm/libnm.ver | 1 + src/settings/plugins/ifcfg-rh/reader.c | 2 +- 9 files changed, 70 insertions(+), 14 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 471353e741..2a1c69d94b 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -329,7 +329,6 @@ gboolean _nm_utils_inet6_is_token (const struct in6_addr *in6addr); /***********************************************************/ -gboolean _nm_utils_check_valid_json (const char *json, GError **error); gboolean _nm_utils_team_config_equal (const char *conf1, const char *conf2, gboolean port); #endif diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 16f0c225b9..f25fa0e1a4 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -1175,7 +1175,7 @@ team_config_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key gs_free_error GError *error = NULL; conf = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key, NULL); - if (conf && conf[0] && !_nm_utils_check_valid_json (conf, &error)) { + if (conf && conf[0] && !nm_utils_is_json_object (conf, &error)) { handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, _("ignoring invalid team configuration: %s"), error->message); diff --git a/libnm-core/nm-setting-team-port.c b/libnm-core/nm-setting-team-port.c index e3ddbdad7c..649d958273 100644 --- a/libnm-core/nm-setting-team-port.c +++ b/libnm-core/nm-setting-team-port.c @@ -117,7 +117,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) } if (priv->config) { - if (!_nm_utils_check_valid_json (priv->config, error)) { + if (!nm_utils_is_json_object (priv->config, error)) { g_prefix_error (error, "%s.%s: ", NM_SETTING_TEAM_PORT_SETTING_NAME, diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c index 5bf11ed921..c49ebff2b2 100644 --- a/libnm-core/nm-setting-team.c +++ b/libnm-core/nm-setting-team.c @@ -89,7 +89,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return FALSE; if (priv->config) { - if (!_nm_utils_check_valid_json (priv->config, error)) { + if (!nm_utils_is_json_object (priv->config, error)) { g_prefix_error (error, "%s.%s: ", NM_SETTING_TEAM_SETTING_NAME, diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 818b0c43ec..cd54ba6b5f 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -4407,8 +4407,20 @@ const char **nm_utils_enum_get_values (GType type, gint from, gint to) } #if WITH_JANSSON +/** + * nm_utils_is_json_object: + * @str: the JSON string to test + * @error: optional error reason + * + * Returns: whether the passed string is valid JSON. + * If libnm is not compiled with libjansson support, this check will + * also return %TRUE for possibly invalid inputs. If that is a problem + * for you, you must validate the JSON yourself. + * + * Since: 1.6 + */ gboolean -_nm_utils_check_valid_json (const char *str, GError **error) +nm_utils_is_json_object (const char *str, GError **error) { json_t *json; json_error_t jerror; @@ -4419,7 +4431,7 @@ _nm_utils_check_valid_json (const char *str, GError **error) g_set_error_literal (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, - "value is NULL or empty"); + str ? _("value is NULL") : _("value is empty")); return FALSE; } @@ -4428,9 +4440,19 @@ _nm_utils_check_valid_json (const char *str, GError **error) g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, - "%s at position %d", - jerror.text, - jerror.position); + _("invalid JSON at position %d (%s)"), + jerror.position, + jerror.text); + return FALSE; + } + + /* valid JSON (depending on the definition) can also be a literal. + * Here we only allow objects. */ + if (!json_is_object (json)) { + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("is not a JSON object")); return FALSE; } @@ -4522,17 +4544,48 @@ out: #else /* WITH_JANSSON */ gboolean -_nm_utils_check_valid_json (const char *str, GError **error) +nm_utils_is_json_object (const char *str, GError **error) { + g_return_val_if_fail (!error || !*error, FALSE); + + if (str) { + /* libjansson also requires only utf-8 encoding. */ + if (!g_utf8_validate (str, -1, NULL)) { + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("not valid utf-8")); + return FALSE; + } + while (g_ascii_isspace (str[0])) + str++; + } + if (!str || !str[0]) { g_set_error_literal (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, - "value is NULL or empty"); + str ? _("value is NULL") : _("value is empty")); return FALSE; } - return TRUE; + /* do some very basic validation to see if this might be a JSON object. */ + if (str[0] == '{') { + g_size l; + + l = strlen (str) - 1; + while (l > 0 && g_ascii_isspace (str[l])) + l--; + + if (str[l] == '}') + return TRUE; + } + + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("is not a JSON object")); + return FALSE; } gboolean diff --git a/libnm-core/nm-utils.h b/libnm-core/nm-utils.h index 407c14e2c4..4fabfdb7b7 100644 --- a/libnm-core/nm-utils.h +++ b/libnm-core/nm-utils.h @@ -90,6 +90,9 @@ gboolean nm_utils_ap_mode_security_valid (NMUtilsSecurityType type, gboolean nm_utils_wep_key_valid (const char *key, NMWepKeyType wep_type); gboolean nm_utils_wpa_psk_valid (const char *psk); +NM_AVAILABLE_IN_1_6 +gboolean nm_utils_is_json_object (const char *json, GError **error); + GVariant *nm_utils_ip4_dns_to_variant (char **dns); char **nm_utils_ip4_dns_from_variant (GVariant *value); GVariant *nm_utils_ip4_addresses_to_variant (GPtrArray *addresses, diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index e8a9404bf3..c75abbf999 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -4685,7 +4685,7 @@ _json_config_check_valid (const char *conf, gboolean expected) gs_free_error GError *error = NULL; gboolean res; - res = _nm_utils_check_valid_json (conf, &error); + res = nm_utils_is_json_object (conf, &error); g_assert_cmpint (res, ==, expected); g_assert (res || error); } diff --git a/libnm/libnm.ver b/libnm/libnm.ver index 6c149f6587..bc47e8625d 100644 --- a/libnm/libnm.ver +++ b/libnm/libnm.ver @@ -1086,4 +1086,5 @@ global: libnm_1_6_0 { nm_capability_get_type; + nm_utils_is_json_object; } libnm_1_4_0; diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c index 5a351cfa06..80c6ca1553 100644 --- a/src/settings/plugins/ifcfg-rh/reader.c +++ b/src/settings/plugins/ifcfg-rh/reader.c @@ -4258,7 +4258,7 @@ read_team_config (shvarFile *ifcfg, const char *key, GError **error) } svUnescape (value); - if (value && value[0] && !_nm_utils_check_valid_json (value, &local_error)) { + if (value && value[0] && !nm_utils_is_json_object (value, &local_error)) { PARSE_WARNING ("ignoring invalid team configuration: %s", local_error->message); g_clear_pointer (&value, g_free); } From 146e0d23bc6a0f148c654003a43c9bab0e93f9f6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Sep 2016 11:16:48 +0200 Subject: [PATCH 2/3] libnm: reject too large team-config JSON --- libnm-core/nm-setting-team-port.c | 10 ++++++++++ libnm-core/nm-setting-team.c | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/libnm-core/nm-setting-team-port.c b/libnm-core/nm-setting-team-port.c index 649d958273..f64aa5f947 100644 --- a/libnm-core/nm-setting-team-port.c +++ b/libnm-core/nm-setting-team-port.c @@ -117,6 +117,16 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) } if (priv->config) { + if (strlen (priv->config) > 1*1024*1024) { + g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("team config exceeds size limit")); + g_prefix_error (error, + "%s.%s: ", + NM_SETTING_TEAM_PORT_SETTING_NAME, + NM_SETTING_TEAM_PORT_CONFIG); + return FALSE; + } + if (!nm_utils_is_json_object (priv->config, error)) { g_prefix_error (error, "%s.%s: ", diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c index c49ebff2b2..e83ce309a4 100644 --- a/libnm-core/nm-setting-team.c +++ b/libnm-core/nm-setting-team.c @@ -89,6 +89,16 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return FALSE; if (priv->config) { + if (strlen (priv->config) > 1*1024*1024) { + g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("team config exceeds size limit")); + g_prefix_error (error, + "%s.%s: ", + NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_CONFIG); + return FALSE; + } + if (!nm_utils_is_json_object (priv->config, error)) { g_prefix_error (error, "%s.%s: ", From c1a4c084b014fdf35fc836a93c2bb6ba834c4bb2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Sep 2016 10:30:57 +0200 Subject: [PATCH 3/3] cli: support explicitly selecting team-config as file or json data nmcli has a heuristic when setting the team-config to accepting both a filename or the plain json text. Add support for two schemes "file://" and "json://" to explicitly determine whether to read from file or from json. Also, no longer silently ignore an all-whitespace word. That is an error (unless you have a file named " "). Also, no longer replace newlines with space. Don't mangle the input text at all. --- clients/cli/common.c | 64 +++++++++++++++++++++++++++++++------------- man/nmcli.xml | 16 ++++++----- 2 files changed, 55 insertions(+), 25 deletions(-) diff --git a/clients/cli/common.c b/clients/cli/common.c index 2d7ecc6811..22e750475b 100644 --- a/clients/cli/common.c +++ b/clients/cli/common.c @@ -826,10 +826,11 @@ nmc_bond_validate_mode (const char *mode, GError **error) /* * nmc_team_check_config: * @config: file name with team config, or raw team JSON config data - * @out_config: raw team JSON config data (with removed new-line characters) + * @out_config: raw team JSON config data + * The value must be freed with g_free(). * @error: location to store error, or %NUL * - * Check team config from @config parameter and return the checked/sanitized + * Check team config from @config parameter and return the checked * config in @out_config. * * Returns: %TRUE if the config is valid, %FALSE if it is invalid @@ -837,33 +838,58 @@ nmc_bond_validate_mode (const char *mode, GError **error) gboolean nmc_team_check_config (const char *config, char **out_config, GError **error) { - char *contents = NULL; + enum { + _TEAM_CONFIG_TYPE_GUESS, + _TEAM_CONFIG_TYPE_FILE, + _TEAM_CONFIG_TYPE_JSON, + } desired_type = _TEAM_CONFIG_TYPE_GUESS; + const char *filename = NULL; size_t c_len = 0; + gs_free char *config_clone = NULL; *out_config = NULL; - if (!config || strlen (config) == strspn (config, " \t")) + if (!config || !config[0]) return TRUE; - /* 'config' can be either a file name or raw JSON config data */ - if (g_file_test (config, G_FILE_TEST_EXISTS)) - (void) g_file_get_contents (config, &contents, NULL, NULL); - else - contents = g_strdup (config); - - if (contents) { - g_strstrip (contents); - c_len = strlen (contents); + if (g_str_has_prefix (config, "file://")) { + config += NM_STRLEN ("file://"); + desired_type = _TEAM_CONFIG_TYPE_FILE; + } else if (g_str_has_prefix (config, "json://")) { + config += NM_STRLEN ("json://"); + desired_type = _TEAM_CONFIG_TYPE_JSON; } - /* Do a simple validity check */ - if (!contents || !contents[0] || c_len > 100000 || contents[0] != '{' || contents[c_len-1] != '}') { - g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, - _("'%s' is not a valid team configuration or file name."), config); - g_free (contents); + if (NM_IN_SET (desired_type, _TEAM_CONFIG_TYPE_FILE, _TEAM_CONFIG_TYPE_GUESS)) { + gs_free char *contents = NULL; + + if (!g_file_get_contents (config, &contents, &c_len, NULL)) { + if (desired_type == _TEAM_CONFIG_TYPE_FILE) { + g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, + _("cannot read team config from file '%s'"), + config); + return FALSE; + } + } else { + filename = config; + config = config_clone = g_steal_pointer (&contents); + } + } + + if (!nm_utils_is_json_object (config, NULL)) { + if (filename) { + g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, + _("'%s' does not contain a valid team configuration"), filename); + } else { + g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, + _("team configuration must be a JSON object")); + } return FALSE; } - *out_config = g_strdelimit (contents, "\r\n", ' '); + + *out_config = (config == config_clone) + ? g_steal_pointer (&config_clone) + : g_strdup (config); return TRUE; } diff --git a/man/nmcli.xml b/man/nmcli.xml index 5a936335c0..965c05f955 100644 --- a/man/nmcli.xml +++ b/man/nmcli.xml @@ -1827,21 +1827,25 @@ It's equivalent of using the +bond.options 'option=value' syn - Team options +
Team options - AliasProperty + AliasPropertyNote - configteam.config + configteam.config + Either a filename or a team configuration in JSON format. To enforce one or the other, the value can be prefixed with "file://" or "json://". +
- Team port options +
Team port options - AliasProperty + AliasPropertyNote - configteam-port.config + configteam-port.config + Either a filename or a team configuration in JSON format. To enforce one or the other, the value can be prefixed with "file://" or "json://". +