commit 893fbfee7b0f9ef770a7b784360ad1e69e6d5f19
parent 46d3e49ff9b9784e5e95875a7db1e39a5255a9cb
Author: Christian Grothoff <christian@grothoff.org>
Date: Thu, 6 Aug 2026 15:05:28 +0200
check client_ID again during insert_token transaction to avoid super-hypothetical issues with concurrent transactions interfering
Diffstat:
4 files changed, 131 insertions(+), 10 deletions(-)
diff --git a/src/challenger/challenger-httpd_token.c b/src/challenger/challenger-httpd_token.c
@@ -714,13 +714,23 @@ CH_handler_token (struct CH_HandlerContext *hc,
{
struct CHALLENGER_AccessTokenP token;
enum GNUNET_DB_QueryStatus qs;
+ unsigned long long token_client_id;
+ char token_dummy;
+ /* bc->client_id was already validated as a number in the client-auth
+ block above; re-parse it here (it is out of scope) so the redemption
+ is restricted to validations of the authenticated client. */
+ GNUNET_assert (1 == sscanf (bc->client_id,
+ "%llu%c",
+ &token_client_id,
+ &token_dummy));
GNUNET_CRYPTO_random_block (&token,
sizeof (token));
for (unsigned int r = 0; r < MAX_RETRIES; r++)
{
qs = CHALLENGERDB_do_insert_token (CH_context,
&bc->nonce,
+ (uint64_t) token_client_id,
&token,
CH_token_expiration,
CH_validation_expiration);
diff --git a/src/challengerdb/do_insert_token.c b/src/challengerdb/do_insert_token.c
@@ -29,10 +29,13 @@
enum GNUNET_DB_QueryStatus
CHALLENGERDB_do_insert_token (struct CHALLENGERDB_PostgresContext *ctx,
const struct CHALLENGER_ValidationNonceP *nonce,
+ uint64_t client_id,
const struct CHALLENGER_AccessTokenP *token,
struct GNUNET_TIME_Relative token_expiration,
struct GNUNET_TIME_Relative address_expiration)
{
+ struct GNUNET_TIME_Absolute now
+ = GNUNET_TIME_absolute_get ();
struct GNUNET_TIME_Absolute ge
= GNUNET_TIME_relative_to_absolute (token_expiration);
struct GNUNET_TIME_Absolute ae
@@ -42,6 +45,8 @@ CHALLENGERDB_do_insert_token (struct CHALLENGERDB_PostgresContext *ctx,
GNUNET_PQ_query_param_auto_from_type (token),
GNUNET_PQ_query_param_absolute_time (&ge),
GNUNET_PQ_query_param_absolute_time (&ae),
+ GNUNET_PQ_query_param_absolute_time (&now),
+ GNUNET_PQ_query_param_uint64 (&client_id),
GNUNET_PQ_query_param_end
};
@@ -53,7 +58,10 @@ CHALLENGERDB_do_insert_token (struct CHALLENGERDB_PostgresContext *ctx,
restricts us to validations that were actually *solved*: -1 is the sentinel
challenger_do_validate_and_solve_pin() writes once the user entered the
correct PIN. The 'address IS NOT NULL' guard additionally avoids violating
- the tokens.address NOT NULL constraint. */
+ the tokens.address NOT NULL constraint. Expiration and client are checked
+ here as well and not merely by the get_validation_pkce() that preceded us:
+ the two run in separate transactions, so anything we do not repeat is a
+ TOCTOU window. */
PREPARE (ctx,
"do_insert_token",
"WITH consumed AS ("
@@ -61,6 +69,8 @@ CHALLENGERDB_do_insert_token (struct CHALLENGERDB_PostgresContext *ctx,
" WHERE nonce=$1"
" AND address IS NOT NULL"
" AND auth_attempts_left < 0"
+ " AND expiration_time > $5"
+ " AND client_serial_id=$6"
" RETURNING address"
") INSERT INTO tokens"
" (access_token"
diff --git a/src/challengerdb/test_challenger_db.c b/src/challengerdb/test_challenger_db.c
@@ -224,18 +224,21 @@ solve_validation (const struct CHALLENGER_ValidationNonceP *nonce,
/**
* Mint an access token for the validation under @a nonce.
*
+ * @param client client redeeming the authorization code
* @param nonce validation to redeem
* @param[out] token set to the (random) access token we tried to store
* @return transaction status of CHALLENGERDB_do_insert_token()
*/
static enum GNUNET_DB_QueryStatus
-insert_token (const struct CHALLENGER_ValidationNonceP *nonce,
+insert_token (uint64_t client,
+ const struct CHALLENGER_ValidationNonceP *nonce,
struct CHALLENGER_AccessTokenP *token)
{
GNUNET_CRYPTO_random_block (token,
sizeof (*token));
return CHALLENGERDB_do_insert_token (pg,
nonce,
+ client,
token,
GNUNET_TIME_UNIT_HOURS,
GNUNET_TIME_UNIT_DAYS);
@@ -288,7 +291,8 @@ test_unsolved_not_redeemable (void)
}
}
if (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS !=
- insert_token (&nonce,
+ insert_token (client_id,
+ &nonce,
&token))
{
GNUNET_break (0);
@@ -319,7 +323,8 @@ test_solved_redeemable (void)
pin)) )
return GNUNET_SYSERR;
if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT !=
- insert_token (&nonce,
+ insert_token (client_id,
+ &nonce,
&token))
{
GNUNET_break (0);
@@ -327,7 +332,8 @@ test_solved_redeemable (void)
}
/* replay: the validation was consumed, so this must not mint a token */
if (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS !=
- insert_token (&nonce,
+ insert_token (client_id,
+ &nonce,
&token))
{
GNUNET_break (0);
@@ -380,7 +386,8 @@ test_address_expiry_is_in_the_future (void)
}
now = GNUNET_TIME_absolute_get ();
if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT !=
- insert_token (&nonce,
+ insert_token (client_id,
+ &nonce,
&token))
{
GNUNET_break (0);
@@ -408,6 +415,93 @@ test_address_expiry_is_in_the_future (void)
/**
+ * Test that an expired validation is not consumable, even though the
+ * caller (``/token``) checked the expiration in an earlier, separate
+ * transaction. Everything get_validation_pkce() filters on has to be
+ * repeated by do_insert_token(), or the gap between the two statements is
+ * an unguarded TOCTOU window.
+ *
+ * @return #GNUNET_OK on success
+ */
+static enum GNUNET_GenericReturnValue
+test_expired_not_redeemable (void)
+{
+ struct GNUNET_PQ_ExecuteStatement es[] = {
+ GNUNET_PQ_make_execute ("UPDATE validations SET expiration_time=1;"),
+ GNUNET_PQ_EXECUTE_STATEMENT_END
+ };
+ struct CHALLENGER_ValidationNonceP nonce;
+ struct CHALLENGER_AccessTokenP token;
+ uint32_t pin;
+
+ if ( (GNUNET_OK !=
+ challenge_validation (&nonce,
+ &pin)) ||
+ (GNUNET_OK !=
+ solve_validation (&nonce,
+ pin)) )
+ return GNUNET_SYSERR;
+ if (GNUNET_OK !=
+ GNUNET_PQ_exec_statements (pg->conn,
+ es))
+ {
+ GNUNET_break (0);
+ return GNUNET_SYSERR;
+ }
+ if (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS !=
+ insert_token (client_id,
+ &nonce,
+ &token))
+ {
+ GNUNET_break (0);
+ return GNUNET_SYSERR;
+ }
+ return GNUNET_OK;
+}
+
+
+/**
+ * Test that a validation cannot be consumed by a client it does not
+ * belong to.
+ *
+ * @return #GNUNET_OK on success
+ */
+static enum GNUNET_GenericReturnValue
+test_foreign_client_not_redeemable (void)
+{
+ struct CHALLENGER_ValidationNonceP nonce;
+ struct CHALLENGER_AccessTokenP token;
+ uint32_t pin;
+
+ if ( (GNUNET_OK !=
+ challenge_validation (&nonce,
+ &pin)) ||
+ (GNUNET_OK !=
+ solve_validation (&nonce,
+ pin)) )
+ return GNUNET_SYSERR;
+ if (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS !=
+ insert_token (client_id + 1,
+ &nonce,
+ &token))
+ {
+ GNUNET_break (0);
+ return GNUNET_SYSERR;
+ }
+ /* ... and the rightful client still can */
+ if (GNUNET_DB_STATUS_SUCCESS_ONE_RESULT !=
+ insert_token (client_id,
+ &nonce,
+ &token))
+ {
+ GNUNET_break (0);
+ return GNUNET_SYSERR;
+ }
+ return GNUNET_OK;
+}
+
+
+/**
* Main function that will be run by the scheduler.
*
* @param cls closure with config
@@ -456,6 +550,10 @@ run (void *cls)
test_solved_redeemable ());
FAILIF (GNUNET_OK !=
test_address_expiry_is_in_the_future ());
+ FAILIF (GNUNET_OK !=
+ test_expired_not_redeemable ());
+ FAILIF (GNUNET_OK !=
+ test_foreign_client_not_redeemable ());
result = 0;
drop:
GNUNET_break (GNUNET_OK ==
diff --git a/src/include/challenger-database/do_insert_token.h b/src/include/challenger-database/do_insert_token.h
@@ -31,12 +31,13 @@
* Add access @a grant to address under @a nonce, consuming the validation in
* the same statement so that the authorization code cannot be replayed.
*
- * Only *solved* validations that carry an address are consumed; anything else
- * yields #GNUNET_DB_STATUS_SUCCESS_NO_RESULTS, which the caller maps to the
- * OAuth 2.0 ``invalid_grant`` error.
+ * Only *solved*, unexpired validations of @a client_id that carry an address
+ * are consumed; anything else yields #GNUNET_DB_STATUS_SUCCESS_NO_RESULTS,
+ * which the caller maps to the OAuth 2.0 ``invalid_grant`` error.
*
* @param cls closure
* @param nonce validation process to grant access to
+ * @param client_id serial id of the authenticated client redeeming the code
* @param grant grant token that grants access
* @param grant_expiration for how long should the grant be valid
* @param address_expiration for how long from now do we consider the address
@@ -44,13 +45,15 @@
* @return transaction status:
* #GNUNET_DB_STATUS_SUCCESS_ONE_RESULT if the token was minted
* #GNUNET_DB_STATUS_SUCCESS_NO_RESULTS if there is no solved validation
- * under @a nonce (unknown, unsolved or already redeemed)
+ * under @a nonce (unknown, unsolved, expired, of another client or
+ * already redeemed)
* #GNUNET_DB_STATUS_HARD_ERROR on failure
*/
enum GNUNET_DB_QueryStatus
CHALLENGERDB_do_insert_token (
struct CHALLENGERDB_PostgresContext *ctx,
const struct CHALLENGER_ValidationNonceP *nonce,
+ uint64_t client_id,
const struct CHALLENGER_AccessTokenP *grant,
struct GNUNET_TIME_Relative grant_expiration,
struct GNUNET_TIME_Relative address_expiration);
