commit 8211f83ce4e41ec6b3be4dbc58c2fb1ae25ede38
parent ff08d11d8b86c0124555414689a7e8f845efe103
Author: Christian Grothoff <christian@grothoff.org>
Date: Sat, 21 Jun 2025 21:12:38 +0200
-clean up logic, this should now fully fix #9556
Diffstat:
4 files changed, 88 insertions(+), 76 deletions(-)
diff --git a/src/backend/taler-merchant-httpd.c b/src/backend/taler-merchant-httpd.c
@@ -469,32 +469,22 @@ TMH_get_scope_by_name (const char *name)
enum GNUNET_GenericReturnValue
-TMH_check_auth (const char *token,
+TMH_check_auth (const char *password,
struct TALER_MerchantAuthenticationSaltP *salt,
struct TALER_MerchantAuthenticationHashP *hash)
{
struct TALER_MerchantAuthenticationHashP val;
- char *dec;
- size_t dec_len;
if (GNUNET_is_zero (hash))
return GNUNET_OK;
- if (NULL == token)
+ if (NULL == password)
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));
TALER_merchant_instance_auth_hash_with_salt (&val,
salt,
- dec);
- GNUNET_free (dec);
+ password);
return (0 ==
GNUNET_memcmp (&val,
hash))
@@ -504,16 +494,18 @@ TMH_check_auth (const char *token,
/**
- * Check if @a token grants access to @a instance.
+ * Check if @a userpass grants access to @a instance.
*
- * @param token FIXME: clarify
+ * @param userpass base64 encoded "$USERNAME:$PASSWORD" value
+ * from HTTP Basic "Authentication" header
* @param instances the access controlled instance
*/
static enum GNUNET_GenericReturnValue
-check_auth_instance (const char *token,
+check_auth_instance (const char *userpass,
struct TMH_MerchantInstance *instance)
{
char *tmp;
+ char *colon;
const char *instance_name;
const char *password;
const char *target_instance = "admin";
@@ -522,31 +514,26 @@ check_auth_instance (const char *token,
/* implicitly a zeroed out hash means no authentication */
if (GNUNET_is_zero (&instance->auth.auth_hash))
return GNUNET_OK;
- if (NULL == token)
+ if (NULL == userpass)
return GNUNET_SYSERR;
if (0 ==
- GNUNET_STRINGS_base64_decode (token,
- strlen (token),
+ GNUNET_STRINGS_base64_decode (userpass,
+ strlen (userpass),
(void**) &tmp))
{
return GNUNET_SYSERR;
}
- instance_name = strtok (tmp,
- ":");
- if (NULL == instance_name)
- {
- GNUNET_free (tmp);
- return GNUNET_SYSERR;
- }
- // FIXME: why would the password end at the first ':'?
- password = strtok (NULL,
- ":");
- if (NULL == password)
+ colon = strchr (tmp,
+ ':');
+ if (NULL == colon)
{
GNUNET_free (tmp);
return GNUNET_SYSERR;
}
- /* FIXME: how can instance->settings.id be NULL? */
+ *colon = '\0';
+ instance_name = tmp;
+ password = colon + 1;
+ /* instance->settings.id can be NULL if there is no instance yet */
if (NULL != instance->settings.id)
target_instance = instance->settings.id;
if (0 != strcmp (instance_name,
@@ -563,7 +550,6 @@ check_auth_instance (const char *token,
}
-// FIXME: comment!?
void
TMH_compute_auth (const char *token,
struct TALER_MerchantAuthenticationSaltP *salt,
@@ -893,13 +879,24 @@ spa_redirect (const struct TMH_RequestHandler *rh,
/**
* Extract the token from authorization header value @a auth.
+ * The @a auth value can be a bearer token or a Basic
+ * authentication header. In both cases, this function
+ * updates @a auth to point to the actual credential,
+ * skipping spaces.
*
- * @param auth pointer to authorization header value,
+ * NOTE: We probably want to replace this function with MHD2
+ * API calls in the future that are more robust.
+ *
+ * @param[in,out] auth pointer to authorization header value,
* will be updated to point to the start of the token
* or set to NULL if header value is invalid
+ * @param[out] is_basic_auth will be set to true if the
+ * authorization header uses basic authentication,
+ * otherwise to false
*/
static void
-extract_auth (const char **auth)
+extract_auth (const char **auth,
+ bool *is_basic_auth)
{
const char *bearer = "Bearer ";
const char *basic = "Basic ";
@@ -907,6 +904,7 @@ extract_auth (const char **auth)
size_t offset = 0;
bool is_bearer = false;
+ *is_basic_auth = false;
if (0 == strncmp (tok,
bearer,
strlen (bearer)))
@@ -919,6 +917,7 @@ extract_auth (const char **auth)
strlen (basic)))
{
offset = strlen (basic);
+ *is_basic_auth = true;
}
else
{
@@ -2201,12 +2200,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 ")));
- extract_auth (&auth);
+ extract_auth (&auth,
+ &is_basic_auth);
if (NULL == auth)
auth_malformed = true;
hc->auth_token = auth;
@@ -2268,39 +2263,48 @@ url_handler (void *cls,
&hc->auth_scope);
if (TALER_EC_NONE != ec)
{
- // FIXME: Deprecated, remove after 1.1
- if (0 == strncasecmp (auth,
+ char *dec;
+ size_t dec_len;
+ const char *token;
+
+ /* NOTE: Deprecated, remove sometime after v1.1 */
+ if (0 != strncasecmp (auth,
RFC_8959_PREFIX,
strlen (RFC_8959_PREFIX)))
{
- 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),
+ GNUNET_break_op (0);
+ return TALER_MHD_reply_with_ec (connection,
+ ec,
+ NULL);
+ }
+ GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+ "Trying deprecated secret-token:password API authN\n");
+ token = auth + strlen (RFC_8959_PREFIX);
+ dec_len = GNUNET_STRINGS_urldecode (token,
+ strlen (token),
+ &dec);
+ if ( (0 == dec_len) ||
+ (GNUNET_OK !=
+ TMH_check_auth (dec,
&hc->instance->auth.auth_salt,
- &hc->instance->auth.auth_hash))
- {
- GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
- "Login failed\n");
- hc->auth_scope = TMH_AS_NONE;
- }
- else
- {
- hc->auth_scope = TMH_AS_ADMIN;
- }
+ &hc->instance->auth.auth_hash)) )
+ {
+ GNUNET_log (GNUNET_ERROR_TYPE_WARNING,
+ "Login failed\n");
+ hc->auth_scope = TMH_AS_NONE;
}
else
{
- return TALER_MHD_reply_with_ec (connection,
- ec,
- NULL);
+ hc->auth_scope = TMH_AS_ADMIN;
}
+ GNUNET_free (dec);
}
}
}
else
+ {
hc->auth_scope = TMH_AS_NONE;
+ }
}
/* We grant access if:
- scope is 'all'
@@ -2312,11 +2316,16 @@ url_handler (void *cls,
{
if (auth_malformed &&
(TMH_AS_NONE == hc->auth_scope) )
- return TALER_MHD_reply_with_error (connection,
- MHD_HTTP_UNAUTHORIZED,
- TALER_EC_GENERIC_PARAMETER_MALFORMED,
- "'" RFC_8959_PREFIX
- "' prefix or 'Bearer' missing in 'Authorization' header");
+ {
+ GNUNET_break_op (0);
+ return TALER_MHD_reply_with_error (
+ connection,
+ MHD_HTTP_UNAUTHORIZED,
+ TALER_EC_GENERIC_PARAMETER_MALFORMED,
+ "'" RFC_8959_PREFIX
+ "' prefix or 'Bearer' missing in 'Authorization' header");
+ }
+ GNUNET_break_op (0);
return TALER_MHD_reply_with_error (connection,
MHD_HTTP_UNAUTHORIZED,
TALER_EC_MERCHANT_GENERIC_UNAUTHORIZED,
diff --git a/src/backend/taler-merchant-httpd.h b/src/backend/taler-merchant-httpd.h
@@ -834,12 +834,12 @@ TMH_check_auth (const char *token,
* Compute a @a hash from @a token hashes for
* merchant instance authentication.
*
- * @param token the token to check
+ * @param password the password to check
* @param[out] salt set to a fresh random salt
* @param[out] hash set to the hash of @a token under @a salt
*/
void
-TMH_compute_auth (const char *token,
+TMH_compute_auth (const char *password,
struct TALER_MerchantAuthenticationSaltP *salt,
struct TALER_MerchantAuthenticationHashP *hash);
diff --git a/src/backend/taler-merchant-httpd_helper.c b/src/backend/taler-merchant-httpd_helper.c
@@ -616,10 +616,13 @@ TMH_check_auth_config (struct MHD_Connection *connection,
"token"))
{
json_t *pw_value;
- pw_value = json_object_get (jauth, "password");
+
+ pw_value = json_object_get (jauth,
+ "password");
if (NULL == pw_value)
{
- pw_value = json_object_get (jauth, "token");
+ pw_value = json_object_get (jauth,
+ "token");
}
if (NULL == pw_value)
{
diff --git a/src/backend/taler-merchant-httpd_private-post-instances.c b/src/backend/taler-merchant-httpd_private-post-instances.c
@@ -51,7 +51,7 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
{
struct TALER_MERCHANTDB_InstanceSettings is = { 0 };
struct TALER_MERCHANTDB_InstanceAuthSettings ias;
- const char *auth_token = NULL;
+ const char *auth_password = NULL;
const char *uts = "business";
struct TMH_WireMethod *wm_head = NULL;
struct TMH_WireMethod *wm_tail = NULL;
@@ -108,7 +108,7 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
ret = TMH_check_auth_config (connection,
jauth,
- &auth_token);
+ &auth_password);
if (GNUNET_OK != ret)
{
GNUNET_JSON_parse_free (spec);
@@ -208,12 +208,12 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
(NULL != is.logo && NULL != mi->settings.logo &&
0 == strcmp (mi->settings.logo,
is.logo))) &&
- ( ( (NULL != auth_token) &&
+ ( ( (NULL != auth_password) &&
(GNUNET_OK ==
- TMH_check_auth (auth_token,
+ TMH_check_auth (auth_password,
&mi->auth.auth_salt,
&mi->auth.auth_hash)) ) ||
- ( (NULL == auth_token) &&
+ ( (NULL == auth_password) &&
(GNUNET_YES ==
GNUNET_is_zero (&mi->auth.auth_hash))) ) &&
(1 == json_equal (mi->settings.address,
@@ -244,7 +244,7 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
}
/* handle authentication token setup */
- if (NULL == auth_token)
+ if (NULL == auth_password)
{
memset (&ias.auth_salt,
0,
@@ -256,7 +256,7 @@ TMH_private_post_instances (const struct TMH_RequestHandler *rh,
else
{
/* Sets 'auth_salt' and 'auth_hash' */
- TMH_compute_auth (auth_token,
+ TMH_compute_auth (auth_password,
&ias.auth_salt,
&ias.auth_hash);
}
