From 29e00fde5889f415d6cde2474e0277a27bb4cbba Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 8 Jan 2014 17:53:04 -0600 Subject: [PATCH 1/6] core: add PropertiesChanged signals to IP4 and IP6 config objects Now that the objects get replaced when IP configuration changes instead of being destroyed and a new one created, they need PropertiesChanged signals. (noticed as a result of auditing all exported D-Bus objects) --- introspection/nm-ip4-config.xml | 8 ++++++++ introspection/nm-ip6-config.xml | 8 ++++++++ src/nm-ip4-config.c | 4 +++- src/nm-ip6-config.c | 5 +++-- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/introspection/nm-ip4-config.xml b/introspection/nm-ip4-config.xml index f21a2e4a0e..6a8750b7f6 100644 --- a/introspection/nm-ip4-config.xml +++ b/introspection/nm-ip4-config.xml @@ -30,6 +30,14 @@ The Windows Internet Name Service servers associated with the connection. Each address is in network byte order. + + + + + A dictionary mapping property names to variant boxed values + + + diff --git a/introspection/nm-ip6-config.xml b/introspection/nm-ip6-config.xml index dcec871391..55c519e701 100644 --- a/introspection/nm-ip6-config.xml +++ b/introspection/nm-ip6-config.xml @@ -20,6 +20,14 @@ A list of dns searches. + + + + + A dictionary mapping property names to variant boxed values + + + diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index c7c00a41d7..b9ce0cbadb 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -1688,5 +1688,7 @@ nm_ip4_config_class_init (NMIP4ConfigClass *config_class) g_object_class_install_properties (object_class, LAST_PROP, obj_properties); - dbus_g_object_type_install_info (G_TYPE_FROM_CLASS (config_class), &dbus_glib_nm_ip4_config_object_info); + nm_dbus_manager_register_exported_type (nm_dbus_manager_get (), + G_TYPE_FROM_CLASS (config_class), + &dbus_glib_nm_ip4_config_object_info); } diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index 6e9f1f2558..178f0b51b1 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -1509,6 +1509,7 @@ nm_ip6_config_class_init (NMIP6ConfigClass *config_class) g_object_class_install_properties (object_class, LAST_PROP, obj_properties); - dbus_g_object_type_install_info (G_TYPE_FROM_CLASS (config_class), - &dbus_glib_nm_ip6_config_object_info); + nm_dbus_manager_register_exported_type (nm_dbus_manager_get (), + G_TYPE_FROM_CLASS (config_class), + &dbus_glib_nm_ip6_config_object_info); } From 8ab8990938b995a8b49e995ff844fa359c9b4443 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 9 Jan 2014 11:44:44 -0600 Subject: [PATCH 2/6] settings: return error from GetConnectionByUuid() if caller not in ACL While this function only returns the path of the requested connection (the actual settings are always protected), callers that aren't in the connection's ACL still probably shouldn't get that, if only to be pedantic. --- introspection/nm-settings.xml | 1 + src/settings/nm-settings.c | 56 ++++++++++++++++++++++++++--------- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/introspection/nm-settings.xml b/introspection/nm-settings.xml index 7e02db7216..e36f206db9 100644 --- a/introspection/nm-settings.xml +++ b/introspection/nm-settings.xml @@ -23,6 +23,7 @@ Retrieve the object path of a connection, given that connection's UUID. + The UUID to find the connection object path for. diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 42c8b95f68..a9bb90599a 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -91,10 +91,10 @@ static gboolean impl_settings_list_connections (NMSettings *self, GPtrArray **connections, GError **error); -static gboolean impl_settings_get_connection_by_uuid (NMSettings *self, - const char *uuid, - char **out_object_path, - GError **error); +static void impl_settings_get_connection_by_uuid (NMSettings *self, + const char *uuid, + char **out_object_path, + DBusGMethodInvocation *context); static void impl_settings_add_connection (NMSettings *self, GHashTable *settings, @@ -268,25 +268,53 @@ nm_settings_get_connection_by_uuid (NMSettings *self, const char *uuid) return NULL; } -static gboolean +static void impl_settings_get_connection_by_uuid (NMSettings *self, const char *uuid, char **out_object_path, - GError **error) + DBusGMethodInvocation *context) { NMSettingsConnection *connection = NULL; + NMAuthSubject *subject; + GError *error = NULL; + char *error_desc = NULL; connection = nm_settings_get_connection_by_uuid (self, uuid); - if (connection) - *out_object_path = g_strdup (nm_connection_get_path (NM_CONNECTION (connection))); - else { - g_set_error_literal (error, - NM_SETTINGS_ERROR, - NM_SETTINGS_ERROR_INVALID_CONNECTION, - "No connection with the UUID was found."); + if (!connection) { + error = g_error_new_literal (NM_SETTINGS_ERROR, + NM_SETTINGS_ERROR_INVALID_CONNECTION, + "No connection with the UUID was found."); + goto error; } - return !!connection; + subject = nm_auth_subject_new_from_context (context); + if (!subject) { + error = g_error_new_literal (NM_SETTINGS_ERROR, + NM_SETTINGS_ERROR_PERMISSION_DENIED, + "Unable to determine UID of request."); + goto error; + } + + if (!nm_auth_uid_in_acl (NM_CONNECTION (connection), + nm_session_monitor_get (), + nm_auth_subject_get_uid (subject), + &error_desc)) { + error = g_error_new_literal (NM_SETTINGS_ERROR, + NM_SETTINGS_ERROR_PERMISSION_DENIED, + error_desc); + g_free (error_desc); + goto error; + } + + g_clear_object (&subject); + dbus_g_method_return (context, nm_connection_get_path (NM_CONNECTION (connection))); + return; + +error: + g_assert (error); + dbus_g_method_return_error (context, error); + g_error_free (error); + g_clear_object (&subject); } static int From 474b76134c63af051aa8efe18baf8ac7d5abfc81 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 9 Jan 2014 12:25:51 -0600 Subject: [PATCH 3/6] sessions: fix return value handling for sd_uid_get_sessions() (bgo #707983) This function returns the number of sessions found, but the return value wasn't being correctly handled for errors. Also fix the require_active parameter value to be 100% clear about what NM wants. --- src/nm-session-monitor-systemd.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/nm-session-monitor-systemd.c b/src/nm-session-monitor-systemd.c index 4d8edab2d2..f195c1e775 100644 --- a/src/nm-session-monitor-systemd.c +++ b/src/nm-session-monitor-systemd.c @@ -234,18 +234,19 @@ nm_session_monitor_uid_has_session (NMSessionMonitor *monitor, const char **out_user, GError **error) { - int ret; + int num_sessions; if (!nm_session_uid_to_user (uid, out_user, error)) return FALSE; - ret = sd_uid_get_sessions (uid, FALSE, NULL) > 0; - if (ret < 0) { + /* Get all sessions (including inactive ones) for the user */ + num_sessions = sd_uid_get_sessions (uid, 0, NULL); + if (num_sessions < 0) { nm_log_warn (LOGD_CORE, "Failed to get systemd sessions for uid %d: %d", - uid, ret); + uid, num_sessions); return FALSE; } - return ret > 0 ? TRUE : FALSE; + return num_sessions > 0; } gboolean @@ -253,13 +254,14 @@ nm_session_monitor_uid_active (NMSessionMonitor *monitor, uid_t uid, GError **error) { - int ret; + int num_sessions; - ret = sd_uid_get_sessions (uid, TRUE, NULL) > 0; - if (ret < 0) { + /* Get active sessions for the user */ + num_sessions = sd_uid_get_sessions (uid, 1, NULL); + if (num_sessions < 0) { nm_log_warn (LOGD_CORE, "Failed to get active systemd sessions for uid %d: %d", - uid, ret); + uid, num_sessions); return FALSE; } - return ret > 0 ? TRUE : FALSE; + return num_sessions > 0; } From f0149b637252fce800885de9039a473dbb1937de Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 22 Jan 2014 13:07:24 -0600 Subject: [PATCH 4/6] core: enforce permissions for SetLogging This was always protected by D-Bus policy permissions, but just to be paranoid, ensure it's also protected by explicit checks on the UID. --- introspection/nm-manager.xml | 1 + src/nm-manager.c | 39 +++++++++++++++++++++++++++--------- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/introspection/nm-manager.xml b/introspection/nm-manager.xml index 2d4cbf8269..27610cfc9b 100644 --- a/introspection/nm-manager.xml +++ b/introspection/nm-manager.xml @@ -209,6 +209,7 @@ + Set logging verbosity and which operations are logged. diff --git a/src/nm-manager.c b/src/nm-manager.c index 4fa1991225..e5a9702966 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -123,10 +123,10 @@ static gboolean impl_manager_get_state (NMManager *manager, guint32 *state, GError **error); -static gboolean impl_manager_set_logging (NMManager *manager, - const char *level, - const char *domains, - GError **error); +static void impl_manager_set_logging (NMManager *manager, + const char *level, + const char *domains, + DBusGMethodInvocation *context); static void impl_manager_get_logging (NMManager *manager, char **level, @@ -4002,13 +4002,31 @@ impl_manager_get_state (NMManager *manager, guint32 *state, GError **error) return TRUE; } -static gboolean +static void impl_manager_set_logging (NMManager *manager, const char *level, const char *domains, - GError **error) + DBusGMethodInvocation *context) { - if (nm_logging_setup (level, domains, NULL, error)) { + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); + GError *error = NULL; + gulong caller_uid = G_MAXULONG; + + if (!nm_dbus_manager_get_caller_info (priv->dbus_mgr, context, NULL, &caller_uid, NULL)) { + error = g_error_new_literal (NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "Failed to get request UID."); + goto done; + } + + if (0 != caller_uid) { + error = g_error_new_literal (NM_MANAGER_ERROR, + NM_MANAGER_ERROR_PERMISSION_DENIED, + "Permission denied"); + goto done; + } + + if (nm_logging_setup (level, domains, NULL, &error)) { char *new_level = nm_logging_level_to_string (); char *new_domains = nm_logging_domains_to_string (); @@ -4016,9 +4034,12 @@ impl_manager_set_logging (NMManager *manager, new_level, new_domains); g_free (new_level); g_free (new_domains); - return TRUE; } - return FALSE; + +done: + if (error) + dbus_g_method_return_error (context, error); + g_clear_error (&error); } static void From 7e0f94f0f5a40cda3ab012069f98fafb77ddfd6d Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 9 Jan 2014 12:10:31 -0600 Subject: [PATCH 5/6] dbus: kill at_console usage in permissions (bgo #707983) (rh #979416) at_console permissions as implemented by D-Bus have some problems: 1) it is now fully redundant with PolicyKit and session tracking via systemd/ConsoleKit 2) it uses a different mechanism than PolicyKit or systemd to determine sessions and whether the user is on local or not (pam_console) 3) it was never widely implemented across so removing it harmonizes D-Bus permissions on all supported distros To that end, remove the at_console section of the D-Bus permissions, and rely on session-tracking and PolicyKit to ensure operations are locked down. No changes are being made to PolicyKit or session-tracking, so any operations denied by those mechanisms are still denied, and no permissions are being relaxed. Instead, this should allow remote users who log in via remote desktop or SSH to inspect network state, change connection parameters, and start/stop interfaces. Obviously if you are remote, you should not touch the interface which your connection is using, but that concern shouldn't prevent all the other nice stuff that you can do with NM. https://bugzilla.gnome.org/show_bug.cgi?id=707983 https://bugzilla.redhat.com/show_bug.cgi?id=979416 --- src/org.freedesktop.NetworkManager.conf | 158 ++++++++++++------------ 1 file changed, 82 insertions(+), 76 deletions(-) diff --git a/src/org.freedesktop.NetworkManager.conf b/src/org.freedesktop.NetworkManager.conf index db68374cc8..bdfe3e6773 100644 --- a/src/org.freedesktop.NetworkManager.conf +++ b/src/org.freedesktop.NetworkManager.conf @@ -26,93 +26,99 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - + + + + + + + From d000d1223fe33141d3df6dc49d9880ed0f29f729 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 9 Jan 2014 12:35:39 -0600 Subject: [PATCH 6/6] policy: allow inactive (remote/SSH) sessions to perform some actions (bgo #707983) (rh #979416) This commit allows inactive sessions (typically SSH or remote desktop logins) to modify their own connections, to modify the system hostname with authorization, and to modify system connections with authorization. https://bugzilla.redhat.com/show_bug.cgi?id=979416 https://bugzilla.gnome.org/show_bug.cgi?id=707983 --- policy/org.freedesktop.NetworkManager.policy.in.in | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/policy/org.freedesktop.NetworkManager.policy.in.in b/policy/org.freedesktop.NetworkManager.policy.in.in index ea3777a470..2de066c1e0 100644 --- a/policy/org.freedesktop.NetworkManager.policy.in.in +++ b/policy/org.freedesktop.NetworkManager.policy.in.in @@ -85,8 +85,7 @@ <_description>Modify personal network connections <_message>System policy prevents modification of personal network settings - no - yes + yes @@ -94,8 +93,7 @@ <_description>Modify network connections for all users <_message>System policy prevents modification of network settings for all users - no - @NM_MODIFY_SYSTEM_POLICY@ + @NM_MODIFY_SYSTEM_POLICY@ @@ -103,8 +101,7 @@ <_description>Modify persistent system hostname <_message>System policy prevents modification of the persistent system hostname - no - auth_admin_keep + auth_admin_keep