commit f31b4b06ff0f7ae47676eee8879ca13d7046bd58
parent b2e71e2a0e5cb4d11dc881cc057a3a0ac0221aab
Author: Christian Grothoff <christian@grothoff.org>
Date: Thu, 6 Aug 2026 15:28:25 +0200
out_last_pin can be NULL, handle it correctly by preserving incoming tan in this case
Diffstat:
4 files changed, 86 insertions(+), 5 deletions(-)
diff --git a/src/challenger/test-challenger-pinfail.sh b/src/challenger/test-challenger-pinfail.sh
@@ -32,12 +32,14 @@ function cleanup()
done
rm -f "$LAST_RESPONSE" "$FILENAME" "$FILENAME.sent"
rm -f "$FILENAME2" "$FILENAME2.sent"
+ rm -f "$FILENAME3" "$FILENAME3.sent"
wait
}
LAST_RESPONSE=$(mktemp responseXXXXXX.log)
FILENAME="test-challenger-pinfail.txt"
FILENAME2="test-challenger-pinfail-changed.txt"
+FILENAME3="test-challenger-pinfail-nopin.txt"
# Install cleanup handler (except for kill -9)
trap cleanup EXIT
@@ -236,4 +238,69 @@ then
fi
echo " OK ($RESULT)"
+# Regression test for finding 24: since the PIN is only promoted to
+# 'last_pin' once the helper confirmed the transmission, a validation
+# whose every transmission failed has no PIN on file at all -- 'last_pin'
+# is SQL NULL. The caller reads 'out_last_pin' through a non-NULLable
+# result spec, so the fall-through path of the stored procedure must not
+# hand it NULL; it did, and that turned an ordinary "no transmissions
+# left" answer into a 500.
+
+# The marker makes cat-once.sh fail from the very first invocation, so no
+# PIN is ever confirmed for this validation.
+touch "${FILENAME3}.sent"
+
+echo -n "Setup a third validation whose PINs never get through..."
+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 "Spending all three transmissions on a helper that always fails..."
+for i in 1 2 3
+do
+ if [ "$i" != "1" ]
+ then
+ # Wait out the retransmission cooldown; the address does not
+ # change, so nothing else would clear it.
+ sleep 1.5
+ fi
+ STATUS=$(curl "${BURL}/challenge/${NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "filename=${FILENAME3}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+ if [ "$STATUS" != "502" ]
+ then
+ exit_fail "Expected 502 Bad Gateway. Got: $STATUS" $(cat $LAST_RESPONSE)
+ fi
+ echo -n "."
+done
+echo " OK"
+
+# No PIN was ever transmitted and no transmission is left, so this takes
+# the fall-through path with 'last_pin' still NULL. Note that this does
+# not depend on the cooldown: the empty transmission budget alone blocks
+# the (re)transmission.
+echo -n "A further /challenge with no PIN on file must not be a server error..."
+STATUS=$(curl "${BURL}/challenge/${NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "filename=${FILENAME3}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+
+if [ "$STATUS" != "429" ]
+then
+ exit_fail "Expected 429 (out of PIN transmissions). Got: $STATUS" \
+ $(cat $LAST_RESPONSE)
+fi
+echo " OK"
+
exit 0
diff --git a/src/challengerdb/challenger-0005.sql b/src/challengerdb/challenger-0005.sql
@@ -37,6 +37,13 @@ COMMENT ON COLUMN validations.pending_pin
ALTER TABLE validations
DROP CONSTRAINT IF EXISTS validations_validation_serial_id_key;
+-- Fix missing 'GENERATED ALWAYS' and UNIQUE constraints on the
+-- grant_serial_id
+ALTER TABLE tokens
+ ADD CONSTRAINT tokens_grant_serial_id_key UNIQUE (grant_serial_id),
+ ALTER COLUMN grant_serial_id SET GENERATED ALWAYS;
+COMMENT ON COLUMN tokens.grant_serial_id
+ IS 'Unique ID of the grant; published by /info as the resource identifier, hence GENERATED ALWAYS: it must not be settable by an INSERT';
COMMIT;
diff --git a/src/challengerdb/do_challenge_address.c b/src/challengerdb/do_challenge_address.c
@@ -69,13 +69,17 @@ CHALLENGERDB_do_challenge_address (
GNUNET_PQ_query_param_end
};
bool not_found;
+ bool no_last_tan;
+ uint32_t tan_out;
struct GNUNET_PQ_ResultSpec rs[] = {
GNUNET_PQ_result_spec_bool ("not_found",
¬_found),
GNUNET_PQ_result_spec_absolute_time ("last_tx_time",
last_tx_time),
- GNUNET_PQ_result_spec_uint32 ("last_pin",
- tan),
+ GNUNET_PQ_result_spec_allow_null (
+ GNUNET_PQ_result_spec_uint32 ("last_pin",
+ &tan_out),
+ &no_last_tan),
GNUNET_PQ_result_spec_bool ("pin_transmit",
pin_transmit),
GNUNET_PQ_result_spec_uint32 ("auth_attempts_left",
@@ -99,6 +103,7 @@ CHALLENGERDB_do_challenge_address (
enum GNUNET_DB_QueryStatus qs;
*client_redirect_uri = NULL;
+ no_last_tan = true;
PREPARE (ctx,
"do_challenge_address",
"SELECT "
@@ -122,6 +127,8 @@ CHALLENGERDB_do_challenge_address (
return qs;
if (not_found)
return GNUNET_DB_STATUS_SUCCESS_NO_RESULTS;
+ if (! no_last_tan)
+ *tan = tan_out;
return qs;
}
diff --git a/src/challengerdb/do_challenge_address.sql b/src/challengerdb/do_challenge_address.sql
@@ -67,7 +67,7 @@ IF NOT FOUND
THEN
out_not_found=TRUE;
out_last_tx_time=0;
- out_last_pin=0;
+ out_last_pin=NULL;
out_pin_transmit=FALSE;
out_auth_attempts_left=0;
out_pin_transmissions_left=0;
@@ -116,7 +116,7 @@ IF ( (0 = my_status.address_attempts_left) AND
my_address_differs )
THEN
out_address_refused=TRUE;
- out_last_pin=0;
+ out_last_pin=NULL;
RETURN;
END IF;
out_address_refused=FALSE;
@@ -146,7 +146,7 @@ THEN
my_status.last_pin = NULL;
my_status.pending_pin = NULL;
my_status.auth_attempts_left = 0;
- out_last_pin = 0;
+ out_last_pin = NULL;
out_auth_attempts_left = 0;
my_do_update=TRUE;
END IF;
