[network-manager-libreswan/th/vpn-plugin-debug-bgo766872: 8/22] properties, service: check for errors in write_config_option()
- From: Thomas Haller <thaller src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [network-manager-libreswan/th/vpn-plugin-debug-bgo766872: 8/22] properties, service: check for errors in write_config_option()
- Date: Thu, 9 Jun 2016 15:45:07 +0000 (UTC)
commit bf3b8ca1e52714e9defcc9b5d8b425ec3ef9f1ce
Author: Thomas Haller <thaller redhat com>
Date: Thu May 26 12:07:34 2016 +0200
properties,service: check for errors in write_config_option()
Check for errors during write, retry on EINTR and retry on
incomplete writes.
When write() or close() fails, report an error instead of logging
a warning with g_warning(). Using g_warning() is not great when run
as a plugin (because a runtime failure should not cause messages in
stdout). And is not great for service either, because it bypasses
whatever logging we want to perform there.
By reporting errors back to the caller, we no longer need to print
a g_warning() inside the utility function.
po/POTFILES.in | 1 +
properties/nm-libreswan-editor-plugin.c | 10 ++++-
shared/nm-default.h | 2 +
shared/utils.c | 67 ++++++++++++++++++-------------
shared/utils.h | 43 ++++++++++++++++----
src/nm-libreswan-service.c | 22 +++++++---
6 files changed, 100 insertions(+), 45 deletions(-)
---
diff --git a/po/POTFILES.in b/po/POTFILES.in
index c98b70f..bfb0960 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -5,6 +5,7 @@ auth-dialog/main.c
properties/nm-libreswan-editor-plugin.c
properties/nm-libreswan-editor.c
src/nm-libreswan-service.c
+shared/utils.h
shared/nm-utils/nm-shared-utils.c
shared/nm-vpn/nm-vpn-plugin-utils.c
[type: gettext/glade]properties/nm-libreswan-dialog.ui
diff --git a/properties/nm-libreswan-editor-plugin.c b/properties/nm-libreswan-editor-plugin.c
index fd74e4a..d152ecc 100644
--- a/properties/nm-libreswan-editor-plugin.c
+++ b/properties/nm-libreswan-editor-plugin.c
@@ -140,11 +140,12 @@ export_to_file (NMVpnEditorPlugin *self,
NMSettingVpn *s_vpn;
gboolean openswan = FALSE;
int fd, errsv;
+ gs_free_error GError *local = NULL;
fd = g_open (path, O_WRONLY | O_CREAT, 0777);
if (fd == -1) {
errsv = errno;
- g_set_error (error, NMV_EDITOR_PLUGIN_ERROR, 0,
+ g_set_error (error, NMV_EDITOR_PLUGIN_ERROR, NMV_EDITOR_PLUGIN_ERROR_FAILED,
_("Can't open file '%s': %s"), path, g_strerror (errsv));
return FALSE;
}
@@ -153,7 +154,12 @@ export_to_file (NMVpnEditorPlugin *self,
if (s_vpn)
openswan = nm_streq (nm_setting_vpn_get_service_type (s_vpn), NM_VPN_SERVICE_TYPE_OPENSWAN);
- nm_libreswan_config_write (fd, connection, NULL, openswan);
+ if (!nm_libreswan_config_write (fd, connection, NULL, openswan, &local)) {
+ g_close (fd, NULL);
+ g_set_error (error, NMV_EDITOR_PLUGIN_ERROR, NMV_EDITOR_PLUGIN_ERROR_FAILED,
+ _("Error writing to file '%s': %s"), path, local->message);
+ return FALSE;
+ }
if (!g_close (fd, error))
return FALSE;
diff --git a/shared/nm-default.h b/shared/nm-default.h
index c46de33..8dfb6af 100644
--- a/shared/nm-default.h
+++ b/shared/nm-default.h
@@ -76,6 +76,7 @@
#define nm_simple_connection_new nm_connection_new
#define NMV_EDITOR_PLUGIN_ERROR NM_SETTING_VPN_ERROR
+#define NMV_EDITOR_PLUGIN_ERROR_FAILED NM_SETTING_VPN_ERROR_UNKNOWN
#define NMV_EDITOR_PLUGIN_ERROR_INVALID_PROPERTY NM_SETTING_VPN_ERROR_INVALID_PROPERTY
#define NMV_EDITOR_PLUGIN_ERROR_FILE_NOT_VPN NM_SETTING_VPN_ERROR_UNKNOWN
@@ -84,6 +85,7 @@
#include <NetworkManager.h>
#define NMV_EDITOR_PLUGIN_ERROR NM_CONNECTION_ERROR
+#define NMV_EDITOR_PLUGIN_ERROR_FAILED NM_CONNECTION_ERROR_FAILED
#define NMV_EDITOR_PLUGIN_ERROR_INVALID_PROPERTY NM_CONNECTION_ERROR_INVALID_PROPERTY
#define NMV_EDITOR_PLUGIN_ERROR_FILE_NOT_VPN NM_CONNECTION_ERROR_FAILED
diff --git a/shared/utils.c b/shared/utils.c
index 626936f..d6128f2 100644
--- a/shared/utils.c
+++ b/shared/utils.c
@@ -30,11 +30,12 @@
gboolean debug = FALSE;
-void
+gboolean
nm_libreswan_config_write (gint fd,
NMConnection *connection,
const char *bus_name,
- gboolean openswan)
+ gboolean openswan,
+ GError **error)
{
NMSettingVpn *s_vpn = nm_connection_get_setting_vpn (connection);
const char *con_name;
@@ -44,6 +45,8 @@ nm_libreswan_config_write (gint fd,
const char *phase2_alg_str;
const char *leftid;
+ g_return_val_if_fail (!error || !*error, FALSE);
+
/* We abuse the presence of bus name to decide if we're exporting
* the connection or actually configuring Pluto. */
if (bus_name)
@@ -57,56 +60,64 @@ nm_libreswan_config_write (gint fd,
leftid = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_LEFTID);
- write_config_option (fd, "conn %s", con_name);
+#define WRITE_CHECK_NEWLINE(fd, new_line, error, ...) \
+ G_STMT_START { \
+ if (!write_config_option_newline ((fd), (new_line), (error), __VA_ARGS__)) \
+ return FALSE; \
+ } G_STMT_END
+#define WRITE_CHECK(fd, error, ...) WRITE_CHECK_NEWLINE (fd, TRUE, error, __VA_ARGS__)
+
+ WRITE_CHECK (fd, error, "conn %s", con_name);
if (leftid) {
- write_config_option (fd, " aggrmode=yes");
- write_config_option (fd, " leftid= %s", leftid);
+ WRITE_CHECK (fd, error, " aggrmode=yes");
+ WRITE_CHECK (fd, error, " leftid= %s", leftid);
}
- write_config_option (fd, " authby=secret");
- write_config_option (fd, " left=%%defaultroute");
- write_config_option (fd, " leftxauthclient=yes");
- write_config_option (fd, " leftmodecfgclient=yes");
+ WRITE_CHECK (fd, error, " authby=secret");
+ WRITE_CHECK (fd, error, " left=%%defaultroute");
+ WRITE_CHECK (fd, error, " leftxauthclient=yes");
+ WRITE_CHECK (fd, error, " leftmodecfgclient=yes");
if (bus_name)
- write_config_option (fd, " leftupdown=\"" NM_LIBRESWAN_HELPER_PATH " --bus-name %s\"",
bus_name);
+ WRITE_CHECK (fd, error, " leftupdown=\"" NM_LIBRESWAN_HELPER_PATH " --bus-name %s\"",
bus_name);
default_username = nm_setting_vpn_get_user_name (s_vpn);
props_username = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_LEFTXAUTHUSER);
if (props_username && strlen (props_username))
- write_config_option (fd, " leftxauthusername=%s", props_username);
+ WRITE_CHECK (fd, error, " leftxauthusername=%s", props_username);
else if (default_username && strlen (default_username))
- write_config_option (fd, " leftxauthusername=%s", default_username);
+ WRITE_CHECK (fd, error, " leftxauthusername=%s", default_username);
- write_config_option (fd, " right=%s", nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_RIGHT));
- write_config_option (fd, " remote_peer_type=cisco");
- write_config_option (fd, " rightxauthserver=yes");
- write_config_option (fd, " rightmodecfgserver=yes");
- write_config_option (fd, " modecfgpull=yes");
- write_config_option (fd, " rightsubnet=0.0.0.0/0");
+ WRITE_CHECK (fd, error, " right=%s", nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_RIGHT));
+ WRITE_CHECK (fd, error, " remote_peer_type=cisco");
+ WRITE_CHECK (fd, error, " rightxauthserver=yes");
+ WRITE_CHECK (fd, error, " rightmodecfgserver=yes");
+ WRITE_CHECK (fd, error, " modecfgpull=yes");
+ WRITE_CHECK (fd, error, " rightsubnet=0.0.0.0/0");
phase1_alg_str = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_IKE);
if (!phase1_alg_str || !strlen (phase1_alg_str))
- write_config_option (fd, " ike=aes-sha1");
+ WRITE_CHECK (fd, error, " ike=aes-sha1");
else
- write_config_option (fd, " ike=%s", phase1_alg_str);
+ WRITE_CHECK (fd, error, " ike=%s", phase1_alg_str);
phase2_alg_str = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_ESP);
if (!phase2_alg_str || !strlen (phase2_alg_str))
- write_config_option (fd, " esp=aes-sha1;modp1024");
+ WRITE_CHECK (fd, error, " esp=aes-sha1;modp1024");
else
- write_config_option (fd, " esp=%s", phase2_alg_str);
+ WRITE_CHECK (fd, error, " esp=%s", phase2_alg_str);
- write_config_option (fd, " rekey=yes");
- write_config_option (fd, " salifetime=24h");
- write_config_option (fd, " ikelifetime=24h");
- write_config_option (fd, " keyingtries=1");
+ WRITE_CHECK (fd, error, " rekey=yes");
+ WRITE_CHECK (fd, error, " salifetime=24h");
+ WRITE_CHECK (fd, error, " ikelifetime=24h");
+ WRITE_CHECK (fd, error, " keyingtries=1");
if (!openswan && g_strcmp0 (nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_VENDOR), "Cisco") == 0)
- write_config_option (fd, " cisco-unity=yes");
+ WRITE_CHECK (fd, error, " cisco-unity=yes");
/* openswan requires a terminating \n (otherwise it segfaults) while
* libreswan fails parsing the configuration if you include the \n.
* WTF?
*/
- write_config_option_newline (fd, (openswan || !bus_name), " auto=add");
+ WRITE_CHECK_NEWLINE (fd, (openswan || !bus_name), error, " auto=add");
+ return TRUE;
}
diff --git a/shared/utils.h b/shared/utils.h
index c7e0379..9a2c25a 100644
--- a/shared/utils.h
+++ b/shared/utils.h
@@ -24,15 +24,20 @@
#ifndef __UTILS_H__
#define __UTILS_H__
+#include <errno.h>
+
extern gboolean debug;
-__attribute__((__format__ (__printf__, 3, 4)))
-static inline void
-write_config_option_newline (int fd, gboolean new_line, const char *format, ...)
+__attribute__((__format__ (__printf__, 4, 5)))
+static inline gboolean
+write_config_option_newline (int fd, gboolean new_line, GError **error, const char *format, ...)
{
gs_free char *string = NULL;
+ const char *p;
va_list args;
gsize l;
+ int errsv;
+ gssize w;
va_start (args, format);
string = g_strdup_vprintf (format, args);
@@ -52,15 +57,37 @@ write_config_option_newline (int fd, gboolean new_line, const char *format, ...)
l++;
}
- if (write (fd, string, l) <= 0)
- g_warning ("nm-libreswan: error in write_config_option");
+ p = string;
+ while (true) {
+ w = write (fd, p, l);
+ if (w == l)
+ return TRUE;
+ if (w > 0) {
+ g_assert (w < l);
+ p += w;
+ l -= w;
+ continue;
+ }
+ if (w == 0) {
+ errno = EIO;
+ break;
+ }
+ errsv = errno;
+ if (errsv == EINTR)
+ continue;
+ break;
+ }
+ g_set_error (error, NMV_EDITOR_PLUGIN_ERROR, NMV_EDITOR_PLUGIN_ERROR,
+ _("Error writing config: %s"), g_strerror (errsv));
+ return FALSE;
}
-#define write_config_option(fd, ...) write_config_option_newline((fd), TRUE, __VA_ARGS__)
+#define write_config_option(fd, error, ...) write_config_option_newline((fd), TRUE, error, __VA_ARGS__)
-void
+gboolean
nm_libreswan_config_write (gint fd,
NMConnection *connection,
const char *bus_name,
- gboolean openswan);
+ gboolean openswan,
+ GError **error);
#endif /* __UTILS_H__ */
diff --git a/src/nm-libreswan-service.c b/src/nm-libreswan-service.c
index fcf4f8a..2292906 100644
--- a/src/nm-libreswan-service.c
+++ b/src/nm-libreswan-service.c
@@ -49,6 +49,7 @@
#include <stdarg.h>
#include <pty.h>
#include <sys/types.h>
+#include <glib/gstdio.h>
#include "nm-libreswan-helper-service-dbus.h"
#include "utils.h"
@@ -654,6 +655,7 @@ nm_libreswan_config_psk_write (NMSettingVpn *s_vpn,
const char *pw_type, *psk, *leftid, *right;
int fd;
int errsv;
+ gboolean success;
/* Check for ignored group password */
pw_type = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_PSK_INPUT_MODES);
@@ -693,15 +695,18 @@ nm_libreswan_config_psk_write (NMSettingVpn *s_vpn,
leftid = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_LEFTID);
if (leftid) {
- write_config_option (fd, "@%s: PSK \"%s\"", leftid, psk);
+ success = write_config_option (fd, error, "@%s: PSK \"%s\"", leftid, psk);
} else {
right = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_RIGHT);
g_assert (right);
- write_config_option (fd, "%s %%any: PSK \"%s\"", right, psk);
+ success = write_config_option (fd, error, "%s %%any: PSK \"%s\"", right, psk);
}
- close (fd);
- return TRUE;
+ if (!success) {
+ g_close (fd, NULL);
+ return FALSE;
+ }
+ return g_close (fd, error);
}
/****************************************************************/
@@ -1598,10 +1603,13 @@ connect_step (NMLibreswanPlugin *self, GError **error)
return FALSE;
priv->watch_id = g_child_watch_add (priv->pid, child_watch_cb, self);
g_object_get (self, NM_VPN_SERVICE_PLUGIN_DBUS_SERVICE_NAME, &bus_name, NULL);
- nm_libreswan_config_write (fd, priv->connection, bus_name, priv->openswan);
+ if (!nm_libreswan_config_write (fd, priv->connection, bus_name, priv->openswan, error)) {
+ g_close (fd, NULL);
+ g_free (bus_name);
+ return FALSE;
+ }
g_free (bus_name);
- close (fd);
- return TRUE;
+ return g_close (fd, error);
case CONNECT_STEP_CONNECT:
g_assert (uuid);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
