From: Nick Porter <nick@portercomputing.co.uk>
Date: Fri, 3 Jun 2022 16:42:30 +0000 (+0100)
Subject: Move libldap init to global_lib framework (#4546)
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bbcebc5bb969e44a3a95b9b7ec34650f57294308;p=thirdparty%2Ffreeradius-server.git

Move libldap init to global_lib framework (#4546)

* Define libldap global config options and init / free callbacks

* Move libldap global options to global {} section

* Switch rlm_ldap to use autoinit of ldap library

* Clear old instance tracking from libldap

* Ensure global.d directory is handled correctly by packages

* Allow per module test global library settings

* Move global ldap library settings to correct location for tests

* Include global.d in list of raddb files to install

* WS

* Newline

Co-authored-by: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
---

diff --git a/debian/freeradius.postinst b/debian/freeradius.postinst
index ce51a1d66c1..72be7109f0d 100644
--- a/debian/freeradius.postinst
+++ b/debian/freeradius.postinst
@@ -47,6 +47,7 @@ case "$1" in
             /etc/freeradius/sites-available \
             /etc/freeradius/sites-enabled \
             /etc/freeradius/mods-config/attr_filter \
+            /etc/freeradius/global.d \
             /etc/freeradius/policy.d
           do
             if ! dpkg-statoverride --list | grep -qw $dir$; then
diff --git a/debian/freeradius.prerm b/debian/freeradius.prerm
index d35359ff000..b1a242ca2de 100644
--- a/debian/freeradius.prerm
+++ b/debian/freeradius.prerm
@@ -29,6 +29,7 @@ case "$1" in
           /etc/freeradius/mods-enabled \
           /etc/freeradius/mods-config/attr_filter \
           /etc/freeradius/mods-config \
+          /etc/freeradius/global.d \
           /etc/freeradius/policy.d \
           /var/run/freeradius \
           /var/log/freeradius
diff --git a/raddb/all.mk b/raddb/all.mk
index c6ac401f263..b281ffc7543 100644
--- a/raddb/all.mk
+++ b/raddb/all.mk
@@ -53,7 +53,7 @@ LEGACY_LINKS :=		$(addprefix $(R)$(raddbdir)/,users)
 
 BUILD_RADDB := $(strip $(foreach x,install clean,$(findstring $(x),$(MAKECMDGOALS))))
 ifneq "$(BUILD_RADDB)" ""
-RADDB_DIRS :=		certs mods-available mods-enabled policy.d template.d \
+RADDB_DIRS :=		certs mods-available mods-enabled global.d policy.d template.d \
 			sites-available sites-enabled \
 			$(patsubst raddb/%,%,$(call FIND_DIRS,raddb/mods-config))
 
@@ -65,6 +65,7 @@ INSTALL_FILES := 	$(wildcard raddb/sites-available/* raddb/mods-available/*) \
 			$(addprefix raddb/,$(LOCAL_FILES)) \
 			$(addprefix raddb/certs/,$(INSTALL_CERT_FILES)) \
 			$(call FIND_FILES,raddb/mods-config) \
+			$(call FIND_FILES,raddb/global.d) \
 			$(call FIND_FILES,raddb/policy.d) \
 			$(call FIND_FILES,raddb/template.d)
 
diff --git a/raddb/global.d/ldap b/raddb/global.d/ldap
new file mode 100644
index 00000000000..e4cb87defc5
--- /dev/null
+++ b/raddb/global.d/ldap
@@ -0,0 +1,47 @@
+ldap {
+	#
+	#  random_file:: Provides random number generator.
+	#
+#	random_file = /dev/urandom
+
+	#
+	#  ldap_debug:: Debug flags for libldap (see OpenLDAP documentation).
+	#  Set this to enable debugging output from different code areas within libldap.
+	#
+	#  NOTE: These debugging options can produce significant amounts of logging output.
+	#
+	#  [options="header,autowidth"]
+	#  |===
+	#  | Option             | Value
+	#  | LDAP_DEBUG_TRACE   | 0x0001
+	#  | LDAP_DEBUG_PACKETS | 0x0002
+	#  | LDAP_DEBUG_ARGS    | 0x0004
+	#  | LDAP_DEBUG_CONNS   | 0x0008
+	#  | LDAP_DEBUG_BER     | 0x0010
+	#  | LDAP_DEBUG_FILTER  | 0x0020
+	#  | LDAP_DEBUG_CONFIG  | 0x0040
+	#  | LDAP_DEBUG_ACL     | 0x0080
+	#  | LDAP_DEBUG_STATS   | 0x0100
+	#  | LDAP_DEBUG_STATS2  | 0x0200
+	#  | LDAP_DEBUG_SHELL   | 0x0400
+	#  | LDAP_DEBUG_PARSE   | 0x0800
+	#  | LDAP_DEBUG_SYNC    | 0x4000
+	#  | LDAP_DEBUG_NONE    | 0x8000
+	#  | LDAP_DEBUG_ANY     | (-1)
+	#  |===
+	#
+	#  e.g:
+	#
+	#  If you want to see the LDAP logs only for `trace` and `parse`,
+	#  facilities you should use:
+	#
+	#    (LDAP_DEBUG_TRACE + LDAP_DEBUG_PARSE) = 0x0801
+	#
+	#  Setting the `ldap_debug` configuration item as follows:
+	#
+	#     ldap_debug = 0x0801
+	#
+	#  Default: 0x0000 (no debugging messages)
+	#
+	ldap_debug = 0x0000
+}
diff --git a/raddb/mods-available/ldap b/raddb/mods-available/ldap
index d811482651d..eac37631a2b 100644
--- a/raddb/mods-available/ldap
+++ b/raddb/mods-available/ldap
@@ -149,57 +149,6 @@ ldap {
 	#
 #	valuepair_attribute = 'radiusAttribute'
 
-	#
-	#  ### Global
-	#
-	global {
-		#
-		#  random_file:: Provides random number generator.
-		#
-#		random_file = /dev/urandom
-
-		#
-		#  ldap_debug:: Debug flags for libldap (see OpenLDAP documentation).
-		#  Set this to enable debugging output from different code areas within libldap.
-		#
-		#  NOTE: These debugging options can produce significant amounts of logging output.
-		#
-		#  [options="header,autowidth"]
-		#  |===
-		#  | Option             | Value
-		#  | LDAP_DEBUG_TRACE   | 0x0001
-		#  | LDAP_DEBUG_PACKETS | 0x0002
-		#  | LDAP_DEBUG_ARGS    | 0x0004
-		#  | LDAP_DEBUG_CONNS   | 0x0008
-		#  | LDAP_DEBUG_BER     | 0x0010
-		#  | LDAP_DEBUG_FILTER  | 0x0020
-		#  | LDAP_DEBUG_CONFIG  | 0x0040
-		#  | LDAP_DEBUG_ACL     | 0x0080
-		#  | LDAP_DEBUG_STATS   | 0x0100
-		#  | LDAP_DEBUG_STATS2  | 0x0200
-		#  | LDAP_DEBUG_SHELL   | 0x0400
-		#  | LDAP_DEBUG_PARSE   | 0x0800
-		#  | LDAP_DEBUG_SYNC    | 0x4000
-		#  | LDAP_DEBUG_NONE    | 0x8000
-		#  | LDAP_DEBUG_ANY     | (-1)
-		#  |===
-		#
-		#  e.g:
-		#
-		#  If you want to see the LDAP logs only for `trace` and `parse`,
-		#  facilities you should use:
-		#
-		#    (LDAP_DEBUG_TRACE + LDAP_DEBUG_PARSE) = 0x0801
-		#
-		#  Setting the `ldap_debug` configuration item as follows:
-		#
-		#     ldap_debug = 0x0801
-		#
-		#  Default: 0x0000 (no debugging messages)
-		#
-		ldap_debug = 0x0000
-	}
-
 	#
 	#  ### Mapping of LDAP directory attributes to RADIUS dictionary attributes.
 	#
diff --git a/raddb/radiusd.conf.in b/raddb/radiusd.conf.in
index 54b7b7461f5..539832e46fb 100644
--- a/raddb/radiusd.conf.in
+++ b/raddb/radiusd.conf.in
@@ -560,6 +560,16 @@ thread pool {
 #
 #$INCLUDE trigger.conf
 
+#
+#  .Global Library Settings
+#
+#  Each library which has global settings will have its own configuration
+#  file in global.d
+#
+global {
+	$INCLUDE global.d/
+}
+
 #
 #  .Module Configuration
 #
diff --git a/redhat/freeradius.spec b/redhat/freeradius.spec
index 5a33ad459e9..ecf3c77c71e 100644
--- a/redhat/freeradius.spec
+++ b/redhat/freeradius.spec
@@ -776,6 +776,8 @@ fi
 %config(noreplace) %{_sysconfdir}/raddb/sites-enabled/*
 %dir %attr(750,root,radiusd) %{_sysconfdir}/raddb/policy.d
 %attr(640,root,radiusd) %config(noreplace) %{_sysconfdir}/raddb/policy.d/*
+%dir %attr(750,root,radiusd) %{_sysconfdir}/raddb/global.d
+%attr(640,root,radiusd) %config(noreplace) %{_sysconfdir}/raddb/global.d/*
 %dir %attr(750,root,radiusd) %{_sysconfdir}/raddb/template.d
 %attr(640,root,radiusd) %config(noreplace) %{_sysconfdir}/raddb/template.d/*
 %dir %attr(750,root,radiusd) %{_sysconfdir}/raddb/mods-available
diff --git a/src/lib/ldap/base.c b/src/lib/ldap/base.c
index 06440b0f576..9028c5ab6b2 100644
--- a/src/lib/ldap/base.c
+++ b/src/lib/ldap/base.c
@@ -41,8 +41,6 @@ LDAP *ldap_global_handle;			//!< Hack for OpenLDAP libldap global initialisation
 static _Thread_local LDAP *ldap_thread_local_handle;	//!< Hack for functions which require an ldap handle
 						///< but don't actually use it for anything.
 
-static uint32_t instance_count = 0;
-
 /** Used to set the global log prefix for functions which don't operate on connections
  *
  */
@@ -93,6 +91,54 @@ fr_table_num_sorted_t const fr_ldap_dereference[] = {
 };
 size_t fr_ldap_dereference_len = NUM_ELEMENTS(fr_ldap_dereference);
 
+static fr_libldap_global_config_t libldap_global_config = {
+	.ldap_debug = 0x00,
+	.tls_random_file = ""
+};
+
+static CONF_PARSER const ldap_global_config[] = {
+	{ FR_CONF_OFFSET("random_file", FR_TYPE_FILE_EXISTS, fr_libldap_global_config_t, tls_random_file) },
+	{ FR_CONF_OFFSET("ldap_debug", FR_TYPE_UINT32, fr_libldap_global_config_t, ldap_debug), .dflt = "0x0000" },
+	CONF_PARSER_TERMINATOR
+};
+
+/** Initialise libldap library and set global options
+ *
+ * Used as a callback from global library initialisation.
+ */
+static int libldap_init(void)
+{
+	if (fr_ldap_init() < 0) return -1;
+
+	fr_ldap_global_config(libldap_global_config.ldap_debug, libldap_global_config.tls_random_file);
+
+	return 0;
+}
+
+/** Free any global libldap resources
+ *
+ */
+static void libldap_free(void)
+{
+	/*
+	 *	Keeping the dummy ld around for the lifetime
+	 *	of the module should always work,
+	 *	irrespective of what changes happen in libldap.
+	 */
+	ldap_unbind_ext_s(ldap_global_handle, NULL, NULL);
+}
+
+/*
+ *	Public symbol modules can reference to auto instantiate libldap
+ */
+global_lib_autoinst_t fr_libldap_global_config = {
+	 .name = "ldap",
+	 .config = (const CONF_PARSER *)ldap_global_config,
+	 .inst = &libldap_global_config,
+	 .init = libldap_init,
+	 .free = libldap_free
+};
+
 typedef struct {
 	fr_ldap_query_t	*query;
 	LDAPMessage	**result;
@@ -978,11 +1024,6 @@ int fr_ldap_init(void)
 	static LDAPAPIInfo	info = { .ldapai_info_version = LDAP_API_INFO_VERSION };	/* static to quiet valgrind about this being uninitialised */
 	fr_ldap_config_t	*handle_config = &ldap_global_handle_config;
 
-	if (instance_count > 0) {
-		instance_count++;
-		return 0;
-	}
-
 	/*
 	 *	Only needs to be done once, prevents races in environment
 	 *	initialisation within libldap.
@@ -1039,22 +1080,5 @@ int fr_ldap_init(void)
 		     LDAP_VENDOR_VERSION_MAJOR, LDAP_VENDOR_VERSION_MINOR, LDAP_VENDOR_VERSION_PATCH);
 	}
 
-	instance_count++;
-
 	return 0;
 }
-
-/** Free any global libldap resources
- *
- */
-void fr_ldap_free(void)
-{
-	if (--instance_count > 0) return;
-
-	/*
-	 *	Keeping the dummy ld around for the lifetime
-	 *	of the module should always work,
-	 *	irrespective of what changes happen in libldap.
-	 */
-	ldap_unbind_ext_s(ldap_global_handle, NULL, NULL);
-}
diff --git a/src/lib/ldap/base.h b/src/lib/ldap/base.h
index e2a47d7764d..6784e3b04da 100644
--- a/src/lib/ldap/base.h
+++ b/src/lib/ldap/base.h
@@ -11,6 +11,7 @@
  */
 #include <freeradius-devel/server/base.h>
 #include <freeradius-devel/server/connection.h>
+#include <freeradius-devel/server/global_lib.h>
 #include <freeradius-devel/server/map.h>
 #include <freeradius-devel/server/trunk.h>
 
@@ -302,6 +303,17 @@ typedef struct {
 	fr_time_delta_t		idle_timeout;		//!< How long to wait before closing unused connections.
 } fr_ldap_config_t;
 
+/** libldap global configuration data
+ *
+ */
+typedef struct {
+	uint32_t		ldap_debug;		//!< LDAP debug level
+	char const		*tls_random_file;	//!< Path to the ramdon file if /dev/random and /dev/urandom
+							//!< are unavailable
+} fr_libldap_global_config_t;
+
+extern global_lib_autoinst_t fr_libldap_global_config;
+
 typedef struct fr_ldap_thread_trunk_s fr_ldap_thread_trunk_t;
 
 /** Tracks the state of a libldap connection handle
diff --git a/src/modules/rlm_ldap/rlm_ldap.c b/src/modules/rlm_ldap/rlm_ldap.c
index 431b24ad407..63da9c444fd 100644
--- a/src/modules/rlm_ldap/rlm_ldap.c
+++ b/src/modules/rlm_ldap/rlm_ldap.c
@@ -182,14 +182,6 @@ static CONF_PARSER option_config[] = {
 	CONF_PARSER_TERMINATOR
 };
 
-static const CONF_PARSER global_config[] = {
-	{ FR_CONF_OFFSET("random_file", FR_TYPE_FILE_EXISTS, rlm_ldap_t, tls_random_file) },
-
-	{ FR_CONF_OFFSET("ldap_debug", FR_TYPE_UINT32, rlm_ldap_t, ldap_debug), .dflt = "0x0000" },		/* Debugging flags to the server */
-
-	CONF_PARSER_TERMINATOR
-};
-
 static const CONF_PARSER module_config[] = {
 	/*
 	 *	Pool config items
@@ -228,8 +220,6 @@ static const CONF_PARSER module_config[] = {
 
 	{ FR_CONF_POINTER("options", FR_TYPE_SUBSECTION, NULL), .subcs = (void const *) option_config },
 
-	{ FR_CONF_POINTER("global", FR_TYPE_SUBSECTION, NULL), .subcs = (void const *) global_config },
-
 	{ FR_CONF_OFFSET("tls", FR_TYPE_SUBSECTION, rlm_ldap_t, handle_config), .subcs = (void const *) tls_config },
 
 	{ FR_CONF_OFFSET("pool", FR_TYPE_SUBSECTION, rlm_ldap_t, trunk_conf), .subcs = (void const *) fr_trunk_config },
@@ -270,6 +260,12 @@ fr_dict_attr_autoload_t rlm_ldap_dict_attr[] = {
 	{ NULL }
 };
 
+extern global_lib_autoinst_t const *rlm_ldap_lib[];
+global_lib_autoinst_t const *rlm_ldap_lib[] = {
+	&fr_libldap_global_config,
+	GLOBAL_LIB_TERMINATOR
+};
+
 
 static xlat_arg_parser_t const ldap_escape_xlat_arg = { .required = true, .concat = true, .type = FR_TYPE_STRING };
 
@@ -2224,31 +2220,12 @@ static int mod_instantiate(module_inst_ctx_t const *mctx)
 		}
 	}
 
-	/*
-	 *	Set global options
-	 */
-	if (fr_ldap_init() < 0) goto error;
-
-	fr_ldap_global_config(inst->ldap_debug, inst->tls_random_file);
-
 	return 0;
 
 error:
 	return -1;
 }
 
-static int mod_load(void)
-{
-	fr_ldap_init();
-
-	return 0;
-}
-
-static void mod_unload(void)
-{
-	fr_ldap_free();
-}
-
 /* globally exported name */
 extern module_rlm_t rlm_ldap;
 module_rlm_t rlm_ldap = {
@@ -2258,8 +2235,6 @@ module_rlm_t rlm_ldap = {
 		.type		= 0,
 		.inst_size	= sizeof(rlm_ldap_t),
 		.config		= module_config,
-		.onload		= mod_load,
-		.unload		= mod_unload,
 		.bootstrap	= mod_bootstrap,
 		.instantiate	= mod_instantiate,
 		.detach		= mod_detach,
diff --git a/src/modules/rlm_ldap/rlm_ldap.h b/src/modules/rlm_ldap/rlm_ldap.h
index 1a44f3cf1f2..3d96db18df8 100644
--- a/src/modules/rlm_ldap/rlm_ldap.h
+++ b/src/modules/rlm_ldap/rlm_ldap.h
@@ -136,14 +136,6 @@ typedef struct {
 	fr_pool_t	*pool;				//!< Connection pool instance.
 	fr_ldap_config_t handle_config;			//!< Connection configuration instance.
 	fr_trunk_conf_t	trunk_conf;			//!< Trunk configuration
-
-	/*
-	 *	Global config
-	 */
-	char const	*tls_random_file;		//!< Path to the random file if /dev/random and /dev/urandom
-							//!< are unavailable.
-
-	uint32_t	ldap_debug;			//!< Debug flag for the SDK.
 } rlm_ldap_t;
 
 extern HIDDEN fr_dict_attr_t const *attr_cleartext_password;
diff --git a/src/tests/modules/ldap/global.conf b/src/tests/modules/ldap/global.conf
new file mode 100644
index 00000000000..f3ffdd0e537
--- /dev/null
+++ b/src/tests/modules/ldap/global.conf
@@ -0,0 +1,12 @@
+global {
+	ldap {
+		#  ldap_debug: debug flag for LDAP SDK
+		#  (see OpenLDAP documentation).  Set this to enable
+		#  huge amounts of LDAP debugging on the screen.
+		#  You should only use this if you are an LDAP expert.
+		#
+		#	default: 0x0000 (no debugging messages)
+		#	Example:(LDAP_DEBUG_FILTER+LDAP_DEBUG_CONNS)
+		ldap_debug = 0x0801
+	}
+}
diff --git a/src/tests/modules/ldap/module.conf b/src/tests/modules/ldap/module.conf
index 20babda37f2..a15b17671d4 100644
--- a/src/tests/modules/ldap/module.conf
+++ b/src/tests/modules/ldap/module.conf
@@ -53,17 +53,6 @@ ldap {
 #		realm = 'example.org'
 	}
 
-	global {
-		#  ldap_debug: debug flag for LDAP SDK
-		#  (see OpenLDAP documentation).  Set this to enable
-		#  huge amounts of LDAP debugging on the screen.
-		#  You should only use this if you are an LDAP expert.
-		#
-		#	default: 0x0000 (no debugging messages)
-		#	Example:(LDAP_DEBUG_FILTER+LDAP_DEBUG_CONNS)
-		ldap_debug = 0x0801
-	}
-
 	#
 	#  Generic valuepair attribute
 	#
diff --git a/src/tests/modules/unit_test_module.conf b/src/tests/modules/unit_test_module.conf
index c0bea1417ec..7edba2b5f4b 100644
--- a/src/tests/modules/unit_test_module.conf
+++ b/src/tests/modules/unit_test_module.conf
@@ -28,6 +28,8 @@ modules {
 	$INCLUDE $ENV{MODULE_TEST_DIR}/module.conf
 }
 
+$-INCLUDE $ENV{MODULE_TEST_DIR}/global.conf
+
 $-INCLUDE $ENV{MODULE_TEST_DIR}/triggers.conf
 
 $-INCLUDE $ENV{MODULE_TEST_DIR}/clients.conf