Commit 4020594d authored by Dan Williams's avatar Dan Williams

core: fix CA cert mishandling after cert file deletion (deb #560067) (rh #546793)

If a connection was created with a CA certificate, but the user later
moved or deleted that CA certificate, the applet would simply provide the
connection to NetworkManager without any CA certificate.  This could cause
NM to connect to the original network (or a network spoofing the original
network) without verifying the identity of the network as the user
expects.

In the future we can/should do better here by (1) alerting the user that
some connection is now no longer complete by flagging it in the connection
editor or notifying the user somehow, and (2) by using a freaking' cert
store already (not that Linux has one yet).
parent 56d87fcb
......@@ -1476,6 +1476,7 @@ add_one_setting (GHashTable *settings,
GError **error)
{
GHashTable *secrets;
GError *tmp_error = NULL;
g_return_val_if_fail (settings != NULL, FALSE);
g_return_val_if_fail (connection != NULL, FALSE);
......@@ -1483,7 +1484,15 @@ add_one_setting (GHashTable *settings,
g_return_val_if_fail (error != NULL, FALSE);
g_return_val_if_fail (*error == NULL, FALSE);
utils_fill_connection_certs (connection);
if (!utils_fill_connection_certs (NM_CONNECTION (connection), &tmp_error)) {
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INTERNAL_ERROR,
"%s.%d (%s): failed to read connection certificates: (%d) %s.",
__FILE__, __LINE__, __func__,
tmp_error ? tmp_error->code : -1,
tmp_error && tmp_error->message ? tmp_error->message : "(unknown)");
return FALSE;
}
secrets = nm_setting_to_hash (setting);
utils_clear_filled_connection_certs (connection);
......
......@@ -650,7 +650,15 @@ get_8021x_secrets_cb (GtkDialog *dialog,
goto done;
}
utils_fill_connection_certs (NM_CONNECTION (connection));
if (!utils_fill_connection_certs (NM_CONNECTION (connection), &err)) {
g_set_error (&err, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INTERNAL_ERROR,
"%s.%d (%s): failed to read connection certificates: (%d) %s.",
__FILE__, __LINE__, __func__,
err ? err->code : -1,
err && err->message ? err->message : "(unknown)");
goto done;
}
secrets = nm_setting_to_hash (setting);
utils_clear_filled_connection_certs (NM_CONNECTION (connection));
......
......@@ -503,14 +503,15 @@ add_connection (NMConnectionList *self,
NMExportedConnection *exported = NULL;
NMConnectionScope scope;
gboolean success = FALSE;
GError *error = NULL;
scope = nm_connection_get_scope (connection);
if (scope == NM_CONNECTION_SCOPE_SYSTEM) {
GError *error = NULL;
utils_fill_connection_certs (connection);
success = nm_dbus_settings_system_add_connection (self->system_settings, connection, &error);
utils_clear_filled_connection_certs (connection);
success = utils_fill_connection_certs (connection, &error);
if (success) {
success = nm_dbus_settings_system_add_connection (self->system_settings, connection, &error);
utils_clear_filled_connection_certs (connection);
}
if (!success) {
gboolean pending_auth = FALSE;
......@@ -705,18 +706,21 @@ update_connection (NMConnectionList *list,
gboolean pending_auth = FALSE;
GtkWindow *parent;
utils_fill_connection_certs (modified);
new_settings = nm_connection_to_hash (modified);
parent = nm_connection_editor_get_window (editor);
/* Hack; make sure that gconf private values are copied */
nm_gconf_copy_private_connection_values (nm_exported_connection_get_connection (original),
modified);
success = utils_fill_connection_certs (modified, &error);
if (success) {
new_settings = nm_connection_to_hash (modified);
success = nm_exported_connection_update (original, new_settings, &error);
g_hash_table_destroy (new_settings);
utils_clear_filled_connection_certs (modified);
/* Hack; make sure that gconf private values are copied */
nm_gconf_copy_private_connection_values (nm_exported_connection_get_connection (original),
modified);
success = nm_exported_connection_update (original, new_settings, &error);
g_hash_table_destroy (new_settings);
utils_clear_filled_connection_certs (modified);
}
parent = nm_connection_editor_get_window (editor);
if (!success) {
if (pk_helper_is_permission_denied_error (error)) {
GError *auth_error = NULL;
......@@ -902,19 +906,28 @@ edit_done_cb (NMConnectionEditor *editor, gint response, GError *error, gpointer
connection = nm_connection_editor_get_connection (editor);
utils_fill_connection_certs (connection);
success = nm_connection_verify (connection, &edit_error);
utils_clear_filled_connection_certs (connection);
if (success) {
update_connection (info->list, editor, info->original_connection,
connection, connection_updated_cb, info);
} else {
g_warning ("%s: invalid connection after update: bug in the "
"'%s' / '%s' invalid: %d",
success = utils_fill_connection_certs (connection, &edit_error);
if (!success) {
g_warning ("%s: error completing connection edit: (%d) %s",
__func__,
g_type_name (nm_connection_lookup_setting_type_by_quark (edit_error->domain)),
edit_error->message, edit_error->code);
edit_error ? edit_error->code : -1,
edit_error && edit_error->message ? edit_error->message : "(unknown)");
} else {
success = nm_connection_verify (connection, &edit_error);
utils_clear_filled_connection_certs (connection);
if (success) {
update_connection (info->list, editor, info->original_connection,
connection, connection_updated_cb, info);
} else {
g_warning ("%s: invalid connection after update: property "
"'%s' / '%s' invalid: %d",
__func__,
g_type_name (nm_connection_lookup_setting_type_by_quark (edit_error->domain)),
edit_error->message, edit_error->code);
}
}
if (!success) {
g_error_free (edit_error);
connection_updated_cb (info->list, FALSE, user_data);
}
......
......@@ -25,6 +25,7 @@
#include <gconf/gconf.h>
#include <gconf/gconf-client.h>
#include <glib.h>
#include <glib/gi18n.h>
#include <gnome-keyring.h>
#include <dbus/dbus-glib.h>
#include <nm-setting-connection.h>
......@@ -992,6 +993,7 @@ typedef struct ReadFromGConfInfo {
GConfClient *client;
const char *dir;
guint32 dir_len;
GError *error;
} ReadFromGConfInfo;
static void
......@@ -1137,57 +1139,81 @@ read_one_setting_value_from_gconf (NMSetting *setting,
}
}
static void
static gboolean
read_one_cert (ReadFromGConfInfo *info,
const char *setting_name,
const char *key)
const char *key,
gboolean fail_if_missing,
GError **error)
{
char *value = NULL;
if (!nm_gconf_get_string_helper (info->client, info->dir, key, setting_name, &value))
return;
if (nm_gconf_get_string_helper (info->client, info->dir, key, setting_name, &value)) {
if (fail_if_missing && !g_file_test (value, G_FILE_TEST_EXISTS)) {
g_set_error (error, 0, 0, _("Certificate %s not found or not accessible."), value);
return FALSE;
}
g_object_set_data_full (G_OBJECT (info->connection),
key, value,
(GDestroyNotify) g_free);
g_object_set_data_full (G_OBJECT (info->connection), key, value, (GDestroyNotify) g_free);
}
return TRUE;
}
static void
read_applet_private_values_from_gconf (NMSetting *setting,
ReadFromGConfInfo *info)
{
if (NM_IS_SETTING_802_1X (setting)) {
const char *setting_name = nm_setting_get_name (setting);
gboolean value;
if (nm_gconf_get_bool_helper (info->client, info->dir,
NMA_CA_CERT_IGNORE_TAG,
setting_name, &value)) {
g_object_set_data (G_OBJECT (info->connection),
NMA_CA_CERT_IGNORE_TAG,
GUINT_TO_POINTER (value));
}
const char *setting_name = nm_setting_get_name (setting);
gboolean value;
GError *error = NULL;
if (!NM_IS_SETTING_802_1X (setting))
return;
if (nm_gconf_get_bool_helper (info->client, info->dir,
NMA_CA_CERT_IGNORE_TAG,
setting_name, &value)) {
g_object_set_data (G_OBJECT (info->connection),
NMA_CA_CERT_IGNORE_TAG,
GUINT_TO_POINTER (value));
}
if (nm_gconf_get_bool_helper (info->client, info->dir,
NMA_PHASE2_CA_CERT_IGNORE_TAG,
setting_name, &value)) {
g_object_set_data (G_OBJECT (info->connection),
NMA_PHASE2_CA_CERT_IGNORE_TAG,
GUINT_TO_POINTER (value));
}
/* Binary certificate and key data doesn't get stored in GConf. Instead,
* the path to the certificate gets stored in a special key and the
* certificate is read and stuffed into the setting right before
* the connection is sent to NM
*/
if (nm_gconf_get_bool_helper (info->client, info->dir,
NMA_PHASE2_CA_CERT_IGNORE_TAG,
setting_name, &value)) {
g_object_set_data (G_OBJECT (info->connection),
NMA_PHASE2_CA_CERT_IGNORE_TAG,
GUINT_TO_POINTER (value));
if (!read_one_cert (info, setting_name, NMA_PATH_CA_CERT_TAG, TRUE, &error)) {
/* Save the first error reading a certificate */
if (!info->error) {
info->error = error;
error = NULL;
}
g_clear_error (&error);
}
/* Binary certificate and key data doesn't get stored in GConf. Instead,
* the path to the certificate gets stored in a special key and the
* certificate is read and stuffed into the setting right before
* the connection is sent to NM
*/
read_one_cert (info, setting_name, NMA_PATH_CA_CERT_TAG);
read_one_cert (info, setting_name, NMA_PATH_CLIENT_CERT_TAG);
read_one_cert (info, setting_name, NMA_PATH_PRIVATE_KEY_TAG);
read_one_cert (info, setting_name, NMA_PATH_PHASE2_CA_CERT_TAG);
read_one_cert (info, setting_name, NMA_PATH_PHASE2_CLIENT_CERT_TAG);
read_one_cert (info, setting_name, NMA_PATH_PHASE2_PRIVATE_KEY_TAG);
if (!read_one_cert (info, setting_name, NMA_PATH_PHASE2_CA_CERT_TAG, TRUE, &error)) {
/* Save the first error reading a certificate */
if (!info->error) {
info->error = error;
error = NULL;
}
g_clear_error (&error);
}
read_one_cert (info, setting_name, NMA_PATH_CLIENT_CERT_TAG, FALSE, NULL);
read_one_cert (info, setting_name, NMA_PATH_PRIVATE_KEY_TAG, FALSE, NULL);
read_one_cert (info, setting_name, NMA_PATH_PHASE2_CLIENT_CERT_TAG, FALSE, NULL);
read_one_cert (info, setting_name, NMA_PATH_PHASE2_PRIVATE_KEY_TAG, FALSE, NULL);
}
static void
......@@ -1214,24 +1240,17 @@ read_one_setting (gpointer data, gpointer user_data)
NMConnection *
nm_gconf_read_connection (GConfClient *client,
const char *dir)
const char *dir,
GError **error)
{
ReadFromGConfInfo info;
GSList *list;
GError *err = NULL;
list = gconf_client_all_dirs (client, dir, &err);
if (err) {
g_warning ("Error while reading connection: %s", err->message);
g_error_free (err);
list = gconf_client_all_dirs (client, dir, error);
if (!list)
return NULL;
}
if (!list) {
g_warning ("Invalid connection (empty)");
return NULL;
}
memset (&info, 0, sizeof (info));
info.connection = nm_connection_new ();
info.client = client;
info.dir = dir;
......@@ -1240,6 +1259,21 @@ nm_gconf_read_connection (GConfClient *client,
g_slist_foreach (list, read_one_setting, &info);
g_slist_free (list);
if (info.error) {
if (error)
*error = info.error;
else {
g_warning ("%s: (%s) error reading connection: (%d) %s",
__func__, info.dir, info.error->code, info.error->message);
g_clear_error (&info.error);
}
if (info.connection) {
g_object_unref (info.connection);
info.connection = NULL;
}
}
return info.connection;
}
......
......@@ -212,7 +212,8 @@ nm_gconf_get_all_connections (GConfClient *client);
NMConnection *
nm_gconf_read_connection (GConfClient *client,
const char *dir);
const char *dir,
GError **error);
void
nm_gconf_write_connection (NMConnection *connection,
......
......@@ -64,19 +64,23 @@ NMAGConfConnection *
nma_gconf_connection_new (GConfClient *client, const char *conf_dir)
{
NMConnection *connection;
NMAGConfConnection *gconf_connection;
NMAGConfConnection *gconf_connection = NULL;
GError *error;
g_return_val_if_fail (GCONF_IS_CLIENT (client), NULL);
g_return_val_if_fail (conf_dir != NULL, NULL);
/* retrieve GConf data */
connection = nm_gconf_read_connection (client, conf_dir);
connection = nm_gconf_read_connection (client, conf_dir, &error);
if (connection) {
gconf_connection = nma_gconf_connection_new_from_connection (client, conf_dir, connection);
g_object_unref (connection);
} else {
nm_warning ("No connection read from GConf at %s.", conf_dir);
gconf_connection = NULL;
g_warning ("%s: (%s) error reading connection: (%d) %s",
__func__, conf_dir,
error ? error->code : -1,
error && error->message ? error->message : "(unknown)");
g_clear_error (&error);
}
return gconf_connection;
......@@ -155,13 +159,23 @@ nma_gconf_connection_changed (NMAGConfConnection *self)
priv = NMA_GCONF_CONNECTION_GET_PRIVATE (self);
wrapped_connection = nm_exported_connection_get_connection (NM_EXPORTED_CONNECTION (self));
gconf_connection = nm_gconf_read_connection (priv->client, priv->dir);
gconf_connection = nm_gconf_read_connection (priv->client, priv->dir, &error);
if (!gconf_connection) {
g_warning ("No connection read from GConf at %s.", priv->dir);
g_warning ("%s: (%s) error reading connection: (%d) %s",
__func__, priv->dir,
error ? error->code : -1,
error && error->message ? error->message : "(unknown)");
goto invalid;
}
if (!utils_fill_connection_certs (gconf_connection, &error)) {
g_warning ("%s: Invalid connection %s: failed to load connection certificates: (%d) %s",
__func__, priv->dir,
error ? error->code : -1,
error && error->message ? error->message : "(unknown)");
goto invalid;
}
utils_fill_connection_certs (gconf_connection);
if (!nm_connection_verify (gconf_connection, &error)) {
utils_clear_filled_connection_certs (gconf_connection);
g_warning ("%s: Invalid connection %s: '%s' / '%s' invalid: %d",
......@@ -180,7 +194,14 @@ nma_gconf_connection_changed (NMAGConfConnection *self)
/* Update private values to catch any certificate path changes */
nm_gconf_copy_private_connection_values (wrapped_connection, gconf_connection);
utils_fill_connection_certs (gconf_connection);
if (!utils_fill_connection_certs (gconf_connection, &error)) {
g_warning ("%s: Invalid connection %s: failed to load connection certificates: (%d) %s",
__func__, priv->dir,
error ? error->code : -1,
error && error->message ? error->message : "(unknown)");
goto invalid;
}
new_settings = nm_connection_to_hash (gconf_connection);
utils_clear_filled_connection_certs (gconf_connection);
......@@ -217,12 +238,23 @@ invalid:
static GHashTable *
get_settings (NMExportedConnection *exported)
{
NMAGConfConnection *self = NMA_GCONF_CONNECTION (exported);
NMAGConfConnectionPrivate *priv = NMA_GCONF_CONNECTION_GET_PRIVATE (self);
NMConnection *connection;
GHashTable *settings;
GError *error = NULL;
connection = nm_exported_connection_get_connection (exported);
utils_fill_connection_certs (connection);
if (!utils_fill_connection_certs (connection, &error)) {
g_warning ("%s: Invalid connection %s: failed to load connection certificates: (%d) %s",
__func__, priv->dir,
error ? error->code : -1,
error && error->message ? error->message : "(unknown)");
g_clear_error (&error);
return NULL;
}
settings = nm_connection_to_hash (connection);
utils_clear_filled_connection_certs (connection);
......@@ -515,7 +547,15 @@ constructor (GType type,
connection = nm_exported_connection_get_connection (NM_EXPORTED_CONNECTION (object));
utils_fill_connection_certs (connection);
if (!utils_fill_connection_certs (connection, &error)) {
g_warning ("%s: Invalid connection %s: failed to load connection certificates: (%d) %s",
__func__, priv->dir,
error ? error->code : -1,
error && error->message ? error->message : "(unknown)");
g_clear_error (&error);
goto err;
}
if (!nm_connection_verify (connection, &error)) {
utils_clear_filled_connection_certs (connection);
g_warning ("Invalid connection: '%s' / '%s' invalid: %d",
......
......@@ -23,6 +23,7 @@
#include <string.h>
#include <netinet/ether.h>
#include <glib.h>
#include <glib/gi18n.h>
#include <nm-device-ethernet.h>
#include <nm-device-wifi.h>
......@@ -244,25 +245,28 @@ fill_one_private_key (NMConnection *connection,
return need_client_cert;
}
void
utils_fill_connection_certs (NMConnection *connection)
gboolean
utils_fill_connection_certs (NMConnection *connection, GError **error)
{
NMSetting8021x *s_8021x;
const char *filename;
GError *error = NULL;
GError *tmp_error = NULL;
gboolean need_client_cert = TRUE;
g_return_if_fail (connection != NULL);
g_return_val_if_fail (connection != NULL, FALSE);
s_8021x = NM_SETTING_802_1X (nm_connection_get_setting (connection, NM_TYPE_SETTING_802_1X));
if (!s_8021x)
return;
return TRUE;
filename = g_object_get_data (G_OBJECT (connection), NMA_PATH_CA_CERT_TAG);
if (filename) {
if (!nm_setting_802_1x_set_ca_cert_from_file (s_8021x, filename, NULL, &error))
g_warning ("%s: couldn't read CA certificate: %d %s", __func__, error->code, error->message);
g_clear_error (&error);
if (!nm_setting_802_1x_set_ca_cert_from_file (s_8021x, filename, NULL, &tmp_error)) {
g_set_error (error, tmp_error->domain, tmp_error->code,
_("Could not read CA certificate: %s"), tmp_error->message);
g_clear_error (&tmp_error);
return FALSE;
}
}
/* If the private key is PKCS#12, don't set the client cert */
......@@ -273,17 +277,23 @@ utils_fill_connection_certs (NMConnection *connection)
if (need_client_cert) {
filename = g_object_get_data (G_OBJECT (connection), NMA_PATH_CLIENT_CERT_TAG);
if (filename) {
if (!nm_setting_802_1x_set_client_cert_from_file (s_8021x, filename, NULL, &error))
g_warning ("%s: couldn't read client certificate: %d %s", __func__, error->code, error->message);
g_clear_error (&error);
if (!nm_setting_802_1x_set_client_cert_from_file (s_8021x, filename, NULL, &tmp_error)) {
g_set_error (error, tmp_error->domain, tmp_error->code,
_("Could not read client certificate: %s"), tmp_error->message);
g_clear_error (&tmp_error);
return FALSE;
}
}
}
filename = g_object_get_data (G_OBJECT (connection), NMA_PATH_PHASE2_CA_CERT_TAG);
if (filename) {
if (!nm_setting_802_1x_set_phase2_ca_cert_from_file (s_8021x, filename, NULL, &error))
g_warning ("%s: couldn't read phase2 CA certificate: %d %s", __func__, error->code, error->message);
g_clear_error (&error);
if (!nm_setting_802_1x_set_phase2_ca_cert_from_file (s_8021x, filename, NULL, &tmp_error)) {
g_set_error (error, tmp_error->domain, tmp_error->code,
_("Could not read inner CA certificate: %s"), tmp_error->message);
g_clear_error (&tmp_error);
return FALSE;
}
}
/* If the private key is PKCS#12, don't set the client cert */
......@@ -294,11 +304,16 @@ utils_fill_connection_certs (NMConnection *connection)
if (need_client_cert) {
filename = g_object_get_data (G_OBJECT (connection), NMA_PATH_PHASE2_CLIENT_CERT_TAG);
if (filename) {
if (!nm_setting_802_1x_set_phase2_client_cert_from_file (s_8021x, filename, NULL, &error))
g_warning ("%s: couldn't read phase2 client certificate: %d %s", __func__, error->code, error->message);
g_clear_error (&error);
if (!nm_setting_802_1x_set_phase2_client_cert_from_file (s_8021x, filename, NULL, &tmp_error)) {
g_set_error (error, tmp_error->domain, tmp_error->code,
_("Could not read inner client certificate: %s"), tmp_error->message);
g_clear_error (&tmp_error);
return FALSE;
}
}
}
return TRUE;
}
void
......
......@@ -30,7 +30,7 @@
const char *utils_get_device_description (NMDevice *device);
void utils_fill_connection_certs (NMConnection *connection);
gboolean utils_fill_connection_certs (NMConnection *connection, GError **error);
void utils_clear_filled_connection_certs (NMConnection *connection);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment