From eaa6078e9fe70940c359cca0883227746456fc94 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Thu, 13 Feb 2025 23:45:50 +0100 Subject: [PATCH v49 4/4] v49 Fixups proposed by Daniel --- config/programs.m4 | 4 +- doc/src/sgml/client-auth.sgml | 8 +- doc/src/sgml/installation.sgml | 10 +-- doc/src/sgml/libpq.sgml | 25 +++--- doc/src/sgml/oauth-validators.sgml | 27 ++++--- meson.build | 8 +- src/backend/libpq/auth-oauth.c | 79 +++++++++++++------ src/include/common/oauth-common.h | 2 +- src/include/libpq/oauth.h | 7 +- src/include/pg_config.h.in | 2 +- src/interfaces/libpq/fe-auth-oauth-curl.c | 18 +++-- src/interfaces/libpq/fe-auth-oauth.c | 4 +- src/test/modules/oauth_validator/Makefile | 2 +- src/test/modules/oauth_validator/README | 6 +- .../modules/oauth_validator/fail_validator.c | 6 +- .../modules/oauth_validator/magic_validator.c | 48 +++++++++++ src/test/modules/oauth_validator/meson.build | 18 ++++- .../oauth_validator/oauth_hook_client.c | 2 +- .../modules/oauth_validator/t/001_server.pl | 31 ++++++-- .../modules/oauth_validator/t/002_client.pl | 2 +- .../modules/oauth_validator/t/OAuth/Server.pm | 4 +- src/test/modules/oauth_validator/validator.c | 2 +- 22 files changed, 220 insertions(+), 95 deletions(-) create mode 100644 src/test/modules/oauth_validator/magic_validator.c diff --git a/config/programs.m4 b/config/programs.m4 index ead427046f5..061b13376ac 100644 --- a/config/programs.m4 +++ b/config/programs.m4 @@ -280,7 +280,7 @@ AC_DEFUN([PGAC_CHECK_STRIP], # PGAC_CHECK_LIBCURL # ------------------ # Check for required libraries and headers, and test to see whether the current -# installation of libcurl is threadsafe. +# installation of libcurl is thread-safe. AC_DEFUN([PGAC_CHECK_LIBCURL], [ @@ -313,7 +313,7 @@ AC_DEFUN([PGAC_CHECK_LIBCURL], [pgac_cv__libcurl_threadsafe_init=unknown])]) if test x"$pgac_cv__libcurl_threadsafe_init" = xyes ; then AC_DEFINE(HAVE_THREADSAFE_CURL_GLOBAL_INIT, 1, - [Define to 1 if curl_global_init() is guaranteed to be threadsafe.]) + [Define to 1 if curl_global_init() is guaranteed to be thread-safe.]) fi # Warn if a thread-friendly DNS resolver isn't built. diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index f84085dbac4..6fc0da57f1b 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -2397,7 +2397,7 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" Resource Server - The system which hosts the protected resources which are + The system hosting the protected resources which are accessed by the client. The PostgreSQL cluster being connected to is the resource server. @@ -2409,7 +2409,7 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" The organization, product vendor, or other entity which develops and/or - administers the OAuth servers and clients for a given application. + administers the OAuth resource servers and clients for a given application. Different providers typically choose different implementation details for their OAuth systems; a client of one provider is not generally guaranteed to have access to the servers of another. @@ -2432,7 +2432,7 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" The system which receives requests from, and issues access tokens to, the client after the authenticated resource owner has given approval. PostgreSQL does not provide an authorization - server; it's obtained from the OAuth provider. + server; it is the responsibility of the OAuth provider. @@ -2500,7 +2500,7 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" exactly match the issuer identifier which is provided in the discovery document, which must in turn match the client's setting. No variations in - case or format are permitted. + case or formatting are permitted. diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml index 96e433179b9..3c95c15a1e4 100644 --- a/doc/src/sgml/installation.sgml +++ b/doc/src/sgml/installation.sgml @@ -1148,8 +1148,8 @@ build-postgresql: Build with libcurl support for OAuth 2.0 client flows. - This requires the curl package to be - installed. Building with this will check for the required header files + Libcurl version 7.61.0 or later is required for this feature. + Building with this will check for the required header files and libraries to make sure that your curl installation is sufficient before proceeding. @@ -2602,9 +2602,9 @@ ninja install Build with libcurl support for OAuth 2.0 client flows. - This requires the curl package to be - installed. Building with this will check for the required header files - and libraries to make sure that your curl + Libcurl version 7.61.0 or later is required for this feature. + Building with this will check for the required header files + and libraries to make sure that your Curl installation is sufficient before proceeding. The default for this option is auto. diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index b2abae8deee..ca84226755d 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -2390,7 +2390,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname The HTTPS URL of a trusted issuer to contact if the server requests an OAuth token for the connection. This parameter is required for all OAuth connections; it should exactly match the issuer - setting in the server's HBA configuration. + setting in the server's HBA configuration. As part of the standard authentication handshake, libpq @@ -2399,8 +2399,9 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname provide a URL that is directly constructed from the components of the oauth_issuer, and this value must exactly match the issuer identifier that is declared in the discovery document itself, or - the connection will fail. This is required to prevent a class of "mix-up - attacks" on OAuth clients. + the connection will fail. This is required to prevent a class of + + "mix-up attacks" on OAuth clients. You may also explicitly set oauth_issuer to the @@ -10140,7 +10141,7 @@ void PQinitSSL(int do_ssl); requests a bearer token during authentication. This flow can be utilized even if the system running the client application does not have a usable web browser, for example when - running a client via SSH. Client applications may implement their own flows + running a client via SSH. Client applications may implement their own flows instead; see . @@ -10152,11 +10153,11 @@ Visit https://example.com/device and enter the code: ABCD-EFGH (This prompt may be customized.) - You will then log into your OAuth provider, which will ask whether you want - to allow libpq and the server to perform actions on your behalf. It is always + The user will then log into their OAuth provider, which will ask whether + to allow libpq and the server to perform actions on their behalf. It is always a good idea to carefully review the URL and permissions displayed, to ensure - they match your expectations, before continuing. Do not give permissions to - untrusted third parties. + they match expectations, before continuing. Permissions should not be given + to untrusted third parties. For an OAuth client flow to be usable, the connection string must at minimum @@ -10199,7 +10200,7 @@ void PQsetAuthDataHook(PQauthDataHook_type hook); int hook_fn(PGauthData type, PGconn *conn, void *data); - which libpq will call when when action is + which libpq will call when an action is required of the application. type describes the request being made, conn is the connection handle being authenticated, and data @@ -10431,7 +10432,7 @@ typedef struct _PGoauthBearerRequest - sprays HTTP traffic (containing several critical secrets) to standard + prints HTTP traffic (containing several critical secrets) to standard error during the OAuth flow @@ -10526,13 +10527,13 @@ int PQisthreadsafe(); - Similarly, if you are using Curl inside your application, + Similarly, if you are using Curl inside your application, and you do not already initialize libcurl globally before starting new threads, you will need to cooperatively lock (again via PQregisterThreadLock) around any code that may initialize libcurl. This restriction is lifted for - more recent versions of Curl that are built to support threadsafe + more recent versions of Curl that are built to support thread-safe initialization; those builds can be identified by the advertisement of a threadsafe feature in their version metadata. diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml index eb8c4431c2d..e9d28d3daea 100644 --- a/doc/src/sgml/oauth-validators.sgml +++ b/doc/src/sgml/oauth-validators.sgml @@ -10,8 +10,8 @@ custom modules to perform server-side validation of OAuth bearer tokens. Because OAuth implementations vary so wildly, and bearer token validation is heavily dependent on the issuing party, the server cannot check the token - itself; validator modules provide the glue between the server and the OAuth - provider in use. + itself; validator modules provide the integration layer between the server + and the OAuth provider in use. OAuth validator modules must at least consist of an initialization function @@ -21,7 +21,7 @@ Since a misbehaving validator might let unauthorized users into the database, - correct implementation is critical. See + validating the correctness of the implementation is critical. See for design considerations. @@ -290,8 +290,9 @@ _PG_oauth_validator_module_init - An OAuth validator module is loaded by dynamically loading one of the shared + OAuth validator modules are dynamically loaded from the shared libraries listed in . + Modules are loaded on demand when requested from a login in progress. The normal library search path is used to locate the library. To provide the validator callbacks and to indicate that the library is an OAuth validator module a function named @@ -356,7 +357,7 @@ typedef bool (*ValidatorValidateCB) (const ValidatorModuleState *state, token will contain the bearer token to validate. - The server has ensured that the token is well-formed syntactically, but no + libpq has ensured that the token is well-formed syntactically, but no other validation has been performed. role will contain the role the user has requested to log in as. The callback must set output parameters in the result struct, which is @@ -371,10 +372,10 @@ typedef struct ValidatorModuleResult The connection will only proceed if the module sets - authorized to true. To + result->authorized to true. To authenticate the user, the authenticated user name (as determined using the - token) shall be palloc'd and returned in the authn_id - field. Alternatively, authn_id may be set to + token) shall be palloc'd and returned in the result->authn_id + field. Alternatively, result->authn_id may be set to NULL if the token is valid but the associated user identity cannot be determined. @@ -386,12 +387,12 @@ typedef struct ValidatorModuleResult The behavior after validate_cb returns depends on the - specific HBA setup. Normally, the authn_id user + specific HBA setup. Normally, the result->authn_id user name must exactly match the role that the user is logging in as. (This behavior may be modified with a usermap.) But when authenticating against - an HBA rule with delegate_ident_mapping turned on, the - server will not perform any checks on the value of - authn_id at all; in this case it is up to the + an HBA rule with delegate_ident_mapping turned on, + PostgreSQL will not perform any checks on the value of + result->authn_id at all; in this case it is up to the validator to ensure that the token carries enough privileges for the user to log in under the indicated role. @@ -402,7 +403,7 @@ typedef struct ValidatorModuleResult The shutdown_cb callback is executed when the backend process associated with the connection exits. If the validator module has - any state, this callback should free it to avoid resource leaks. + any allocated state, this callback should free it to avoid resource leaks. typedef void (*ValidatorShutdownCB) (ValidatorModuleState *state); diff --git a/meson.build b/meson.build index 5c35159b4f1..574f992ed49 100644 --- a/meson.build +++ b/meson.build @@ -867,7 +867,7 @@ if not libcurlopt.disabled() if libcurl.found() cdata.set('USE_LIBCURL', 1) - # Check to see whether the current platform supports threadsafe Curl + # Check to see whether the current platform supports thread-safe Curl # initialization. libcurl_threadsafe_init = false @@ -897,11 +897,11 @@ if not libcurlopt.disabled() assert(r.compiled()) if r.returncode() == 0 libcurl_threadsafe_init = true - message('curl_global_init is threadsafe') + message('curl_global_init is thread-safe') elif r.returncode() == 1 - message('curl_global_init is not threadsafe') + message('curl_global_init is not thread-safe') else - message('curl_global_init failed; assuming not threadsafe') + message('curl_global_init failed; assuming not thread-safe') endif endif diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index e2b5d1ed913..db56cd8c200 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -4,9 +4,9 @@ * Server-side implementation of the SASL OAUTHBEARER mechanism. * * See the following RFC for more details: - * - RFC 7628: https://tools.ietf.org/html/rfc7628 + * - RFC 7628: https://datatracker.ietf.org/doc/html/rfc7628 * - * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * * src/backend/libpq/auth-oauth.c @@ -70,7 +70,7 @@ struct oauth_ctx const char *scope; }; -static char *sanitize_char(char c); +static void sanitize_char(char c, char *buf, size_t buflen); static char *parse_kvpairs_for_auth(char **input); static void generate_error_response(struct oauth_ctx *ctx, char **output, int *outputlen); static bool validate(Port *port, const char *auth); @@ -139,6 +139,7 @@ oauth_exchange(void *opaq, const char *input, int inputlen, char cbind_flag; char *auth; int status; + char errmsgbuf[5]; struct oauth_ctx *ctx = opaq; @@ -162,6 +163,7 @@ oauth_exchange(void *opaq, const char *input, int inputlen, /* * Check that the input length agrees with the string length of the input. + * Possible reasons for discrepancies include embedded nulls in the string. */ if (inputlen == 0) ereport(ERROR, @@ -223,22 +225,29 @@ oauth_exchange(void *opaq, const char *input, int inputlen, case 'y': /* fall through */ case 'n': - p++; + if (!*(++p)) + goto endofmessage; + if (*p != ',') + { + sanitize_char(*p, errmsgbuf, sizeof(errmsgbuf)); ereport(ERROR, errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("malformed OAUTHBEARER message"), errdetail("Comma expected, but found character \"%s\".", - sanitize_char(*p))); - p++; + errmsgbuf)); + } + if (!*(++p)) + goto endofmessage; break; default: + sanitize_char(*p, errmsgbuf, sizeof(errmsgbuf)); ereport(ERROR, errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("malformed OAUTHBEARER message"), errdetail("Unexpected channel-binding flag \"%s\".", - sanitize_char(cbind_flag))); + errmsgbuf)); } /* @@ -249,21 +258,29 @@ oauth_exchange(void *opaq, const char *input, int inputlen, errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("client uses authorization identity, but it is not supported")); if (*p != ',') + { + sanitize_char(*p, errmsgbuf, sizeof(errmsgbuf)); ereport(ERROR, errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("malformed OAUTHBEARER message"), errdetail("Unexpected attribute \"%s\" in client-first-message.", - sanitize_char(*p))); - p++; + errmsgbuf)); + } + if (!*(++p)) + goto endofmessage; /* All remaining fields are separated by the RFC's kvsep (\x01). */ if (*p != KVSEP) + { + sanitize_char(*p, errmsgbuf, sizeof(errmsgbuf)); ereport(ERROR, errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("malformed OAUTHBEARER message"), errdetail("Key-value separator expected, but found character \"%s\".", - sanitize_char(*p))); - p++; + errmsgbuf)); + } + if (!*(++p)) + goto endofmessage; auth = parse_kvpairs_for_auth(&p); if (!auth) @@ -296,6 +313,13 @@ oauth_exchange(void *opaq, const char *input, int inputlen, explicit_bzero(input_copy, inputlen); return status; + +endofmessage: + explicit_bzero(input_copy, inputlen); + ereport(ERROR, + errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("malformed OAUTHBEARER message")); + pg_unreachable(); } /* @@ -303,19 +327,14 @@ oauth_exchange(void *opaq, const char *input, int inputlen, * * If it's a printable ASCII character, print it as a single character. * otherwise, print it in hex. - * - * The returned pointer points to a static buffer. */ -static char * -sanitize_char(char c) +static void +sanitize_char(char c, char *buf, size_t buflen) { - static char buf[5]; - if (c >= 0x21 && c <= 0x7E) - snprintf(buf, sizeof(buf), "'%c'", c); + snprintf(buf, buflen, "'%c'", c); else - snprintf(buf, sizeof(buf), "0x%02x", (unsigned char) c); - return buf; + snprintf(buf, buflen, "0x%02x", (unsigned char) c); } /* @@ -660,7 +679,9 @@ validate(Port *port, const char *auth) if (!ValidatorCallbacks->validate_cb(validator_module_state, token, port->user_name, ret)) { - ereport(LOG, errmsg("internal error in OAuth validator module")); + ereport(WARNING, + errcode(ERRCODE_INTERNAL_ERROR), + errmsg("internal error in OAuth validator module")); return false; } @@ -738,6 +759,11 @@ load_validator_library(const char *libname) OAuthValidatorModuleInit validator_init; MemoryContextCallback *mcb; + /* + * Thre presence, and validity, of libname has already been established by + * check_oauth_validator so we don't need to perform more than Assert level + * checking here. + */ Assert(libname && *libname); validator_init = (OAuthValidatorModuleInit) @@ -768,6 +794,15 @@ load_validator_library(const char *libname) errdetail("Server has magic number 0x%08X, module has 0x%08X.", PG_OAUTH_VALIDATOR_MAGIC, ValidatorCallbacks->magic)); + /* + * Make sure all required callbacks are present in the ValidatorCallbacks + * structure. Right now only the validation callback is required. + */ + if (ValidatorCallbacks->validate_cb == NULL) + ereport(ERROR, + errmsg("%s module \"%s\" must define the symbol %s", + "OAuth validator", libname, "validate_cb")); + /* Allocate memory for validator library private state data */ validator_module_state = (ValidatorModuleState *) palloc0(sizeof(ValidatorModuleState)); validator_module_state->sversion = PG_VERSION_NUM; @@ -804,7 +839,7 @@ bool check_oauth_validator(HbaLine *hbaline, int elevel, char **err_msg) { int line_num = hbaline->linenumber; - char *file_name = hbaline->sourcefile; + const char *file_name = hbaline->sourcefile; char *rawstring; List *elemlist = NIL; diff --git a/src/include/common/oauth-common.h b/src/include/common/oauth-common.h index 8fe56267780..5fb559d84b2 100644 --- a/src/include/common/oauth-common.h +++ b/src/include/common/oauth-common.h @@ -3,7 +3,7 @@ * oauth-common.h * Declarations for helper functions used for OAuth/OIDC authentication * - * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * * src/include/common/oauth-common.h diff --git a/src/include/libpq/oauth.h b/src/include/libpq/oauth.h index 7e249613e10..2f01b669633 100644 --- a/src/include/libpq/oauth.h +++ b/src/include/libpq/oauth.h @@ -3,7 +3,7 @@ * oauth.h * Interface to libpq/auth-oauth.c * - * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * * src/include/libpq/oauth.h @@ -83,8 +83,9 @@ typedef struct OAuthValidatorCallbacks } OAuthValidatorCallbacks; /* - * Type of the shared library symbol _PG_oauth_validator_module_init that is - * looked up when loading a validator module. + * Type of the shared library symbol _PG_oauth_validator_module_init which is + * required for all validator modules. This function will be invoked during + * module loading. */ typedef const OAuthValidatorCallbacks *(*OAuthValidatorModuleInit) (void); extern PGDLLEXPORT const OAuthValidatorCallbacks *_PG_oauth_validator_module_init(void); diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index c04ee38d086..db6454090d2 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -445,7 +445,7 @@ /* Define to 1 if you have the header file. */ #undef HAVE_TERMIOS_H -/* Define to 1 if curl_global_init() is guaranteed to be threadsafe. */ +/* Define to 1 if curl_global_init() is guaranteed to be thread-safe. */ #undef HAVE_THREADSAFE_CURL_GLOBAL_INIT /* Define to 1 if your compiler understands `typeof' or something similar. */ diff --git a/src/interfaces/libpq/fe-auth-oauth-curl.c b/src/interfaces/libpq/fe-auth-oauth-curl.c index 74323de309a..c9aa51b1007 100644 --- a/src/interfaces/libpq/fe-auth-oauth-curl.c +++ b/src/interfaces/libpq/fe-auth-oauth-curl.c @@ -4,7 +4,7 @@ * The libcurl implementation of OAuth/OIDC authentication, using the * OAuth Device Authorization Grant (RFC 8628). * - * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION @@ -917,7 +917,7 @@ parse_interval(struct async_ctx *actx, const char *interval_str) if (parsed < 1) return actx->debugging ? 0 : 1; - else if (INT_MAX <= parsed) + else if (parsed >= INT_MAX) return INT_MAX; return parsed; @@ -940,7 +940,7 @@ parse_expires_in(struct async_ctx *actx, const char *expires_in_str) parsed = parse_json_number(expires_in_str); parsed = round(parsed); - if (INT_MAX <= parsed) + if (parsed >= INT_MAX) return INT_MAX; else if (parsed <= INT_MIN) return INT_MIN; @@ -966,6 +966,10 @@ parse_device_authz(struct async_ctx *actx, struct device_authz *authz) */ {"verification_url", JSON_TOKEN_STRING, {&authz->verification_uri}, REQUIRED}, + /* + * There is no evidence of verification_uri_complete being spelled + * with "url" instead with any service provider, so only support "uri". + */ {"verification_uri_complete", JSON_TOKEN_STRING, {&authz->verification_uri_complete}, OPTIONAL}, {"interval", JSON_TOKEN_NUMBER, {&authz->interval_str}, OPTIONAL}, @@ -1870,6 +1874,7 @@ append_urlencoded(PQExpBuffer buf, const char *s) char *haystack; char *match; + /* The first parameter to curl_easy_escape is deprecated by Curl */ escaped = curl_easy_escape(NULL, s, 0); if (!escaped) { @@ -2273,6 +2278,7 @@ finish_device_authz(struct async_ctx *actx) return false; } + /* Copy the token error into the context error buffer */ record_token_error(actx, &err); free_token_error(&err); @@ -2541,7 +2547,7 @@ initialize_curl(PGconn *conn) /* * If we determined at configure time that the Curl installation is - * threadsafe, our job here is much easier. We simply initialize above + * thread-safe, our job here is much easier. We simply initialize above * without any locking (concurrent or duplicated calls are fine in that * situation), then double-check to make sure the runtime setting agrees, * to try to catch silent downgrades. @@ -2553,8 +2559,8 @@ initialize_curl(PGconn *conn) * In a downgrade situation, the damage is already done. Curl global * state may be corrupted. Be noisy. */ - libpq_append_conn_error(conn, "libcurl is no longer threadsafe\n" - "\tCurl initialization was reported threadsafe when libpq\n" + libpq_append_conn_error(conn, "libcurl is no longer thread-safe\n" + "\tCurl initialization was reported thread-safe when libpq\n" "\twas compiled, but the currently installed version of\n" "\tlibcurl reports that it is not. Recompile libpq against\n" "\tthe installed version of libcurl."); diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c index 8beae9604c7..24448c3e209 100644 --- a/src/interfaces/libpq/fe-auth-oauth.c +++ b/src/interfaces/libpq/fe-auth-oauth.c @@ -4,7 +4,7 @@ * The front-end (client) implementation of OAuth/OIDC authentication * using the SASL OAUTHBEARER mechanism. * - * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION @@ -272,7 +272,7 @@ oauth_json_scalar(void *state, char *token, JsonTokenType type) * Assert and don't continue any further for production builds. */ Assert(false); - oauth_json_set_error(ctx, /* don't bother translating */ + oauth_json_set_error(ctx, "internal error: target scalar found at nesting level %d during OAUTHBEARER parsing", ctx->nested); return JSON_SEM_ACTION_FAILED; diff --git a/src/test/modules/oauth_validator/Makefile b/src/test/modules/oauth_validator/Makefile index f297ed5c968..bbd2a98023b 100644 --- a/src/test/modules/oauth_validator/Makefile +++ b/src/test/modules/oauth_validator/Makefile @@ -2,7 +2,7 @@ # # Makefile for src/test/modules/oauth_validator # -# Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group +# Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group # Portions Copyright (c) 1994, Regents of the University of California # # src/test/modules/oauth_validator/Makefile diff --git a/src/test/modules/oauth_validator/README b/src/test/modules/oauth_validator/README index 138a8104622..54eac5b117e 100644 --- a/src/test/modules/oauth_validator/README +++ b/src/test/modules/oauth_validator/README @@ -8,6 +8,6 @@ by t/OAuth/Server.pm and t/oauth_server.py, to run the libpq Device Authorization flow. The tests in t/002_client exercise custom OAuth flows and don't need an authorization server. -Tests in this folder generally require 'oauth' to be present in PG_TEST_EXTRA, -since localhost HTTP servers will be started. A Python installation is required -to run the mock authorization server. +Tests in this folder require 'oauth' to be present in PG_TEST_EXTRA, since +HTTPS servers listening on localhost with TCP/IP sockets will be started. A +Python installation is required to run the mock authorization server. diff --git a/src/test/modules/oauth_validator/fail_validator.c b/src/test/modules/oauth_validator/fail_validator.c index 7b1e69518d9..a4c7a4451d3 100644 --- a/src/test/modules/oauth_validator/fail_validator.c +++ b/src/test/modules/oauth_validator/fail_validator.c @@ -1,10 +1,10 @@ /*------------------------------------------------------------------------- * * fail_validator.c - * Test module for serverside OAuth token validation callbacks, which always - * fails + * Test module for serverside OAuth token validation callbacks, which is + * guaranteed to always fail in the validation callback * - * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * * src/test/modules/oauth_validator/fail_validator.c diff --git a/src/test/modules/oauth_validator/magic_validator.c b/src/test/modules/oauth_validator/magic_validator.c new file mode 100644 index 00000000000..5ce68cdf405 --- /dev/null +++ b/src/test/modules/oauth_validator/magic_validator.c @@ -0,0 +1,48 @@ +/*------------------------------------------------------------------------- + * + * magic_validator.c + * Test module for serverside OAuth token validation callbacks, which + * should fail due to using the wrong PG_OAUTH_VALIDATOR_MAGIC marker + * and thus the wrong ABI version + * + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/test/modules/oauth_validator/fail_validator.c + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include "fmgr.h" +#include "libpq/oauth.h" + +PG_MODULE_MAGIC; + +static bool validate_token(const ValidatorModuleState *state, + const char *token, + const char *role, + ValidatorModuleResult *result); + +/* Callback implementations (we only need the main one) */ +static const OAuthValidatorCallbacks validator_callbacks = { + 0xdeadbeef, + + .validate_cb = validate_token, +}; + +const OAuthValidatorCallbacks * +_PG_oauth_validator_module_init(void) +{ + return &validator_callbacks; +} + +static bool +validate_token(const ValidatorModuleState *state, + const char *token, const char *role, + ValidatorModuleResult *res) +{ + elog(FATAL, "magic_validator: this should be unreachable"); + pg_unreachable(); +} diff --git a/src/test/modules/oauth_validator/meson.build b/src/test/modules/oauth_validator/meson.build index 4b78c90557c..36d1b26369f 100644 --- a/src/test/modules/oauth_validator/meson.build +++ b/src/test/modules/oauth_validator/meson.build @@ -1,4 +1,4 @@ -# Copyright (c) 2024, PostgreSQL Global Development Group +# Copyright (c) 2025, PostgreSQL Global Development Group validator_sources = files( 'validator.c', @@ -32,6 +32,22 @@ fail_validator = shared_module('fail_validator', ) test_install_libs += fail_validator +magic_validator_sources = files( + 'magic_validator.c', +) + +if host_system == 'windows' + magic_validator_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ + '--NAME', 'magic_validator', + '--FILEDESC', 'magic_validator - ABI incompatible OAuth validator module',]) +endif + +magic_validator = shared_module('magic_validator', + magic_validator_sources, + kwargs: pg_test_mod_args, +) +test_install_libs += magic_validator + oauth_hook_client_sources = files( 'oauth_hook_client.c', ) diff --git a/src/test/modules/oauth_validator/oauth_hook_client.c b/src/test/modules/oauth_validator/oauth_hook_client.c index fc003030ff8..9f553792c05 100644 --- a/src/test/modules/oauth_validator/oauth_hook_client.c +++ b/src/test/modules/oauth_validator/oauth_hook_client.c @@ -4,7 +4,7 @@ * Test driver for t/002_client.pl, which verifies OAuth hook * functionality in libpq. * - * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * * diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index d2dda62a2d4..dada89e95cc 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -3,7 +3,7 @@ # Tests the libpq builtin OAuth flow, as well as server-side HBA and validator # setup. # -# Copyright (c) 2021-2024, PostgreSQL Global Development Group +# Copyright (c) 2021-2025, PostgreSQL Global Development Group # use strict; @@ -14,24 +14,23 @@ use MIME::Base64 qw(encode_base64); use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; -use Config; use FindBin; use lib $FindBin::RealBin; use OAuth::Server; -if ($Config{osname} eq 'MSWin32') -{ - plan skip_all => 'OAuth server-side tests are not supported on Windows'; -} - if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\boauth\b/) { plan skip_all => 'Potentially unsafe test oauth not enabled in PG_TEST_EXTRA'; } +if ($windows_os) +{ + plan skip_all => 'OAuth server-side tests are not supported on Windows'; +} + if ($ENV{with_libcurl} ne 'yes') { plan skip_all => 'client-side OAuth not supported by this build'; @@ -570,6 +569,24 @@ $node->connect_fails( "fail_validator is used for $user", expected_stderr => qr/FATAL:\s+fail_validator: sentinel error/); +# +# Test ABI compatability magic marker +# +$node->append_conf('postgresql.conf', + "oauth_validator_libraries = 'magic_validator'\n"); +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf( + 'pg_hba.conf', qq{ +local all test oauth validator=magic_validator issuer="$issuer" scope="openid postgres" +}); +$node->restart; + +$log_start = $node->wait_for_log(qr/ready to accept connections/, $log_start); + +$node->connect_fails( + "user=test dbname=postgres oauth_issuer=$issuer/.well-known/oauth-authorization-server/alternate oauth_client_id=f02c6361-0636", + "magic_validator is used for $user", + expected_stderr => qr/FATAL:\s+OAuth validator module "magic_validator": magic number mismatch/); $node->stop; done_testing(); diff --git a/src/test/modules/oauth_validator/t/002_client.pl b/src/test/modules/oauth_validator/t/002_client.pl index 95cccf90dd8..ab83258d736 100644 --- a/src/test/modules/oauth_validator/t/002_client.pl +++ b/src/test/modules/oauth_validator/t/002_client.pl @@ -2,7 +2,7 @@ # Exercises the API for custom OAuth client flows, using the oauth_hook_client # test driver. # -# Copyright (c) 2021-2024, PostgreSQL Global Development Group +# Copyright (c) 2021-2025, PostgreSQL Global Development Group # use strict; diff --git a/src/test/modules/oauth_validator/t/OAuth/Server.pm b/src/test/modules/oauth_validator/t/OAuth/Server.pm index f0f23d1d1a8..655b2870b0b 100644 --- a/src/test/modules/oauth_validator/t/OAuth/Server.pm +++ b/src/test/modules/oauth_validator/t/OAuth/Server.pm @@ -1,5 +1,5 @@ -# Copyright (c) 2024, PostgreSQL Global Development Group +# Copyright (c) 2025, PostgreSQL Global Development Group =pod @@ -46,7 +46,7 @@ use Test::More; =over -=item SSL::Server->new() +=item OAuth::Server->new() Create a new OAuth Server object. diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c index e218f5c8902..b2e5d182e1b 100644 --- a/src/test/modules/oauth_validator/validator.c +++ b/src/test/modules/oauth_validator/validator.c @@ -3,7 +3,7 @@ * validator.c * Test module for serverside OAuth token validation callbacks * - * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * * src/test/modules/oauth_validator/validator.c -- 2.39.3 (Apple Git-146)