diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/exchange/taler-exchange-httpd.c | 12 | ||||
| -rw-r--r-- | src/exchange/taler-exchange-httpd.h | 8 | ||||
| -rw-r--r-- | src/exchange/taler-exchange-httpd_keystate.c | 291 | ||||
| -rw-r--r-- | src/include/taler_error_codes.h | 7 | ||||
| -rw-r--r-- | src/testing/testing_api_helpers_exchange.c | 1 |
5 files changed, 213 insertions, 106 deletions
diff --git a/src/exchange/taler-exchange-httpd.c b/src/exchange/taler-exchange-httpd.c index 4095d00fa..0754163bf 100644 --- a/src/exchange/taler-exchange-httpd.c +++ b/src/exchange/taler-exchange-httpd.c @@ -83,6 +83,14 @@ char *TEH_exchange_directory; char *TEH_revocation_directory; /** + * Are clients allowed to request /keys for times other than the + * current time? Allowing this could be abused in a DoS-attack + * as building new /keys responses is expensive. Should only be + * enabled for testcases, development and test systems. + */ +int TEH_allow_keys_timetravel; + +/** * The exchange's configuration (global) */ struct GNUNET_CONFIGURATION_Handle *TEH_cfg; @@ -1183,6 +1191,10 @@ main (int argc, char *logfile = NULL; int connection_close = GNUNET_NO; const struct GNUNET_GETOPT_CommandLineOption options[] = { + GNUNET_GETOPT_option_flag ('a', + "allow-timetravel", + "allow clients to request /keys for arbitrary timestamps (for testing and development only)", + &TEH_allow_keys_timetravel), GNUNET_GETOPT_option_flag ('C', "connection-close", "force HTTP connections to be closed after each request", diff --git a/src/exchange/taler-exchange-httpd.h b/src/exchange/taler-exchange-httpd.h index 137cee720..512fae8f0 100644 --- a/src/exchange/taler-exchange-httpd.h +++ b/src/exchange/taler-exchange-httpd.h @@ -44,6 +44,14 @@ extern struct GNUNET_CONFIGURATION_Handle *TEH_cfg; extern char *TEH_exchange_directory; /** + * Are clients allowed to request /keys for times other than the + * current time? Allowing this could be abused in a DoS-attack + * as building new /keys responses is expensive. Should only be + * enabled for testcases, development and test systems. + */ +extern int TEH_allow_keys_timetravel; + +/** * Main directory with revocation data. */ extern char *TEH_revocation_directory; diff --git a/src/exchange/taler-exchange-httpd_keystate.c b/src/exchange/taler-exchange-httpd_keystate.c index 5bc893247..4117c13a0 100644 --- a/src/exchange/taler-exchange-httpd_keystate.c +++ b/src/exchange/taler-exchange-httpd_keystate.c @@ -121,7 +121,7 @@ struct KeysResponseData { /** - * Response to return if the client supports (gzip) compression. + * Response to return if the client supports (deflate) compression. */ struct MHD_Response *response_compressed; @@ -212,14 +212,14 @@ struct ResponseFactoryContext struct TEH_KS_StateHandle *key_state; /** - * Length of the @e denomkey_array. + * Time stamp used as "now". */ - unsigned int denomkey_array_length; + struct GNUNET_TIME_Absolute now; /** - * Time stamp used as "now". + * Length of the @e denomkey_array. */ - struct GNUNET_TIME_Absolute now; + unsigned int denomkey_array_length; }; @@ -249,7 +249,7 @@ struct TEH_KS_StateHandle struct GNUNET_CONTAINER_MultiHashMap *revoked_map; /** - * Sorted array of responses to /keys (sorted by cherry-picking date) of + * Sorted array of responses to /keys (MUST be sorted by cherry-picking date) of * length @e krd_array_length; */ struct KeysResponseData *krd_array; @@ -309,6 +309,12 @@ static struct TEH_KS_StateHandle *internal_key_state; */ static pthread_mutex_t internal_key_state_mutex = PTHREAD_MUTEX_INITIALIZER; +/** + * Configuration value LOOKAHEAD_PROVIDE that says for how far in the + * future we want to provide exchange key information to clients. + */ +static struct GNUNET_TIME_Relative conf_duration_provide; + /* ************************** Clean up logic *********************** */ @@ -368,6 +374,11 @@ destroy_response_builder (struct ResponseBuilderContext *rbc) json_decref (rbc->auditors_array); rbc->auditors_array = NULL; } + if (NULL != rbc->hash_context) + { + GNUNET_CRYPTO_hash_context_abort (rbc->hash_context); + rbc->hash_context = NULL; + } } @@ -447,7 +458,7 @@ ks_free (struct TEH_KS_StateHandle *key_state) /** * Pipe used for signaling reloading of our key state. */ -static int reload_pipe[2]; +static int reload_pipe[2] = { -1, -1 }; /** @@ -458,23 +469,15 @@ static int reload_pipe[2]; static void handle_signal (int signal_number) { - ssize_t res; - char c = signal_number; - - res = write (reload_pipe[1], - &c, - 1); - if ( (res < 0) && - (EINTR != errno) ) - { - GNUNET_break (0); - return; - } - if (0 == res) - { - GNUNET_break (0); - return; - } + char c = (char) signal_number; /* never seen a signal_number > 127 on any platform */ + + (void) ! write (reload_pipe[1], + &c, + 1); + /* While one might like to "handle errors" here, even logging via fprintf() + isn't safe inside of a signal handler. So there is nothing we safely CAN + do. OTOH, also very little that can go wrong in pratice. Calling _exit() + on errors might be a possibility, but that might do more harm than good. */// } @@ -482,11 +485,10 @@ handle_signal (int signal_number) /** - * Convert the public part of a denomination key issue to a JSON - * object. + * Convert the public part of denomination key data to a JSON object. * - * @param pk public key of the denomination key - * @param dki the denomination key issue + * @param pk public key of the denomination + * @param dki the denomination key issue information * @return a JSON object describing the denomination key isue (public part) */ static json_t * @@ -511,7 +513,9 @@ denom_key_issue_to_json ( TALER_amount_ntoh (&fee_refund, &dki->properties.fee_refund); return - json_pack ("{s:o, s:o, s:o, s:o, s:o, s:o, s:o, s:o, s:o, s:o, s:o}", + json_pack ("{s:o, s:o, s:o, s:o, s:o," + " s:o, s:o, s:o, s:o, s:o," + " s:o}", "master_sig", GNUNET_JSON_from_data_auto (&dki->signature), "stamp_start", @@ -526,6 +530,7 @@ denom_key_issue_to_json ( "stamp_expire_legal", GNUNET_JSON_from_time_abs (GNUNET_TIME_absolute_ntoh ( dki->properties.expire_legal)), + /* 5 entries until here */ "denom_pub", GNUNET_JSON_from_rsa_public_key (pk->rsa_public_key), "value", @@ -536,6 +541,7 @@ denom_key_issue_to_json ( TALER_JSON_from_amount (&fee_deposit), "fee_refresh", TALER_JSON_from_amount (&fee_refresh), + /* 10 entries until here */ "fee_refund", TALER_JSON_from_amount (&fee_refund)); } @@ -553,20 +559,37 @@ static int store_in_map (struct GNUNET_CONTAINER_MultiHashMap *map, const struct TALER_EXCHANGEDB_DenominationKey *dki) { - struct TALER_EXCHANGEDB_DenominationKey *d2; - int res; - + /* First, we verify that the @a dki is actually well-formed. While it comes + from our own hard disk, there is the possibility of missconfiguration + (i.e. bogus file in the directory), or that the administrator used the + wrong master public key, and we should not accept entries that are not + well-formed. */// { const struct TALER_EXCHANGEDB_DenominationKeyInformationP *dkip; struct TALER_DenominationKeyValidityPS denom_key_issue; dkip = &dki->issue; denom_key_issue = dkip->properties; + /* The above is straight from our disk. We should not trust + that it is well-formed, so we setup crucial fields ourselves. */ denom_key_issue.purpose.purpose = htonl (TALER_SIGNATURE_MASTER_DENOMINATION_KEY_VALIDITY); denom_key_issue.purpose.size = htonl (sizeof (struct TALER_DenominationKeyValidityPS)); denom_key_issue.master = TEH_master_public_key; + + /* Check that the data we read matches our expectations */ + if (0 != + GNUNET_memcmp (&denom_key_issue, + &dkip->properties)) + { + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "Invalid fields in denomination key `%s'\n", + GNUNET_h2s (&dkip->properties.denom_hash)); + return GNUNET_SYSERR; + } + + /* Also check the signature by the master public key */ if (GNUNET_SYSERR == GNUNET_CRYPTO_eddsa_verify ( TALER_SIGNATURE_MASTER_DENOMINATION_KEY_VALIDITY, @@ -581,27 +604,36 @@ store_in_map (struct GNUNET_CONTAINER_MultiHashMap *map, } } - d2 = GNUNET_new (struct TALER_EXCHANGEDB_DenominationKey); - d2->issue = dki->issue; - if (NULL != dki->denom_priv.rsa_private_key) - d2->denom_priv.rsa_private_key - = GNUNET_CRYPTO_rsa_private_key_dup (dki->denom_priv.rsa_private_key); - d2->denom_pub.rsa_public_key - = GNUNET_CRYPTO_rsa_public_key_dup (dki->denom_pub.rsa_public_key); - res = GNUNET_CONTAINER_multihashmap_put (map, + /* We need to make a deep copy of the @a dki, as the original was allocated + elsewhere and will be freed by the caller. */ + { + struct TALER_EXCHANGEDB_DenominationKey *d2; + + d2 = GNUNET_new (struct TALER_EXCHANGEDB_DenominationKey); + d2->issue = dki->issue; + if (GNUNET_OK != + GNUNET_CONTAINER_multihashmap_put (map, &d2->issue.properties.denom_hash, d2, - GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY); - if (GNUNET_OK != res) - { - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "Duplicate denomination key `%s'\n", - GNUNET_h2s (&d2->issue.properties.denom_hash)); - if (NULL != d2->denom_priv.rsa_private_key) - GNUNET_CRYPTO_rsa_private_key_free (d2->denom_priv.rsa_private_key); - GNUNET_CRYPTO_rsa_public_key_free (d2->denom_pub.rsa_public_key); - GNUNET_free (d2); - return GNUNET_NO; + GNUNET_CONTAINER_MULTIHASHMAPOPTION_UNIQUE_ONLY)) + { + GNUNET_log (GNUNET_ERROR_TYPE_WARNING, + "Duplicate denomination key `%s'\n", + GNUNET_h2s (&d2->issue.properties.denom_hash)); + GNUNET_free (d2); + return GNUNET_NO; + } + + /* finish *deep* part of deep copy */ + if (NULL != dki->denom_priv.rsa_private_key) + { + /* we might be past the withdraw period, so private key could have been deleted, + so only try to (deep) copy if non-NULL. */ + d2->denom_priv.rsa_private_key + = GNUNET_CRYPTO_rsa_private_key_dup (dki->denom_priv.rsa_private_key); + } + d2->denom_pub.rsa_public_key + = GNUNET_CRYPTO_rsa_public_key_dup (dki->denom_pub.rsa_public_key); } return GNUNET_OK; } @@ -625,33 +657,6 @@ struct AddRevocationContext /** - * Get the relative time value that describes how - * far in the future do we want to provide coin keys. - * - * @return the provide duration - */ -static struct GNUNET_TIME_Relative -TALER_EXCHANGE_conf_duration_provide (void) -{ - struct GNUNET_TIME_Relative rel; - - if (GNUNET_OK != - GNUNET_CONFIGURATION_get_value_time (TEH_cfg, - "exchange", - "LOOKAHEAD_PROVIDE", - &rel)) - { - GNUNET_log_config_invalid (GNUNET_ERROR_TYPE_ERROR, - "exchange", - "LOOKAHEAD_PROVIDE", - "time value required"); - GNUNET_assert (0); - } - return rel; -} - - -/** * Execute transaction to add revocations. * * @param cls closure with the `struct AddRevocationContext *` @@ -771,7 +776,7 @@ reload_keys_denom_iter (void *cls, } horizon = GNUNET_TIME_absolute_add (rfc->now, - TALER_EXCHANGE_conf_duration_provide ()); + conf_duration_provide); start = GNUNET_TIME_absolute_ntoh (dki->issue.properties.start); if (start.abs_value_us > horizon.abs_value_us) { @@ -944,8 +949,7 @@ reload_keys_sign_iter ( struct GNUNET_TIME_Absolute now; struct GNUNET_TIME_Absolute horizon; - horizon = GNUNET_TIME_relative_to_absolute ( - TALER_EXCHANGE_conf_duration_provide ()); + horizon = GNUNET_TIME_relative_to_absolute (conf_duration_provide); if (GNUNET_TIME_absolute_ntoh (ski->issue.start).abs_value_us > horizon.abs_value_us) { @@ -1127,7 +1131,9 @@ initialize_denomkey_array (void *cls, /** * Comparator used to sort the `struct DenominationKeyEntry` array - * by the validity period's starting time of the keys. + * by the validity period's starting time of the keys. Must match + * the expected sorting by cherry_pick_date (which is based on the + * issue.properties.start) used in #krd_search_comparator. * * @param k1 a `struct DenominationKeyEntry *` * @param k2 a `struct DenominationKeyEntry *` @@ -1460,7 +1466,6 @@ build_keys_response (const struct ResponseFactoryContext *rfc, &free_auditor_entry, NULL); GNUNET_CONTAINER_multihashmap_destroy (auditors); - GNUNET_CRYPTO_hash_context_abort (rbc.hash_context); return GNUNET_SYSERR; } } @@ -1822,6 +1827,8 @@ make_fresh_key_state (struct GNUNET_TIME_Absolute now) if (last.abs_value_us == d.abs_value_us) continue; + /* must be monotonically increasing as per qsort() call above: */ + GNUNET_assert (last.abs_value_us < d.abs_value_us); last = d; off++; } @@ -2277,11 +2284,31 @@ static struct GNUNET_SIGNAL_Context *sigchld; /** - * Setup initial #internal_key_state. + * Setup initial #internal_key_state and our signal handlers. */ int TEH_KS_init (void) { + if (GNUNET_OK != + GNUNET_CONFIGURATION_get_value_time (TEH_cfg, + "exchange", + "LOOKAHEAD_PROVIDE", + &conf_duration_provide)) + { + GNUNET_log_config_invalid (GNUNET_ERROR_TYPE_ERROR, + "exchange", + "LOOKAHEAD_PROVIDE", + "time value required"); + return GNUNET_SYSERR; + } + if (0 == conf_duration_provide.rel_value_us) + { + GNUNET_log_config_invalid (GNUNET_ERROR_TYPE_ERROR, + "exchange", + "LOOKAHEAD_PROVIDE", + "value cannot be zero"); + return GNUNET_SYSERR; + } if (0 != pipe (reload_pipe)) { GNUNET_log_strerror (GNUNET_ERROR_TYPE_ERROR, @@ -2312,7 +2339,7 @@ TEH_KS_init (void) /** - * Finally release #internal_key_state. + * Finally release #internal_key_state and our signal handlers. */ void TEH_KS_free (void) @@ -2322,18 +2349,43 @@ TEH_KS_free (void) /* Note: locking is no longer be required, as we are again single-threaded. */ ks = internal_key_state; - if (NULL == ks) - return; - GNUNET_assert (1 == ks->refcnt); - ks->refcnt--; - ks_free (ks); - GNUNET_SIGNAL_handler_uninstall (sigusr1); - GNUNET_SIGNAL_handler_uninstall (sigterm); - GNUNET_SIGNAL_handler_uninstall (sigint); - GNUNET_SIGNAL_handler_uninstall (sighup); - GNUNET_SIGNAL_handler_uninstall (sigchld); - GNUNET_break (0 == close (reload_pipe[0])); - GNUNET_break (0 == close (reload_pipe[1])); + if (NULL != ks) + { + GNUNET_assert (1 == ks->refcnt); + ks->refcnt--; + ks_free (ks); + } + if (NULL != sigusr1) + { + GNUNET_SIGNAL_handler_uninstall (sigusr1); + sigusr1 = NULL; + } + if (NULL != sigterm) + { + GNUNET_SIGNAL_handler_uninstall (sigterm); + sigterm = NULL; + } + if (NULL != sigint) + { + GNUNET_SIGNAL_handler_uninstall (sigint); + sigint = NULL; + } + if (NULL != sighup) + { + GNUNET_SIGNAL_handler_uninstall (sighup); + sighup = NULL; + } + if (NULL != sigchld) + { + GNUNET_SIGNAL_handler_uninstall (sigchld); + sigchld = NULL; + } + if (-1 != reload_pipe[0]) + { + GNUNET_break (0 == close (reload_pipe[0])); + GNUNET_break (0 == close (reload_pipe[1])); + reload_pipe[0] = reload_pipe[1] = -1; + } } @@ -2356,10 +2408,11 @@ TEH_KS_sign (const struct GNUNET_CRYPTO_EccSignaturePurpose *purpose, key_state = TEH_KS_acquire (GNUNET_TIME_absolute_get ()); if (NULL == key_state) { - /* This *can* happen if the exchange's keys are - not properly maintained. */ + /* This *can* happen if the exchange's keys are not properly maintained + (i.e. administrator forgets to provision us with non-expired signing + keys or to send signal to reload keys after provisioning). */ GNUNET_log (GNUNET_ERROR_TYPE_ERROR, - "Cannot sign request, no valid keys available\n"); + "Cannot sign request, no valid signing keys available. Were they properly provisioned and did you signal the exchange to reload the keys?\n"); return GNUNET_SYSERR; } *pub = key_state->current_sign_key_issue.issue.signkey_pub; @@ -2374,8 +2427,8 @@ TEH_KS_sign (const struct GNUNET_CRYPTO_EccSignaturePurpose *purpose, /** - * Comparator used for a binary search for @a key in the - * `struct KeysResponseData` array. + * Comparator used for a binary search by cherry_pick_date for @a key in the + * `struct KeysResponseData` array. See libc's qsort() and bsearch() functions. * * @param key pointer to a `struct GNUNET_TIME_Absolute` * @param value pointer to a `struct KeysResponseData` array entry @@ -2437,6 +2490,10 @@ TEH_handler_keys (const struct TEH_RequestHandler *rh, TALER_EC_KEYS_HAVE_NOT_NUMERIC, "last_issue_date"); } + /* The following multiplication may overflow; but this should not really + be a problem, as giving back 'older' data than what the client asks for + (given that the client asks for data in the distant future) is not + problematic */ last_issue_date.abs_value_us = (uint64_t) cherrypickn * 1000000LLU; } else @@ -2458,11 +2515,26 @@ TEH_handler_keys (const struct TEH_RequestHandler *rh, { GNUNET_break_op (0); return TALER_MHD_reply_with_error (connection, - MHD_HTTP_BAD_REQUEST, + MHD_HTTP_FORBIDDEN, TALER_EC_KEYS_HAVE_NOT_NUMERIC, "now"); } - now.abs_value_us = (uint64_t) fakenown * 1000000LLU; + if (TEH_allow_keys_timetravel) + { + /* The following multiplication may overflow; but this should not really + be a problem, as giving back 'older' data than what the client asks for + (given that the client asks for data in the distant future) is not + problematic */ + now.abs_value_us = (uint64_t) fakenown * 1000000LLU; + } + else + { + /* Option not allowed by configuration */ + return TALER_MHD_reply_with_error (connection, + MHD_HTTP_BAD_REQUEST, + TALER_EC_KEYS_TIMETRAVEL_FORBIDDEN, + "timetravel not allowed by this exchange"); + } } GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Handling request for /keys (%s/%s)\n", @@ -2474,11 +2546,14 @@ TEH_handler_keys (const struct TEH_RequestHandler *rh, key_state = TEH_KS_acquire (now); if (NULL == key_state) { - TALER_LOG_ERROR ("Lacking keys to operate\n"); + /* Maybe client picked time stamp too far in the future? In that case, + #MHD_HTTP_INTERNAL_SERVER_ERROR might be missleading, could be more like a + NOT_FOUND situation. But, OTOH, for 'sane' clients it is more likely + to be our fault, so let's speculatively assume we are to blame ;-) */ return TALER_MHD_reply_with_error (connection, MHD_HTTP_INTERNAL_SERVER_ERROR, TALER_EC_EXCHANGE_BAD_CONFIGURATION, - "no keys"); + "no keys for requested time"); } krd = bsearch (&last_issue_date, key_state->krd_array, @@ -2501,6 +2576,10 @@ TEH_handler_keys (const struct TEH_RequestHandler *rh, } if (NULL == krd) { + /* Maybe client picked time stamp too far in the future? In that case, + "INTERNAL_SERVER_ERROR" might be missleading, could be more like a + NOT_FOUND situation. But, OTOH, for 'sane' clients it is more likely + to be our fault, so let's speculatively assume we are to blame ;-) */// GNUNET_break (0); TEH_KS_release (key_state); return TALER_MHD_reply_with_error (connection, diff --git a/src/include/taler_error_codes.h b/src/include/taler_error_codes.h index 9a7c544b9..0969261a8 100644 --- a/src/include/taler_error_codes.h +++ b/src/include/taler_error_codes.h @@ -1052,6 +1052,13 @@ enum TALER_ErrorCode TALER_EC_KEYS_MISSING = 1901, /** + * This exchange does not allow clients to request /keys for times + * other than the current (exchange) time. This reponse is provied + * with an HTTP status code of MHD_HTTP_FORBIDDEN. + */ + TALER_EC_KEYS_TIMETRAVEL_FORBIDDEN = 1902, + + /** * The backend could not find the merchant instance specified in the * request. This response is provided with HTTP status code * MHD_HTTP_NOT_FOUND. diff --git a/src/testing/testing_api_helpers_exchange.c b/src/testing/testing_api_helpers_exchange.c index 967a4efb4..9f5716849 100644 --- a/src/testing/testing_api_helpers_exchange.c +++ b/src/testing/testing_api_helpers_exchange.c @@ -762,6 +762,7 @@ TALER_TESTING_setup_with_exchange_cfg (void *cls, NULL, NULL, NULL, "taler-exchange-httpd", "taler-exchange-httpd", + "-a", /* some tests may need timetravel */ "-c", setup_ctx->config_filename, NULL); |