merchant

Merchant backend to process payments, run by merchants
Log | Files | Refs | Submodules | README | LICENSE

commit ca62ed41b3bdca9368cdf28e8c79ff0faf5995ec
parent 6856adaab2778b89f2fc99204c1065b0a965713d
Author: Christian Grothoff <christian@grothoff.org>
Date:   Sat, 21 Jun 2025 18:38:46 +0200

final cleanup of 9556 branch patch, and remove -a option from taler-merchant-httpd (now that we have taler-merchant-passwd)

Diffstat:
Msrc/backend/taler-merchant-httpd.c | 164++++++++++++++++++++++++++++++++++++-------------------------------------------
Msrc/backend/taler-merchant-httpd.h | 7+------
Msrc/backend/taler-merchant-httpd_private-post-instances-ID-auth.c | 17++++-------------
Msrc/backend/taler-merchant-httpd_private-post-instances.c | 7-------
Msrc/backenddb/pg_lookup_instances.c | 1+
Msrc/merchant-tools/taler-merchant-passwd.c | 5+++++
Msrc/testing/test_merchant_instance_auth.sh | 10+++++++---
Msrc/testing/test_merchant_product_creation.sh | 2--
8 files changed, 92 insertions(+), 121 deletions(-)

diff --git a/src/backend/taler-merchant-httpd.c b/src/backend/taler-merchant-httpd.c @@ -1,6 +1,6 @@ /* This file is part of TALER - (C) 2014-2024 Taler Systems SA + (C) 2014-2025 Taler Systems SA TALER 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 @@ -153,8 +153,8 @@ static struct GNUNET_DB_EventHandler *instance_eh; struct GNUNET_CONTAINER_MultiHashMap *TMH_by_id_map; /** - * GNUNET_YES if protocol version 42 is strictly enforced. - * (Default is GNUNET_NO) + * #GNUNET_YES if protocol version 42 is strictly enforced. + * (Default is #GNUNET_NO) */ int TMH_strict_v42; @@ -207,11 +207,6 @@ static int global_ret; static const struct GNUNET_CONFIGURATION_Handle *cfg; /** - * Initial authorization token. - */ -char *TMH_default_pass; - -/** * Maximum length of a permissions string of a scope */ #define TMH_MAX_SCOPE_PERMISSIONS_LEN 4096 @@ -296,6 +291,7 @@ struct ScopePermissionMap scope_permissions[] = { } }; + /** * Get permissions string for scope. * Also extracts the leftmost bit into the @a refreshable @@ -306,14 +302,15 @@ struct ScopePermissionMap scope_permissions[] = { * @return the permissions string, or NULL if no such scope found */ static const char* -get_scope_permissions (enum TMH_AuthScope as, bool *refreshable) +get_scope_permissions (enum TMH_AuthScope as, + bool *refreshable) { *refreshable = as & TMH_AS_REFRESHABLE; - if (as != TMH_AS_ADMIN) - as &= ~TMH_AS_REFRESHABLE; - for (int i = 0; TMH_AS_NONE != scope_permissions[i].as; i++) + for (unsigned int i = 0; TMH_AS_NONE != scope_permissions[i].as; i++) { - if (as == scope_permissions[i].as) + /* We ignore the TMH_AS_REFRESHABLE bit */ + if ( (as & ~TMH_AS_REFRESHABLE) == + (scope_permissions[i].as & ~TMH_AS_REFRESHABLE) ) return scope_permissions[i].permissions; } return NULL; @@ -333,7 +330,7 @@ permission_in_scope (const char *permission_required, enum TMH_AuthScope scope) { char *permissions; - const char*perms_tmp; + const char *perms_tmp; bool is_read_perm; bool is_write_perm; bool refreshable; @@ -346,7 +343,6 @@ permission_in_scope (const char *permission_required, GNUNET_break_op (0); return false; } - last_dash = strrchr (permission_required, '-'); if (NULL != last_dash) @@ -412,24 +408,31 @@ bool TMH_scope_is_subset (enum TMH_AuthScope as, enum TMH_AuthScope candidate) { - const char*as_perms; - const char*candidate_perms; + const char *as_perms; + const char *candidate_perms; char *permissions; bool as_refreshable; bool cand_refreshable; - as_perms = get_scope_permissions (as, &as_refreshable); - candidate_perms = get_scope_permissions (candidate, &cand_refreshable); + as_perms = get_scope_permissions (as, + &as_refreshable); + candidate_perms = get_scope_permissions (candidate, + &cand_refreshable); if (! as_refreshable && cand_refreshable) return false; - if ((NULL == as_perms) && (NULL != candidate_perms)) + if ( (NULL == as_perms) && + (NULL != candidate_perms) ) return false; - if ((NULL == candidate_perms) || - (0 == strcmp ("*", as_perms))) + if ( (NULL == candidate_perms) || + (0 == strcmp ("*", + as_perms))) return true; permissions = GNUNET_strdup (candidate_perms); { - char *perm = strtok (permissions, ","); + const char *perm; + + perm = strtok (permissions, + ","); if (NULL == perm) { GNUNET_free (permissions); @@ -437,12 +440,14 @@ TMH_scope_is_subset (enum TMH_AuthScope as, } while (NULL != perm) { - if (! permission_in_scope (perm, as)) + if (! permission_in_scope (perm, + as)) { GNUNET_free (permissions); return false; } - perm = strtok (NULL, ","); + perm = strtok (NULL, + ","); } } GNUNET_free (permissions); @@ -453,9 +458,10 @@ TMH_scope_is_subset (enum TMH_AuthScope as, enum TMH_AuthScope TMH_get_scope_by_name (const char *name) { - for (int i = 0; TMH_AS_NONE != scope_permissions[i].as; i++) + for (unsigned int i = 0; TMH_AS_NONE != scope_permissions[i].as; i++) { - if (0 == strcasecmp (scope_permissions[i].name, name)) + if (0 == strcasecmp (scope_permissions[i].name, + name)) return scope_permissions[i].as; } return TMH_AS_NONE; @@ -475,12 +481,18 @@ TMH_check_auth (const char *token, return GNUNET_OK; if (NULL == token) return GNUNET_SYSERR; + // FIXME: why do we urldecode here? This seems wrong + // if we got a password from 'Basic' auth header. + // In deprecated secret-token:password case, this + // was applicable -> move logic there! dec_len = GNUNET_STRINGS_urldecode (token, strlen (token), &dec); GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Checking against token with salt %s\n", TALER_B2S (salt)); + // FIXME: factor into common routine in libtalerutil + // see also taler-merchant-passwd and same call below! GNUNET_assert (GNUNET_YES == GNUNET_CRYPTO_kdf (&val, sizeof (val), @@ -515,19 +527,23 @@ TMH_check_auth_instance (const char *token, return GNUNET_OK; if (NULL == token) return GNUNET_SYSERR; - if (0 == GNUNET_STRINGS_base64_decode (token, - strlen (token), - (void**) &tmp)) + if (0 == + GNUNET_STRINGS_base64_decode (token, + strlen (token), + (void**) &tmp)) { return GNUNET_SYSERR; } - instance_name = strtok (tmp, ":"); + instance_name = strtok (tmp, + ":"); if (NULL == instance_name) { GNUNET_free (tmp); return GNUNET_SYSERR; } - password = strtok (NULL, ":"); + // FIXME: why would the password end at the first ':'? + password = strtok (NULL, + ":"); if (NULL == password) { GNUNET_free (tmp); @@ -536,7 +552,9 @@ TMH_check_auth_instance (const char *token, if (NULL != instance->settings.id) target_instance = instance->settings.id; - if (0 != strncmp (instance_name, target_instance, + // FIXME: why a *prefix* match and not a full-string match? + if (0 != strncmp (instance_name, + target_instance, strlen (target_instance))) { GNUNET_free (tmp); @@ -550,6 +568,8 @@ TMH_check_auth_instance (const char *token, } +// FIXME: factor into common routine in libtalerutil +// see also taler-merchant-passwd! void TMH_compute_auth (const char *token, struct TALER_MerchantAuthenticationSaltP *salt, @@ -898,15 +918,15 @@ extract_auth (const char **auth) const char *bearer = "Bearer "; const char *basic = "Basic "; const char *tok = *auth; - int offset = 0; - int is_bearer = GNUNET_NO; + size_t offset = 0; + bool is_bearer = false; if (0 == strncmp (tok, bearer, strlen (bearer))) { offset = strlen (bearer); - is_bearer = GNUNET_YES; + is_bearer = true; } else if (0 == strncmp (tok, basic, @@ -919,8 +939,6 @@ extract_auth (const char **auth) *auth = NULL; return; } - - tok += offset; while (' ' == *tok) tok++; @@ -1971,20 +1989,6 @@ url_handler (void *cls, (0 == strcmp ("admin", instance_id)) ) hc->instance = TMH_lookup_instance (NULL); - if ( (0 == strcmp ("admin", - instance_id)) && - (NULL != TMH_default_pass) && - (NULL != hc->instance) ) - { - /* Override default instance access control */ - GNUNET_log (GNUNET_ERROR_TYPE_INFO, - "Command-line override of access control\n"); - TMH_compute_auth (TMH_default_pass, - &hc->instance->auth.auth_salt, - &hc->instance->auth.auth_hash); - hc->instance->auth_override = true; - GNUNET_free (TMH_default_pass); - } GNUNET_free (instance_id); if (NULL == slash) url = ""; @@ -1996,18 +2000,6 @@ url_handler (void *cls, /* use 'default' */ use_default = true; hc->instance = TMH_lookup_instance (NULL); - if ( (NULL != TMH_default_pass) && - (NULL != hc->instance) ) - { - /* Override default instance access control */ - GNUNET_log (GNUNET_ERROR_TYPE_INFO, - "Command-line override of access control\n"); - TMH_compute_auth (TMH_default_pass, - &hc->instance->auth.auth_salt, - &hc->instance->auth.auth_hash); - hc->instance->auth_override = true; - GNUNET_free (TMH_default_pass); - } } if (NULL != hc->instance) { @@ -2234,6 +2226,8 @@ url_handler (void *cls, because some reverse proxy is already doing it, and then that reverse proxy may forward malformed auth headers to the backend. */ + // FIXME: why do we check here *and* in extract_auth + // for "Basic "? Bad style! is_basic_auth = (0 == strncmp (auth, "Basic ", strlen ("Basic "))); @@ -2244,11 +2238,9 @@ url_handler (void *cls, } /* If we have zero configured instances (not even ones that have been - purged) AND no override credentials, THEN we accept anything (no access + purged), THEN we accept anything (no access control), as we then also have no data to protect. */ - // FIXME this must somehow carry over to tokens - if ( (0 == GNUNET_CONTAINER_multihashmap_size (TMH_by_id_map)) && - (NULL == TMH_default_pass) ) + if (0 == GNUNET_CONTAINER_multihashmap_size (TMH_by_id_map)) { hc->auth_scope = TMH_AS_ADMIN; } @@ -2260,11 +2252,13 @@ url_handler (void *cls, * The only time we need to handle authentication like this is * for the token endpoint! */ - if ((0 != strncmp (hc->rh->url_prefix, "/token", strlen ("/token"))) || - (0 != strncmp (MHD_HTTP_METHOD_POST, - hc->rh->method, - strlen (MHD_HTTP_METHOD_POST))) || - (NULL == hc->instance)) + if ( (0 != strncmp (hc->rh->url_prefix, + "/token", + strlen ("/token"))) || + (0 != strncmp (MHD_HTTP_METHOD_POST, + hc->rh->method, + strlen (MHD_HTTP_METHOD_POST))) || + (NULL == hc->instance)) { // FIXME this should never happen, but according to the comment below, // We must not error out here for some reason that has to do with @@ -2273,7 +2267,6 @@ url_handler (void *cls, } else { - // FIXME: Do we want to check CLI provided credentials here? if (GNUNET_OK == TMH_check_auth_instance (auth, hc->instance)) @@ -2308,9 +2301,10 @@ url_handler (void *cls, GNUNET_log (GNUNET_ERROR_TYPE_WARNING, "Trying deprecated secret-token:password API authN\n") ; - if (GNUNET_OK != TMH_check_auth (auth + strlen (RFC_8959_PREFIX), - &hc->instance->auth.auth_salt, - &hc->instance->auth.auth_hash)) + if (GNUNET_OK != + TMH_check_auth (auth + strlen (RFC_8959_PREFIX), + &hc->instance->auth.auth_salt, + &hc->instance->auth.auth_hash)) { GNUNET_log (GNUNET_ERROR_TYPE_WARNING, "Login failed\n"); @@ -2337,8 +2331,9 @@ url_handler (void *cls, - scope is 'all' - rh has an explicit non-NONE scope that matches - scope is 'read only' and we have a GET request */ - if ((NULL != hc->rh->permission) && - (! permission_in_scope (hc->rh->permission, hc->auth_scope))) + if ( (NULL != hc->rh->permission) && + (! permission_in_scope (hc->rh->permission, + hc->auth_scope))) { if (auth_malformed && (TMH_AS_NONE == hc->auth_scope) ) @@ -2456,7 +2451,6 @@ add_instance_cb (void *cls, &mi->h_instance, mi); } - mi = GNUNET_new (struct TMH_MerchantInstance); mi->settings = *is; mi->auth = *ias; @@ -2638,15 +2632,10 @@ run (void *cls, { enum TALER_MHD_GlobalOptions go; int elen; - const char *tok; (void) cls; (void) args; (void) cfgfile; - tok = getenv ("TALER_MERCHANT_PASSWORD"); - if ( (NULL != tok) && - (NULL == TMH_default_pass) ) - TMH_default_pass = GNUNET_strdup (tok); cfg = config; GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Starting taler-merchant-httpd\n"); @@ -2857,11 +2846,6 @@ main (int argc, &merchant_connection_close), GNUNET_GETOPT_option_timetravel ('T', "timetravel"), - GNUNET_GETOPT_option_string ('a', - "auth", - "TOKEN", - "use TOKEN to initially authenticate access to the default instance (you can also set the TALER_MERCHANT_TOKEN environment variable instead)", - &TMH_default_pass), GNUNET_GETOPT_option_version (PACKAGE_VERSION "-" VCS_VERSION), GNUNET_GETOPT_OPTION_END }; diff --git a/src/backend/taler-merchant-httpd.h b/src/backend/taler-merchant-httpd.h @@ -178,7 +178,7 @@ struct TMH_MerchantInstance /** * The authentication settings for this instance - * were changed via the command-line. Do not check + * do not apply due to administrative action. Do not check * against the DB value when updating the auth token. */ bool auth_override; @@ -750,11 +750,6 @@ extern struct GNUNET_CONTAINER_MultiHashMap *TMH_by_id_map; */ extern struct GNUNET_TIME_Relative TMH_legal_expiration; -/** - * Initial authorization token. - */ -extern char *TMH_default_pass; - /** * Callback that frees an instances removing diff --git a/src/backend/taler-merchant-httpd_private-post-instances-ID-auth.c b/src/backend/taler-merchant-httpd_private-post-instances-ID-auth.c @@ -184,17 +184,7 @@ retry: /* Finally, also update our running process */ mi->auth = ias; } - - mi->auth_override = false; - if (0 == strcmp (mi->settings.id, - "admin")) - { - /* The default auth string should've been - cleared with the first request - for the default instance. */ - GNUNET_assert (NULL == TMH_default_pass); - } TMH_reload_instances (mi->settings.id); return TALER_MHD_reply_static (connection, MHD_HTTP_NO_CONTENT, @@ -218,9 +208,10 @@ TMH_private_post_instances_ID_auth (const struct TMH_RequestHandler *rh, MHD_RESULT -TMH_private_post_instances_default_ID_auth (const struct TMH_RequestHandler *rh, - struct MHD_Connection *connection, - struct TMH_HandlerContext *hc) +TMH_private_post_instances_default_ID_auth ( + const struct TMH_RequestHandler *rh, + struct MHD_Connection *connection, + struct TMH_HandlerContext *hc) { struct TMH_MerchantInstance *mi; MHD_RESULT ret; diff --git a/src/backend/taler-merchant-httpd_private-post-instances.c b/src/backend/taler-merchant-httpd_private-post-instances.c @@ -364,13 +364,6 @@ retry: TMH_add_instance (mi)); TMH_reload_instances (mi->settings.id); } - if (0 == strcmp (is.id, - "admin")) - { - GNUNET_free (TMH_default_pass); /* clear it if the default instance was - created */ - } - GNUNET_JSON_parse_free (spec); return TALER_MHD_reply_static (connection, MHD_HTTP_NO_CONTENT, diff --git a/src/backenddb/pg_lookup_instances.c b/src/backenddb/pg_lookup_instances.c @@ -86,6 +86,7 @@ struct LookupInstancesContext static enum GNUNET_DB_QueryStatus prepare (struct PostgresClosure *pg) { + // FIXME: use a LEFT JOIN here! PREPARE (pg, "lookup_instance_private_key", "SELECT" diff --git a/src/merchant-tools/taler-merchant-passwd.c b/src/merchant-tools/taler-merchant-passwd.c @@ -81,6 +81,11 @@ run (void *cls, GNUNET_CRYPTO_random_block (GNUNET_CRYPTO_QUALITY_NONCE, &ias.auth_salt, sizeof (ias.auth_salt)); + fprintf (stderr, + "Hashing %s/%s\n", + TALER_B2S (&ias.auth_salt), + pw); + // FIXME: use TMH_compute_auth here! GNUNET_assert (GNUNET_YES == GNUNET_CRYPTO_kdf (&ias.auth_hash, sizeof (ias.auth_hash), diff --git a/src/testing/test_merchant_instance_auth.sh b/src/testing/test_merchant_instance_auth.sh @@ -92,14 +92,18 @@ setup -c test_template.conf \ -u "exchange-account-2" \ -r "merchant-exchange-default" -NEW_SECRET=different_value +NEW_SECRET="different_value" taler-merchant-exchangekeyupdate \ -c "${CONF}" \ -L DEBUG \ 2> taler-merchant-exchangekeyupdate2.log & +taler-merchant-passwd \ + -c "${CONF}" \ + -L DEBUG \ + "$NEW_SECRET" \ + 2> taler-merchant-passwd.log taler-merchant-httpd \ - -a "${NEW_SECRET}" \ -c "${CONF}" \ -L DEBUG \ 2> taler-merchant-httpd2.log & @@ -131,7 +135,7 @@ fi echo " OK" >&2 -BASIC_AUTH=$(echo -n admin:$NEW_SECRET | base64) +BASIC_AUTH=$(echo -n "admin:$NEW_SECRET" | base64) STATUS=$(curl -H "Content-Type: application/json" -X POST \ -H "Authorization: Basic $BASIC_AUTH" \ diff --git a/src/testing/test_merchant_product_creation.sh b/src/testing/test_merchant_product_creation.sh @@ -52,8 +52,6 @@ setup -c "test_template.conf" \ -em \ $BANK_FLAGS -bash - LAST_RESPONSE=$(mktemp -p "${TMPDIR:-/tmp}" test_response.conf-XXXXXX) WALLET_DB=$(mktemp -p "${TMPDIR:-/tmp}" test_wallet.json-XXXXXX) CONF="test_template.conf.edited"