exchange

Base system with REST service to issue digital coins, run by the payment service provider
Log | Files | Refs | Submodules | README | LICENSE

commit 9f243046efb7ffc3bf7b0690632434645effac81
parent 0b995184993af12cbb3012cccf245efa75679e8d
Author: Christian Grothoff <christian@grothoff.org>
Date:   Thu, 24 Sep 2026 00:24:56 +0200

kyclogic: bind the OAuth2 state to the KYC process

The state was just the account hash, so anyone knowing an account could fail
its KYC process with a forged error redirect, or complete it with a code
obtained for another process; it now carries an HMAC tag and mismatches are
refused with 403 without touching the process.

Issue: https://bugs.taler.net/n/11740
Signed-off-by: Christian Grothoff <christian@grothoff.org>

Diffstat:
Mcontrib/meson.build | 1+
Acontrib/oauth2-state-invalid.en.must | 18++++++++++++++++++
Msrc/exchange/taler-exchange-httpd_get-kyc-proof-PROVIDER_NAME.c | 47++++++++++++++++++++++++++++++++++++++++++++---
Msrc/include/taler/exchange/get-kyc-proof-PROVIDER_NAME.h | 8+++++---
Msrc/include/taler/taler_testing_lib.h | 7+++++--
Msrc/kyclogic/plugin_kyclogic_oauth2.c | 123++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
Msrc/kyclogic/taler-exchange-kyc-tester.c | 3++-
Msrc/lib/exchange_api_get-kyc-proof-PROVIDER_NAME.c | 21+++++++++------------
Msrc/testing/test_exchange_api_age_restriction.c | 2+-
Msrc/testing/test_exchange_p2p.c | 2+-
Msrc/testing/test_kyc_api.c | 14+++++++-------
Msrc/testing/testing_api_cmd_kyc_proof.c | 85+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------
12 files changed, 278 insertions(+), 53 deletions(-)

diff --git a/contrib/meson.build b/contrib/meson.build @@ -15,6 +15,7 @@ contrib_tmplpkgdata_DATA = [ 'oauth2-bad-request.en.must', 'oauth2-conversion-failure.en.must', 'oauth2-provider-failure.en.must', + 'oauth2-state-invalid.en.must', 'persona-exchange-unauthorized.en.must', 'persona-load-failure.en.must', 'persona-exchange-unpaid.en.must', diff --git a/contrib/oauth2-state-invalid.en.must b/contrib/oauth2-state-invalid.en.must @@ -0,0 +1,18 @@ +<html> +<head> +<title>403: KYC process not recognized</title> +</head> +<body> +The link that brought you here does not belong to the KYC process +currently running for this account, so it was ignored and the +process remains unchanged. Please restart the KYC process from +your wallet or merchant backend if needed. +<pre> +<!-- Taler error code --> {{ code }}: +<!-- GANA EC hint --> {{ hint }} +</pre> +<p> +<!-- optional human-readable message --> {{ message }} +</p> +</body> +</html> diff --git a/src/exchange/taler-exchange-httpd_get-kyc-proof-PROVIDER_NAME.c b/src/exchange/taler-exchange-httpd_get-kyc-proof-PROVIDER_NAME.c @@ -447,6 +447,16 @@ proof_cb ( TALER_EC_NONE, NULL); break; + case TALER_KYCLOGIC_STATUS_KEEP: + /* The logic refused the request without learning anything about + the process, e.g. as the request was not issued for it. */ + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "KYC process #%llu unchanged by proof request\n", + (unsigned long long) kpc->process_row); + proof_finish (kpc, + TALER_EC_NONE, + NULL); + break; default: GNUNET_break (0); GNUNET_log (GNUNET_ERROR_TYPE_INFO, @@ -526,9 +536,40 @@ TEH_handler_kyc_proof ( kpc->rc = rc; rc->rh_ctx = kpc; rc->rh_cleaner = &clean_kpc; - TALER_MHD_parse_request_arg_auto_t (rc->connection, - "state", - &kpc->h_payto); + { + const char *state; + const char *tag; + + /* The state starts with the account, possibly followed by a + '-' and a tag that only the KYC logic understands. */ + state = MHD_lookup_connection_value (rc->connection, + MHD_GET_ARGUMENT_KIND, + "state"); + if (NULL == state) + { + GNUNET_break_op (0); + return TALER_MHD_reply_with_error (rc->connection, + MHD_HTTP_BAD_REQUEST, + TALER_EC_GENERIC_PARAMETER_MISSING, + "state"); + } + tag = strchr (state, + '-'); + if (GNUNET_OK != + GNUNET_STRINGS_string_to_data (state, + (NULL == tag) + ? strlen (state) + : (size_t) (tag - state), + &kpc->h_payto, + sizeof (kpc->h_payto))) + { + GNUNET_break_op (0); + return TALER_MHD_reply_with_error (rc->connection, + MHD_HTTP_BAD_REQUEST, + TALER_EC_GENERIC_PARAMETER_MALFORMED, + "state"); + } + } if (GNUNET_OK != TALER_KYCLOGIC_lookup_logic ( provider_name_or_logic, diff --git a/src/include/taler/exchange/get-kyc-proof-PROVIDER_NAME.h b/src/include/taler/exchange/get-kyc-proof-PROVIDER_NAME.h @@ -82,7 +82,9 @@ struct TALER_EXCHANGE_GetKycProofHandle; * * @param ctx the context * @param url base URL of the exchange - * @param h_payto hash of the payto URI identifying the target account + * @param state OAuth2 state the KYC provider redirected with, the + * encoded hash of the payto URI identifying the target account, + * possibly followed by a tag the KYC logic added * @param logic name of the KYC logic / provider to use * @return handle to operation */ @@ -90,7 +92,7 @@ struct TALER_EXCHANGE_GetKycProofHandle * TALER_EXCHANGE_get_kyc_proof_create ( struct GNUNET_CURL_Context *ctx, const char *url, - const struct TALER_NormalizedPaytoHashP *h_payto, + const char *state, const char *logic); @@ -148,7 +150,7 @@ TALER_EXCHANGE_get_kyc_proof_set_options_ ( * * TALER_EXCHANGE_get_kyc_proof_set_options ( * gkph, - * TALER_EXCHANGE_get_kyc_proof_option_args ("&state=xyz")); + * TALER_EXCHANGE_get_kyc_proof_option_args ("&code=xyz")); * * @param gkph the request to set the options for * @param ... the list of options, each created by a diff --git a/src/include/taler/taler_testing_lib.h b/src/include/taler/taler_testing_lib.h @@ -2306,7 +2306,10 @@ TALER_TESTING_cmd_post_kyc_form ( * logic, as it generates an OAuth2.0-specific request. * * @param label command label. - * @param payment_target_reference command with a payment target to query + * @param state_reference KYC start command whose KYC URL carries the + * OAuth2 state to use; or, to act like someone who does not know + * that state, a command with a payment target whose bare hash is + * used instead * @param logic_section name of the KYC provider section * in the exchange configuration for this proof * @param code OAuth 2.0 code to use @@ -2316,7 +2319,7 @@ TALER_TESTING_cmd_post_kyc_form ( struct TALER_TESTING_Command TALER_TESTING_cmd_proof_kyc_oauth2 ( const char *label, - const char *payment_target_reference, + const char *state_reference, const char *logic_section, const char *code, unsigned int expected_response_code); diff --git a/src/kyclogic/plugin_kyclogic_oauth2.c b/src/kyclogic/plugin_kyclogic_oauth2.c @@ -251,6 +251,11 @@ struct TALER_KYCLOGIC_ProofHandle char *post_body; /** + * OAuth2 ``state`` of the process, see compute_state(). + */ + char *state; + + /** * KYC attributes returned about the user by the OAuth 2.0 server. */ json_t *attributes; @@ -593,6 +598,66 @@ oauth2_initiate_cancel (struct TALER_KYCLOGIC_InitiateHandle *ih) /** + * Compute the OAuth2 ``state`` for a KYC process. Next to the + * account the ``/kyc-proof`` handler needs to find the process, it + * carries a tag that only we and the provider can compute. The state + * is thus unguessable and bound to the process, as RFC 6749 (section + * 10.12) requires: whoever did not see the authorization request can + * neither fail the process with a forged error redirect nor complete + * it with an authorization code obtained for another process. + * + * @param pd provider the process runs with, its client secret keys the tag + * @param h_payto account the process is for + * @param process_row row of the process in the legitimization processes table + * @return the state, "$H_PAYTO-$TAG" + */ +static char * +compute_state (const struct TALER_KYCLOGIC_ProviderDetails *pd, + const struct TALER_NormalizedPaytoHashP *h_payto, + uint64_t process_row) +{ + static const char context[] = "taler-kyc-oauth2-state"; + char msg[sizeof (context) + sizeof (*h_payto) + sizeof (uint64_t)]; + uint64_t row_nbo = GNUNET_htonll (process_row); + struct GNUNET_HashCode hmac; + struct GNUNET_ShortHashCode tag; + char *hps; + char *tags; + char *state; + + memcpy (msg, + context, + sizeof (context)); + memcpy (&msg[sizeof (context)], + h_payto, + sizeof (*h_payto)); + memcpy (&msg[sizeof (context) + sizeof (*h_payto)], + &row_nbo, + sizeof (row_nbo)); + GNUNET_CRYPTO_hmac_raw ((const unsigned char *) pd->client_secret, + strlen (pd->client_secret), + msg, + sizeof (msg), + &hmac); + GNUNET_static_assert (sizeof (tag) <= sizeof (hmac)); + memcpy (&tag, + &hmac, + sizeof (tag)); + hps = GNUNET_STRINGS_data_to_string_alloc (h_payto, + sizeof (*h_payto)); + tags = GNUNET_STRINGS_data_to_string_alloc (&tag, + sizeof (tag)); + GNUNET_asprintf (&state, + "%s-%s", + hps, + tags); + GNUNET_free (tags); + GNUNET_free (hps); + return state; +} + + +/** * Logic to asynchronously return the response for * how to begin the OAuth2.0 checking process to * the client. @@ -607,7 +672,7 @@ initiate_with_url (struct TALER_KYCLOGIC_InitiateHandle *ih, const struct TALER_KYCLOGIC_ProviderDetails *pd = ih->pd; struct PluginState *ps = pd->ps; - char *hps; + char *state; char *url; char legi_s[42]; @@ -615,8 +680,9 @@ initiate_with_url (struct TALER_KYCLOGIC_InitiateHandle *ih, sizeof (legi_s), "%llu", (unsigned long long) ih->legitimization_uuid); - hps = GNUNET_STRINGS_data_to_string_alloc (&ih->h_payto, - sizeof (ih->h_payto)); + state = compute_state (pd, + &ih->h_payto, + ih->legitimization_uuid); { char *redirect_uri_encoded; char *client_id_encoded; @@ -641,7 +707,7 @@ initiate_with_url (struct TALER_KYCLOGIC_InitiateHandle *ih, authorize_url, client_id_encoded, redirect_uri_encoded, - hps, + state, scope_encoded); GNUNET_free (scope_encoded); GNUNET_free (client_id_encoded); @@ -654,7 +720,7 @@ initiate_with_url (struct TALER_KYCLOGIC_InitiateHandle *ih, legi_s, NULL /* no error */); GNUNET_free (url); - GNUNET_free (hps); + GNUNET_free (state); oauth2_initiate_cancel (ih); } @@ -987,6 +1053,7 @@ oauth2_proof_cancel (struct TALER_KYCLOGIC_ProofHandle *ph) if (NULL != ph->attributes) json_decref (ph->attributes); GNUNET_free (ph->post_body); + GNUNET_free (ph->state); GNUNET_free (ph); } @@ -1667,6 +1734,46 @@ oauth2_proof (void *cls, ph->h_payto = *account_id; ph->cb = cb; ph->cb_cls = cb_cls; + ph->state = compute_state (pd, + account_id, + process_row); + { + const char *state; + + state = MHD_lookup_connection_value (connection, + MHD_GET_ARGUMENT_KIND, + "state"); + GNUNET_assert (NULL != state); /* checked by the /kyc-proof handler */ + if ( (strlen (state) != strlen (ph->state)) || + (0 != GNUNET_memcmp_ct_ (state, + ph->state, + strlen (ph->state))) ) + { + json_t *body; + + /* Not the state we issued for this process: a forged or stale + redirect, which must neither complete nor fail the process. */ + GNUNET_break_op (0); + ph->status = TALER_KYCLOGIC_STATUS_KEEP; + ph->http_status = MHD_HTTP_FORBIDDEN; + body = GNUNET_JSON_PACK ( + GNUNET_JSON_pack_string ("message", + "'state' does not match the KYC process"), + TALER_JSON_pack_ec ( + TALER_EC_GENERIC_FORBIDDEN)); + GNUNET_break ( + GNUNET_SYSERR != + templating_build (ph->connection, + &ph->http_status, + "oauth2-state-invalid", + body, + &ph->response)); + json_decref (body); + ph->task = GNUNET_SCHEDULER_add_now (&return_proof_response, + ph); + return ph; + } + } code = MHD_lookup_connection_value (connection, MHD_GET_ARGUMENT_KIND, "code"); @@ -1776,10 +1883,7 @@ oauth2_proof (void *cls, char *client_secret; char *authorization_code; char *redirect_uri_encoded; - char *hps; - hps = GNUNET_STRINGS_data_to_string_alloc (&ph->h_payto, - sizeof (ph->h_payto)); { char *redirect_uri; @@ -1807,13 +1911,12 @@ oauth2_proof (void *cls, "client_id=%s&redirect_uri=%s&state=%s&client_secret=%s&code=%s&grant_type=authorization_code", client_id, redirect_uri_encoded, - hps, + ph->state, client_secret, authorization_code); curl_free (authorization_code); curl_free (client_secret); GNUNET_free (redirect_uri_encoded); - GNUNET_free (hps); curl_free (client_id); } GNUNET_assert (CURLE_OK == diff --git a/src/kyclogic/taler-exchange-kyc-tester.c b/src/kyclogic/taler-exchange-kyc-tester.c @@ -825,7 +825,8 @@ handler_kyc_proof_get ( } if (GNUNET_OK != GNUNET_STRINGS_string_to_data (h_paytos, - strlen (h_paytos), + strcspn (h_paytos, + "-"), &h_payto, sizeof (h_payto))) { diff --git a/src/lib/exchange_api_get-kyc-proof-PROVIDER_NAME.c b/src/lib/exchange_api_get-kyc-proof-PROVIDER_NAME.c @@ -71,9 +71,9 @@ struct TALER_EXCHANGE_GetKycProofHandle struct GNUNET_CURL_Context *ctx; /** - * Hash of the payto URI identifying the target account. + * OAuth2 state identifying the target account. Heap-allocated copy. */ - struct TALER_NormalizedPaytoHashP h_payto; + char *state; /** * Name of the KYC logic / provider. Heap-allocated copy. @@ -137,6 +137,9 @@ handle_get_kyc_proof_finished (void *cls, break; case MHD_HTTP_UNAUTHORIZED: break; + case MHD_HTTP_FORBIDDEN: + /* KYC process failed, or the request was not issued for it */ + break; case MHD_HTTP_NOT_FOUND: break; case MHD_HTTP_INTERNAL_SERVER_ERROR: @@ -169,7 +172,7 @@ struct TALER_EXCHANGE_GetKycProofHandle * TALER_EXCHANGE_get_kyc_proof_create ( struct GNUNET_CURL_Context *ctx, const char *url, - const struct TALER_NormalizedPaytoHashP *h_payto, + const char *state, const char *logic) { struct TALER_EXCHANGE_GetKycProofHandle *gkph; @@ -177,7 +180,7 @@ TALER_EXCHANGE_get_kyc_proof_create ( gkph = GNUNET_new (struct TALER_EXCHANGE_GetKycProofHandle); gkph->ctx = ctx; gkph->base_url = GNUNET_strdup (url); - gkph->h_payto = *h_payto; + gkph->state = GNUNET_strdup (state); gkph->logic = GNUNET_strdup (logic); return gkph; } @@ -221,22 +224,15 @@ TALER_EXCHANGE_get_kyc_proof_start ( gkph->cb = cb; gkph->cb_cls = cb_cls; { - char hstr[sizeof (gkph->h_payto) * 2]; char *arg_str; - char *end; char *url; GNUNET_asprintf (&arg_str, "kyc-proof/%s", gkph->logic); - end = GNUNET_STRINGS_data_to_string (&gkph->h_payto, - sizeof (gkph->h_payto), - hstr, - sizeof (hstr)); - *end = '\0'; url = TALER_url_join (gkph->base_url, arg_str, - "state", hstr, + "state", gkph->state, NULL); GNUNET_free (arg_str); if (NULL == url) @@ -297,6 +293,7 @@ TALER_EXCHANGE_get_kyc_proof_cancel ( } GNUNET_free (gkph->url); GNUNET_free (gkph->logic); + GNUNET_free (gkph->state); GNUNET_free (gkph->base_url); GNUNET_free (gkph); } diff --git a/src/testing/test_exchange_api_age_restriction.c b/src/testing/test_exchange_api_age_restriction.c @@ -334,7 +334,7 @@ run (void *cls, MHD_HTTP_OK), TALER_TESTING_cmd_proof_kyc_oauth2 ( "proof-withdraw-kyc", - "withdraw-coin-1-lacking-kyc", + "start-kyc-process", "test-oauth2", "pass", MHD_HTTP_SEE_OTHER), diff --git a/src/testing/test_exchange_p2p.c b/src/testing/test_exchange_p2p.c @@ -506,7 +506,7 @@ run (void *cls, MHD_HTTP_OK), TALER_TESTING_cmd_proof_kyc_oauth2 ( "proof-close-kyc", - "reserve-101-close-kyc", + "start-kyc-process", "test-oauth2", "pass", MHD_HTTP_SEE_OTHER), diff --git a/src/testing/test_kyc_api.c b/src/testing/test_kyc_api.c @@ -163,7 +163,7 @@ run (void *cls, MHD_HTTP_OK), TALER_TESTING_cmd_proof_kyc_oauth2 ( "proof-kyc-withdraw-oauth2", - "withdraw-coin-1-lacking-kyc", + "start-kyc-process-withdraw", "test-oauth2", "pass", MHD_HTTP_SEE_OTHER), @@ -267,7 +267,7 @@ run (void *cls, MHD_HTTP_OK), TALER_TESTING_cmd_proof_kyc_oauth2 ( "proof-kyc-no-service", - "track-deposit-kyc-ready", + "start-kyc-process-deposit", "test-oauth2", "bad", MHD_HTTP_BAD_GATEWAY), @@ -277,7 +277,7 @@ run (void *cls, 6666), TALER_TESTING_cmd_proof_kyc_oauth2 ( "proof-kyc-fail", - "track-deposit-kyc-ready", + "start-kyc-process-deposit", "test-oauth2", "bad", MHD_HTTP_FORBIDDEN), @@ -298,7 +298,7 @@ run (void *cls, MHD_HTTP_OK), TALER_TESTING_cmd_proof_kyc_oauth2 ( "proof-kyc-pass", - "track-deposit-kyc-ready", + "start-kyc-process-deposit-again", "test-oauth2", "pass", MHD_HTTP_SEE_OTHER), @@ -338,7 +338,7 @@ run (void *cls, MHD_HTTP_OK), TALER_TESTING_cmd_proof_kyc_oauth2 ( "proof-wallet-kyc", - "wallet-kyc-fail", + "start-kyc-wallet", "test-oauth2", "pass", MHD_HTTP_SEE_OTHER), @@ -474,7 +474,7 @@ run (void *cls, MHD_HTTP_OK), TALER_TESTING_cmd_proof_kyc_oauth2 ( "p2p_proof-kyc", - "purse-merge-into-reserve", + "start-kyc-process-purse-merge-into-reserve", "test-oauth2", "pass", MHD_HTTP_SEE_OTHER), @@ -528,7 +528,7 @@ run (void *cls, MHD_HTTP_OK), TALER_TESTING_cmd_proof_kyc_oauth2 ( "p2p_proof-kyc-pull", - "purse-create-with-reserve", + "start-kyc-process-purse-create", "test-oauth2", "pass", MHD_HTTP_SEE_OTHER), diff --git a/src/testing/testing_api_cmd_kyc_proof.c b/src/testing/testing_api_cmd_kyc_proof.c @@ -42,9 +42,11 @@ struct KycProofGetState { /** - * Command to get a reserve private key from. + * Command to get the OAuth2 state from: a KYC start command + * offering the KYC URL, or any command offering a payment target, + * whose bare hash is then used as the state. */ - const char *payment_target_reference; + const char *state_reference; /** * Code to pass. @@ -127,6 +129,39 @@ proof_kyc_cb ( /** + * Extract the OAuth2 state from the authorization URL + * the exchange returned to start a KYC process. + * + * @param kyc_url authorization URL to parse + * @return the state, NULL if @a kyc_url has none + */ +static char * +get_state (const char *kyc_url) +{ + const char *q; + + q = strchr (kyc_url, + '?'); + while (NULL != q) + { + q++; + if (0 == strncmp (q, + "state=", + strlen ("state="))) + { + q += strlen ("state="); + return GNUNET_strndup (q, + strcspn (q, + "&#")); + } + q = strchr (q, + '&'); + } + return NULL; +} + + +/** * Run the command. * * @param cls closure. @@ -140,7 +175,8 @@ proof_kyc_run (void *cls, { struct KycProofGetState *kps = cls; const struct TALER_TESTING_Command *res_cmd; - const struct TALER_NormalizedPaytoHashP *h_payto; + const char *kyc_url; + char *state; const char *exchange_url; (void) cmd; @@ -153,20 +189,42 @@ proof_kyc_run (void *cls, } res_cmd = TALER_TESTING_interpreter_lookup_command ( kps->is, - kps->payment_target_reference); + kps->state_reference); if (NULL == res_cmd) { GNUNET_break (0); TALER_TESTING_interpreter_fail (kps->is); return; } - if (GNUNET_OK != - TALER_TESTING_get_trait_h_normalized_payto (res_cmd, - &h_payto)) + if (GNUNET_OK == + TALER_TESTING_get_trait_kyc_url (res_cmd, + &kyc_url)) { - GNUNET_break (0); - TALER_TESTING_interpreter_fail (kps->is); - return; + /* Use the state the exchange issued, as the provider would. */ + state = get_state (kyc_url); + if (NULL == state) + { + GNUNET_break (0); + TALER_TESTING_interpreter_fail (kps->is); + return; + } + } + else + { + const struct TALER_NormalizedPaytoHashP *h_payto; + + /* Only know the account, like anyone who did not see the + authorization request. */ + if (GNUNET_OK != + TALER_TESTING_get_trait_h_normalized_payto (res_cmd, + &h_payto)) + { + GNUNET_break (0); + TALER_TESTING_interpreter_fail (kps->is); + return; + } + state = GNUNET_STRINGS_data_to_string_alloc (h_payto, + sizeof (*h_payto)); } if (NULL != kps->code) GNUNET_asprintf (&kps->uargs, @@ -175,8 +233,9 @@ proof_kyc_run (void *cls, kps->kph = TALER_EXCHANGE_get_kyc_proof_create ( TALER_TESTING_interpreter_get_context (is), exchange_url, - h_payto, + state, kps->logic); + GNUNET_free (state); GNUNET_assert (NULL != kps->kph); if (NULL != kps->uargs) TALER_EXCHANGE_get_kyc_proof_set_options ( @@ -256,7 +315,7 @@ proof_kyc_traits (void *cls, struct TALER_TESTING_Command TALER_TESTING_cmd_proof_kyc_oauth2 ( const char *label, - const char *payment_target_reference, + const char *state_reference, const char *logic_section, const char *code, unsigned int expected_response_code) @@ -266,7 +325,7 @@ TALER_TESTING_cmd_proof_kyc_oauth2 ( kps = GNUNET_new (struct KycProofGetState); kps->code = code; kps->logic = logic_section; - kps->payment_target_reference = payment_target_reference; + kps->state_reference = state_reference; kps->expected_response_code = expected_response_code; { struct TALER_TESTING_Command cmd = {