Fix rlm_totp and add new test suite

src/modules/rlm_totp/rlm_totp.c

@@ -39,13 +39,13 @@ typedef struct {
 
 static const call_env_t call_env[] = {
 	{ FR_CALL_ENV_OFFSET("secret", FR_TYPE_STRING, rlm_totp_call_env_t, secret,
-			     "&control.TOTP.Secret", T_BARE_WORD, false, false, false) },
+			     "&control.TOTP.Secret", T_BARE_WORD, false, true, false) },
 
 	{ FR_CALL_ENV_OFFSET("key", FR_TYPE_STRING, rlm_totp_call_env_t, key,
-			     "&control.TOTP.key", T_BARE_WORD, false, false, false) },
+			     "&control.TOTP.key", T_BARE_WORD, false, true, false) },
 
 	{ FR_CALL_ENV_OFFSET("user_password", FR_TYPE_STRING, rlm_totp_call_env_t, user_password,
-			     "&request.TOTP.From-User", T_BARE_WORD, false, false, false) },
+			     "&request.TOTP.From-User", T_BARE_WORD, false, true, false) },
 
 	CALL_ENV_TERMINATOR
 };
@@ -77,9 +77,7 @@ static int mod_bootstrap(module_inst_ctx_t const *mctx)
 	CONF_SECTION *conf = mctx->inst->conf;
 
 	inst->name = cf_section_name2(conf);
-		if (!inst->name) {
-		inst->name = cf_section_name1(conf);
-	}
+	if (!inst->name) inst->name = cf_section_name1(conf);
 
 	return 0;
 }
@@ -163,7 +161,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authenticate(rlm_rcode_t *p_result,
 		our_keylen = len;
 	}
 
-	if (fr_totp_cmp(&inst->totp, request, fr_time_to_sec(request->packet->timestamp), our_key, our_keylen, secret->vb_strvalue) != 0) RETURN_MODULE_FAIL;
+	if (fr_totp_cmp(&inst->totp, request, fr_time_to_sec(request->packet->timestamp), our_key, our_keylen, user_password->vb_strvalue) != 0) RETURN_MODULE_FAIL;
 
 	RETURN_MODULE_OK;
 }
@@ -188,7 +186,8 @@ module_rlm_t rlm_totp = {
 		.instantiate	= mod_instantiate
 	},
 	.method_names = (module_method_name_t[]){
-		{ .name1 = "authenticate",	.name2 = CF_IDENT_ANY,		.method = mod_authenticate,	.method_env = &method_env },
+		{ .name1 = "authenticate",	.name2 = CF_IDENT_ANY,	.method = mod_authenticate,	.method_env = &method_env },
+		{ .name1 = CF_IDENT_ANY,	.name2 = CF_IDENT_ANY,	.method = mod_authenticate,	.method_env = &method_env },
 		MODULE_NAME_TERMINATOR
 	}
 };

src/tests/all.mk

@@ -64,6 +64,7 @@ test: \
 		test.tacacs	\
 		test.vmps	\
 		test.ldap_sync	\
+		test.totp	\
 		| build.raddb
 
 clean: clean.test

src/tests/totp/.gitignore

@@ -0,0 +1,4 @@
+# We use foo.txt.in to generate foo.txt
+*.txt
+# We use foo.out.in to generate foo.out
+*.out

src/tests/totp/all.mk

@@ -0,0 +1,107 @@
+#
+#	Unit tests for totp+radclient against the radiusd/rlm_totp.
+#
+
+#
+#	Test name
+#
+TEST  := test.totp
+FILES := $(subst $(DIR)/,,$(wildcard $(DIR)/*.txt.in))
+
+$(eval $(call TEST_BOOTSTRAP))
+
+#
+#  Ensure that the digest tests are run if the server or rlm_digest module changes
+#
+$(FILES.$(TEST)): $(BUILD_DIR)/lib/rlm_totp.la $(BUILD_DIR)/bin/radiusd$(E) $(BUILD_DIR)/bin/radclient$(E)
+
+#
+#	Config settings
+#
+TOTP_BUILD_DIR  := $(BUILD_DIR)/tests/totp
+TOTP_RADIUS_LOG := $(RADCLIENT_BUILD_DIR)/radiusd.log
+TOTP_GDB_LOG    := $(RADCLIENT_BUILD_DIR)/gdb.log
+
+#
+#	Client port
+#
+RADCLIENT_CLIENT_PORT := 1234
+
+#
+#	Get the TOTP token
+#
+TOTP_KEY := 12345678901234567890
+export TOTP_KEY
+
+#
+#  Generic rules to start / stop the radius service.
+#
+CLIENT := radclient
+include src/tests/radiusd.mk
+$(eval $(call RADIUSD_SERVICE,radiusd,$(OUTPUT)))
+
+#
+#	Run the radclient commands against the radiusd.
+#
+$(OUTPUT)/%: $(DIR)/% | $(TEST).radiusd_kill $(TEST).radiusd_start
+	$(eval TARGET_IN     := $(patsubst %.txt.in,%.txt,$<))
+	$(eval TARGET        := $(notdir $(TARGET_IN))$(E))
+	$(eval EXPECTED      := $(patsubst %.txt.in,%.out,$<))
+	$(eval FOUND         := $(patsubst %.txt.in,%.out,$@))
+	$(eval TOTP_GEN_ARGV := $(shell grep "#.*ARGV:" $< | cut -f2 -d ':'))
+	$(eval TOTP_TOKEN    := $(shell $(top_srcdir)/scripts/totp/totp-gen.py -k $(TOTP_KEY) $(TOTP_GEN_ARGV)))
+
+	${Q}echo "TOTP-TEST INPUT=$(TARGET) TOKEN=$(TOTP_TOKEN) TOTP_GEN_ARGV=\"$(TOTP_GEN_ARGV)\""
+	${Q}[ -f $(dir $@)/radiusd.pid ] || exit 1
+#
+#	Create the 'expected' with generated otp token
+#
+
+	${Q}sed "s/%TOTP_TOKEN%/$(TOTP_TOKEN)/g" $< > $(TARGET_IN)
+	${Q}sed "s/%TOTP_TOKEN%/$(TOTP_TOKEN)/g" $(EXPECTED).in > $(EXPECTED)
+	${Q}if ! $(TEST_BIN)/radclient -f $(TARGET_IN) -xF -d src/tests/totp/config -D share/dictionary 127.0.0.1:$(totp_port) auth $(SECRET) 1> $(FOUND) 2>&1; then \
+		echo "FAILED";                                              \
+		cat $(FOUND);                                               \
+		rm -f $(BUILD_DIR)/tests/test.totp;		                    \
+		$(MAKE) --no-print-directory test.totp.radiusd_kill;        \
+		echo "RADIUSD:   $(RADIUSD_RUN)";                           \
+		echo "RADCLIENT: $(TEST_BIN)/radclient -f $(TARGET_IN) -xF -d src/tests/totp/config -D share/dictionary 127.0.0.1:$(totp_port) auth $(SECRET)"; \
+		exit 1;                                                     \
+	fi
+#
+#	Lets normalize the loopback interface on OSX and FreeBSD
+#
+	${Q}if [ "$$(uname -s)" = "Darwin" ]; then sed -i.bak 's/via lo0/via lo/g' $(FOUND); fi
+	${Q}if [ "$$(uname -s)" = "FreeBSD" ]; then sed -i.bak 's/via (null)/via lo/g' $(FOUND); fi
+#
+#	Remove all entries with "^_EXIT.*CALLED .*/"
+#	It is necessary to match all builds with/without -DNDEBUG
+#
+	${Q}sed -i.bak '/^_EXIT.*CALLED .*/d' $(FOUND)
+#
+#	Ignore spurious output from jlibtool when VERBOSE=1
+#
+	${Q}sed -i.bak '$${/Executing: /d;}' $(FOUND)
+#
+#	Checking.
+#
+#	diff between src/test/totp/$test.out & build/test/totp/$test.out
+#
+	${Q}if ! diff -I 'Sent' -I 'Received' $(EXPECTED) $(FOUND); then  \
+		echo "RADCLIENT FAILED $@";                                 \
+		echo "RADIUSD:   $(RADIUSD_RUN)";                           \
+		echo "RADCLIENT: $(TEST_BIN)/radclient -f $(TARGET_IN) -xF -d src/tests/totp/config -D share/dictionary 127.0.0.1:$(totp_port) auth $(SECRET)"; \
+		echo "ERROR: File $(FOUND) is not the same as $(EXPECTED)"; \
+		echo "If you did some update on the radclient code, please be sure to update the unit tests."; \
+		echo "e.g: $(EXPECTED)";                                    \
+		diff -I 'Sent' -I 'Received' $(EXPECTED) $(FOUND);                                  \
+		rm -f $(BUILD_DIR)/tests/test.totp;		    \
+		$(MAKE) --no-print-directory test.totp.radiusd_kill;   \
+		exit 1;                                                     \
+	fi
+	${Q}touch $@
+
+.NO_PARALLEL: $(TEST)
+$(TEST):
+	${Q}$(MAKE) --no-print-directory $@.radiusd_stop
+	@touch $(BUILD_DIR)/tests/$@

src/tests/totp/config/radiusd.conf

@@ -0,0 +1,148 @@
+#  -*- text -*-
+#
+#  test configuration file.  Do not install.
+#
+#  $Id$
+#
+
+#
+#  Minimal radiusd.conf for testing
+#
+
+testdir      = $ENV{TESTDIR}
+output       = $ENV{OUTPUT}
+run_dir      = ${output}
+raddb        = raddb
+pidfile      = ${run_dir}/radiusd.pid
+panic_action = "gdb -batch -x src/tests/panic.gdb %e %p > ${run_dir}/gdb.log 2>&1; cat ${run_dir}/gdb.log"
+
+maindir      = ${raddb}
+radacctdir   = ${run_dir}/radacct
+modconfdir   = ${maindir}/mods-config
+certdir      = ${maindir}/certs
+cadir        = ${maindir}/certs
+test_port    = $ENV{TEST_PORT}
+
+#  Only for testing!
+#  Setting this on a production system is a BAD IDEA.
+security {
+	allow_vulnerable_openssl = yes
+}
+
+policy {
+	files.authorize {
+		if (&User-Name == "bob") {
+			&control.Password.Cleartext := "bob"
+		}
+	}
+	$INCLUDE ${maindir}/policy.d/
+}
+
+client localhost {
+	ipaddr = 127.0.0.1
+	secret = testing123
+}
+
+modules {
+	always reject {
+		rcode = reject
+	}
+	always fail {
+		rcode = fail
+	}
+	always ok {
+		rcode = ok
+	}
+	always handled {
+		rcode = handled
+	}
+	always invalid {
+		rcode = invalid
+	}
+	always disallow {
+		rcode = disallow
+	}
+	always notfound {
+		rcode = notfound
+	}
+	always noop {
+		rcode = noop
+	}
+	always updated {
+		rcode = updated
+	}
+
+	totp totp_6digit {
+		time_step = 30
+		otp_length = 6
+		lookback_steps = 1
+		lookback_interval = 30
+	}
+
+	totp totp_8digit {
+		time_step = 30
+		otp_length = 8
+		lookback_steps = 1
+		lookback_interval = 30
+	}
+}
+
+#
+#  This virtual server is chosen for processing requests when using:
+#
+#	radiusd -Xd src/tests/ -i 127.0.0.1 -p 12340 -n test
+#
+server test {
+	namespace = radius
+
+	listen {
+		type = Access-Request
+		type = Accounting-Request
+		transport = udp
+
+		udp {
+			ipaddr = 127.0.0.1
+			port = ${test_port}
+		}
+	}
+
+	recv Access-Request {
+		# Key...
+		&control.TOTP.key = "$ENV{TOTP_KEY}"
+#		&control.TOTP.Secret = $ENV{TOTP_KEY}
+
+		# Then, let's get the token
+		if (&request.User-Name =~ /^([a-zA-Z0-9]+)\+([0-9]+)$/) {
+			&request.User-Name := %{1}
+			&request.TOTP.From-User := %{2}
+
+			if (%(length:%{request.TOTP.From-User}) == 6) {
+				totp_6digit
+			} elsif (%(length:%{request.TOTP.From-User}) == 8) {
+				totp_8digit
+			} else {
+				&reply.Reply-Message := "FAILED! Token has incorrect length. Expected 6 or 8"
+				reject
+			}
+
+			if (ok) {
+				&reply.Reply-Message := "TOTP@%{request.TOTP.From-User} OK"
+				accept
+			} else {
+				&reply.Reply-Message := "TOTP@%{request.TOTP.From-User} FAILED"
+				reject
+			}
+		} else {
+			&reply.Reply-Message := "FAILED! Excepted format: username+token"
+			reject
+		}
+	}
+
+	authenticate totp_6digit {
+		totp_6digit
+	}
+
+	authenticate totp_8digit {
+		totp_8digit
+	}
+}

src/tests/totp/now_token_6digits.out.in

@@ -0,0 +1,5 @@
+Sent Access-Request Id 0 from 0.0.0.0:1235 to 127.0.0.1:12354 length 32 
+        User-Name = "bob+%TOTP_TOKEN%"
+Received Access-Accept Id 0 from 127.0.0.1:12354 to 0.0.0.0:1235 via lo length 29 
+        Reply-Message = "TOTP@%TOTP_TOKEN% OK"
+(0) src/tests/totp/now_token_6digits.txt response code 2

src/tests/totp/now_token_6digits.txt.in

@@ -0,0 +1,4 @@
+#
+#	ARGV: -d 6 -D sha1
+#
+User-Name = "bob+%TOTP_TOKEN%"

src/tests/totp/now_token_8digits.out.in

@@ -0,0 +1,5 @@
+Sent Access-Request Id 0 from 0.0.0.0:1235 to 127.0.0.1:12354 length 32 
+        User-Name = "bob+%TOTP_TOKEN%"
+Received Access-Accept Id 0 from 127.0.0.1:12354 to 0.0.0.0:1235 via lo length 29 
+        Reply-Message = "TOTP@%TOTP_TOKEN% OK"
+(0) src/tests/totp/now_token_8digits.txt response code 2

src/tests/totp/now_token_8digits.txt.in

@@ -0,0 +1,4 @@
+#
+#	ARGV: -d 8 -D sha1
+#
+User-Name = "bob+%TOTP_TOKEN%"