commit 5df615ac3647e50a71feb195c17b2ba3dc2b491b
parent f857c916e86f2ff4ff12f8fed51646ebb2f59082
Author: Christian Grothoff <christian@grothoff.org>
Date: Thu, 6 Aug 2026 14:47:55 +0200
more precise PIN state management: distinguish between PINs that were actually sent and those that we are trying to send in the DB
Diffstat:
13 files changed, 917 insertions(+), 4 deletions(-)
diff --git a/src/challenger/cat-once.sh b/src/challenger/cat-once.sh
@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+# This file is in the public domain.
+#
+# Like cat.sh, but only for the first challenge: every later invocation
+# fails without delivering anything, simulating an SMS/e-mail gateway
+# that went down between two PIN transmissions.
+TARGET="$(echo $1 | jq -r ".filename")"
+if [ -e "${TARGET}.sent" ]
+then
+ exit 1
+fi
+touch "${TARGET}.sent"
+cat - > "${TARGET}"
+exit 0
diff --git a/src/challenger/challenger-httpd_challenge.c b/src/challenger/challenger-httpd_challenge.c
@@ -187,6 +187,12 @@ struct ChallengeContext
bool db_finished;
/**
+ * True if we handed a new PIN to the transmission helper and it still
+ * needs to be confirmed in the database once the helper succeeded.
+ */
+ bool pin_pending;
+
+ /**
* Is the upload in JSON?
*/
bool is_json;
@@ -491,6 +497,9 @@ send_tan (struct ChallengeContext *bc)
bc->cwh = GNUNET_wait_child (bc->child,
&child_done_cb,
bc);
+ /* The PIN is now in the hands of the helper; if it terminates
+ successfully, the main handler promotes it in the database. */
+ bc->pin_pending = true;
MHD_suspend_connection (bc->hc->connection);
bc->suspended = GNUNET_YES;
GNUNET_CONTAINER_DLL_insert (bc_head,
@@ -1064,6 +1073,49 @@ CH_handler_challenge (struct CH_HandlerContext *hc,
}
}
+ if (bc->pin_pending)
+ {
+ /* The helper terminated successfully (checked above), so the PIN really
+ went out and only now becomes the PIN we accept. Had the helper
+ failed, we returned 502 above and the previous PIN (if any) remains
+ valid. */
+ bc->pin_pending = false;
+ for (unsigned int r = 0; r<MAX_RETRIES; r++)
+ {
+ enum GNUNET_DB_QueryStatus qs;
+
+ qs = CHALLENGERDB_do_challenge_address_confirm_pin (
+ CH_context,
+ &bc->nonce,
+ &bc->pin_attempts_left);
+ switch (qs)
+ {
+ case GNUNET_DB_STATUS_HARD_ERROR:
+ GNUNET_break (0);
+ return reply_error (bc,
+ MHD_HTTP_INTERNAL_SERVER_ERROR,
+ TALER_EC_GENERIC_DB_STORE_FAILED,
+ "do_challenge_address_confirm_pin");
+ case GNUNET_DB_STATUS_SOFT_ERROR:
+ if (r < MAX_RETRIES - 1)
+ continue;
+ GNUNET_break (0);
+ return reply_error (bc,
+ MHD_HTTP_INTERNAL_SERVER_ERROR,
+ TALER_EC_GENERIC_DB_STORE_FAILED,
+ "do_challenge_address_confirm_pin");
+ case GNUNET_DB_STATUS_SUCCESS_NO_RESULTS:
+ /* Validation was solved or removed while we were transmitting;
+ there is nothing to promote, but the PIN did go out. */
+ GNUNET_break_op (0);
+ break;
+ case GNUNET_DB_STATUS_SUCCESS_ONE_RESULT:
+ break;
+ }
+ break;
+ }
+ }
+
{
json_t *args;
struct MHD_Response *resp;
diff --git a/src/challenger/meson.build b/src/challenger/meson.build
@@ -16,6 +16,7 @@ check_SCRIPTS = [
'test-challenger-pinlimit',
'test-challenger-db-retry',
'test-challenger-token-errors',
+ 'test-challenger-pinfail',
]
test_helper_cat = configure_file(input: 'cat.sh', output: 'cat.sh', copy: true)
@@ -38,6 +39,18 @@ test_conf = configure_file(
copy: true,
)
+test_conf = configure_file(
+ input: 'test-challenger-pinfail.conf',
+ output: 'test-challenger-pinfail.conf',
+ copy: true,
+)
+
+test_helper_cat_once = configure_file(
+ input: 'cat-once.sh',
+ output: 'cat-once.sh',
+ copy: true,
+)
+
foreach s : check_SCRIPTS
tscript = '@0@.sh'.format(s)
test_exe = configure_file(input: tscript, output: tscript, copy: true)
diff --git a/src/challenger/test-challenger-pinfail.conf b/src/challenger/test-challenger-pinfail.conf
@@ -0,0 +1,21 @@
+[challenger]
+
+# Delivers the first PIN and then fails, see cat-once.sh.
+AUTH_COMMAND = cat-once.sh
+
+# What address type are we validating? (SMS, e-mail, etc.)
+ADDRESS_TYPE = file-access
+
+# Base URL
+BASE_URL = http://localhost/
+
+# Minimal cooldown between (re)transmissions, so that the test can ask
+# for a retransmission after a short sleep. Do not lower this below 1s:
+# /authorize reports 'last_tx_time + PIN_RETRANSMISSION_FREQUENCY' as a
+# timestamp, and timestamps are truncated to seconds -- a zero timestamp
+# cannot be packed into the reply.
+PIN_RETRANSMISSION_FREQUENCY = 1 s
+
+[challengerdb-postgres]
+#The connection string the plugin has to use for connecting to the database
+CONFIG = postgres:///talercheck
diff --git a/src/challenger/test-challenger-pinfail.sh b/src/challenger/test-challenger-pinfail.sh
@@ -0,0 +1,239 @@
+#!/usr/bin/env bash
+# This file is in the public domain.
+#
+# Tests that a failed PIN transmission does not invalidate the PIN the
+# user already received.
+#
+# Regression test: /challenge used to commit the new PIN (overwriting
+# last_pin) before forking the AUTH_COMMAND helper, so a helper failure
+# returned 502 *and* left the user without a usable PIN until the
+# retransmission cooldown elapsed.
+
+set -eu
+
+# Exit, with status code "skip" (no 'real' failure)
+function exit_skip() {
+ echo " SKIP: $1"
+ exit 77
+}
+
+# Exit, with error message (hard failure)
+function exit_fail() {
+ echo " FAIL: $@"
+ exit 1
+}
+
+# Cleanup to run whenever we exit
+function cleanup()
+{
+ for n in $(jobs -p)
+ do
+ kill $n 2> /dev/null || true
+ done
+ rm -f "$LAST_RESPONSE" "$FILENAME" "$FILENAME.sent"
+ rm -f "$FILENAME2" "$FILENAME2.sent"
+ wait
+}
+
+LAST_RESPONSE=$(mktemp responseXXXXXX.log)
+FILENAME="test-challenger-pinfail.txt"
+FILENAME2="test-challenger-pinfail-changed.txt"
+
+# Install cleanup handler (except for kill -9)
+trap cleanup EXIT
+
+export PATH="$PATH:."
+
+echo -n "Testing for jq"
+jq -h > /dev/null || exit_skip "jq required"
+echo " FOUND"
+echo -n "Testing for curl"
+curl -h > /dev/null || exit_skip "curl required"
+echo " FOUND"
+echo -n "Testing for wget"
+wget -h > /dev/null || exit_skip "wget required"
+echo " FOUND"
+echo -n "Testing for challenger-httpd ..."
+challenger-httpd -h > /dev/null || exit_skip "challenger-httpd required"
+echo " FOUND"
+
+CONF="test-challenger-pinfail.conf"
+BURL="http://localhost:9967"
+REDIRECT_URI="http://client.example.com/"
+
+rm -f "$FILENAME" "$FILENAME.sent"
+rm -f "$FILENAME2" "$FILENAME2.sent"
+
+echo -n "Initialize challenger database ..."
+challenger-dbinit -r -c "${CONF}" &> dbinit.log
+echo " OK"
+
+echo -n "Add challenger client ..."
+CLIENT_SECRET="secret-token:secret"
+challenger-admin -c "${CONF}" -a "${CLIENT_SECRET}" "${REDIRECT_URI}" &> admin.log
+echo " OK"
+# We just reset the DB, thus the client ID must be 1 here:
+CLIENT_ID=1
+
+echo -n "Start challenger-httpd ..."
+challenger-httpd -L INFO -c "${CONF}" &> httpd.log &
+
+# Wait for challenger to be available
+for n in $(seq 1 50)
+do
+ echo -n "."
+ sleep 0.2
+ OK=0
+ wget --tries=1 --timeout=1 "${BURL}/config" -o /dev/null -O /dev/null >/dev/null || continue
+ OK=1
+ break
+done
+if [ 1 != $OK ]
+then
+ exit_skip "Failed to launch challenger service"
+fi
+echo " OK"
+
+echo -n "Setup new validation process..."
+STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -d '' \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+NONCE=$(jq -r .nonce < "$LAST_RESPONSE")
+echo " OK"
+
+CLIENT_STATE="the-client-state"
+CLIENT_SCOPE="the-client-scope"
+
+echo -n "Initiating user login..."
+STATUS=$(curl "${BURL}/authorize/${NONCE}" \
+ -G \
+ -H "Accept: application/json" \
+ --data-urlencode "response_type=code" \
+ --data-urlencode "client_id=${CLIENT_ID}" \
+ --data-urlencode "redirect_uri=${REDIRECT_URI}" \
+ --data-urlencode "state=${CLIENT_STATE}" \
+ --data-urlencode "scope=${CLIENT_SCOPE}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+echo " OK"
+
+echo -n "Initiating address submission..."
+STATUS=$(curl "${BURL}/challenge/${NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "filename=${FILENAME}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+echo " OK"
+
+PIN=$(cat ${FILENAME} | awk '{print $5}')
+
+# The helper now refuses to deliver anything (see cat-once.sh).
+sleep 1.5
+echo -n "Requesting a retransmission that the helper will fail..."
+STATUS=$(curl "${BURL}/challenge/${NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "filename=${FILENAME}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "502" ]
+then
+ exit_fail "Expected 502 Bad Gateway. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+echo " OK"
+
+# The PIN the user got in the first message must still work: the PIN of
+# the failed transmission never reached anybody.
+echo -n "Solving with the PIN ${PIN} from the delivered message..."
+RESULT=$(curl "${BURL}/solve/${NONCE}" \
+ -X POST \
+ -H "Accept: text/html" \
+ --data-urlencode "pin=${PIN}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$RESULT" != "302" ]
+then
+ exit_fail "Expected 302 (PIN still valid). Got: $RESULT" $(cat $LAST_RESPONSE)
+fi
+echo " OK"
+
+# The flip side: deferring the new PIN must not keep the *old* PIN alive
+# across an address change. A changed address always (re)transmits -- the
+# change refills the transmission budget and clears the cooldown -- so
+# before this patch the PIN on file was replaced in the same call. Now it
+# is only replaced once the helper confirms, and a helper failure must
+# therefore leave the validation with no usable PIN rather than with the
+# PIN that went to the address the user just replaced.
+
+echo -n "Setup a second validation process..."
+STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -d '' \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+NONCE=$(jq -r .nonce < "$LAST_RESPONSE")
+echo " OK"
+
+echo -n "Transmitting a PIN for the first address..."
+STATUS=$(curl "${BURL}/challenge/${NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "filename=${FILENAME2}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+OLD_PIN=$(awk '{print $5}' < "${FILENAME2}")
+echo " OK"
+
+# The helper refuses to deliver anything from here on (see cat-once.sh);
+# the address change transmits regardless of the cooldown.
+echo -n "Changing the address while the helper is down..."
+STATUS=$(curl "${BURL}/challenge/${NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "filename=${FILENAME2}" \
+ --data-urlencode "tag=corrected" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "502" ]
+then
+ exit_fail "Expected 502 Bad Gateway. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+echo " OK"
+
+echo -n "The PIN of the previous address must not solve the new one..."
+RESULT=$(curl "${BURL}/solve/${NONCE}" \
+ -X POST \
+ -H "Accept: text/html" \
+ --data-urlencode "pin=${OLD_PIN}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$RESULT" = "302" ] || [ "$RESULT" = "200" ]
+then
+ exit_fail "the PIN sent for the previous address attested the new one"
+fi
+echo " OK ($RESULT)"
+
+exit 0
diff --git a/src/challenger/test-challenger-pkce-downgrade.sh b/src/challenger/test-challenger-pkce-downgrade.sh
@@ -0,0 +1,216 @@
+#!/usr/bin/env bash
+# This file is in the public domain.
+#
+# Regression test: a second /authorize must not be able to *remove* the PKCE
+# code challenge of a validation that already has one. /authorize
+# authenticates nobody and the nonce is recoverable from an issued code, so
+# stripping the binding would let anyone who learns the nonce turn a
+# PKCE-protected authorization into an unprotected one.
+
+set -eu
+
+# Exit, with status code "skip" (no 'real' failure)
+function exit_skip() {
+ echo " SKIP: $1"
+ exit 77
+}
+
+# Exit, with error message (hard failure)
+function exit_fail() {
+ echo " FAIL: $@"
+ exit 1
+}
+
+# Cleanup to run whenever we exit
+function cleanup()
+{
+ for n in $(jobs -p)
+ do
+ kill $n 2> /dev/null || true
+ done
+ rm -f "$LAST_RESPONSE" "$FILENAME"
+ wait
+}
+
+LAST_RESPONSE=$(mktemp responseXXXXXX.log)
+FILENAME="test-challenger-pkce-downgrade.txt"
+
+# Install cleanup handler (except for kill -9)
+trap cleanup EXIT
+
+export PATH="$PATH:."
+
+echo -n "Testing for jq"
+jq -h > /dev/null || exit_skip "jq required"
+echo " FOUND"
+echo -n "Testing for curl"
+curl -h > /dev/null || exit_skip "curl required"
+echo " FOUND"
+echo -n "Testing for wget"
+wget -h > /dev/null || exit_skip "wget required"
+echo " FOUND"
+echo -n "Testing for challenger-httpd ..."
+challenger-httpd -h > /dev/null || exit_skip "challenger-httpd required"
+echo " FOUND"
+
+CONF="test-challenger-pkce.conf"
+BURL="http://localhost:9967"
+REDIRECT_URI="http://client.example.com/"
+
+echo -n "Initialize challenger database ..."
+challenger-dbinit -r -c "${CONF}" &> dbinit.log
+echo " OK"
+
+echo -n "Add challenger client ..."
+CLIENT_SECRET="secret-token:secret"
+challenger-admin -c "${CONF}" -a "${CLIENT_SECRET}" "${REDIRECT_URI}" &> admin.log
+echo " OK"
+# We run this test on a fresh database, thus the client ID must be 1 here:
+CLIENT_ID=1
+
+echo -n "Start challenger-httpd ..."
+challenger-httpd -L INFO -c "${CONF}" &> httpd.log &
+
+# Wait for challenger to be available
+for n in $(seq 1 50)
+do
+ echo -n "."
+ sleep 0.2
+ OK=0
+ wget --tries=1 --timeout=1 "${BURL}/config" -o /dev/null -O /dev/null >/dev/null || continue
+ OK=1
+ break
+done
+if [ 1 != $OK ]
+then
+ exit_skip "Failed to launch challenger service"
+fi
+
+echo -n "Setup new validation process..."
+STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -d '' \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+NONCE=$(jq -r .nonce < "$LAST_RESPONSE")
+echo " OK"
+
+CLIENT_STATE="the-client-state"
+CLIENT_SCOPE="the-client-scope"
+CODE_CHALLENGE_METHOD="S256"
+CODE_CHALLENGE="3FtH5SoMllpyFP-nlcdmkGYraTpnhA-9U1N6tHoUYv8"
+CODE_VERIFIER="z167JIUt0F.II.qLPlCaXmL8BI6x9E-qqHAE_xEO_8p"
+
+echo -n "Initiating user login with code_challenge..."
+STATUS=$(curl "${BURL}/authorize/${NONCE}" \
+ -G \
+ -H "Accept: application/json" \
+ --data-urlencode "response_type=code" \
+ --data-urlencode "client_id=${CLIENT_ID}" \
+ --data-urlencode "redirect_uri=${REDIRECT_URI}" \
+ --data-urlencode "state=${CLIENT_STATE}" \
+ --data-urlencode "scope=${CLIENT_SCOPE}" \
+ --data-urlencode "code_challenge_method=${CODE_CHALLENGE_METHOD}" \
+ --data-urlencode "code_challenge=${CODE_CHALLENGE}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+echo "OK"
+
+# This is the attack: the very same (unauthenticated) request again, but
+# without any PKCE arguments. It is allowed to happen, it may just not
+# discard the code challenge that is already on file.
+echo -n "Replaying user login without code_challenge..."
+STATUS=$(curl "${BURL}/authorize/${NONCE}" \
+ -G \
+ -H "Accept: application/json" \
+ --data-urlencode "response_type=code" \
+ --data-urlencode "client_id=${CLIENT_ID}" \
+ --data-urlencode "redirect_uri=${REDIRECT_URI}" \
+ --data-urlencode "state=${CLIENT_STATE}" \
+ --data-urlencode "scope=${CLIENT_SCOPE}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+echo "OK"
+
+echo -n "Initiating address submission..."
+STATUS=$(curl "${BURL}/challenge/${NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "filename=${FILENAME}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+echo "OK"
+
+PIN=$(cat ${FILENAME} | awk '{print $5}')
+
+echo -n "Initiating PIN ${PIN} submission..."
+RESULT=$(curl "${BURL}/solve/${NONCE}" \
+ -X POST \
+ -H "Accept: text/html" \
+ --data-urlencode "pin=${PIN}" \
+ -w "%{http_code} %{redirect_url}" -s -o $LAST_RESPONSE)
+STATUS=$(echo "$RESULT" | awk '{print $1}')
+TARGET=$(echo "$RESULT" | awk '{print $2}')
+
+if [ "$STATUS" != "302" ]
+then
+ exit_fail "Expected 302. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+TCODE=$(echo "$TARGET" | sed -e "s/.*?code=//g" -e "s/&.*//g")
+echo "OK"
+
+echo -n "Requesting authorization without code_verifier ..."
+STATUS=$(curl "${BURL}/token" \
+ -X POST \
+ --data-urlencode "client_id=${CLIENT_ID}" \
+ --data-urlencode "redirect_uri=${REDIRECT_URI}" \
+ --data-urlencode "client_secret=${CLIENT_SECRET}" \
+ --data-urlencode "code=${TCODE}" \
+ --data-urlencode "grant_type=authorization_code" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "400" ]
+then
+ exit_fail "PKCE binding was downgraded: expected 400 without code_verifier, got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+echo "OK"
+
+echo -n "Requesting authorization with the original code_verifier ..."
+STATUS=$(curl "${BURL}/token" \
+ -X POST \
+ --data-urlencode "client_id=${CLIENT_ID}" \
+ --data-urlencode "redirect_uri=${REDIRECT_URI}" \
+ --data-urlencode "client_secret=${CLIENT_SECRET}" \
+ --data-urlencode "code=${TCODE}" \
+ --data-urlencode "grant_type=authorization_code" \
+ --data-urlencode "code_verifier=${CODE_VERIFIER}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+TOKEN_TYPE="$(cat $LAST_RESPONSE | jq -r .token_type)"
+if [ "$TOKEN_TYPE" != "Bearer" ]
+then
+ exit_fail "Expected Bearer token. Got: $TOKEN_TYPE"
+fi
+echo "OK"
+
+exit 0
diff --git a/src/challenger/test-challenger-token-errors.sh b/src/challenger/test-challenger-token-errors.sh
@@ -0,0 +1,230 @@
+#!/usr/bin/env bash
+# This file is in the public domain.
+#
+# Regression test: /token must not be an oracle for the state of a
+# validation. The nonce is trivially recovered from any authorization
+# code (CH_code_to_nonce() just parses the part before the '-'), so every
+# check that runs before the code MAC is verified answers a question about
+# a validation the caller may not hold a code for.
+#
+# Three requests that all carry a *forged* code must therefore be answered
+# identically:
+# 1. a validation that exists but has no proven address yet,
+# 2. a nonce that is syntactically valid but unknown,
+# 3. a validation that was solved (so only the MAC is wrong).
+#
+# Before the fix these produced 409/invalid_request/9759,
+# 400/invalid_grant/9755 and 400/invalid_grant/9754 respectively.
+
+set -eu
+
+# Exit, with status code "skip" (no 'real' failure)
+function exit_skip() {
+ echo " SKIP: $1"
+ exit 77
+}
+
+# Exit, with error message (hard failure)
+function exit_fail() {
+ echo " FAIL: $@"
+ exit 1
+}
+
+# Cleanup to run whenever we exit
+function cleanup()
+{
+ for n in $(jobs -p)
+ do
+ kill $n 2> /dev/null || true
+ done
+ rm -f "$LAST_RESPONSE" "$FILENAME"
+ wait
+}
+
+LAST_RESPONSE=$(mktemp responseXXXXXX.log)
+FILENAME="test-challenger-token-errors.txt"
+
+# Install cleanup handler (except for kill -9)
+trap cleanup EXIT
+
+export PATH="$PATH:."
+
+echo -n "Testing for jq"
+jq -h > /dev/null || exit_skip "jq required"
+echo " FOUND"
+echo -n "Testing for curl"
+curl -h > /dev/null || exit_skip "curl required"
+echo " FOUND"
+echo -n "Testing for wget"
+wget -h > /dev/null || exit_skip "wget required"
+echo " FOUND"
+echo -n "Testing for challenger-httpd ..."
+challenger-httpd -h > /dev/null || exit_skip "challenger-httpd required"
+echo " FOUND"
+
+CONF="test-challenger.conf"
+BURL="http://localhost:9967"
+REDIRECT_URI="http://client.example.com/"
+
+echo -n "Initialize challenger database ..."
+challenger-dbinit -r -c "${CONF}" &> dbinit.log
+echo " OK"
+
+echo -n "Add challenger client ..."
+CLIENT_SECRET="secret-token:secret"
+challenger-admin -c "${CONF}" -a "${CLIENT_SECRET}" "${REDIRECT_URI}" &> admin.log
+echo " OK"
+# We just reset the DB, thus the client ID must be 1 here:
+CLIENT_ID=1
+
+echo -n "Start challenger-httpd ..."
+challenger-httpd -L INFO -c "${CONF}" &> httpd.log &
+
+# Wait for challenger to be available
+for n in $(seq 1 50)
+do
+ echo -n "."
+ sleep 0.2
+ OK=0
+ wget --tries=1 --timeout=1 "${BURL}/config" -o /dev/null -O /dev/null >/dev/null || continue
+ OK=1
+ break
+done
+if [ 1 != $OK ]
+then
+ exit_skip "Failed to launch challenger service"
+fi
+
+CLIENT_STATE="the-client-state"
+CLIENT_SCOPE="the-client-scope"
+
+# The MAC half of an authorization code is never parsed, only compared,
+# so any syntactically plausible value serves as a forgery.
+FORGED_MAC="00000000000000000000000000000000000000000000000000000"
+
+# Post a code to /token and store "<status> <body>" in TOKEN_RESULT.
+function post_token()
+{
+ local STATUS
+ STATUS=$(curl "${BURL}/token" \
+ -X POST \
+ --data-urlencode "client_id=${CLIENT_ID}" \
+ --data-urlencode "redirect_uri=${REDIRECT_URI}" \
+ --data-urlencode "client_secret=${CLIENT_SECRET}" \
+ --data-urlencode "code=$1" \
+ --data-urlencode "grant_type=authorization_code" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+ TOKEN_RESULT="${STATUS} $(jq -cS . < "$LAST_RESPONSE")"
+}
+
+# Run /setup and /authorize, leaving the nonce in NONCE.
+function start_validation()
+{
+ local STATUS
+ STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -d '' \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+ if [ "$STATUS" != "200" ]
+ then
+ exit_fail "Expected 200 OK from /setup. Got: $STATUS" $(cat $LAST_RESPONSE)
+ fi
+ NONCE=$(jq -r .nonce < "$LAST_RESPONSE")
+ STATUS=$(curl "${BURL}/authorize/${NONCE}" \
+ -G \
+ -H "Accept: application/json" \
+ --data-urlencode "response_type=code" \
+ --data-urlencode "client_id=${CLIENT_ID}" \
+ --data-urlencode "redirect_uri=${REDIRECT_URI}" \
+ --data-urlencode "state=${CLIENT_STATE}" \
+ --data-urlencode "scope=${CLIENT_SCOPE}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+ if [ "$STATUS" != "200" ]
+ then
+ exit_fail "Expected 200 OK from /authorize. Got: $STATUS" $(cat $LAST_RESPONSE)
+ fi
+}
+
+
+echo -n "Redeeming a forged code for a validation without address..."
+start_validation
+NO_ADDRESS_NONCE="${NONCE}"
+post_token "${NO_ADDRESS_NONCE}-${FORGED_MAC}"
+NO_ADDRESS_RESULT="${TOKEN_RESULT}"
+case "${NO_ADDRESS_RESULT}" in
+ 400\ *)
+ # Good!
+ ;;
+ *)
+ exit_fail "Expected 400 for a forged code, got: ${NO_ADDRESS_RESULT}"
+ ;;
+esac
+echo " OK"
+
+echo -n "Redeeming a forged code for an unknown nonce..."
+# Mangle the first character; the trailing bits of the last one are
+# padding, so only the leading character can be changed safely.
+case "${NO_ADDRESS_NONCE}" in
+ X*) UNKNOWN_NONCE="Y${NO_ADDRESS_NONCE:1}" ;;
+ *) UNKNOWN_NONCE="X${NO_ADDRESS_NONCE:1}" ;;
+esac
+post_token "${UNKNOWN_NONCE}-${FORGED_MAC}"
+UNKNOWN_RESULT="${TOKEN_RESULT}"
+echo " OK"
+
+echo -n "Redeeming a forged code for a solved validation..."
+start_validation
+SOLVED_NONCE="${NONCE}"
+STATUS=$(curl "${BURL}/challenge/${SOLVED_NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "filename=${FILENAME}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK from /challenge. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+PIN=$(cat ${FILENAME} | awk '{print $5}')
+RESULT=$(curl "${BURL}/solve/${SOLVED_NONCE}" \
+ -X POST \
+ -H "Accept: text/html" \
+ --data-urlencode "pin=${PIN}" \
+ -w "%{http_code} %{redirect_url}" -s -o $LAST_RESPONSE)
+STATUS=$(echo "$RESULT" | awk '{print $1}')
+if [ "$STATUS" != "302" ]
+then
+ exit_fail "Expected 302 from /solve. Got: $STATUS" $(cat $LAST_RESPONSE)
+fi
+TCODE=$(echo "$RESULT" | awk '{print $2}' | sed -e "s/.*?code=//g" -e "s/&.*//g")
+post_token "${SOLVED_NONCE}-${FORGED_MAC}"
+BAD_MAC_RESULT="${TOKEN_RESULT}"
+echo " OK"
+
+echo -n "Checking /token does not distinguish the three failures..."
+if [ "${NO_ADDRESS_RESULT}" != "${BAD_MAC_RESULT}" ]
+then
+ exit_fail "/token reveals that no address was provided:" \
+ "'${NO_ADDRESS_RESULT}' vs '${BAD_MAC_RESULT}'"
+fi
+if [ "${UNKNOWN_RESULT}" != "${BAD_MAC_RESULT}" ]
+then
+ exit_fail "/token reveals that the validation is unknown:" \
+ "'${UNKNOWN_RESULT}' vs '${BAD_MAC_RESULT}'"
+fi
+echo " OK"
+
+# The genuine code must of course still work; the solved validation above
+# was never consumed, since all we sent for it was a forgery.
+echo -n "Redeeming the genuine code..."
+post_token "${TCODE}"
+case "${TOKEN_RESULT}" in
+ 200\ *)
+ # Good!
+ ;;
+ *)
+ exit_fail "Expected 200 for the genuine code, got: ${TOKEN_RESULT}"
+ ;;
+esac
+echo " OK"
+
+exit 0
diff --git a/src/challengerdb/challenger-0005.sql b/src/challengerdb/challenger-0005.sql
@@ -0,0 +1,34 @@
+--
+-- This file is part of Challenger
+-- Copyright (C) 2026 Taler Systems SA
+--
+-- Challenger is free software; you can redistribute it and/or modify it under the
+-- terms of the GNU General Public License as published by the Free Software
+-- Foundation; either version 3, or (at your option) any later version.
+--
+-- Challenger is distributed in the hope that it will be useful, but WITHOUT ANY
+-- WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+-- A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+--
+-- You should have received a copy of the GNU General Public License along with
+-- Challenger; see the file COPYING. If not, see <http://www.gnu.org/licenses/>
+--
+
+-- Everything in one big transaction
+BEGIN;
+
+-- Check patch versioning is in place.
+SELECT _v.register_patch('challenger-0005', NULL, NULL);
+
+SET search_path TO challenger;
+
+-- A PIN that was generated for transmission but whose delivery has not
+-- been confirmed yet. It only becomes 'last_pin' once the AUTH_COMMAND
+-- helper exited successfully, so that a failed transmission does not
+-- invalidate the PIN the user may already hold.
+ALTER TABLE validations
+ ADD COLUMN pending_pin INT4 DEFAULT NULL;
+COMMENT ON COLUMN validations.pending_pin
+ IS 'PIN that was passed to the AUTH_COMMAND helper but whose transmission was not confirmed yet; promoted to last_pin when the helper terminates successfully';
+
+COMMIT;
diff --git a/src/challengerdb/do_challenge_address.c b/src/challengerdb/do_challenge_address.c
@@ -120,3 +120,39 @@ CHALLENGERDB_do_challenge_address (
return GNUNET_DB_STATUS_SUCCESS_NO_RESULTS;
return qs;
}
+
+
+enum GNUNET_DB_QueryStatus
+CHALLENGERDB_do_challenge_address_confirm_pin (
+ struct CHALLENGERDB_PostgresContext *ctx,
+ const struct CHALLENGER_ValidationNonceP *nonce,
+ uint32_t *auth_attempts_left)
+{
+ struct GNUNET_PQ_QueryParam params[] = {
+ GNUNET_PQ_query_param_auto_from_type (nonce),
+ GNUNET_PQ_query_param_end
+ };
+ struct GNUNET_PQ_ResultSpec rs[] = {
+ GNUNET_PQ_result_spec_uint32 ("auth_attempts_left",
+ auth_attempts_left),
+ GNUNET_PQ_result_spec_end
+ };
+
+ PREPARE (ctx,
+ "do_challenge_address_confirm_pin",
+ "UPDATE validations SET"
+ " last_pin=pending_pin"
+ " ,pending_pin=NULL"
+ " ,auth_attempts_left=3"
+ " WHERE nonce=$1"
+ /* nothing to promote if we did not just transmit a PIN */
+ " AND pending_pin IS NOT NULL"
+ /* never resurrect an already solved validation */
+ " AND auth_attempts_left >= 0"
+ " RETURNING auth_attempts_left;");
+ return GNUNET_PQ_eval_prepared_singleton_select (
+ ctx->conn,
+ "do_challenge_address_confirm_pin",
+ params,
+ rs);
+}
diff --git a/src/challengerdb/do_challenge_address.sql b/src/challengerdb/do_challenge_address.sql
@@ -50,6 +50,7 @@ SELECT address
,last_tx_time
,client_redirect_uri
,last_pin
+ ,pending_pin
,auth_attempts_left
,client_state
INTO my_status
@@ -130,17 +131,31 @@ THEN
my_status.address = in_address::JSONB::TEXT;
my_status.pin_transmissions_left = 3;
my_status.last_tx_time = 0;
+ -- The PIN generated below is now merely 'pending' until the helper
+ -- confirms the transmission, so -- unlike before this patch -- this call
+ -- no longer necessarily overwrites 'last_pin'. The PIN on file went to
+ -- the *previous* address, so it must be dropped here: if the helper then
+ -- fails, the user must be left without a usable PIN rather than with one
+ -- that attests an address it was never sent to.
+ my_status.last_pin = NULL;
+ my_status.pending_pin = NULL;
+ my_status.auth_attempts_left = 0;
+ out_last_pin = 0;
+ out_auth_attempts_left = 0;
my_do_update=TRUE;
END IF;
IF ( (my_status.pin_transmissions_left > 0) AND
(my_status.last_tx_time <= in_retransmit_cutoff) )
THEN
- -- enough time has passed since the last transmission, so
- -- we are changing the PIN, update counters
+ -- Enough time has passed since the last transmission, so we are changing
+ -- the PIN, update counters. The new PIN is only stored as 'pending_pin':
+ -- it is promoted to 'last_pin' (and 'auth_attempts_left' reset) by
+ -- CHALLENGERDB_do_challenge_address_confirm_pin() once the AUTH_COMMAND
+ -- helper confirmed the transmission. Until then the PIN the user may
+ -- already hold from an earlier transmission stays valid.
my_status.pin_transmissions_left = my_status.pin_transmissions_left - 1;
- my_status.last_pin = in_tan;
- my_status.auth_attempts_left = 3;
+ my_status.pending_pin = in_tan;
my_status.last_tx_time = in_now;
out_auth_attempts_left = 3;
out_pin_transmit=TRUE;
@@ -157,6 +172,7 @@ THEN
,pin_transmissions_left=my_status.pin_transmissions_left
,last_tx_time=my_status.last_tx_time
,last_pin=my_status.last_pin
+ ,pending_pin=my_status.pending_pin
,auth_attempts_left=my_status.auth_attempts_left
WHERE nonce=in_nonce;
END IF;
diff --git a/src/challengerdb/meson.build b/src/challengerdb/meson.build
@@ -20,6 +20,7 @@ generated_sql = [
['challenger-0002.sql', ['challenger-0002.sql']],
['challenger-0003.sql', ['challenger-0003.sql']],
['challenger-0004.sql', ['challenger-0004.sql']],
+ ['challenger-0005.sql', ['challenger-0005.sql']],
]
foreach g : generated_sql
diff --git a/src/challengerdb/test_challenger_db.c b/src/challengerdb/test_challenger_db.c
@@ -149,6 +149,16 @@ challenge_validation (struct CHALLENGER_ValidationNonceP *nonce,
&address_refused,
&solved);
json_decref (address);
+ /* The PIN is only pending until its transmission is confirmed; this
+ stands in for a successful AUTH_COMMAND run. */
+ qs = CHALLENGERDB_do_challenge_address_confirm_pin (pg,
+ nonce,
+ &auth_attempts_left);
+ if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs)
+ {
+ GNUNET_break (0);
+ return GNUNET_SYSERR;
+ }
GNUNET_free (state);
GNUNET_free (redirect_uri);
if ( (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs) ||
diff --git a/src/include/challenger-database/do_challenge_address.h b/src/include/challenger-database/do_challenge_address.h
@@ -33,6 +33,11 @@
* address did not change, the operation is successful even without
* the counter change.
*
+ * Note that a newly generated PIN is only stored as *pending*: it does not
+ * become the PIN we accept until
+ * #CHALLENGERDB_do_challenge_address_confirm_pin() is called, so that a
+ * failed transmission does not invalidate a PIN the user already holds.
+ *
* @param cls
* @param nonce unique nonce to use to identify the validation
* @param address the new address to validate
@@ -68,4 +73,30 @@ CHALLENGERDB_do_challenge_address (
bool *address_refused,
bool *solved);
+
+/**
+ * Confirm that the PIN generated by the last
+ * #CHALLENGERDB_do_challenge_address() call was actually transmitted to the
+ * address, and thus make it the PIN we accept from the user. Also resets
+ * the number of authentication attempts the user has on the new PIN.
+ *
+ * Must only be called once the transmission helper terminated successfully;
+ * if it is never called, the previous PIN (if any) remains valid.
+ *
+ * @param ctx database context to use
+ * @param nonce unique nonce identifying the validation
+ * @param[out] auth_attempts_left set to the number of attempts the user has
+ * on the now-current PIN
+ * @return transaction status:
+ * #GNUNET_DB_STATUS_SUCCESS_ONE_RESULT if the PIN was promoted
+ * #GNUNET_DB_STATUS_SUCCESS_NO_RESULTS if there was no pending PIN, or the
+ * validation is unknown or already solved
+ * #GNUNET_DB_STATUS_HARD_ERROR on failure
+ */
+enum GNUNET_DB_QueryStatus
+CHALLENGERDB_do_challenge_address_confirm_pin (
+ struct CHALLENGERDB_PostgresContext *ctx,
+ const struct CHALLENGER_ValidationNonceP *nonce,
+ uint32_t *auth_attempts_left);
+
#endif
