diff --git a/SOURCES/0003-service-fix-wrong-refcounting-in-D-Bus-handler-for-C.patch b/SOURCES/0003-service-fix-wrong-refcounting-in-D-Bus-handler-for-C.patch new file mode 100644 index 0000000..348f671 --- /dev/null +++ b/SOURCES/0003-service-fix-wrong-refcounting-in-D-Bus-handler-for-C.patch @@ -0,0 +1,68 @@ +From 4be4c56b4f8a52b1cd5f8aadee273706c28ae332 Mon Sep 17 00:00:00 2001 +From: Beniamino Galvani +Date: Sat, 13 Jan 2024 18:10:02 +0100 +Subject: [PATCH 1/1] service: fix wrong refcounting in D-Bus handler for + Callback() + +The Callback() D-Bus method is handled via a GDBus-generated skeleton +code in nm-libreswan-helper-service-dbus.c, function +_nmdbus_libreswan_helper_skeleton_handle_method_call(). The function +emits signal "handle-callback" to let the program handle the incoming +method. As documented in the GDoc comments, the signal handler must +return TRUE if it handles the call. + +``` + /** + * NMDBusLibreswanHelper::handle-callback: + * @object: A #NMDBusLibreswanHelper. + * @invocation: A #GDBusMethodInvocation. + * @arg_environment: Argument passed by remote caller. + + * Signal emitted when a remote caller is invoking the Callback() + D-Bus method. + + * If a signal handler returns %TRUE, it means the signal handler + will handle the invocation (e.g. take a reference to @invocation + and eventually call nmdbus_libreswan_helper_complete_callback() + or e.g. g_dbus_method_invocation_return_error() on it) and no + other signal handlers will run. If no signal handler handles the + invocation, the %G_DBUS_ERROR_UNKNOWN_METHOD error is returned. + + * Returns: %G_DBUS_METHOD_INVOCATION_HANDLED or %TRUE if the + invocation was handled, %G_DBUS_METHOD_INVOCATION_UNHANDLED or + %FALSE to let other signal handlers run. + */ +``` + +At the moment, in case of error the handler first calls +nmdbus_libreswan_helper_complete_callback() which decreases the +refcount of "invocation", and then returns FALSE which tells the +skeleton code to return an error, also unreferencing the +invocation. This causes a crash. + +Since the G_DBUS_METHOD_INVOCATION_HANDLED alias for TRUE is only +available since GLib 2.68 (while we target 2.36), just return TRUE. + +Fixes: acb9eb9de50b ('service: process the configuration in the service, not the helper') +(cherry picked from commit 8ceb901719acac3778e1d76779d9c14289185157) +--- + src/nm-libreswan-service.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/src/nm-libreswan-service.c b/src/nm-libreswan-service.c +index fc470a6..4850729 100644 +--- a/src/nm-libreswan-service.c ++++ b/src/nm-libreswan-service.c +@@ -1379,7 +1379,8 @@ out: + } + + nmdbus_libreswan_helper_complete_callback (object, invocation); +- return success; ++ ++ return TRUE; + } + + /****************************************************************/ +-- +2.43.0 + diff --git a/SOURCES/0004-ipsec-conf-escaping-cve-2024-9050.patch b/SOURCES/0004-ipsec-conf-escaping-cve-2024-9050.patch new file mode 100644 index 0000000..74a9ced --- /dev/null +++ b/SOURCES/0004-ipsec-conf-escaping-cve-2024-9050.patch @@ -0,0 +1,1512 @@ +From dcf656747eece80e2534dc21b79c15e13bb28b5a Mon Sep 17 00:00:00 2001 +From: Lubomir Rintel +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 +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 +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 ++ ++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 +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 +- +-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 + #include + +-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 +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 + diff --git a/SPECS/NetworkManager-libreswan.spec b/SPECS/NetworkManager-libreswan.spec index 9797b08..09f0fa5 100644 --- a/SPECS/NetworkManager-libreswan.spec +++ b/SPECS/NetworkManager-libreswan.spec @@ -11,13 +11,15 @@ Summary: NetworkManager VPN plug-in for IPsec VPN Name: NetworkManager-libreswan Version: 1.2.10 -Release: 4%{?dist} +Release: 7%{?dist} License: GPLv2+ URL: http://www.gnome.org/projects/NetworkManager/ Group: System Environment/Base Source0: https://download.gnome.org/sources/NetworkManager-libreswan/1.2/%{name}-%{version}.tar.xz Patch0: 0001-po-import-translations-from-Red-Hat-translators.patch Patch1: 0002-properties-set-advanced-dialog-modal.patch +Patch2: 0003-service-fix-wrong-refcounting-in-D-Bus-handler-for-C.patch +Patch3: 0004-ipsec-conf-escaping-cve-2024-9050.patch BuildRequires: gtk3-devel BuildRequires: libnl3-devel @@ -109,6 +111,15 @@ update-desktop-database &> /dev/null || : %endif %changelog +* Thu Oct 03 2024 Lubomir Rintel - 1.2.10-7 +- Unbreak validation of unknown keys + +* Wed Sep 25 2024 Lubomir Rintel - 1.2.10-6 +- Fix improper escaping of Libreswan configuration (CVE-2024-9050) + +* Mon Feb 5 2024 Wen Liang - 1.2.10-5 +- Fix crash in libreswan_add_profile_wrong_password (RHEL-13123) + * Tue Jul 25 2023 MSVSphere Packaging Team - 1.2.10-4 - Rebuilt for MSVSphere 8.8