commit 8260a2cb4a8953cedb11b326a1df5dacb492b7ea
parent 8c52d25123a2551794b769d5afc6e08c842b0fb7
Author: Christian Grothoff <christian@grothoff.org>
Date: Sat, 10 Oct 2026 16:16:28 +0200
only accept challenges after challenge was actually sent
Diffstat:
11 files changed, 105 insertions(+), 33 deletions(-)
diff --git a/src/backend/taler-merchant-httpd_get-management-instances-INSTANCE.c b/src/backend/taler-merchant-httpd_get-management-instances-INSTANCE.c
@@ -118,7 +118,9 @@ get_instances_ID (struct TMH_MerchantInstance *mi,
mime = MHD_lookup_connection_value (connection,
MHD_HEADER_KIND,
MHD_HTTP_HEADER_ACCEPT);
- if (NULL == mime)
+ if ( (NULL == mime) ||
+ (0 == strcmp (mime,
+ "*/*")) )
mime = "application/json";
if (0 == strcmp (mime,
"text/plain"))
diff --git a/src/backend/taler-merchant-httpd_mfa.c b/src/backend/taler-merchant-httpd_mfa.c
@@ -270,36 +270,25 @@ mfa_challenge_start (
struct TALER_MERCHANT_MFA_BodySalt salt;
struct TALER_MERCHANT_MFA_BodyHash h_body;
uint64_t challenge_serial;
- unsigned long long challenge_num;
- char *code;
GNUNET_CRYPTO_random_block (&salt,
sizeof (salt));
TALER_MERCHANT_mfa_body_hash (hc->request_body,
&salt,
&h_body);
- challenge_num = (unsigned long long)
- GNUNET_CRYPTO_random_u64 (1000 * 1000 * 100);
- /* Note: if this is changed, the code in
- taler-merchant-httpd_post-challenge-ID.c and
- taler-merchant-httpd_post-challenge-ID-confirm.c must
- possibly also be updated! */
- GNUNET_asprintf (&code,
- "%04llu-%04llu",
- challenge_num / 10000,
- challenge_num % 10000);
+ /* The challenge is stored without a code: the TAN is only generated
+ when it is sent (taler-merchant-httpd_post-challenge-ID.c), and
+ until then the challenge cannot be solved. */
qs = TALER_MERCHANTDB_insert_mfa_challenge (TMH_db,
instance_name,
op,
&h_body,
&salt,
- code,
expiration_date,
GNUNET_TIME_UNIT_ZERO_ABS,
channel,
required_address,
&challenge_serial);
- GNUNET_free (code);
switch (qs)
{
case GNUNET_DB_STATUS_HARD_ERROR:
diff --git a/src/backend/taler-merchant-httpd_post-challenge-ID-confirm.c b/src/backend/taler-merchant-httpd_post-challenge-ID-confirm.c
@@ -71,7 +71,8 @@ TMH_post_challenge_ID_confirm (const struct TMH_RequestHandler *rh,
{
/* User may have submitted the entire challenge as a single
8-digit number instead of (4-digits)-"-"-(4-digits) which
- is the new format generated by taler-merchant-httpd_mfa.c.
+ is the new format generated by
+ taler-merchant-httpd_post-challenge-ID.c.
Thus, in this case, we convert the format to the string
with the dash and use that. */
unsigned long long challenge_num;
diff --git a/src/backend/taler-merchant-httpd_post-challenge-ID.c b/src/backend/taler-merchant-httpd_post-challenge-ID.c
@@ -554,6 +554,9 @@ phase_send_challenge (struct MfaState *mfa)
challenge_num = (unsigned long long)
GNUNET_CRYPTO_random_u64 (1000 * 1000 * 100);
+ /* Note: if this is changed, the code in
+ taler-merchant-httpd_post-challenge-ID-confirm.c must
+ possibly also be updated! */
GNUNET_asprintf (&mfa->code,
"%04llu-%04llu",
challenge_num / 10000,
diff --git a/src/backenddb/do_solve_mfa_challenge.sql b/src/backenddb/do_solve_mfa_challenge.sql
@@ -35,6 +35,7 @@ BEGIN
SELECT
tc.confirmation_date
,tc.retry_counter
+ ,(tc.code IS NULL) AS not_sent
,(tc.code = in_solution) AS solved
INTO
my_rec
@@ -75,6 +76,15 @@ BEGIN
RETURN;
END IF;
+ -- No TAN was sent for this challenge yet, so there is no code that
+ -- in_solution could match. Nothing was guessed, so the retry counter
+ -- is left alone.
+ IF my_rec.not_sent
+ THEN
+ out_solved = FALSE;
+ RETURN;
+ END IF;
+
IF (0 = out_retry_counter)
THEN
out_solved = FALSE;
@@ -102,6 +112,8 @@ COMMENT ON FUNCTION merchant_do_solve_mfa_challenge(INT8, BYTEA, TEXT, INT8)
IS 'Checks in_solution against the code of the challenge identified by'
' in_challenge_id and in_h_body, and confirms the challenge if it'
' matches. Returns out_solved=FALSE with a NULL out_retry_counter if no'
- ' such (unexpired) challenge exists. Confirming an already confirmed'
+ ' such (unexpired) challenge exists. A challenge for which no TAN was'
+ ' sent yet (code is NULL) is never solved and costs no attempt.'
+ ' Confirming an already confirmed'
' challenge is idempotent: out_solved=TRUE is returned regardless of'
' in_solution and the retry counter is not decremented.';
diff --git a/src/backenddb/insert_mfa_challenge.c b/src/backenddb/insert_mfa_challenge.c
@@ -33,7 +33,6 @@ TALER_MERCHANTDB_insert_mfa_challenge (
enum TALER_MERCHANT_MFA_CriticalOperation op,
const struct TALER_MERCHANT_MFA_BodyHash *h_body,
const struct TALER_MERCHANT_MFA_BodySalt *salt,
- const char *code,
struct GNUNET_TIME_Absolute expiration_date,
struct GNUNET_TIME_Absolute retransmission_date,
enum TALER_MERCHANT_MFA_Channel tan_channel,
@@ -47,13 +46,12 @@ TALER_MERCHANTDB_insert_mfa_challenge (
GNUNET_PQ_query_param_auto_from_type (h_body),
GNUNET_PQ_query_param_auto_from_type (salt),
GNUNET_PQ_query_param_string (op_str),
- GNUNET_PQ_query_param_string (code),
- GNUNET_PQ_query_param_absolute_time (&now), /* $5 */
- GNUNET_PQ_query_param_absolute_time (&expiration_date),
+ GNUNET_PQ_query_param_absolute_time (&now),
+ GNUNET_PQ_query_param_absolute_time (&expiration_date), /* $5 */
GNUNET_PQ_query_param_absolute_time (&retransmission_date),
GNUNET_PQ_query_param_string (channel_str),
- GNUNET_PQ_query_param_string (required_address), /* $9 */
- GNUNET_PQ_query_param_string (instance_name),
+ GNUNET_PQ_query_param_string (required_address),
+ GNUNET_PQ_query_param_string (instance_name), /* $9 */
GNUNET_PQ_query_param_end
};
struct GNUNET_PQ_ResultSpec rs[] = {
@@ -68,7 +66,7 @@ TALER_MERCHANTDB_insert_mfa_challenge (
" (h_body"
" ,salt"
" ,op"
- " ,code"
+ " ,code" /* NULL until a TAN is sent */
" ,creation_date"
" ,expiration_date"
" ,retransmission_date"
@@ -77,7 +75,7 @@ TALER_MERCHANTDB_insert_mfa_challenge (
" ,required_address"
" ,instance_name)"
" VALUES"
- " ($1, $2, $3, $4, $5, $6, $7, 3, $8, $9, $10)"
+ " ($1, $2, $3, NULL, $4, $5, $6, 3, $7, $8, $9)"
" RETURNING challenge_id;");
return GNUNET_PQ_eval_prepared_singleton_select (pg->conn,
"insert_mfa_challenge",
diff --git a/src/backenddb/sql-schema/merchant-0049.sql b/src/backenddb/sql-schema/merchant-0049.sql
@@ -0,0 +1,40 @@
+--
+-- This file is part of TALER
+-- Copyright (C) 2026 Taler Systems SA
+--
+-- TALER 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.
+--
+-- TALER 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
+-- TALER; see the file COPYING. If not, see <http://www.gnu.org/licenses/>
+--
+
+-- @file merchant-0049.sql
+-- @brief An MFA challenge has no code until a TAN was actually sent.
+
+BEGIN;
+
+SELECT _v.register_patch('merchant-0049', NULL, NULL);
+
+SET search_path TO merchant;
+
+ALTER TABLE tan_challenges
+ ALTER COLUMN code DROP NOT NULL;
+
+COMMENT ON COLUMN tan_challenges.code
+ IS 'The TAN that was sent to the user; NULL while no TAN was sent, and then the challenge cannot be solved';
+
+-- Challenges created before this migration got a code that was stored
+-- but not sent. Those that were never transmitted (retransmission_date
+-- is still 0) must not be solvable with it.
+UPDATE tan_challenges
+ SET code = NULL
+ WHERE retransmission_date = 0
+ AND confirmation_date IS NULL;
+
+COMMIT;
diff --git a/src/backenddb/sql-schema/meson.build b/src/backenddb/sql-schema/meson.build
@@ -148,6 +148,7 @@ generated_sql = [
['merchant-0046.sql'],
['merchant-0047.sql'],
['merchant-0048.sql'],
+ ['merchant-0049.sql'],
]
migration_sql = []
diff --git a/src/backenddb/test_merchantdb.c b/src/backenddb/test_merchantdb.c
@@ -56,6 +56,7 @@
#include "merchant-database/insert_exchange_signing_key.h"
#include "merchant-database/insert_instance.h"
#include "merchant-database/insert_mfa_challenge.h"
+#include "merchant-database/update_mfa_challenge.h"
#include "merchant-database/do_solve_mfa_challenge.h"
#include "merchant-database/insert_order.h"
#include "merchant-database/insert_order_lock.h"
@@ -8900,16 +8901,16 @@ test_insert_kyc_failure (
uint64_t after;
TEST_RET_ON_FAIL (query_sql_num (
- "SELECT next_kyc_poll AS num FROM merchant_kyc"
- " WHERE exchange_url='https://exchange2.com/'", &before));
+ "SELECT next_kyc_poll AS num FROM merchant_kyc"
+ " WHERE exchange_url='https://exchange2.com/'", &before));
expected.called = 0;
GNUNET_assert (1 == TALER_MERCHANTDB_iterate_kyc_statuses (
pg, instance->instance.id, &account->h_wire,
exchange_url, false, &kyc_failure_check, &expected));
GNUNET_assert ((! expected.failed) && (1 == expected.called));
TEST_RET_ON_FAIL (query_sql_num (
- "SELECT next_kyc_poll AS num FROM merchant_kyc"
- " WHERE exchange_url='https://exchange2.com/'", &after));
+ "SELECT next_kyc_poll AS num FROM merchant_kyc"
+ " WHERE exchange_url='https://exchange2.com/'", &after));
GNUNET_assert (before == after);
GNUNET_assert (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT ==
TALER_MERCHANTDB_insert_kyc_failure (
@@ -11676,13 +11677,38 @@ test_mfa_challenges (void)
TALER_MERCHANT_MFA_CO_AUTH_CONFIGURATION,
&h_body,
&salt,
- "1234-5678",
GNUNET_TIME_relative_to_absolute (GNUNET_TIME_UNIT_DAYS),
GNUNET_TIME_UNIT_ZERO_ABS,
TALER_MERCHANT_MFA_CHANNEL_EMAIL,
"test@example.com",
&challenge_id),
"Insert MFA challenge failed\n");
+ /* Before a TAN was sent, no TAN solves the challenge, and trying
+ costs no attempt. */
+ TEST_COND_RET_ON_FAIL (
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT ==
+ TALER_MERCHANTDB_do_solve_mfa_challenge (pg,
+ challenge_id,
+ &h_body,
+ "1234-5678",
+ &solved,
+ &retry_counter),
+ "Solving MFA challenge failed\n");
+ TEST_COND_RET_ON_FAIL (! solved,
+ "Challenge solved before a TAN was sent\n");
+ TEST_COND_RET_ON_FAIL (3 == retry_counter,
+ "Guess before a TAN was sent cost an attempt\n");
+ /* Sending the TAN stores its code. */
+ TEST_COND_RET_ON_FAIL (
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT ==
+ TALER_MERCHANTDB_update_mfa_challenge (
+ pg,
+ challenge_id,
+ "1234-5678",
+ 3,
+ GNUNET_TIME_relative_to_absolute (GNUNET_TIME_UNIT_HOURS),
+ GNUNET_TIME_relative_to_absolute (GNUNET_TIME_UNIT_HOURS)),
+ "Update MFA challenge failed\n");
/* A challenge is only addressable with the matching body hash. */
TEST_COND_RET_ON_FAIL (
GNUNET_DB_STATUS_SUCCESS_NO_RESULTS ==
diff --git a/src/include/merchant-database/insert_mfa_challenge.h b/src/include/merchant-database/insert_mfa_challenge.h
@@ -28,6 +28,8 @@
/**
* Create new multi-factor authorization (MFA) challenge in the database.
+ * The challenge has no code and thus cannot be solved until a TAN is
+ * sent and stored with TALER_MERCHANTDB_update_mfa_challenge().
*
* @param pg database context
* @param instance_name name of the instance impacted by the challenge;
@@ -36,7 +38,6 @@
* @param op operation that triggered the MFA request
* @param h_body hash of the request body
* @param salt salt used to compute @a h_body
- * @param code challenge code sent to the user
* @param expiration_date when should the challenge expire
* @param retransmission_date when do we next allow retransmission
* of the challenge
@@ -53,7 +54,6 @@ TALER_MERCHANTDB_insert_mfa_challenge (
enum TALER_MERCHANT_MFA_CriticalOperation op,
const struct TALER_MERCHANT_MFA_BodyHash *h_body,
const struct TALER_MERCHANT_MFA_BodySalt *salt,
- const char *code,
struct GNUNET_TIME_Absolute expiration_date,
struct GNUNET_TIME_Absolute retransmission_date,
enum TALER_MERCHANT_MFA_Channel tan_channel,
diff --git a/src/testing/test_merchant_kyc.sh b/src/testing/test_merchant_kyc.sh
@@ -101,7 +101,7 @@ fi
echo " OK"
-echo -n "Check the instance exists ..."
+echo -n "Check the (admin) instance exists ..."
STATUS=$(curl -H "Content-Type: application/json" \
-X GET \
