commit 208ab476cf9d4268cec169afe31787957f369d82
parent c851cae63bef921a6264a904da1a573bd6f9869a
Author: Christian Grothoff <christian@grothoff.org>
Date: Mon, 16 Sep 2024 13:25:32 +0200
various minor fixes, most importantly correct DB migration
Diffstat:
8 files changed, 207 insertions(+), 126 deletions(-)
diff --git a/src/challenger/challenger-httpd_authorize.c b/src/challenger/challenger-httpd_authorize.c
@@ -63,7 +63,7 @@ CH_handler_authorize (struct CH_HandlerContext *hc,
const char *state;
const char *scope;
const char *code_challenge;
- const char *code_challenge_method;
+ enum CHALLENGER_CM code_challenge_method_enum;
struct CHALLENGER_ValidationNonceP nonce;
(void) upload_data;
@@ -145,44 +145,41 @@ CH_handler_authorize (struct CH_HandlerContext *hc,
MHD_GET_ARGUMENT_KIND,
"redirect_uri");
- code_challenge = MHD_lookup_connection_value (hc->connection,
- MHD_GET_ARGUMENT_KIND,
- "code_challenge");
-
- code_challenge_method = MHD_lookup_connection_value (hc->connection,
- MHD_GET_ARGUMENT_KIND,
- "code_challenge_method");
-
- enum CHALLENGER_CM code_challenge_method_enum = CHALLENGER_cm_from_string (
- code_challenge_method);
+ {
+ const char *code_challenge_method;
+ code_challenge_method
+ = MHD_lookup_connection_value (hc->connection,
+ MHD_GET_ARGUMENT_KIND,
+ "code_challenge_method");
+ code_challenge_method_enum
+ = CHALLENGER_cm_from_string (
+ code_challenge_method);
+ }
if (CHALLENGER_CM_UNKNOWN == code_challenge_method_enum)
{
+ GNUNET_break_op (0);
return reply_error (hc,
"invalid-request",
MHD_HTTP_BAD_REQUEST,
TALER_EC_GENERIC_PARAMETER_MALFORMED,
"Unsupported code_challenge_method, supported only \"plain\", \"S256\".");
}
-
- if (NULL != code_challenge)
- {
- if (NULL == code_challenge_method)
- code_challenge_method_enum = CHALLENGER_CM_PLAIN;
- }
+ code_challenge = MHD_lookup_connection_value (hc->connection,
+ MHD_GET_ARGUMENT_KIND,
+ "code_challenge");
+ /* If we have a code challenge, we default to PLAIN instead of EMPTY */
+ if ( (NULL != code_challenge) &&
+ (CHALLENGER_CM_EMPTY == code_challenge_method_enum) )
+ code_challenge_method_enum = CHALLENGER_CM_PLAIN;
/**
- * Safe check to not allow public clients without s256 code_challenge
+ * Safety check to not allow public clients without s256 code_challenge
*/
if ( (NULL != redirect_uri) &&
- (0 != strncmp (redirect_uri,
- "http://",
- strlen ("http://"))) &&
- (0 != strncmp (redirect_uri,
- "https://",
- strlen ("https://"))) &&
- ((CHALLENGER_CM_EMPTY == code_challenge_method_enum) ||
- (CHALLENGER_CM_PLAIN == code_challenge_method_enum) ) )
+ (! TALER_is_web_url (redirect_uri)) &&
+ ( (CHALLENGER_CM_EMPTY == code_challenge_method_enum) ||
+ (CHALLENGER_CM_PLAIN == code_challenge_method_enum) ) )
{
GNUNET_break_op (0);
return reply_error (
diff --git a/src/challenger/challenger-httpd_config.c b/src/challenger/challenger-httpd_config.c
@@ -28,7 +28,7 @@
*
* 0: original design
* 1: revision to support SPA
- * 2:
+ * 2: add support to restrict addresses by REGEX and a few other SPA enhancements
* 3: added support for RFC7636
*/
diff --git a/src/challenger/challenger-httpd_token.c b/src/challenger/challenger-httpd_token.c
@@ -371,6 +371,7 @@ CH_handler_token (struct CH_HandlerContext *hc,
uint32_t code_challenge_method;
enum GNUNET_DB_QueryStatus qs;
char *code;
+ enum CHALLENGER_CM code_challenge_method_enum;
qs = CH_db->validation_get_pkce (CH_db->cls,
&bc->nonce,
@@ -405,11 +406,11 @@ CH_handler_token (struct CH_HandlerContext *hc,
break;
}
- enum CHALLENGER_CM code_challenge_method_enum = CHALLENGER_cm_from_int (
+ code_challenge_method_enum = CHALLENGER_cm_from_int (
code_challenge_method);
-
if (CHALLENGER_CM_UNKNOWN == code_challenge_method_enum)
{
+ GNUNET_break (0);
return TALER_MHD_reply_with_error (
hc->connection,
MHD_HTTP_INTERNAL_SERVER_ERROR,
@@ -436,84 +437,119 @@ CH_handler_token (struct CH_HandlerContext *hc,
"code_verifier is missing");
}
- if (CHALLENGER_CM_S256 == code_challenge_method_enum)
+ switch (code_challenge_method_enum)
{
- gcry_md_hd_t hd;
- unsigned char hash[32];
- char *encoded_hash = NULL;
- size_t encoded_len;
-
- if (GPG_ERR_NO_ERROR != gcry_md_open (&hd, GCRY_MD_SHA256, 0))
- {
- GNUNET_break_op (0);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
- GNUNET_free (code_challenge);
- return TALER_MHD_reply_with_oauth_error (
- hc->connection,
- MHD_HTTP_INTERNAL_SERVER_ERROR,
- "server_error",
- TALER_EC_CHALLENGER_HELPER_EXEC_FAILED,
- "Failed to initialize SHA256 hash function");
- }
- gcry_md_write (hd, bc->code_verifier, strlen (bc->code_verifier));
- memcpy (hash, gcry_md_read (hd, 0), 32);
- gcry_md_close (hd);
-
- // Perform base64url encoding
- encoded_len = GNUNET_STRINGS_base64url_encode (hash, 32, &encoded_hash);
-
- if ((0 == encoded_len) || (NULL == encoded_hash))
+ case CHALLENGER_CM_S256:
{
- GNUNET_break_op (0);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
- GNUNET_free (code_challenge);
- return TALER_MHD_reply_with_oauth_error (
- hc->connection,
- MHD_HTTP_INTERNAL_SERVER_ERROR,
- "server_error",
- TALER_EC_CHALLENGER_HELPER_EXEC_FAILED,
- "Failed to encode hash to Base64 URL");
+ gcry_md_hd_t hd;
+ unsigned char hash[32];
+ char *encoded_hash = NULL;
+ size_t encoded_len;
+ const void *md;
+
+ if (GPG_ERR_NO_ERROR !=
+ gcry_md_open (&hd,
+ GCRY_MD_SHA256,
+ 0))
+ {
+ GNUNET_break (0);
+ GNUNET_free (client_scope);
+ GNUNET_free (client_secret);
+ GNUNET_free (client_redirect_uri);
+ GNUNET_free (client_state);
+ GNUNET_free (code_challenge);
+ return TALER_MHD_reply_with_oauth_error (
+ hc->connection,
+ MHD_HTTP_INTERNAL_SERVER_ERROR,
+ "server_error",
+ TALER_EC_GENERIC_INTERNAL_INVARIANT_FAILURE,
+ "Failed to initialize SHA256 hash function");
+ }
+ gcry_md_write (hd,
+ bc->code_verifier,
+ strlen (bc->code_verifier));
+ md = gcry_md_read (hd,
+ 0);
+ GNUNET_assert (NULL != md);
+ memcpy (hash,
+ md,
+ sizeof (hash));
+ gcry_md_close (hd);
+
+ encoded_len
+ = GNUNET_STRINGS_base64url_encode (hash,
+ sizeof (hash),
+ &encoded_hash);
+
+ if ( (0 == encoded_len) ||
+ (NULL == encoded_hash) )
+ {
+ GNUNET_break (0);
+ GNUNET_free (client_scope);
+ GNUNET_free (client_secret);
+ GNUNET_free (client_redirect_uri);
+ GNUNET_free (client_state);
+ GNUNET_free (code_challenge);
+ return TALER_MHD_reply_with_oauth_error (
+ hc->connection,
+ MHD_HTTP_INTERNAL_SERVER_ERROR,
+ "server_error",
+ TALER_EC_GENERIC_INTERNAL_INVARIANT_FAILURE,
+ "Failed to encode hash to Base64 URL");
+ }
+
+ if (0 != strcmp (encoded_hash,
+ code_challenge))
+ {
+ GNUNET_break_op (0);
+ GNUNET_free (client_scope);
+ GNUNET_free (client_secret);
+ GNUNET_free (client_redirect_uri);
+ GNUNET_free (client_state);
+ GNUNET_free (code_challenge);
+ return TALER_MHD_reply_with_oauth_error (
+ hc->connection,
+ MHD_HTTP_UNAUTHORIZED,
+ "invalid_grant",
+ TALER_EC_CHALLENGER_CLIENT_FORBIDDEN_BAD_CODE,
+ "code_verifier does not match code_challenge (SHA256)");
+ }
}
-
- if (0 != strcmp (encoded_hash, code_challenge))
+ break;
+ case CHALLENGER_CM_PLAIN:
{
- GNUNET_break_op (0);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
- GNUNET_free (code_challenge);
- return TALER_MHD_reply_with_oauth_error (
- hc->connection,
- MHD_HTTP_UNAUTHORIZED,
- "invalid_grant",
- TALER_EC_CHALLENGER_CLIENT_FORBIDDEN_BAD_CODE,
- "code_verifier does not match code_challenge");
- }
- }
- else if (CHALLENGER_CM_PLAIN == code_challenge_method_enum)
- {
- if (0 != strcmp (bc->code_verifier, code_challenge))
- {
- GNUNET_break_op (0);
- GNUNET_free (client_scope);
- GNUNET_free (client_secret);
- GNUNET_free (client_redirect_uri);
- GNUNET_free (client_state);
- GNUNET_free (code_challenge);
- return TALER_MHD_reply_with_oauth_error (
- hc->connection,
- MHD_HTTP_UNAUTHORIZED,
- "invalid_grant",
- TALER_EC_CHALLENGER_CLIENT_FORBIDDEN_BAD_CODE,
- "code_verifier does not match code_challenge");
+ if (0 != strcmp (bc->code_verifier,
+ code_challenge))
+ {
+ GNUNET_break_op (0);
+ GNUNET_free (client_scope);
+ GNUNET_free (client_secret);
+ GNUNET_free (client_redirect_uri);
+ GNUNET_free (client_state);
+ GNUNET_free (code_challenge);
+ return TALER_MHD_reply_with_oauth_error (
+ hc->connection,
+ MHD_HTTP_UNAUTHORIZED,
+ "invalid_grant",
+ TALER_EC_CHALLENGER_CLIENT_FORBIDDEN_BAD_CODE,
+ "code_verifier does not match code_challenge (PLAIN)");
+ }
}
+ break;
+ case CHALLENGER_CM_UNKNOWN:
+ case CHALLENGER_CM_EMPTY:
+ GNUNET_break (0);
+ GNUNET_free (client_scope);
+ GNUNET_free (client_secret);
+ GNUNET_free (client_redirect_uri);
+ GNUNET_free (client_state);
+ GNUNET_free (code_challenge);
+ return TALER_MHD_reply_with_oauth_error (
+ hc->connection,
+ MHD_HTTP_INTERNAL_SERVER_ERROR,
+ "server_error",
+ TALER_EC_GENERIC_DB_INVARIANT_FAILURE,
+ "Database has empty or unknown challenge mode but with code_challenge");
}
}
diff --git a/src/challenger/challenger_cm_enums.c b/src/challenger/challenger_cm_enums.c
@@ -19,7 +19,6 @@
* @author Bohdan Potuzhnyi
* @author Vlada Svirsh
*/
-
#include "challenger_cm_enums.h"
#include <string.h>
#include <stdint.h>
@@ -28,16 +27,18 @@
enum CHALLENGER_CM
CHALLENGER_cm_from_string (const char *method_str)
{
- if ((NULL == method_str) || (0 == strcmp (method_str, "")))
+ if ( (NULL == method_str) ||
+ (0 == strcmp (method_str,
+ "")) )
return CHALLENGER_CM_EMPTY;
-
- if (0 == strcmp (method_str, "plain"))
+ if (0 == strcmp (method_str,
+ "plain"))
return CHALLENGER_CM_PLAIN;
-
- if ((0 == strcmp (method_str, "S256")) || (0 == strcmp (method_str,
- "sha256")))
+ if ( (0 == strcmp (method_str,
+ "S256")) ||
+ (0 == strcmp (method_str,
+ "sha256")))
return CHALLENGER_CM_S256;
-
return CHALLENGER_CM_UNKNOWN;
}
@@ -54,6 +55,7 @@ CHALLENGER_cm_from_int (uint32_t method_int)
case 2:
return CHALLENGER_CM_S256;
default:
- return CHALLENGER_CM_UNKNOWN; // Invalid or unrecognized value
+ /* Invalid or unrecognized value */
+ return CHALLENGER_CM_UNKNOWN;
}
-}
-\ No newline at end of file
+}
diff --git a/src/challenger/challenger_cm_enums.h b/src/challenger/challenger_cm_enums.h
@@ -25,11 +25,29 @@
#include <stdint.h>
+/**
+ * Different types of RFC 7636 code_challenges.
+ */
enum CHALLENGER_CM
{
+ /**
+ * No code challenge set.
+ */
CHALLENGER_CM_EMPTY,
+
+ /**
+ * Plain mode, challenge is equal to the verifier.
+ */
CHALLENGER_CM_PLAIN,
+
+ /**
+ * SHA-256 mode, code_challenge = BASE64URL-ENCODE(SHA256(code_verifier))
+ */
CHALLENGER_CM_S256,
+
+ /**
+ * Unknown mode (unsupported input).
+ */
CHALLENGER_CM_UNKNOWN
};
@@ -55,4 +73,4 @@ enum CHALLENGER_CM
CHALLENGER_cm_from_int (uint32_t method_int);
-#endif /* CHALLENGER_CM_ENUMS_H */
-\ No newline at end of file
+#endif /* CHALLENGER_CM_ENUMS_H */
diff --git a/src/challengerdb/Makefile.am b/src/challengerdb/Makefile.am
@@ -30,14 +30,12 @@ sql_DATA = \
versioning.sql \
procedures.sql \
challenger-0001.sql \
+ challenger-0002.sql \
drop.sql
BUILT_SOURCES = \
procedures.sql
-CLEANFILES = \
- exchange-0002.sql
-
procedures.sql: procedures.sql.in challenger_do_*.sql
chmod +w $@ || true
gcc -E -P -undef - < procedures.sql.in 2>/dev/null | sed -e "s/--.*//" | awk 'NF' - >$@
diff --git a/src/challengerdb/challenger-0001.sql b/src/challengerdb/challenger-0001.sql
@@ -61,10 +61,7 @@ CREATE TABLE IF NOT EXISTS validations
,client_redirect_uri VARCHAR
);
- -- Add columns for PKCE (RFC 7636)
-ALTER TABLE validations
-ADD COLUMN IF NOT EXISTS code_challenge VARCHAR,
-ADD COLUMN IF NOT EXISTS code_challenge_method INT4 DEFAULT(0);
+
COMMENT ON TABLE validations
IS 'Active validations where we send a challenge to an address of a user';
@@ -92,10 +89,6 @@ COMMENT ON COLUMN validations.last_tx_time
IS 'When did we last sent the challenge (guard against DDoS)';
COMMENT ON COLUMN validations.expiration_time
IS 'When will the challenge expire';
-COMMENT ON COLUMN validations.code_challenge
- IS 'Code challenge used for PKCE';
-COMMENT ON COLUMN validations.code_challenge_method
- IS 'Code challenge method used for PKCE (plain, s256)';
CREATE INDEX IF NOT EXISTS validations_serial
ON validations (validation_serial_id);
diff --git a/src/challengerdb/challenger-0002.sql b/src/challengerdb/challenger-0002.sql
@@ -0,0 +1,39 @@
+--
+-- This file is part of Challenger
+-- Copyright (C) 2023 Taler Systems SA
+--
+-- Challenger 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.
+--
+-- Challenger 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
+-- Challenger; see the file COPYING. If not, see <http://www.gnu.org/licenses/>
+--
+
+-- Everything in one big transaction
+BEGIN;
+
+-- Check patch versioning is in place.
+SELECT _v.register_patch('challenger-0002', NULL, NULL);
+
+CREATE SCHEMA challenger;
+COMMENT ON SCHEMA challenger IS 'challenger data';
+
+SET search_path TO challenger;
+
+
+-- Add columns for PKCE (RFC 7636)
+ALTER TABLE validations
+ ADD COLUMN IF NOT EXISTS code_challenge VARCHAR,
+ ADD COLUMN IF NOT EXISTS code_challenge_method INT4 DEFAULT(0);
+
+COMMENT ON COLUMN validations.code_challenge
+ IS 'Code challenge used for PKCE';
+COMMENT ON COLUMN validations.code_challenge_method
+ IS 'Code challenge method used for PKCE (plain, s256)';
+
+COMMIT;
