commit f857c916e86f2ff4ff12f8fed51646ebb2f59082
parent cd01a7898a49f86ee470cfe6233de0a9209944a9
Author: Christian Grothoff <christian@grothoff.org>
Date: Thu, 6 Aug 2026 14:44:55 +0200
verify /token code MAC first, ensure all failures result in 400 invalid_grant to minimize information disclosure
Diffstat:
2 files changed, 57 insertions(+), 82 deletions(-)
diff --git a/src/challenger/challenger-httpd_token.c b/src/challenger/challenger-httpd_token.c
@@ -468,13 +468,18 @@ CH_handler_token (struct CH_HandlerContext *hc,
case GNUNET_DB_STATUS_SOFT_ERROR:
continue;
case GNUNET_DB_STATUS_SUCCESS_NO_RESULTS:
+ /* Unknown, expired, foreign or unsolved validation. RFC 6749 5.2
+ has exactly one error for all of these, and we must not say which
+ one it was: the nonce is recoverable from any code, so a
+ distinguishable answer would let anybody track the progress of a
+ user's validation. */
GNUNET_break_op (0);
return CH_reply_with_oauth_error (
hc->connection,
MHD_HTTP_BAD_REQUEST,
"invalid_grant",
- TALER_EC_CHALLENGER_GENERIC_VALIDATION_UNKNOWN,
- "get_validation_pkce");
+ TALER_EC_CHALLENGER_CLIENT_FORBIDDEN_BAD_CODE,
+ NULL);
case GNUNET_DB_STATUS_SUCCESS_ONE_RESULT:
break;
}
@@ -508,17 +513,61 @@ CH_handler_token (struct CH_HandlerContext *hc,
"Invalid code_challenge_method");
}
+ /* Authenticate the authorization code *before* looking at anything else
+ about the validation. CH_code_to_nonce() only parses the nonce out of
+ the code, so every check that runs before the MAC comparison answers
+ questions about a validation the caller may not hold a code for. All
+ failures below therefore collapse into one indistinguishable
+ 'invalid_grant' (RFC 6749 5.2). */
+ if (NULL == address)
+ {
+ /* Cannot happen: only solved validations are returned, and those
+ always carry an address. Answered like a bad code regardless. */
+ GNUNET_break_op (0);
+ GNUNET_free (client_scope);
+ GNUNET_free (client_secret);
+ GNUNET_free (client_redirect_uri);
+ GNUNET_free (client_state);
+ GNUNET_free (code_challenge);
+ return CH_reply_with_oauth_error (
+ hc->connection,
+ MHD_HTTP_BAD_REQUEST,
+ "invalid_grant",
+ TALER_EC_CHALLENGER_CLIENT_FORBIDDEN_BAD_CODE,
+ NULL);
+ }
+ code = CH_compute_code (&bc->nonce,
+ client_secret,
+ client_scope,
+ address,
+ client_redirect_uri);
+ json_decref (address);
+ GNUNET_free (client_scope);
+ GNUNET_free (client_secret);
+ GNUNET_free (client_redirect_uri);
+ GNUNET_free (client_state);
+ if (0 !=
+ TALER_strcmp_ct (code,
+ bc->code))
+ {
+ GNUNET_break_op (0);
+ GNUNET_free (code);
+ GNUNET_free (code_challenge);
+ return CH_reply_with_oauth_error (
+ hc->connection,
+ MHD_HTTP_BAD_REQUEST,
+ "invalid_grant",
+ TALER_EC_CHALLENGER_CLIENT_FORBIDDEN_BAD_CODE,
+ NULL);
+ }
+ GNUNET_free (code);
+
/* Verify the code_challenge if present*/
if (NULL != code_challenge)
{
if (NULL == bc->code_verifier)
{
GNUNET_break_op (0);
- json_decref (address);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
GNUNET_free (code_challenge);
return CH_reply_with_oauth_error (
hc->connection,
@@ -536,11 +585,6 @@ CH_handler_token (struct CH_HandlerContext *hc,
(cv_len > 128) )
{
GNUNET_break_op (0);
- json_decref (address);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
GNUNET_free (code_challenge);
return CH_reply_with_oauth_error (
hc->connection,
@@ -556,11 +600,6 @@ CH_handler_token (struct CH_HandlerContext *hc,
if (! TALER_url_is_reserved (c))
continue;
GNUNET_break_op (0);
- json_decref (address);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
GNUNET_free (code_challenge);
return CH_reply_with_oauth_error (
hc->connection,
@@ -587,11 +626,6 @@ CH_handler_token (struct CH_HandlerContext *hc,
0))
{
GNUNET_break (0);
- json_decref (address);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
GNUNET_free (code_challenge);
return CH_reply_with_oauth_error (
hc->connection,
@@ -620,11 +654,6 @@ CH_handler_token (struct CH_HandlerContext *hc,
(NULL == encoded_hash) )
{
GNUNET_break (0);
- json_decref (address);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
GNUNET_free (code_challenge);
GNUNET_free (encoded_hash);
return CH_reply_with_oauth_error (
@@ -640,11 +669,6 @@ CH_handler_token (struct CH_HandlerContext *hc,
code_challenge))
{
GNUNET_break_op (0);
- json_decref (address);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
GNUNET_free (code_challenge);
GNUNET_free (encoded_hash);
return CH_reply_with_oauth_error (
@@ -663,11 +687,6 @@ CH_handler_token (struct CH_HandlerContext *hc,
code_challenge))
{
GNUNET_break_op (0);
- json_decref (address);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
GNUNET_free (code_challenge);
return CH_reply_with_oauth_error (
hc->connection,
@@ -680,11 +699,6 @@ CH_handler_token (struct CH_HandlerContext *hc,
case CHALLENGER_CM_UNKNOWN:
case CHALLENGER_CM_EMPTY:
GNUNET_break (0);
- json_decref (address);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
GNUNET_free (code_challenge);
return CH_reply_with_oauth_error (
hc->connection,
@@ -694,47 +708,7 @@ CH_handler_token (struct CH_HandlerContext *hc,
"Database has empty or unknown challenge mode but with code_challenge");
}
}
-
- if (NULL == address)
- {
- GNUNET_break_op (0);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
- GNUNET_free (code_challenge);
- return CH_reply_with_oauth_error (
- hc->connection,
- MHD_HTTP_CONFLICT,
- "invalid_request",
- TALER_EC_CHALLENGER_MISSING_ADDRESS,
- "code");
- }
- code = CH_compute_code (&bc->nonce,
- client_secret,
- client_scope,
- address,
- client_redirect_uri);
- json_decref (address);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
GNUNET_free (code_challenge);
- if (0 !=
- TALER_strcmp_ct (code,
- bc->code))
- {
- GNUNET_break_op (0);
- GNUNET_free (code);
- return CH_reply_with_oauth_error (
- hc->connection,
- MHD_HTTP_BAD_REQUEST,
- "invalid_grant",
- TALER_EC_CHALLENGER_CLIENT_FORBIDDEN_BAD_CODE,
- "code");
- }
- GNUNET_free (code);
}
{
diff --git a/src/challenger/meson.build b/src/challenger/meson.build
@@ -15,6 +15,7 @@ check_SCRIPTS = [
'test-challenger-badutf8',
'test-challenger-pinlimit',
'test-challenger-db-retry',
+ 'test-challenger-token-errors',
]
test_helper_cat = configure_file(input: 'cat.sh', output: 'cat.sh', copy: true)
