From e8982ab2a6cd8ae66d96b5b384febe3e3a6dacc9 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Wed, 7 Oct 2009 12:28:10 -0700 Subject: [PATCH 01/15] doc: update code style docs --- CONTRIBUTING | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CONTRIBUTING b/CONTRIBUTING index baf6d25905..47f93700d0 100644 --- a/CONTRIBUTING +++ b/CONTRIBUTING @@ -17,3 +17,25 @@ with #ifdef MY_DEFINE / #endif in the code. ... } +* Keep a space between the function name and the opening '('. + GOOD: g_strdup (x) + BAD: g_strdup(x) + +* C-style comments, except for FIXMEs. + GOOD: f(x); /* comment */ + BAD: f(x); // comment + + GOOD: // FIXME: juice the gooblygok + BAD: /* FIXME: juice the gooblygok */ + +* Keep assignments in the variable declaration area pretty short. + GOOD: MyObject *object; + BAD: MyObject *object = complex_and_long_init_function(arg1, arg2, arg3); + +* 80-cols is a guideline, don't make the code uncomfortable in order to fit in + less than 80 cols. + +* Constants are CAPS_WITH_UNDERSCORES and use the preprocessor. + GOOD: #define MY_CONSTANT 42 + BAD: static const unsigned myConstant = 42; + From c31b3e455414a5a7bf57d9fed1442367e9e4c917 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 8 Oct 2009 16:18:04 -0700 Subject: [PATCH 02/15] ifcfg-rh: recognize 'static' BOOTPROTO (rh #528068) --- system-settings/plugins/ifcfg-rh/reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system-settings/plugins/ifcfg-rh/reader.c b/system-settings/plugins/ifcfg-rh/reader.c index 3fee59d72a..d26145930a 100644 --- a/system-settings/plugins/ifcfg-rh/reader.c +++ b/system-settings/plugins/ifcfg-rh/reader.c @@ -668,7 +668,7 @@ make_ip4_setting (shvarFile *ifcfg, NM_SETTING_IP4_CONFIG_NEVER_DEFAULT, never_default, NULL); return NM_SETTING (s_ip4); - } else if (!g_ascii_strcasecmp (value, "none")) { + } else if (!g_ascii_strcasecmp (value, "none") || !g_ascii_strcasecmp (value, "static")) { /* Static IP */ } else if (strlen (value)) { g_set_error (error, ifcfg_plugin_error_quark (), 0, From bc653d222519965e552736b0ab30dc1b40539152 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 8 Oct 2009 23:00:01 -0700 Subject: [PATCH 03/15] vpn: clear secrets when the connection fails NM previously only cleared secrets when the VPN service daemon quit, and the service daemons are on a 10-second inactivity timer. So if the user tried to re-activate the failed VPN connection within 10 seconds the old secrets would get used, which clearly isn't what we want. Ensure that whenever the VPN connection fails or disconnects, we ask the settings service for secrets again the next time. --- src/vpn-manager/nm-vpn-connection.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index a89c7101f7..f8360dc669 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -912,6 +912,12 @@ vpn_cleanup (NMVPNConnection *connection) g_free (priv->tundev); priv->tundev = NULL; } + + /* Clear out connection secrets to ensure that the settings service + * gets asked for them next time the connection is activated. + */ + if (priv->connection) + nm_connection_clear_secrets (priv->connection); } static void From 679d548e09a0b58815ca62989627d00e7b8c7d96 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 13 Oct 2009 12:32:06 -0700 Subject: [PATCH 04/15] ifcfg-rh: add testcase for (rh #528068) --- .../tests/network-scripts/Makefile.am | 1 + .../ifcfg-test-wired-static-bootproto | 15 +++++ .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 67 ++++++++++--------- 3 files changed, 50 insertions(+), 33 deletions(-) create mode 100644 system-settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-static-bootproto diff --git a/system-settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am b/system-settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am index 206b206e15..9a2d899a14 100644 --- a/system-settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am +++ b/system-settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am @@ -2,6 +2,7 @@ EXTRA_DIST = \ ifcfg-test-minimal \ ifcfg-test-nm-controlled \ ifcfg-test-wired-static \ + ifcfg-test-wired-static-bootproto \ ifcfg-test-wired-dhcp \ ifcfg-test-wired-global-gateway \ network-test-wired-global-gateway \ diff --git a/system-settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-static-bootproto b/system-settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-static-bootproto new file mode 100644 index 0000000000..ee821503e2 --- /dev/null +++ b/system-settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-static-bootproto @@ -0,0 +1,15 @@ +# Intel Corporation 82540EP Gigabit Ethernet Controller (Mobile) +TYPE=Ethernet +DEVICE=eth0 +HWADDR=00:11:22:33:44:ee +BOOTPROTO=static +ONBOOT=yes +USERCTL=yes +IPV6INIT=no +MTU=1492 +NM_CONTROLLED=yes +DNS1=4.2.2.1 +DNS2=4.2.2.2 +IPADDR=192.168.1.5 +NETMASK=255.255.255.0 +GATEWAY=192.168.1.1 diff --git a/system-settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/system-settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 4d83145982..83ffee4bc1 100644 --- a/system-settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/system-settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -413,10 +413,8 @@ test_read_unmanaged (void) g_object_unref (connection); } -#define TEST_IFCFG_WIRED_STATIC TEST_IFCFG_DIR"/network-scripts/ifcfg-test-wired-static" - static void -test_read_wired_static (void) +test_read_wired_static (const char *file, const char *expected_id) { NMConnection *connection; NMSettingConnection *s_con; @@ -429,7 +427,6 @@ test_read_wired_static (void) const GByteArray *array; char expected_mac_address[ETH_ALEN] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0xee }; const char *tmp; - const char *expected_id = "System test-wired-static"; const char *expected_dns1 = "4.2.2.1"; const char *expected_dns2 = "4.2.2.2"; struct in_addr addr; @@ -437,7 +434,7 @@ test_read_wired_static (void) const char *expected_address1_gw = "192.168.1.1"; NMIP4Address *ip4_addr; - connection = connection_from_file (TEST_IFCFG_WIRED_STATIC, + connection = connection_from_file (file, NULL, TYPE_ETHERNET, NULL, @@ -446,46 +443,46 @@ test_read_wired_static (void) &error, &ignore_error); ASSERT (connection != NULL, - "wired-static-read", "failed to read %s: %s", TEST_IFCFG_WIRED_STATIC, error->message); + "wired-static-read", "failed to read %s: %s", file, error->message); ASSERT (nm_connection_verify (connection, &error), - "wired-static-verify", "failed to verify %s: %s", TEST_IFCFG_WIRED_STATIC, error->message); + "wired-static-verify", "failed to verify %s: %s", file, error->message); ASSERT (unmanaged == FALSE, - "wired-static-verify", "failed to verify %s: unexpected unmanaged value", TEST_IFCFG_WIRED_STATIC); + "wired-static-verify", "failed to verify %s: unexpected unmanaged value", file); /* ===== CONNECTION SETTING ===== */ s_con = NM_SETTING_CONNECTION (nm_connection_get_setting (connection, NM_TYPE_SETTING_CONNECTION)); ASSERT (s_con != NULL, "wired-static-verify-connection", "failed to verify %s: missing %s setting", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_CONNECTION_SETTING_NAME); /* ID */ tmp = nm_setting_connection_get_id (s_con); ASSERT (tmp != NULL, "wired-static-verify-connection", "failed to verify %s: missing %s / %s key", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_ID); ASSERT (strcmp (tmp, expected_id) == 0, "wired-static-verify-connection", "failed to verify %s: unexpected %s / %s key value", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_ID); /* Timestamp */ ASSERT (nm_setting_connection_get_timestamp (s_con) == 0, "wired-static-verify-connection", "failed to verify %s: unexpected %s /%s key value", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TIMESTAMP); /* Autoconnect */ ASSERT (nm_setting_connection_get_autoconnect (s_con) == TRUE, "wired-static-verify-connection", "failed to verify %s: unexpected %s /%s key value", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_AUTOCONNECT); @@ -494,30 +491,30 @@ test_read_wired_static (void) s_wired = NM_SETTING_WIRED (nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRED)); ASSERT (s_wired != NULL, "wired-static-verify-wired", "failed to verify %s: missing %s setting", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_WIRED_SETTING_NAME); /* MAC address */ array = nm_setting_wired_get_mac_address (s_wired); ASSERT (array != NULL, "wired-static-verify-wired", "failed to verify %s: missing %s / %s key", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_WIRED_SETTING_NAME, NM_SETTING_WIRED_MAC_ADDRESS); ASSERT (array->len == ETH_ALEN, "wired-static-verify-wired", "failed to verify %s: unexpected %s / %s key value length", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_WIRED_SETTING_NAME, NM_SETTING_WIRED_MAC_ADDRESS); ASSERT (memcmp (array->data, &expected_mac_address[0], sizeof (expected_mac_address)) == 0, "wired-static-verify-wired", "failed to verify %s: unexpected %s / %s key value", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_WIRED_SETTING_NAME, NM_SETTING_WIRED_MAC_ADDRESS); ASSERT (nm_setting_wired_get_mtu (s_wired) == 1492, "wired-static-verify-wired", "failed to verify %s: unexpected %s / %s key value", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_WIRED_SETTING_NAME, NM_SETTING_WIRED_MTU); @@ -526,49 +523,49 @@ test_read_wired_static (void) s_ip4 = NM_SETTING_IP4_CONFIG (nm_connection_get_setting (connection, NM_TYPE_SETTING_IP4_CONFIG)); ASSERT (s_ip4 != NULL, "wired-static-verify-ip4", "failed to verify %s: missing %s setting", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME); /* Method */ tmp = nm_setting_ip4_config_get_method (s_ip4); ASSERT (strcmp (tmp, NM_SETTING_IP4_CONFIG_METHOD_MANUAL) == 0, "wired-static-verify-ip4", "failed to verify %s: unexpected %s / %s key value", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_METHOD); /* DNS Addresses */ ASSERT (nm_setting_ip4_config_get_num_dns (s_ip4) == 2, "wired-static-verify-ip4", "failed to verify %s: unexpected %s / %s key value", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_DNS); ASSERT (inet_pton (AF_INET, expected_dns1, &addr) > 0, "wired-static-verify-ip4", "failed to verify %s: couldn't convert DNS IP address #1", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_DNS); ASSERT (nm_setting_ip4_config_get_dns (s_ip4, 0) == addr.s_addr, "wired-static-verify-ip4", "failed to verify %s: unexpected %s / %s key value #1", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_DNS); ASSERT (inet_pton (AF_INET, expected_dns2, &addr) > 0, "wired-static-verify-ip4", "failed to verify %s: couldn't convert DNS IP address #2", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_DNS); ASSERT (nm_setting_ip4_config_get_dns (s_ip4, 1) == addr.s_addr, "wired-static-verify-ip4", "failed to verify %s: unexpected %s / %s key value #2", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_DNS); ASSERT (nm_setting_ip4_config_get_num_addresses (s_ip4) == 1, "wired-static-verify-ip4", "failed to verify %s: unexpected %s / %s key value", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_DNS); @@ -576,35 +573,35 @@ test_read_wired_static (void) ip4_addr = nm_setting_ip4_config_get_address (s_ip4, 0); ASSERT (ip4_addr, "wired-static-verify-ip4", "failed to verify %s: missing IP4 address #1", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_ADDRESSES); ASSERT (nm_ip4_address_get_prefix (ip4_addr) == 24, "wired-static-verify-ip4", "failed to verify %s: unexpected IP4 address #1 prefix", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_ADDRESSES); ASSERT (inet_pton (AF_INET, expected_address1, &addr) > 0, "wired-static-verify-ip4", "failed to verify %s: couldn't convert IP address #1", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_DNS); ASSERT (nm_ip4_address_get_address (ip4_addr) == addr.s_addr, "wired-static-verify-ip4", "failed to verify %s: unexpected IP4 address #1", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_ADDRESSES); ASSERT (inet_pton (AF_INET, expected_address1_gw, &addr) > 0, "wired-static-verify-ip4", "failed to verify %s: couldn't convert IP address #1 gateway", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_ADDRESSES); ASSERT (nm_ip4_address_get_gateway (ip4_addr) == addr.s_addr, "wired-static-verify-ip4", "failed to verify %s: unexpected IP4 address #1 gateway", - TEST_IFCFG_WIRED_STATIC, + file, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_SETTING_IP4_CONFIG_ADDRESSES); @@ -5624,6 +5621,9 @@ test_write_mobile_broadband (gboolean gsm) #define TEST_IFCFG_WIFI_OPEN_SSID_LONG_HEX TEST_IFCFG_DIR"/network-scripts/ifcfg-test-wifi-open-ssid-long-hex" +#define TEST_IFCFG_WIRED_STATIC TEST_IFCFG_DIR"/network-scripts/ifcfg-test-wired-static" +#define TEST_IFCFG_WIRED_STATIC_BOOTPROTO TEST_IFCFG_DIR"/network-scripts/ifcfg-test-wired-static-bootproto" + #define DEFAULT_HEX_PSK "7d308b11df1b4243b0f78e5f3fc68cdbb9a264ed0edf4c188edf329ff5b467f0" int main (int argc, char **argv) @@ -5641,7 +5641,8 @@ int main (int argc, char **argv) /* The tests */ test_read_unmanaged (); test_read_minimal (); - test_read_wired_static (); + test_read_wired_static (TEST_IFCFG_WIRED_STATIC, "System test-wired-static"); + test_read_wired_static (TEST_IFCFG_WIRED_STATIC_BOOTPROTO, "System test-wired-static-bootproto"); test_read_wired_dhcp (); test_read_wired_global_gateway (); test_read_wired_never_default (); From 7f63b4885467d1976fb6f1a5821492e4a53c0157 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 15 Oct 2009 20:52:00 -0700 Subject: [PATCH 05/15] system-settings: fix writing connections when an earlier plugin can't (bgo #581758) The error object passed to the plugin's add-connection handler wasn't getting properly cleared if an earlier plugin had failed to write the connection and fell back to the current plugin. --- src/system-settings/nm-sysconfig-settings.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/system-settings/nm-sysconfig-settings.c b/src/system-settings/nm-sysconfig-settings.c index 32895be169..9bba219e10 100644 --- a/src/system-settings/nm-sysconfig-settings.c +++ b/src/system-settings/nm-sysconfig-settings.c @@ -594,8 +594,10 @@ add_new_connection (NMSysconfigSettings *self, connection, &tmp_error); g_clear_error (&last_error); - if (!success) + if (!success) { last_error = tmp_error; + tmp_error = NULL; + } } if (!success) From 8d6551543561263d1e6f260054004bc4bdbddd2e Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 16 Oct 2009 11:52:27 -0700 Subject: [PATCH 06/15] manager: automatically pick a base connection for VPNs If let callers pass "/" for the specific object, and NM will automatically pick the default device. --- introspection/nm-manager.xml | 11 ++++++++-- src/nm-manager.c | 40 ++++++++++++++++++++++++++++-------- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/introspection/nm-manager.xml b/introspection/nm-manager.xml index a93ee5897b..02e8bcf51a 100644 --- a/introspection/nm-manager.xml +++ b/introspection/nm-manager.xml @@ -39,12 +39,19 @@ - The device to be activated. + The object path of device to be activated for physical connections. This parameter is ignored for VPN connections, because the specific_object (if provided) specifies the device to use. - The path of a device-type-specific object this activation should use, for example a WiFi access point. + The path of a connection-type-specific object this activation should use. + This parameter is currently ignored for wired and mobile broadband connections, + and the value of "/" should be used (ie, no specific object). For WiFi + connections, pass the object path of a specific AP from the card's scan + list, or "/" to pick and AP automatically. For VPN connections, pass + the object path of an ActiveConnection object that should serve as the + "base" connection (to which the VPN connections lifetime will be tied), + or pass "/" and NM will automatically use the current default device. diff --git a/src/nm-manager.c b/src/nm-manager.c index 51a1e693ff..eb0fd2bfbd 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1996,6 +1996,7 @@ nm_manager_activate_connection (NMManager *manager, gboolean user_requested, GError **error) { + NMManagerPrivate *priv; NMDevice *device = NULL; NMSettingConnection *s_con; NMVPNConnection *vpn_connection; @@ -2006,26 +2007,47 @@ nm_manager_activate_connection (NMManager *manager, g_return_val_if_fail (error != NULL, NULL); g_return_val_if_fail (*error == NULL, NULL); + priv = NM_MANAGER_GET_PRIVATE (manager); + s_con = NM_SETTING_CONNECTION (nm_connection_get_setting (connection, NM_TYPE_SETTING_CONNECTION)); g_assert (s_con); if (!strcmp (nm_setting_connection_get_connection_type (s_con), NM_SETTING_VPN_SETTING_NAME)) { - NMActRequest *req; + NMActRequest *req = NULL; NMVPNManager *vpn_manager; /* VPN connection */ - req = nm_manager_get_act_request_by_path (manager, specific_object, &device); - if (!req) { - g_set_error (error, - NM_MANAGER_ERROR, NM_MANAGER_ERROR_CONNECTION_NOT_ACTIVE, - "%s", "Base connection for VPN connection not active."); - return NULL; + + if (specific_object) { + /* Find the specifc connection the client requested we use */ + req = nm_manager_get_act_request_by_path (manager, specific_object, &device); + if (!req) { + g_set_error (error, + NM_MANAGER_ERROR, NM_MANAGER_ERROR_CONNECTION_NOT_ACTIVE, + "%s", "Base connection for VPN connection not active."); + return NULL; + } + } else { + GSList *iter; + + /* Just find the current default connection */ + for (iter = priv->devices; iter; iter = g_slist_next (iter)) { + NMDevice *candidate = NM_DEVICE (iter->data); + NMActRequest *candidate_req; + + candidate_req = nm_device_get_act_request (candidate); + if (candidate_req && nm_act_request_get_default (candidate_req)) { + device = candidate; + req = candidate_req; + break; + } + } } - if (!device) { + if (!device || !req) { g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_DEVICE, - "%s", "Source connection had no active device."); + "%s", "Could not find source connection, or the source connection had no active device."); return NULL; } From a770a14fd22a726bfe0c165b3ee1502e80b5e41e Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sun, 18 Oct 2009 22:51:43 -0700 Subject: [PATCH 07/15] libnm-util: ensure GSM setting default values Broken by 00f945e54efc112e60ed547c9c7796a7657e4117. --- libnm-util/nm-setting-gsm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libnm-util/nm-setting-gsm.c b/libnm-util/nm-setting-gsm.c index 309c097933..f13ac872d9 100644 --- a/libnm-util/nm-setting-gsm.c +++ b/libnm-util/nm-setting-gsm.c @@ -395,9 +395,11 @@ get_property (GObject *object, guint prop_id, break; case PROP_PUK: /* deprecated */ + g_value_set_string (value, NULL); break; case PROP_BAND: /* deprecated */ + g_value_set_int (value, -1); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); From 40c91efa212219658932f36a3fb9c63ecc14aad3 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sun, 18 Oct 2009 23:36:47 -0700 Subject: [PATCH 08/15] libnm-util: fix checking for TLS and TTLS phase2 secrets Two errors here; first, need_secrets_tls() was not updated correctly for the certificate paths changes that landed recently, and would have incorrectly returned "no secrets required" for the PATH scheme. Second, an incorrect strcmp() comparison in need_secrets_phase2() meant that the wrong TTLS phase2 method would get asked if it required secrets. --- libnm-util/nm-setting-8021x.c | 87 ++-- libnm-util/tests/Makefile.am | 17 +- libnm-util/tests/test-need-secrets.c | 566 +++++++++++++++++++++++++++ 3 files changed, 646 insertions(+), 24 deletions(-) create mode 100644 libnm-util/tests/test-need-secrets.c diff --git a/libnm-util/nm-setting-8021x.c b/libnm-util/nm-setting-8021x.c index d52bd1e3e2..a137e8704a 100644 --- a/libnm-util/nm-setting-8021x.c +++ b/libnm-util/nm-setting-8021x.c @@ -2173,27 +2173,39 @@ need_secrets_sim (NMSetting8021x *self, } static gboolean -need_private_key_password (GByteArray *key, const char *password) +need_private_key_password (const GByteArray *blob, + const char *path, + const char *password) { - GError *error = NULL; - gboolean needed = TRUE; - - /* See if a private key password is needed, which basically is whether - * or not the private key is a PKCS#12 file or not, since PKCS#1 files - * are decrypted by the settings service. + /* Private key password is only un-needed if the private key scheme is BLOB, + * because BLOB keys are decrypted by the settings service. A private key + * password is required if the private key is PKCS#12 format, or if the + * private key scheme is PATH. */ - if (!crypto_is_pkcs12_data (key)) - return FALSE; + if (path) { + GByteArray *tmp; + NMCryptoKeyType key_type = NM_CRYPTO_KEY_TYPE_UNKNOWN; + NMCryptoFileFormat key_format = NM_CRYPTO_FILE_FORMAT_UNKNOWN; - if (crypto_verify_pkcs12 (key, password, &error)) - return FALSE; /* pkcs#12 validation successful */ + /* check the password */ + tmp = crypto_get_private_key (path, password, &key_type, &key_format, NULL); + if (tmp) { + /* Decrypt/verify successful; password must be OK */ + g_byte_array_free (tmp, TRUE); + return FALSE; + } + } else if (blob) { + /* Non-PKCS#12 blob-scheme keys are already decrypted by their settings + * service, thus if the private key is not PKCS#12 format, a new password + * is not required. If the PKCS#12 key can be decrypted with the given + * password, then we don't need a new password either. + */ + if (!crypto_is_pkcs12_data (blob) || crypto_verify_pkcs12 (blob, password, NULL)) + return FALSE; + } else + g_warning ("%s: unknown private key password scheme", __func__); - /* If the error was a decryption error then a password is needed */ - if (!error || g_error_matches (error, NM_CRYPTO_ERROR, NM_CRYPTO_ERR_CIPHER_DECRYPT_FAILED)) - needed = TRUE; - - g_clear_error (&error); - return needed; + return TRUE; } static void @@ -2202,16 +2214,47 @@ need_secrets_tls (NMSetting8021x *self, gboolean phase2) { NMSetting8021xPrivate *priv = NM_SETTING_802_1X_GET_PRIVATE (self); + NMSetting8021xCKScheme scheme; + const GByteArray *blob = NULL; + const char *path = NULL; if (phase2) { - if (!priv->phase2_private_key || !priv->phase2_private_key->len) + if (!priv->phase2_private_key || !priv->phase2_private_key->len) { g_ptr_array_add (secrets, NM_SETTING_802_1X_PHASE2_PRIVATE_KEY); - else if (need_private_key_password (priv->phase2_private_key, priv->phase2_private_key_password)) + return; + } + + scheme = nm_setting_802_1x_get_phase2_private_key_scheme (self); + if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) + path = nm_setting_802_1x_get_phase2_private_key_path (self); + else if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) + blob = nm_setting_802_1x_get_phase2_private_key_blob (self); + else { + g_warning ("%s: unknown phase2 private key scheme %d", __func__, scheme); + g_ptr_array_add (secrets, NM_SETTING_802_1X_PHASE2_PRIVATE_KEY); + return; + } + + if (need_private_key_password (blob, path, priv->phase2_private_key_password)) g_ptr_array_add (secrets, NM_SETTING_802_1X_PHASE2_PRIVATE_KEY_PASSWORD); } else { - if (!priv->private_key || !priv->private_key->len) + if (!priv->private_key || !priv->private_key->len) { g_ptr_array_add (secrets, NM_SETTING_802_1X_PRIVATE_KEY); - else if (need_private_key_password (priv->private_key, priv->private_key_password)) + return; + } + + scheme = nm_setting_802_1x_get_private_key_scheme (self); + if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH) + path = nm_setting_802_1x_get_private_key_path (self); + else if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) + blob = nm_setting_802_1x_get_private_key_blob (self); + else { + g_warning ("%s: unknown private key scheme %d", __func__, scheme); + g_ptr_array_add (secrets, NM_SETTING_802_1X_PRIVATE_KEY); + return; + } + + if (need_private_key_password (blob, path, priv->private_key_password)) g_ptr_array_add (secrets, NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD); } } @@ -2439,7 +2482,7 @@ need_secrets_phase2 (NMSetting8021x *self, for (i = 0; eap_methods_table[i].method; i++) { if (eap_methods_table[i].ns_func == NULL) continue; - if (strcmp (eap_methods_table[i].method, method)) { + if (!strcmp (eap_methods_table[i].method, method)) { (*eap_methods_table[i].ns_func) (self, secrets, TRUE); break; } diff --git a/libnm-util/tests/Makefile.am b/libnm-util/tests/Makefile.am index 78e94c2593..d0e3477df3 100644 --- a/libnm-util/tests/Makefile.am +++ b/libnm-util/tests/Makefile.am @@ -4,7 +4,7 @@ INCLUDES = \ -I$(top_srcdir)/include \ -I$(top_srcdir)/libnm-util -noinst_PROGRAMS = test-settings-defaults test-crypto +noinst_PROGRAMS = test-settings-defaults test-crypto test-need-secrets test_settings_defaults_SOURCES = \ test-settings-defaults.c @@ -29,11 +29,24 @@ test_crypto_LDADD = \ $(top_builddir)/libnm-util/libnm-util.la \ $(GLIB_LIBS) +test_need_secrets_SOURCES = \ + test-need-secrets.c + +test_need_secrets_CPPFLAGS = \ + -DTEST_CERT_DIR=\"$(top_srcdir)/libnm-util/tests/certs/\" \ + $(GLIB_CFLAGS) \ + $(DBUS_CFLAGS) + +test_need_secrets_LDADD = \ + $(top_builddir)/libnm-util/libnm-util.la \ + $(GLIB_LIBS) \ + $(DBUS_LIBS) if WITH_TESTS -check-local: test-settings-defaults test-crypto +check-local: test-settings-defaults test-crypto test-need-secrets $(abs_builddir)/test-settings-defaults + $(abs_builddir)/test-need-secrets # Cert with 8 bytes of tail padding $(abs_builddir)/test-crypto \ diff --git a/libnm-util/tests/test-need-secrets.c b/libnm-util/tests/test-need-secrets.c new file mode 100644 index 0000000000..517e2e01d7 --- /dev/null +++ b/libnm-util/tests/test-need-secrets.c @@ -0,0 +1,566 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ +/* + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Copyright (C) 2008 - 2009 Red Hat, Inc. + * + */ + +#include +#include +#include + +#include "nm-test-helpers.h" +#include + +#include "nm-setting-connection.h" +#include "nm-setting-wired.h" +#include "nm-setting-8021x.h" +#include "nm-setting-ip4-config.h" +#include "nm-setting-wireless-security.h" +#include "nm-setting-cdma.h" +#include "nm-setting-gsm.h" +#include "nm-setting-ppp.h" +#include "nm-setting-pppoe.h" +#include "nm-setting-vpn.h" + + +#define TEST_NEED_SECRETS_EAP_TLS_CA_CERT TEST_CERT_DIR "/test_ca_cert.pem" +#define TEST_NEED_SECRETS_EAP_TLS_CLIENT_CERT TEST_CERT_DIR "/test_key_and_cert.pem" +#define TEST_NEED_SECRETS_EAP_TLS_PRIVATE_KEY TEST_CERT_DIR "/test_key_and_cert.pem" + +static gboolean +find_hints_item (GPtrArray *hints, const char *item) +{ + int i; + + for (i = 0; i < hints->len; i++) { + if (!strcmp (item, (const char *) g_ptr_array_index (hints, i))) + return TRUE; + } + return FALSE; +} + +static NMConnection * +make_tls_connection (const char *detail, NMSetting8021xCKScheme scheme) +{ + NMConnection *connection; + NMSettingConnection *s_con; + NMSetting8021x *s_8021x; + NMSettingWired *s_wired; + NMSettingIP4Config *s_ip4; + char *uuid; + gboolean success; + GError *error = NULL; + + connection = nm_connection_new (); + ASSERT (connection != NULL, + detail, "failed to allocate new connection"); + + /* Connection setting */ + s_con = (NMSettingConnection *) nm_setting_connection_new (); + ASSERT (s_con != NULL, + detail, "failed to allocate new %s setting", + NM_SETTING_CONNECTION_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_con)); + + uuid = nm_utils_uuid_generate (); + g_object_set (s_con, + NM_SETTING_CONNECTION_ID, "Test Need TLS Secrets", + NM_SETTING_CONNECTION_UUID, uuid, + NM_SETTING_CONNECTION_AUTOCONNECT, TRUE, + NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRED_SETTING_NAME, + NULL); + g_free (uuid); + + /* Wired setting */ + s_wired = (NMSettingWired *) nm_setting_wired_new (); + ASSERT (s_wired != NULL, + detail, "failed to allocate new %s setting", + NM_SETTING_WIRED_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_wired)); + + /* Wireless security setting */ + s_8021x = (NMSetting8021x *) nm_setting_802_1x_new (); + ASSERT (s_8021x != NULL, + detail, "failed to allocate new %s setting", + NM_SETTING_802_1X_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_8021x)); + + g_object_set (s_8021x, NM_SETTING_802_1X_IDENTITY, "Bill Smith", NULL); + + nm_setting_802_1x_add_eap_method (s_8021x, "tls"); + + success = nm_setting_802_1x_set_ca_cert (s_8021x, + TEST_NEED_SECRETS_EAP_TLS_CA_CERT, + scheme, + NULL, + &error); + ASSERT (success == TRUE, + detail, "failed to set CA certificate '%s': %s", + TEST_NEED_SECRETS_EAP_TLS_CA_CERT, error->message); + + success = nm_setting_802_1x_set_client_cert (s_8021x, + TEST_NEED_SECRETS_EAP_TLS_CLIENT_CERT, + scheme, + NULL, + &error); + ASSERT (success == TRUE, + detail, "failed to set client certificate '%s': %s", + TEST_NEED_SECRETS_EAP_TLS_CLIENT_CERT, error->message); + + success = nm_setting_802_1x_set_private_key (s_8021x, + TEST_NEED_SECRETS_EAP_TLS_PRIVATE_KEY, + "test", + scheme, + NULL, + &error); + ASSERT (success == TRUE, + detail, "failed to set private key '%s': %s", + TEST_NEED_SECRETS_EAP_TLS_PRIVATE_KEY, error->message); + + /* IP4 setting */ + s_ip4 = (NMSettingIP4Config *) nm_setting_ip4_config_new (); + ASSERT (s_ip4 != NULL, + detail, "failed to allocate new %s setting", + NM_SETTING_IP4_CONFIG_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_ip4)); + + g_object_set (s_ip4, NM_SETTING_IP4_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO, NULL); + + ASSERT (nm_connection_verify (connection, &error) == TRUE, + detail, "failed to verify connection: %s", + (error && error->message) ? error->message : "(unknown)"); + + return connection; +} + +static void +test_need_tls_secrets_path (void) +{ + NMConnection *connection; + const char *setting_name; + GPtrArray *hints = NULL; + NMSetting8021x *s_8021x; + + connection = make_tls_connection ("need-tls-secrets-path-key", NM_SETTING_802_1X_CK_SCHEME_PATH); + ASSERT (connection != NULL, + "need-tls-secrets-path-key", + "error creating test connection"); + + /* Ensure we don't need any secrets since we just set up the connection */ + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name == NULL, + "need-tls-secrets-path-key", + "secrets are unexpectedly required"); + ASSERT (hints == NULL, + "need-tls-secrets-path-key", + "hints should be NULL since no secrets were required"); + + /* Connection is good; clear secrets and ensure private key is then required */ + nm_connection_clear_secrets (connection); + + hints = NULL; + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name != NULL, + "need-tls-secrets-path-key", + "unexpected secrets success"); + ASSERT (strcmp (setting_name, NM_SETTING_802_1X_SETTING_NAME) == 0, + "need-tls-secrets-path-key", + "unexpected setting secrets required"); + + ASSERT (hints != NULL, + "need-tls-secrets-path-key", + "expected returned secrets hints"); + ASSERT (find_hints_item (hints, NM_SETTING_802_1X_PRIVATE_KEY), + "need-tls-secrets-path-key", + "expected to require private key, but it wasn't"); + + g_object_unref (connection); + + /*** Just clear the private key this time ***/ + + connection = make_tls_connection ("need-tls-secrets-path-key-password", NM_SETTING_802_1X_CK_SCHEME_PATH); + ASSERT (connection != NULL, + "need-tls-secrets-path-key-password", + "error creating test connection"); + + s_8021x = (NMSetting8021x *) nm_connection_get_setting (connection, NM_TYPE_SETTING_802_1X); + ASSERT (s_8021x != NULL, + "need-tls-secrets-path-key-password", + "error getting test 802.1x setting"); + + g_object_set (G_OBJECT (s_8021x), NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD, NULL, NULL); + + hints = NULL; + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name != NULL, + "need-tls-secrets-path-key-password", + "unexpected secrets success"); + ASSERT (strcmp (setting_name, NM_SETTING_802_1X_SETTING_NAME) == 0, + "need-tls-secrets-path-key-password", + "unexpected setting secrets required"); + + ASSERT (hints != NULL, + "need-tls-secrets-path-key-password", + "expected returned secrets hints"); + ASSERT (find_hints_item (hints, NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD), + "need-tls-secrets-path-key-password", + "expected to require private key password, but it wasn't"); + + g_object_unref (connection); +} + +static void +test_need_tls_secrets_blob (void) +{ + NMConnection *connection; + const char *setting_name; + GPtrArray *hints = NULL; + NMSetting8021x *s_8021x; + + connection = make_tls_connection ("need-tls-secrets-blob-key", NM_SETTING_802_1X_CK_SCHEME_BLOB); + ASSERT (connection != NULL, + "need-tls-secrets-blob-key", + "error creating test connection"); + + /* Ensure we don't need any secrets since we just set up the connection */ + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name == NULL, + "need-tls-secrets-blob-key", + "secrets are unexpectedly required"); + ASSERT (hints == NULL, + "need-tls-secrets-blob-key", + "hints should be NULL since no secrets were required"); + + /* Connection is good; clear secrets and ensure private key is then required */ + nm_connection_clear_secrets (connection); + + hints = NULL; + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name != NULL, + "need-tls-secrets-blob-key", + "unexpected secrets success"); + ASSERT (strcmp (setting_name, NM_SETTING_802_1X_SETTING_NAME) == 0, + "need-tls-secrets-blob-key", + "unexpected setting secrets required"); + + ASSERT (hints != NULL, + "need-tls-secrets-blob-key", + "expected returned secrets hints"); + ASSERT (find_hints_item (hints, NM_SETTING_802_1X_PRIVATE_KEY), + "need-tls-secrets-blob-key", + "expected to require private key, but it wasn't"); + + g_object_unref (connection); + + /*** Just clear the private key this time ***/ + + connection = make_tls_connection ("need-tls-secrets-blob-key-password", NM_SETTING_802_1X_CK_SCHEME_BLOB); + ASSERT (connection != NULL, + "need-tls-secrets-blob-key-password", + "error creating test connection"); + + s_8021x = (NMSetting8021x *) nm_connection_get_setting (connection, NM_TYPE_SETTING_802_1X); + ASSERT (s_8021x != NULL, + "need-tls-secrets-blob-key-password", + "error getting test 802.1x setting"); + + g_object_set (G_OBJECT (s_8021x), NM_SETTING_802_1X_PRIVATE_KEY_PASSWORD, NULL, NULL); + + /* Blobs are already decrypted and don't need a password */ + hints = NULL; + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name == NULL, + "need-tls-secrets-blob-key-password", + "unexpected secrets failure"); + ASSERT (hints == NULL, + "need-tls-secrets-blob-key-password", + "hints should be NULL since no secrets were required"); + + g_object_unref (connection); +} + +static NMConnection * +make_tls_phase2_connection (const char *detail, NMSetting8021xCKScheme scheme) +{ + NMConnection *connection; + NMSettingConnection *s_con; + NMSetting8021x *s_8021x; + NMSettingWired *s_wired; + NMSettingIP4Config *s_ip4; + char *uuid; + gboolean success; + GError *error = NULL; + + connection = nm_connection_new (); + ASSERT (connection != NULL, + detail, "failed to allocate new connection"); + + /* Connection setting */ + s_con = (NMSettingConnection *) nm_setting_connection_new (); + ASSERT (s_con != NULL, + detail, "failed to allocate new %s setting", + NM_SETTING_CONNECTION_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_con)); + + uuid = nm_utils_uuid_generate (); + g_object_set (s_con, + NM_SETTING_CONNECTION_ID, "Test Need TLS Secrets", + NM_SETTING_CONNECTION_UUID, uuid, + NM_SETTING_CONNECTION_AUTOCONNECT, TRUE, + NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRED_SETTING_NAME, + NULL); + g_free (uuid); + + /* Wired setting */ + s_wired = (NMSettingWired *) nm_setting_wired_new (); + ASSERT (s_wired != NULL, + detail, "failed to allocate new %s setting", + NM_SETTING_WIRED_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_wired)); + + /* Wireless security setting */ + s_8021x = (NMSetting8021x *) nm_setting_802_1x_new (); + ASSERT (s_8021x != NULL, + detail, "failed to allocate new %s setting", + NM_SETTING_802_1X_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_8021x)); + + g_object_set (s_8021x, NM_SETTING_802_1X_ANONYMOUS_IDENTITY, "blahblah", NULL); + g_object_set (s_8021x, NM_SETTING_802_1X_IDENTITY, "Bill Smith", NULL); + + nm_setting_802_1x_add_eap_method (s_8021x, "ttls"); + g_object_set (s_8021x, NM_SETTING_802_1X_PHASE2_AUTH, "tls", NULL); + + success = nm_setting_802_1x_set_phase2_ca_cert (s_8021x, + TEST_NEED_SECRETS_EAP_TLS_CA_CERT, + scheme, + NULL, + &error); + ASSERT (success == TRUE, + detail, "failed to set phase2 CA certificate '%s': %s", + TEST_NEED_SECRETS_EAP_TLS_CA_CERT, error->message); + + success = nm_setting_802_1x_set_phase2_client_cert (s_8021x, + TEST_NEED_SECRETS_EAP_TLS_CLIENT_CERT, + scheme, + NULL, + &error); + ASSERT (success == TRUE, + detail, "failed to set phase2 client certificate '%s': %s", + TEST_NEED_SECRETS_EAP_TLS_CLIENT_CERT, error->message); + + success = nm_setting_802_1x_set_phase2_private_key (s_8021x, + TEST_NEED_SECRETS_EAP_TLS_PRIVATE_KEY, + "test", + scheme, + NULL, + &error); + ASSERT (success == TRUE, + detail, "failed to set phase2 private key '%s': %s", + TEST_NEED_SECRETS_EAP_TLS_PRIVATE_KEY, error->message); + + /* IP4 setting */ + s_ip4 = (NMSettingIP4Config *) nm_setting_ip4_config_new (); + ASSERT (s_ip4 != NULL, + detail, "failed to allocate new %s setting", + NM_SETTING_IP4_CONFIG_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_ip4)); + + g_object_set (s_ip4, NM_SETTING_IP4_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO, NULL); + + ASSERT (nm_connection_verify (connection, &error) == TRUE, + detail, "failed to verify connection: %s", + (error && error->message) ? error->message : "(unknown)"); + + return connection; +} + +static void +test_need_tls_phase2_secrets_path (void) +{ + NMConnection *connection; + const char *setting_name; + GPtrArray *hints = NULL; + NMSetting8021x *s_8021x; + + connection = make_tls_phase2_connection ("need-tls-phase2-secrets-path-key", + NM_SETTING_802_1X_CK_SCHEME_PATH); + ASSERT (connection != NULL, + "need-tls-phase2-secrets-path-key", + "error creating test connection"); + + /* Ensure we don't need any secrets since we just set up the connection */ + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name == NULL, + "need-tls-phase2-secrets-path-key", + "secrets are unexpectedly required"); + ASSERT (hints == NULL, + "need-tls-phase2-secrets-path-key", + "hints should be NULL since no secrets were required"); + + /* Connection is good; clear secrets and ensure private key is then required */ + nm_connection_clear_secrets (connection); + + hints = NULL; + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name != NULL, + "need-tls-phase2-secrets-path-key", + "unexpected secrets success"); + ASSERT (strcmp (setting_name, NM_SETTING_802_1X_SETTING_NAME) == 0, + "need-tls-phase2-secrets-path-key", + "unexpected setting secrets required"); + + ASSERT (hints != NULL, + "need-tls-phase2-secrets-path-key", + "expected returned secrets hints"); + ASSERT (find_hints_item (hints, NM_SETTING_802_1X_PHASE2_PRIVATE_KEY), + "need-tls-phase2-secrets-path-key", + "expected to require private key, but it wasn't"); + + g_object_unref (connection); + + /*** Just clear the private key this time ***/ + + connection = make_tls_phase2_connection ("need-tls-phase2-secrets-path-key-password", + NM_SETTING_802_1X_CK_SCHEME_PATH); + ASSERT (connection != NULL, + "need-tls-phase2-secrets-path-key-password", + "error creating test connection"); + + s_8021x = (NMSetting8021x *) nm_connection_get_setting (connection, NM_TYPE_SETTING_802_1X); + ASSERT (s_8021x != NULL, + "need-tls-phase2-secrets-path-key-password", + "error getting test 802.1x setting"); + + g_object_set (G_OBJECT (s_8021x), NM_SETTING_802_1X_PHASE2_PRIVATE_KEY_PASSWORD, NULL, NULL); + + hints = NULL; + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name != NULL, + "need-tls-phase2-secrets-path-key-password", + "unexpected secrets success"); + ASSERT (strcmp (setting_name, NM_SETTING_802_1X_SETTING_NAME) == 0, + "need-tls-phase2-secrets-path-key-password", + "unexpected setting secrets required"); + + ASSERT (hints != NULL, + "need-tls-phase2-secrets-path-key-password", + "expected returned secrets hints"); + ASSERT (find_hints_item (hints, NM_SETTING_802_1X_PHASE2_PRIVATE_KEY_PASSWORD), + "need-tls-phase2-secrets-path-key-password", + "expected to require private key password, but it wasn't"); + + g_object_unref (connection); +} + +static void +test_need_tls_phase2_secrets_blob (void) +{ + NMConnection *connection; + const char *setting_name; + GPtrArray *hints = NULL; + NMSetting8021x *s_8021x; + + connection = make_tls_phase2_connection ("need-tls-phase2-secrets-blob-key", + NM_SETTING_802_1X_CK_SCHEME_BLOB); + ASSERT (connection != NULL, + "need-tls-phase2-secrets-blob-key", + "error creating test connection"); + + /* Ensure we don't need any secrets since we just set up the connection */ + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name == NULL, + "need-tls-phase2-secrets-blob-key", + "secrets are unexpectedly required"); + ASSERT (hints == NULL, + "need-tls-phase2-secrets-blob-key", + "hints should be NULL since no secrets were required"); + + /* Connection is good; clear secrets and ensure private key is then required */ + nm_connection_clear_secrets (connection); + + hints = NULL; + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name != NULL, + "need-tls-phase2-secrets-blob-key", + "unexpected secrets success"); + ASSERT (strcmp (setting_name, NM_SETTING_802_1X_SETTING_NAME) == 0, + "need-tls-phase2-secrets-blob-key", + "unexpected setting secrets required"); + + ASSERT (hints != NULL, + "need-tls-phase2-secrets-blob-key", + "expected returned secrets hints"); + ASSERT (find_hints_item (hints, NM_SETTING_802_1X_PHASE2_PRIVATE_KEY), + "need-tls-phase2-secrets-blob-key", + "expected to require private key, but it wasn't"); + + g_object_unref (connection); + + /*** Just clear the private key this time ***/ + + connection = make_tls_phase2_connection ("need-tls-phase2-secrets-blob-key-password", + NM_SETTING_802_1X_CK_SCHEME_BLOB); + ASSERT (connection != NULL, + "need-tls-phase2-secrets-blob-key-password", + "error creating test connection"); + + s_8021x = (NMSetting8021x *) nm_connection_get_setting (connection, NM_TYPE_SETTING_802_1X); + ASSERT (s_8021x != NULL, + "need-tls-phase2-secrets-blob-key-password", + "error getting test 802.1x setting"); + + g_object_set (G_OBJECT (s_8021x), NM_SETTING_802_1X_PHASE2_PRIVATE_KEY_PASSWORD, NULL, NULL); + + /* Blobs are already decrypted and don't need a password */ + hints = NULL; + setting_name = nm_connection_need_secrets (connection, &hints); + ASSERT (setting_name == NULL, + "need-tls-phase2-secrets-blob-key-password", + "unexpected secrets failure"); + ASSERT (hints == NULL, + "need-tls-phase2-secrets-blob-key-password", + "hints should be NULL since no secrets were required"); + + g_object_unref (connection); +} + +int main (int argc, char **argv) +{ + GError *error = NULL; + DBusGConnection *bus; + char *base; + + g_type_init (); + bus = dbus_g_bus_get (DBUS_BUS_SESSION, NULL); + + if (!nm_utils_init (&error)) + FAIL ("nm-utils-init", "failed to initialize libnm-util: %s", error->message); + + /* The tests */ + test_need_tls_secrets_path (); + test_need_tls_secrets_blob (); + test_need_tls_phase2_secrets_path (); + test_need_tls_phase2_secrets_blob (); + + base = g_path_get_basename (argv[0]); + fprintf (stdout, "%s: SUCCESS\n", base); + g_free (base); + return 0; +} + From 4eb2346b674de1f862caa55e5c082d0ed18043f3 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 19 Oct 2009 10:20:25 -0700 Subject: [PATCH 09/15] ifcfg-rh: fix writing LEAP connections --- .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 135 ++++++++++++++++++ system-settings/plugins/ifcfg-rh/writer.c | 2 +- 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/system-settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/system-settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 83ffee4bc1..cfbe0d6872 100644 --- a/system-settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/system-settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -4207,6 +4207,140 @@ test_write_wifi_wep_adhoc (void) g_object_unref (reread); } +static void +test_write_wifi_leap (void) +{ + NMConnection *connection; + NMConnection *reread; + NMSettingConnection *s_con; + NMSettingWireless *s_wifi; + NMSettingWirelessSecurity *s_wsec; + NMSettingIP4Config *s_ip4; + char *uuid; + gboolean success; + GError *error = NULL; + char *testfile = NULL; + char *unmanaged = NULL; + char *keyfile = NULL; + gboolean ignore_error = FALSE; + GByteArray *ssid; + const unsigned char ssid_data[] = "blahblah"; + struct stat statbuf; + + connection = nm_connection_new (); + ASSERT (connection != NULL, + "wifi-leap-write", "failed to allocate new connection"); + + /* Connection setting */ + s_con = (NMSettingConnection *) nm_setting_connection_new (); + ASSERT (s_con != NULL, + "wifi-leap-write", "failed to allocate new %s setting", + NM_SETTING_CONNECTION_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_con)); + + uuid = nm_utils_uuid_generate (); + g_object_set (s_con, + NM_SETTING_CONNECTION_ID, "Test Write Wifi LEAP", + NM_SETTING_CONNECTION_UUID, uuid, + NM_SETTING_CONNECTION_AUTOCONNECT, TRUE, + NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRELESS_SETTING_NAME, + NULL); + g_free (uuid); + + /* Wifi setting */ + s_wifi = (NMSettingWireless *) nm_setting_wireless_new (); + ASSERT (s_wifi != NULL, + "wifi-leap-write", "failed to allocate new %s setting", + NM_SETTING_WIRELESS_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_wifi)); + + ssid = g_byte_array_sized_new (sizeof (ssid_data)); + g_byte_array_append (ssid, ssid_data, sizeof (ssid_data)); + + g_object_set (s_wifi, + NM_SETTING_WIRELESS_SSID, ssid, + NM_SETTING_WIRELESS_MODE, "infrastructure", + NM_SETTING_WIRELESS_SEC, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME, + NULL); + + g_byte_array_free (ssid, TRUE); + + /* Wireless security setting */ + s_wsec = (NMSettingWirelessSecurity *) nm_setting_wireless_security_new (); + ASSERT (s_wsec != NULL, + "wifi-leap-write", "failed to allocate new %s setting", + NM_SETTING_WIRELESS_SECURITY_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_wsec)); + + g_object_set (s_wsec, + NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "ieee8021x", + NM_SETTING_WIRELESS_SECURITY_AUTH_ALG, "leap", + NM_SETTING_WIRELESS_SECURITY_LEAP_USERNAME, "Bill Smith", + NM_SETTING_WIRELESS_SECURITY_LEAP_PASSWORD, "foobar22", + NULL); + + /* IP4 setting */ + s_ip4 = (NMSettingIP4Config *) nm_setting_ip4_config_new (); + ASSERT (s_ip4 != NULL, + "wifi-leap-write", "failed to allocate new %s setting", + NM_SETTING_IP4_CONFIG_SETTING_NAME); + nm_connection_add_setting (connection, NM_SETTING (s_ip4)); + + g_object_set (s_ip4, NM_SETTING_IP4_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO, NULL); + + ASSERT (nm_connection_verify (connection, &error) == TRUE, + "wifi-leap-write", "failed to verify connection: %s", + (error && error->message) ? error->message : "(unknown)"); + + /* Save the ifcfg */ + success = writer_new_connection (connection, + TEST_SCRATCH_DIR "/network-scripts/", + &testfile, + &error); + ASSERT (success == TRUE, + "wifi-leap-write", "failed to write connection to disk: %s", + (error && error->message) ? error->message : "(unknown)"); + + ASSERT (testfile != NULL, + "wifi-leap-write", "didn't get ifcfg file path back after writing connection"); + + /* re-read the connection for comparison */ + reread = connection_from_file (testfile, + NULL, + TYPE_WIRELESS, + NULL, + &unmanaged, + &keyfile, + &error, + &ignore_error); + unlink (testfile); + + ASSERT (keyfile != NULL, + "wifi-leap-write-reread", "expected keyfile for '%s'", testfile); + + ASSERT (stat (keyfile, &statbuf) == 0, + "wifi-leap-write-reread", "couldn't stat() '%s'", keyfile); + ASSERT (S_ISREG (statbuf.st_mode), + "wifi-leap-write-reread", "keyfile '%s' wasn't a normal file", keyfile); + ASSERT ((statbuf.st_mode & 0077) == 0, + "wifi-leap-write-reread", "keyfile '%s' wasn't readable only by its owner", keyfile); + + unlink (keyfile); + + ASSERT (reread != NULL, + "wifi-leap-write-reread", "failed to read %s: %s", testfile, error->message); + + ASSERT (nm_connection_verify (reread, &error), + "wifi-leap-write-reread-verify", "failed to verify %s: %s", testfile, error->message); + + ASSERT (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT) == TRUE, + "wifi-leap-write", "written and re-read connection weren't the same."); + + g_free (testfile); + g_object_unref (connection); + g_object_unref (reread); +} + static void test_write_wifi_wpa_psk (const char *name, const char *test_name, @@ -5671,6 +5805,7 @@ int main (int argc, char **argv) test_write_wifi_open_hex_ssid (); test_write_wifi_wep (); test_write_wifi_wep_adhoc (); + test_write_wifi_leap (); test_write_wifi_wpa_psk ("Test Write Wifi WPA PSK", "wifi-wpa-psk-write", FALSE, diff --git a/system-settings/plugins/ifcfg-rh/writer.c b/system-settings/plugins/ifcfg-rh/writer.c index 11ab2d182d..1ba9455837 100644 --- a/system-settings/plugins/ifcfg-rh/writer.c +++ b/system-settings/plugins/ifcfg-rh/writer.c @@ -575,7 +575,7 @@ write_wireless_security_setting (NMConnection *connection, svSetValue (ifcfg, "SECURITYMODE", "open", FALSE); else if (!strcmp (auth_alg, "leap")) { svSetValue (ifcfg, "SECURITYMODE", "leap", FALSE); - svSetValue (ifcfg, "IEEE_8021X_USERNAME", + svSetValue (ifcfg, "IEEE_8021X_IDENTITY", nm_setting_wireless_security_get_leap_username (s_wsec), FALSE); set_secret (ifcfg, "IEEE_8021X_PASSWORD", From 442dc676ef94705aa1b21f08ac8e271bb17f09c6 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 19 Oct 2009 14:05:36 -0700 Subject: [PATCH 10/15] ip6: use device's IP interface, not its physical interface --- src/nm-device.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/nm-device.c b/src/nm-device.c index 0d3481cb37..1a7b8f8ae2 100644 --- a/src/nm-device.c +++ b/src/nm-device.c @@ -532,7 +532,7 @@ ip6_addrconf_complete (NMIP6Manager *ip6_manager, NMDevice *self = NM_DEVICE (user_data); NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - if (strcmp (nm_device_get_iface (self), iface) != 0) + if (strcmp (nm_device_get_ip_iface (self), iface) != 0) return; if (!nm_device_get_act_request (self)) return; @@ -553,7 +553,7 @@ ip6_config_changed (NMIP6Manager *ip6_manager, { NMDevice *self = NM_DEVICE (user_data); - if (strcmp (nm_device_get_iface (self), iface) != 0) + if (strcmp (nm_device_get_ip_iface (self), iface) != 0) return; if (!nm_device_get_act_request (self)) return; @@ -567,7 +567,7 @@ nm_device_setup_ip6 (NMDevice *self) NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMActRequest *req; NMConnection *connection; - const char *iface, *method = NULL; + const char *ip_iface, *method = NULL; NMSettingIP6Config *s_ip6; req = nm_device_get_act_request (self); @@ -598,8 +598,8 @@ nm_device_setup_ip6 (NMDevice *self) priv->ip6_waiting_for_config = FALSE; - iface = nm_device_get_iface (self); - nm_ip6_manager_prepare_interface (priv->ip6_manager, iface, s_ip6); + ip_iface = nm_device_get_ip_iface (self); + nm_ip6_manager_prepare_interface (priv->ip6_manager, ip_iface, s_ip6); } static void @@ -1139,7 +1139,7 @@ static NMActStageReturn real_act_stage3_ip6_config_start (NMDevice *self, NMDeviceStateReason *reason) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - const char *iface = nm_device_get_iface (self); + const char *ip_iface = nm_device_get_ip_iface (self); g_return_val_if_fail (reason != NULL, NM_ACT_STAGE_RETURN_FAILURE); @@ -1150,7 +1150,7 @@ real_act_stage3_ip6_config_start (NMDevice *self, NMDeviceStateReason *reason) return NM_ACT_STAGE_RETURN_SUCCESS; priv->ip6_waiting_for_config = TRUE; - nm_ip6_manager_begin_addrconf (priv->ip6_manager, iface); + nm_ip6_manager_begin_addrconf (priv->ip6_manager, ip_iface); return NM_ACT_STAGE_RETURN_POSTPONE; } From 23fec8dc72e8c90505f3d2eae228f2ffd3284955 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 19 Oct 2009 15:38:21 -0700 Subject: [PATCH 11/15] ip6: save the accept_ra value and re-set it when the device is deactivated --- src/ip6-manager/nm-ip6-manager.c | 53 +++++++++++++++++++++++++++++--- src/nm-device.c | 27 ++++++++-------- 2 files changed, 62 insertions(+), 18 deletions(-) diff --git a/src/ip6-manager/nm-ip6-manager.c b/src/ip6-manager/nm-ip6-manager.c index 7f280cec1a..8908c86048 100644 --- a/src/ip6-manager/nm-ip6-manager.c +++ b/src/ip6-manager/nm-ip6-manager.c @@ -18,6 +18,7 @@ * Copyright (C) 2009 Red Hat, Inc. */ +#include #include #include @@ -68,6 +69,10 @@ typedef struct { char *iface; int index; + char *accept_ra_path; + gboolean accept_ra_save_valid; + guint32 accept_ra_save; + guint finish_addrconf_id; guint config_changed_id; @@ -179,6 +184,14 @@ nm_ip6_manager_class_init (NMIP6ManagerClass *manager_class) static void nm_ip6_device_destroy (NMIP6Device *device) { + g_return_if_fail (device != NULL); + + /* reset the saved RA value */ + if (device->accept_ra_save_valid) { + nm_utils_do_sysctl (device->accept_ra_path, + device->accept_ra_save ? "1\n" : "0\n"); + } + if (device->finish_addrconf_id) g_source_remove (device->finish_addrconf_id); if (device->config_changed_id) @@ -189,6 +202,7 @@ nm_ip6_device_destroy (NMIP6Device *device) if (device->rdnss_timeout_id) g_source_remove (device->rdnss_timeout_id); + g_free (device->accept_ra_path); g_slice_free (NMIP6Device, device); } @@ -600,6 +614,8 @@ nm_ip6_device_new (NMIP6Manager *manager, const char *iface) { NMIP6ManagerPrivate *priv = NM_IP6_MANAGER_GET_PRIVATE (manager); NMIP6Device *device; + GError *error = NULL; + char *contents = NULL; device = g_slice_new0 (NMIP6Device); if (!device) { @@ -616,6 +632,14 @@ nm_ip6_device_new (NMIP6Manager *manager, const char *iface) } device->index = nm_netlink_iface_to_index (iface); + device->accept_ra_path = g_strdup_printf ("/proc/sys/net/ipv6/conf/%s/accept_ra", iface); + if (!device->accept_ra_path) { + nm_warning ("%s: Out of memory creating IP6 addrconf object " + "property 'accept_ra_path'.", + iface); + goto error; + } + device->manager = manager; device->rdnss_servers = g_array_new (FALSE, FALSE, sizeof (NMIP6RDNSS)); @@ -623,6 +647,27 @@ nm_ip6_device_new (NMIP6Manager *manager, const char *iface) g_hash_table_replace (priv->devices_by_iface, device->iface, device); g_hash_table_replace (priv->devices_by_index, GINT_TO_POINTER (device->index), device); + /* Grab the original value of "accept_ra" so we can restore it when the + * device is taken down. + */ + if (!g_file_get_contents (device->accept_ra_path, &contents, NULL, &error)) { + nm_warning ("%s: error reading %s: (%d) %s", + iface, device->accept_ra_path, + error ? error->code : -1, + error && error->message ? error->message : "(unknown)"); + g_clear_error (&error); + } else { + long int tmp; + + errno = 0; + tmp = strtol (contents, NULL, 10); + if ((errno == 0) && (tmp == 0 || tmp == 1)) { + device->accept_ra_save = (guint32) tmp; + device->accept_ra_save_valid = TRUE; + } + g_free (contents); + } + return device; error: @@ -638,7 +683,6 @@ nm_ip6_manager_prepare_interface (NMIP6Manager *manager, NMIP6ManagerPrivate *priv; NMIP6Device *device; const char *method = NULL; - char *sysctl_path; g_return_if_fail (NM_IS_IP6_MANAGER (manager)); g_return_if_fail (iface != NULL); @@ -662,10 +706,9 @@ nm_ip6_manager_prepare_interface (NMIP6Manager *manager, strcmp (iface, "all") != 0 && strcmp (iface, "default") != 0); - sysctl_path = g_strdup_printf ("/proc/sys/net/ipv6/conf/%s/accept_ra", iface); - nm_utils_do_sysctl (sysctl_path, - device->target_state >= NM_IP6_DEVICE_GOT_ADDRESS ? "1\n" : "0\n"); - g_free (sysctl_path); + /* Turn router advertisement acceptance on or off... */ + nm_utils_do_sysctl (device->accept_ra_path, + device->target_state >= NM_IP6_DEVICE_GOT_ADDRESS ? "1\n" : "0\n"); } void diff --git a/src/nm-device.c b/src/nm-device.c index 1a7b8f8ae2..6c0f99ca47 100644 --- a/src/nm-device.c +++ b/src/nm-device.c @@ -607,21 +607,22 @@ nm_device_cleanup_ip6 (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - if (priv->ip6_manager) { - if (priv->ip6_addrconf_sigid) { - g_signal_handler_disconnect (priv->ip6_manager, - priv->ip6_addrconf_sigid); - priv->ip6_addrconf_sigid = 0; - } - if (priv->ip6_config_changed_sigid) { - g_signal_handler_disconnect (priv->ip6_manager, - priv->ip6_config_changed_sigid); - priv->ip6_config_changed_sigid = 0; - } + if (!priv->ip6_manager) + return; - g_object_unref (priv->ip6_manager); - priv->ip6_manager = NULL; + if (priv->ip6_addrconf_sigid) { + g_signal_handler_disconnect (priv->ip6_manager, + priv->ip6_addrconf_sigid); + priv->ip6_addrconf_sigid = 0; } + if (priv->ip6_config_changed_sigid) { + g_signal_handler_disconnect (priv->ip6_manager, + priv->ip6_config_changed_sigid); + priv->ip6_config_changed_sigid = 0; + } + + g_object_unref (priv->ip6_manager); + priv->ip6_manager = NULL; } /* From 4b73cf2421cfd68b38066c1e78dce48000f6fd6b Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 20 Oct 2009 11:23:10 -0700 Subject: [PATCH 12/15] ipv6: fix incorrect address config signal emission device->want_signal was never set to TRUE when addrconf was started, causing random netlink events (say for link-local address addition or removal) to trigger the config-changed signal from nm_ip6_device_sync_from_netlink() at the wrong time. This would cause IPv6 address configuration to look like it succeeded, when in fact the config timeout was still in-force. Thus device activation would proceed if IPv4 was enabled, but a few seconds later the device would be deactivated due to the still active IPv6 timeout. So fix that and clarify when the events from the IPv6 manager happen, and what the want_signal variable is really for. --- src/ip6-manager/nm-ip6-manager.c | 10 ++++++---- src/ip6-manager/nm-ip6-manager.h | 8 ++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/ip6-manager/nm-ip6-manager.c b/src/ip6-manager/nm-ip6-manager.c index 8908c86048..f41169cb15 100644 --- a/src/ip6-manager/nm-ip6-manager.c +++ b/src/ip6-manager/nm-ip6-manager.c @@ -78,7 +78,7 @@ typedef struct { NMIP6DeviceState state; NMIP6DeviceState target_state; - gboolean want_signal; + gboolean addrconf_complete; GArray *rdnss_servers; guint rdnss_timeout_id; @@ -241,7 +241,7 @@ finish_addrconf (gpointer user_data) char *iface_copy; device->finish_addrconf_id = 0; - device->want_signal = FALSE; + device->addrconf_complete = TRUE; if (device->state >= device->target_state) { g_signal_emit (manager, signals[ADDRCONF_COMPLETE], 0, @@ -368,7 +368,7 @@ nm_ip6_device_sync_from_netlink (NMIP6Device *device, gboolean config_changed) // if (flags & (IF_RA_MANAGED | IF_RA_OTHERCONF)) // device->need_dhcp = TRUE; - if (device->want_signal) { + if (!device->addrconf_complete) { if (device->state >= device->target_state || device->state == NM_IP6_DEVICE_GOT_ROUTER_ADVERTISEMENT) { /* device->finish_addrconf_id may currently be a timeout @@ -466,7 +466,7 @@ process_prefix (NMIP6Manager *manager, struct nl_msg *msg) pmsg = (struct prefixmsg *) NLMSG_DATA (nlmsg_hdr (msg)); device = nm_ip6_manager_get_device (manager, pmsg->prefix_ifindex); - if (!device || !device->want_signal) + if (!device || device->addrconf_complete) return NULL; return device; @@ -728,6 +728,8 @@ nm_ip6_manager_begin_addrconf (NMIP6Manager *manager, nm_info ("Activation (%s) Beginning IP6 addrconf.", iface); + device->addrconf_complete = FALSE; + /* Set up a timeout on the transaction to kill it after the timeout */ device->finish_addrconf_id = g_timeout_add_seconds (NM_IP6_TIMEOUT, finish_addrconf, diff --git a/src/ip6-manager/nm-ip6-manager.h b/src/ip6-manager/nm-ip6-manager.h index 2c79a454a1..33b2b9893a 100644 --- a/src/ip6-manager/nm-ip6-manager.h +++ b/src/ip6-manager/nm-ip6-manager.h @@ -43,8 +43,16 @@ typedef struct { GObjectClass parent; /* Signals */ + + /* addrconf_complete is emitted only during initial configuration to indicate + * that the initial configuration is complete. + */ void (*addrconf_complete) (NMIP6Manager *manager, char *iface, gboolean success); + /* config_changed gets emitted only *after* initial configuration is + * complete; it's like DHCP renew and indicates that the existing config + * of the interface has changed. + */ void (*config_changed) (NMIP6Manager *manager, char *iface); } NMIP6ManagerClass; From 9e356dab83591286ff0e69f238dfb385c3622a76 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 20 Oct 2009 11:36:47 -0700 Subject: [PATCH 13/15] libnm-glib: fix warning tearing down connections GLib-CRITICAL **: g_hash_table_iter_next: assertion `ri->version == ri->hash_table->version' failed --- libnm-glib/nm-remote-settings.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/libnm-glib/nm-remote-settings.c b/libnm-glib/nm-remote-settings.c index d7be8fbb04..55ae0f3a14 100644 --- a/libnm-glib/nm-remote-settings.c +++ b/libnm-glib/nm-remote-settings.c @@ -268,10 +268,20 @@ remove_connections (gpointer user_data) NMRemoteSettingsPrivate *priv = NM_REMOTE_SETTINGS_GET_PRIVATE (self); GHashTableIter iter; gpointer value; + GSList *list = NULL, *list_iter; + /* Build up the list of connections; we can't emit "removed" during hash + * table iteration because emission of the "removed" signal may trigger code + * that explicitly removes the the connection from the hash table somewhere + * else. + */ g_hash_table_iter_init (&iter, priv->connections); while (g_hash_table_iter_next (&iter, NULL, &value)) - g_signal_emit_by_name (NM_REMOTE_CONNECTION (value), "removed"); + list = g_slist_prepend (list, NM_REMOTE_CONNECTION (value)); + + for (list_iter = list; list_iter; list_iter = g_slist_next (list_iter)) + g_signal_emit_by_name (NM_REMOTE_CONNECTION (list_iter->data), "removed"); + g_slist_free (list); g_hash_table_remove_all (priv->connections); return FALSE; From 3d194df94af66097c8255dd4df13f9494cde5f19 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 20 Oct 2009 11:48:23 -0700 Subject: [PATCH 14/15] libnm-glib: warning cleanups --- libnm-glib/nm-object.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/libnm-glib/nm-object.c b/libnm-glib/nm-object.c index 7e9bc2fa0b..b0d2236e81 100644 --- a/libnm-glib/nm-object.c +++ b/libnm-glib/nm-object.c @@ -83,7 +83,7 @@ constructor (GType type, priv = NM_OBJECT_GET_PRIVATE (object); if (priv->connection == NULL || priv->path == NULL) { - g_warning ("Connection or path not received."); + g_warning ("%s: bus connection and path required.", __func__); g_object_unref (object); return NULL; } @@ -331,7 +331,10 @@ handle_property_changed (gpointer key, gpointer data, gpointer user_data) prop_name = wincaps_to_dash ((char *) key); pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (G_OBJECT (self)), prop_name); if (!pspec) { - g_warning ("Property '%s' change detected but couldn't be found on the object.", prop_name); + g_warning ("%s: property '%s' changed but wasn't defined by object type %s.", + __func__, + prop_name, + G_OBJECT_TYPE_NAME (self)); goto out; } @@ -350,8 +353,12 @@ handle_property_changed (gpointer key, gpointer data, gpointer user_data) #if DEBUG g_warning ("Property '%s' unhandled.", prop_name); #endif - } else if (!success) - g_warning ("Property '%s' could not be set due to errors.", prop_name); + } else if (!success) { + g_warning ("%s: failed to update property '%s' of object type %s.", + __func__, + prop_name, + G_OBJECT_TYPE_NAME (self)); + } out: g_free (prop_name); @@ -486,11 +493,17 @@ _nm_object_get_property (NMObject *object, G_TYPE_INVALID, G_TYPE_VALUE, value, G_TYPE_INVALID)) { - g_warning ("%s: Error getting '%s' for %s: %s\n", - __func__, - prop_name, - nm_object_get_path (object), - err->message); + /* Don't warn about D-Bus no reply/timeout errors; it's mostly noise and + * happens for example when NM quits and the applet is still running. + */ + if (err->code != DBUS_GERROR_NO_REPLY) { + g_warning ("%s: Error getting '%s' for %s: (%d) %s\n", + __func__, + prop_name, + nm_object_get_path (object), + err->code, + err->message); + } g_error_free (err); return FALSE; } From c9d2d977dd1ec6b882a7975c8c6f160096e2e9ce Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 20 Oct 2009 12:10:30 -0700 Subject: [PATCH 15/15] libnm-glib: tighter warning print checks Should be checking for dbus-glib errors of the right type, instead of any error code (dbus-glib or not) that happens to be 4. --- libnm-glib/nm-object.c | 2 +- libnm-glib/nm-remote-settings-system.c | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/libnm-glib/nm-object.c b/libnm-glib/nm-object.c index b0d2236e81..94bda26e56 100644 --- a/libnm-glib/nm-object.c +++ b/libnm-glib/nm-object.c @@ -496,7 +496,7 @@ _nm_object_get_property (NMObject *object, /* Don't warn about D-Bus no reply/timeout errors; it's mostly noise and * happens for example when NM quits and the applet is still running. */ - if (err->code != DBUS_GERROR_NO_REPLY) { + if (!(err->domain == DBUS_GERROR && err->code == DBUS_GERROR_NO_REPLY)) { g_warning ("%s: Error getting '%s' for %s: (%d) %s\n", __func__, prop_name, diff --git a/libnm-glib/nm-remote-settings-system.c b/libnm-glib/nm-remote-settings-system.c index 02aaff4cde..4e30fda800 100644 --- a/libnm-glib/nm-remote-settings-system.c +++ b/libnm-glib/nm-remote-settings-system.c @@ -90,10 +90,15 @@ get_all_cb (DBusGProxy *proxy, if (!dbus_g_proxy_end_call (proxy, call, &error, DBUS_TYPE_G_MAP_OF_VARIANT, &props, G_TYPE_INVALID)) { - g_warning ("%s: couldn't retrieve system settings properties: (%d) %s.", - __func__, - error ? error->code : -1, - (error && error->message) ? error->message : "(unknown)"); + /* Don't warn when the call times out because the settings service can't + * be activated or whatever. + */ + if (!(error->domain == DBUS_GERROR && error->code == DBUS_GERROR_NO_REPLY)) { + g_warning ("%s: couldn't retrieve system settings properties: (%d) %s.", + __func__, + error ? error->code : -1, + (error && error->message) ? error->message : "(unknown)"); + } g_clear_error (&error); return; }