commit bbe3b5102013989149746f0aa9c3c04ea96befc7 parent c785d91aba07066bfc5cc17ae6da421903b135e5 Author: Christian Grothoff <christian@grothoff.org> Date: Wed, 23 Sep 2026 14:39:28 +0200 challenger: report permanently failed validations to the client Once the user is out of address changes, TAN transmissions and guesses (a read-only address counting as unchangeable), answer 410 and send them back to the client with an RFC 6749 access_denied error, so the exchange learns about the failure instead of the process staying pending forever. Issue: https://bugs.taler.net/n/11740 Signed-off-by: Christian Grothoff <christian@grothoff.org> Diffstat:
18 files changed, 834 insertions(+), 51 deletions(-)
diff --git a/src/challenger/challenger-httpd_authorize.c b/src/challenger/challenger-httpd_authorize.c @@ -253,6 +253,8 @@ CH_handler_authorize (struct CH_HandlerContext *hc, uint32_t auth_attempts_left; struct GNUNET_TIME_Absolute last_tx_time; bool solved; + bool failed; + char *final_redirect_uri = NULL; enum GNUNET_DB_QueryStatus qs; /* update_validation will return 0 if a 'redirect_uri' was @@ -273,6 +275,8 @@ CH_handler_authorize (struct CH_HandlerContext *hc, &pin_transmissions_left, &auth_attempts_left, &solved, + &failed, + &final_redirect_uri, &last_tx_time); switch (qs) { @@ -308,6 +312,22 @@ CH_handler_authorize (struct CH_HandlerContext *hc, } break; } + if (failed) + { + enum MHD_Result res; + + /* The user came back to a validation they can no longer pass; + send them (or tell the SPA to send them) to the client. */ + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Authorization requested for failed validation\n"); + json_decref (last_address); + res = CH_reply_validation_failed (hc->connection, + final_redirect_uri, + state); + GNUNET_free (final_redirect_uri); + return res; + } + GNUNET_free (final_redirect_uri); if (0 == CH_get_output_type (hc->connection)) { char *prev_full_url = hc->full_url; diff --git a/src/challenger/challenger-httpd_challenge.c b/src/challenger/challenger-httpd_challenge.c @@ -182,6 +182,12 @@ struct ChallengeContext bool retransmit; /** + * True if the validation failed permanently, i.e. the user is out + * of address changes, TAN transmissions and TAN attempts. + */ + bool failed; + + /** * Is the challenge already solved? */ bool solved; @@ -1171,7 +1177,8 @@ CH_handler_challenge (struct CH_HandlerContext *hc, &bc->retransmit, &bc->client_redirect_uri, &bc->address_refused, - &bc->solved); + &bc->solved, + &bc->failed); switch (qs) { case GNUNET_DB_STATUS_HARD_ERROR: @@ -1224,6 +1231,14 @@ CH_handler_challenge (struct CH_HandlerContext *hc, GNUNET_free (url); return ret; } + if (bc->failed) + { + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Validation failed, client exhausted all chances\n"); + return CH_reply_validation_failed (hc->connection, + bc->client_redirect_uri, + bc->state); + } if (bc->address_refused) { GNUNET_log (GNUNET_ERROR_TYPE_INFO, diff --git a/src/challenger/challenger-httpd_common.c b/src/challenger/challenger-httpd_common.c @@ -251,6 +251,91 @@ CH_reply_with_oauth_error ( } +enum MHD_Result +CH_reply_validation_failed (struct MHD_Connection *connection, + const char *client_redirect_uri, + const char *client_state) +{ + const char *desc = "address validation failed: all attempts exhausted"; + char *url = NULL; + struct MHD_Response *response; + unsigned int http_status; + enum MHD_Result ret; + + if (NULL != client_redirect_uri) + { + char *desc_enc; + char *state_enc; + char sep; + + /* RFC 6749 3.1.2 permits redirect URIs with a query component */ + sep = (NULL == strchr (client_redirect_uri, '?')) ? '?' : '&'; + desc_enc = TALER_urlencode (desc); + state_enc = (NULL == client_state) + ? NULL + : TALER_urlencode (client_state); + GNUNET_asprintf (&url, + "%s%cerror=access_denied&error_description=%s%s%s", + client_redirect_uri, + sep, + desc_enc, + (NULL == state_enc) ? "" : "&state=", + (NULL == state_enc) ? "" : state_enc); + GNUNET_free (desc_enc); + GNUNET_free (state_enc); + } + if ( (NULL != url) && + (0 == CH_get_output_type (connection)) ) + { + response = MHD_create_response_from_buffer (strlen (desc), + (void *) desc, + MHD_RESPMEM_PERSISTENT); + if (NULL == response) + { + GNUNET_break (0); + GNUNET_free (url); + return MHD_NO; + } + TALER_MHD_add_global_headers (response, + false); + GNUNET_break (MHD_YES == + MHD_add_response_header (response, + MHD_HTTP_HEADER_CONTENT_TYPE, + "text/plain")); + if (MHD_NO == + MHD_add_response_header (response, + MHD_HTTP_HEADER_LOCATION, + url)) + { + GNUNET_break (0); + MHD_destroy_response (response); + GNUNET_free (url); + return MHD_NO; + } + http_status = MHD_HTTP_FOUND; + } + else + { + response = TALER_MHD_MAKE_JSON_PACK ( + TALER_JSON_pack_ec (TALER_EC_CHALLENGER_VALIDATION_FAILED), + GNUNET_JSON_pack_allow_null ( + GNUNET_JSON_pack_string ("redirect_url", + url))); + http_status = MHD_HTTP_GONE; + } + GNUNET_free (url); + GNUNET_break (MHD_YES == + MHD_add_response_header (response, + MHD_HTTP_HEADER_CACHE_CONTROL, + "no-store,no-cache")); + ret = MHD_queue_response (connection, + http_status, + response); + MHD_destroy_response (response); + return ret; +} + + enum GNUNET_GenericReturnValue CH_build_full_redirect_url ( const struct CHALLENGER_ValidationNonceP *nonce, diff --git a/src/challenger/challenger-httpd_common.h b/src/challenger/challenger-httpd_common.h @@ -110,6 +110,28 @@ CH_build_full_redirect_url (const struct CHALLENGER_ValidationNonceP *nonce, /** + * The validation failed permanently: the user exhausted every address + * change, TAN transmission and TAN attempt. Report this to the client + * with an "access_denied" error response per section 4.1.2.1 of RFC 6749. + * + * A user agent asking for HTML is redirected to the client right away. + * Otherwise, we reply with a 410 #TALER_EC_CHALLENGER_VALIDATION_FAILED + * whose "redirect_url" the user agent must send the user to, so that the + * client learns about the failure. + * + * @param connection the MHD connection to use + * @param client_redirect_uri the client's redirect URI, NULL if unknown + * (then there is nobody to report to, and we only reply with the 410) + * @param client_state the client's OAuth2 state, NULL if none + * @return a MHD result code + */ +enum MHD_Result +CH_reply_validation_failed (struct MHD_Connection *connection, + const char *client_redirect_uri, + const char *client_state); + + +/** * Send a OAuth 2.0 response indicating an error following * section 5.2 of RFC 6749. * diff --git a/src/challenger/challenger-httpd_config.c b/src/challenger/challenger-httpd_config.c @@ -47,6 +47,13 @@ * and /info always carry the RFC 6749 "error" member plus a full * RFC 6750 "WWW-Authenticate" challenge. * 9: /setup returns the expiration time of the validation process. + * 10: a validation the user can no longer pass (out of address changes, + * TAN transmissions and TAN attempts) has failed for good: /solve, + * /challenge and /authorize answer 410 with + * TALER_EC_CHALLENGER_VALIDATION_FAILED and the "redirect_url" of an + * RFC 6749 "access_denied" error response to the client, to which + * HTML user agents are redirected right away. /solve no longer + * answers 429 with TALER_EC_CHALLENGER_TOO_MANY_ATTEMPTS. */ @@ -75,7 +82,7 @@ CH_handler_config (struct CH_HandlerContext *hc, GNUNET_JSON_pack_object_incref ("restrictions", CH_restrictions), GNUNET_JSON_pack_string ("version", - "9:0:7")); + "10:0:8")); } return MHD_queue_response (hc->connection, MHD_HTTP_OK, diff --git a/src/challenger/challenger-httpd_solve.c b/src/challenger/challenger-httpd_solve.c @@ -269,6 +269,7 @@ CH_handler_solve (struct CH_HandlerContext *hc, bool solved; bool exhausted; bool no_challenge; + bool failed; if (1 != sscanf (bc->pin, "%u%c", @@ -291,6 +292,7 @@ CH_handler_solve (struct CH_HandlerContext *hc, &solved, &exhausted, &no_challenge, + &failed, &bc->state, &bc->addr_left, &bc->auth_attempts_left, @@ -325,6 +327,19 @@ CH_handler_solve (struct CH_HandlerContext *hc, } break; } + if (failed) + { + /* Only given up on if *no* option remains: a /challenge + retransmission resets auth_attempts_left to 3, so as long as the + user may still request another TAN transmission (or change the + address) they can still succeed. This includes the response to + the guess that used up the last chance. */ + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Client exhausted all chances to satisfy challenge\n"); + return CH_reply_validation_failed (hc->connection, + bc->client_redirect_uri, + bc->state); + } if (! solved) { enum MHD_Result ret; @@ -332,22 +347,6 @@ CH_handler_solve (struct CH_HandlerContext *hc, unsigned int http_status; enum TALER_ErrorCode ec; - /* Only give up if *no* option remains: a /challenge retransmission - resets auth_attempts_left to 3, so as long as the user may still - request another TAN transmission they can still succeed. */ - if ( (0 == bc->addr_left) && - (0 == bc->auth_attempts_left) && - (0 == bc->pin_transmissions_left) ) - { - /* Terminal: no address change, no retransmission and no guess - remains. Reported with the same body as every other /solve - failure (all counters are zero) so that a client only ever has - to parse one shape; the error code tells the cases apart. */ - GNUNET_log (GNUNET_ERROR_TYPE_INFO, - "Client exhausted all chances to satisfy challenge\n"); - http_status = MHD_HTTP_TOO_MANY_REQUESTS; - ec = TALER_EC_CHALLENGER_TOO_MANY_ATTEMPTS; - } /* Distinguish the three ways in which a /solve can fail. Only the last one is actually about the TAN that was submitted; reporting the other two as "the TAN code provided is incorrect" misleads @@ -355,7 +354,7 @@ CH_handler_solve (struct CH_HandlerContext *hc, tell "provide your address first" from "wait or ask for a new TAN" from "that TAN was wrong" without parsing the human-readable hint. */ - else if (no_challenge) + if (no_challenge) { /* No TAN was ever transmitted for this validation, so there is nothing to check; the user must POST /challenge first. */ @@ -367,8 +366,8 @@ CH_handler_solve (struct CH_HandlerContext *hc, else if (exhausted) { /* The TAN was not even looked at: no attempts left for it. The - user may still request a retransmission (otherwise we would - have taken the terminal branch above). */ + user may still request a retransmission (otherwise the + validation would have failed above). */ GNUNET_log (GNUNET_ERROR_TYPE_INFO, "No attempts left to check the TAN\n"); http_status = MHD_HTTP_TOO_MANY_REQUESTS; diff --git a/src/challenger/test-challenger-exhaustion.sh b/src/challenger/test-challenger-exhaustion.sh @@ -28,6 +28,14 @@ # #TALER_EC_CHALLENGER_NO_CHALLENGE_TRANSMITTED, and 'exhausted' is set on # the response that consumes the last attempt rather than on the one after # it. +# +# Also a regression test for bug 11740: once the user is out of address +# changes, TAN transmissions and guesses, the validation has failed for +# good. That is reported as 410 #TALER_EC_CHALLENGER_VALIDATION_FAILED +# (not as a 429, which would tell the user agent to try again later) +# together with an RFC 6749 'access_denied' error redirect to the client, +# so that the client learns about the failure. This must also hold for +# a read-only address, which the user can never change. set -eu @@ -80,6 +88,7 @@ BURL="http://localhost:9967" REDIRECT_URI="http://client.example.com/" CLIENT_SECRET="secret-token:secret" CLIENT_ID=1 +STATE="the-client-state" echo -n "Initialize challenger database ..." challenger-dbinit -r -c "${CONF}" &> dbinit.log @@ -109,10 +118,19 @@ fi echo " OK" # Start a validation and get it into the 'address submitted' state. +# $1, if given, is the address the client fixes at /setup. function new_validation() { + local setup_body="" + local setup_type=() + if [ $# -gt 0 ] + then + setup_body="$1" + setup_type=(-H "Content-Type: application/json") + fi STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \ -H "Authorization: Bearer ${CLIENT_SECRET}" \ - -d '' \ + "${setup_type[@]}" \ + -d "${setup_body}" \ -w "%{http_code}" -s -o $LAST_RESPONSE) if [ "$STATUS" != "200" ] then @@ -125,6 +143,7 @@ function new_validation() { --data-urlencode "response_type=code" \ --data-urlencode "client_id=${CLIENT_ID}" \ --data-urlencode "redirect_uri=${REDIRECT_URI}" \ + --data-urlencode "state=${STATE}" \ -w "%{http_code}" -s -o $LAST_RESPONSE) if [ "$STATUS" != "200" ] then @@ -322,8 +341,8 @@ done # Spend what remains of the TAN transmission budget. burn_guesses() cannot # be reused for the last round: once addresses, transmissions and guesses # are all spent, the response that consumes the final guess is already the -# terminal one, so it is a 429 rather than the 403 burn_guesses() expects. -while true +# terminal one, so it is a 410 rather than the 403 burn_guesses() expects. +while [ "$STATUS" != "410" ] do TRANSMISSIONS_LEFT=$(jq -r .pin_transmissions_left < "$LAST_RESPONSE") if [ "${TRANSMISSIONS_LEFT}" -le 0 ] @@ -340,31 +359,124 @@ do done echo " OK" -# Since v8 this uses the same InvalidPinResponse shape as every other -# /solve failure, so that a client only ever parses one body. -echo -n "Terminal exhaustion uses the InvalidPinResponse shape ..." +# check_failed_json $1=endpoint: the last response must be the 410 +# reporting the permanent failure, with the OAuth2 error redirect to the +# client that the user agent has to follow. +function check_failed_json() { + CODE=$(jq -r .code < "$LAST_RESPONSE") + if [ "$STATUS" != "410" ] || [ "$CODE" != "9772" ] + then + exit_fail "$1: expected 410 with code 9772 (VALIDATION_FAILED) once nothing is left. Got: $STATUS / $CODE" $(cat $LAST_RESPONSE) + fi + check_failed_url "$1" "$(jq -r .redirect_url < "$LAST_RESPONSE")" +} + +# check_failed_url $1=endpoint $2=url: $2 must be the RFC 6749 4.1.2.1 +# error response to the client, carrying the client's state. +function check_failed_url() { + case "$2" in + "${REDIRECT_URI}?error=access_denied&"*) + ;; + *) + exit_fail "$1: expected a redirect to ${REDIRECT_URI} with error=access_denied. Got: $2" + ;; + esac + case "$2" in + *"&state=${STATE}") + ;; + *) + exit_fail "$1: expected the error redirect to carry state=${STATE}. Got: $2" + ;; + esac + case "$2" in + *"code="*) + exit_fail "$1: the error redirect must not carry an authorization code. Got: $2" + ;; + esac +} + +# The response that consumed the last guess of the last TAN (in the loop +# above) already reported the failure; so does every later one. +echo -n "Terminal exhaustion is a permanent failure ..." solve "11111111" -CODE=$(jq -r .code < "$LAST_RESPONSE") -if [ "$STATUS" != "429" ] || [ "$CODE" != "9757" ] -then - exit_fail "/solve: expected 429 with code 9757 (TOO_MANY_ATTEMPTS) once nothing is left. Got: $STATUS / $CODE" $(cat $LAST_RESPONSE) -fi -if [ "$(jq -r .type < "$LAST_RESPONSE")" != "pending" ] +check_failed_json "/solve" +echo " OK" + +echo -n "HTML user agents are redirected to the client ..." +STATUS=$(curl "${BURL}/solve/${NONCE}" \ + -X POST \ + -H "Accept: text/html" \ + --data-urlencode "pin=11111111" \ + -w "%{http_code}" -s -o /dev/null \ + -D "$LAST_RESPONSE") +if [ "$STATUS" != "302" ] then - exit_fail "/solve: terminal 429 must use the InvalidPinResponse shape. Got: $(cat $LAST_RESPONSE)" + exit_fail "/solve: expected 302 for an HTML user agent. Got: $STATUS" fi -for field in addresses_left pin_transmissions_left auth_attempts_left +LOCATION=$(grep -i "^location:" "$LAST_RESPONSE" | cut -d' ' -f2- | tr -d '\r') +check_failed_url "/solve" "${LOCATION}" +echo " OK" + +echo -n "/challenge reports the failure ..." +STATUS=$(curl "${BURL}/challenge/${NONCE}" \ + -X POST \ + -H "Accept: application/json" \ + --data-urlencode "filename=${FILENAME}" \ + --data-urlencode "note=three" \ + -w "%{http_code}" -s -o $LAST_RESPONSE) +check_failed_json "/challenge" +echo " OK" + +echo -n "/authorize reports the failure to a returning user ..." +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=${STATE}" \ + -w "%{http_code}" -s -o $LAST_RESPONSE) +check_failed_json "/authorize" +echo " OK" + +# A client such as the Taler exchange usually fixes the address at /setup +# and marks it read-only, so the user can never change it. The validation +# must then fail once the transmissions and guesses are spent, although +# address_attempts_left was never touched. +echo -n "Driving a validation with a read-only address to failure ..." +new_validation '{"filename":"'"${FILENAME}"'","read_only":true}' +while true do - if [ "$(jq -r ".${field}" < "$LAST_RESPONSE")" != "0" ] + rm -f "${FILENAME}" + 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 "/solve: expected ${field}=0 on the terminal 429. Got: $(cat $LAST_RESPONSE)" + exit_fail "/challenge: expected 200 OK. Got: $STATUS" $(cat $LAST_RESPONSE) fi + TRANSMITTED=$(jq -r .transmitted < "$LAST_RESPONSE") + RETRANSMISSION_TIME=$(jq -r .retransmission_time.t_s < "$LAST_RESPONSE") + expect_transmitted "true" "a TAN for the read-only address" + TAN=$(awk '{print $5}' < "${FILENAME}") + for i in 1 2 3 + do + solve "$(( (10#${TAN} + i) % 100000000 ))" + done + if [ "$STATUS" = "410" ] + then + break + fi + TRANSMISSIONS_LEFT=$(jq -r .pin_transmissions_left < "$LAST_RESPONSE") + if [ "${TRANSMISSIONS_LEFT}" -le 0 ] + then + exit_fail "/solve: read-only validation out of transmissions and guesses, but not failed. Got: $STATUS $(cat $LAST_RESPONSE)" + fi + await_retransmission done -if [ "$(jq -r .exhausted < "$LAST_RESPONSE")" != "true" ] || \ - [ "$(jq -r .no_challenge < "$LAST_RESPONSE")" != "false" ] -then - exit_fail "/solve: expected exhausted=true and no_challenge=false. Got: $(cat $LAST_RESPONSE)" -fi +check_failed_json "/solve" echo " OK" exit 0 diff --git a/src/challengerdb/challenger-0006.sql b/src/challengerdb/challenger-0006.sql @@ -0,0 +1,41 @@ +-- +-- 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-0006', NULL, NULL); + +SET search_path TO challenger; + +ALTER TABLE validations + ADD COLUMN failed BOOLEAN NOT NULL DEFAULT FALSE; +COMMENT ON COLUMN validations.failed + IS 'TRUE once the user exhausted every address change, TAN transmission and TAN attempt; the validation can then never succeed and is reported to the client as access_denied. Final: nothing resets it'; + +-- Validations that are already out of options. An address marked +-- 'read_only' by the client cannot be changed, whatever +-- address_attempts_left says. +UPDATE validations + SET failed=TRUE + WHERE auth_attempts_left=0 + AND pin_transmissions_left=0 + AND ( (address_attempts_left=0) + OR COALESCE(address::JSONB->'read_only' = 'true'::JSONB, FALSE) ); + + +COMMIT; diff --git a/src/challengerdb/do_challenge_address.c b/src/challengerdb/do_challenge_address.c @@ -40,7 +40,8 @@ CHALLENGERDB_do_challenge_address ( bool *pin_transmit, char **client_redirect_uri, bool *address_refused, - bool *solved) + bool *solved, + bool *failed) { struct GNUNET_TIME_Absolute now = GNUNET_TIME_absolute_get (); @@ -98,6 +99,8 @@ CHALLENGERDB_do_challenge_address ( address_refused), GNUNET_PQ_result_spec_bool ("solved", solved), + GNUNET_PQ_result_spec_bool ("failed", + failed), GNUNET_PQ_result_spec_end }; enum GNUNET_DB_QueryStatus qs; @@ -117,6 +120,7 @@ CHALLENGERDB_do_challenge_address ( ",out_client_redirect_uri AS client_redirect_uri" ",out_address_refused AS address_refused" ",out_solved AS solved" + ",out_failed AS failed" " FROM challenger_do_challenge_set_address_and_pin" " ($1,$2,$3,$4,$5);"); qs = GNUNET_PQ_eval_prepared_singleton_select (ctx->conn, @@ -160,6 +164,10 @@ CHALLENGERDB_do_challenge_address_confirm_pin ( " AND pending_pin IS NOT NULL" /* never resurrect an already solved validation */ " AND auth_attempts_left >= 0" + /* nor a failed one, which may already have been reported to + the client; this only matters if the user spent their last + guess while the last TAN was still in transit */ + " AND NOT failed" " RETURNING auth_attempts_left;"); return GNUNET_PQ_eval_prepared_singleton_select ( ctx->conn, diff --git a/src/challengerdb/do_challenge_address.sql b/src/challengerdb/do_challenge_address.sql @@ -37,7 +37,10 @@ CREATE FUNCTION challenger_do_challenge_set_address_and_pin ( OUT out_pin_transmissions_left INT4, OUT out_client_redirect_uri TEXT, OUT out_address_refused BOOLEAN, - OUT out_solved BOOLEAN) + OUT out_solved BOOLEAN, + -- TRUE if the validation failed permanently: no address change, TAN + -- transmission or TAN attempt is left. + OUT out_failed BOOLEAN) LANGUAGE plpgsql AS $$ DECLARE @@ -57,6 +60,7 @@ SELECT address ,pending_pin ,auth_attempts_left ,client_state + ,failed INTO my_status FROM validations WHERE nonce=in_nonce @@ -74,10 +78,12 @@ THEN out_client_redirect_uri=NULL; out_address_refused=TRUE; out_solved=FALSE; + out_failed=FALSE; out_state=NULL; RETURN; END IF; out_not_found=FALSE; +out_failed=FALSE; out_last_tx_time=my_status.last_tx_time; out_last_pin=my_status.last_pin; out_pin_transmit=FALSE; @@ -95,6 +101,31 @@ THEN END IF; out_solved=FALSE; +-- Once out of options, nothing this call could do helps: an address change +-- is refused (address_attempts_left is 0, or the C layer refused to change +-- a read-only address before calling us) and no TAN may be transmitted. +-- A validation can get here without a /solve, e.g. if the helper failed on +-- the last transmission after the guesses on the previous TAN were spent. +IF ( my_status.failed OR + ( (0 = my_status.auth_attempts_left) AND + (0 = my_status.pin_transmissions_left) AND + ( (0 = my_status.address_attempts_left) OR + COALESCE (my_status.address::JSONB->'read_only' + = 'true'::JSONB, FALSE) ) ) ) +THEN + IF NOT my_status.failed + THEN + UPDATE validations + SET failed=TRUE + WHERE nonce=in_nonce; + END IF; + out_failed=TRUE; + out_address_refused=TRUE; + out_last_pin=NULL; + out_auth_attempts_left=0; + RETURN; +END IF; + -- Two addresses are the same address if they are the same JSON *value*. -- Comparing the raw text instead would make a purely cosmetic difference -- -- a different field order, redundant whitespace -- count as a different diff --git a/src/challengerdb/do_solve_challenge.c b/src/challengerdb/do_solve_challenge.c @@ -34,6 +34,7 @@ CHALLENGERDB_do_solve_challenge ( bool *solved, bool *exhausted, bool *no_challenge, + bool *failed, char **state, uint32_t *addr_left, uint32_t *auth_attempts_left, @@ -58,6 +59,8 @@ CHALLENGERDB_do_solve_challenge ( exhausted), GNUNET_PQ_result_spec_bool ("no_challenge", no_challenge), + GNUNET_PQ_result_spec_bool ("failed", + failed), GNUNET_PQ_result_spec_uint32 ("address_attempts_left", addr_left), GNUNET_PQ_result_spec_uint32 ("auth_attempts_left", @@ -84,6 +87,7 @@ CHALLENGERDB_do_solve_challenge ( ",out_solved AS solved" ",out_exhausted AS exhausted" ",out_no_challenge AS no_challenge" + ",out_failed AS failed" ",out_state AS state" ",out_address_attempts_left AS address_attempts_left" ",out_auth_attempts_left AS auth_attempts_left" diff --git a/src/challengerdb/do_solve_challenge.sql b/src/challengerdb/do_solve_challenge.sql @@ -21,6 +21,9 @@ CREATE FUNCTION challenger_do_validate_and_solve_pin ( IN in_now INT8, OUT out_not_found BOOLEAN, OUT out_exhausted BOOLEAN, -- set to TRUE if attempts were already exhausted + -- TRUE if the validation failed permanently: no address change, TAN + -- transmission or TAN attempt is left, now or from an earlier call. + OUT out_failed BOOLEAN, OUT out_no_challenge BOOLEAN, OUT out_solved BOOLEAN, OUT out_state TEXT, @@ -32,10 +35,13 @@ LANGUAGE plpgsql AS $$ DECLARE my_status RECORD; + my_fixed_address BOOLEAN; BEGIN SELECT auth_attempts_left ,address_attempts_left + ,address + ,failed ,pin_transmissions_left ,last_pin ,client_redirect_uri @@ -51,6 +57,7 @@ THEN out_not_found=TRUE; out_no_challenge=TRUE; out_exhausted=FALSE; + out_failed=FALSE; out_solved=FALSE; out_address_attempts_left=0; out_auth_attempts_left=0; @@ -64,6 +71,37 @@ out_address_attempts_left=my_status.address_attempts_left; out_pin_transmissions_left=my_status.pin_transmissions_left; out_client_redirect_uri=my_status.client_redirect_uri; out_state=my_status.client_state; +out_failed=FALSE; + +-- The user cannot move to another address if they used up their address +-- changes or if the client marked the address as read-only. +my_fixed_address = ( (0 = my_status.address_attempts_left) OR + COALESCE (my_status.address::JSONB->'read_only' + = 'true'::JSONB, FALSE) ); + +-- Checked before 'last_pin', as a validation where every transmission +-- failed has no TAN but has failed nevertheless. A validation can end up +-- out of options without a /solve, e.g. if the helper failed on the last +-- transmission after the guesses on the previous TAN were spent; hence +-- the counters are checked here, too, and not just the flag. +IF ( my_status.failed OR + ( (0 = my_status.auth_attempts_left) AND + (0 = my_status.pin_transmissions_left) AND + my_fixed_address ) ) +THEN + IF NOT my_status.failed + THEN + UPDATE validations + SET failed=TRUE + WHERE nonce=in_nonce; + END IF; + out_failed=TRUE; + out_solved=FALSE; + out_exhausted=TRUE; + out_no_challenge=FALSE; + out_auth_attempts_left=0; + RETURN; +END IF; IF (my_status.last_pin IS NULL) THEN @@ -96,12 +134,18 @@ out_solved = (my_status.last_pin = in_new_pin); IF NOT out_solved THEN out_auth_attempts_left=my_status.auth_attempts_left-1; + -- That was the last chance if no retransmission or address change can + -- bring a fresh TAN. + out_failed = ( (0 = out_auth_attempts_left) AND + (0 = my_status.pin_transmissions_left) AND + my_fixed_address ); ELSE out_auth_attempts_left=-1; -- solved: no more attempts END IF; UPDATE validations SET auth_attempts_left=out_auth_attempts_left + ,failed=out_failed WHERE nonce=in_nonce; RETURN; diff --git a/src/challengerdb/meson.build b/src/challengerdb/meson.build @@ -21,6 +21,7 @@ generated_sql = [ ['challenger-0003.sql', ['challenger-0003.sql']], ['challenger-0004.sql', ['challenger-0004.sql']], ['challenger-0005.sql', ['challenger-0005.sql']], + ['challenger-0006.sql', ['challenger-0006.sql']], ] foreach g : generated_sql diff --git a/src/challengerdb/test_challenger_db.c b/src/challengerdb/test_challenger_db.c @@ -34,6 +34,7 @@ #include "challenger-database/do_insert_token.h" #include "challenger-database/get_token.h" #include "challenger-database/get_validation_pkce.h" +#include "challenger-database/update_validation.h" #include "challenger_util.h" #include "pg_helper.h" #include "challenger-database/delete_client.h" @@ -122,6 +123,7 @@ challenge_validation (struct CHALLENGER_ValidationNonceP *nonce, bool pin_transmit; bool address_refused; bool solved; + bool failed; enum GNUNET_DB_QueryStatus qs; GNUNET_CRYPTO_random_block (nonce, @@ -154,7 +156,8 @@ challenge_validation (struct CHALLENGER_ValidationNonceP *nonce, &pin_transmit, &redirect_uri, &address_refused, - &solved); + &solved, + &failed); json_decref (address); /* The TAN is only pending until its transmission is confirmed; this stands in for a successful AUTH_COMMAND run. */ @@ -171,7 +174,8 @@ challenge_validation (struct CHALLENGER_ValidationNonceP *nonce, if ( (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs) || (! pin_transmit) || address_refused || - solved) + solved || + failed) { GNUNET_break (0); return GNUNET_SYSERR; @@ -199,6 +203,7 @@ solve_validation (const struct CHALLENGER_ValidationNonceP *nonce, bool solved; bool exhausted; bool no_challenge; + bool failed; enum GNUNET_DB_QueryStatus qs; qs = CHALLENGERDB_do_solve_challenge (pg, @@ -207,6 +212,7 @@ solve_validation (const struct CHALLENGER_ValidationNonceP *nonce, &solved, &exhausted, &no_challenge, + &failed, &state, &addr_left, &auth_attempts_left, @@ -217,7 +223,8 @@ solve_validation (const struct CHALLENGER_ValidationNonceP *nonce, if ( (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs) || (! solved) || exhausted || - no_challenge) + no_challenge || + failed) { GNUNET_break (0); return GNUNET_SYSERR; @@ -308,6 +315,354 @@ test_unsolved_not_redeemable (void) /** + * Create a fresh validation for #client_id whose address was set by the + * client and marked read-only, so the user cannot change it. + * + * @param[out] nonce set to the nonce identifying the new validation + * @return #GNUNET_OK on success + */ +static enum GNUNET_GenericReturnValue +setup_read_only_validation (struct CHALLENGER_ValidationNonceP *nonce) +{ + struct GNUNET_TIME_Absolute expiration + = GNUNET_TIME_relative_to_absolute (GNUNET_TIME_UNIT_HOURS); + json_t *address; + enum GNUNET_DB_QueryStatus qs; + + GNUNET_CRYPTO_random_block (nonce, + sizeof (*nonce)); + address = json_pack ("{s:s, s:b}", + "filename", + "test-challenger-db.txt", + "read_only", + true); + GNUNET_assert (NULL != address); + qs = CHALLENGERDB_do_insert_validation (pg, + client_id, + CLIENT_SECRET, + nonce, + expiration, + address); + json_decref (address); + if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + return GNUNET_OK; +} + + +/** + * Post the read-only address of the validation under @a nonce to + * ``/challenge`` again, as the user would to get a (new) TAN. + * + * @param nonce validation to use + * @param confirm true to confirm the transmission of a new TAN, false to + * act as if the transmission helper failed + * @param[out] pin set to the TAN that was transmitted, if any + * @param[out] pin_transmit set to true if a TAN was to be transmitted + * @param[out] failed set to true if the validation failed permanently + * @return #GNUNET_OK on success + */ +static enum GNUNET_GenericReturnValue +challenge_read_only (const struct CHALLENGER_ValidationNonceP *nonce, + bool confirm, + uint32_t *pin, + bool *pin_transmit, + bool *failed) +{ + json_t *address; + char *state = NULL; + char *redirect_uri = NULL; + struct GNUNET_TIME_Absolute last_tx_time; + uint32_t auth_attempts_left; + uint32_t pin_transmissions_left; + bool address_refused; + bool solved; + enum GNUNET_DB_QueryStatus qs; + + address = json_pack ("{s:s, s:b}", + "filename", + "test-challenger-db.txt", + "read_only", + true); + GNUNET_assert (NULL != address); + *pin = GNUNET_CRYPTO_random_u32 (100000000); + qs = CHALLENGERDB_do_challenge_address (pg, + nonce, + address, + GNUNET_TIME_UNIT_ZERO, + pin, + &state, + &last_tx_time, + &auth_attempts_left, + &pin_transmissions_left, + pin_transmit, + &redirect_uri, + &address_refused, + &solved, + failed); + json_decref (address); + GNUNET_free (state); + GNUNET_free (redirect_uri); + if ( (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs) || + solved) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + if (confirm && *pin_transmit) + { + 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; + } + } + return GNUNET_OK; +} + + +/** + * Enter the wrong TAN @a pin for the validation under @a nonce. + * + * @param nonce validation to use + * @param pin (wrong) TAN to enter + * @param[out] failed set to true if the validation failed permanently + * @return #GNUNET_OK on success + */ +static enum GNUNET_GenericReturnValue +guess_wrong (const struct CHALLENGER_ValidationNonceP *nonce, + uint32_t pin, + bool *failed) +{ + char *state = NULL; + char *redirect_uri = NULL; + uint32_t addr_left; + uint32_t auth_attempts_left; + uint32_t pin_transmissions_left; + bool solved; + bool exhausted; + bool no_challenge; + enum GNUNET_DB_QueryStatus qs; + + qs = CHALLENGERDB_do_solve_challenge (pg, + nonce, + pin, + &solved, + &exhausted, + &no_challenge, + failed, + &state, + &addr_left, + &auth_attempts_left, + &pin_transmissions_left, + &redirect_uri); + GNUNET_free (state); + GNUNET_free (redirect_uri); + if ( (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != qs) || + solved) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + return GNUNET_OK; +} + + +/** + * Test that a validation with a read-only address fails once all TAN + * transmissions and guesses are spent. The user cannot change a + * read-only address, so ``address_attempts_left`` (still at its initial + * value) must not keep the validation alive; the guess that uses up the + * last chance already reports the failure, and a failed validation stays + * failed. + * + * @return #GNUNET_OK on success + */ +static enum GNUNET_GenericReturnValue +test_read_only_exhaustion_fails (void) +{ + struct CHALLENGER_ValidationNonceP nonce; + uint32_t pin; + bool pin_transmit; + bool failed; + + if (GNUNET_OK != + setup_read_only_validation (&nonce)) + return GNUNET_SYSERR; + for (unsigned int tx = 0; tx < 3; tx++) + { + if (GNUNET_OK != + challenge_read_only (&nonce, + true, + &pin, + &pin_transmit, + &failed)) + return GNUNET_SYSERR; + if ( (! pin_transmit) || + failed) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + for (unsigned int guess = 0; guess < 3; guess++) + { + if (GNUNET_OK != + guess_wrong (&nonce, + (pin + 1) % 100000000, + &failed)) + return GNUNET_SYSERR; + if (failed != ( (2 == tx) && (2 == guess) )) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + } + } + /* failed is final, whatever the user tries next */ + if (GNUNET_OK != + guess_wrong (&nonce, + pin, + &failed)) + return GNUNET_SYSERR; + if (! failed) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + if (GNUNET_OK != + challenge_read_only (&nonce, + true, + &pin, + &pin_transmit, + &failed)) + return GNUNET_SYSERR; + if ( (! failed) || + pin_transmit) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + return GNUNET_OK; +} + + +/** + * Test that a validation fails if the transmission helper fails on the + * last TAN after the guesses on the previous TAN were spent: the user + * then never got anything left to try, even though no ``/solve`` spent + * the last chance. Also checks that ``/authorize`` reports the failure. + * + * @return #GNUNET_OK on success + */ +static enum GNUNET_GenericReturnValue +test_failed_last_transmission_fails (void) +{ + struct CHALLENGER_ValidationNonceP nonce; + uint32_t pin; + bool pin_transmit; + bool failed; + + if (GNUNET_OK != + setup_read_only_validation (&nonce)) + return GNUNET_SYSERR; + if (GNUNET_OK != + challenge_read_only (&nonce, + true, + &pin, + &pin_transmit, + &failed)) + return GNUNET_SYSERR; + for (unsigned int guess = 0; guess < 3; guess++) + if (GNUNET_OK != + guess_wrong (&nonce, + (pin + 1) % 100000000, + &failed)) + return GNUNET_SYSERR; + if (failed) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + /* The remaining two transmissions fail. */ + for (unsigned int tx = 0; tx < 2; tx++) + { + if (GNUNET_OK != + challenge_read_only (&nonce, + false, + &pin, + &pin_transmit, + &failed)) + return GNUNET_SYSERR; + if ( (! pin_transmit) || + failed) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + } + { + json_t *last_address; + uint32_t address_attempts_left; + uint32_t pin_transmissions_left; + uint32_t auth_attempts_left; + bool solved; + char *redirect_uri; + struct GNUNET_TIME_Absolute last_tx_time; + + if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT != + CHALLENGERDB_update_validation (pg, + &nonce, + client_id, + NULL, + "the-state", + NULL, + NULL, + 0, + &last_address, + &address_attempts_left, + &pin_transmissions_left, + &auth_attempts_left, + &solved, + &failed, + &redirect_uri, + &last_tx_time)) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + json_decref (last_address); + if ( (! failed) || + (NULL == redirect_uri) || + (0 != strcmp (redirect_uri, + CLIENT_URI)) ) + { + GNUNET_break (0); + GNUNET_free (redirect_uri); + return GNUNET_SYSERR; + } + GNUNET_free (redirect_uri); + } + if (GNUNET_OK != + guess_wrong (&nonce, + pin, + &failed)) + return GNUNET_SYSERR; + if (! failed) + { + GNUNET_break (0); + return GNUNET_SYSERR; + } + return GNUNET_OK; +} + + +/** * Test that a solved validation is redeemable, and redeemable exactly * once (the authorization code must not be replayable). * @@ -753,6 +1108,10 @@ run (void *cls) FAILIF (GNUNET_OK != test_foreign_client_not_redeemable ()); FAILIF (GNUNET_OK != + test_read_only_exhaustion_fails ()); + FAILIF (GNUNET_OK != + test_failed_last_transmission_fails ()); + FAILIF (GNUNET_OK != test_insert_client ()); FAILIF (GNUNET_OK != test_delete_client ()); diff --git a/src/challengerdb/update_validation.c b/src/challengerdb/update_validation.c @@ -43,6 +43,8 @@ CHALLENGERDB_update_validation ( uint32_t *pin_transmissions_left, uint32_t *auth_attempts_left, bool *solved, + bool *failed, + char **final_redirect_uri, struct GNUNET_TIME_Absolute *last_tx_time) { struct GNUNET_TIME_Absolute now @@ -79,12 +81,17 @@ CHALLENGERDB_update_validation ( auth_attempts_left), GNUNET_PQ_result_spec_bool ("solved", solved), + GNUNET_PQ_result_spec_bool ("failed", + failed), + GNUNET_PQ_result_spec_string ("client_redirect_uri", + final_redirect_uri), GNUNET_PQ_result_spec_absolute_time ("last_tx_time", last_tx_time), GNUNET_PQ_result_spec_end }; *last_address = NULL; + *final_redirect_uri = NULL; /* A repeated /authorize must never *weaken* an existing PKCE binding (RFC 7636): /authorize authenticates nobody (the client_id is a plain query argument) and the nonce is recoverable from an issued code, so @@ -93,7 +100,12 @@ CHALLENGERDB_update_validation ( like for client_redirect_uri above. code_challenge_method must move with the challenge it describes: the column is NOT NULL DEFAULT 0, so writing it unconditionally would leave a retained challenge with the - method of the request that tried to drop it. */ + method of the request that tried to drop it. + 'failed' is updated here, too, as a validation can run out of options + without a /solve or /challenge noticing, e.g. if the transmission + helper failed on the last TAN; see do_challenge_address.sql for the + same condition. The WHERE clause guarantees that client_redirect_uri + is not NULL after the update. */ PREPARE (ctx, "update_validation", "UPDATE validations SET" @@ -106,6 +118,12 @@ CHALLENGERDB_update_validation ( " THEN code_challenge_method" " ELSE $7" " END" + " ,failed=failed" + " OR ( (auth_attempts_left=0)" + " AND (pin_transmissions_left=0)" + " AND ( (address_attempts_left=0)" + " OR COALESCE(address::JSONB->'read_only'" + " = 'true'::JSONB, FALSE) ) )" " WHERE nonce=$1" " AND client_serial_id=$2" " AND expiration_time > $8" @@ -118,6 +136,8 @@ CHALLENGERDB_update_validation ( " ,pin_transmissions_left" " ,GREATEST(0, auth_attempts_left) AS auth_attempts_left" " ,auth_attempts_left = -1 AS solved" + " ,failed" + " ,client_redirect_uri" " ,last_tx_time;"); return GNUNET_PQ_eval_prepared_singleton_select (ctx->conn, "update_validation", diff --git a/src/include/challenger-database/do_challenge_address.h b/src/include/challenger-database/do_challenge_address.h @@ -55,9 +55,12 @@ * @param[out] client_redirect_uri redirection URI of the client (for reporting failures) * @param[out] address_refused set to true if the address was refused (address change attempts exhausted) * @param[out] solved set to true if the challenge is already solved + * @param[out] failed set to true if the validation failed permanently: no + * address change, TAN transmission or TAN attempt is left, so it can + * never succeed anymore * @return transaction status: * #GNUNET_DB_STATUS_SUCCESS_ONE_RESULT if the validation @a nonce exists - * (inspect @a address_refused / @a solved / @a pin_transmit for the actual + * (inspect @a failed / @a address_refused / @a solved / @a pin_transmit for the actual * outcome) * #GNUNET_DB_STATUS_SUCCESS_NO_RESULTS if the validation @a nonce is unknown * #GNUNET_DB_STATUS_HARD_ERROR on failure @@ -76,7 +79,8 @@ CHALLENGERDB_do_challenge_address ( bool *pin_transmit, char **client_redirect_uri, bool *address_refused, - bool *solved); + bool *solved, + bool *failed); /** @@ -95,7 +99,7 @@ CHALLENGERDB_do_challenge_address ( * @return transaction status: * #GNUNET_DB_STATUS_SUCCESS_ONE_RESULT if the TAN was promoted * #GNUNET_DB_STATUS_SUCCESS_NO_RESULTS if there was no pending TAN, or the - * validation is unknown or already solved + * validation is unknown, already solved or failed * #GNUNET_DB_STATUS_HARD_ERROR on failure */ enum GNUNET_DB_QueryStatus diff --git a/src/include/challenger-database/do_solve_challenge.h b/src/include/challenger-database/do_solve_challenge.h @@ -36,6 +36,9 @@ * @param[out] solved set to true if the TAN was correct * @param[out] exhausted set to true if the number of attempts to enter the correct TAN was exhausted before this call and @a new_pin was not evaluated * @param[out] no_challenge set to true if we never even issued a challenge + * @param[out] failed set to true if the validation failed permanently: no + * address change, TAN transmission or TAN attempt is left, so it can + * never succeed anymore * @param[out] state set to client's OAuth2 state if available * @param[out] addr_left set to number of address changes remaining * @param[out] auth_attempts_left set to number of authentication attempts @@ -57,6 +60,7 @@ CHALLENGERDB_do_solve_challenge ( bool *solved, bool *exhausted, bool *no_challenge, + bool *failed, char **state, uint32_t *addr_left, uint32_t *auth_attempts_left, diff --git a/src/include/challenger-database/update_validation.h b/src/include/challenger-database/update_validation.h @@ -52,6 +52,11 @@ * @param[out] pin_transmissions_left set to number of times the TAN can still be re-requested * @param[out] auth_attempts_left set to number of authentication attempts remaining * @param[out] solved set to true if the challenge is already solved + * @param[out] failed set to true if the validation failed permanently: no + * address change, TAN transmission or TAN attempt is left + * @param[out] final_redirect_uri set to the redirect URI now on file, which + * is the registered one if @a client_redirect_uri is NULL; the + * caller must free it * @param[out] last_tx_time set to the last time when we (presumably) send a TAN to @a last_address; 0 if never sent * @return transaction status: * #GNUNET_DB_STATUS_SUCCESS_ONE_RESULT if the validation exists and the @@ -76,6 +81,8 @@ CHALLENGERDB_update_validation ( uint32_t *pin_transmissions_left, uint32_t *auth_attempts_left, bool *solved, + bool *failed, + char **final_redirect_uri, struct GNUNET_TIME_Absolute *last_tx_time);