nsp: delete session_check directive

Sending SIGHUP to the process is meant to reload the GAL. This is
done by destroying all ab_base objects on SIGHUP, and recreating
ab_base on next use of the AB provider.

Each new ab_base object has a GUID of its own, to detect such GAL
changes. An NSP client that has connected earlier, recording the GUID
of the time, falls prey to seeing a new GUID after SIGHUP, and
rejecting all NSP requests.

The change of GUID should, with current judgment, not be treated as
an error condition. Instead, an NSP session should keep a reference
to the earlier ab_base. Considering that minids are stable in Gromox
due to being constructed from strictly monotonically increasing SQL
user/domain ids makes keeping around old ab_base objects unnecessary.
Subsequently, deleting the session_check is the logical choice.

Fixes: gromox-0~666

Author: jengelh

doc/changelog.rst

@@ -11,6 +11,8 @@ Enhancements:
 
 Fixes:
 
+* nsp: remove meaningless session_check directive;
+  no longer erroneously reject requests after daemon received SIGHUP
 * oxcical: avoid setting out-of-spec MAPI recurnum for FREQ=MONTHLY,BYDAY=
   recurrences
 * oxcical: fix wrong BYMONTH calculation for MONTHNTH recurrences being

doc/exchange_nsp.4gx

@@ -24,9 +24,6 @@ Dump more data.
 .br
 Default: \fI0\fP
 .TP
-\fBsession_check\fP
-Default: \fIfalse\fP
-.TP
 \fBx500_org_name\fP
 Default: (unspecified)
 .SH Notes

exch/nsp/main.cpp

@@ -29,7 +29,6 @@ static constexpr cfg_directive nsp_cfg_defaults[] = {
 	{"cache_interval", "5min", CFG_TIME, "1s", "1d"},
 	{"hash_table_size", "3000", CFG_SIZE, "1"},
 	{"nsp_trace", "0"},
-	{"session_check", "1", CFG_BOOL},
 	{"x500_org_name", "Gromox default"},
 	CFG_TABLE_END,
 };
@@ -58,7 +57,6 @@ static constexpr DCERPC_INTERFACE interface = {
 
 BOOL PROC_exchange_nsp(enum plugin_op reason, const struct dlfuncs &ppdata)
 {
-	BOOL b_check;
 	const char *org_name;
 	int cache_interval;
 	char temp_buff[45];
@@ -83,9 +81,6 @@ BOOL PROC_exchange_nsp(enum plugin_op reason, const struct dlfuncs &ppdata)
 		HX_unit_seconds(temp_buff, std::size(temp_buff), cache_interval, 0);
 		mlog(LV_INFO, "nsp: address book tree item"
 				" cache interval is %s", temp_buff);
-		b_check = pfile->get_ll("session_check");
-		if (b_check)
-			mlog(LV_INFO, "nsp: bind session will be checked");
 		ab_tree::AB.init(org_name, cache_interval);
 
 		query_service2("exmdb_client_get_named_propids", get_named_propids);
@@ -137,7 +132,7 @@ BOOL PROC_exchange_nsp(enum plugin_op reason, const struct dlfuncs &ppdata)
 			mlog(LV_ERR, "nsp: failed to run address book tree");
 			return FALSE;
 		}
-		nsp_interface_init(b_check);
+		nsp_interface_init();
 		return TRUE;
 	}
 	case PLUGIN_FREE:

exch/nsp/nsp_interface.cpp

@@ -59,7 +59,6 @@ enum {
 };
 
 unsigned int g_nsp_trace;
-static bool g_session_check;
 static gromox::archive abkt_archive;
 
 static void nsp_trace(const char *func, bool is_exit, const STAT *s,
@@ -565,9 +564,8 @@ static ec_error_t nsp_interface_fetch_row(const ab_tree::ab_node &node,
 	return ecSuccess;
 }
 
-void nsp_interface_init(bool b_check)
+void nsp_interface_init()
 {
-	g_session_check = b_check;
 	static constexpr char pk[] = PKGDATADIR "/abkt.pak";
 	auto err = abkt_archive.open(pk);
 	if (err != 0)
@@ -686,7 +684,12 @@ static void nsp_interface_position_in_table(const STAT *pstat,
 
 static inline bool session_check(const NSPI_HANDLE &h, const ab_tree::ab_base &base)
 {
-	return g_session_check && base.guid() == h.guid;
+	/*
+	 * Gromox minid assignment is stable across reloads;
+	 * at present, there is no need for action when GUIDs are different.
+	 */
+	//return do_session_checking && base.guid() == h.guid;
+	return true;
 }
 
 ec_error_t nsp_interface_update_stat(NSPI_HANDLE handle, uint32_t reserved,
@@ -699,7 +702,10 @@ ec_error_t nsp_interface_update_stat(NSPI_HANDLE handle, uint32_t reserved,
 	auto pbase = ab_tree::AB.get(handle.guid);
 	if (pbase == nullptr || !session_check(handle, *pbase))
 		return ecError;
-
+	/*
+	 * pbase->guid can be different from handle.guid, but for now that is not
+	 * an actionable condition, as minids are stable across AB reloads.
+	 */
 	uint32_t init_row = 0, total = 0;
 	ab_tree::ab_node node{};
 	if (0 == pstat->container_id) {
@@ -944,6 +950,8 @@ ec_error_t nsp_interface_seek_entries(NSPI_HANDLE handle, uint32_t reserved,
 	} else if (pproptags->cvalues > 100) {
 		return ecTableTooBig;
 	}
+	if (handle.handle_type != HANDLE_EXCHANGE_NSP)
+		return ecError;
 	auto pbase = ab_tree::AB.get(handle.guid);
 	if (pbase == nullptr || !session_check(handle, *pbase))
 		return ecError;

exch/nsp/nsp_interface.hpp

@@ -4,7 +4,7 @@
 
 #include "nsp_types.hpp"
 
-extern void nsp_interface_init(bool b_check);
+extern void nsp_interface_init();
 extern ec_error_t nsp_interface_bind(uint64_t hrpc, uint32_t flags, const STAT *, FLATUID *server_guid, NSPI_HANDLE *);
 extern ec_error_t nsp_interface_unbind(NSPI_HANDLE *, uint32_t);
 extern ec_error_t nsp_interface_update_stat(NSPI_HANDLE, uint32_t, STAT *, int32_t *delta);

lib/ab_tree.cpp

@@ -11,6 +11,7 @@
 #include <gromox/proc_common.h>
 #include <gromox/usercvt.hpp>
 #include <gromox/util.hpp>
+#include <gromox/process.hpp>
 
 namespace gromox::ab_tree
 {