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