commit b4664821d397aec393e2c3449d1657ec2b89a3c0
parent e23d3ceeda8810ad21d3d1d23566811d9be7cc6d
Author: Christian Grothoff <christian@grothoff.org>
Date: Wed, 14 Feb 2024 19:21:49 +0100
bugfixes to error handling
Diffstat:
12 files changed, 341 insertions(+), 87 deletions(-)
diff --git a/src/challenger/challenger-httpd_authorize.c b/src/challenger/challenger-httpd_authorize.c
@@ -23,6 +23,7 @@
#include <gnunet/gnunet_util_lib.h>
#include <taler/taler_templating_lib.h>
#include "challenger-httpd_authorize.h"
+#include "challenger-httpd_common.h"
MHD_RESULT
@@ -175,6 +176,17 @@ CH_handler_authorize (struct CH_HandlerContext *hc,
case GNUNET_DB_STATUS_SUCCESS_ONE_RESULT:
break;
}
+ if (0 == address_attempts_left)
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+ "Refusing authorization: zero address attempts left\n");
+ return TALER_MHD_redirect_with_oauth_status (
+ hc->connection,
+ redirect_uri,
+ "unauthorized_client",
+ "client exceeded authorization attempts limit (too many addresses)",
+ NULL);
+ }
{
enum GNUNET_GenericReturnValue ret;
json_t *args;
diff --git a/src/challenger/challenger-httpd_challenge.c b/src/challenger/challenger-httpd_challenge.c
@@ -26,6 +26,7 @@
#include <taler/taler_templating_lib.h>
#include <taler/taler_merchant_service.h>
#include <taler/taler_signatures.h>
+#include "challenger-httpd_common.h"
/**
@@ -85,6 +86,11 @@ struct ChallengeContext
char *data;
/**
+ * Where to redirect the client on errors?
+ */
+ char *client_redirect_uri;
+
+ /**
* When did we transmit last?
*/
struct GNUNET_TIME_Absolute last_tx_time;
@@ -130,6 +136,12 @@ struct ChallengeContext
enum GNUNET_GenericReturnValue suspended;
/**
+ * True if the provided address was refused, usually because
+ * the user tried too many different addresses already.
+ */
+ bool address_refused;
+
+ /**
* Should we retransmit the PIN?
*/
bool retransmit;
@@ -199,6 +211,7 @@ cleanup_ctx (void *cls)
json_decref (bc->address);
GNUNET_free (bc->data);
GNUNET_free (bc->last_key);
+ GNUNET_free (bc->client_redirect_uri);
GNUNET_free (bc);
}
@@ -518,6 +531,7 @@ CH_handler_challenge (struct CH_HandlerContext *hc,
{
enum GNUNET_DB_QueryStatus qs;
+ GNUNET_assert (NULL == bc->client_redirect_uri);
qs = CH_db->challenge_set_address_and_pin (CH_db->cls,
&bc->nonce,
bc->address,
@@ -525,7 +539,9 @@ CH_handler_challenge (struct CH_HandlerContext *hc,
&bc->tan,
&bc->last_tx_time,
&bc->pin_attempts_left,
- &bc->retransmit);
+ &bc->retransmit,
+ &bc->client_redirect_uri,
+ &bc->address_refused);
switch (qs)
{
case GNUNET_DB_STATUS_HARD_ERROR:
@@ -549,15 +565,27 @@ CH_handler_challenge (struct CH_HandlerContext *hc,
break;
}
bc->db_finished = true;
+ if (bc->address_refused)
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_INFO,
+ "Address changes exhausted address change limit for this process\n");
+ return TALER_MHD_redirect_with_oauth_status (
+ hc->connection,
+ bc->client_redirect_uri,
+ "unauthorized_client",
+ "client exceeded authorization attempts limit (too many addresses attempted)",
+ NULL);
+ }
if (0 == bc->pin_attempts_left)
{
GNUNET_log (GNUNET_ERROR_TYPE_INFO,
- "Attempts exhausted for this PIN\n");
- return TALER_TEMPLATING_reply_error (hc->connection,
- "attempts-exhausted",
- MHD_HTTP_TOO_MANY_REQUESTS,
- TALER_EC_CHALLENGER_TOO_MANY_ATTEMPTS,
- NULL);
+ "Address changes exhausted PIN limit for this address\n");
+ return TALER_MHD_redirect_with_oauth_status (
+ hc->connection,
+ bc->client_redirect_uri,
+ "unauthorized_client",
+ "client exceeded authorization attempts limit (too many PINs)",
+ NULL);
}
if (bc->retransmit)
diff --git a/src/challenger/challenger-httpd_common.c b/src/challenger/challenger-httpd_common.c
@@ -1,6 +1,6 @@
/*
This file is part of Challenger
- Copyright (C) 2023 Taler Systems SA
+ Copyright (C) 2023, 2024 Taler Systems SA
Challenger is free software; you can redistribute it and/or modify it under the
terms of the GNU Affero General Public License as published by the Free Software
@@ -157,3 +157,61 @@ TALER_MHD_reply_with_oauth_error (
MHD_destroy_response (resp);
return mret;
}
+
+
+MHD_RESULT
+TALER_MHD_redirect_with_oauth_status (
+ struct MHD_Connection *connection,
+ const char *client_redirect_uri,
+ const char *oauth_error,
+ const char *oauth_error_description,
+ const char *oauth_error_uri)
+{
+ struct MHD_Response *response;
+
+ response = MHD_create_response_from_buffer (strlen (oauth_error),
+ (void *) oauth_error,
+ MHD_RESPMEM_PERSISTENT);
+ if (NULL == response)
+ {
+ GNUNET_break (0);
+ return MHD_NO;
+ }
+ TALER_MHD_add_global_headers (response);
+ GNUNET_break (MHD_YES ==
+ MHD_add_response_header (response,
+ MHD_HTTP_HEADER_CONTENT_TYPE,
+ "text/plain"));
+ {
+ char *url;
+
+ url = TALER_url_join (
+ client_redirect_uri,
+ "",
+ "error", oauth_error,
+ "error_description", oauth_error_description,
+ "error_uri", oauth_error_uri,
+ NULL);
+ if (MHD_NO ==
+ MHD_add_response_header (response,
+ MHD_HTTP_HEADER_LOCATION,
+ url))
+ {
+ GNUNET_break (0);
+ MHD_destroy_response (response);
+ GNUNET_free (url);
+ return MHD_NO;
+ }
+ GNUNET_free (url);
+ }
+
+ {
+ MHD_RESULT ret;
+
+ ret = MHD_queue_response (connection,
+ MHD_HTTP_FOUND,
+ response);
+ MHD_destroy_response (response);
+ return ret;
+ }
+}
diff --git a/src/challenger/challenger-httpd_common.h b/src/challenger/challenger-httpd_common.h
@@ -79,11 +79,31 @@ CH_code_to_nonce (const char *code,
* @return a MHD result code
*/
MHD_RESULT
-TALER_MHD_reply_with_oauth_error (struct MHD_Connection *connection,
- unsigned int http_status,
- const char *oauth_error,
- enum TALER_ErrorCode ec,
- const char *detail);
+TALER_MHD_reply_with_oauth_error (
+ struct MHD_Connection *connection,
+ unsigned int http_status,
+ const char *oauth_error,
+ enum TALER_ErrorCode ec,
+ const char *detail);
+/**
+ * Redirect the client on @a connection to the given
+ * @a client_redirect_uri providing the given OAuth2.0
+ * error details.
+ *
+ * @param connection HTTP request to handle
+ * @param client_redirect_uri base URI where to redirect
+ * @param oauth_error error status to return (e.g. "invalid_scope")
+ * @param oauth_error_description longer description to return, optional, can be NULL
+ * @param oauth_error_uri URI with additional information about the error, optional, can be NULL
+ * @return MHD response queueing status
+ */
+MHD_RESULT
+TALER_MHD_redirect_with_oauth_status (
+ struct MHD_Connection *connection,
+ const char *client_redirect_uri,
+ const char *oauth_error,
+ const char *oauth_error_description,
+ const char *oauth_error_uri);
#endif
diff --git a/src/challenger/challenger-httpd_solve.c b/src/challenger/challenger-httpd_solve.c
@@ -229,6 +229,8 @@ CH_handler_solve (struct CH_HandlerContext *hc,
}
if (! solved)
{
+ // FIXME: if no more attempts remaining,
+ // redirect to server instead!
return TALER_TEMPLATING_reply_error (hc->connection,
"invalid-pin",
MHD_HTTP_FORBIDDEN,
diff --git a/src/challengerdb/Makefile.am b/src/challengerdb/Makefile.am
@@ -20,13 +20,29 @@ if USE_COVERAGE
XLIB = -lgcov
endif
+sqlinputs = \
+ challenger_do_*.sql \
+ procedures.sql.in
+
sqldir = $(prefix)/share/challenger/sql/
sql_DATA = \
versioning.sql \
+ procedures.sql \
challenger-0001.sql \
drop.sql
+BUILT_SOURCES = \
+ procedures.sql
+
+CLEANFILES = \
+ exchange-0002.sql
+
+procedures.sql: procedures.sql.in challenger_do_*.sql
+ chmod +w $@ || true
+ gcc -E -P -undef - < procedures.sql.in 2>/dev/null | sed -e "s/--.*//" | awk 'NF' - >$@
+ chmod ugo-w $@
+
bin_PROGRAMS = \
challenger-dbinit
@@ -96,6 +112,9 @@ TESTS = \
test_challenger_db-postgres
EXTRA_DIST = \
+ $(sqlinputs) \
$(pkgcfg_DATA) \
$(sql_DATA) \
- test_challenger_db_postgres.conf
+ test_challenger_db_postgres.conf \
+ pg_template.h pg_template.c \
+ pg_template.sh
diff --git a/src/challengerdb/challenger_do_challenge_set_address_and_pin.sql b/src/challengerdb/challenger_do_challenge_set_address_and_pin.sql
@@ -0,0 +1,117 @@
+--
+-- This file is part of TALER
+-- Copyright (C) 2024 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/>
+--
+
+
+
+CREATE OR REPLACE FUNCTION challenger_do_challenge_set_address_and_pin (
+ IN in_nonce BYTEA,
+ IN in_address TEXT,
+ IN in_next_tx_time INT8,
+ IN in_now INT8,
+ IN in_tan INT4,
+ OUT out_not_found BOOLEAN,
+ OUT out_last_tx_time INT8,
+ OUT out_last_pin INT4,
+ OUT out_pin_transmit BOOLEAN,
+ OUT out_auth_attempts_left INT4,
+ OUT out_client_redirect_uri TEXT,
+ OUT out_address_refused BOOLEAN)
+LANGUAGE plpgsql
+AS $$
+DECLARE
+ my_status RECORD;
+ my_do_update BOOL;
+BEGIN
+
+my_do_update = FALSE;
+
+SELECT address
+ ,address_attempts_left
+ ,pin_transmissions_left
+ ,last_tx_time
+ ,client_redirect_uri
+ ,last_pin
+ ,auth_attempts_left
+ INTO my_status
+ FROM validations
+ WHERE nonce=in_nonce;
+
+IF NOT FOUND
+THEN
+ out_not_found=TRUE;
+ out_last_tx_time=0;
+ out_last_pin=0;
+ out_pin_transmit=FALSE;
+ out_auth_attempts_left=0;
+ out_client_redirect_uri=NULL;
+ out_address_refused=TRUE;
+ RETURN;
+END IF;
+out_not_found=FALSE;
+out_last_tx_time=my_status.last_tx_time;
+out_last_pin=my_status.last_pin;
+out_pin_transmit=FALSE;
+out_auth_attempts_left=my_status.auth_attempts_left;
+out_client_redirect_uri=my_status.client_redirect_uri;
+
+IF ( (0 == my_status.address_attempts_left) AND
+ (in_address != my_status.address) )
+THEN
+ out_address_refused=TRUE;
+ RETURN;
+END IF;
+out_address_refused=FALSE;
+
+IF (in_address != my_status.address)
+THEN
+ -- we are changing the address, update counters
+ my_status.address_attempts_left = my_status.address_attempts_left - 1;
+ my_status.address = in_address;
+ my_status.pin_transmissions_left = 3;
+ my_status.last_tx_time = 0;
+ my_do_update=TRUE;
+END IF;
+
+IF ( (my_status.pin_transmissions_left > 0) AND
+ (my_status.last_tx_time <= in_next_tx_time) )
+THEN
+ -- we are changing the PIN, update counters
+ my_status.pin_transmissions_left = my_status.pin_transmissions_left - 1;
+ my_status.last_pin = in_tan;
+ my_status.auth_attempts_left = 3;
+ out_auth_attempts_left = 3;
+ out_pin_transmit=TRUE;
+ out_last_pin = in_tan;
+ my_status.last_tx_time = in_now;
+ out_last_tx_time = in_now;
+ my_do_update=TRUE;
+END IF;
+
+IF my_do_update
+THEN
+ UPDATE validations SET
+ address=my_status.address
+ ,address_attempts_left=my_status.address_attempts_left
+ ,pin_transmissions_left=my_status.pin_transmissions_left
+ ,last_tx_time=my_status.last_tx_time
+ ,last_pin=my_status.last_pin
+ ,auth_attempts_left=my_status.auth_attempts_left
+ WHERE nonce=$1;
+END IF;
+
+RETURN;
+
+END $$;
diff --git a/src/challengerdb/pg_challenge_set_address_and_pin.c b/src/challengerdb/pg_challenge_set_address_and_pin.c
@@ -35,7 +35,9 @@ CH_PG_challenge_set_address_and_pin (
uint32_t *tan,
struct GNUNET_TIME_Absolute *last_tx_time,
uint32_t *auth_attempts_left,
- bool *pin_transmit)
+ bool *pin_transmit,
+ char **client_redirect_uri,
+ bool *address_refused)
{
struct PostgresClosure *pg = cls;
struct GNUNET_TIME_Absolute now
@@ -51,7 +53,10 @@ CH_PG_challenge_set_address_and_pin (
GNUNET_PQ_query_param_uint32 (tan),
GNUNET_PQ_query_param_end
};
+ bool not_found;
struct GNUNET_PQ_ResultSpec rs[] = {
+ GNUNET_PQ_result_spec_bool ("not_found",
+ ¬_found),
GNUNET_PQ_result_spec_absolute_time ("last_tx_time",
last_tx_time),
GNUNET_PQ_result_spec_uint32 ("last_pin",
@@ -60,76 +65,36 @@ CH_PG_challenge_set_address_and_pin (
pin_transmit),
GNUNET_PQ_result_spec_uint32 ("auth_attempts_left",
auth_attempts_left),
+ GNUNET_PQ_result_spec_allow_null (
+ GNUNET_PQ_result_spec_string ("client_redirect_uri",
+ client_redirect_uri),
+ NULL),
+ GNUNET_PQ_result_spec_bool ("address_refused",
+ address_refused),
GNUNET_PQ_result_spec_end
};
+ enum GNUNET_DB_QueryStatus qs;
+ *client_redirect_uri = NULL;
PREPARE (pg,
- "challenge_set_address_and_pin",
- "WITH decisions AS ("
- " SELECT "
- " ( (address IS NULL) OR"
- " (address != $2) ) AND"
- " (address_attempts_left > 0)"
- " AS addr_changed"
- " ,( (pin_transmissions_left > 0) OR"
- " (address_attempts_left > 0) ) AND"
- " ( (address IS NULL) OR"
- " (address != $2) OR"
- " (last_tx_time < $3) ) AS send_pin"
- " FROM validations"
- " WHERE nonce=$1"
- "),"
- "result AS ("
- "UPDATE validations SET"
- " address_attempts_left=CASE"
- " WHEN (SELECT addr_changed FROM decisions)"
- " THEN address_attempts_left - 1 "
- " ELSE address_attempts_left "
- " END"
- " ,last_pin = CASE "
- " WHEN (SELECT addr_changed FROM decisions)"
- " THEN $5"
- " ELSE last_pin"
- " END"
- " ,pin_transmissions_left=CASE"
- " WHEN (SELECT addr_changed FROM decisions)"
- " THEN 3 "
- " ELSE CASE"
- " WHEN (SELECT send_pin FROM decisions)"
- " THEN pin_transmissions_left - 1"
- " ELSE pin_transmissions_left"
- " END"
- " END"
- " ,auth_attempts_left=CASE"
- " WHEN (SELECT addr_changed FROM decisions)"
- " THEN 3 "
- " ELSE auth_attempts_left"
- " END"
- " ,last_tx_time=CASE"
- " WHEN (SELECT send_pin FROM decisions)"
- " THEN $4"
- " ELSE last_tx_time"
- " END"
- " ,address=CASE"
- " WHEN (SELECT addr_changed FROM decisions)"
- " THEN $2"
- " ELSE address"
- " END"
- " WHERE nonce=$1"
- " RETURNING"
- " last_tx_time"
- " ,last_pin"
- " ,auth_attempts_left"
- ")"
- " SELECT"
- " last_tx_time"
- " ,decisions.send_pin AS pin_transmit"
- " ,last_pin"
- " ,auth_attempts_left"
- " FROM result"
- " FULL OUTER JOIN decisions ON (TRUE);");
- return GNUNET_PQ_eval_prepared_singleton_select (pg->conn,
- "challenge_set_address_and_pin",
- params,
- rs);
+ "do_challenge_set_address_and_pin",
+ "SELECT "
+ " out_not_found AS not_found"
+ ",out_last_tx_time AS last_tx_time"
+ ",out_pin_transmit AS pin_transmit"
+ ",out_last_pin AS last_pin"
+ ",out_auth_attempts_left AS auth_attempts_left"
+ ",out_client_redirect_uri AS client_redirect_uri"
+ ",out_address_refused AS address_refused"
+ " FROM challenger_do_challenge_set_address_and_pin"
+ " ($1,$2,$3,$4,$5);");
+ qs = GNUNET_PQ_eval_prepared_singleton_select (pg->conn,
+ "challenge_set_address_and_pin",
+ params,
+ rs);
+ if (qs <= 0)
+ return qs;
+ if (not_found)
+ return GNUNET_DB_STATUS_SUCCESS_NO_RESULTS;
+ return qs;
}
diff --git a/src/challengerdb/pg_challenge_set_address_and_pin.h b/src/challengerdb/pg_challenge_set_address_and_pin.h
@@ -40,6 +40,8 @@
* @param[out] last_tx_time set to the last time when we (presumably) send a PIN to @a address, input should be current time to use if the existing value for tx_time is past @a next_tx_time
* @param[out] pin_transmit set to true if we should transmit the @a last_pin to the @a address
* @param[out] auth_attempts_left set to number of attempts the user has left on this pin
+ * @param[out] client_redirect_uri redirection URI of the client (for reporting failures)
+ * @param[out] address_refused set to true if the address was refused (address change attempts exhausted)
* @return transaction status:
* #GNUNET_DB_STATUS_SUCCESS_ONE_RESULT if the address was changed
* #GNUNET_DB_STATUS_SUCCESS_NO_RESULTS if we do not permit further changes to the address (attempts exhausted)
@@ -54,7 +56,8 @@ CH_PG_challenge_set_address_and_pin (
uint32_t *tan,
struct GNUNET_TIME_Absolute *last_tx_time,
uint32_t *auth_attempts_left,
- bool *pin_transmit);
-
+ bool *pin_transmit,
+ char **client_redirect_uri,
+ bool *address_refused);
#endif
diff --git a/src/challengerdb/plugin_challengerdb_postgres.c b/src/challengerdb/plugin_challengerdb_postgres.c
@@ -322,6 +322,7 @@ postgres_create_tables (void *cls)
GNUNET_PQ_make_execute ("SET search_path TO challenger;"),
GNUNET_PQ_EXECUTE_STATEMENT_END
};
+ enum GNUNET_GenericReturnValue ret;
conn = GNUNET_PQ_connect_with_cfg (pc->cfg,
"challengerdb-postgres",
@@ -330,8 +331,10 @@ postgres_create_tables (void *cls)
NULL);
if (NULL == conn)
return GNUNET_SYSERR;
+ ret = GNUNET_PQ_exec_sql (conn,
+ "procedures");
GNUNET_PQ_disconnect (conn);
- return GNUNET_OK;
+ return ret;
}
diff --git a/src/challengerdb/procedures.sql.in b/src/challengerdb/procedures.sql.in
@@ -0,0 +1,23 @@
+--
+-- This file is part of TALER
+-- Copyright (C) 2024 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/>
+--
+
+BEGIN;
+
+SET search_path TO challenger;
+
+#include "challenger_do_challenge_set_address_and_pin.sql"
+
+COMMIT;
diff --git a/src/include/challenger_database_plugin.h b/src/include/challenger_database_plugin.h
@@ -261,6 +261,8 @@ struct CHALLENGER_DatabasePlugin
* @param[out] last_tx_time set to the last time when we (presumably) send a PIN to @a address, input should be current time to use if the existing value for tx_time is past @a next_tx_time
* @param[out] pin_transmit set to true if we should transmit the @a last_pin to the @a address
* @param[out] auth_attempts_left set to number of attempts the user has left on this pin
+ * @param[out] client_redirect_uri redirection URI of the client (for reporting failures)
+ * @param[out] address_refused set to true if the address was refused (address change attempts exhausted)
* @return transaction status:
* #GNUNET_DB_STATUS_SUCCESS_ONE_RESULT if the address was changed
* #GNUNET_DB_STATUS_SUCCESS_NO_RESULTS if we do not permit further changes to the address (attempts exhausted)
@@ -275,7 +277,9 @@ struct CHALLENGER_DatabasePlugin
uint32_t *tan,
struct GNUNET_TIME_Absolute *last_tx_time,
uint32_t *auth_attempts_left,
- bool *pin_transmit);
+ bool *pin_transmit,
+ char **client_redirect_uri,
+ bool *address_refused);
/**
