From 5ff6c6eefe58137a3300ccc77ff24200f4a4d40b Mon Sep 17 00:00:00 2001 From: Jorge Pereira Date: Thu, 14 Sep 2023 01:00:05 -0300 Subject: [PATCH 1/4] totp: Add tests validating rlm_totp * That script get a real token using scripts/totp/totp-gen.py * It's validate using 'radclient' sending a valid packet. --- src/tests/all.mk | 1 + src/tests/totp/.gitignore | 4 + src/tests/totp/all.mk | 107 +++++++++++++++++ src/tests/totp/config/radiusd.conf | 148 ++++++++++++++++++++++++ src/tests/totp/now_token_6digits.out.in | 5 + src/tests/totp/now_token_6digits.txt.in | 4 + src/tests/totp/now_token_8digits.out.in | 5 + src/tests/totp/now_token_8digits.txt.in | 4 + 8 files changed, 278 insertions(+) create mode 100644 src/tests/totp/.gitignore create mode 100644 src/tests/totp/all.mk create mode 100644 src/tests/totp/config/radiusd.conf create mode 100644 src/tests/totp/now_token_6digits.out.in create mode 100644 src/tests/totp/now_token_6digits.txt.in create mode 100644 src/tests/totp/now_token_8digits.out.in create mode 100644 src/tests/totp/now_token_8digits.txt.in diff --git a/src/tests/all.mk b/src/tests/all.mk index 5f1dd13fd47da..f4d62fb0fdbf7 100644 --- a/src/tests/all.mk +++ b/src/tests/all.mk @@ -64,6 +64,7 @@ test: \ test.tacacs \ test.vmps \ test.ldap_sync \ + test.totp \ | build.raddb clean: clean.test diff --git a/src/tests/totp/.gitignore b/src/tests/totp/.gitignore new file mode 100644 index 0000000000000..6c03d5cf1242c --- /dev/null +++ b/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 diff --git a/src/tests/totp/all.mk b/src/tests/totp/all.mk new file mode 100644 index 0000000000000..ce79aa80d903c --- /dev/null +++ b/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/$@ diff --git a/src/tests/totp/config/radiusd.conf b/src/tests/totp/config/radiusd.conf new file mode 100644 index 0000000000000..89f43a1f186ae --- /dev/null +++ b/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 + } +} diff --git a/src/tests/totp/now_token_6digits.out.in b/src/tests/totp/now_token_6digits.out.in new file mode 100644 index 0000000000000..402d3a1b8b6fb --- /dev/null +++ b/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 diff --git a/src/tests/totp/now_token_6digits.txt.in b/src/tests/totp/now_token_6digits.txt.in new file mode 100644 index 0000000000000..c3655ea71bf91 --- /dev/null +++ b/src/tests/totp/now_token_6digits.txt.in @@ -0,0 +1,4 @@ +# +# ARGV: -d 6 -D sha1 +# +User-Name = "bob+%TOTP_TOKEN%" diff --git a/src/tests/totp/now_token_8digits.out.in b/src/tests/totp/now_token_8digits.out.in new file mode 100644 index 0000000000000..7458c639653bd --- /dev/null +++ b/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 diff --git a/src/tests/totp/now_token_8digits.txt.in b/src/tests/totp/now_token_8digits.txt.in new file mode 100644 index 0000000000000..c54f65a093159 --- /dev/null +++ b/src/tests/totp/now_token_8digits.txt.in @@ -0,0 +1,4 @@ +# +# ARGV: -d 8 -D sha1 +# +User-Name = "bob+%TOTP_TOKEN%" From 6fff68e080be13024209dcd04b5123372129828f Mon Sep 17 00:00:00 2001 From: Jorge Pereira Date: Wed, 4 Oct 2023 13:42:09 -0300 Subject: [PATCH 2/4] totp: Fix call_env setting The 'nullable' field should be true. --- src/modules/rlm_totp/rlm_totp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/rlm_totp/rlm_totp.c b/src/modules/rlm_totp/rlm_totp.c index d9d36660da153..077a235a05a98 100644 --- a/src/modules/rlm_totp/rlm_totp.c +++ b/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 }; From 03a3715769d00a0b51f42f5e0f5ed02063282ad1 Mon Sep 17 00:00:00 2001 From: Jorge Pereira Date: Wed, 4 Oct 2023 13:43:11 -0300 Subject: [PATCH 3/4] totp: The fr_totp_cmp() expects 'user_password' value. The user TOTP token is expected over 'contro.TOTP.From-User' --- src/modules/rlm_totp/rlm_totp.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/modules/rlm_totp/rlm_totp.c b/src/modules/rlm_totp/rlm_totp.c index 077a235a05a98..cb5c229763b0e 100644 --- a/src/modules/rlm_totp/rlm_totp.c +++ b/src/modules/rlm_totp/rlm_totp.c @@ -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; } From 91fae752ad0cc8b1666a99bf8c57af2c2f70dac3 Mon Sep 17 00:00:00 2001 From: Jorge Pereira Date: Wed, 4 Oct 2023 13:44:49 -0300 Subject: [PATCH 4/4] totp: Fix methods set --- src/modules/rlm_totp/rlm_totp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/rlm_totp/rlm_totp.c b/src/modules/rlm_totp/rlm_totp.c index cb5c229763b0e..d204fa339450f 100644 --- a/src/modules/rlm_totp/rlm_totp.c +++ b/src/modules/rlm_totp/rlm_totp.c @@ -186,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 } };