From eee0ee6d369e6baba327cd73fb7238eb0e1cb881 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Fri, 14 Feb 2025 16:50:45 -0800 Subject: [PATCH v50 2/4] fixup! Add OAUTHBEARER SASL mechanism --- doc/src/sgml/client-auth.sgml | 2 +- doc/src/sgml/libpq.sgml | 2 +- doc/src/sgml/oauth-validators.sgml | 4 ++-- src/backend/libpq/auth-oauth.c | 8 ++++---- src/interfaces/libpq/fe-auth-oauth-curl.c | 19 +++++++++++++------ src/test/modules/oauth_validator/Makefile | 2 +- .../modules/oauth_validator/magic_validator.c | 2 +- .../modules/oauth_validator/t/001_server.pl | 6 ++++-- 8 files changed, 27 insertions(+), 18 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 6fc0da57f1b..832b616a7bb 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -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 resource servers and clients for a given application. + administers the OAuth authorization 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. diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index ca84226755d..ddb3596df83 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -10527,7 +10527,7 @@ 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 diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml index e9d28d3daea..356f11d3bd8 100644 --- a/doc/src/sgml/oauth-validators.sgml +++ b/doc/src/sgml/oauth-validators.sgml @@ -21,7 +21,7 @@ Since a misbehaving validator might let unauthorized users into the database, - validating the correctness of the implementation is critical. See + correct implementation is crucial for server safety. See for design considerations. @@ -357,7 +357,7 @@ typedef bool (*ValidatorValidateCB) (const ValidatorModuleState *state, token will contain the bearer token to validate. - libpq has ensured that the token is well-formed syntactically, but no + PostgreSQL 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 diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c index 830f2002683..27f7af7be00 100644 --- a/src/backend/libpq/auth-oauth.c +++ b/src/backend/libpq/auth-oauth.c @@ -741,9 +741,9 @@ load_validator_library(const char *libname) 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. + * The 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); @@ -781,7 +781,7 @@ load_validator_library(const char *libname) */ if (ValidatorCallbacks->validate_cb == NULL) ereport(ERROR, - errmsg("%s module \"%s\" must define the symbol %s", + errmsg("%s module \"%s\" must provide a %s callback", "OAuth validator", libname, "validate_cb")); /* Allocate memory for validator library private state data */ diff --git a/src/interfaces/libpq/fe-auth-oauth-curl.c b/src/interfaces/libpq/fe-auth-oauth-curl.c index c9aa51b1007..a80e2047bb7 100644 --- a/src/interfaces/libpq/fe-auth-oauth-curl.c +++ b/src/interfaces/libpq/fe-auth-oauth-curl.c @@ -211,7 +211,7 @@ struct async_ctx * something like the following, with errctx and/or curl_err omitted when * absent: * - * connection to server ... failed: errctx: errbuf (curl_err) + * connection to server ... failed: errctx: errbuf (libcurl: curl_err) */ const char *errctx; /* not freed; must point to static allocation */ PQExpBufferData errbuf; @@ -930,7 +930,7 @@ parse_interval(struct async_ctx *actx, const char *interval_str) * Similar to parse_interval, but we have even fewer requirements for reasonable * values since we don't use the expiration time directly (it's passed to the * PQAUTHDATA_PROMPT_OAUTH_DEVICE hook, in case the application wants to do - * something with it). We simply round and clamp to int range. + * something with it). We simply round down and clamp to int range. */ static int parse_expires_in(struct async_ctx *actx, const char *expires_in_str) @@ -938,7 +938,7 @@ parse_expires_in(struct async_ctx *actx, const char *expires_in_str) double parsed; parsed = parse_json_number(expires_in_str); - parsed = round(parsed); + parsed = floor(parsed); if (parsed >= INT_MAX) return INT_MAX; @@ -968,7 +968,8 @@ parse_device_authz(struct async_ctx *actx, struct device_authz *authz) /* * There is no evidence of verification_uri_complete being spelled - * with "url" instead with any service provider, so only support "uri". + * 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}, @@ -1226,6 +1227,8 @@ register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx, return -1; } + + return 0; #endif #ifdef HAVE_SYS_EVENT_H struct async_ctx *actx = ctx; @@ -1307,9 +1310,12 @@ register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx, return -1; } } -#endif return 0; +#endif + + actx_error(actx, "libpq does not support multiplexer sockets on this platform"); + return -1; } /* @@ -2808,7 +2814,8 @@ error_return: { size_t len; - appendPQExpBuffer(&conn->errorMessage, " (%s)", actx->curl_err); + appendPQExpBuffer(&conn->errorMessage, + " (libcurl: %s)", actx->curl_err); /* Sometimes libcurl adds a newline to the error buffer. :( */ len = conn->errorMessage.len; diff --git a/src/test/modules/oauth_validator/Makefile b/src/test/modules/oauth_validator/Makefile index bbd2a98023b..05b9f06ed73 100644 --- a/src/test/modules/oauth_validator/Makefile +++ b/src/test/modules/oauth_validator/Makefile @@ -9,7 +9,7 @@ # #------------------------------------------------------------------------- -MODULES = validator fail_validator +MODULES = validator fail_validator magic_validator PGFILEDESC = "validator - test OAuth validator module" PROGRAM = oauth_hook_client diff --git a/src/test/modules/oauth_validator/magic_validator.c b/src/test/modules/oauth_validator/magic_validator.c index 5ce68cdf405..9dc55b602e3 100644 --- a/src/test/modules/oauth_validator/magic_validator.c +++ b/src/test/modules/oauth_validator/magic_validator.c @@ -8,7 +8,7 @@ * 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 + * src/test/modules/oauth_validator/magic_validator.c * *------------------------------------------------------------------------- */ diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl index dada89e95cc..6fa59fbeb25 100644 --- a/src/test/modules/oauth_validator/t/001_server.pl +++ b/src/test/modules/oauth_validator/t/001_server.pl @@ -570,7 +570,7 @@ $node->connect_fails( expected_stderr => qr/FATAL:\s+fail_validator: sentinel error/); # -# Test ABI compatability magic marker +# Test ABI compatibility magic marker # $node->append_conf('postgresql.conf', "oauth_validator_libraries = 'magic_validator'\n"); @@ -586,7 +586,9 @@ $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/); + expected_stderr => + qr/FATAL:\s+OAuth validator module "magic_validator": magic number mismatch/ +); $node->stop; done_testing(); -- 2.34.1