commit ba03c8e56c18cdceac81ce132d38e80a4193e7eb parent 37fcba7e895f0e6e5d5f9d1a7fa990d2aedaecce Author: Christian Grothoff <christian@grothoff.org> Date: Thu, 30 Jul 2026 20:38:10 +0200 fix GC, document major NEWS Diffstat:
37 files changed, 502 insertions(+), 264 deletions(-)
diff --git a/NEWS b/NEWS @@ -0,0 +1,71 @@ +Noteworthy changes in the next release +====================================== + +* INCOMPATIBLE CHANGE, backups made by earlier C clients cannot be + recovered. + + derive_key() used to pass the key derivation context to GNUnet's KDF as a + *pointer*, so only the first byte of each context string was mixed in. + Every key derived by anastasis_encrypt()/anastasis_decrypt() was therefore + computed from the wrong input: recovery documents ("erd"), key shares + ("eks"), truths ("ect"), per-policy master keys ("emk"), core secrets + ("cse") and metadata ("rmd") -- and, for the secure-question method, only + the first character of the answer took part in the derivation. + + The full context string is now used, which is what + doc/sphinx/cryptography.rst always specified and what the TypeScript + implementation has always done, so this makes the C code agree with the + deployed protocol rather than changing it. The two are not compatible: + data written by an earlier C client cannot be read by this one, and no + fallback read path is provided. + + Recovery of such a backup fails with "failed to decrypt policy document + with the given identity attributes". There is no way for the client to + tell that case apart from genuinely mistyped identity attributes, since + the ciphertext looks the same either way -- the message names both + possibilities for that reason. + + The protocol version was bumped for this, so a client and a provider that + disagree about it refuse each other rather than failing later during + recovery. Providers must be upgraded together with the clients that use + them, and the TypeScript client must be on a version speaking the same + protocol. + +* The provider REST API is now at protocol version 2:0:1. Clients that + advertise protocol version 0 (i.e. anything predating the key derivation + change above) are refused. + +* libanastasisutil has a new soname (libanastasisutil.so.1). + ANASTASIS_CRYPTO_recovery_document_decrypt(), _keyshare_decrypt(), + _truth_decrypt() and _core_secret_recover() changed from returning void to + returning enum GNUNET_GenericReturnValue: they used to ignore decryption + failures and then abort() on the result, so a malicious provider could + crash a recovering client. Out-of-tree users of these functions -- + notably anastasis-gtk -- must be updated and rebuilt. + +* Providers upgrading an existing database must run anastasis-dbinit to pick + up the stasis-0002 migration; it replaces the challenge-code index with + one garbage collection can actually use. + +* Garbage collection now runs as a single stored procedure and actually + reclaims data: expired accounts with their recovery documents and + payments, expired truths with their challenge codes and payments, + challenge payments whose truth is gone, and stale IBAN authentication + transfers. Previously it aborted with a foreign key violation for any + account that had ever uploaded a recovery document, i.e. in the normal + case, and reclaimed nothing. How long expired data is kept is now + configurable via GC_BACKUP_GRACE and GC_PAYMENT_RETENTION. + +* The build is hardened by default (PIE, RELRO/BIND_NOW, _FORTIFY_SOURCE, + stack protector); use ./configure --disable-hardening for profiling or + debugging. + +* The "file" authorization method, which is meant for testing, used the + client-supplied truth directly as the path it wrote the challenge code to. + It now only writes inside the directory configured as + "[authorization-file] DIRECTORY". + +* Albania: identity numbers were rejected outright, because the validation + regular expression was missing the day field and so could only match an + 8-character number where a real one has 10. There is still no check-digit + validation for Albania. diff --git a/RELEASE.md b/RELEASE.md @@ -2,7 +2,21 @@ ## Checklist -- [ ] bump version in configure.ac +- [ ] bump version in meson.build (`project(... version:)`; the tag is what + `scripts/get_version.sh` reports) +- [ ] if `ANASTASIS_PROTOCOL_CURRENT` (src/restclient/anastasis_api_config.c) + or the version served by `/config` + (src/backend/anastasis-httpd_config.c) changed, say so in NEWS, state + whether stored data is still readable, and confirm which + taler-typescript-core release speaks the same protocol version -- + the C and TypeScript clients have to bump in lock-step +- [ ] if a public library's ABI changed, bump its entry in `libltversions` + in meson.build, and say so in NEWS (out-of-tree users such as + anastasis-gtk need to know) +- [ ] if `src/stasis/stasis-*.sql` gained a migration, confirm it was tested + against a database created by the *previous* release, not only against + a fresh one +- [ ] add entry to NEWS - [ ] add entry to ChangeLog - [ ] add entry to debian/changelog - [ ] check CI (contrib/ci, buildbot.taler.net) diff --git a/configure b/configure @@ -1,7 +1,10 @@ #!/bin/sh pkg_name="anastasis" -pkg_default_features="" +# Features in pkg_optional_features default to OFF, those in +# pkg_default_features to ON; hardening therefore belongs in the latter, and is +# turned off with --disable-hardening. +pkg_default_features="hardening" pkg_optional_features="coverage logging only-doc install-rpath" pkg_optional_dependencies="" diff --git a/contrib/redux.al.json b/contrib/redux.al.json @@ -23,8 +23,7 @@ "tooltip": "Numri i Identitetit", "widget": "anastasis_gtk_ia_nid_al", "uuid": "256e5d30-d65e-481b-9ac4-55f5ac03b24a", - "validation-regex": "^[0-9A-T][0-9](((0|5)[0-9])|10|11|51|52)[0-9]{3}[A-W]$", - "validation-logic": "AL_NID_check" + "validation-regex": "^[0-9A-T][0-9](0[1-9]|1[0-2]|5[1-9]|6[0-2])(0[1-9]|[12][0-9]|3[01])[0-9]{3}[A-W]$" } ] } diff --git a/meson.build b/meson.build @@ -67,18 +67,25 @@ if not get_option('only-doc') '-fstack-clash-protection', ) # _FORTIFY_SOURCE needs optimisation to do anything and warns without - # it, so only set it when the build is actually optimised. - if get_option('optimization') not in ['0', 'g'] + # it, so only set it when the build is actually optimised. 'plain' is + # excluded as well: it means the packager supplies the flags, and + # distributions that pass their own -D_FORTIFY_SOURCE would then get a + # "redefined" warning in every translation unit. + if get_option('optimization') not in ['0', 'g', 'plain'] + # The obvious test, '#if _FORTIFY_SOURCE < 3', can never fail: it + # reads back the value just put on the command line. glibc does + # not lower _FORTIFY_SOURCE when it cannot honour the level, it + # clamps __USE_FORTIFY_LEVEL and emits a #warning -- hence both the + # macro tested here and the -Werror that turns that warning into + # the failure it should be. A libc that defines neither is treated + # as not supporting level 3. if cc.compiles( '''#include <string.h> - #if __has_include(<features.h>) - #include <features.h> - #endif - #if _FORTIFY_SOURCE < 3 + #if !defined(__USE_FORTIFY_LEVEL) || __USE_FORTIFY_LEVEL < 3 #error no level 3 #endif int main(void) { return 0; }''', - args: ['-O2', '-D_FORTIFY_SOURCE=3'], + args: ['-O2', '-Werror', '-D_FORTIFY_SOURCE=3'], name: '_FORTIFY_SOURCE=3', ) hardening_cflags += '-D_FORTIFY_SOURCE=3' @@ -88,10 +95,7 @@ if not get_option('only-doc') endif add_project_arguments(hardening_cflags, language: 'c') add_project_link_arguments( - cc.get_supported_link_arguments( - '-Wl,-z,relro', - '-Wl,-z,now', - ), + cc.get_supported_link_arguments('-Wl,-z,relro', '-Wl,-z,now'), language: 'c', ) endif @@ -120,10 +124,6 @@ if not get_option('only-doc') if not zlib_dep.found() zlib_dep = cc.find_library('z', required: true) endif - dl_dep = dependency('libdl', required: false) - if not dl_dep.found() - dl_dep = cc.find_library('dl', required: true) - endif m_dep = cc.find_library('m', required: false) if m_dep.found() private_config.set('HAVE_LIBM', 1) diff --git a/src/authorization/anastasis_authorization_plugin_email.c b/src/authorization/anastasis_authorization_plugin_email.c @@ -189,6 +189,18 @@ get_message (const json_t *messages, return MISSING_MESSAGE; } GNUNET_JSON_parse_free (spec); + if (NULL == ret) + { + /* The parser of TALER_JSON_spec_i18n_string returns #GNUNET_OK even + when the field is absent or is not a string, in which case it stores + NULL; the check above therefore never fires for a missing message and + this one is what keeps NULL out of the format argument of the + GNUNET_asprintf() calls below. */ + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "Message `%s' is missing or not a string in the configured messages file\n", + msg_id); + return MISSING_MESSAGE; + } return ret; } } diff --git a/src/authorization/anastasis_authorization_plugin_file.c b/src/authorization/anastasis_authorization_plugin_file.c @@ -31,6 +31,81 @@ /** + * Context of the plugin, shared by all challenges it serves. + */ +struct FileContext +{ + /** + * Configuration we are using. + */ + const struct ANASTASIS_AuthorizationContext *ac; + + /** + * Directory challenge files are written to. Always ends in '/'. + */ + char *directory; +}; + + +/** + * Check that @a data names a file this plugin is allowed to write. + * + * The "address" of this method is a file name, and it arrives with the + * truth, i.e. it is chosen by whoever uploaded the truth and must be + * treated as hostile: without this check any client could make us write + * to an arbitrary path. Accepted are paths inside @a ctx->directory that + * cannot climb back out of it again. + * + * @param ctx our plugin context + * @param data the file name to check, not necessarily 0-terminated + * @param data_length number of bytes in @a data + * @return true if @a data may be written to + */ +static bool +filename_ok (const struct FileContext *ctx, + const char *data, + size_t data_length) +{ + size_t dlen = strlen (ctx->directory); + const char *rest; + bool ok; + char *fn; + + if (0 == data_length) + return false; + /* An embedded 0 would make GNUNET_strndup() keep a different (shorter) + name than the one checked here, so validate and start would disagree. */ + if (NULL != memchr (data, + '\0', + data_length)) + return false; + if (data_length <= dlen) + return false; + if (0 != strncmp (data, + ctx->directory, + dlen)) + return false; + if ('/' == data[data_length - 1]) + return false; + fn = GNUNET_strndup (data, + data_length); + /* ctx->directory ends in '/', so a leading ".." shows up as "/.." here */ + rest = fn + dlen - 1; + { + size_t rlen = strlen (rest); + + ok = ( (NULL == strstr (rest, + "/../")) && + ( (rlen < 3) || + (0 != strcmp (rest + rlen - 3, + "/..")) ) ); + } + GNUNET_free (fn); + return ok; +} + + +/** * Saves the state of a authorization process */ struct ANASTASIS_AUTHORIZATION_State @@ -65,7 +140,7 @@ struct ANASTASIS_AUTHORIZATION_State * * To be possibly used before issuing a 402 payment required to the client. * - * @param cls closure with a `const struct ANASTASIS_AuthorizationContext *` + * @param cls closure with a `const struct FileContext *` * @param connection HTTP client request (for queuing response) * @param truth_mime mime type of @e data * @param data input to validate (i.e. is it a valid phone number, etc.) @@ -81,26 +156,17 @@ file_validate (void *cls, const char *data, size_t data_length) { - char *filename; - bool flag; + const struct FileContext *ctx = cls; - (void) cls; if (NULL == data) return GNUNET_SYSERR; - filename = GNUNET_STRINGS_data_to_string_alloc (data, - data_length); - flag = false; - for (size_t i = 0; i<strlen (filename); i++) - { - if ( (filename[i] == ' ') || - (filename[i] == '/') ) - { - flag = true; - break; - } - } - GNUNET_free (filename); - if (flag) + /* Screen exactly the bytes file_start() will use as the file name. The + old check ran over the Crockford base32 *encoding* of the truth, whose + alphabet contains neither ' ' nor '/', so it could never reject + anything, while the name actually opened went unchecked. */ + if (! filename_ok (ctx, + data, + data_length)) { /* Invalid input is #GNUNET_NO with a reply queued; #GNUNET_SYSERR is reserved for "invalid, and we could not even answer". */ @@ -120,7 +186,7 @@ file_validate (void *cls, * Begin issuing authentication challenge to user based on @a data. * I.e. start to send SMS or e-mail or launch video identification. * - * @param cls closure + * @param cls closure with a `const struct FileContext *` * @param trigger function to call when we made progress * @param trigger_cls closure for @a trigger * @param truth_uuid Identifier of the challenge, to be (if possible) included in the @@ -142,8 +208,18 @@ file_start (void *cls, { struct ANASTASIS_AUTHORIZATION_State *as; enum GNUNET_DB_QueryStatus qs; - - (void) cls; + const struct FileContext *ctx = cls; + + /* @e validate is optional as far as the plugin API is concerned, so the + screen has to be repeated here: this is the function whose result is + actually opened for writing. */ + if (! filename_ok (ctx, + data, + data_length)) + { + GNUNET_break_op (0); + return NULL; + } /* If the user can show this challenge code, this plugin is already happy (no additional requirements), so mark this challenge as @@ -310,9 +386,43 @@ libanastasis_plugin_authorization_file_init (void *cls) { const struct ANASTASIS_AuthorizationContext *ac = cls; struct ANASTASIS_AuthorizationPlugin *plugin; + struct FileContext *ctx; + char *dir; + + if (GNUNET_OK != + GNUNET_CONFIGURATION_get_value_filename (ac->cfg, + "authorization-file", + "DIRECTORY", + &dir)) + { + const char *tmpdir = getenv ("TMPDIR"); + if (NULL == tmpdir) + tmpdir = "/tmp"; + GNUNET_asprintf (&dir, + "%s/anastasis-file-challenges", + tmpdir); + } + ctx = GNUNET_new (struct FileContext); + ctx->ac = ac; + /* filename_ok() relies on the trailing '/' to tell "inside the directory" + from "a sibling whose name merely starts the same way". */ + GNUNET_asprintf (&ctx->directory, + "%s/", + dir); + GNUNET_free (dir); + if (GNUNET_OK != + GNUNET_DISK_directory_create (ctx->directory)) + { + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "Failed to create directory `%s' for file challenges\n", + ctx->directory); + GNUNET_free (ctx->directory); + GNUNET_free (ctx); + return NULL; + } plugin = GNUNET_new (struct ANASTASIS_AuthorizationPlugin); - plugin->cls = (void *) ac; + plugin->cls = ctx; plugin->retry_counter = INITIAL_RETRY_COUNTER; plugin->code_validity_period = GNUNET_TIME_UNIT_MINUTES; plugin->code_rotation_period = GNUNET_TIME_UNIT_MINUTES; @@ -339,7 +449,10 @@ void * libanastasis_plugin_authorization_file_done (void *cls) { struct ANASTASIS_AuthorizationPlugin *plugin = cls; + struct FileContext *ctx = plugin->cls; + GNUNET_free (ctx->directory); + GNUNET_free (ctx); GNUNET_free (plugin); return NULL; } diff --git a/src/authorization/anastasis_authorization_plugin_iban.c b/src/authorization/anastasis_authorization_plugin_iban.c @@ -175,6 +175,18 @@ get_message (const json_t *messages, return MISSING_MESSAGE; } GNUNET_JSON_parse_free (spec); + if (NULL == ret) + { + /* The parser of TALER_JSON_spec_i18n_string returns #GNUNET_OK even + when the field is absent or is not a string, in which case it stores + NULL; the check above therefore never fires for a missing message and + this one is what keeps NULL out of the format argument of the + GNUNET_asprintf() calls below. */ + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "Message `%s' is missing or not a string in the configured messages file\n", + msg_id); + return MISSING_MESSAGE; + } return ret; } } diff --git a/src/authorization/anastasis_authorization_plugin_post.c b/src/authorization/anastasis_authorization_plugin_post.c @@ -186,6 +186,18 @@ get_message (const json_t *messages, return MISSING_MESSAGE; } GNUNET_JSON_parse_free (spec); + if (NULL == ret) + { + /* The parser of TALER_JSON_spec_i18n_string returns #GNUNET_OK even + when the field is absent or is not a string, in which case it stores + NULL; the check above therefore never fires for a missing message and + this one is what keeps NULL out of the format argument of the + GNUNET_asprintf() calls below. */ + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "Message `%s' is missing or not a string in the configured messages file\n", + msg_id); + return MISSING_MESSAGE; + } return ret; } } diff --git a/src/authorization/anastasis_authorization_plugin_sms.c b/src/authorization/anastasis_authorization_plugin_sms.c @@ -188,6 +188,18 @@ get_message (const json_t *messages, return MISSING_MESSAGE; } GNUNET_JSON_parse_free (spec); + if (NULL == ret) + { + /* The parser of TALER_JSON_spec_i18n_string returns #GNUNET_OK even + when the field is absent or is not a string, in which case it stores + NULL; the check above therefore never fires for a missing message and + this one is what keeps NULL out of the format argument of the + GNUNET_asprintf() calls below. */ + GNUNET_log (GNUNET_ERROR_TYPE_ERROR, + "Message `%s' is missing or not a string in the configured messages file\n", + msg_id); + return MISSING_MESSAGE; + } return ret; } } diff --git a/src/backend/anastasis-httpd_policy-upload.c b/src/backend/anastasis-httpd_policy-upload.c @@ -128,8 +128,10 @@ struct PolicyUploadContext struct ANASTASIS_PaymentSecretP payment_identifier; /** - * Timestamp of the order in @e payment_identifier. Used to - * select the most recent unpaid offer. + * Creation time of the payment record named by @e payment_identifier, + * zero if there is none. Non-zero means the client is asking about a + * payment we already know, so the answer is a repeated payment request + * rather than a fresh order. */ struct GNUNET_TIME_Timestamp existing_pi_timestamp; @@ -892,8 +894,8 @@ AH_handler_policy_post ( if (puc->payment_identifier_provided) { /* check if payment identifier is valid (existing and paid) */ - bool paid; - bool valid_counter; + bool paid = false; + bool valid_counter = false; enum GNUNET_DB_QueryStatus qs; qs = ANASTASIS_DB_get_recdoc_payment ( @@ -906,6 +908,18 @@ AH_handler_policy_post ( MHD_HTTP_INTERNAL_SERVER_ERROR, TALER_EC_GENERIC_DB_FETCH_FAILED, NULL); + if (GNUNET_DB_STATUS_SUCCESS_NO_RESULTS == qs) + { + /* The client named a payment identifier we have no record of. The + result specification was not touched in that case, so we must not + look at @e paid or @e valid_counter; treat it as unpaid. */ + GNUNET_log (GNUNET_ERROR_TYPE_INFO, + "Given payment identifier is unknown, initiating payment\n"); + puc->payment_identifier_provided = false; + if (0 == puc->years_to_pay) + puc->years_to_pay = 1; + return prepare_payment (puc); + } if ( (! paid) || (! valid_counter) ) diff --git a/src/backend/anastasis.conf b/src/backend/anastasis.conf @@ -26,6 +26,15 @@ UNIXPATH_MODE = 660 # Which database backend do we use? DB = postgres +# How long after they expire are backups, truths and challenge codes kept +# before "anastasis-dbinit -g" collects them? Keeping them longer than +# necessary means keeping user data longer than necessary. +# GC_BACKUP_GRACE = 6 months + +# How long are payment records that never resulted in a payment (or that were +# refunded) kept before "anastasis-dbinit -g" collects them? +# GC_PAYMENT_RETENTION = 10 years + # Display name of the business running this anastasis provider. # BUSINESS_NAME = ... diff --git a/src/cli/anastasis-cli-redux.c b/src/cli/anastasis-cli-redux.c @@ -108,6 +108,7 @@ persist_new_state (json_t *state, GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "Could not dump state to `%s'\n", filename); + global_ret = 1; return; } return; @@ -120,6 +121,7 @@ persist_new_state (json_t *state, { GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "Could not serialize state to JSON\n"); + global_ret = 1; return; } if (-1 >= @@ -129,6 +131,7 @@ persist_new_state (json_t *state, { GNUNET_log (GNUNET_ERROR_TYPE_ERROR, "Could not dump state to stdout\n"); + global_ret = 1; GNUNET_free (state_str); return; } diff --git a/src/include/anastasis/anastasis-database/gc_challenge_codes.h b/src/include/anastasis/anastasis-database/gc_challenge_codes.h @@ -1,37 +0,0 @@ -/* - This file is part of Anastasis - Copyright (C) 2020-2022 Anastasis SARL - - Anastasis 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 - Foundation; either version 3, or (at your option) any later version. - - Anastasis 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 Affero General Public License for more details. - - You should have received a copy of the GNU Affero General Public License along with - Anastasis; see the file COPYING.GPL. If not, see <http://www.gnu.org/licenses/> -*/ -/** - * @file include/anastasis/anastasis-database/gc_challenge_codes.h - * @brief Anastasis database: gc challenge codes - * @author Christian Grothoff - */ -#ifndef ANASTASIS_DATABASE_GC_CHALLENGE_CODES_H -#define ANASTASIS_DATABASE_GC_CHALLENGE_CODES_H - -#include <gnunet/gnunet_util_lib.h> -#include <gnunet/gnunet_db_lib.h> -#include "anastasis_service.h" -#include "anastasis/anastasis-database/common.h" - -/** - * Function called to remove all expired codes from the database. - * - * @return transaction status - */ -enum GNUNET_DB_QueryStatus -ANASTASIS_DB_gc_challenge_codes (void); - -#endif diff --git a/src/include/anastasis_database_lib.h b/src/include/anastasis_database_lib.h @@ -52,7 +52,6 @@ #include "anastasis/anastasis-database/get_exists_challenge_code_satisfied.h" #include "anastasis/anastasis-database/insert_challenge_code.h" #include "anastasis/anastasis-database/update_to_challenge_code_sent.h" -#include "anastasis/anastasis-database/gc_challenge_codes.h" #include "anastasis/anastasis-database/insert_challenge_payment.h" #include "anastasis/anastasis-database/update_to_challenge_payment_refunded.h" #include "anastasis/anastasis-database/get_pending_challenge_payment.h" diff --git a/src/include/anastasis_testing_lib.h b/src/include/anastasis_testing_lib.h @@ -197,6 +197,22 @@ ANASTASIS_TESTING_run_anastasis (const char *config_filename, /** + * Construct a file name for the "file" authorization method to write a + * challenge code to. The plugin only accepts names inside the directory + * configured as "[authorization-file] DIRECTORY", so the name is derived + * from that configuration and a fresh subdirectory is created underneath + * it. + * + * @param config_filename configuration filename. + * + * @return the file name, or NULL upon errors. Must be freed + * by the caller. + */ +char * +ANASTASIS_TESTING_make_file_challenge_name (const char *config_filename); + + +/** * Prepare the anastasis execution. Create tables and check if * the port is available. * diff --git a/src/include/meson.build b/src/include/meson.build @@ -46,7 +46,6 @@ install_data( 'anastasis/anastasis-database/get_exists_challenge_code_satisfied.h', 'anastasis/anastasis-database/insert_challenge_code.h', 'anastasis/anastasis-database/update_to_challenge_code_sent.h', - 'anastasis/anastasis-database/gc_challenge_codes.h', 'anastasis/anastasis-database/insert_challenge_payment.h', 'anastasis/anastasis-database/update_to_challenge_payment_refunded.h', 'anastasis/anastasis-database/get_pending_challenge_payment.h', diff --git a/src/reducer/anastasis_api_discovery.c b/src/reducer/anastasis_api_discovery.c @@ -25,7 +25,6 @@ #include <taler/taler_json_lib.h> #include "anastasis_api_redux.h" #include "anastasis_api_redux_state.h" -#include <dlfcn.h> /** diff --git a/src/reducer/anastasis_api_recovery_redux.c b/src/reducer/anastasis_api_recovery_redux.c @@ -368,8 +368,13 @@ error_by_status (enum ANASTASIS_RecoveryStatus rc, ec = TALER_EC_ANASTASIS_REDUCER_POLICY_LOOKUP_FAILED; break; case ANASTASIS_RS_POLICY_DECRYPTION_FAILED: + /* The two causes are indistinguishable to us: a wrong key and a key + derived differently both just fail to authenticate the ciphertext. + Name both, or a user with a pre-flag-day backup retypes their + identity attributes forever. See NEWS on the key derivation + change. */ *msg = gettext_noop ( - "failed to decrypt policy document with the given identity attributes"); + "failed to decrypt policy document: either the identity attributes are not the ones used for the backup, or the backup was made before the key derivation change and cannot be recovered with this version"); ec = TALER_EC_ANASTASIS_REDUCER_POLICY_MALFORMED; break; case ANASTASIS_RS_CORE_SECRET_RECOVERY_FAILED: diff --git a/src/reducer/anastasis_api_redux_state.h b/src/reducer/anastasis_api_redux_state.h @@ -209,7 +209,8 @@ struct ANASTASIS_ReduxAttributeSpec char *validation_regex; /** - * Name of a validation function to `dlsym()`, NULL if not given. + * Name of a validation function, resolved against the fixed table in + * reducer/validation.c. NULL if not given. */ char *validation_logic; diff --git a/src/reducer/meson.build b/src/reducer/meson.build @@ -10,7 +10,6 @@ libanastasisredux_SOURCES = [ 'anastasis_api_recovery_redux.c', 'anastasis_api_backup_redux.c', 'validation.c', - 'validation_AL_NID.c', 'validation_BE_NRN.c', 'validation_CH_AHV.c', 'validation_CZ_BN.c', @@ -42,7 +41,6 @@ libanastasisredux = library( talerjson_dep, json_dep, gcrypt_dep, - dl_dep, m_dep, libanastasisutil_dep, libanastasisrest_dep, @@ -90,11 +88,7 @@ test_validation = executable( build_by_default: false, install: false, ) -test( - 'test_validation', - test_validation, - suite: ['reducer'], -) +test('test_validation', test_validation, suite: ['reducer']) libanastasisredux_dep = declare_dependency(link_with: libanastasisredux) pkg.generate( diff --git a/src/reducer/test_validation.c b/src/reducer/test_validation.c @@ -27,7 +27,6 @@ * contrib/redux.*.json. All of them must resolve. */ static const char *known[] = { - "AL_NID_check", "BE_NRN_check", "CH_AHV_check", "CZ_BN_check", @@ -217,9 +216,9 @@ main (int argc, return 1; } - /* AL_NID_check has no implementation yet and deliberately accepts - everything; when it is implemented, this expectation must change. */ - if (! AL_NID_check ("I05101999I")) + /* Albania has no check-digit implementation, so contrib/redux.al.json + carries no "validation-logic" at all and the name must not resolve. */ + if (NULL != ANASTASIS_REDUX_validation_lookup_ ("AL_NID_check")) { GNUNET_break (0); return 1; diff --git a/src/reducer/validation.c b/src/reducer/validation.c @@ -46,7 +46,6 @@ struct ValidationLogic * rejected by #ANASTASIS_REDUX_validation_lookup_(). */ static const struct ValidationLogic logics[] = { - { "AL_NID_check", &AL_NID_check }, { "BE_NRN_check", &BE_NRN_check }, { "CH_AHV_check", &CH_AHV_check }, { "CZ_BN_check", &CZ_BN_check }, diff --git a/src/reducer/validation.h b/src/reducer/validation.h @@ -49,9 +49,6 @@ ANASTASIS_REDUX_validation_lookup_ (const char *name); bool -AL_NID_check (const char *nid_number); - -bool BE_NRN_check (const char *nrn_number); bool diff --git a/src/reducer/validation_AL_NID.c b/src/reducer/validation_AL_NID.c @@ -1,50 +0,0 @@ -/* - This file is part of Anastasis - Copyright (C) 2026 Anastasis SARL - - Anastasis 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. - - Anastasis 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 - Anastasis; see the file COPYING.GPL. If not, see <http://www.gnu.org/licenses/> -*/ -/** - * @file reducer/validation_AL_NID.c - * @brief Validation for Albanian national identity numbers - * @author Christian Grothoff - */ -#include "platform.h" -#include <gnunet/gnunet_util_lib.h> -#include <string.h> -#include <stdbool.h> - - -/** - * Function to validate an Albanian national identity number (NIPT/NID). - * - * NOT YET IMPLEMENTED: the algorithm computing the trailing check letter has - * not been confirmed against an authoritative specification, and guessing it - * would reject valid numbers. Until it is, this deliberately accepts any - * input and logs that no check was performed. The - * "validation-regex" in contrib/redux.al.json still applies. - * - * @param nid_number identity number to validate (input) - * @return true, always, as no check is implemented yet - */ -bool -AL_NID_check (const char *nid_number); - -/* declaration to fix compiler warning */ -bool -AL_NID_check (const char *nid_number) -{ - (void) nid_number; - GNUNET_log (GNUNET_ERROR_TYPE_WARNING, - "Albanian identity numbers are not checked: AL_NID_check is not implemented\n"); - return true; -} diff --git a/src/stasis/anastasis-dbinit.c b/src/stasis/anastasis-dbinit.c @@ -82,18 +82,32 @@ run (void *cls, struct GNUNET_TIME_Absolute expire_backups; struct GNUNET_TIME_Absolute expire_payments; struct GNUNET_TIME_Absolute now; + struct GNUNET_TIME_Relative backup_grace; + struct GNUNET_TIME_Relative payment_retention; - now = GNUNET_TIME_absolute_get (); - expire_backups = GNUNET_TIME_absolute_subtract ( - now, - GNUNET_TIME_relative_multiply ( + /* How long expired data is kept before it is collected is a + data-minimization decision, so let the operator make it. */ + if (GNUNET_OK != + GNUNET_CONFIGURATION_get_value_time (cfg, + "anastasis", + "GC_BACKUP_GRACE", + &backup_grace)) + backup_grace = GNUNET_TIME_relative_multiply ( GNUNET_TIME_UNIT_MONTHS, - 6)); - expire_payments = GNUNET_TIME_absolute_subtract ( - now, - GNUNET_TIME_relative_multiply ( + 6); + if (GNUNET_OK != + GNUNET_CONFIGURATION_get_value_time (cfg, + "anastasis", + "GC_PAYMENT_RETENTION", + &payment_retention)) + payment_retention = GNUNET_TIME_relative_multiply ( GNUNET_TIME_UNIT_YEARS, - 10)); + 10); + now = GNUNET_TIME_absolute_get (); + expire_backups = GNUNET_TIME_absolute_subtract (now, + backup_grace); + expire_payments = GNUNET_TIME_absolute_subtract (now, + payment_retention); if (0 > ANASTASIS_DB_do_gc ( expire_backups, expire_payments)) diff --git a/src/stasis/do_gc.sql b/src/stasis/do_gc.sql @@ -65,7 +65,31 @@ DELETE FROM anastasis_challenge_payment WHERE (paid=FALSE OR refunded) AND creation_date < in_expire_pending_payments; +-- A paid challenge payment is not covered by the horizon above, and +-- truth_uuid is not a foreign key, so nothing would ever remove it. Once the +-- truth it paid for is gone -- deleted just above, or never created -- the +-- record cannot be acted on again, so it is collected here regardless of age. +-- This runs after the truth deletes on purpose, so that a truth expiring in +-- this same pass takes its payments with it. +DELETE FROM anastasis_challenge_payment cp + WHERE NOT EXISTS + (SELECT 1 + FROM anastasis_truth t + WHERE t.truth_uuid = cp.truth_uuid); + +-- Bank statements collected by the IBAN authorization helper carry customer +-- account details, so they should not be kept indefinitely either. The +-- newest row per credit account is retained whatever its age: the helper +-- resumes from the highest wire_reference it finds here, and deleting that +-- would make it re-download the bank history from the beginning. +DELETE FROM anastasis_auth_iban_in ai + WHERE ai.execution_date < in_expire_backups + AND ai.wire_reference <> + (SELECT MAX(x.wire_reference) + FROM anastasis_auth_iban_in x + WHERE x.credit_account_details = ai.credit_account_details); + END $$; COMMENT ON FUNCTION anastasis_do_gc - IS 'Removes expired accounts with their recovery documents and payments, expired truths with their challenge codes and payments, and payment records that were never paid or were refunded. Runs as a single statement, so it either collects everything or nothing.'; + IS 'Removes expired accounts with their recovery documents and payments, expired truths with their challenge codes and payments, challenge payments whose truth is gone, payment records that were never paid or were refunded, and stale IBAN authentication transfers. Runs as a single statement, so it either collects everything or nothing.'; diff --git a/src/stasis/gc_challenge_codes.c b/src/stasis/gc_challenge_codes.c @@ -1,55 +0,0 @@ -/* - This file is part of Anastasis - Copyright (C) 2020-2022 Anastasis SARL - - Anastasis 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 - Foundation; either version 3, or (at your option) any later version. - - Anastasis 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 Affero General Public License for more details. - - You should have received a copy of the GNU Affero General Public License along with - Anastasis; see the file COPYING.GPL. If not, see <http://www.gnu.org/licenses/> -*/ -/** - * @file stasis/gc_challenge_codes.c - * @brief Anastasis database: gc challenge codes - * @author Christian Grothoff - */ -#include "platform.h" -#include "anastasis-db_pg.h" -#include "anastasis/anastasis-database/gc_challenge_codes.h" -#include "anastasis/anastasis-database/transaction.h" -#include "anastasis/anastasis-database/preflight.h" -#include <taler/taler_pq_lib.h> - - -/** - * Function called to remove all expired codes from the database. - * - * @return transaction status - */ -enum GNUNET_DB_QueryStatus -ANASTASIS_DB_gc_challenge_codes (void) -{ - struct GNUNET_TIME_Timestamp time_now = GNUNET_TIME_timestamp_get (); - struct GNUNET_PQ_QueryParam params[] = { - GNUNET_PQ_query_param_timestamp (&time_now), - GNUNET_PQ_query_param_end - }; - - GNUNET_break (GNUNET_OK == - ANASTASIS_DB_preflight ()); - PREPARE ("gc_challenge_codes", - "DELETE FROM anastasis_challengecode " - "WHERE " - "expiration_date < $1;"); - return GNUNET_PQ_eval_prepared_non_select (pg->conn, - "gc_challenge_codes", - params); -} - - -/* end of gc_challenge_codes.c */ diff --git a/src/stasis/meson.build b/src/stasis/meson.build @@ -40,7 +40,6 @@ libanastasisdb_SOURCES = [ 'create_tables.c', 'drop_tables.c', 'do_gc.c', - 'gc_challenge_codes.c', 'get_challenge_payment.c', 'get_recdoc_payment.c', 'get_truth_payment.c', diff --git a/src/stasis/stasis-0001.sql b/src/stasis/stasis-0001.sql @@ -119,7 +119,7 @@ CREATE TABLE IF NOT EXISTS anastasis_challenge_payment paid BOOLEAN NOT NULL DEFAULT FALSE, refunded BOOLEAN NOT NULL DEFAULT FALSE ); -COMMENT ON TABLE anastasis_challenge_payment +COMMENT ON TABLE anastasis_recdoc_payment IS 'Records a payment for a challenge'; COMMENT ON COLUMN anastasis_challenge_payment.payment_id IS 'Serial number which identifies the payment'; @@ -197,6 +197,12 @@ CREATE INDEX IF NOT EXISTS anastasis_challengecode_uuid_index COMMENT ON INDEX anastasis_challengecode_uuid_index IS 'for challenge lookup'; +CREATE INDEX IF NOT EXISTS anastasis_challengecode_expiration_index + ON anastasis_challengecode + (truth_uuid,expiration_date); +COMMENT ON INDEX anastasis_challengecode_expiration_index + IS 'for challenge garbage collection'; + CREATE TABLE IF NOT EXISTS anastasis_auth_iban_in (auth_in_serial_id BIGINT GENERATED BY DEFAULT AS IDENTITY UNIQUE diff --git a/src/stasis/stasis-0002.sql b/src/stasis/stasis-0002.sql @@ -25,12 +25,30 @@ SET search_path TO anastasis; -- Garbage collection deletes WHERE expiration_date < $1 with no truth_uuid -- predicate, so the index must lead with expiration_date; the existing -- (truth_uuid, expiration_date) lookup index cannot serve that scan. -CREATE INDEX IF NOT EXISTS anastasis_challengecode_expiration_index +-- +-- stasis-0001 already created an index of this name, as a straight duplicate +-- of anastasis_challengecode_uuid_index on (truth_uuid, expiration_date). It +-- has to be dropped first: CREATE INDEX IF NOT EXISTS matches on the name +-- alone and does not compare definitions, so without this DROP every provider +-- that ran 0001 would silently keep the useless index, never get the one +-- garbage collection needs, and still have 0002 recorded as applied. +DROP INDEX IF EXISTS anastasis_challengecode_expiration_index; +CREATE INDEX anastasis_challengecode_expiration_index ON anastasis_challengecode (expiration_date); COMMENT ON INDEX anastasis_challengecode_expiration_index IS 'for challenge garbage collection'; +-- stasis-0001 commented anastasis_recdoc_payment twice: the second one, in +-- the anastasis_challenge_payment block, overwrote the recovery-document text +-- with the challenge text and left anastasis_challenge_payment itself +-- undocumented. 0001 is released and must stay byte-identical to what +-- providers already ran, so the correction lives here. +COMMENT ON TABLE anastasis_recdoc_payment + IS 'Records a payment for a recovery document'; +COMMENT ON TABLE anastasis_challenge_payment + IS 'Records a payment for a challenge'; + -- A TOTP code stays valid for a whole window of time steps, so without -- remembering which step was already used an observed code can simply be -- replayed while the window lasts. RFC 6238 section 5.2 requires that a code diff --git a/src/testing/test_anastasis.c b/src/testing/test_anastasis.c @@ -355,20 +355,9 @@ main (int argc, char *const *argv) { int ret; - { - char dir[] = "/tmp/test-anastasis-file-XXXXXX"; - - if (NULL == mkdtemp (dir)) - { - GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR, - "mkdtemp", - dir); - return 77; - } - GNUNET_asprintf (&file_secret, - "%s/.secret", - dir); - } + file_secret = ANASTASIS_TESTING_make_file_challenge_name (CONFIG_FILE); + if (NULL == file_secret) + return 77; id_data = ANASTASIS_TESTING_make_id_data_example ("MaxMuster123456789"); merchant_payto.full_payto = (char *) "payto://iban/SANDBOXX/" MERCHANT_ACCOUNT_IBAN diff --git a/src/testing/test_anastasis_api.c b/src/testing/test_anastasis_api.c @@ -309,20 +309,9 @@ main (int argc, { int ret; - { - char dir[] = "/tmp/test-anastasis-file-XXXXXX"; - - if (NULL == mkdtemp (dir)) - { - GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR, - "mkdtemp", - dir); - return 77; - } - GNUNET_asprintf (&file_secret, - "%s/.secret", - dir); - } + file_secret = ANASTASIS_TESTING_make_file_challenge_name (CONFIG_FILE); + if (NULL == file_secret) + return 77; merchant_payto.full_payto = (char *) "payto://iban/SANDBOXX/" MERCHANT_ACCOUNT_IBAN "?receiver-name=merchant"; diff --git a/src/testing/test_anastasis_api.conf b/src/testing/test_anastasis_api.conf @@ -72,6 +72,11 @@ COST = EUR:0 [authorization-file] CURRENCY = EUR COST = EUR:1 +# The file method writes the challenge code to a file whose name comes from +# the (client-supplied) truth, so the plugin only accepts names inside this +# directory. Per-user, like TALER_RUNTIME_DIR, so that concurrent runs by +# different users on one host do not collide. +DIRECTORY = ${TMPDIR:-${TMP:-/tmp}}/${USER:-}/anastasis-file-challenges/ [authorization-email] CURRENCY = EUR diff --git a/src/testing/testing_api_helpers.c b/src/testing/testing_api_helpers.c @@ -97,6 +97,63 @@ ANASTASIS_TESTING_run_anastasis (const char *config_filename, char * +ANASTASIS_TESTING_make_file_challenge_name (const char *config_filename) +{ + struct GNUNET_CONFIGURATION_Handle *cfg; + char *dir; + char *tmpl; + char *fn; + + cfg = GNUNET_CONFIGURATION_create (ANASTASIS_project_data ()); + if (GNUNET_OK != + GNUNET_CONFIGURATION_load (cfg, + config_filename)) + ANASTASIS_FAIL (); + if (GNUNET_OK != + GNUNET_CONFIGURATION_get_value_filename (cfg, + "authorization-file", + "DIRECTORY", + &dir)) + { + GNUNET_log_config_missing (GNUNET_ERROR_TYPE_ERROR, + "authorization-file", + "DIRECTORY"); + GNUNET_CONFIGURATION_destroy (cfg); + return NULL; + } + GNUNET_CONFIGURATION_destroy (cfg); + if (GNUNET_OK != + GNUNET_DISK_directory_create (dir)) + { + GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR, + "mkdir", + dir); + GNUNET_free (dir); + return NULL; + } + /* The plugin only writes inside DIRECTORY, so the scratch directory has to + live there too; a private subdirectory keeps concurrent runs apart. */ + GNUNET_asprintf (&tmpl, + "%s/test-anastasis-file-XXXXXX", + dir); + GNUNET_free (dir); + if (NULL == mkdtemp (tmpl)) + { + GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR, + "mkdtemp", + tmpl); + GNUNET_free (tmpl); + return NULL; + } + GNUNET_asprintf (&fn, + "%s/.secret", + tmpl); + GNUNET_free (tmpl); + return fn; +} + + +char * ANASTASIS_TESTING_prepare_anastasis (const char *config_filename) { struct GNUNET_CONFIGURATION_Handle *cfg; diff --git a/src/util/anastasis_crypto.c b/src/util/anastasis_crypto.c @@ -180,7 +180,10 @@ anastasis_decrypt (const void *key, if (data_size < crypto_secretbox_NONCEBYTES + crypto_secretbox_MACBYTES) { - GNUNET_break (0); + /* short ciphertext is what a hostile provider sends, not a local bug */ + GNUNET_break_op (0); + *res = NULL; + *res_size = 0; return GNUNET_SYSERR; } nonce = data; @@ -199,7 +202,8 @@ anastasis_decrypt (const void *key, (void *) nonce, (void *) &skey)) { - GNUNET_break (0); + /* likewise: a wrong answer or a hostile provider, not a local bug */ + GNUNET_break_op (0); GNUNET_free (*res); *res = NULL; *res_size = 0; @@ -534,11 +538,6 @@ ANASTASIS_CRYPTO_core_secret_recover ( GNUNET_break_op (0); return GNUNET_SYSERR; } - GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, - "At %s:%d encrypted core secret is %s-%llu b\n", __FILE__, - __LINE__, - TALER_b2s (encrypted_core_secret, encrypted_core_secret_size), - (unsigned long long) encrypted_core_secret_size); if (GNUNET_OK != anastasis_decrypt (master_key, master_key_size, @@ -553,11 +552,9 @@ ANASTASIS_CRYPTO_core_secret_recover ( return GNUNET_SYSERR; } GNUNET_free (master_key); - GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, - "At %s:%d decrypted core secret is %s-%llu b\n", __FILE__, - __LINE__, - TALER_b2s (*core_secret, *core_secret_size), - (unsigned long long) *core_secret_size); + /* Deliberately not logged: TALER_b2s() of the core secret is a ~40 bit + fingerprint of it, which is enough to confirm a guess for anyone who can + read the debug log. */ return GNUNET_OK; } diff --git a/src/util/meson.build b/src/util/meson.build @@ -42,10 +42,10 @@ test_anastasis_crypto = executable( 'test_anastasis_crypto', ['test_anastasis_crypto.c'], dependencies: [ - gnunetutil_dep, - talerutil_dep, - libanastasisutil_dep, - json_dep, + gnunetutil_dep, + talerutil_dep, + libanastasisutil_dep, + json_dep, ], include_directories: [incdir, configuration_inc], build_by_default: false,