Merge branch '878-install-named-plugins-into-a-separate-directory' into 'master'

Install named plugins into a separate directory

Closes #878

See merge request isc-projects/bind9!1512

CHANGES

@@ -1,3 +1,9 @@
+5171.	[func]		named plugins are now installed into a separate
+			directory.  Supplying a filename (a string without path
+			separators) in a "plugin" configuration stanza now
+			causes named to look for that plugin in that directory.
+			[GL #878]
+
 5170.	[test]		Added --with-dlz-filesystem to feature-test. [GL !1587]
 
 5169.	[bug]		The presence of certain types in an otherwise

bin/check/named-checkconf.c

@@ -63,7 +63,7 @@ usage(void) ISC_PLATFORM_NORETURN_POST;
 
 static void
 usage(void) {
-	fprintf(stderr, "usage: %s [-hjlvz] [-p [-x]] [-t directory] "
+	fprintf(stderr, "usage: %s [-chjlvz] [-p [-x]] [-t directory] "
 		"[named.conf]\n", program);
 	exit(1);
 }

bin/named/server.c

@@ -3655,9 +3655,21 @@ register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj,
 		    void *callback_data)
 {
 	dns_view_t *view = callback_data;
+	char full_path[PATH_MAX];
 	isc_result_t result;
 
-	result = ns_plugin_register(plugin_path, parameters, config,
+	result = ns_plugin_expandpath(plugin_path,
+				      full_path, sizeof(full_path));
+	if (result != ISC_R_SUCCESS) {
+		isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
+			      NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR,
+			      "%s: plugin configuration failed: "
+			      "unable to get full plugin path: %s",
+			      plugin_path, isc_result_totext(result));
+		return (result);
+	}
+
+	result = ns_plugin_register(full_path, parameters, config,
 				    cfg_obj_file(obj), cfg_obj_line(obj),
 				    named_g_mctx, named_g_lctx,
 				    named_g_aclconfctx, view);
@@ -3665,7 +3677,7 @@ register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj,
 		isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 			      NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR,
 			      "%s: plugin configuration failed: %s",
-			      plugin_path, isc_result_totext(result));
+			      full_path, isc_result_totext(result));
 	}
 
 	return (result);

bin/plugins/Makefile.in

@@ -59,14 +59,14 @@ clean distclean::
 	rm -f ${TARGETS} ${OBJS}
 
 installdirs:
-	$(SHELL) ${top_srcdir}/mkinstalldirs ${DESTDIR}${libdir}
+	$(SHELL) ${top_srcdir}/mkinstalldirs ${DESTDIR}${plugindir}
 	$(SHELL) ${top_srcdir}/mkinstalldirs ${DESTDIR}${mandir}/man8
 
 install:: filter-aaaa.@SO@ installdirs
 	${LIBTOOL_MODE_INSTALL} ${INSTALL_LIBRARY} filter-aaaa.@SO@ \
-		${DESTDIR}${libdir}
+		${DESTDIR}${plugindir}
 	${INSTALL_DATA} ${srcdir}/filter-aaaa.8 ${DESTDIR}${mandir}/man8
 
 uninstall::
-	${LIBTOOL_MODE_UNINSTALL} rm -f ${DESTDIR}${libdir}/filter-aaaa.@SO@
+	${LIBTOOL_MODE_UNINSTALL} rm -f ${DESTDIR}${plugindir}/filter-aaaa.@SO@
 	rm -f ${DESTDIR}${mandir}/man8/filter-aaaa.8

lib/bind9/check.c

@@ -3409,15 +3409,26 @@ check_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj,
 		 void *callback_data)
 {
 	struct check_one_plugin_data *data = callback_data;
+	char full_path[PATH_MAX];
 	isc_result_t result;
 
-	result = ns_plugin_check(plugin_path, parameters, config,
+	result = ns_plugin_expandpath(plugin_path,
+				      full_path, sizeof(full_path));
+	if (result != ISC_R_SUCCESS) {
+		cfg_obj_log(obj, data->lctx, ISC_LOG_ERROR,
+			    "%s: plugin check failed: "
+			    "unable to get full plugin path: %s",
+			    plugin_path, isc_result_totext(result));
+		return (result);
+	}
+
+	result = ns_plugin_check(full_path, parameters, config,
 				 cfg_obj_file(obj), cfg_obj_line(obj),
 				 data->mctx, data->lctx, data->actx);
 	if (result != ISC_R_SUCCESS) {
 		cfg_obj_log(obj, data->lctx, ISC_LOG_ERROR,
 			    "%s: plugin check failed: %s",
-			    plugin_path, isc_result_totext(result));
+			    full_path, isc_result_totext(result));
 		*data->check_result = result;
 	}
 

lib/ns/Makefile.in

@@ -28,7 +28,7 @@ CINCLUDES =	-I. -I${top_srcdir}/lib/ns -Iinclude \
 		${NS_INCLUDES} ${DNS_INCLUDES} ${ISC_INCLUDES} \
 		@OPENSSL_INCLUDES@ @DST_GSSAPI_INC@
 
-CDEFINES =
+CDEFINES =	-DNAMED_PLUGINDIR=\"${plugindir}\"
 
 CWARNINGS =
 

lib/ns/hooks.c

@@ -13,6 +13,8 @@
 
 #include <config.h>
 
+#include <errno.h>
+#include <stdio.h>
 #include <string.h>
 
 #if HAVE_DLFCN_H
@@ -21,10 +23,12 @@
 #include <windows.h>
 #endif
 
+#include <isc/errno.h>
 #include <isc/list.h>
 #include <isc/log.h>
 #include <isc/mem.h>
 #include <isc/mutex.h>
+#include <isc/print.h>
 #include <isc/result.h>
 #include <isc/platform.h>
 #include <isc/util.h>
@@ -58,6 +62,41 @@ struct ns_plugin {
 static ns_hooklist_t default_hooktable[NS_HOOKPOINTS_COUNT];
 LIBNS_EXTERNAL_DATA ns_hooktable_t *ns__hook_table = &default_hooktable;
 
+isc_result_t
+ns_plugin_expandpath(const char *src, char *dst, size_t dstsize) {
+	int result;
+
+#ifndef WIN32
+	/*
+	 * On Unix systems, differentiate between paths and filenames.
+	 */
+	if (strchr(src, '/') != NULL) {
+		/*
+		 * 'src' is an absolute or relative path.  Copy it verbatim.
+		 */
+		result = snprintf(dst, dstsize, "%s", src);
+	} else {
+		/*
+		 * 'src' is a filename.  Prepend default plugin directory path.
+		 */
+		result = snprintf(dst, dstsize, "%s/%s", NAMED_PLUGINDIR, src);
+	}
+#else
+	/*
+	 * On Windows, always copy 'src' do 'dst'.
+	 */
+	result = snprintf(dst, dstsize, "%s", src);
+#endif
+
+	if (result < 0) {
+		return (isc_errno_toresult(errno));
+	} else if ((size_t)result >= dstsize) {
+		return (ISC_R_NOSPACE);
+	} else {
+		return (ISC_R_SUCCESS);
+	}
+}
+
 #if HAVE_DLFCN_H && HAVE_DLOPEN
 static isc_result_t
 load_symbol(void *handle, const char *modpath,
@@ -253,7 +292,7 @@ load_plugin(isc_mem_t *mctx, const char *modpath, ns_plugin_t **pluginp) {
 	CHECK(load_symbol(handle, modpath, "plugin_version",
 			  (void **)&version_func));
 
-	version = version_func(NULL);
+	version = version_func();
 	if (version < (NS_PLUGIN_VERSION - NS_PLUGIN_AGE) ||
 	    version > NS_PLUGIN_VERSION)
 	{

lib/ns/include/ns/hooks.h

@@ -315,6 +315,30 @@ ns_plugin_destroy_t plugin_destroy;
 ns_plugin_register_t plugin_register;
 ns_plugin_version_t plugin_version;
 
+isc_result_t
+ns_plugin_expandpath(const char *src, char *dst, size_t dstsize);
+/*%<
+ * Prepare the plugin location to be passed to dlopen() based on the plugin
+ * path or filename found in the configuration file ('src').  Store the result
+ * in 'dst', which is 'dstsize' bytes large.
+ *
+ * On Unix systems, two classes of 'src' are recognized:
+ *
+ *   - If 'src' is an absolute or relative path, it will be copied to 'dst'
+ *     verbatim.
+ *
+ *   - If 'src' is a filename (i.e. does not contain a path separator), the
+ *     path to the directory into which named plugins are installed will be
+ *     prepended to it and the result will be stored in 'dst'.
+ *
+ * On Windows, 'src' is always copied to 'dst' verbatim.
+ *
+ * Returns:
+ *\li	#ISC_R_SUCCESS	Success
+ *\li	#ISC_R_NOSPACE	'dst' is not large enough to hold the output string
+ *\li	Other result	snprintf() returned a negative value
+ */
+
 isc_result_t
 ns_plugin_register(const char *modpath, const char *parameters,
 		   const void *cfg, const char *cfg_file,

lib/ns/tests/Kyuafile

@@ -3,4 +3,5 @@ test_suite('bind9')
 
 tap_test_program{name='listenlist_test'}
 tap_test_program{name='notify_test'}
+tap_test_program{name='plugin_test'}
 tap_test_program{name='query_test'}

lib/ns/tests/Makefile.in

@@ -17,7 +17,7 @@ VERSION=@BIND9_VERSION@
 
 CINCLUDES =	-I. -Iinclude ${NS_INCLUDES} ${DNS_INCLUDES} ${ISC_INCLUDES} \
 		@OPENSSL_INCLUDES@ @CMOCKA_CFLAGS@
-CDEFINES =	-DTESTS="\"${top_builddir}/lib/ns/tests/\""
+CDEFINES =	-DTESTS="\"${top_builddir}/lib/ns/tests/\"" -DNAMED_PLUGINDIR=\"${plugindir}\"
 
 ISCLIBS =	../../isc/libisc.@A@ @OPENSSL_LIBS@
 ISCDEPLIBS =	../../isc/libisc.@A@
@@ -33,12 +33,14 @@ OBJS =		nstest.@O@
 SRCS =		nstest.c \
 		listenlist_test.c \
 		notify_test.c \
+		plugin_test.c \
 		query_test.c
 
 SUBDIRS =
 TARGETS =	listenlist_test@EXEEXT@ \
 		notify_test@EXEEXT@ \
-		query_test
+		plugin_test@EXEEXT@ \
+		query_test@EXEEXT@
 
 @BIND9_MAKE_RULES@
 
@@ -52,6 +54,11 @@ notify_test@EXEEXT@: notify_test.@O@ nstest.@O@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNS
 		${LDFLAGS} -o $@ notify_test.@O@ nstest.@O@ \
 		${NSLIBS} ${DNSLIBS} ${ISCLIBS} ${LIBS}
 
+plugin_test@EXEEXT@: plugin_test.@O@ nstest.@O@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNSDEPLIBS}
+	${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} \
+		${LDFLAGS} -o $@ plugin_test.@O@ nstest.@O@ \
+		${NSLIBS} ${DNSLIBS} ${ISCLIBS} ${LIBS}
+
 query_test@EXEEXT@: query_test.@O@ nstest.@O@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNSDEPLIBS}
 	${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} \
 		${LDFLAGS} -o $@ query_test.@O@ nstest.@O@ \

lib/ns/tests/plugin_test.c

@@ -0,0 +1,205 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+#include <config.h>
+
+#if HAVE_CMOCKA
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <setjmp.h>
+
+#define UNIT_TESTING
+#include <cmocka.h>
+
+#include <limits.h>
+#include <stdbool.h>
+#include <string.h>
+
+#include <isc/mem.h>
+#include <isc/result.h>
+#include <isc/types.h>
+#include <isc/util.h>
+
+#include <ns/hooks.h>
+
+#include "nstest.h"
+
+static int
+_setup(void **state) {
+	isc_result_t result;
+
+	UNUSED(state);
+
+	result = ns_test_begin(NULL, false);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	return (0);
+}
+
+static int
+_teardown(void **state) {
+	if (*state != NULL) {
+		isc_mem_free(mctx, *state);
+	}
+
+	ns_test_end();
+
+	return (0);
+}
+
+/*%
+ * Structure containing parameters for run_full_path_test().
+ */
+typedef struct {
+	const ns_test_id_t id;	/* libns test identifier */
+	const char *input;	/* source string - plugin name or path */
+	size_t output_size;	/* size of target char array to allocate */
+	isc_result_t result;	/* expected return value */
+	const char *output;	/* expected output string */
+} ns_plugin_expandpath_test_params_t;
+
+/*%
+ * Perform a single ns_plugin_expandpath() check using given parameters.
+ */
+static void
+run_full_path_test(const ns_plugin_expandpath_test_params_t *test,
+		   void **state)
+{
+	char **target = (char **)state;
+	isc_result_t result;
+
+	REQUIRE(test != NULL);
+	REQUIRE(test->id.description != NULL);
+	REQUIRE(test->input != NULL);
+	REQUIRE(test->result != ISC_R_SUCCESS || test->output != NULL);
+
+	/*
+	 * Prepare a target buffer of given size.  Store it in 'state' so that
+	 * it can get cleaned up by _teardown() if the test fails.
+	 */
+	*target = isc_mem_allocate(mctx, test->output_size);
+
+	/*
+	 * Call ns_plugin_expandpath().
+	 */
+	result = ns_plugin_expandpath(test->input,
+					 *target, test->output_size);
+
+	/*
+	 * Check return value.
+	 */
+	if (result != test->result) {
+		fail_msg("# test \"%s\" on line %d: "
+			 "expected result %d (%s), got %d (%s)",
+			 test->id.description, test->id.lineno,
+			 test->result, isc_result_totext(test->result),
+			 result, isc_result_totext(result));
+	}
+
+	/*
+	 * Check output string if return value indicates success.
+	 */
+	if (result == ISC_R_SUCCESS && strcmp(*target, test->output) != 0) {
+		fail_msg("# test \"%s\" on line %d: "
+			 "expected output \"%s\", got \"%s\"",
+			 test->id.description, test->id.lineno,
+			 test->output, *target);
+	}
+
+	isc_mem_free(mctx, *target);
+}
+
+/* test ns_plugin_expandpath() */
+static void
+ns_plugin_expandpath_test(void **state) {
+	size_t i;
+
+	const ns_plugin_expandpath_test_params_t tests[] = {
+		{
+			NS_TEST_ID("correct use with an absolute path"),
+			.input = "/usr/lib/named/foo.so",
+			.output_size = PATH_MAX,
+			.result = ISC_R_SUCCESS,
+			.output = "/usr/lib/named/foo.so",
+		},
+		{
+			NS_TEST_ID("correct use with a relative path"),
+			.input = "../../foo.so",
+			.output_size = PATH_MAX,
+			.result = ISC_R_SUCCESS,
+			.output = "../../foo.so",
+		},
+		{
+			NS_TEST_ID("correct use with a filename"),
+			.input = "foo.so",
+			.output_size = PATH_MAX,
+			.result = ISC_R_SUCCESS,
+#ifndef WIN32
+			.output = NAMED_PLUGINDIR "/foo.so",
+#else
+			.output = "foo.so",
+#endif
+		},
+		{
+			NS_TEST_ID("no space at all in target buffer"),
+			.input = "/usr/lib/named/foo.so",
+			.output_size = 0,
+			.result = ISC_R_NOSPACE,
+		},
+		{
+			NS_TEST_ID("target buffer too small to fit input"),
+			.input = "/usr/lib/named/foo.so",
+			.output_size = 1,
+			.result = ISC_R_NOSPACE,
+		},
+		{
+			NS_TEST_ID("target buffer too small to fit NULL byte"),
+			.input = "/foo.so",
+			.output_size = 7,
+			.result = ISC_R_NOSPACE,
+		},
+#ifndef WIN32
+		{
+			NS_TEST_ID("target buffer too small to fit full path"),
+			.input = "foo.so",
+			.output_size = 7,
+			.result = ISC_R_NOSPACE,
+		},
+#endif
+	};
+
+	for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
+		run_full_path_test(&tests[i], state);
+	}
+}
+
+int
+main(void) {
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test_setup_teardown(ns_plugin_expandpath_test,
+						_setup, _teardown),
+	};
+
+	return (cmocka_run_group_tests(tests, NULL, NULL));
+}
+#else /* HAVE_CMOCKA */
+
+#include <stdio.h>
+
+int
+main(void) {
+	printf("1..0 # Skipped: cmocka not available\n");
+	return (0);
+}
+
+#endif

lib/ns/win32/libns.def

@@ -75,6 +75,7 @@ ns_log_init
 ns_log_setcontext
 ns_notify_start
 ns_plugin_check
+ns_plugin_expandpath
 ns_plugin_register
 ns_plugins_create
 ns_plugins_free

make/rules.in

@@ -28,6 +28,8 @@ localstatedir =	@localstatedir@
 mandir =	@mandir@
 datarootdir =   @datarootdir@
 
+plugindir =	${libdir}/named
+
 DESTDIR =
 
 @SET_MAKE@

util/copyrights

@@ -2510,6 +2510,7 @@
 ./lib/ns/tests/notify_test.c			C	2017,2018,2019
 ./lib/ns/tests/nstest.c				C	2017,2018,2019
 ./lib/ns/tests/nstest.h				C	2017,2018,2019
+./lib/ns/tests/plugin_test.c			C	2019
 ./lib/ns/tests/query_test.c			C	2017,2018,2019
 ./lib/ns/tests/testdata/notify/notify1.msg	X	2017,2018,2019
 ./lib/ns/update.c				C	2017,2018,2019