diff options
| -rw-r--r-- | ChangeLog | 20 | ||||
| -rw-r--r-- | libnm-util/nm-setting-vpn.c | 40 | ||||
| -rw-r--r-- | libnm-util/nm-setting-vpn.h | 12 | ||||
| -rw-r--r-- | src/vpn-manager/nm-vpn-connection.c | 5 | ||||
| -rw-r--r-- | system-settings/plugins/keyfile/nm-keyfile-connection.c | 135 | ||||
| -rw-r--r-- | system-settings/plugins/keyfile/reader.c | 68 | ||||
| -rw-r--r-- | system-settings/plugins/keyfile/reader.h | 4 | ||||
| -rw-r--r-- | system-settings/plugins/keyfile/writer.c | 14 | ||||
| -rw-r--r-- | vpn-daemons/vpnc/src/nm-vpnc-service.c | 10 |
9 files changed, 281 insertions, 27 deletions
@@ -1,3 +1,23 @@ +2008-09-04 Dan Williams <dcbw@redhat.com> + + * libnm-util/nm-setting-vpn.c + libnm-util/nm-setting-vpn.h + - Split VPN secrets from VPN data so that settings services can actually + figure out that they are secrets and store them accordingly + + * system-settings/plugins/keyfile/nm-keyfile-connection.c + system-settings/plugins/keyfile/reader.c + system-settings/plugins/keyfile/reader.h + system-settings/plugins/keyfile/writer.c + - Store VPN secrets separately from VPN data so that they can be fetched + on demand + - Implement the get_secrets() call so that (a) secrets don't leak out + to unprivileged callers, and (b) secrets can be sent to privileged + callers when needed + + * vpn-daemons/vpnc/src/nm-vpnc-service.c + - Handle split VPN secrets + 2008-08-27 Dan Williams <dcbw@redhat.com> * system-settings/plugins/ifcfg-fedora/reader.c diff --git a/libnm-util/nm-setting-vpn.c b/libnm-util/nm-setting-vpn.c index ef9ebe15f3..0c2118312b 100644 --- a/libnm-util/nm-setting-vpn.c +++ b/libnm-util/nm-setting-vpn.c @@ -71,6 +71,7 @@ enum { PROP_SERVICE_TYPE, PROP_USER_NAME, PROP_DATA, + PROP_SECRETS, LAST_PROP }; @@ -123,8 +124,17 @@ update_one_secret (NMSetting *setting, const char *key, GValue *value) g_return_if_fail (value != NULL); g_return_if_fail (G_VALUE_HOLDS_STRING (value)); - /* Secrets are really only known to the VPNs themselves. */ - g_hash_table_insert (self->data, g_strdup (key), g_value_dup_string (value)); + g_hash_table_insert (self->secrets, g_strdup (key), g_value_dup_string (value)); +} + +static void +destroy_one_secret (gpointer data) +{ + char *secret = (char *) data; + + /* Don't leave the secret lying around in memory */ + memset (secret, 0, strlen (secret)); + g_free (secret); } static void @@ -133,6 +143,7 @@ nm_setting_vpn_init (NMSettingVPN *setting) NM_SETTING (setting)->name = g_strdup (NM_SETTING_VPN_SETTING_NAME); setting->data = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); + setting->secrets = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, destroy_one_secret); } static void @@ -143,6 +154,7 @@ finalize (GObject *object) g_free (self->service_type); g_free (self->user_name); g_hash_table_destroy (self->data); + g_hash_table_destroy (self->secrets); G_OBJECT_CLASS (nm_setting_vpn_parent_class)->finalize (object); } @@ -158,6 +170,7 @@ set_property (GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec) { NMSettingVPN *setting = NM_SETTING_VPN (object); + GHashTable *new_hash; switch (prop_id) { case PROP_SERVICE_TYPE: @@ -171,7 +184,16 @@ set_property (GObject *object, guint prop_id, case PROP_DATA: /* Must make a deep copy of the hash table here... */ g_hash_table_remove_all (setting->data); - g_hash_table_foreach (g_value_get_boxed (value), copy_hash, setting->data); + new_hash = g_value_get_boxed (value); + if (new_hash) + g_hash_table_foreach (new_hash, copy_hash, setting->data); + break; + case PROP_SECRETS: + /* Must make a deep copy of the hash table here... */ + g_hash_table_remove_all (setting->secrets); + new_hash = g_value_get_boxed (value); + if (new_hash) + g_hash_table_foreach (new_hash, copy_hash, setting->secrets); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -195,6 +217,9 @@ get_property (GObject *object, guint prop_id, case PROP_DATA: g_value_set_boxed (value, setting->data); break; + case PROP_SECRETS: + g_value_set_boxed (value, setting->secrets); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -238,4 +263,13 @@ nm_setting_vpn_class_init (NMSettingVPNClass *setting_class) "VPN Service specific data", DBUS_TYPE_G_MAP_OF_STRING, G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE)); + + g_object_class_install_property + (object_class, PROP_SECRETS, + _nm_param_spec_specialized (NM_SETTING_VPN_SECRETS, + "Secrets", + "VPN Service specific secrets", + DBUS_TYPE_G_MAP_OF_STRING, + G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE | NM_SETTING_PARAM_SECRET)); } + diff --git a/libnm-util/nm-setting-vpn.h b/libnm-util/nm-setting-vpn.h index f87365e63d..ff7a80e4fd 100644 --- a/libnm-util/nm-setting-vpn.h +++ b/libnm-util/nm-setting-vpn.h @@ -55,6 +55,7 @@ GQuark nm_setting_vpn_error_quark (void); #define NM_SETTING_VPN_SERVICE_TYPE "service-type" #define NM_SETTING_VPN_USER_NAME "user-name" #define NM_SETTING_VPN_DATA "data" +#define NM_SETTING_VPN_SECRETS "secrets" typedef struct { NMSetting parent; @@ -72,9 +73,18 @@ typedef struct { * a char * -> char * mapping, and both the key * and value are owned by the hash table, and should * be allocated with functions whose value can be - * freed with g_free() + * freed with g_free(). Should not contain secrets. */ GHashTable *data; + + /* The hash table is created at setting object + * init time and should not be replaced. It is + * a char * -> char * mapping, and both the key + * and value are owned by the hash table, and should + * be allocated with functions whose value can be + * freed with g_free(). Should contain secrets only. + */ + GHashTable *secrets; } NMSettingVPN; typedef struct { diff --git a/src/vpn-manager/nm-vpn-connection.c b/src/vpn-manager/nm-vpn-connection.c index 78491787dc..ddb8d285f2 100644 --- a/src/vpn-manager/nm-vpn-connection.c +++ b/src/vpn-manager/nm-vpn-connection.c @@ -658,13 +658,12 @@ static void update_vpn_properties_secrets (gpointer key, gpointer data, gpointer user_data) { NMConnection *connection = NM_CONNECTION (user_data); + GHashTable *secrets = (GHashTable *) data; if (strcmp (key, NM_SETTING_VPN_SETTING_NAME)) return; - nm_connection_update_secrets (connection, - NM_SETTING_VPN_SETTING_NAME, - (GHashTable *) data); + nm_connection_update_secrets (connection, NM_SETTING_VPN_SETTING_NAME, secrets); } static void diff --git a/system-settings/plugins/keyfile/nm-keyfile-connection.c b/system-settings/plugins/keyfile/nm-keyfile-connection.c index 2e8f5a8798..7c483305f5 100644 --- a/system-settings/plugins/keyfile/nm-keyfile-connection.c +++ b/system-settings/plugins/keyfile/nm-keyfile-connection.c @@ -6,6 +6,7 @@ #include <nm-setting-connection.h> #include <nm-utils.h> +#include "nm-dbus-glib-types.h" #include "nm-keyfile-connection.h" #include "reader.h" #include "writer.h" @@ -49,6 +50,137 @@ get_settings (NMExportedConnection *exported) return nm_connection_to_hash (nm_exported_connection_get_connection (exported)); } +static GValue * +string_to_gvalue (const char *str) +{ + GValue *val; + + val = g_slice_new0 (GValue); + g_value_init (val, G_TYPE_STRING); + g_value_set_string (val, str); + + return val; +} + +static void +copy_one_secret (gpointer key, gpointer value, gpointer user_data) +{ + g_hash_table_insert ((GHashTable *) user_data, + g_strdup ((char *) key), + string_to_gvalue (value)); +} + +static void +add_secrets (NMSetting *setting, + const char *key, + const GValue *value, + gboolean secret, + gpointer user_data) +{ + GHashTable *secrets = user_data; + + if (!secret) + return; + + if (G_VALUE_HOLDS_STRING (value)) { + g_hash_table_insert (secrets, g_strdup (key), string_to_gvalue (g_value_get_string (value))); + } else if (G_VALUE_HOLDS (value, DBUS_TYPE_G_MAP_OF_STRING)) { + /* Flatten the string hash by pulling its keys/values out */ + g_hash_table_foreach (g_value_get_boxed (value), copy_one_secret, secrets); + } else + g_message ("%s: unhandled secret %s type %s", __func__, key, G_VALUE_TYPE_NAME (value)); +} + +static void +destroy_gvalue (gpointer data) +{ + GValue *value = (GValue *) data; + + g_value_unset (value); + g_slice_free (GValue, value); +} + +static GHashTable * +extract_secrets (NMKeyfileConnection *exported, + const char *setting_name, + GError **error) +{ + NMKeyfileConnectionPrivate *priv = NM_KEYFILE_CONNECTION_GET_PRIVATE (exported); + NMConnection *tmp; + GHashTable *secrets; + NMSetting *setting; + + tmp = connection_from_file (priv->filename, TRUE); + if (!tmp) { + g_set_error (error, NM_SETTINGS_ERROR, 1, + "%s.%d - Could not read secrets from file %s.", + __FILE__, __LINE__, priv->filename); + return NULL; + } + + setting = nm_connection_get_setting_by_name (tmp, setting_name); + if (!setting) { + g_object_unref (tmp); + g_set_error (error, NM_SETTINGS_ERROR, 1, + "%s.%d - Could not read secrets from file %s.", + __FILE__, __LINE__, priv->filename); + return NULL; + } + + /* Add the secrets from this setting to the secrets hash */ + secrets = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, destroy_gvalue); + nm_setting_enumerate_values (setting, add_secrets, secrets); + + g_object_unref (tmp); + + return secrets; +} + +static void +get_secrets (NMExportedConnection *exported, + const gchar *setting_name, + const gchar **hints, + gboolean request_new, + DBusGMethodInvocation *context) +{ + NMConnection *connection; + GError *error = NULL; + GHashTable *settings = NULL; + GHashTable *secrets = NULL; + NMSetting *setting; + + connection = nm_exported_connection_get_connection (exported); + setting = nm_connection_get_setting_by_name (connection, setting_name); + if (!setting) { + g_set_error (&error, NM_SETTINGS_ERROR, 1, + "%s.%d - Connection didn't have requested setting '%s'.", + __FILE__, __LINE__, setting_name); + goto error; + } + + /* Returned secrets are a{sa{sv}}; this is the outer a{s...} hash that + * will contain all the individual settings hashes. + */ + settings = g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, (GDestroyNotify) g_hash_table_destroy); + + /* Read in a temporary connection and just extract the secrets */ + secrets = extract_secrets (NM_KEYFILE_CONNECTION (exported), setting_name, &error); + if (!secrets) + goto error; + + g_hash_table_insert (settings, g_strdup (setting_name), secrets); + + dbus_g_method_return (context, settings); + g_hash_table_destroy (settings); + return; + +error: + nm_warning ("%s", error->message); + dbus_g_method_return_error (context, error); + g_error_free (error); +} + static gboolean update (NMExportedConnection *exported, GHashTable *new_settings, @@ -118,7 +250,7 @@ constructor (GType type, goto err; } - wrapped = connection_from_file (priv->filename); + wrapped = connection_from_file (priv->filename, FALSE); if (!wrapped) goto err; @@ -205,6 +337,7 @@ nm_keyfile_connection_class_init (NMKeyfileConnectionClass *keyfile_connection_c object_class->finalize = finalize; connection_class->get_settings = get_settings; + connection_class->get_secrets = get_secrets; connection_class->update = update; connection_class->delete = delete; diff --git a/system-settings/plugins/keyfile/reader.c b/system-settings/plugins/keyfile/reader.c index e43486a209..2702bff18f 100644 --- a/system-settings/plugins/keyfile/reader.c +++ b/system-settings/plugins/keyfile/reader.c @@ -315,6 +315,11 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key) g_strfreev (keys); } +typedef struct { + GKeyFile *keyfile; + gboolean secrets; +} ReadSettingInfo; + static void read_one_setting_value (NMSetting *setting, const char *key, @@ -322,7 +327,8 @@ read_one_setting_value (NMSetting *setting, gboolean secret, gpointer user_data) { - GKeyFile *file = (GKeyFile *) user_data; + ReadSettingInfo *info = (ReadSettingInfo *) user_data; + GKeyFile *file = info->keyfile; GType type; GError *err = NULL; gboolean check_for_key = TRUE; @@ -331,9 +337,15 @@ read_one_setting_value (NMSetting *setting, if (!strcmp (key, NM_SETTING_NAME)) return; - /* IPv4 addresses don't have the exact key name */ + /* Don't read in secrets unless we want to */ + if (secret && !info->secrets) + return; + + /* IPv4 addresses and VPN properties don't have the exact key name */ if (NM_IS_SETTING_IP4_CONFIG (setting) && !strcmp (key, NM_SETTING_IP4_CONFIG_ADDRESSES)) check_for_key = FALSE; + else if (NM_IS_SETTING_VPN (setting)) + check_for_key = FALSE; if (check_for_key && !g_key_file_has_key (file, setting->name, key, &err)) { if (err) { @@ -407,7 +419,7 @@ read_one_setting_value (NMSetting *setting, g_object_set (setting, key, array, NULL); g_byte_array_free (array, TRUE); g_free (tmp); - } else if (type == dbus_g_type_get_collection ("GSList", G_TYPE_STRING)) { + } else if (type == DBUS_TYPE_G_LIST_OF_STRING) { gchar **sa; gsize length; int i; @@ -422,7 +434,7 @@ read_one_setting_value (NMSetting *setting, g_slist_free (list); g_strfreev (sa); - } else if (type == dbus_g_type_get_map ("GHashTable", G_TYPE_STRING, G_TYPE_STRING)) { + } else if (type == DBUS_TYPE_G_MAP_OF_STRING) { read_hash_of_string (file, setting, key); } else if (type == DBUS_TYPE_G_UINT_ARRAY) { if (!read_array_of_uint (file, setting, key)) { @@ -441,21 +453,41 @@ read_one_setting_value (NMSetting *setting, } static NMSetting * -read_setting (GKeyFile *file, const char *name) +read_setting (GKeyFile *file, const char *name, gboolean secrets) { NMSetting *setting; + ReadSettingInfo info; + + info.keyfile = file; + info.secrets = secrets; setting = nm_connection_create_setting (name); - if (setting) { - nm_setting_enumerate_values (setting, read_one_setting_value, file); - } else + if (setting) + nm_setting_enumerate_values (setting, read_one_setting_value, &info); + else g_warning ("Invalid setting name '%s'", name); return setting; } +static void +read_vpn_secrets (GKeyFile *file, NMSettingVPN *s_vpn) +{ + char **keys, **iter; + + keys = g_key_file_get_keys (file, VPN_SECRETS_GROUP, NULL, NULL); + for (iter = keys; *iter; iter++) { + char *secret; + + secret = g_key_file_get_string (file, VPN_SECRETS_GROUP, *iter, NULL); + if (secret) + g_hash_table_insert (s_vpn->secrets, g_strdup (*iter), secret); + } + g_strfreev (keys); +} + NMConnection * -connection_from_file (const char *filename) +connection_from_file (const char *filename, gboolean secrets) { GKeyFile *key_file; struct stat statbuf; @@ -479,6 +511,7 @@ connection_from_file (const char *filename) gchar **groups; gsize length; int i; + gboolean vpn_secrets = FALSE; connection = nm_connection_new (); @@ -486,11 +519,26 @@ connection_from_file (const char *filename) for (i = 0; i < length; i++) { NMSetting *setting; - setting = read_setting (key_file, groups[i]); + /* Only read out secrets when needed */ + if (!strcmp (groups[i], VPN_SECRETS_GROUP)) { + vpn_secrets = TRUE; + continue; + } + + setting = read_setting (key_file, groups[i], secrets); if (setting) nm_connection_add_setting (connection, setting); } + /* Handle vpn secrets after the 'vpn' setting was read */ + if (secrets && vpn_secrets) { + NMSettingVPN *s_vpn; + + s_vpn = (NMSettingVPN *) nm_connection_get_setting (connection, NM_TYPE_SETTING_VPN); + if (s_vpn) + read_vpn_secrets (key_file, s_vpn); + } + g_strfreev (groups); } else { g_warning ("Error parsing file '%s': %s", filename, err->message); diff --git a/system-settings/plugins/keyfile/reader.h b/system-settings/plugins/keyfile/reader.h index 6e5ca471b1..bba315cdd4 100644 --- a/system-settings/plugins/keyfile/reader.h +++ b/system-settings/plugins/keyfile/reader.h @@ -3,9 +3,11 @@ #ifndef _KEYFILE_PLUGIN_READER_H #define _KEYFILE_PLUGIN_READER_H +#define VPN_SECRETS_GROUP "vpn-secrets" + #include <glib.h> #include <nm-connection.h> -NMConnection *connection_from_file (const char *filename); +NMConnection *connection_from_file (const char *filename, gboolean secrets); #endif /* _KEYFILE_PLUGIN_READER_H */ diff --git a/system-settings/plugins/keyfile/writer.c b/system-settings/plugins/keyfile/writer.c index 235fe81f31..34cc8af442 100644 --- a/system-settings/plugins/keyfile/writer.c +++ b/system-settings/plugins/keyfile/writer.c @@ -14,6 +14,7 @@ #include "nm-dbus-glib-types.h" #include "writer.h" +#include "reader.h" static gboolean write_array_of_uint (GKeyFile *file, @@ -154,10 +155,6 @@ write_hash_of_string_helper (gpointer key, gpointer data, gpointer user_data) const char *property = (const char *) key; const char *value = (const char *) data; - if ( !strcmp (info->setting_name, NM_SETTING_VPN_SETTING_NAME) - && !strcmp (property, NM_SETTING_VPN_SERVICE_TYPE)) - return; - g_key_file_set_string (info->file, info->setting_name, property, @@ -174,7 +171,14 @@ write_hash_of_string (GKeyFile *file, WriteStringHashInfo info; info.file = file; - info.setting_name = setting->name; + + /* Write VPN secrets out to a different group to keep them separate */ + if ( (G_OBJECT_TYPE (setting) == NM_TYPE_SETTING_VPN) + && !strcmp (key, NM_SETTING_VPN_SECRETS)) { + info.setting_name = VPN_SECRETS_GROUP; + } else + info.setting_name = setting->name; + g_hash_table_foreach (hash, write_hash_of_string_helper, &info); } diff --git a/vpn-daemons/vpnc/src/nm-vpnc-service.c b/vpn-daemons/vpnc/src/nm-vpnc-service.c index 0d90e74d7d..feac3a00c2 100644 --- a/vpn-daemons/vpnc/src/nm-vpnc-service.c +++ b/vpn-daemons/vpnc/src/nm-vpnc-service.c @@ -321,6 +321,7 @@ static gboolean nm_vpnc_config_write (gint vpnc_fd, const char *default_user_name, GHashTable *properties, + GHashTable *secrets, GError **error) { WriteConfigInfo *info; @@ -354,6 +355,7 @@ nm_vpnc_config_write (gint vpnc_fd, info = g_malloc0 (sizeof (WriteConfigInfo)); info->fd = vpnc_fd; g_hash_table_foreach (properties, write_one_property, info); + g_hash_table_foreach (secrets, write_one_property, info); *error = info->error; g_free (info); @@ -373,12 +375,14 @@ real_connect (NMVPNPlugin *plugin, g_assert (s_vpn); if (!nm_vpnc_properties_validate (s_vpn->data, error)) goto out; + if (!nm_vpnc_properties_validate (s_vpn->secrets, error)) + goto out; vpnc_fd = nm_vpnc_start_vpnc_binary (NM_VPNC_PLUGIN (plugin), error); if (vpnc_fd < 0) goto out; - if (!nm_vpnc_config_write (vpnc_fd, s_vpn->user_name, s_vpn->data, error)) + if (!nm_vpnc_config_write (vpnc_fd, s_vpn->user_name, s_vpn->data, s_vpn->secrets, error)) goto out; success = TRUE; @@ -412,11 +416,11 @@ real_need_secrets (NMVPNPlugin *plugin, // FIXME: there are some configurations where both passwords are not // required. Make sure they work somehow. - if (!g_hash_table_lookup (s_vpn->data, NM_VPNC_KEY_SECRET)) { + if (!g_hash_table_lookup (s_vpn->secrets, NM_VPNC_KEY_SECRET)) { *setting_name = NM_SETTING_VPN_SETTING_NAME; return TRUE; } - if (!g_hash_table_lookup (s_vpn->data, NM_VPNC_KEY_XAUTH_PASSWORD)) { + if (!g_hash_table_lookup (s_vpn->secrets, NM_VPNC_KEY_XAUTH_PASSWORD)) { *setting_name = NM_SETTING_VPN_SETTING_NAME; return TRUE; } |