commit 4cce9f30d17521bdbf2207ad526c9624f164cd2a
parent 06b57c086347ad986c37c0719dbacdcd31dfee9b
Author: Christian Grothoff <christian@grothoff.org>
Date: Thu, 6 Aug 2026 19:29:41 +0200
fix /setup handling of POSTed body, add test
Diffstat:
3 files changed, 371 insertions(+), 1 deletion(-)
diff --git a/src/challenger/challenger-httpd_setup.c b/src/challenger/challenger-httpd_setup.c
@@ -55,6 +55,13 @@ struct SetupContext
*/
json_t *root;
+ /**
+ * Did the client actually send a request body? The body is optional
+ * for this endpoint, so we must not hand an empty upload to the JSON
+ * parser (which would reject it as malformed).
+ */
+ bool have_upload;
+
};
@@ -98,8 +105,16 @@ CH_handler_setup (struct CH_HandlerContext *hc,
hc->ctx = sc;
return MHD_YES;
}
+ /* Remember that a body is coming. We must keep calling the parser after
+ this point even once *upload_data_size has dropped back to zero: that
+ final call is the one where GNUNET_MHD_post_parser() actually decodes
+ the buffered bytes. Conversely, if no body ever arrives we must not
+ call the parser at all, as it would report an empty document as
+ malformed JSON -- the body of this endpoint is optional. */
+ if (0 != *upload_data_size)
+ sc->have_upload = true;
if ( (NULL == sc->root) &&
- (0 != *upload_data_size) )
+ (sc->have_upload) )
{
/* parse byte stream upload into JSON */
enum GNUNET_GenericReturnValue res;
@@ -133,6 +148,17 @@ CH_handler_setup (struct CH_HandlerContext *hc,
stderr,
JSON_INDENT (2));
#endif
+ if (! json_is_object (sc->root))
+ {
+ /* The address is stored verbatim and later packed with
+ GNUNET_JSON_pack_object_steal() by /authorize, which asserts on
+ anything that is not an object; refuse it here instead. */
+ GNUNET_break_op (0);
+ return TALER_MHD_reply_with_error (hc->connection,
+ MHD_HTTP_BAD_REQUEST,
+ TALER_EC_GENERIC_PARAMETER_MALFORMED,
+ "address");
+ }
}
{
diff --git a/src/challenger/meson.build b/src/challenger/meson.build
@@ -23,6 +23,7 @@ check_SCRIPTS = [
'test-challenger-exhaustion',
'test-challenger-auth-errors',
'test-challenger-upload-limits',
+ 'test-challenger-setup-address',
]
test_helper_cat = configure_file(input: 'cat.sh', output: 'cat.sh', copy: true)
diff --git a/src/challenger/test-challenger-setup-address.sh b/src/challenger/test-challenger-setup-address.sh
@@ -0,0 +1,343 @@
+#!/usr/bin/env bash
+# This file is in the public domain.
+#
+# Tests that an address passed in the body of POST /setup/$CLIENT_ID is
+# actually stored and honoured, i.e. protocol v4's "pre-initialize the
+# address" feature and the 'read_only' bit documented for
+# ChallengeSetupRequest.
+#
+# Regression test: CH_handler_setup() gates the body parse on
+# '0 != *upload_data_size', but GNUNET_MHD_post_parser() only finalizes the
+# upload (and thus only runs json_loadb()) on the callback where
+# *upload_data_size IS zero. That callback was therefore skipped, sc->root
+# stayed NULL, and the uploaded address was silently discarded: /authorize
+# reported last_address=null and /challenge happily accepted any address,
+# so 403 TALER_EC_CHALLENGER_CLIENT_FORBIDDEN_READ_ONLY was unreachable.
+#
+# None of the other tests catch this because they all call /setup with an
+# empty body ("-d ''").
+
+set -eu
+
+# Exit, with status code "skip" (no 'real' failure)
+function exit_skip() {
+ echo " SKIP: $1"
+ exit 77
+}
+
+# Exit, with error message (hard failure)
+function exit_fail() {
+ echo " FAIL: $@"
+ exit 1
+}
+
+# Cleanup to run whenever we exit
+function cleanup()
+{
+ for n in $(jobs -p)
+ do
+ kill $n 2> /dev/null || true
+ done
+ rm -f "$LAST_RESPONSE" "$FIXED_FILE" "$OTHER_FILE"
+ wait
+}
+
+LAST_RESPONSE=$(mktemp responseXXXXXX.log)
+FIXED_FILE="test-challenger-setup-address.txt"
+OTHER_FILE="test-challenger-setup-other.txt"
+
+# Install cleanup handler (except for kill -9)
+trap cleanup EXIT
+
+export PATH="$PATH:."
+
+echo -n "Testing for jq"
+jq -h > /dev/null || exit_skip "jq required"
+echo " FOUND"
+echo -n "Testing for curl"
+curl -h > /dev/null || exit_skip "curl required"
+echo " FOUND"
+echo -n "Testing for wget"
+wget -h > /dev/null || exit_skip "wget required"
+echo " FOUND"
+echo -n "Testing for challenger-httpd ..."
+challenger-httpd -h > /dev/null || exit_skip "challenger-httpd required"
+echo " FOUND"
+
+CONF="test-challenger.conf"
+BURL="http://localhost:9967"
+REDIRECT_URI="http://client.example.com/"
+
+echo -n "Initialize challenger database ..."
+challenger-dbinit -r -c "${CONF}" &> dbinit.log
+echo " OK"
+
+echo -n "Add challenger client ..."
+CLIENT_SECRET="secret-token:secret"
+challenger-admin -c "${CONF}" -a "${CLIENT_SECRET}" "${REDIRECT_URI}" &> admin.log
+echo " OK"
+# We just reset the DB, thus the client ID must be 1 here:
+CLIENT_ID=1
+
+echo -n "Start challenger-httpd ..."
+challenger-httpd -L INFO -c "${CONF}" &> httpd.log &
+
+# Wait for challenger to be available
+for n in $(seq 1 50)
+do
+ echo -n "."
+ sleep 0.2
+ OK=0
+ wget --tries=1 --timeout=1 "${BURL}/config" -o /dev/null -O /dev/null >/dev/null || continue
+ OK=1
+ break
+done
+if [ 1 != $OK ]
+then
+ exit_skip "Failed to launch challenger service"
+fi
+echo " OK"
+
+# Run /setup with $1 as the JSON request body, leaving the nonce in NONCE.
+function setup_with_body()
+{
+ local STATUS
+ STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -H "Content-Type: application/json" \
+ -d "$1" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+ if [ "$STATUS" != "200" ]
+ then
+ exit_fail "Expected 200 OK from /setup. Got: $STATUS" $(cat $LAST_RESPONSE)
+ fi
+ NONCE=$(jq -r .nonce < "$LAST_RESPONSE")
+}
+
+# GET /authorize for $NONCE, leaving the JSON body in $LAST_RESPONSE.
+function authorize()
+{
+ local STATUS
+ STATUS=$(curl "${BURL}/authorize/${NONCE}" \
+ -G \
+ -H "Accept: application/json" \
+ --data-urlencode "response_type=code" \
+ --data-urlencode "client_id=${CLIENT_ID}" \
+ --data-urlencode "redirect_uri=${REDIRECT_URI}" \
+ --data-urlencode "state=state" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+ if [ "$STATUS" != "200" ]
+ then
+ exit_fail "Expected 200 OK from /authorize. Got: $STATUS" $(cat $LAST_RESPONSE)
+ fi
+}
+
+
+echo -n "/setup stores a plain pre-initialized address ..."
+setup_with_body "{\"filename\":\"${FIXED_FILE}\"}"
+authorize
+if [ "$(jq -r '.last_address.filename' < "$LAST_RESPONSE")" != "${FIXED_FILE}" ]
+then
+ exit_fail "/authorize did not return the address given to /setup:" \
+ "$(cat $LAST_RESPONSE)"
+fi
+# Without 'read_only' the address must still be editable.
+if [ "$(jq -r .fix_address < "$LAST_RESPONSE")" != "false" ]
+then
+ exit_fail "/authorize fixed an address that was not marked read_only:" \
+ "$(cat $LAST_RESPONSE)"
+fi
+echo " OK"
+
+echo -n "/setup honours the read_only bit ..."
+setup_with_body "{\"read_only\":true,\"filename\":\"${FIXED_FILE}\"}"
+authorize
+if [ "$(jq -r '.last_address.filename' < "$LAST_RESPONSE")" != "${FIXED_FILE}" ]
+then
+ exit_fail "/authorize did not return the read-only address:" \
+ "$(cat $LAST_RESPONSE)"
+fi
+if [ "$(jq -r .fix_address < "$LAST_RESPONSE")" != "true" ]
+then
+ exit_fail "/authorize did not report fix_address for a read_only address:" \
+ "$(cat $LAST_RESPONSE)"
+fi
+if [ "$(jq -r .changes_left < "$LAST_RESPONSE")" != "0" ]
+then
+ exit_fail "/authorize left address changes for a read_only address:" \
+ "$(cat $LAST_RESPONSE)"
+fi
+echo " OK"
+
+echo -n "/challenge refuses to change a read_only address ..."
+STATUS=$(curl "${BURL}/challenge/${NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "filename=${OTHER_FILE}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "403" ]
+then
+ exit_fail "Expected 403 when changing a read_only address. Got: $STATUS" \
+ "$(cat $LAST_RESPONSE)"
+fi
+CODE=$(jq -r .code < "$LAST_RESPONSE")
+# 9760 == TALER_EC_CHALLENGER_CLIENT_FORBIDDEN_READ_ONLY
+if [ "$CODE" != "9760" ]
+then
+ exit_fail "Expected code 9760 (CLIENT_FORBIDDEN_READ_ONLY). Got: $CODE" \
+ "$(cat $LAST_RESPONSE)"
+fi
+if [ -f "${OTHER_FILE}" ]
+then
+ exit_fail "A challenge was transmitted to the refused address ${OTHER_FILE}"
+fi
+echo " OK"
+
+echo -n "/challenge accepts the unchanged read_only address ..."
+STATUS=$(curl "${BURL}/challenge/${NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "filename=${FIXED_FILE}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK from /challenge. Got: $STATUS" \
+ "$(cat $LAST_RESPONSE)"
+fi
+if [ ! -f "${FIXED_FILE}" ]
+then
+ exit_fail "No challenge was transmitted to ${FIXED_FILE}"
+fi
+echo " OK"
+
+echo -n "The PIN sent to the fixed address solves the challenge ..."
+PIN=$(awk '{print $5}' < "${FIXED_FILE}")
+STATUS=$(curl "${BURL}/solve/${NONCE}" \
+ -X POST \
+ -H "Accept: application/json" \
+ --data-urlencode "pin=${PIN}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK from /solve. Got: $STATUS" \
+ "$(cat $LAST_RESPONSE)"
+fi
+if [ "$(jq -r .type < "$LAST_RESPONSE")" != "completed" ]
+then
+ exit_fail "Expected type=completed from /solve. Got: $(cat $LAST_RESPONSE)"
+fi
+echo " OK"
+
+# The body of /setup is optional, so an absent or empty body must keep
+# working. Every other test script relies on this ("-d ''"), and a fix
+# that simply always runs the JSON parser breaks all of them: the parser
+# reports an empty document as malformed JSON.
+echo -n "/setup still accepts an empty body ..."
+STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -d '' \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK for an empty /setup body. Got: $STATUS" \
+ "$(cat $LAST_RESPONSE)"
+fi
+echo " OK"
+
+echo -n "/setup still accepts a request with no body at all ..."
+STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -X POST \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK for a /setup without a body. Got: $STATUS" \
+ "$(cat $LAST_RESPONSE)"
+fi
+echo " OK"
+
+echo -n "/setup accepts an empty JSON object ..."
+STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -H "Content-Type: application/json" \
+ -d '{}' \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK for an empty JSON object. Got: $STATUS" \
+ "$(cat $LAST_RESPONSE)"
+fi
+echo " OK"
+
+# A chunked upload has no Content-Length, so the body is only recognized
+# by the fact that data actually arrived.
+echo -n "/setup stores an address sent with chunked encoding ..."
+STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -H "Content-Type: application/json" \
+ -H "Transfer-Encoding: chunked" \
+ -d "{\"filename\":\"${FIXED_FILE}\"}" \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Expected 200 OK for a chunked /setup body. Got: $STATUS" \
+ "$(cat $LAST_RESPONSE)"
+fi
+NONCE=$(jq -r .nonce < "$LAST_RESPONSE")
+authorize
+if [ "$(jq -r '.last_address.filename' < "$LAST_RESPONSE")" != "${FIXED_FILE}" ]
+then
+ exit_fail "/authorize did not return the chunked address:" \
+ "$(cat $LAST_RESPONSE)"
+fi
+echo " OK"
+
+# The stored address is later packed with GNUNET_JSON_pack_object_steal()
+# by /authorize, which GNUNET_assert()s on anything that is not an object.
+# Storing a non-object would therefore take the daemon down on the very
+# next /authorize, so /setup has to refuse it up front.
+echo -n "A non-object /setup body is rejected ..."
+STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -H "Content-Type: application/json" \
+ -d '[1,2]' \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "400" ]
+then
+ exit_fail "Expected 400 for a JSON array body. Got: $STATUS" \
+ "$(cat $LAST_RESPONSE)"
+fi
+STATUS=$(curl "${BURL}/config" -w "%{http_code}" -s -o /dev/null)
+if [ "$STATUS" != "200" ]
+then
+ exit_fail "Service died after a non-object /setup body"
+fi
+echo " OK"
+
+echo -n "A malformed /setup body is rejected ..."
+STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -H "Content-Type: application/json" \
+ -d '{not json' \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "400" ]
+then
+ exit_fail "Expected 400 for a body that is not JSON. Got: $STATUS" \
+ "$(cat $LAST_RESPONSE)"
+fi
+echo " OK"
+
+echo -n "A non-boolean read_only is rejected ..."
+STATUS=$(curl "${BURL}/setup/${CLIENT_ID}" \
+ -H "Authorization: Bearer ${CLIENT_SECRET}" \
+ -H "Content-Type: application/json" \
+ -d '{"read_only":"yes"}' \
+ -w "%{http_code}" -s -o $LAST_RESPONSE)
+if [ "$STATUS" != "400" ]
+then
+ exit_fail "Expected 400 for a non-boolean read_only. Got: $STATUS" \
+ "$(cat $LAST_RESPONSE)"
+fi
+echo " OK"
+
+exit 0
