diff --git a/.travis.yml b/.travis.yml index b907bf13ed..44e5e0a247 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,7 @@ before_install: libnl-3-dev libnl-route-3-dev libnl-genl-3-dev ppp-dev libpolkit-gobject-1-dev libgnutls28-dev libgcrypt11-dev uuid-dev libudev-dev libgudev-1.0-dev libgirepository1.0-dev gobject-introspection libsoup2.4-dev gtk-doc-tools libglib2.0-doc libreadline-dev libnewt-dev libnss3-dev iptables make python-software-properties python-gi - python-dbus dbus dbus-x11 + python-dbus dbus dbus-x11 libjansson4 libjansson-dev - sudo dbus-uuidgen --ensure - sudo apt-add-repository 'deb http://archive.ubuntu.com/ubuntu trusty main' - sudo apt-add-repository 'deb http://archive.ubuntu.com/ubuntu trusty-backports main restricted universe multiverse' diff --git a/configure.ac b/configure.ac index a7bd024a00..01696ebaa4 100644 --- a/configure.ac +++ b/configure.ac @@ -553,6 +553,18 @@ else fi AM_CONDITIONAL(WITH_TEAMDCTL, test "${enable_teamdctl}" = "yes") +PKG_CHECK_MODULES(JANSSON, jansson, [have_jansson=yes], [have_jansson=no]) +AC_ARG_ENABLE(json-validation, AS_HELP_STRING([--enable-json-validation], [Enable JSON validation in libnm]), + [enable_json_validation=${enableval}], [enable_json_validation=${have_jansson}]) +if (test "${enable_json_validation}" = "yes"); then + if test x"$have_jansson" = x"no"; then + AC_MSG_ERROR(Jansson is required for JSON validation) + fi + AC_DEFINE(WITH_JANSSON, 1, [Define if JANSSON is enabled]) +else + AC_DEFINE(WITH_JANSSON, 0, [Define if JANSSON is enabled]) +fi + # we usually compile with polkit support. --enable-polkit=yes|no only sets the # default configuration for main.auth-polkit. User can always enable/disable polkit # autorization via config. Only when specifying --enable-polkit=disabled, we do @@ -1178,4 +1190,5 @@ echo " more-asserts: $more_asserts" echo " valgrind: $with_valgrind $with_valgrind_suppressions" echo " code coverage: $enable_code_coverage" echo " LTO: $enable_lto" +echo " JSON validation: $enable_json_validation" echo diff --git a/contrib/fedora/REQUIRED_PACKAGES b/contrib/fedora/REQUIRED_PACKAGES index 559ac848f6..a5c218a616 100644 --- a/contrib/fedora/REQUIRED_PACKAGES +++ b/contrib/fedora/REQUIRED_PACKAGES @@ -42,4 +42,5 @@ yum install \ rpm-build \ perl-YAML-LibYAM \ perl-YAML \ + jansson-devel \ diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index 1f40e01a5f..54a78a55f1 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -169,6 +169,7 @@ BuildRequires: pygobject3-base BuildRequires: dbus-python BuildRequires: libselinux-devel BuildRequires: polkit-devel +BuildRequires: jansson-devel %description diff --git a/libnm-core/Makefile.am b/libnm-core/Makefile.am index 23aa042d44..4bc99b3e4e 100644 --- a/libnm-core/Makefile.am +++ b/libnm-core/Makefile.am @@ -13,7 +13,8 @@ AM_CPPFLAGS = \ -DNMLIBDIR=\"$(nmlibdir)\" \ -DNETWORKMANAGER_COMPILATION=NM_NETWORKMANAGER_COMPILATION_LIB \ $(GLIB_CFLAGS) \ - $(CODE_COVERAGE_CFLAGS) + $(CODE_COVERAGE_CFLAGS) \ + $(JANSSON_CFLAGS) noinst_LTLIBRARIES = libnm-core.la @@ -35,7 +36,8 @@ GLIB_MKENUMS_C_FLAGS = --identifier-prefix NM libnm_core_la_LIBADD = \ $(GLIB_LIBS) \ - $(UUID_LIBS) + $(UUID_LIBS) \ + $(JANSSON_LIBS) libnm_core_la_LDFLAGS = \ $(CODE_COVERAGE_LDFLAGS) diff --git a/libnm-core/nm-setting-team-port.c b/libnm-core/nm-setting-team-port.c index 9671f1d915..d9a444fe45 100644 --- a/libnm-core/nm-setting-team-port.c +++ b/libnm-core/nm-setting-team-port.c @@ -85,6 +85,18 @@ nm_setting_team_port_get_config (NMSettingTeamPort *setting) static gboolean verify (NMSetting *setting, NMConnection *connection, GError **error) { + NMSettingTeamPortPrivate *priv = NM_SETTING_TEAM_PORT_GET_PRIVATE (setting); + + if (priv->config) { + if (!_nm_utils_check_valid_json (priv->config, error)) { + g_prefix_error (error, + "%s.%s: ", + NM_SETTING_TEAM_PORT_SETTING_NAME, + NM_SETTING_TEAM_PORT_CONFIG); + return FALSE; + } + } + if (connection) { NMSettingConnection *s_con; const char *slave_type; @@ -116,6 +128,25 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return TRUE; } +static gboolean +compare_property (NMSetting *setting, + NMSetting *other, + const GParamSpec *prop_spec, + NMSettingCompareFlags flags) +{ + NMSettingClass *parent_class; + + if (nm_streq0 (prop_spec->name, NM_SETTING_TEAM_PORT_CONFIG)) { + return _nm_utils_team_config_equal (NM_SETTING_TEAM_PORT_GET_PRIVATE (setting)->config, + NM_SETTING_TEAM_PORT_GET_PRIVATE (other)->config, + TRUE); + } + + /* Otherwise chain up to parent to handle generic compare */ + parent_class = NM_SETTING_CLASS (nm_setting_team_port_parent_class); + return parent_class->compare_property (setting, other, prop_spec, flags); +} + static void nm_setting_team_port_init (NMSettingTeamPort *setting) { @@ -173,10 +204,11 @@ nm_setting_team_port_class_init (NMSettingTeamPortClass *setting_class) g_type_class_add_private (setting_class, sizeof (NMSettingTeamPortPrivate)); /* virtual methods */ - object_class->set_property = set_property; - object_class->get_property = get_property; - object_class->finalize = finalize; - parent_class->verify = verify; + object_class->set_property = set_property; + object_class->get_property = get_property; + object_class->finalize = finalize; + parent_class->compare_property = compare_property; + parent_class->verify = verify; /* Properties */ /** diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c index d47b1e72ca..f3bf758633 100644 --- a/libnm-core/nm-setting-team.c +++ b/libnm-core/nm-setting-team.c @@ -27,6 +27,7 @@ #include "nm-utils.h" #include "nm-utils-private.h" #include "nm-connection-private.h" +#include "nm-utils-private.h" /** * SECTION:nm-setting-team @@ -82,7 +83,41 @@ nm_setting_team_get_config (NMSettingTeam *setting) static gboolean verify (NMSetting *setting, NMConnection *connection, GError **error) { - return _nm_connection_verify_required_interface_name (connection, error); + NMSettingTeamPrivate *priv = NM_SETTING_TEAM_GET_PRIVATE (setting); + + if (!_nm_connection_verify_required_interface_name (connection, error)) + return FALSE; + + if (priv->config) { + if (!_nm_utils_check_valid_json (priv->config, error)) { + g_prefix_error (error, + "%s.%s: ", + NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_TEAM_CONFIG); + return FALSE; + } + } + + return TRUE; +} + +static gboolean +compare_property (NMSetting *setting, + NMSetting *other, + const GParamSpec *prop_spec, + NMSettingCompareFlags flags) +{ + NMSettingClass *parent_class; + + if (nm_streq0 (prop_spec->name, NM_SETTING_TEAM_CONFIG)) { + return _nm_utils_team_config_equal (NM_SETTING_TEAM_GET_PRIVATE (setting)->config, + NM_SETTING_TEAM_GET_PRIVATE (other)->config, + FALSE); + } + + /* Otherwise chain up to parent to handle generic compare */ + parent_class = NM_SETTING_CLASS (nm_setting_team_parent_class); + return parent_class->compare_property (setting, other, prop_spec, flags); } static void @@ -142,10 +177,11 @@ nm_setting_team_class_init (NMSettingTeamClass *setting_class) g_type_class_add_private (setting_class, sizeof (NMSettingTeamPrivate)); /* virtual methods */ - object_class->set_property = set_property; - object_class->get_property = get_property; - object_class->finalize = finalize; - parent_class->verify = verify; + object_class->set_property = set_property; + object_class->get_property = get_property; + object_class->finalize = finalize; + parent_class->compare_property = compare_property; + parent_class->verify = verify; /* Properties */ /** diff --git a/libnm-core/nm-utils-private.h b/libnm-core/nm-utils-private.h index 68aaaa1c80..611c467d06 100644 --- a/libnm-core/nm-utils-private.h +++ b/libnm-core/nm-utils-private.h @@ -31,6 +31,9 @@ gboolean _nm_utils_string_slist_validate (GSList *list, const char **valid_values); +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); + /* D-Bus transform funcs */ GVariant * _nm_utils_hwaddr_to_dbus (const GValue *prop_value); diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 70c268faa2..1a7de119c0 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -33,6 +33,10 @@ #include #include +#if WITH_JANSSON +#include +#endif + #include "nm-utils-private.h" #include "nm-setting-private.h" #include "crypto.h" @@ -4090,3 +4094,140 @@ const char **nm_utils_enum_get_values (GType type, gint from, gint to) return (const char **) g_ptr_array_free (array, FALSE); } +#if WITH_JANSSON +gboolean +_nm_utils_check_valid_json (const char *str, GError **error) +{ + json_t *json; + json_error_t jerror; + + g_return_val_if_fail (!error || !*error, FALSE); + + if (!str || !str[0]) { + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + "value is NULL or empty"); + return FALSE; + } + + json = json_loads (str, 0, &jerror); + if (!json) { + g_set_error (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + "%s at position %d", + jerror.text, + jerror.position); + return FALSE; + } + + json_decref (json); + return TRUE; +} + +/* json_object_foreach_safe() is only available since Jansson 2.8, + * reimplement it */ +#define _json_object_foreach_safe(object, n, key, value) \ + for (key = json_object_iter_key (json_object_iter (object)), \ + n = json_object_iter_next (object, json_object_iter_at (object, key)); \ + key && (value = json_object_iter_value (json_object_iter_at (object, key))); \ + key = json_object_iter_key (n), \ + n = json_object_iter_next (object, json_object_iter_at (object, key))) + +gboolean +_nm_utils_team_config_equal (const char *conf1, + const char *conf2, + gboolean port_config) +{ + json_t *json1 = NULL, *json2 = NULL, *json; + gs_free char *dump1 = NULL, *dump2 = NULL; + json_t *value, *property; + json_error_t jerror; + const char *key; + gboolean ret; + void *tmp; + int i; + + if (nm_streq0 (conf1, conf2)) + return TRUE; + + /* A NULL configuration is equivalent to default value '{}' */ + json1 = json_loads (conf1 ?: "{}", 0, &jerror); + if (json1) + json2 = json_loads (conf2 ?: "{}", 0, &jerror); + + if (!json1 || !json2) { + ret = FALSE; + goto out; + } + + /* Some properties are added by teamd when missing from the initial + * configuration. Add them with the default value if necessary, depending + * on the configuration type. + */ + for (i = 0, json = json1; i < 2; i++, json = json2) { + if (port_config) { + property = json_object_get (json, "link_watch"); + if (!property) { + property = json_object (); + json_object_set_new (property, "name", json_string ("ethtool")); + json_object_set_new (json, "link_watch", property); + } + } else { + property = json_object_get (json, "runner"); + if (!property) { + property = json_object (); + json_object_set_new (property, "name", json_string ("roundrobin")); + json_object_set_new (json, "runner", property); + } + } + } + + /* Only consider a given subset of nodes, others can change depending on + * current state */ + for (i = 0, json = json1; i < 2; i++, json = json2) { + _json_object_foreach_safe (json, tmp, key, value) { + if (!NM_IN_STRSET (key, "runner", "link_watch")) + json_object_del (json, key); + } + } + + dump1 = json_dumps (json1, JSON_INDENT(0) | JSON_ENSURE_ASCII | JSON_SORT_KEYS); + dump2 = json_dumps (json2, JSON_INDENT(0) | JSON_ENSURE_ASCII | JSON_SORT_KEYS); + + ret = nm_streq0 (dump1, dump2); +out: + + if (json1) + json_decref (json1); + if (json2) + json_decref (json2); + + return ret; +} + +#else /* WITH_JANSSON */ + +gboolean +_nm_utils_check_valid_json (const char *str, GError **error) +{ + if (!str || !str[0]) { + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + "value is NULL or empty"); + return FALSE; + } + + return TRUE; +} + +gboolean +_nm_utils_team_config_equal (const char *conf1, + const char *conf2, + gboolean port_config) +{ + return nm_streq0 (conf1, conf2); +} +#endif diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index dcbcb88021..a9f851d1ab 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -4382,6 +4382,110 @@ test_nm_utils_dns_option_find_idx (void) /******************************************************************************/ +static void +_json_config_check_valid (const char *conf, gboolean expected) +{ + GError *error = NULL; + gboolean res; + + res = _nm_utils_check_valid_json (conf, &error); + g_assert_cmpint (res, ==, expected); + g_assert (res || error); +} + +static void +test_nm_utils_check_valid_json (void) +{ + _json_config_check_valid (NULL, FALSE); + _json_config_check_valid ("", FALSE); +#if WITH_JANSSON + _json_config_check_valid ("{ }", TRUE); + _json_config_check_valid ("{ \"a\" : 1 }", TRUE); + _json_config_check_valid ("{ \"a\" : }", FALSE); +#else + /* Without JSON library everything except empty string is considered valid */ + _json_config_check_valid ("{ }", TRUE); + _json_config_check_valid ("{'%!-a1", TRUE); +#endif +} + +static void +_team_config_equal_check (const char *conf1, + const char *conf2, + gboolean port_config, + gboolean expected) +{ + g_assert_cmpint (_nm_utils_team_config_equal (conf1, conf2, port_config), ==, expected); +} + +static void +test_nm_utils_team_config_equal (void) +{ +#if WITH_JANSSON + _team_config_equal_check ("", "", TRUE, TRUE); + _team_config_equal_check ("{}", + "{ }", + TRUE, + TRUE); + _team_config_equal_check ("{}", + "{", + TRUE, + FALSE); + + /* team config */ + _team_config_equal_check ("{ }", + "{ \"runner\" : { \"name\" : \"roundrobin\"} }", + FALSE, + TRUE); + _team_config_equal_check ("{ }", + "{ \"runner\" : { \"name\" : \"random\"} }", + FALSE, + FALSE); + _team_config_equal_check ("{ \"runner\" : { \"name\" : \"roundrobin\"} }", + "{ \"runner\" : { \"name\" : \"random\"} }", + FALSE, + FALSE); + _team_config_equal_check ("{ \"runner\" : { \"name\" : \"random\"} }", + "{ \"runner\" : { \"name\" : \"random\"} }", + FALSE, + TRUE); + _team_config_equal_check ("{ \"runner\" : { \"name\" : \"random\"}, \"ports\" : { \"eth0\" : {} } }", + "{ \"runner\" : { \"name\" : \"random\"}, \"ports\" : { \"eth1\" : {} } }", + FALSE, + TRUE); + + /* team port config */ + _team_config_equal_check ("{ }", + "{ \"link_watch\" : { \"name\" : \"ethtool\"} }", + TRUE, + TRUE); + _team_config_equal_check ("{ }", + "{ \"link_watch\" : { \"name\" : \"arp_ping\"} }", + TRUE, + FALSE); + _team_config_equal_check ("{ \"link_watch\" : { \"name\" : \"ethtool\"} }", + "{ \"link_watch\" : { \"name\" : \"arp_ping\"} }", + TRUE, + FALSE); + _team_config_equal_check ("{ \"link_watch\" : { \"name\" : \"arp_ping\"} }", + "{ \"link_watch\" : { \"name\" : \"arp_ping\"} }", + TRUE, + TRUE); + _team_config_equal_check ("{ \"link_watch\" : { \"name\" : \"arp_ping\"}, \"ports\" : { \"eth0\" : {} } }", + "{ \"link_watch\" : { \"name\" : \"arp_ping\"}, \"ports\" : { \"eth1\" : {} } }", + TRUE, + TRUE); +#else + /* Without JSON library, strings are compared for equality */ + _team_config_equal_check ("", "", TRUE, TRUE); + _team_config_equal_check ("", " ", TRUE, FALSE); + _team_config_equal_check ("{ \"a\": 1 }", "{ \"a\": 1 }", TRUE, TRUE); + _team_config_equal_check ("{ \"a\": 1 }", "{ \"a\": 1 }", TRUE, FALSE); +#endif +} + +/******************************************************************************/ + enum TEST_IS_POWER_OF_TWP_ENUM_SIGNED { _DUMMY_1 = -1, }; @@ -5066,7 +5170,8 @@ int main (int argc, char **argv) g_test_add_func ("/core/general/_nm_utils_dns_option_validate", test_nm_utils_dns_option_validate); g_test_add_func ("/core/general/_nm_utils_dns_option_find_idx", test_nm_utils_dns_option_find_idx); - + g_test_add_func ("/core/general/_nm_utils_validate_json", test_nm_utils_check_valid_json); + g_test_add_func ("/core/general/_nm_utils_team_config_equal", test_nm_utils_team_config_equal); g_test_add_func ("/core/general/test_nm_utils_enum", test_nm_utils_enum); return g_test_run (); diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 60842c18ab..47930f437c 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -51,6 +51,7 @@ typedef struct { guint teamd_process_watch; guint teamd_timeout; guint teamd_dbus_watch; + gboolean teamd_dbus_name_owned; } NMDeviceTeamPrivate; static gboolean teamd_start (NMDevice *device, NMSettingTeam *s_team); @@ -160,12 +161,11 @@ update_connection (NMDevice *device, NMConnection *connection) } g_object_set (G_OBJECT (s_team), NM_SETTING_TEAM_CONFIG, NULL, NULL); - if (priv->tdc) { + if (ensure_teamd_connection (device)) { const char *config = NULL; int err; - err = teamdctl_config_get_raw_direct (NM_DEVICE_TEAM_GET_PRIVATE (device)->tdc, - (char **)&config); + err = teamdctl_config_get_raw_direct (priv->tdc, (char **)&config); if (err == 0) g_object_set (G_OBJECT (s_team), NM_SETTING_TEAM_CONFIG, config, NULL); else @@ -296,6 +296,7 @@ teamd_dbus_appeared (GDBusConnection *connection, gboolean success; g_return_if_fail (priv->teamd_dbus_watch); + priv->teamd_dbus_name_owned = TRUE; _LOGI (LOGD_TEAM, "teamd appeared on D-Bus"); nm_device_queue_recheck_assume (device); @@ -349,7 +350,7 @@ teamd_dbus_vanished (GDBusConnection *dbus_connection, g_return_if_fail (priv->teamd_dbus_watch); - if (!priv->tdc) { + if (!priv->teamd_dbus_name_owned) { /* g_bus_watch_name will always raise an initial signal, to indicate whether the * name exists/not exists initially. Do not take this as a failure if it hadn't * previously appeared. @@ -358,6 +359,8 @@ teamd_dbus_vanished (GDBusConnection *dbus_connection, return; } + priv->teamd_dbus_name_owned = FALSE; + _LOGI (LOGD_TEAM, "teamd vanished from D-Bus"); teamd_cleanup (device, TRUE);