challenger

commit cd01a7898a49f86ee470cfe6233de0a9209944a9
parent c63b925b9ddaf440d044dd7e881096e45ff7c950
Author: Christian Grothoff <christian@grothoff.org>
Date:   Thu,  6 Aug 2026 14:40:39 +0200

prevent PKCE downgrade

Diffstat:
M
src/challenger/meson.build
 | 
1
+
M
src/challengerdb/update_validation.c
 | 
17
+++++++++++++++--
M
src/include/challenger-database/update_validation.h
 | 
7
+++++--

3 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/challenger/meson.build b/src/challenger/meson.build
@@ -10,6 +10,7 @@ install_data('challenger.conf', install_dir: pkgcfgdir)
 check_SCRIPTS = [
     'test-challenger',
     'test-challenger-pkce',
+    'test-challenger-pkce-downgrade',
     'test-challenger-revisit',
     'test-challenger-badutf8',
     'test-challenger-pinlimit',
diff --git a/src/challengerdb/update_validation.c b/src/challengerdb/update_validation.c
@@ -85,14 +85,27 @@ CHALLENGERDB_update_validation (
   };
 
   *last_address = NULL;
+  /* A repeated /authorize must never *weaken* an existing PKCE binding
+     (RFC 7636): /authorize authenticates nobody (the client_id is a plain
+     query argument) and the nonce is recoverable from an issued code, so
+     replaying /authorize without a code_challenge would otherwise strip the
+     binding from a validation that already had one.  Hence COALESCE, just
+     like for client_redirect_uri above.  code_challenge_method must move
+     with the challenge it describes: the column is NOT NULL DEFAULT 0, so
+     writing it unconditionally would leave a retained challenge with the
+     method of the request that tried to drop it. */
   PREPARE (ctx,
            "update_validation",
            "UPDATE validations SET"
            "  client_scope=$3"
            " ,client_state=$4"
            " ,client_redirect_uri=COALESCE($5::VARCHAR,client_redirect_uri)"
-           " ,code_challenge=$6"
-           " ,code_challenge_method=$7"
+           " ,code_challenge=COALESCE($6::VARCHAR,code_challenge)"
+           " ,code_challenge_method=CASE"
+           "    WHEN $6::VARCHAR IS NULL"
+           "    THEN code_challenge_method"
+           "    ELSE $7"
+           "  END"
            " WHERE nonce=$1"
            "   AND client_serial_id=$2"
            "   AND expiration_time > $8"
diff --git a/src/include/challenger-database/update_validation.h b/src/include/challenger-database/update_validation.h
@@ -42,8 +42,11 @@
  * @param client_scope scope of the validation
  * @param client_state state of the client
  * @param client_redirect_uri where to redirect at the end, NULL to use a unique one registered for the client
- * @param code_challenge PKCE code challenge
- * @param code_challenge_method PKCE code challenge method enum
+ * @param code_challenge PKCE code challenge, NULL to keep the one already
+ *        stored (a repeated authorization may add a code challenge, but it
+ *        may never remove one)
+ * @param code_challenge_method PKCE code challenge method enum; ignored if
+ *        @a code_challenge is NULL
  * @param[out] last_address set to the last address used
  * @param[out] address_attempts_left set to number of address changing attempts left for this address
  * @param[out] pin_transmissions_left set to number of times the PIN can still be re-requested