NetworkManager-libreswan/SOURCES/0004-ipsec-conf-escaping-cve-2024-9050.patch

From dcf656747eece80e2534dc21b79c15e13bb28b5a Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Fri, 13 Sep 2024 14:49:12 +0200
Subject: [PATCH 1/6] shared/nm-glib: import newer g_steal_pointer()

The version that's there doesn't work with current glib, still
considering g_steal_pointer() deprecated.

We should probably do a full import, but it's became such a mess
in NetworkManager.git that it's not feasible at this point.

[lkundrak@v3.sk: Backported from 1.24.0]
---
 shared/nm-utils/nm-glib.h | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/shared/nm-utils/nm-glib.h b/shared/nm-utils/nm-glib.h
index 770cf0f..1b6487c 100644
--- a/shared/nm-utils/nm-glib.h
+++ b/shared/nm-utils/nm-glib.h
@@ -412,24 +412,26 @@ _nm_g_hash_table_get_keys_as_array (GHashTable *hash_table,
 
 /*****************************************************************************/
 
-#if !GLIB_CHECK_VERSION(2, 44, 0)
-static inline gpointer
-g_steal_pointer (gpointer pp)
-{
-	gpointer *ptr = (gpointer *) pp;
-	gpointer ref;
+#define _NM_ENSURE_POINTER(value)                                                 \
+    do {                                                                          \
+        _nm_unused const void *const _unused_for_type_check = 0 ? (value) : NULL; \
+    } while (0)
 
-	ref = *ptr;
-	*ptr = NULL;
-
-	return ref;
-}
-
-/* type safety */
-#define g_steal_pointer(pp) \
-  (0 ? (*(pp)) : (g_steal_pointer) (pp))
+#ifdef g_steal_pointer
+#undef g_steal_pointer
 #endif
 
+#define g_steal_pointer(pp)              \
+    ({                                   \
+        typeof(*(pp)) *const _pp = (pp); \
+        typeof(*_pp)         _p  = *_pp; \
+                                         \
+        _NM_ENSURE_POINTER(_p);          \
+                                         \
+        *_pp = NULL;                     \
+        _p;                              \
+    })
+
 /*****************************************************************************/
 
 static inline gboolean
-- 
2.46.0

From 50b019de99a9005065db6d069167ffacbe62151b Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Tue, 17 Sep 2024 13:28:58 +0200
Subject: [PATCH 2/6] build: get rid of {properties,src}/libutils.la

Useless build of an extra libraries, just making the build slower and
more complicated. Get rid of then, and just roll src/libutils.la.

[lkundrak@v3.sk: Backported from 1.24.0]
---
 Makefile.am | 47 ++++++++++++++++++-----------------------------
 1 file changed, 18 insertions(+), 29 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 8442d64..e2847d4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -53,23 +53,25 @@ common_CFLAGS = \
 
 ###############################################################################
 
-noinst_LTLIBRARIES += properties/libutils.la
+noinst_LTLIBRARIES += shared/libutils.la
 
-properties_libutils_la_SOURCES = \
-	shared/utils.c \
-	shared/utils.h \
-	shared/nm-utils/nm-vpn-plugin-utils.c \
-	shared/nm-utils/nm-vpn-plugin-utils.h \
+shared_libutils_la_SOURCES = \
 	shared/nm-utils/nm-shared-utils.c \
 	shared/nm-utils/nm-shared-utils.h \
+	shared/utils.c \
+	shared/utils.h \
 	shared/nm-service-defines.h
 
-properties_libutils_la_CPPFLAGS = \
-	-DNETWORKMANAGER_COMPILATION=NM_NETWORKMANAGER_COMPILATION_LIB_BASE \
-	-DNM_PLUGIN_DIR=\"$(NM_PLUGIN_DIR)\" \
+shared_libutils_la_CFLAGS = \
 	$(common_CFLAGS) \
 	$(LIBNM_CFLAGS)
 
+shared_libutils_la_LIBADD = \
+	$(GLIB_LIBS) \
+	$(LIBNM_LIBS)
+
+###############################################################################
+
 plugin_LTLIBRARIES += properties/libnm-vpn-plugin-libreswan.la
 
 properties_libnm_vpn_plugin_libreswan_la_CFLAGS = \
@@ -79,10 +81,13 @@ properties_libnm_vpn_plugin_libreswan_la_CFLAGS = \
 	$(LIBNM_CFLAGS)
 
 properties_libnm_vpn_plugin_libreswan_la_SOURCES = \
-	$(plugin_sources)
+	shared/nm-utils/nm-vpn-plugin-utils.c \
+	shared/nm-utils/nm-vpn-plugin-utils.h \
+	properties/nm-libreswan-editor-plugin.c \
+	properties/nm-libreswan-editor-plugin.h
 
 properties_libnm_vpn_plugin_libreswan_la_LIBADD = \
-	properties/libutils.la \
+	shared/libutils.la \
 	$(LIBNM_LIBS) \
 	$(DL_LIBS)
 
@@ -216,22 +221,6 @@ src/nm-libreswan-helper-service-dbus.h: src/nm-libreswan-helper-service.xml
 src/nm-libreswan-helper-service-dbus.c: src/nm-libreswan-helper-service-dbus.h
 	@true
 
-noinst_LTLIBRARIES += src/libutils.la
-
-src_libutils_la_SOURCES = \
-	shared/nm-utils/nm-shared-utils.c \
-	shared/nm-utils/nm-shared-utils.h \
-	shared/utils.c \
-	shared/utils.h \
-	shared/nm-service-defines.h
-
-src_libutils_la_CPPFLAGS = \
-	$(src_cppflags)
-
-src_libutils_la_LIBADD = \
-	$(GLIB_LIBS) \
-	$(LIBNM_LIBS)
-
 ###############################################################################
 
 libexec_PROGRAMS += src/nm-libreswan-service
@@ -241,7 +230,7 @@ src_nm_libreswan_service_CPPFLAGS = \
 
 src_nm_libreswan_service_LDADD = \
 	src/libnm-libreswan-helper-service-dbus.la \
-	src/libutils.la \
+	shared/libutils.la \
 	$(GLIB_LIBS) \
 	$(LIBNM_LIBS) \
 	$(LIBNL_LIBS) \
@@ -258,7 +247,7 @@ src_nm_libreswan_service_helper_CPPFLAGS = \
 
 src_nm_libreswan_service_helper_LDADD = \
 	src/libnm-libreswan-helper-service-dbus.la \
-	src/libutils.la \
+	shared/libutils.la \
 	$(GLIB_LIBS) \
 	$(LIBNM_LIBS)
 
-- 
2.46.0

From a076344da47a3ec930f01d7b70d1929431c301cc Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Mon, 23 Sep 2024 11:39:22 +0200
Subject: [PATCH 3/6] shared/test-utils: cover config write with unit tests

Test that we generate good configuration for typical IKEv1 and IKEv2
cases.

[lkundrak@v3.sk: Backported from 1.24.0]
---
 Makefile.am         |  16 ++++++
 shared/test-utils.c | 127 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 143 insertions(+)
 create mode 100644 shared/test-utils.c

diff --git a/Makefile.am b/Makefile.am
index e2847d4..d97d3c3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -19,6 +19,8 @@ libexec_PROGRAMS =
 
 noinst_PROGRAMS =
 
+TESTS =
+
 SUBDIRS = po man
 
 ###############################################################################
@@ -70,6 +72,20 @@ shared_libutils_la_LIBADD = \
 	$(GLIB_LIBS) \
 	$(LIBNM_LIBS)
 
+noinst_PROGRAMS += shared/test-utils
+
+TESTS += shared/test-utils
+
+shared_test_utils_SOURCES = shared/test-utils.c
+
+shared_test_utils_CFLAGS = \
+	$(common_CFLAGS) \
+	$(LIBNM_CFLAGS)
+
+shared_test_utils_LDADD = \
+	shared/libutils.la \
+	$(LIBNM_LIBS)
+
 ###############################################################################
 
 plugin_LTLIBRARIES += properties/libnm-vpn-plugin-libreswan.la
diff --git a/shared/test-utils.c b/shared/test-utils.c
new file mode 100644
index 0000000..82ee933
--- /dev/null
+++ b/shared/test-utils.c
@@ -0,0 +1,127 @@
+#include "nm-default.h"
+
+#include "utils.h"
+
+#include <gio/gfiledescriptorbased.h>
+
+static char *
+_setting_into_ipsec_conf (NMSetting *s_vpn, const char *name, GError **error)
+{
+	gs_unref_object NMConnection *connection = NULL;
+	gs_unref_object GFile *tmp = NULL;
+	GFileIOStream *tmpstream;
+	char buf[4096];
+	gboolean res;
+	gsize count;
+	gint fd;
+
+	connection = nm_simple_connection_new ();
+	nm_connection_add_setting (connection, s_vpn);
+
+	tmp = g_file_new_tmp (NULL, &tmpstream, error);
+	if (tmp == NULL)
+		return NULL;
+
+	res = g_file_delete (tmp, NULL, error);
+	if (res == FALSE)
+		return NULL;
+
+	fd = g_file_descriptor_based_get_fd(G_FILE_DESCRIPTOR_BASED(
+		g_io_stream_get_output_stream(G_IO_STREAM (tmpstream))));
+
+	res = nm_libreswan_config_write (fd, 4, connection,
+	                                 name, NULL,
+	                                 FALSE, TRUE, NULL,
+	                                 error);
+	if (res == FALSE)
+		return NULL;
+
+	res = g_seekable_seek (G_SEEKABLE(tmpstream),
+	                       0, G_SEEK_SET, NULL, error);
+	if (res == FALSE)
+		return NULL;
+
+	res = g_input_stream_read_all (
+		g_io_stream_get_input_stream(G_IO_STREAM (tmpstream)),
+		buf,
+		sizeof(buf)-1,
+		&count,
+		NULL,
+		error);
+	if (res == FALSE)
+		return NULL;
+
+	buf[count] = '\0';
+	return g_strdup(buf);
+}
+
+static void
+test_config_write (void)
+{
+	GError *error = NULL;
+	NMSetting *s_vpn;
+	char *str;
+
+	s_vpn = nm_setting_vpn_new ();
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "right", "11.12.13.14");
+	str = _setting_into_ipsec_conf (s_vpn, "con_name", &error);
+	g_assert_no_error (error);
+	g_assert_cmpstr (str, ==,
+	                 "conn con_name\n"
+	                 " authby=secret\n"
+	                 " left=%defaultroute\n"
+	                 " leftmodecfgclient=yes\n"
+	                 " right=11.12.13.14\n"
+	                 " rightmodecfgserver=yes\n"
+	                 " modecfgpull=yes\n"
+	                 " rightsubnet=0.0.0.0/0\n"
+	                 " leftxauthclient=yes\n"
+	                 " remote-peer-type=cisco\n"
+	                 " rightxauthserver=yes\n"
+	                 " ikelifetime=24h\n"
+	                 " salifetime=24h\n"
+	                 " rekey=yes\n"
+	                 " keyingtries=1\n"
+	                 " ikev2=never\n"
+	                 " nm-configured=yes\n"
+	                 " auto=add\n");
+	g_free (str);
+
+	s_vpn = nm_setting_vpn_new ();
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "ikev2", "insist");
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "leftcert", "LibreswanClient");
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "leftid", "%fromcert");
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "right", "11.12.13.14");
+	str = _setting_into_ipsec_conf (s_vpn,
+	                                "f0008435-07af-4836-a53d-b43e8730e68f",
+	                                &error);
+	g_assert_no_error (error);
+	g_assert_cmpstr (str, ==,
+	                 "conn f0008435-07af-4836-a53d-b43e8730e68f\n"
+	                 " leftid=%fromcert\n"
+	                 " leftcert=LibreswanClient\n"
+	                 " leftrsasigkey=%cert\n"
+	                 " rightrsasigkey=%cert\n"
+	                 " left=%defaultroute\n"
+	                 " leftmodecfgclient=yes\n"
+	                 " right=11.12.13.14\n"
+	                 " rightmodecfgserver=yes\n"
+	                 " modecfgpull=yes\n"
+	                 " rightsubnet=0.0.0.0/0\n"
+	                 " rekey=yes\n"
+	                 " keyingtries=1\n"
+	                 " ikev2=insist\n"
+	                 " nm-configured=yes\n"
+	                 " auto=add\n");
+	g_free (str);
+}
+
+int
+main (int argc, char **argv)
+{
+	g_test_init (&argc, &argv, NULL);
+
+	g_test_add_func ("/utils/config/write", test_config_write);
+
+	return g_test_run ();
+}
-- 
2.46.0

From 486c9e7a8517a1f376cd9f290e2e16298770e004 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Sun, 22 Sep 2024 14:20:22 +0200
Subject: [PATCH 4/6] all: rework formatting of ipsec.conf

Simplify the interface and validate each item carefully.

Note this fixes a security issue (CVE-2024-9050), with an "Important"
impact severity, where insufficient validation could lead to injection
of potentialy malicious values into the ipsec.conf snippet we send over
to pluto.

https://issues.redhat.com/browse/RHEL-59565

[lkundrak@v3.sk: Backported from 1.24.0]
---
 properties/nm-libreswan-editor-plugin.c |  29 +-
 shared/test-utils.c                     |  99 ++----
 shared/utils.c                          | 407 ++++++++++++++----------
 shared/utils.h                          |  26 +-
 src/nm-libreswan-service.c              | 176 +++++-----
 5 files changed, 375 insertions(+), 362 deletions(-)

diff --git a/properties/nm-libreswan-editor-plugin.c b/properties/nm-libreswan-editor-plugin.c
index d6b63f2..6f23936 100644
--- a/properties/nm-libreswan-editor-plugin.c
+++ b/properties/nm-libreswan-editor-plugin.c
@@ -266,38 +266,25 @@ 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, 0666);
-	if (fd == -1) {
-		errsv = errno;
-		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;
-	}
+	gs_free char *ipsec_conf = NULL;
 
 	s_vpn = nm_connection_get_setting_vpn (connection);
 	if (s_vpn)
 		openswan = nm_streq (nm_setting_vpn_get_service_type (s_vpn), NM_VPN_SERVICE_TYPE_OPENSWAN);
 
-	if (!nm_libreswan_config_write (fd,
-	                                connection,
-	                                nm_connection_get_id (connection),
-	                                NULL,
-	                                openswan,
-	                                TRUE,
-	                                NULL,
-	                                &local)) {
-		g_close (fd, NULL);
+	ipsec_conf = nm_libreswan_get_ipsec_conf (s_vpn,
+	                                          nm_connection_get_id (connection),
+	                                          NULL, openswan, TRUE, error);
+	if (ipsec_conf == NULL)
+		return FALSE;
+
+	if (!g_file_set_contents (path, ipsec_conf, -1, &local)) {
 		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;
-
 	return TRUE;
 }
 
diff --git a/shared/test-utils.c b/shared/test-utils.c
index 82ee933..965daef 100644
--- a/shared/test-utils.c
+++ b/shared/test-utils.c
@@ -2,117 +2,60 @@
 
 #include "utils.h"
 
-#include <gio/gfiledescriptorbased.h>
-
-static char *
-_setting_into_ipsec_conf (NMSetting *s_vpn, const char *name, GError **error)
-{
-	gs_unref_object NMConnection *connection = NULL;
-	gs_unref_object GFile *tmp = NULL;
-	GFileIOStream *tmpstream;
-	char buf[4096];
-	gboolean res;
-	gsize count;
-	gint fd;
-
-	connection = nm_simple_connection_new ();
-	nm_connection_add_setting (connection, s_vpn);
-
-	tmp = g_file_new_tmp (NULL, &tmpstream, error);
-	if (tmp == NULL)
-		return NULL;
-
-	res = g_file_delete (tmp, NULL, error);
-	if (res == FALSE)
-		return NULL;
-
-	fd = g_file_descriptor_based_get_fd(G_FILE_DESCRIPTOR_BASED(
-		g_io_stream_get_output_stream(G_IO_STREAM (tmpstream))));
-
-	res = nm_libreswan_config_write (fd, 4, connection,
-	                                 name, NULL,
-	                                 FALSE, TRUE, NULL,
-	                                 error);
-	if (res == FALSE)
-		return NULL;
-
-	res = g_seekable_seek (G_SEEKABLE(tmpstream),
-	                       0, G_SEEK_SET, NULL, error);
-	if (res == FALSE)
-		return NULL;
-
-	res = g_input_stream_read_all (
-		g_io_stream_get_input_stream(G_IO_STREAM (tmpstream)),
-		buf,
-		sizeof(buf)-1,
-		&count,
-		NULL,
-		error);
-	if (res == FALSE)
-		return NULL;
-
-	buf[count] = '\0';
-	return g_strdup(buf);
-}
-
 static void
 test_config_write (void)
 {
 	GError *error = NULL;
-	NMSetting *s_vpn;
+	NMSettingVpn *s_vpn;
 	char *str;
 
-	s_vpn = nm_setting_vpn_new ();
+	s_vpn = NM_SETTING_VPN (nm_setting_vpn_new ());
 	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "right", "11.12.13.14");
-	str = _setting_into_ipsec_conf (s_vpn, "con_name", &error);
+	str = nm_libreswan_get_ipsec_conf (s_vpn, "con_name", NULL, FALSE, TRUE, &error);
 	g_assert_no_error (error);
 	g_assert_cmpstr (str, ==,
 	                 "conn con_name\n"
+	                 " ikev2=never\n"
+	                 " right=11.12.13.14\n"
 	                 " authby=secret\n"
 	                 " left=%defaultroute\n"
 	                 " leftmodecfgclient=yes\n"
-	                 " right=11.12.13.14\n"
-	                 " rightmodecfgserver=yes\n"
-	                 " modecfgpull=yes\n"
 	                 " rightsubnet=0.0.0.0/0\n"
 	                 " leftxauthclient=yes\n"
-	                 " remote-peer-type=cisco\n"
+	                 " remote_peer_type=cisco\n"
 	                 " rightxauthserver=yes\n"
 	                 " ikelifetime=24h\n"
 	                 " salifetime=24h\n"
-	                 " rekey=yes\n"
 	                 " keyingtries=1\n"
-	                 " ikev2=never\n"
-	                 " nm-configured=yes\n"
-	                 " auto=add\n");
+	                 " rekey=yes\n"
+	                 " rightmodecfgserver=yes\n"
+	                 " modecfgpull=yes\n");
 	g_free (str);
 
-	s_vpn = nm_setting_vpn_new ();
+	s_vpn = NM_SETTING_VPN (nm_setting_vpn_new ());
 	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "ikev2", "insist");
 	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "leftcert", "LibreswanClient");
 	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "leftid", "%fromcert");
 	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "right", "11.12.13.14");
-	str = _setting_into_ipsec_conf (s_vpn,
-	                                "f0008435-07af-4836-a53d-b43e8730e68f",
-	                                &error);
+	str = nm_libreswan_get_ipsec_conf (s_vpn,
+	                                   "f0008435-07af-4836-a53d-b43e8730e68f",
+			                   NULL, FALSE, TRUE, &error);
 	g_assert_no_error (error);
 	g_assert_cmpstr (str, ==,
 	                 "conn f0008435-07af-4836-a53d-b43e8730e68f\n"
+	                 " ikev2=insist\n"
+	                 " right=11.12.13.14\n"
 	                 " leftid=%fromcert\n"
-	                 " leftcert=LibreswanClient\n"
-	                 " leftrsasigkey=%cert\n"
-	                 " rightrsasigkey=%cert\n"
+	                 " leftcert=\"LibreswanClient\"\n"
+	                 " leftrsasigkey=\"%cert\"\n"
+	                 " rightrsasigkey=\"%cert\"\n"
 	                 " left=%defaultroute\n"
 	                 " leftmodecfgclient=yes\n"
-	                 " right=11.12.13.14\n"
-	                 " rightmodecfgserver=yes\n"
-	                 " modecfgpull=yes\n"
 	                 " rightsubnet=0.0.0.0/0\n"
-	                 " rekey=yes\n"
 	                 " keyingtries=1\n"
-	                 " ikev2=insist\n"
-	                 " nm-configured=yes\n"
-	                 " auto=add\n");
+	                 " rekey=yes\n"
+	                 " rightmodecfgserver=yes\n"
+	                 " modecfgpull=yes\n");
 	g_free (str);
 }
 
diff --git a/shared/utils.c b/shared/utils.c
index 36af877..7533f7f 100644
--- a/shared/utils.c
+++ b/shared/utils.c
@@ -30,81 +30,108 @@
 #include <string.h>
 #include <errno.h>
 
-gboolean
-write_config_option_newline (int fd,
-                             gboolean new_line,
-                             NMDebugWriteFcn debug_write_fcn,
-                             GError **error,
-                             const char *format, ...)
+static gboolean
+printable_val (GString *str, const char *val, GError **error)
 {
-	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);
-	va_end (args);
+	g_return_val_if_fail (val, FALSE);
 
-	if (debug_write_fcn)
-		debug_write_fcn (string);
-
-	l = strlen (string);
-	if (new_line) {
-		gs_free char *s = string;
-
-		string = g_new (char, l + 1 + 1);
-		memcpy (string, s, l);
-		string[l] = '\n';
-		string[l + 1] = '\0';
-		l++;
+	for (p = val; *p != '\0'; p++) {
+		/* Printable characters except " and space allowed. */
+		if (*p != '"' && !g_ascii_isspace (*p) && g_ascii_isprint (*p))
+			continue;
+		g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_INVALID_ARGUMENT,
+			     _("Invalid character in '%s'"), val);
+		return FALSE;
 	}
 
-	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) {
-			errsv = 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;
+	g_string_append (str, val);
+	g_string_append_c (str, '\n');
+	return TRUE;
 }
 
-gboolean
-nm_libreswan_config_write (gint fd,
-                           NMConnection *connection,
-                           const char *con_name,
-                           const char *leftupdown_script,
-                           gboolean openswan,
-                           gboolean trailing_newline,
-                           NMDebugWriteFcn debug_write_fcn,
-                           GError **error)
+static gboolean
+string_val (GString *str, const char *val, GError **error)
 {
-	NMSettingVpn *s_vpn;
-	const char *props_username;
-	const char *default_username;
+	const char *p;
+
+	g_return_val_if_fail (val, FALSE);
+
+	for (p = val; *p != '\0'; p++) {
+		/* Printable characters except " allowed. */
+		if (*p != '"' && g_ascii_isprint (*p))
+			continue;
+		g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_INVALID_ARGUMENT,
+			     _("Invalid character in '%s'"), val);
+		return FALSE;
+	}
+
+	g_string_append_printf (str, "\"%s\"\n", val);
+	return TRUE;
+}
+
+static inline gboolean
+optional_string_val (GString *str, const char *key, const char *val, GError **error)
+{
+	if (val == NULL || val[0] == '\0')
+		return TRUE;
+	g_string_append_c (str, ' ');
+	g_string_append (str, key);
+	g_string_append_c (str, '=');
+
+	if (!string_val (str, val, error)) {
+		g_prefix_error (error, _("Invalid value for '%s': "), key);
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+static inline gboolean
+optional_printable_val (GString *str, const char *key, const char *val, GError **error)
+{
+	if (val == NULL || val[0] == '\0')
+		return TRUE;
+
+	g_string_append_c (str, ' ');
+	g_string_append (str, key);
+	g_string_append_c (str, '=');
+
+	if (!printable_val (str, val, error)) {
+		g_prefix_error (error, _("Invalid value for '%s': "), key);
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+
+static inline gboolean
+optional_printable (GString *str, NMSettingVpn *s_vpn, const char *key, GError **error)
+{
+	return optional_printable_val (str,
+	                               key,
+	                               nm_setting_vpn_get_data_item (s_vpn, key),
+	                               error);
+}
+
+char *
+nm_libreswan_get_ipsec_conf (NMSettingVpn *s_vpn,
+                             const char *con_name,
+                             const char *leftupdown_script,
+                             gboolean openswan,
+                             gboolean trailing_newline,
+                             GError **error)
+{
+	nm_auto_free_gstring GString *ipsec_conf = NULL;
+	const char *username;
 	const char *phase1_alg_str;
 	const char *phase2_alg_str;
 	const char *phase1_lifetime_str;
 	const char *phase2_lifetime_str;
 	const char *left;
+	const char *right;
 	const char *leftid;
 	const char *leftcert;
 	const char *leftrsasigkey;
@@ -112,122 +139,158 @@ nm_libreswan_config_write (gint fd,
 	const char *remote_network;
 	const char *ikev2 = NULL;
 	const char *rightid;
-	const char *narrowing;
 	const char *rekey;
-	const char *fragmentation;
-	const char *mobike;
 	const char *pfs;
 	gboolean is_ikev2 = FALSE;
 
-	g_return_val_if_fail (fd > 0, FALSE);
-	g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
+	g_return_val_if_fail (NM_IS_SETTING_VPN (s_vpn), FALSE);
 	g_return_val_if_fail (!error || !*error, FALSE);
 	g_return_val_if_fail (con_name && *con_name, FALSE);
 
-	s_vpn = nm_connection_get_setting_vpn (connection);
-	g_return_val_if_fail (NM_IS_SETTING_VPN (s_vpn), FALSE);
+	ipsec_conf = g_string_sized_new (1024);
+	g_string_append (ipsec_conf, "conn ");
+	if (!printable_val (ipsec_conf, con_name, error)) {
+		g_prefix_error (error, _("Bad connection name: "));
+		return FALSE;
+	}
 
-	is_ikev2 = nm_libreswan_utils_setting_is_ikev2 (s_vpn, &ikev2);
+	if (leftupdown_script) {
+		g_string_append (ipsec_conf, " auto=add\n");
+		g_string_append (ipsec_conf, " nm-configured=yes\n");
+		g_string_append (ipsec_conf, " leftupdown=");
+		if (!string_val (ipsec_conf, leftupdown_script, error))
+			g_return_val_if_reached (FALSE);
+	}
 
 	/* When using IKEv1 (default in our plugin), we should ensure that we make
 	 * it explicit to Libreswan (which now defaults to IKEv2): when crypto algorithms
 	 * are not specified ("esp" & "ike") Libreswan will use system-wide crypto
 	 * policies based on the IKE version in place.
 	 */
+	is_ikev2 = nm_libreswan_utils_setting_is_ikev2 (s_vpn, &ikev2);
 	if (!ikev2)
 		ikev2 = NM_LIBRESWAN_IKEV2_NEVER;
+	g_string_append (ipsec_conf, " ikev2=");
+	if (!printable_val (ipsec_conf, ikev2, error)) {
+		g_prefix_error (error, _("Invalid value for '%s': "), "ikev2");
+		return FALSE;
+	}
+
+	right = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_RIGHT);
+	if (right && right[0] != '\0') {
+		g_string_append (ipsec_conf, " right=");
+		if (!printable_val (ipsec_conf, right, error)) {
+			g_prefix_error (error, _("Invalid value for '%s': "),
+					NM_LIBRESWAN_KEY_RIGHT);
+			return FALSE;
+		}
+	} else {
+		g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_INVALID_ARGUMENT,
+			     _("'%s' key needs to be present."), NM_LIBRESWAN_KEY_RIGHT);
+		return FALSE;
+	}
 
 	leftid = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_LEFTID);
-
-#define WRITE_CHECK_NEWLINE(fd, new_line, debug_write_fcn, error, ...) \
-	G_STMT_START { \
-		if (!write_config_option_newline ((fd), (new_line), debug_write_fcn, (error), __VA_ARGS__)) \
-			return FALSE; \
-	} G_STMT_END
-#define WRITE_CHECK(fd, debug_write_fcn, error, ...) WRITE_CHECK_NEWLINE (fd, TRUE, debug_write_fcn, error, __VA_ARGS__)
-
-	WRITE_CHECK (fd, debug_write_fcn, error, "conn %s", con_name);
-	if (leftid && strlen (leftid)) {
+	if (leftid && leftid[0] != '\0') {
 		if (!is_ikev2)
-			WRITE_CHECK (fd, debug_write_fcn, error, " aggrmode=yes");
+			g_string_append (ipsec_conf, " aggrmode=yes\n");
 
 		if (   leftid[0] == '%'
 		    || leftid[0] == '@'
 		    || nm_utils_parse_inaddr_bin (AF_UNSPEC, leftid, NULL)) {
-			WRITE_CHECK (fd, debug_write_fcn, error, " leftid=%s", leftid);
+			g_string_append (ipsec_conf, " leftid=");
 		} else
-			WRITE_CHECK (fd, debug_write_fcn, error, " leftid=@%s", leftid);
+			g_string_append (ipsec_conf, " leftid=@");
+		if (!printable_val (ipsec_conf, leftid, error)) {
+			g_prefix_error (error, _("Invalid value for '%s': "),
+			                NM_LIBRESWAN_KEY_LEFTID);
+			return FALSE;
+		}
 	}
 
 	leftrsasigkey = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_LEFTRSASIGKEY);
 	rightrsasigkey = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_RIGHTRSASIGKEY);
 	leftcert = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_LEFTCERT);
-	if (leftcert && strlen (leftcert)) {
-		WRITE_CHECK (fd, debug_write_fcn, error, " leftcert=%s", leftcert);
+	if (leftcert && leftcert[0] != '\0') {
+		g_string_append (ipsec_conf, " leftcert=");
+		if (!string_val (ipsec_conf, leftcert, error)) {
+			g_prefix_error (error, _("Invalid value for '%s': "),
+			                NM_LIBRESWAN_KEY_LEFTCERT);
+			return FALSE;
+		}
 		if (!leftrsasigkey)
 			leftrsasigkey = "%cert";
 		if (!rightrsasigkey)
 			rightrsasigkey = "%cert";
 	}
-	if (leftrsasigkey && strlen (leftrsasigkey))
-		WRITE_CHECK (fd, debug_write_fcn, error, " leftrsasigkey=%s", leftrsasigkey);
-	if (rightrsasigkey && strlen (rightrsasigkey))
-		WRITE_CHECK (fd, debug_write_fcn, error, " rightrsasigkey=%s", rightrsasigkey);
-	if (   !(leftrsasigkey && strlen (leftrsasigkey))
-	    && !(rightrsasigkey && strlen (rightrsasigkey))) {
-		WRITE_CHECK (fd, debug_write_fcn, error, " authby=secret");
+	if (!optional_string_val (ipsec_conf, NM_LIBRESWAN_KEY_LEFTRSASIGKEY, leftrsasigkey, error))
+		return FALSE;
+	if (!optional_string_val (ipsec_conf, NM_LIBRESWAN_KEY_RIGHTRSASIGKEY, rightrsasigkey, error))
+		return FALSE;
+	if (   !(leftrsasigkey && leftrsasigkey[0] != '\0')
+	    && !(rightrsasigkey && rightrsasigkey[0] != '\0')) {
+		g_string_append (ipsec_conf, " authby=secret\n");
 	}
 
 	left = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_LEFT);
-	if (left && strlen (left))
-		WRITE_CHECK (fd, debug_write_fcn, error, " left=%s", left);
-	else
-		WRITE_CHECK (fd, debug_write_fcn, error, " left=%%defaultroute");
+	if (left == NULL || left[0] == '\0')
+		left = "%defaultroute";
+	g_string_append (ipsec_conf, " left=");
+	if (!printable_val (ipsec_conf, left, error)) {
+		g_prefix_error (error, _("Invalid value for '%s': "),
+		                NM_LIBRESWAN_KEY_LEFT);
+		return FALSE;
+	}
 
-	WRITE_CHECK (fd, debug_write_fcn, error, " leftmodecfgclient=yes");
-	if (leftupdown_script)
-		WRITE_CHECK (fd, debug_write_fcn, error, " leftupdown=%s", leftupdown_script);
+	g_string_append (ipsec_conf, " leftmodecfgclient=yes\n");
 
-	WRITE_CHECK (fd, debug_write_fcn, error, " right=%s", nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_RIGHT));
 	rightid = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_RIGHTID);
-	if (rightid && strlen (rightid)) {
+	if (rightid && rightid[0] != '\0') {
 		if (   rightid[0] == '@'
 		    || rightid[0] == '%'
-		    ||  nm_utils_parse_inaddr_bin (AF_UNSPEC, rightid, NULL)) {
-			WRITE_CHECK (fd, debug_write_fcn, error, " rightid=%s", rightid);
-		} else
-			WRITE_CHECK (fd, debug_write_fcn, error, " rightid=@%s", rightid);
+		    || nm_utils_parse_inaddr_bin (AF_UNSPEC, rightid, NULL)) {
+			g_string_append (ipsec_conf, " rightid=");
+		} else {
+			g_string_append (ipsec_conf, " rightid=@");
+		}
+		if (!printable_val (ipsec_conf, rightid, error)) {
+			g_prefix_error (error, _("Invalid value for '%s': "),
+			                NM_LIBRESWAN_KEY_RIGHTID);
+			return FALSE;
+		}
 	}
-	WRITE_CHECK (fd, debug_write_fcn, error, " rightmodecfgserver=yes");
-	WRITE_CHECK (fd, debug_write_fcn, error, " modecfgpull=yes");
 
-	remote_network = nm_setting_vpn_get_data_item (s_vpn,
-						       NM_LIBRESWAN_KEY_REMOTENETWORK);
-	if (!remote_network || !strlen (remote_network))
-		WRITE_CHECK (fd, debug_write_fcn, error, " rightsubnet=0.0.0.0/0");
-	else
-		WRITE_CHECK (fd, debug_write_fcn, error, " rightsubnet=%s",
-			     remote_network);
+	remote_network = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_REMOTENETWORK);
+	if (!remote_network || remote_network[0] == '\0')
+		remote_network = "0.0.0.0/0";
+	g_string_append (ipsec_conf, " rightsubnet=");
+	if (!printable_val (ipsec_conf, remote_network, error)) {
+		g_prefix_error (error, _("Invalid value for '%s': "),
+				NM_LIBRESWAN_KEY_REMOTENETWORK);
+		return FALSE;
+	}
 
 	if (!is_ikev2) {
 		/* When IKEv1 is in place, we enforce XAUTH: so, use IKE version
 		 * also to check if XAUTH conf options should be passed to Libreswan.
 		 */
-		WRITE_CHECK (fd, debug_write_fcn, error, " leftxauthclient=yes");
+		g_string_append (ipsec_conf, " leftxauthclient=yes\n");
 
-		default_username = nm_setting_vpn_get_user_name (s_vpn);
-		props_username = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_LEFTXAUTHUSER);
-		if (props_username && strlen (props_username))
-			WRITE_CHECK (fd, debug_write_fcn, error, " leftxauthusername=%s", props_username);
-		else if (default_username && strlen (default_username))
-			WRITE_CHECK (fd, debug_write_fcn, error, " leftxauthusername=%s", default_username);
+		username = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_LEFTXAUTHUSER);
+		if (username == NULL || username[0] == '\0')
+			username = nm_setting_vpn_get_user_name (s_vpn);
+		if (username != NULL && username[0] != '\0') {
+			g_string_append (ipsec_conf, " leftxauthusername=");
+			if (!string_val (ipsec_conf, username, error)) {
+				g_prefix_error (error, _("Invalid username: "));
+				return FALSE;
+			}
+		}
 
-		WRITE_CHECK (fd, debug_write_fcn, error, " remote_peer_type=cisco");
-		WRITE_CHECK (fd, debug_write_fcn, error, " rightxauthserver=yes");
+		g_string_append (ipsec_conf, " remote_peer_type=cisco\n");
+		g_string_append (ipsec_conf, " rightxauthserver=yes\n");
 	}
 
-
-	phase1_alg_str = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_IKE);
 	/* When the crypto is unspecified, let Libreswan use many sets of crypto
 	 * proposals (just leave the property unset). An exception should be made
 	 * for IKEv1 connections in aggressive mode: there the DH group in the crypto
@@ -236,62 +299,70 @@ nm_libreswan_config_write (gint fd,
 	 * force the best proposal that should be accepted by all obsolete VPN SW/HW
 	 * acting as a remote access VPN server.
 	 */
-	if (phase1_alg_str && strlen (phase1_alg_str))
-		WRITE_CHECK (fd, debug_write_fcn, error, " ike=%s", phase1_alg_str);
-	else if (!is_ikev2 && leftid)
-		WRITE_CHECK (fd, debug_write_fcn, error, " ike=%s", NM_LIBRESWAN_AGGRMODE_DEFAULT_IKE);
+	phase1_alg_str = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_IKE);
+	if (phase1_alg_str == NULL || phase1_alg_str[0] == '\0') {
+		if (!is_ikev2 && leftid)
+			phase1_alg_str = NM_LIBRESWAN_AGGRMODE_DEFAULT_IKE;
+	}
+	if (!optional_string_val (ipsec_conf, NM_LIBRESWAN_KEY_IKE, phase1_alg_str, error))
+		return FALSE;
 
 	phase2_alg_str = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_ESP);
-	if (phase2_alg_str && strlen (phase2_alg_str))
-		WRITE_CHECK (fd, debug_write_fcn, error, " phase2alg=%s", phase2_alg_str);
-	else if (!is_ikev2 && leftid)
-		WRITE_CHECK (fd, debug_write_fcn, error, " phase2alg=%s", NM_LIBRESWAN_AGGRMODE_DEFAULT_ESP);
+	if (phase2_alg_str == NULL || phase2_alg_str[0] == '\0') {
+		if (!is_ikev2 && leftid)
+			phase2_alg_str = NM_LIBRESWAN_AGGRMODE_DEFAULT_ESP;
+	}
+	if (!optional_string_val (ipsec_conf, "phase2alg", phase2_alg_str, error))
+		return FALSE;
 
 	pfs = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_PFS);
 	if (pfs && !strcmp (pfs, "no"))
-		WRITE_CHECK (fd, debug_write_fcn, error, " pfs=no");
+		g_string_append (ipsec_conf, " pfs=no\n");
 
-	phase1_lifetime_str = nm_setting_vpn_get_data_item (s_vpn,
-							    NM_LIBRESWAN_KEY_IKELIFETIME);
-	if (phase1_lifetime_str && strlen (phase1_lifetime_str))
-		WRITE_CHECK (fd, debug_write_fcn, error, " ikelifetime=%s", phase1_lifetime_str);
-	else if (!is_ikev2)
-		WRITE_CHECK (fd, debug_write_fcn, error, " ikelifetime=%s", NM_LIBRESWAN_IKEV1_DEFAULT_LIFETIME);
+	phase1_lifetime_str = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_IKELIFETIME);
+	if (phase1_lifetime_str == NULL || phase1_lifetime_str[0] == '\0') {
+		if (!is_ikev2)
+			phase1_lifetime_str = NM_LIBRESWAN_IKEV1_DEFAULT_LIFETIME;
+	}
+	if (!optional_printable_val (ipsec_conf, NM_LIBRESWAN_KEY_IKELIFETIME, phase1_lifetime_str, error))
+		return FALSE;
 
-	phase2_lifetime_str = nm_setting_vpn_get_data_item (s_vpn,
-							    NM_LIBRESWAN_KEY_SALIFETIME);
-	if (phase2_lifetime_str && strlen (phase2_lifetime_str))
-		WRITE_CHECK (fd, debug_write_fcn, error, " salifetime=%s", phase2_lifetime_str);
-	else if (!is_ikev2)
-		WRITE_CHECK (fd, debug_write_fcn, error, " salifetime=%s", NM_LIBRESWAN_IKEV1_DEFAULT_LIFETIME);
+	phase2_lifetime_str = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_SALIFETIME);
+	if (phase2_lifetime_str == NULL || phase2_lifetime_str[0] == '\0') {
+		if (!is_ikev2)
+			phase2_lifetime_str = NM_LIBRESWAN_IKEV1_DEFAULT_LIFETIME;
+	}
+	if (!optional_printable_val (ipsec_conf, NM_LIBRESWAN_KEY_SALIFETIME, phase2_lifetime_str, error))
+		return FALSE;
 
 	rekey = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_REKEY);
-	if (!rekey || !strlen (rekey)) {
-		WRITE_CHECK (fd, debug_write_fcn, error, " rekey=yes");
-		WRITE_CHECK (fd, debug_write_fcn, error, " keyingtries=1");
-	} else
-		WRITE_CHECK (fd, debug_write_fcn, error, " rekey=%s", rekey);
+	if (!rekey || rekey[0] == '\0') {
+		g_string_append (ipsec_conf, " keyingtries=1\n");
+		rekey = "yes";
+	}
+	g_string_append (ipsec_conf, " rekey=");
+	if (!printable_val (ipsec_conf, rekey, error)) {
+		g_prefix_error (error, _("Invalid value for '%s': "),
+		                NM_LIBRESWAN_KEY_REKEY);
+		return FALSE;
+	}
 
 	if (!openswan && g_strcmp0 (nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_VENDOR), "Cisco") == 0)
-		WRITE_CHECK (fd, debug_write_fcn, error, " cisco-unity=yes");
+		g_string_append (ipsec_conf, " cisco-unity=yes\n");
 
-	WRITE_CHECK (fd, debug_write_fcn, error, " ikev2=%s", ikev2);
+	if (!optional_printable (ipsec_conf, s_vpn, NM_LIBRESWAN_KEY_NARROWING, error))
+		return FALSE;
+	if (!optional_printable (ipsec_conf, s_vpn, NM_LIBRESWAN_KEY_FRAGMENTATION, error))
+		return FALSE;
+	if (!optional_printable (ipsec_conf, s_vpn, NM_LIBRESWAN_KEY_MOBIKE, error))
+		return FALSE;
+	if (!optional_printable (ipsec_conf, s_vpn, NM_LIBRESWAN_KEY_DPDTIMEOUT, error))
+		return FALSE;
 
-	narrowing = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_NARROWING);
-	if (narrowing && strlen (narrowing))
-		WRITE_CHECK (fd, debug_write_fcn, error, " narrowing=%s", narrowing);
+	g_string_append (ipsec_conf, " rightmodecfgserver=yes\n");
+	g_string_append (ipsec_conf, " modecfgpull=yes");
+	if (trailing_newline)
+		g_string_append_c (ipsec_conf, '\n');
 
-	fragmentation = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_FRAGMENTATION);
-	if (fragmentation && strlen (fragmentation))
-		WRITE_CHECK (fd, debug_write_fcn, error, " fragmentation=%s", fragmentation);
-
-	mobike = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_MOBIKE);
-	if (mobike && strlen (mobike))
-		WRITE_CHECK (fd, debug_write_fcn, error, " mobike=%s", mobike);
-
-	WRITE_CHECK (fd, debug_write_fcn, error, " nm-configured=yes");
-
-	WRITE_CHECK_NEWLINE (fd, trailing_newline, debug_write_fcn, error, " auto=add");
-
-	return TRUE;
+	return g_string_free (g_steal_pointer (&ipsec_conf), FALSE);
 }
diff --git a/shared/utils.h b/shared/utils.h
index b5d8f53..839c03a 100644
--- a/shared/utils.h
+++ b/shared/utils.h
@@ -24,26 +24,12 @@
 #ifndef __UTILS_H__
 #define __UTILS_H__
 
-typedef void (*NMDebugWriteFcn) (const char *setting);
-
-__attribute__((__format__ (__printf__, 5, 6)))
-gboolean write_config_option_newline (int fd,
-                                      gboolean new_line,
-                                      NMDebugWriteFcn debug_write_fcn,
-                                      GError **error,
-                                      const char *format, ...);
-
-#define write_config_option(fd, debug_write_fcn, error, ...) write_config_option_newline((fd), TRUE, debug_write_fcn, error, __VA_ARGS__)
-
-gboolean
-nm_libreswan_config_write (gint fd,
-                           NMConnection *connection,
-                           const char *con_name,
-                           const char *leftupdown_script,
-                           gboolean openswan,
-                           gboolean trailing_newline,
-                           NMDebugWriteFcn debug_write_fcn,
-                           GError **error);
+char *nm_libreswan_get_ipsec_conf (NMSettingVpn *s_vpn,
+                                   const char *con_name,
+                                   const char *leftupdown_script,
+                                   gboolean openswan,
+                                   gboolean trailing_newline,
+                                   GError **error);
 
 static inline gboolean
 nm_libreswan_utils_setting_is_ikev2 (NMSettingVpn *s_vpn, const char **out_ikev2)
diff --git a/src/nm-libreswan-service.c b/src/nm-libreswan-service.c
index e69deb8..24108f6 100644
--- a/src/nm-libreswan-service.c
+++ b/src/nm-libreswan-service.c
@@ -103,6 +103,8 @@ typedef struct {
 	const char *whack_path;
 	char *secrets_path;
 
+	char *ipsec_conf;
+
 	gboolean openswan;
 	gboolean interactive;
 	gboolean pending_auth;
@@ -153,12 +155,6 @@ _LOGD_enabled (void)
 #define _LOGW(...) _NMLOG(LOG_WARNING, __VA_ARGS__)
 #define _LOGE(...) _NMLOG(LOG_EMERG, __VA_ARGS__)
 
-static void
-_debug_write_option (const char *setting)
-{
-	_LOGD ("Config %s", setting);
-}
-
 /****************************************************************/
 
 guint32
@@ -726,9 +722,9 @@ nm_libreswan_config_psk_write (NMSettingVpn *s_vpn,
                                GError **error)
 {
 	const char *pw_type, *psk, *leftid, *right;
-	int fd;
-	int errsv;
-	gboolean success;
+	gs_free const char *secrets = NULL;
+	mode_t old_mask;
+	gboolean res;
 
 	/* Check for ignored group password */
 	pw_type = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_PSK_INPUT_MODES);
@@ -739,47 +735,32 @@ nm_libreswan_config_psk_write (NMSettingVpn *s_vpn,
 	if (!psk)
 		return TRUE;
 
-	/* Write the PSK */
-	errno = 0;
-	fd = open (secrets_path, O_RDWR|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR);
-	if (fd < 0) {
-		errsv = errno;
-
-		if (errsv == ENOENT) {
-			gs_free char *dirname = g_path_get_dirname (secrets_path);
-
-			if (!g_file_test (dirname, G_FILE_TEST_IS_DIR)) {
-				g_set_error (error,
-				             NM_VPN_PLUGIN_ERROR,
-				             NM_VPN_PLUGIN_ERROR_LAUNCH_FAILED,
-				             "Failed to open secrets file: no directory %s",
-				             dirname);
-				return FALSE;
-			}
-		}
-
-		g_set_error (error,
-		             NM_VPN_PLUGIN_ERROR,
-		             NM_VPN_PLUGIN_ERROR_LAUNCH_FAILED,
-		             "Failed to open secrets file: (%d) %s.",
-		             errsv, g_strerror (errsv));
-		return FALSE;
-	}
-
 	leftid = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_LEFTID);
 	if (leftid) {
-		success = write_config_option (fd, NULL, error, "@%s: PSK \"%s\"", leftid, psk);
+		if (strchr (leftid, '"') || strchr (leftid, '\n')) {
+			g_set_error_literal (error,
+			                     NM_VPN_PLUGIN_ERROR,
+			                     NM_VPN_PLUGIN_ERROR_INVALID_CONNECTION,
+			                     _("Invalid character in password."));
+			return FALSE;
+		}
+		secrets = g_strdup_printf ("@%s: PSK \"%s\"", leftid, psk);
 	} else {
 		right = nm_setting_vpn_get_data_item (s_vpn, NM_LIBRESWAN_KEY_RIGHT);
-		g_assert (right);
-		success = write_config_option (fd, NULL, error, "%s %%any: PSK \"%s\"", right, psk);
+
+		/* nm_libreswan_get_ipsec_conf() in _connect_common should've check these. */
+		g_return_val_if_fail (right != NULL, FALSE);
+		g_return_val_if_fail (strchr (right, '"') == NULL, FALSE);
+		g_return_val_if_fail (strchr (right, '\n') == NULL, FALSE);
+
+		secrets = g_strdup_printf ("%s %%any: PSK \"%s\"", right, psk);
 	}
 
-	if (!success) {
-		g_close (fd, NULL);
-		return FALSE;
-	}
-	return g_close (fd, error);
+
+	old_mask = umask (S_IRWXG | S_IRWXO);
+	res = g_file_set_contents (secrets_path, secrets, -1, error);
+	umask (old_mask);
+	return res;
 }
 
 /****************************************************************/
@@ -1547,6 +1528,44 @@ done:
 	return success ? G_SOURCE_CONTINUE : G_SOURCE_REMOVE;
 }
 
+static gboolean
+write_config (int fd,
+              const char *string,
+              GError **error)
+{
+        const char *p;
+        gsize l;
+        int errsv;
+        gssize w;
+
+	_LOGD ("Config %s", string);
+
+        l = strlen (string);
+        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) {
+                        errsv = 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;
+}
+
 static gboolean
 connect_step (NMLibreswanPlugin *self, GError **error)
 {
@@ -1629,36 +1648,12 @@ connect_step (NMLibreswanPlugin *self, GError **error)
 		return TRUE;
 
 	case CONNECT_STEP_CONFIG_ADD: {
-		gboolean trailing_newline;
-		gs_free char *bus_name = NULL;
-		gs_free char *ifupdown_script = NULL;
 
 		if (!do_spawn (self, &priv->pid, &fd, NULL, error, priv->ipsec_path,
 		               "auto", "--replace", "--config", "-", uuid, NULL))
 			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);
-
-		/* openswan requires a terminating \n (otherwise it segfaults) while
-		 * libreswan fails parsing the configuration if you include the \n.
-		 * WTF?
-		 */
-		trailing_newline = priv->openswan;
-
-		ifupdown_script = g_strdup_printf ("\"%s %d %ld %s\"",
-		                                   NM_LIBRESWAN_HELPER_PATH,
-		                                   LOG_DEBUG,
-		                                   (long) getpid (),
-		                                   bus_name);
-
-		if (!nm_libreswan_config_write (fd,
-		                                priv->connection,
-		                                uuid,
-		                                ifupdown_script,
-		                                priv->openswan,
-		                                trailing_newline,
-		                                _debug_write_option,
-		                                error)) {
+		if (!write_config (fd, priv->ipsec_conf, error)) {
 			g_close (fd, NULL);
 			return FALSE;
 		}
@@ -1721,12 +1716,24 @@ _connect_common (NMVpnServicePlugin   *plugin,
 	NMLibreswanPluginPrivate *priv = NM_LIBRESWAN_PLUGIN_GET_PRIVATE (self);
 	NMSettingVpn *s_vpn;
 	const char *con_name = nm_connection_get_uuid (connection);
+	gs_free char *ipsec_banner = NULL;
+	gs_free char *ifupdown_script = NULL;
+	gs_free char *bus_name = NULL;
+	gboolean trailing_newline;
 
 	if (_LOGD_enabled ()) {
 		_LOGD ("connection:");
 		nm_connection_dump (connection);
 	}
 
+	if (priv->connect_step != CONNECT_STEP_FIRST) {
+		g_set_error_literal (error,
+			                 NM_VPN_PLUGIN_ERROR,
+			                 NM_VPN_PLUGIN_ERROR_LAUNCH_FAILED,
+			                 "Already connecting!");
+		return FALSE;
+	}
+
 	priv->ipsec_path = find_helper_bin ("ipsec", error);
 	if (!priv->ipsec_path)
 		return FALSE;
@@ -1750,13 +1757,30 @@ _connect_common (NMVpnServicePlugin   *plugin,
 	if (!nm_libreswan_secrets_validate (s_vpn, error))
 		return FALSE;
 
-	if (priv->connect_step != CONNECT_STEP_FIRST) {
-		g_set_error_literal (error,
-			                 NM_VPN_PLUGIN_ERROR,
-			                 NM_VPN_PLUGIN_ERROR_LAUNCH_FAILED,
-			                 "Already connecting!");
+	g_object_get (self, NM_VPN_SERVICE_PLUGIN_DBUS_SERVICE_NAME, &bus_name, NULL);
+
+	ifupdown_script = g_strdup_printf ("%s %d %ld %s",
+					   NM_LIBRESWAN_HELPER_PATH,
+					   LOG_DEBUG,
+					   (long) getpid (),
+					   bus_name);
+
+	/* openswan requires a terminating \n (otherwise it segfaults) while
+	 * libreswan fails parsing the configuration if you include the \n.
+	 * WTF?
+	 */
+	trailing_newline = priv->openswan;
+
+	/* Compose the ipsec.conf early, to catch configuration errors before
+	 * we initiate the conneciton. */
+	priv->ipsec_conf = nm_libreswan_get_ipsec_conf (s_vpn,
+		                                        con_name,
+		                                        ifupdown_script,
+		                                        priv->openswan,
+		                                        trailing_newline,
+		                                        error);
+	if (priv->ipsec_conf == NULL)
 		return FALSE;
-	}
 
 	/* XAUTH is not part of the IKEv2 standard and we always enforce it in IKEv1 */
 	priv->xauth_enabled = !nm_libreswan_utils_setting_is_ikev2 (s_vpn, NULL);
@@ -1928,6 +1952,7 @@ real_disconnect (NMVpnServicePlugin *plugin, GError **error)
 		priv->watch_id = g_child_watch_add (priv->pid, child_watch_cb, plugin);
 
 	g_clear_object (&priv->connection);
+	g_clear_pointer (&priv->ipsec_conf, g_free);
 
 	return ret;
 }
@@ -1960,6 +1985,7 @@ finalize (GObject *object)
 {
 	NMLibreswanPluginPrivate *priv = NM_LIBRESWAN_PLUGIN_GET_PRIVATE (object);
 
+	g_clear_pointer (&priv->ipsec_conf, g_free);
 	delete_secrets_file (NM_LIBRESWAN_PLUGIN (object));
 	connect_cleanup (NM_LIBRESWAN_PLUGIN (object));
 	g_clear_object (&priv->connection);
-- 
2.46.0

From b3ab419db37186d2e888cfe1d91ca0a82c0be884 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Tue, 24 Sep 2024 10:55:02 +0200
Subject: [PATCH 6/6] shared/test-utils: add more test cases

Test ipsec.conf formatting more thoroughly, include negative cases.

[lkundrak@v3.sk: Backported from 1.24.0]
---
 shared/test-utils.c | 81 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/shared/test-utils.c b/shared/test-utils.c
index 965daef..35d9a76 100644
--- a/shared/test-utils.c
+++ b/shared/test-utils.c
@@ -2,6 +2,8 @@
 
 #include "utils.h"
 
+#include "nm-utils/nm-shared-utils.h"
+
 static void
 test_config_write (void)
 {
@@ -57,6 +59,85 @@ test_config_write (void)
 	                 " rightmodecfgserver=yes\n"
 	                 " modecfgpull=yes\n");
 	g_free (str);
+
+	s_vpn = NM_SETTING_VPN (nm_setting_vpn_new ());
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "ikev2", "insist");
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "leftrsasigkey", "hello");
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "rightrsasigkey", "world");
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "right", "11.12.13.14");
+	str = nm_libreswan_get_ipsec_conf (s_vpn, "conn", NULL, FALSE, TRUE, &error);
+	g_assert_no_error (error);
+	g_assert_cmpstr (str, ==,
+	                 "conn conn\n"
+	                 " ikev2=insist\n"
+	                 " right=11.12.13.14\n"
+	                 " leftrsasigkey=\"hello\"\n"
+	                 " rightrsasigkey=\"world\"\n"
+	                 " left=%defaultroute\n"
+	                 " leftmodecfgclient=yes\n"
+	                 " rightsubnet=0.0.0.0/0\n"
+	                 " keyingtries=1\n"
+	                 " rekey=yes\n"
+	                 " rightmodecfgserver=yes\n"
+	                 " modecfgpull=yes\n");
+	g_free (str);
+
+	s_vpn = NM_SETTING_VPN (nm_setting_vpn_new ());
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "right", "11.12.13.14");
+	str = nm_libreswan_get_ipsec_conf (s_vpn,
+	                                   "my_con",
+			                   "/foo/bar/ifupdown hello 123 456",
+	                                   TRUE, FALSE, &error);
+	g_assert_no_error (error);
+	g_assert_cmpstr (str, ==,
+	                 "conn my_con\n"
+	                 " auto=add\n"
+	                 " nm-configured=yes\n"
+	                 " leftupdown=\"/foo/bar/ifupdown hello 123 456\"\n"
+	                 " ikev2=never\n"
+	                 " right=11.12.13.14\n"
+	                 " authby=secret\n"
+	                 " left=%defaultroute\n"
+	                 " leftmodecfgclient=yes\n"
+	                 " rightsubnet=0.0.0.0/0\n"
+	                 " leftxauthclient=yes\n"
+	                 " remote_peer_type=cisco\n"
+	                 " rightxauthserver=yes\n"
+	                 " ikelifetime=24h\n"
+	                 " salifetime=24h\n"
+	                 " keyingtries=1\n"
+	                 " rekey=yes\n"
+	                 " rightmodecfgserver=yes\n"
+	                 " modecfgpull=yes");
+	g_free (str);
+
+	s_vpn = NM_SETTING_VPN (nm_setting_vpn_new ());
+	str = nm_libreswan_get_ipsec_conf (s_vpn, "conn", NULL, FALSE, TRUE, &error);
+	g_assert_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_INVALID_ARGUMENT);
+	g_assert_null (str);
+	g_clear_error (&error);
+
+	s_vpn = NM_SETTING_VPN (nm_setting_vpn_new ());
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "right", "11.12.13.14");
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "ikev2", "hello world");
+	str = nm_libreswan_get_ipsec_conf (s_vpn, "conn", NULL, FALSE, TRUE, &error);
+	g_assert_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_INVALID_ARGUMENT);
+	g_assert_null (str);
+	g_clear_error (&error);
+
+	s_vpn = NM_SETTING_VPN (nm_setting_vpn_new ());
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "right", "11.12\n13.14");
+	str = nm_libreswan_get_ipsec_conf (s_vpn, "conn", NULL, FALSE, TRUE, &error);
+	g_assert_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_INVALID_ARGUMENT);
+	g_assert_null (str);
+	g_clear_error (&error);
+
+	s_vpn = NM_SETTING_VPN (nm_setting_vpn_new ());
+	nm_setting_vpn_add_data_item (NM_SETTING_VPN(s_vpn), "rightcert", "\"cert\"");
+	str = nm_libreswan_get_ipsec_conf (s_vpn, "conn", NULL, FALSE, TRUE, &error);
+	g_assert_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_INVALID_ARGUMENT);
+	g_assert_null (str);
+	g_clear_error (&error);
 }
 
 int
-- 
2.46.0