libnm/vpn: search VPN plugin in NMPLUGINDIR

In commit ca000cffbb9ef20c6dd965283df3f1babf0a7745, we changed to
accept a plugin library name without path. One reason for that
is to keep architecture dependent parts out of the .name file
and possibly support multilib.

However, the shared libraries of VPN plugins are not installed in
a global library search path, but for example into
"/usr/lib64/NetworkManager/libnm-vpn-plugin-openvpn.so".
In that case, specifying "plugin=libnm-vpn-plugin-openvpn.so"
would not be enough to find plugin.

Instead, when configuring a plugin name without path, assume
it is in NMPLUGINDIR directory. Modify nm_vpn_editor_plugin_load_from_file()
to allow path-less plugin-names. Previously such names would be rejected
as not being absolute. This API allows to do file verification
before loading the plugin, but it now supports prepending NMPLUGINDIR
to the plugin name. Basically, this function mangles the plugin_name
argument and checks that such a file exists.

The recently added nm_vpn_editor_plugin_load() continues to behave
as before: it does no checks whatsoever and passes the name directly
to dlopen(). That way, it uses system search paths like LD_LIBRARY_PATH
and performs no checks on the file.

Fixes: ca000cffbb9ef20c6dd965283df3f1babf0a7745

5 files changed, 68 insertions, 77 deletions

diff --git a/libnm-core/Makefile.am b/libnm-core/Makefile.am
index 4bc99b3e4e..4ae3ce00f8 100644
--- a/libnm-core/Makefile.am
+++ b/libnm-core/Makefile.am
@@ -11,6 +11,7 @@ AM_CPPFLAGS = \
 	-DLOCALEDIR=\"$(datadir)/locale\" \
 	-DNMCONFDIR=\"$(nmconfdir)\" \
 	-DNMLIBDIR=\"$(nmlibdir)\" \
+	-DNMPLUGINDIR=\"$(pkglibdir)\" \
 	-DNETWORKMANAGER_COMPILATION=NM_NETWORKMANAGER_COMPILATION_LIB \
 	$(GLIB_CFLAGS) \
 	$(CODE_COVERAGE_CFLAGS) \
diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h
index 8d97b066ee..2041a1d3a1 100644
--- a/libnm-core/nm-core-internal.h
+++ b/libnm-core/nm-core-internal.h
@@ -246,14 +246,6 @@ GSList *_nm_vpn_plugin_info_list_load_dir (const char *dirname,
                                            NMUtilsCheckFilePredicate check_file,
                                            gpointer user_data);
 
-NMVpnEditorPlugin * _nm_vpn_editor_plugin_load (const char *plugin_filename,
-                                                gboolean force_absolute_path,
-                                                const char *check_service,
-                                                int check_owner,
-                                                NMUtilsCheckFilePredicate check_file,
-                                                gpointer user_data,
-                                                GError **error);
-
 /***********************************************************/
 
 typedef struct {
diff --git a/libnm-core/nm-vpn-editor-plugin.c b/libnm-core/nm-vpn-editor-plugin.c
index 84297a82b7..52c89816ad 100644
--- a/libnm-core/nm-vpn-editor-plugin.c
+++ b/libnm-core/nm-vpn-editor-plugin.c
@@ -71,9 +71,9 @@ nm_vpn_editor_plugin_default_init (NMVpnEditorPluginInterface *iface)
 
 /*********************************************************************/
 
-NMVpnEditorPlugin *
-_nm_vpn_editor_plugin_load (const char *plugin_filename,
-                            gboolean force_absolute_filename,
+static NMVpnEditorPlugin *
+_nm_vpn_editor_plugin_load (const char *plugin_name,
+                            gboolean do_file_checks,
                             const char *check_service,
                             int check_owner,
                             NMUtilsCheckFilePredicate check_file,
@@ -81,54 +81,49 @@ _nm_vpn_editor_plugin_load (const char *plugin_filename,
                             GError **error)
 {
 	GModule *module = NULL;
-	gs_free_error GError *local = NULL;
 	NMVpnEditorPluginFactory factory = NULL;
 	NMVpnEditorPlugin *editor_plugin = NULL;
-	gboolean search_lib = FALSE;
-
-	g_return_val_if_fail (plugin_filename && *plugin_filename, NULL);
-
-	if (   !force_absolute_filename
-	    && !strchr (plugin_filename, '/')
-	    && !g_str_has_suffix (plugin_filename, ".la")) {
-		/* we allow omitting the (absolute) path.
-		 *
-		 * If the @plugin_filename contains no '/', we skip any checks
-		 * for the file and pass it directly to g_module_open()/dlopen().
-		 * One exception is that we don't allow for the "la" suffix. The
-		 * reason is that g_module_open() interprets files with this extension
-		 * special and we don't want that. */
-		search_lib = TRUE;
-	}
+	gs_free char *plugin_filename_free = NULL;
+	const char *plugin_filename;
 
-	/* _nm_utils_check_module_file() fails with ENOENT if the plugin file
-	 * does not exist. That is relevant, because nm-applet checks for that. */
-	if (   search_lib
-	    || _nm_utils_check_module_file (plugin_filename,
-	                                    check_owner,
-	                                    check_file,
-	                                    user_data,
-	                                    &local))
-		module = g_module_open (plugin_filename, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);
+	g_return_val_if_fail (plugin_name && *plugin_name, NULL);
 
-	if (!module) {
-		if (local) {
-			g_propagate_error (error, local);
-			local = NULL;
-		} else if (search_lib) {
-			g_set_error (error,
-			             G_FILE_ERROR,
-			             G_FILE_ERROR_NOENT,
-			             _("Plugin does not exist (%s)"), plugin_filename);
-		} else {
-			g_set_error (error,
-			             NM_VPN_PLUGIN_ERROR,
-			             NM_VPN_PLUGIN_ERROR_FAILED,
-			             _("cannot load plugin %s"), plugin_filename);
+	/* if @do_file_checks is FALSE, we pass plugin_name directly to
+	 * g_module_open().
+	 *
+	 * Otherwise, we allow for library names without path component.
+	 * In which case, we prepend the plugin directory and form an
+	 * absolute path. In that case, we perform checks on the file.
+	 *
+	 * One exception is that we don't allow for the "la" suffix. The
+	 * reason is that g_module_open() interprets files with this extension
+	 * special and we don't want that. */
+	plugin_filename = plugin_name;
+	if (do_file_checks) {
+		if (   !strchr (plugin_name, '/')
+		    && !g_str_has_suffix (plugin_name, ".la")) {
+			plugin_filename_free = g_module_build_path (NMPLUGINDIR, plugin_name);
+			plugin_filename = plugin_filename_free;
 		}
+
+		/* _nm_utils_check_module_file() fails with ENOENT if the plugin file
+		 * does not exist. That is relevant, because nm-applet checks for that. */
+		if (!_nm_utils_check_module_file (plugin_filename,
+		                                  check_owner,
+		                                  check_file,
+		                                  user_data,
+		                                  error))
+			return NULL;
+	}
+
+	module = g_module_open (plugin_filename, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL);
+	if (!module) {
+		g_set_error (error,
+		             NM_VPN_PLUGIN_ERROR,
+		             NM_VPN_PLUGIN_ERROR_FAILED,
+		             _("cannot load plugin %s"), plugin_name);
 		return NULL;
 	}
-	g_clear_error (&local);
 
 	if (g_module_symbol (module, "nm_vpn_editor_plugin_factory", (gpointer) &factory)) {
 		gs_free_error GError *factory_error = NULL;
@@ -175,7 +170,7 @@ _nm_vpn_editor_plugin_load (const char *plugin_filename,
 				g_set_error (error,
 				             NM_VPN_PLUGIN_ERROR,
 				             NM_VPN_PLUGIN_ERROR_FAILED,
-				             _("unknown error initializing plugin %s"), plugin_filename);
+				             _("unknown error initializing plugin %s"), plugin_name);
 			}
 		}
 
@@ -198,8 +193,10 @@ _nm_vpn_editor_plugin_load (const char *plugin_filename,
 
 /**
  * nm_vpn_editor_plugin_load_from_file:
- * @plugin_filename: The path to the shared library to load.
- *  The path must be an absolute filename to an existing file.
+ * @plugin_name: The path or name of the shared library to load.
+ *  The path must either be an absolute filename to an existing file.
+ *  Alternatively, it can be the name (without path) of a library in the
+ *  plugin directory of NetworkManager.
  * @check_service: if not-null, check that the loaded plugin advertises
  *  the given service.
  * @check_owner: if non-negative, check whether the file is owned
@@ -210,23 +207,29 @@ _nm_vpn_editor_plugin_load (const char *plugin_filename,
  * @user_data: user data for @check_file
  * @error: on failure the error reason.
  *
- * Load the shared libary @plugin_filename and create a new
+ * Load the shared libary @plugin_name and create a new
  * #NMVpnEditorPlugin instace via the #NMVpnEditorPluginFactory
  * function.
  *
+ * If @plugin_name is not an absolute path name, it assumes the file
+ * is in the plugin directory of NetworkManager. In any case, the call
+ * will do certain checks on the file before passing it to dlopen.
+ * A consequence for that is, that you cannot omit the ".so" suffix
+ * as you could for nm_vpn_editor_plugin_load().
+ *
  * Returns: (transfer full): a new plugin instance or %NULL on error.
  *
  * Since: 1.2
  */
 NMVpnEditorPlugin *
-nm_vpn_editor_plugin_load_from_file  (const char *plugin_filename,
+nm_vpn_editor_plugin_load_from_file  (const char *plugin_name,
                                       const char *check_service,
                                       int check_owner,
                                       NMUtilsCheckFilePredicate check_file,
                                       gpointer user_data,
                                       GError **error)
 {
-	return _nm_vpn_editor_plugin_load (plugin_filename,
+	return _nm_vpn_editor_plugin_load (plugin_name,
 	                                   TRUE,
 	                                   check_service,
 	                                   check_owner,
@@ -238,24 +241,20 @@ nm_vpn_editor_plugin_load_from_file  (const char *plugin_filename,
 /**
  * nm_vpn_editor_plugin_load:
  * @plugin_name: The name of the shared library to load.
- *  If it is an absolute path, for further checks will be
- *  performed on the file (aside from being a regular file
- *  and existing).
- *  The @plugin_name can also be a library name only. In this
- *  case, system dependent directories will be searched for a
- *  matching library.
+ *  This path will be directly passed to dlopen() without
+ *  further checks.
  * @check_service: if not-null, check that the loaded plugin advertises
  *  the given service.
-  * @error: on failure the error reason.
+ * @error: on failure the error reason.
  *
- * Load the shared libary @plugin_filename and create a new
+ * Load the shared libary @plugin_name and create a new
  * #NMVpnEditorPlugin instace via the #NMVpnEditorPluginFactory
  * function.
  *
  * This is similar to nm_vpn_editor_plugin_load_from_file(), but
- * also allows searching for a matching library and not perform
- * any checks on the file. If you have the full path to a plugin
- * file, nm_vpn_editor_plugin_load_from_file() is preferred.
+ * it does no validation of the plugin name, instead passes it directly
+ * to dlopen(). If you have the full path to a plugin file,
+ * nm_vpn_editor_plugin_load_from_file() is preferred.
  *
  * Returns: (transfer full): a new plugin instance or %NULL on error.
  *
diff --git a/libnm-core/nm-vpn-editor-plugin.h b/libnm-core/nm-vpn-editor-plugin.h
index 7f70a40c2a..82c609b969 100644
--- a/libnm-core/nm-vpn-editor-plugin.h
+++ b/libnm-core/nm-vpn-editor-plugin.h
@@ -140,7 +140,7 @@ char         *nm_vpn_editor_plugin_get_suggested_filename (NMVpnEditorPlugin *pl
                                                            NMConnection *connection);
 
 NM_AVAILABLE_IN_1_2
-NMVpnEditorPlugin *nm_vpn_editor_plugin_load_from_file  (const char *plugin_filename,
+NMVpnEditorPlugin *nm_vpn_editor_plugin_load_from_file  (const char *plugin_name,
                                                          const char *check_service,
                                                          int check_owner,
                                                          NMUtilsCheckFilePredicate check_file,
diff --git a/libnm-core/nm-vpn-plugin-info.c b/libnm-core/nm-vpn-plugin-info.c
index 6c77f3c142..2e6275e341 100644
--- a/libnm-core/nm-vpn-plugin-info.c
+++ b/libnm-core/nm-vpn-plugin-info.c
@@ -752,13 +752,12 @@ nm_vpn_plugin_info_load_editor_plugin (NMVpnPluginInfo *self, GError **error)
 	}
 
 	priv->editor_plugin_loaded = TRUE;
-	priv->editor_plugin = _nm_vpn_editor_plugin_load (plugin_filename,
-	                                                  FALSE,
-	                                                  priv->service,
-	                                                  getuid (),
-	                                                  NULL,
-	                                                  NULL,
-	                                                  error);
+	priv->editor_plugin = nm_vpn_editor_plugin_load_from_file (plugin_filename,
+	                                                           priv->service,
+	                                                           getuid (),
+	                                                           NULL,
+	                                                           NULL,
+	                                                           error);
 	return priv->editor_plugin;
 }