system-settings: implement common GetSettings and GetSecrets methods (rh #486696)

Fix a few problems... No plugin should return secrets in the GetSettings method,
which some plugins did.  When that was committed in the commit "system-settings:
don't return secrets in the settings", it broke those plugins that didn't implement
GetSecrets.  Each plugin can actually use the same code for GetSettings and
GetSecrets, so implement those generically in the NMExportedConnection class and
remove plugin-specific implementations that all did the same thing.
This commit is contained in:
Dan Williams 2009-02-22 17:16:09 -05:00
parent a82b93c31a
commit 09412869c1
8 changed files with 150 additions and 266 deletions

View file

@ -255,10 +255,32 @@ nm_exported_connection_new (NMConnection *wrapped)
NULL);
}
static GHashTable *
real_get_settings (NMExportedConnection *exported)
{
NMExportedConnectionPrivate *priv;
NMConnection *no_secrets;
GHashTable *hash;
g_return_val_if_fail (NM_IS_EXPORTED_CONNECTION (exported), NULL);
priv = NM_EXPORTED_CONNECTION_GET_PRIVATE (exported);
/* Secrets should *never* be returned by the GetSettings method, they
* get returned by the GetSecrets method which can be better
* protected against leakage of secrets to unprivileged callers.
*/
no_secrets = nm_connection_duplicate (priv->wrapped);
nm_connection_clear_secrets (no_secrets);
hash = nm_connection_to_hash (no_secrets);
g_object_unref (G_OBJECT (no_secrets));
return hash;
}
static gboolean
impl_exported_connection_get_settings (NMExportedConnection *connection,
GHashTable **settings,
GError **error)
GHashTable **settings,
GError **error)
{
NMExportedConnectionPrivate *priv;
@ -267,7 +289,7 @@ impl_exported_connection_get_settings (NMExportedConnection *connection,
priv = NM_EXPORTED_CONNECTION_GET_PRIVATE (connection);
if (!EXPORTED_CONNECTION_CLASS (connection)->get_settings)
*settings = nm_connection_to_hash (priv->wrapped);
*settings = real_get_settings (connection);
else
*settings = EXPORTED_CONNECTION_CLASS (connection)->get_settings (connection);
@ -351,6 +373,103 @@ impl_exported_connection_delete (NMExportedConnection *connection,
return success;
}
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)
{
const char *value_str = (const char *) value;
if (value_str) {
g_hash_table_insert ((GHashTable *) user_data,
g_strdup ((char *) key),
string_to_gvalue (value_str));
}
}
static void
add_secrets (NMSetting *setting,
const char *key,
const GValue *value,
GParamFlags flags,
gpointer user_data)
{
GHashTable *secrets = user_data;
if (!(flags & NM_SETTING_PARAM_SECRET))
return;
if (G_VALUE_HOLDS_STRING (value)) {
const char *tmp;
tmp = g_value_get_string (value);
if (tmp)
g_hash_table_insert (secrets, g_strdup (key), string_to_gvalue (tmp));
} 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);
}
}
static void
destroy_gvalue (gpointer data)
{
GValue *value = (GValue *) data;
g_value_unset (value);
g_slice_free (GValue, value);
}
static void
real_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, NM_SETTINGS_ERROR_INVALID_CONNECTION,
"%s.%d - Connection didn't have requested setting '%s'.",
__FILE__, __LINE__, setting_name);
dbus_g_method_return_error (context, error);
g_error_free (error);
return;
}
/* 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);
/* Add the secrets from this setting to the inner secrets hash for this setting */
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_hash_table_insert (settings, g_strdup (setting_name), secrets);
dbus_g_method_return (context, settings);
g_hash_table_destroy (settings);
}
static void
impl_exported_connection_get_secrets (NMExportedConnection *connection,
const gchar *setting_name,
@ -369,16 +488,10 @@ impl_exported_connection_get_secrets (NMExportedConnection *connection,
return;
}
if (!EXPORTED_CONNECTION_CLASS (connection)->service_get_secrets) {
g_set_error (&error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_SECRETS_UNAVAILABLE,
"%s.%d - Missing implementation for ConnectionSettings::GetSecrets.",
__FILE__, __LINE__);
dbus_g_method_return_error (context, error);
g_error_free (error);
return;
}
EXPORTED_CONNECTION_CLASS (connection)->service_get_secrets (connection, setting_name, hints, request_new, context);
if (!EXPORTED_CONNECTION_CLASS (connection)->service_get_secrets)
real_get_secrets (connection, setting_name, hints, request_new, context);
else
EXPORTED_CONNECTION_CLASS (connection)->service_get_secrets (connection, setting_name, hints, request_new, context);
}
static void
@ -451,6 +564,9 @@ nm_exported_connection_class_init (NMExportedConnectionClass *exported_connectio
object_class->get_property = get_property;
object_class->dispose = nm_exported_connection_dispose;
exported_connection_class->get_settings = real_get_settings;
exported_connection_class->service_get_secrets = real_get_secrets;
/* Properties */
g_object_class_install_property
(object_class, PROP_CONNECTION,

View file

@ -316,15 +316,6 @@ nm_ifcfg_connection_get_unmanaged (NMIfcfgConnection *self)
return NM_IFCFG_CONNECTION_GET_PRIVATE (self)->unmanaged;
}
static GHashTable *
get_settings (NMExportedConnection *exported)
{
NMConnection *connection = nm_exported_connection_get_connection (exported);
nm_connection_clear_secrets (connection);
return nm_connection_to_hash (connection);
}
static gboolean
update (NMExportedConnection *exported, GHashTable *new_settings, GError **error)
{
@ -448,7 +439,6 @@ nm_ifcfg_connection_class_init (NMIfcfgConnectionClass *ifcfg_connection_class)
object_class->get_property = get_property;
object_class->finalize = finalize;
connection_class->get_settings = get_settings;
connection_class->update = update;
connection_class->do_delete = do_delete;

View file

@ -123,15 +123,6 @@ nm_suse_connection_new (const char *iface, NMDeviceType dev_type)
return exported;
}
static GHashTable *
get_settings (NMExportedConnection *exported)
{
NMConnection *connection = nm_exported_connection_get_connection (exported);
nm_connection_clear_secrets (connection);
return nm_connection_to_hash (connection);
}
static gboolean
update (NMExportedConnection *exported,
GHashTable *new_settings,
@ -190,7 +181,6 @@ nm_suse_connection_class_init (NMSuseConnectionClass *suse_connection_class)
/* Virtual methods */
object_class->finalize = finalize;
connection_class->get_settings = get_settings;
connection_class->update = update;
connection_class->do_delete = do_delete;
}

View file

@ -67,15 +67,6 @@ nm_ifupdown_connection_new (if_block *block)
NULL);
}
static GHashTable *
get_settings (NMExportedConnection *exported)
{
NMConnection *connection = nm_exported_connection_get_connection (exported);
nm_connection_clear_secrets (connection);
return nm_connection_to_hash (connection);
}
static gboolean
update (NMExportedConnection *exported,
GHashTable *new_settings,
@ -188,7 +179,6 @@ nm_ifupdown_connection_class_init (NMIfupdownConnectionClass *ifupdown_connectio
object_class->get_property = get_property;
object_class->finalize = finalize;
connection_class->get_settings = get_settings;
connection_class->update = update;
connection_class->do_delete = do_delete;
connection_class->service_get_secrets = service_get_secrets;
@ -209,54 +199,14 @@ service_get_secrets (NMExportedConnection *exported,
gboolean request_new,
DBusGMethodInvocation *context)
{
NMConnection *connection;
GError *error = NULL;
GHashTable *settings = NULL;
GHashTable *secrets = NULL;
NMSetting *setting;
PLUGIN_PRINT ("SCPlugin-Ifupdown", "get_secrets for setting_name:'%s')", setting_name);
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,
NM_SETTINGS_ERROR_INVALID_CONNECTION,
"%s.%d - Connection didn't have requested setting '%s'.",
__FILE__, __LINE__, setting_name);
PLUGIN_PRINT ("SCPlugin-Ifupdown", "%s", error->message);
dbus_g_method_return_error (context, error);
g_error_free (error);
return;
}
settings = g_hash_table_new_full (g_str_hash, g_str_equal,
g_free, (GDestroyNotify) g_hash_table_destroy);
if (!settings) {
g_set_error (&error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INTERNAL_ERROR,
"%s.%d - failed to hash setting (OOM?)",
__FILE__, __LINE__);
dbus_g_method_return_error (context, error);
g_error_free (error);
return;
}
if (!strcmp (setting_name, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME)) {
secrets = nm_setting_to_hash (setting);
if (secrets) {
g_hash_table_insert(settings, g_strdup(setting_name), secrets);
dbus_g_method_return (context, settings);
} else {
g_set_error (&error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INTERNAL_ERROR,
"%s.%d - nm_setting_to_hash failed (OOM?)",
__FILE__, __LINE__);
dbus_g_method_return_error (context, error);
g_error_free (error);
g_hash_table_destroy (settings);
}
} else {
/* FIXME: Only wifi secrets are supported for now */
if (!strcmp (setting_name, NM_SETTING_WIRELESS_SECURITY_SETTING_NAME))
NM_EXPORTED_CONNECTION_CLASS (nm_ifupdown_connection_parent_class)->service_get_secrets (exported, setting_name, hints, request_new, context);
else {
g_set_error (&error, NM_SETTING_WIRELESS_SECURITY_ERROR, 1,
"%s.%d - security setting name not supported '%s'.",
__FILE__, __LINE__, setting_name);
@ -265,3 +215,4 @@ service_get_secrets (NMExportedConnection *exported,
g_error_free (error);
}
}

View file

@ -398,11 +398,6 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key)
}
typedef struct {
GKeyFile *keyfile;
gboolean secrets;
} ReadSettingInfo;
typedef struct {
const char *setting_name;
const char *key;
@ -450,8 +445,7 @@ read_one_setting_value (NMSetting *setting,
GParamFlags flags,
gpointer user_data)
{
ReadSettingInfo *info = (ReadSettingInfo *) user_data;
GKeyFile *file = info->keyfile;
GKeyFile *file = user_data;
const char *setting_name;
GType type;
GError *err = NULL;
@ -466,10 +460,6 @@ read_one_setting_value (NMSetting *setting,
if (!strcmp (key, NM_SETTING_NAME))
return;
/* Don't read in secrets unless we want to */
if ((flags & NM_SETTING_PARAM_SECRET) && !info->secrets)
return;
/* Don't read the NMSettingConnection object's 'read-only' property */
if ( NM_IS_SETTING_CONNECTION (setting)
&& !strcmp (key, NM_SETTING_CONNECTION_READ_ONLY))
@ -606,17 +596,13 @@ read_one_setting_value (NMSetting *setting,
}
static NMSetting *
read_setting (GKeyFile *file, const char *name, gboolean secrets)
read_setting (GKeyFile *file, const char *name)
{
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, &info);
nm_setting_enumerate_values (setting, read_one_setting_value, (gpointer) file);
else
g_warning ("Invalid setting name '%s'", name);
@ -642,7 +628,7 @@ read_vpn_secrets (GKeyFile *file, NMSettingVPN *s_vpn)
}
NMConnection *
connection_from_file (const char *filename, gboolean secrets)
connection_from_file (const char *filename)
{
GKeyFile *key_file;
struct stat statbuf;
@ -656,10 +642,10 @@ connection_from_file (const char *filename, gboolean secrets)
bad_owner = getuid () != statbuf.st_uid;
bad_permissions = statbuf.st_mode & 0077;
if (bad_owner || bad_permissions) {
g_warning ("Ignorning insecure configuration file '%s'", filename);
return NULL;
}
if (bad_owner || bad_permissions) {
g_warning ("Ignorning insecure configuration file '%s'", filename);
return NULL;
}
key_file = g_key_file_new ();
if (g_key_file_load_from_file (key_file, filename, G_KEY_FILE_NONE, &err)) {
@ -680,13 +666,13 @@ connection_from_file (const char *filename, gboolean secrets)
continue;
}
setting = read_setting (key_file, groups[i], secrets);
setting = read_setting (key_file, groups[i]);
if (setting)
nm_connection_add_setting (connection, setting);
}
/* Handle vpn secrets after the 'vpn' setting was read */
if (secrets && vpn_secrets) {
if (vpn_secrets) {
NMSettingVPN *s_vpn;
s_vpn = (NMSettingVPN *) nm_connection_get_setting (connection, NM_TYPE_SETTING_VPN);

View file

@ -27,6 +27,6 @@
#include <glib.h>
#include <nm-connection.h>
NMConnection *connection_from_file (const char *filename, gboolean secrets);
NMConnection *connection_from_file (const char *filename);
#endif /* _KEYFILE_PLUGIN_READER_H */

View file

@ -64,153 +64,6 @@ nm_keyfile_connection_get_filename (NMKeyfileConnection *self)
return NM_KEYFILE_CONNECTION_GET_PRIVATE (self)->filename;
}
static GHashTable *
get_settings (NMExportedConnection *exported)
{
NMConnection *connection = nm_exported_connection_get_connection (exported);
nm_connection_clear_secrets (connection);
return nm_connection_to_hash (connection);
}
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)
{
const char *value_str = (const char *) value;
if (value_str)
g_hash_table_insert ((GHashTable *) user_data,
g_strdup ((char *) key),
string_to_gvalue (value_str));
}
static void
add_secrets (NMSetting *setting,
const char *key,
const GValue *value,
GParamFlags flags,
gpointer user_data)
{
GHashTable *secrets = user_data;
if (!(flags & NM_SETTING_PARAM_SECRET))
return;
if (G_VALUE_HOLDS_STRING (value)) {
const char *tmp;
tmp = g_value_get_string (value);
if (tmp)
g_hash_table_insert (secrets, g_strdup (key), string_to_gvalue (tmp));
} 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, NM_SETTINGS_ERROR_SECRETS_UNAVAILABLE,
"%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, NM_SETTINGS_ERROR_SECRETS_UNAVAILABLE,
"%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
service_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, NM_SETTINGS_ERROR_INVALID_CONNECTION,
"%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,
@ -283,7 +136,7 @@ constructor (GType type,
goto err;
}
wrapped = connection_from_file (priv->filename, FALSE);
wrapped = connection_from_file (priv->filename);
if (!wrapped)
goto err;
@ -373,8 +226,6 @@ nm_keyfile_connection_class_init (NMKeyfileConnectionClass *keyfile_connection_c
object_class->get_property = get_property;
object_class->finalize = finalize;
connection_class->get_settings = get_settings;
connection_class->service_get_secrets = service_get_secrets;
connection_class->update = update;
connection_class->do_delete = do_delete;

View file

@ -67,7 +67,7 @@ test_read_valid_wired_connection (void)
const char *expected_address2_gw = "1.2.1.1";
NMIP4Address *ip4_addr;
connection = connection_from_file (TEST_WIRED_FILE, TRUE);
connection = connection_from_file (TEST_WIRED_FILE);
ASSERT (connection != NULL,
"connection-read", "failed to read %s", TEST_WIRED_FILE);
@ -432,7 +432,7 @@ test_write_wired_connection (void)
"connection-write", "didn't get keyfile name back after writing connection");
/* Read the connection back in and compare it to the one we just wrote out */
reread = connection_from_file (testfile, TRUE);
reread = connection_from_file (testfile);
ASSERT (reread != NULL, "connection-write", "failed to re-read test connection");
ASSERT (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT) == TRUE,
@ -462,7 +462,7 @@ test_read_valid_wireless_connection (void)
const guint64 expected_timestamp = 1226604314;
guint64 timestamp;
connection = connection_from_file (TEST_WIRELESS_FILE, TRUE);
connection = connection_from_file (TEST_WIRELESS_FILE);
ASSERT (connection != NULL,
"connection-read", "failed to read %s", TEST_WIRELESS_FILE);
@ -652,7 +652,7 @@ test_write_wireless_connection (void)
"connection-write", "didn't get keyfile name back after writing connection");
/* Read the connection back in and compare it to the one we just wrote out */
reread = connection_from_file (testfile, TRUE);
reread = connection_from_file (testfile);
ASSERT (reread != NULL, "connection-write", "failed to re-read test connection");
ASSERT (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT) == TRUE,