From: Nick Porter <nick@portercomputing.co.uk>
Date: Thu, 2 May 2024 10:15:48 +0000 (+0100)
Subject: Add `lookforward_steps` option to `rlm_totp`
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=bcde219f925953f6e21fdda4158bf17dd083a2cd;p=thirdparty%2Ffreeradius-server.git

Add `lookforward_steps` option to `rlm_totp`

Helps with clock skew between client and server (and module test where
the OTP is calcuated using an exec call after the packet starts
processing)
---

diff --git a/raddb/mods-available/totp b/raddb/mods-available/totp
index 22eabe8dc3..d1dc0cc20b 100644
--- a/raddb/mods-available/totp
+++ b/raddb/mods-available/totp
@@ -66,6 +66,11 @@ totp {
 	#
 	lookback_steps = 1
 
+	#
+	#  lookforward_steps:: How many steps forward in time we look for a matching OTP.
+	#
+	lookforward_steps = 0
+
 	#
 	#  lookback_interval:: Time delta between steps.
 	#
diff --git a/src/modules/rlm_totp/rlm_totp.c b/src/modules/rlm_totp/rlm_totp.c
index 4325bdbed6..8a292c3a3b 100644
--- a/src/modules/rlm_totp/rlm_totp.c
+++ b/src/modules/rlm_totp/rlm_totp.c
@@ -67,6 +67,7 @@ static const conf_parser_t module_config[] = {
 	{ FR_CONF_OFFSET("otp_length", rlm_totp_t, totp.otp_length), .dflt = "6" },
 	{ FR_CONF_OFFSET("lookback_steps", rlm_totp_t, totp.lookback_steps), .dflt = "1" },
 	{ FR_CONF_OFFSET("lookback_interval", rlm_totp_t, totp.lookback_interval), .dflt = "30" },
+	{ FR_CONF_OFFSET("lookforward_steps", rlm_totp_t, totp.lookforward_steps), .dflt = "0" },
 	CONF_PARSER_TERMINATOR
 };
 
@@ -101,6 +102,8 @@ static int mod_instantiate(module_inst_ctx_t const *mctx)
 	FR_INTEGER_BOUND_CHECK("lookback_steps", inst->totp.lookback_steps, >=, 1);
 	FR_INTEGER_BOUND_CHECK("lookback_steps", inst->totp.lookback_steps, <=, 10);
 
+	FR_INTEGER_BOUND_CHECK("lookforward_steps", inst->totp.lookforward_steps, <=, 10);
+
 	FR_INTEGER_BOUND_CHECK("lookback_interval", inst->totp.lookback_interval, <=, inst->totp.time_step);
 
 	FR_INTEGER_BOUND_CHECK("otp_length", inst->totp.otp_length, >=, 6);
diff --git a/src/modules/rlm_totp/totp.c b/src/modules/rlm_totp/totp.c
index 91b9cfcd42..76f4c30578 100644
--- a/src/modules/rlm_totp/totp.c
+++ b/src/modules/rlm_totp/totp.c
@@ -70,14 +70,15 @@ static void totp_log(char const *fmt, ...)
  */
 int fr_totp_cmp(fr_totp_t const *cfg, request_t *request, time_t now, uint8_t const *key, size_t keylen, char const *totp)
 {
-	time_t then;
-	unsigned int i;
-	uint8_t offset;
-	uint32_t challenge;
-	uint64_t padded;
-	uint8_t data[8];
-	uint8_t digest[SHA1_DIGEST_LENGTH];
-	char buffer[9];
+	time_t		diff, then;
+	uint32_t	steps;
+	unsigned int	i;
+	uint8_t		offset;
+	uint32_t	challenge;
+	uint64_t	padded;
+	uint8_t		data[8];
+	uint8_t		digest[SHA1_DIGEST_LENGTH];
+	char		buffer[9];
 
 	fr_assert(cfg->otp_length == 6 || cfg->otp_length == 8);
 
@@ -99,12 +100,15 @@ int fr_totp_cmp(fr_totp_t const *cfg, request_t *request, time_t now, uint8_t co
 
 	/*
 	 *	First try to authenticate against the current OTP, then step
-	 *	back in increments of `lookback_interval`, up to `lookback_steps` times,
+	 *	back and forwards in increments of `lookback_interval`, up to `lookback_steps` times,
 	 *	to authenticate properly in cases of long transit delay, as
 	 *	described in RFC 6238, section 5.2.
 	 */
-
-	for (i = 0, then = now; i <= cfg->lookback_steps; i++, then -= cfg->lookback_interval) {
+	steps = cfg->lookback_steps > cfg->lookforward_steps ? cfg->lookback_steps : cfg->lookforward_steps;
+	for (i = 0, diff = 0; i <= steps; i++, diff += cfg->lookback_interval) {
+		if (i > cfg->lookback_steps) goto forwards;
+		then = now - diff;
+	repeat:
 		padded = ((uint64_t) then) / cfg->time_step;
 		data[0] = padded >> 56;
 		data[1] = padded >> 48;
@@ -153,6 +157,15 @@ int fr_totp_cmp(fr_totp_t const *cfg, request_t *request, time_t now, uint8_t co
 		}
 
 		if (fr_digest_cmp((uint8_t const *) buffer, (uint8_t const *) totp, cfg->otp_length) == 0) return 0;
+
+		/*
+		 *	We've tested backwards, now do the equivalent time slot forwards
+		 */
+		if ((then < now) && (i <= cfg->lookforward_steps)) {
+		forwards:
+			then = now + offset;
+			goto repeat;
+		}
 	}
 
 	return -1;
diff --git a/src/modules/rlm_totp/totp.h b/src/modules/rlm_totp/totp.h
index 33503b4e7b..cb7c7d456f 100644
--- a/src/modules/rlm_totp/totp.h
+++ b/src/modules/rlm_totp/totp.h
@@ -35,6 +35,7 @@ typedef struct {
 	uint32_t otp_length;		//!< forced to 6 or 8
 	uint32_t lookback_steps;	//!< number of steps to look back
 	uint32_t lookback_interval;	//!< interval in seconds between steps
+	uint32_t lookforward_steps;	//!< number of steps to look forwards
 } fr_totp_t;
 
 int fr_totp_cmp(fr_totp_t const *cfg, request_t *request, time_t now, uint8_t const *key, size_t keylen, char const *totp) CC_HINT(nonnull(1,4,6));
diff --git a/src/tests/modules/totp/module.conf b/src/tests/modules/totp/module.conf
index a6f27232cd..91fd59845e 100644
--- a/src/tests/modules/totp/module.conf
+++ b/src/tests/modules/totp/module.conf
@@ -1,4 +1,5 @@
 totp {
+	lookforward_steps = 1
 }
 
 exec {