From 2e2a558a59d0e7a25953cddf2afb67b7ab96135c Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Fri, 27 Aug 2021 17:44:01 +0200 Subject: fix retry counter logic, deal with negative retry counters, limit code reuse for iban method --- .../anastasis_authorization_plugin_email.c | 5 + .../anastasis_authorization_plugin_file.c | 6 + .../anastasis_authorization_plugin_iban.c | 1 + .../anastasis_authorization_plugin_post.c | 7 + .../anastasis_authorization_plugin_sms.c | 5 + src/backend/anastasis-httpd_truth.c | 56 +++----- src/include/anastasis_authorization_plugin.h | 6 + src/include/anastasis_database_plugin.h | 30 +--- src/reducer/anastasis_api_recovery_redux.c | 1 - src/restclient/anastasis_api_keyshare_lookup.c | 4 + src/stasis/plugin_anastasis_postgres.c | 154 +++------------------ src/stasis/test_anastasis_db.c | 3 + 12 files changed, 82 insertions(+), 196 deletions(-) diff --git a/src/authorization/anastasis_authorization_plugin_email.c b/src/authorization/anastasis_authorization_plugin_email.c index 78c12ca..0eefcc5 100644 --- a/src/authorization/anastasis_authorization_plugin_email.c +++ b/src/authorization/anastasis_authorization_plugin_email.c @@ -27,6 +27,10 @@ #include #include "anastasis_database_lib.h" +/** + * How many retries do we allow per code? + */ +#define INITIAL_RETRY_COUNTER 3 /** * Saves the State of a authorization plugin. @@ -595,6 +599,7 @@ libanastasis_plugin_authorization_email_init (void *cls) } plugin = GNUNET_new (struct ANASTASIS_AuthorizationPlugin); + plugin->retry_counter = INITIAL_RETRY_COUNTER; plugin->code_validity_period = GNUNET_TIME_UNIT_DAYS; plugin->code_rotation_period = GNUNET_TIME_UNIT_HOURS; plugin->code_retransmission_frequency = GNUNET_TIME_UNIT_MINUTES; diff --git a/src/authorization/anastasis_authorization_plugin_file.c b/src/authorization/anastasis_authorization_plugin_file.c index db9dc67..66dbbe1 100644 --- a/src/authorization/anastasis_authorization_plugin_file.c +++ b/src/authorization/anastasis_authorization_plugin_file.c @@ -24,6 +24,11 @@ #include #include "anastasis_database_lib.h" +/** + * How many retries do we allow per code? + */ +#define INITIAL_RETRY_COUNTER 3 + /** * Saves the state of a authorization process @@ -293,6 +298,7 @@ libanastasis_plugin_authorization_file_init (void *cls) plugin = GNUNET_new (struct ANASTASIS_AuthorizationPlugin); plugin->cls = (void *) ac; + plugin->retry_counter = INITIAL_RETRY_COUNTER; plugin->code_validity_period = GNUNET_TIME_UNIT_MINUTES; plugin->code_rotation_period = GNUNET_TIME_UNIT_MINUTES; plugin->code_retransmission_frequency = GNUNET_TIME_UNIT_MINUTES; diff --git a/src/authorization/anastasis_authorization_plugin_iban.c b/src/authorization/anastasis_authorization_plugin_iban.c index be8f33b..cdd51a7 100644 --- a/src/authorization/anastasis_authorization_plugin_iban.c +++ b/src/authorization/anastasis_authorization_plugin_iban.c @@ -697,6 +697,7 @@ libanastasis_plugin_authorization_iban_init (void *cls) ctx->ac = ac; plugin = GNUNET_new (struct ANASTASIS_AuthorizationPlugin); plugin->payment_plugin_managed = true; + plugin->retry_counter = UINT32_MAX; /* long polling */ plugin->code_validity_period = CODE_VALIDITY_PERIOD; plugin->code_rotation_period = GNUNET_TIME_UNIT_ZERO; plugin->code_retransmission_frequency = GNUNET_TIME_UNIT_ZERO; /* not applicable */ diff --git a/src/authorization/anastasis_authorization_plugin_post.c b/src/authorization/anastasis_authorization_plugin_post.c index 98dd042..4adeffd 100644 --- a/src/authorization/anastasis_authorization_plugin_post.c +++ b/src/authorization/anastasis_authorization_plugin_post.c @@ -27,6 +27,12 @@ #include #include "anastasis_database_lib.h" +/** + * How many retries do we allow per code? + */ +#define INITIAL_RETRY_COUNTER 3 + + /** * Saves the State of a authorization plugin. */ @@ -633,6 +639,7 @@ libanastasis_plugin_authorization_post_init (void *cls) GNUNET_free (fn); } plugin = GNUNET_new (struct ANASTASIS_AuthorizationPlugin); + plugin->retry_counter = INITIAL_RETRY_COUNTER; plugin->code_validity_period = GNUNET_TIME_UNIT_MONTHS; plugin->code_rotation_period = GNUNET_TIME_UNIT_WEEKS; plugin->code_retransmission_frequency diff --git a/src/authorization/anastasis_authorization_plugin_sms.c b/src/authorization/anastasis_authorization_plugin_sms.c index 4922380..94b2c0d 100644 --- a/src/authorization/anastasis_authorization_plugin_sms.c +++ b/src/authorization/anastasis_authorization_plugin_sms.c @@ -27,6 +27,10 @@ #include #include "anastasis_database_lib.h" +/** + * How many retries do we allow per code? + */ +#define INITIAL_RETRY_COUNTER 3 /** * Saves the State of a authorization plugin. @@ -585,6 +589,7 @@ libanastasis_plugin_authorization_sms_init (void *cls) } } plugin = GNUNET_new (struct ANASTASIS_AuthorizationPlugin); + plugin->retry_counter = INITIAL_RETRY_COUNTER; plugin->code_validity_period = GNUNET_TIME_UNIT_DAYS; plugin->code_rotation_period = GNUNET_TIME_UNIT_HOURS; plugin->code_retransmission_frequency = GNUNET_TIME_UNIT_MINUTES; diff --git a/src/backend/anastasis-httpd_truth.c b/src/backend/anastasis-httpd_truth.c index df105dd..613e27f 100644 --- a/src/backend/anastasis-httpd_truth.c +++ b/src/backend/anastasis-httpd_truth.c @@ -56,6 +56,7 @@ */ #define INITIAL_RETRY_COUNTER 3 + struct GetContext { @@ -1289,6 +1290,8 @@ AH_handler_truth_get ( but check that the hash matches */ if (is_question) { + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Handling security question challenge\n"); if (! gc->have_response) { GNUNET_free (decrypted_truth); @@ -1306,6 +1309,7 @@ AH_handler_truth_get ( enum ANASTASIS_DB_CodeStatus cs; struct GNUNET_HashCode hc; bool satisfied; + uint64_t dummy; rt = GNUNET_TIME_UNIT_FOREVER_ABS; qs = db->create_challenge_code (db->cls, @@ -1340,6 +1344,7 @@ AH_handler_truth_get ( cs = db->verify_challenge_code (db->cls, &gc->truth_uuid, &hc, + &dummy, &satisfied); switch (cs) { @@ -1390,11 +1395,13 @@ AH_handler_truth_get ( { enum ANASTASIS_DB_CodeStatus cs; bool satisfied; + uint64_t code; GNUNET_free (truth_mime); cs = db->verify_challenge_code (db->cls, &gc->truth_uuid, &gc->challenge_response, + &code, &satisfied); switch (cs) { @@ -1415,15 +1422,14 @@ AH_handler_truth_get ( TALER_EC_GENERIC_DB_FETCH_FAILED, "verify_challenge_code"); case ANASTASIS_DB_CODE_STATUS_NO_RESULTS: - GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, - "No challenge known (challenge is invalidated after %u requests)\n", - INITIAL_RETRY_COUNTER); - GNUNET_free (decrypted_truth); - return TALER_MHD_reply_with_error (connection, - MHD_HTTP_TOO_MANY_REQUESTS, - TALER_EC_ANASTASIS_TRUTH_RATE_LIMITED, - NULL); + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Response code unknown (possibly expired). Testing if we may provide a new one.\n"); + gc->have_response = false; + break; case ANASTASIS_DB_CODE_STATUS_VALID_CODE_STORED: + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Response code valid (%s)\n", + satisfied ? "satisfied" : "unsatisfied"); if (satisfied) { GNUNET_free (decrypted_truth); @@ -1431,43 +1437,19 @@ AH_handler_truth_get ( connection); } /* continue with authorization plugin below */ - { - enum GNUNET_DB_QueryStatus qs; - - qs = db->get_unlimited_challenge_code ( - db->cls, - &gc->truth_uuid, - gc->authorization->code_rotation_period, - gc->authorization->code_validity_period, - &gc->code); - switch (qs) - { - case GNUNET_DB_STATUS_HARD_ERROR: - case GNUNET_DB_STATUS_SOFT_ERROR: - case GNUNET_DB_STATUS_SUCCESS_NO_RESULTS: - GNUNET_break (0); - GNUNET_free (decrypted_truth); - return TALER_MHD_reply_with_error (gc->connection, - MHD_HTTP_INTERNAL_SERVER_ERROR, - TALER_EC_GENERIC_DB_FETCH_FAILED, - "create_challenge_code"); - case GNUNET_DB_STATUS_SUCCESS_ONE_RESULT: - /* challenge code was stored successfully*/ - GNUNET_log (GNUNET_ERROR_TYPE_INFO, - "Got challenge code\n"); - break; - } - } + gc->code = code; break; default: GNUNET_break (0); return MHD_NO; } } - else + if (! gc->have_response) { /* Not security question and no answer: use plugin to check if decrypted truth is a valid challenge! */ + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "No challenge provided, creating fresh challenge\n"); { enum GNUNET_GenericReturnValue ret; @@ -1502,7 +1484,7 @@ AH_handler_truth_get ( &gc->truth_uuid, gc->authorization->code_rotation_period, gc->authorization->code_validity_period, - INITIAL_RETRY_COUNTER, + gc->authorization->retry_counter, &transmission_date, &gc->code); switch (qs) diff --git a/src/include/anastasis_authorization_plugin.h b/src/include/anastasis_authorization_plugin.h index 55f6369..91a88f8 100644 --- a/src/include/anastasis_authorization_plugin.h +++ b/src/include/anastasis_authorization_plugin.h @@ -126,6 +126,12 @@ struct ANASTASIS_AuthorizationPlugin */ bool payment_plugin_managed; + /** + * How often are retries allowed for challenges created + * by this plugin? + */ + uint32_t retry_counter; + /** * How long should a generated challenge be valid for this type of method. */ diff --git a/src/include/anastasis_database_plugin.h b/src/include/anastasis_database_plugin.h index 079201d..565ad69 100644 --- a/src/include/anastasis_database_plugin.h +++ b/src/include/anastasis_database_plugin.h @@ -586,6 +586,7 @@ struct ANASTASIS_DatabasePlugin * @param cls closure * @param truth_uuid identification of the challenge which the code corresponds to * @param hashed_code code which the user provided and wants to verify + * @param[out] code set to the original numeric code * @param[out] satisfied set to true if the challenge is set to satisfied * @return transaction status */ @@ -594,6 +595,7 @@ struct ANASTASIS_DatabasePlugin void *cls, const struct ANASTASIS_CRYPTO_TruthUUIDP *truth_uuid, const struct GNUNET_HashCode *hashed_code, + uint64_t *code, bool *satisfied); @@ -655,37 +657,11 @@ struct ANASTASIS_DatabasePlugin const struct ANASTASIS_CRYPTO_TruthUUIDP *truth_uuid, struct GNUNET_TIME_Relative rotation_period, struct GNUNET_TIME_Relative validity_period, - unsigned int retry_counter, + uint32_t retry_counter, struct GNUNET_TIME_Absolute *retransmission_date, uint64_t *code); - /** - * Setup challenge code for a given challenge identified by the - * challenge public key. The function will first check if there is - * already a valid code for this challenge present and won't insert - * a new one in this case. This variant is not rate-limited, will - * return the existing challenge if it has not yet expired and will - * simply create new challenges when the old ones need to be - * rotated. - * - * @param cls closure - * @param truth_uuid the identifier for the challenge - * @param rotation_period for how long is the code available - * @param validity_period for how long is the code available - * @param[out] code set to the code which will be checked for later - * @return transaction status, - * #GNUNET_DB_STATUS_SUCCESS_ONE_RESULT if @a code is now in the DB - */ - enum GNUNET_DB_QueryStatus - (*get_unlimited_challenge_code)( - void *cls, - const struct ANASTASIS_CRYPTO_TruthUUIDP *truth_uuid, - struct GNUNET_TIME_Relative rotation_period, - struct GNUNET_TIME_Relative validity_period, - uint64_t *code); - - /** * Remember in the database that we successfully sent a challenge. * diff --git a/src/reducer/anastasis_api_recovery_redux.c b/src/reducer/anastasis_api_recovery_redux.c index 864ded2..fa550c6 100644 --- a/src/reducer/anastasis_api_recovery_redux.c +++ b/src/reducer/anastasis_api_recovery_redux.c @@ -722,7 +722,6 @@ answer_feedback_cb ( uuid, err)); } - GNUNET_break_op (0); set_state (sctx->state, ANASTASIS_RECOVERY_STATE_CHALLENGE_SELECTING); sctx->cb (sctx->cb_cls, diff --git a/src/restclient/anastasis_api_keyshare_lookup.c b/src/restclient/anastasis_api_keyshare_lookup.c index c0c2812..5eeeee6 100644 --- a/src/restclient/anastasis_api_keyshare_lookup.c +++ b/src/restclient/anastasis_api_keyshare_lookup.c @@ -473,6 +473,8 @@ ANASTASIS_keyshare_lookup ( { answer_s = GNUNET_STRINGS_data_to_string_alloc (hashed_answer, sizeof (*hashed_answer)); + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Querying challenge with existing response code\n"); kslo->url = TALER_url_join (backend_url, path, "response", @@ -486,6 +488,8 @@ ANASTASIS_keyshare_lookup ( } else { + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Querying challenge without response code\n"); kslo->url = TALER_url_join (backend_url, path, "timeout_ms", diff --git a/src/stasis/plugin_anastasis_postgres.c b/src/stasis/plugin_anastasis_postgres.c index 506c304..8ee16ad 100644 --- a/src/stasis/plugin_anastasis_postgres.c +++ b/src/stasis/plugin_anastasis_postgres.c @@ -1762,6 +1762,11 @@ struct CheckValidityContext */ struct PostgresClosure *pg; + /** + * Set to the matching challenge code (if @e valid). + */ + uint64_t code; + /** * Set to true if a code matching @e hashed_code was found. */ @@ -1828,6 +1833,7 @@ check_valid_code (void *cls, cvc->hashed_code)) { cvc->valid = true; + cvc->code = server_code; cvc->satisfied = (0 != sat); } else @@ -1862,6 +1868,7 @@ check_valid_code (void *cls, * @param cls closure * @param truth_uuid identification of the challenge which the code corresponds to * @param hashed_code code which the user provided and wants to verify + * @param[out] code set to the original numeric code * @param[out] satisfied set to true if the challenge is set to satisfied * @return code validity status */ @@ -1870,6 +1877,7 @@ postgres_verify_challenge_code ( void *cls, const struct ANASTASIS_CRYPTO_TruthUUIDP *truth_uuid, const struct GNUNET_HashCode *hashed_code, + uint64_t *code, bool *satisfied) { struct PostgresClosure *pg = cls; @@ -1897,6 +1905,7 @@ postgres_verify_challenge_code ( if ( (qs < 0) || (cvc.db_failure) ) return ANASTASIS_DB_CODE_STATUS_HARD_ERROR; + *code = cvc.code; if (cvc.valid) { *satisfied = cvc.satisfied; @@ -2059,7 +2068,7 @@ postgres_create_challenge_code ( const struct ANASTASIS_CRYPTO_TruthUUIDP *truth_uuid, struct GNUNET_TIME_Relative rotation_period, struct GNUNET_TIME_Relative validity_period, - unsigned int retry_counter, + uint32_t retry_counter, struct GNUNET_TIME_Absolute *retransmission_date, uint64_t *code) { @@ -2117,15 +2126,22 @@ postgres_create_challenge_code ( case GNUNET_DB_STATUS_SOFT_ERROR: goto retry; case GNUNET_DB_STATUS_SUCCESS_NO_RESULTS: - /* no active challenge, create fresh one (below) */ + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "No active challenge found, creating a fresh one\n"); break; case GNUNET_DB_STATUS_SUCCESS_ONE_RESULT: if (0 == old_retry_counter) { rollback (pg); + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Active challenge %llu has zero tries left, refusing to create another one\n", + (unsigned long long) code); return GNUNET_DB_STATUS_SUCCESS_NO_RESULTS; } rollback (pg); + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Active challenge has %u tries left, returning old challenge\n", + (unsigned int) old_retry_counter); return qs; } } @@ -2158,132 +2174,9 @@ postgres_create_challenge_code ( rollback (pg); return GNUNET_DB_STATUS_HARD_ERROR; case GNUNET_DB_STATUS_SUCCESS_ONE_RESULT: - break; - } - } - qs = commit_transaction (pg); - if (GNUNET_DB_STATUS_SOFT_ERROR == qs) - goto retry; - if (qs < 0) - return qs; - return GNUNET_DB_STATUS_SUCCESS_ONE_RESULT; -retry: - rollback (pg); - } - return GNUNET_DB_STATUS_SOFT_ERROR; -} - - -/** - * Setup challenge code for a given challenge identified by the - * challenge public key. The function will first check if there is - * already a valid code for this challenge present and won't insert - * a new one in this case. This variant is not rate-limited, will - * return the existing challenge if it has not yet expired and will - * simply create new challenges when the old ones need to be - * rotated. - * - * @param cls closure - * @param truth_uuid the identifier for the challenge - * @param rotation_period for how long is the code available - * @param validity_period for how long is the code available - * @param[out] code set to the code which will be checked for later - * @return transaction status, - * #GNUNET_DB_STATUS_SUCCESS_ONE_RESULT if @a code is now in the DB - */ -static enum GNUNET_DB_QueryStatus -postgres_get_unlimited_challenge_code ( - void *cls, - const struct ANASTASIS_CRYPTO_TruthUUIDP *truth_uuid, - struct GNUNET_TIME_Relative rotation_period, - struct GNUNET_TIME_Relative validity_period, - uint64_t *code) -{ - struct PostgresClosure *pg = cls; - enum GNUNET_DB_QueryStatus qs; - struct GNUNET_TIME_Absolute now = GNUNET_TIME_absolute_get (); - struct GNUNET_TIME_Absolute expiration_date; - struct GNUNET_TIME_Absolute ex_rot; - - check_connection (pg); - GNUNET_TIME_round_abs (&now); - expiration_date = GNUNET_TIME_absolute_add (now, - validity_period); - ex_rot = GNUNET_TIME_absolute_subtract (now, - rotation_period); - for (unsigned int retries = 0; retriesconn, - "challengecode_select_meta", - params, - rs); - switch (qs) - { - case GNUNET_DB_STATUS_HARD_ERROR: - GNUNET_break (0); - rollback (pg); - return qs; - case GNUNET_DB_STATUS_SOFT_ERROR: - goto retry; - case GNUNET_DB_STATUS_SUCCESS_NO_RESULTS: - /* no active challenge, create fresh one (below) */ - break; - case GNUNET_DB_STATUS_SUCCESS_ONE_RESULT: - rollback (pg); - return qs; - } - } - - *code = GNUNET_CRYPTO_random_u64 (GNUNET_CRYPTO_QUALITY_NONCE, - NONCE_MAX_VALUE); - { - uint32_t retry_counter = UINT32_MAX; - struct GNUNET_PQ_QueryParam params[] = { - GNUNET_PQ_query_param_auto_from_type (truth_uuid), - GNUNET_PQ_query_param_uint64 (code), - TALER_PQ_query_param_absolute_time (&now), - TALER_PQ_query_param_absolute_time (&expiration_date), - GNUNET_PQ_query_param_uint32 (&retry_counter), - GNUNET_PQ_query_param_end - }; - - qs = GNUNET_PQ_eval_prepared_non_select (pg->conn, - "challengecode_insert", - params); - switch (qs) - { - case GNUNET_DB_STATUS_HARD_ERROR: - rollback (pg); - return GNUNET_DB_STATUS_HARD_ERROR; - case GNUNET_DB_STATUS_SOFT_ERROR: - goto retry; - case GNUNET_DB_STATUS_SUCCESS_NO_RESULTS: - GNUNET_break (0); - rollback (pg); - return GNUNET_DB_STATUS_HARD_ERROR; - case GNUNET_DB_STATUS_SUCCESS_ONE_RESULT: + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Created fresh challenge with %u tries left\n", + (unsigned int) retry_counter); break; } } @@ -2675,7 +2568,7 @@ libanastasis_plugin_db_postgres_init (void *cls) " FROM anastasis_challengecode" " WHERE truth_uuid=$1" " AND expiration_date > $2" - " AND retry_counter > 0;", + " AND retry_counter != 0;", 2), GNUNET_PQ_make_prepare ("challengecode_set_satisfied", "UPDATE anastasis_challengecode" @@ -2715,7 +2608,7 @@ libanastasis_plugin_db_postgres_init (void *cls) " SET retry_counter=retry_counter - 1" " WHERE truth_uuid=$1" " AND code=$2" - " AND retry_counter > 0;", + " AND retry_counter != 0;", 1), GNUNET_PQ_make_prepare ("challengepayment_dec_counter", "UPDATE anastasis_challenge_payment" @@ -2807,7 +2700,6 @@ libanastasis_plugin_db_postgres_init (void *cls) plugin->test_challenge_code_satisfied = &postgres_test_challenge_code_satisfied; plugin->create_challenge_code = &postgres_create_challenge_code; - plugin->get_unlimited_challenge_code = &postgres_get_unlimited_challenge_code; plugin->mark_challenge_sent = &postgres_mark_challenge_sent; plugin->challenge_gc = &postgres_challenge_gc; plugin->record_truth_upload_payment = &postgres_record_truth_upload_payment; diff --git a/src/stasis/test_anastasis_db.c b/src/stasis/test_anastasis_db.c index 8f11827..204307a 100644 --- a/src/stasis/test_anastasis_db.c +++ b/src/stasis/test_anastasis_db.c @@ -270,11 +270,13 @@ run (void *cls) &c_hash); { bool sat; + uint64_t r_code; FAILIF (ANASTASIS_DB_CODE_STATUS_CHALLENGE_CODE_MISMATCH != plugin->verify_challenge_code (plugin->cls, &truth_uuid, &c_hash, + &r_code, &sat)); ANASTASIS_hash_answer (challenge_code, @@ -283,6 +285,7 @@ run (void *cls) plugin->verify_challenge_code (plugin->cls, &truth_uuid, &c_hash, + &r_code, &sat)); } if (-1 == result) -- cgit v1.2.3