1: aa553b7700 = 1: 9fc1df7509 Revert ECPG's use of pnstrdup() 2: d6cae9157e = 2: fdd89bdee0 Remove fe_memutils from libpgcommon_shlib 3: 5543539169 = 3: ae3ae1cfaa common/jsonapi: support libpq as a client 4: 1639f7eb9a ! 4: 92b257643e libpq: add OAUTHBEARER SASL mechanism @@ Commit message - fix intermittent failure in the cleanup callback tests (race condition?) - support require_auth + - fill in documentation stubs - ...and more. Co-authored-by: Daniel Gustafsson + ## config/programs.m4 ## +@@ config/programs.m4: if test "$pgac_cv_ldap_safe" != yes; then + *** also uses LDAP will crash on exit.]) + fi]) + ++# PGAC_CHECK_LIBCURL ++# ------------------ ++# Check for libcurl 7.61.0 or higher (corresponding to RHEL8 and the ability to ++# explicitly set TLS 1.3 ciphersuites). + ++AC_DEFUN([PGAC_CHECK_LIBCURL], ++[AC_CACHE_CHECK([for compatible libcurl], [pgac_cv_check_libcurl], ++[AC_COMPILE_IFELSE([AC_LANG_PROGRAM( ++[#include ++#if LIBCURL_VERSION_MAJOR < 7 || (LIBCURL_VERSION_MAJOR == 7 && LIBCURL_VERSION_MINOR < 61) ++choke me ++#endif], [])], ++[pgac_cv_check_libcurl=yes], ++[pgac_cv_check_libcurl=no])]) ++ ++if test "$pgac_cv_check_libcurl" != yes; then ++ AC_MSG_ERROR([ ++*** The installed version of libcurl is too old to use with PostgreSQL. ++*** libcurl version 7.61.0 or later is required.]) ++fi]) + + # PGAC_CHECK_READLINE + # ------------------- + ## configure ## @@ configure: with_uuid with_readline @@ configure: fi + as_fn_error $? "library 'curl' is required for --with-oauth=curl" "$LINENO" 5 +fi + ++ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for compatible libcurl" >&5 ++$as_echo_n "checking for compatible libcurl... " >&6; } ++if ${pgac_cv_check_libcurl+:} false; then : ++ $as_echo_n "(cached) " >&6 ++else ++ cat confdefs.h - <<_ACEOF >conftest.$ac_ext ++/* end confdefs.h. */ ++#include ++#if LIBCURL_VERSION_MAJOR < 7 || (LIBCURL_VERSION_MAJOR == 7 && LIBCURL_VERSION_MINOR < 61) ++choke me ++#endif ++int ++main () ++{ ++ ++ ; ++ return 0; ++} ++_ACEOF ++if ac_fn_c_try_compile "$LINENO"; then : ++ pgac_cv_check_libcurl=yes ++else ++ pgac_cv_check_libcurl=no ++fi ++rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext ++fi ++{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_check_libcurl" >&5 ++$as_echo "$pgac_cv_check_libcurl" >&6; } ++ ++if test "$pgac_cv_check_libcurl" != yes; then ++ as_fn_error $? " ++*** The installed version of libcurl is too old to use with PostgreSQL. ++*** libcurl version 7.61.0 or later is required." "$LINENO" 5 ++fi +fi + # for contrib/sepgsql @@ configure.ac: fi +if test "$with_oauth" = curl ; then + AC_CHECK_LIB(curl, curl_multi_init, [], [AC_MSG_ERROR([library 'curl' is required for --with-oauth=curl])]) ++ PGAC_CHECK_LIBCURL +fi + # for contrib/sepgsql @@ configure.ac: elif test "$with_uuid" = ossp ; then AC_CHECK_HEADERS(crtdefs.h) fi + ## doc/src/sgml/libpq.sgml ## +@@ doc/src/sgml/libpq.sgml: postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + + + ++ ++ ++ oauth_client_id ++ ++ ++ TODO ++ ++ ++ ++ ++ ++ oauth_client_secret ++ ++ ++ TODO ++ ++ ++ ++ ++ ++ oauth_issuer ++ ++ ++ TODO ++ ++ ++ ++ ++ ++ oauth_scope ++ ++ ++ TODO ++ ++ ++ ++ + + + +@@ doc/src/sgml/libpq.sgml: void PQinitSSL(int do_ssl); + + + ++ ++ OAuth Support ++ ++ ++ TODO ++ ++ ++ ++ ++ ++ PQsetAuthDataHookPQsetAuthDataHook ++ ++ ++ ++ TODO ++ ++void PQsetAuthDataHook(PQauthDataHook_type hook); ++ ++ ++ ++ ++ ++ ++ PQgetAuthDataHookPQgetAuthDataHook ++ ++ ++ ++ TODO ++ ++PQauthDataHook_type PQgetAuthDataHook(void); ++ ++ ++ ++ ++ ++ ++ ++ ++ + + + Behavior in Threaded Programs + ## meson.build ## @@ meson.build: endif @@ meson.build: endif +endif + +if oauthopt in ['auto', 'curl'] -+ oauth = dependency('libcurl', required: (oauthopt == 'curl')) ++ # Check for libcurl 7.61.0 or higher (corresponding to RHEL8 and the ability ++ # to explicitly set TLS 1.3 ciphersuites). ++ oauth = dependency('libcurl', version: '>= 7.61.0', required: (oauthopt == 'curl')) + + if oauth.found() + oauth_library = 'curl' @@ src/include/common/oauth-common.h (new) + * oauth-common.h + * Declarations for helper functions used for OAuth/OIDC authentication + * -+ * Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group ++ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/common/oauth-common.h @@ src/interfaces/libpq/Makefile: endif else SHLIB_LINK += $(filter -lcrypt -ldes -lcom_err -lcrypto -lk5crypto -lkrb5 -lgssapi32 -lssl -lsocket -lnsl -lresolv -lintl -lm $(PTHREAD_LIBS), $(LIBS)) $(LDAP_LIBS_FE) endif +@@ src/interfaces/libpq/Makefile: backend_src = $(top_srcdir)/src/backend + # which seems to insert references to that even in pure C code. Excluding + # __tsan_func_exit is necessary when using ThreadSanitizer data race detector + # which use this function for instrumentation of function exit. ++# libcurl registers an exit handler in the memory debugging code when running ++# with LeakSanitizer. + # Skip the test when profiling, as gcc may insert exit() calls for that. + # Also skip the test on platforms where libpq infrastructure may be provided + # by statically-linked libraries, as we can't expect them to honor this +@@ src/interfaces/libpq/Makefile: backend_src = $(top_srcdir)/src/backend + libpq-refs-stamp: $(shlib) + ifneq ($(enable_coverage), yes) + ifeq (,$(filter solaris,$(PORTNAME))) +- @if nm -A -u $< 2>/dev/null | grep -v -e __cxa_atexit -e __tsan_func_exit | grep exit; then \ ++ @if nm -A -u $< 2>/dev/null | grep -v -e __cxa_atexit -e __tsan_func_exit -e _atexit | grep exit; then \ + echo 'libpq must not be calling any function which invokes exit'; exit 1; \ + fi + endif ## src/interfaces/libpq/exports.txt ## @@ src/interfaces/libpq/exports.txt: PQcancelFinish 202 @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + * fe-auth-oauth-curl.c + * The libcurl implementation of OAuth/OIDC authentication. + * -+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group ++ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) +} OAuthStep; + +/* -+ * The async_ctx holds onto state that needs to persist across multiple calls to -+ * pg_fe_run_oauth_flow(). Almost everything interacts with this in some way. ++ * The async_ctx holds onto state that needs to persist across multiple calls ++ * to pg_fe_run_oauth_flow(). Almost everything interacts with this in some ++ * way. + */ +struct async_ctx +{ @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + int timerfd; /* a timerfd for signaling async timeouts */ +#endif + pgsocket mux; /* the multiplexer socket containing all -+ * descriptors tracked by cURL, plus the ++ * descriptors tracked by libcurl, plus the + * timerfd */ -+ CURLM *curlm; /* top-level multi handle for cURL operations */ ++ CURLM *curlm; /* top-level multi handle for libcurl ++ * operations */ + CURL *curl; /* the (single) easy handle for serial + * requests */ + @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + * actx_error[_str] to manipulate this. This must be filled + * with something useful on an error. + * -+ * - curl_err: an optional static error buffer used by cURL to put ++ * - curl_err: an optional static error buffer used by libcurl to put + * detailed information about failures. Unfortunately + * untranslatable. + * @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + + if (err) + libpq_append_conn_error(conn, -+ "cURL easy handle removal failed: %s", ++ "libcurl easy handle removal failed: %s", + curl_multi_strerror(err)); + } + @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + + if (err) + libpq_append_conn_error(conn, -+ "cURL multi handle cleanup failed: %s", ++ "libcurl multi handle cleanup failed: %s", + curl_multi_strerror(err)); + } + @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + appendPQExpBufferStr(&(ACTX)->errbuf, S) + +/* -+ * Macros for getting and setting state for the connection's two cURL handles, -+ * so you don't have to write out the error handling every time. ++ * Macros for getting and setting state for the connection's two libcurl ++ * handles, so you don't have to write out the error handling every time. + */ + +#define CHECK_MSETOPT(ACTX, OPT, VAL, FAILACTION) \ @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) +} + +/* ++ * Checks the Content-Type header against the expected type. Parameters are ++ * allowed but ignored. ++ */ ++static bool ++check_content_type(struct async_ctx *actx, const char *type) ++{ ++ const size_t type_len = strlen(type); ++ char *content_type; ++ ++ CHECK_GETINFO(actx, CURLINFO_CONTENT_TYPE, &content_type, return false); ++ ++ if (!content_type) ++ { ++ actx_error(actx, "no content type was provided"); ++ return false; ++ } ++ ++ /* ++ * We need to perform a length limited comparison and not compare the whole ++ * string. ++ */ ++ if (pg_strncasecmp(content_type, type, type_len) != 0) ++ goto fail; ++ ++ /* On an exact match, we're done. */ ++ Assert(strlen(content_type) >= type_len); ++ if (content_type[type_len] == '\0') ++ return true; ++ ++ /* ++ * Only a semicolon (optionally preceded by HTTP optional whitespace) is ++ * acceptable after the prefix we checked. This marks the start of media ++ * type parameters, which we currently have no use for. ++ */ ++ for (size_t i = type_len; content_type[i]; ++i) ++ { ++ switch (content_type[i]) ++ { ++ case ';': ++ return true; /* success! */ ++ ++ /* HTTP optional whitespace allows only spaces and htabs. */ ++ case ' ': ++ case '\t': ++ break; ++ ++ default: ++ goto fail; ++ } ++ } ++ ++fail: ++ actx_error(actx, "unexpected content type: \"%s\"", content_type); ++ return false; ++} ++ ++/* + * A helper function for general JSON parsing. fields is the array of field + * definitions with their backing pointers. The response will be parsed from + * actx->curl and actx->work_data (as set up by start_request()), and any @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) +parse_oauth_json(struct async_ctx *actx, const struct json_field *fields) +{ + PQExpBuffer resp = &actx->work_data; -+ char *content_type; + JsonLexContext lex = {0}; + JsonSemAction sem = {0}; + JsonParseErrorType err; + struct oauth_parse ctx = {0}; + bool success = false; + -+ /* Make sure the server thinks it's given us JSON. */ -+ CHECK_GETINFO(actx, CURLINFO_CONTENT_TYPE, &content_type, return false); -+ -+ if (!content_type) -+ { -+ actx_error(actx, "no content type was provided"); -+ goto cleanup; -+ } -+ else if (strcasecmp(content_type, "application/json") != 0) -+ { -+ actx_error(actx, "unexpected content type \"%s\"", content_type); -+ goto cleanup; -+ } ++ if (!check_content_type(actx, "application/json")) ++ return false; + + if (strlen(resp->data) != resp->len) + { + actx_error(actx, "response contains embedded NULLs"); -+ goto cleanup; ++ return false; + } + + makeJsonLexContextCstringLen(&lex, resp->data, resp->len, PG_UTF8, true); @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + authz->interval = parse_interval(authz->interval_str); + else + { -+ /* TODO: handle default interval of 5 seconds */ ++ /* ++ * RFC 8628 specify 5 seconds as the default value if the server ++ * doesn't provide an interval. ++ */ ++ authz->interval = 5; + } + + return true; @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) +} + +/* -+ * cURL Multi Setup/Callbacks ++ * libcurl Multi Setup/Callbacks + */ + +/* @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + +/* + * Adds and removes sockets from the multiplexer set, as directed by the -+ * cURL multi handle. ++ * libcurl multi handle. + */ +static int +register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx, @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + break; + + default: -+ actx_error(actx, "unknown cURL socket operation (%d)", what); ++ actx_error(actx, "unknown libcurl socket operation: %d", what); + return -1; + } + @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + break; + + default: -+ actx_error(actx, "unknown cURL socket operation (%d)", what); ++ actx_error(actx, "unknown libcurl socket operation: %d", what); + return -1; + } + @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + /* + * EV_RECEIPT should guarantee one EV_ERROR result for every change, + * whether successful or not. Failed entries contain a non-zero errno -+ * in the `data` field. ++ * in the data field. + */ + Assert(ev_out[i].flags & EV_ERROR); + @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + +/* + * Adds or removes timeouts from the multiplexer set, as directed by the -+ * cURL multi handle. Rather than continually adding and removing the timer, -+ * we keep it in the set at all times and just disarm it when it's not -+ * needed. ++ * libcurl multi handle. Rather than continually adding and removing the timer, ++ * we keep it in the set at all times and just disarm it when it's not needed. + */ +static int +register_timer(CURLM *curlm, long timeout, void *ctx) @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + else if (timeout == 0) + { + /* -+ * A zero timeout means cURL wants us to call back immediately. That's -+ * not technically an option for timerfd, but we can make the timeout -+ * ridiculously short. ++ * A zero timeout means libcurl wants us to call back immediately. ++ * That's not technically an option for timerfd, but we can make the ++ * timeout ridiculously short. + * + * TODO: maybe just signal drive_request() to immediately call back in + * this case? @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) +} + +/* -+ * Initializes the two cURL handles in the async_ctx. The multi handle, ++ * Initializes the two libcurl handles in the async_ctx. The multi handle, + * actx->curlm, is what drives the asynchronous engine and tells us what to do + * next. The easy handle, actx->curl, encapsulates the state for a single + * request/response. It's added to the multi handle as needed, during @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) +static bool +setup_curl_handles(struct async_ctx *actx) +{ -+ curl_version_info_data *curl_info; ++ curl_version_info_data *curl_info; + + /* + * Create our multi handle. This encapsulates the entire conversation with -+ * cURL for this connection. ++ * libcurl for this connection. + */ + actx->curlm = curl_multi_init(); + if (!actx->curlm) + { + /* We don't get a lot of feedback on the failure reason. */ -+ actx_error(actx, "failed to create cURL multi handle"); ++ actx_error(actx, "failed to create libcurl multi handle"); + return false; + } + @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + actx->curl = curl_easy_init(); + if (!actx->curl) + { -+ actx_error(actx, "failed to create cURL handle"); ++ actx_error(actx, "failed to create libcurl handle"); + return false; + } + @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + */ + +/* -+ * Response callback from cURL; appends the response body into actx->work_data. -+ * See start_request(). ++ * Response callback from libcurl which appends the response body into ++ * actx->work_data (see start_request()). The maximum size of the data is ++ * defined by CURL_MAX_WRITE_SIZE which by default is 16kb (and can only be ++ * changed by recompiling libcurl). + */ +static size_t +append_data(char *buf, size_t size, size_t nmemb, void *userdata) @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + if (msg->msg != CURLMSG_DONE) + { + /* -+ * Future cURL versions may define new message types; we don't ++ * Future libcurl versions may define new message types; we don't + * know how to handle them, so we'll ignore them. + */ + continue; @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + err = curl_multi_remove_handle(actx->curlm, msg->easy_handle); + if (err) + { -+ actx_error(actx, "cURL easy handle removal failed: %s", ++ actx_error(actx, "libcurl easy handle removal failed: %s", + curl_multi_strerror(err)); + return PGRES_POLLING_FAILED; + } @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + return true; +} + ++ +/* -+ * The top-level, nonblocking entry point for the cURL implementation. This will -+ * be called several times to pump the async engine. ++ * The top-level, nonblocking entry point for the libcurl implementation. This ++ * will be called several times to pump the async engine. + * + * The architecture is based on PQconnectPoll(). The first half drives the + * connection state forward as necessary, returning if we're not ready to @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + struct token tok = {0}; + + /* -+ * XXX This is not safe. cURL has stringent requirements for the thread ++ * XXX This is not safe. libcurl has stringent requirements for the thread + * context in which you call curl_global_init(), because it's going to try + * initializing a bunch of other libraries (OpenSSL, Winsock...). And we + * probably need to consider both the TLS backend libcurl is compiled @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + * Recent versions of libcurl have improved the thread-safety situation, + * but you apparently can't check at compile time whether the + * implementation is thread-safe, and there's a chicken-and-egg problem -+ * where you can't check the thread safety until you've initialized cURL, -+ * which you can't do before you've made sure it's thread-safe... ++ * where you can't check the thread safety until you've initialized ++ * libcurl, which you can't do before you've made sure it's thread-safe... + * + * We know we've already initialized Winsock by this point, so we should -+ * be able to safely skip that bit. But we have to tell cURL to initialize -+ * everything else, because other pieces of our client executable may -+ * already be using cURL for their own purposes. If we initialize libcurl -+ * first, with only a subset of its features, we could break those other -+ * clients nondeterministically, and that would probably be a nightmare to -+ * debug. ++ * be able to safely skip that bit. But we have to tell libcurl to ++ * initialize everything else, because other pieces of our client ++ * executable may already be using libcurl for their own purposes. If we ++ * initialize libcurl first, with only a subset of its features, we could ++ * break those other clients nondeterministically, and that would probably ++ * be a nightmare to debug. + */ + curl_global_init(CURL_GLOBAL_ALL + & ~CURL_GLOBAL_WIN32); /* we already initialized Winsock */ @@ src/interfaces/libpq/fe-auth-oauth-curl.c (new) + */ + if (strcmp(err->error, "slow_down") == 0) + { -+ actx->authz.interval += 5; /* TODO check for overflow? */ ++ int prev_interval = actx->authz.interval; ++ ++ actx->authz.interval += 5; ++ if (actx->authz.interval < prev_interval) ++ { ++ actx_error(actx, "slow_down interval overflow"); ++ goto error_return; ++ } + } + + /* @@ src/interfaces/libpq/fe-auth-oauth.c (new) + * fe-auth-oauth.c + * The front-end (client) implementation of OAuth/OIDC authentication. + * -+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group ++ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * IDENTIFICATION @@ src/interfaces/libpq/fe-auth-oauth.h (new) + * + * Definitions for OAuth authentication implementations + * -+ * Portions Copyright (c) 2023, PostgreSQL Global Development Group ++ * Portions Copyright (c) 2024, PostgreSQL Global Development Group + * + * src/interfaces/libpq/fe-auth-oauth.h + * 5: 044d8b08e9 ! 5: 16994b449d backend: add OAUTHBEARER SASL mechanism @@ Commit message - use logdetail during auth failures - allow passing the configured issuer to the oauth_validator_command, to deal with multi-issuer setups + - fill in documentation stubs - ...and more. Co-authored-by: Daniel Gustafsson @@ .cirrus.tasks.yml: task: ### # Test that code can be built with gcc/clang without warnings + ## doc/src/sgml/client-auth.sgml ## +@@ doc/src/sgml/client-auth.sgml: include_dir directory + + + ++ ++ ++ oauth ++ ++ ++ Authorize and optionally authenticate using a third-party OAuth 2.0 ++ identity provider. See for details. ++ ++ ++ + + + +@@ doc/src/sgml/client-auth.sgml: omicron bryanh guest1 + only on OpenBSD). + + ++ ++ ++ OAuth authorization/authentication, ++ which relies on an external OAuth 2.0 identity provider. ++ ++ + + + +@@ doc/src/sgml/client-auth.sgml: host ... radius radiusservers="server1,server2" radiussecrets="""secret one"","" + + + ++ ++ OAuth Authorization/Authentication ++ ++ ++ OAuth Authorization/Authentication ++ ++ ++ ++ TODO ++ ++ ++ + + Authentication Problems + + + ## doc/src/sgml/filelist.sgml ## +@@ + + + ++ + + + + + ## doc/src/sgml/oauth-validators.sgml (new) ## +@@ ++ ++ ++ ++ Implementing OAuth Validator Modules ++ ++ ++ TODO ++ ++ + + ## doc/src/sgml/postgres.sgml ## +@@ doc/src/sgml/postgres.sgml: break is not needed in a wider output rendering. + &bki; + &planstats; + &backup-manifest; ++ &oauth-validators; + + + + ## src/backend/libpq/Makefile ## @@ src/backend/libpq/Makefile: include $(top_builddir)/src/Makefile.global # be-fsstubs is here for historical reasons, probably belongs elsewhere @@ src/backend/libpq/auth-oauth.c (new) + initStringInfo(&buf); + + /* -+ * TODO: note that escaping here should be belt-and-suspenders, since -+ * escapable characters aren't valid in either the issuer URI or the scope -+ * list, but the HBA doesn't enforce that yet. ++ * Escaping the string here is belt-and-suspenders defensive programming ++ * since escapable characters aren't valid in either the issuer URI or the ++ * scope list, but the HBA doesn't enforce that yet. + */ + appendStringInfoString(&buf, "{ \"status\": \"invalid_token\", "); + @@ src/include/libpq/sasl.h: typedef struct pg_be_sasl_mech /* Common implementation for auth.c */ + ## src/interfaces/libpq/fe-auth-oauth-curl.c ## +@@ src/interfaces/libpq/fe-auth-oauth-curl.c: free_token(struct token *tok) + /* States for the overall async machine. */ + typedef enum + { +- OAUTH_STEP_INIT, ++ OAUTH_STEP_INIT = 0, + OAUTH_STEP_DISCOVERY, + OAUTH_STEP_DEVICE_AUTHORIZATION, + OAUTH_STEP_TOKEN_REQUEST, + ## src/test/modules/Makefile ## @@ src/test/modules/Makefile: SUBDIRS = \ dummy_index_am \ @@ src/test/modules/oauth_validator/t/001_server.pl (new) +use strict; +use warnings FATAL => 'all'; + ++use JSON::PP qw(encode_json); ++use MIME::Base64 qw(encode_base64); +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::OAuthServer; @@ src/test/modules/oauth_validator/t/001_server.pl (new) + +$node->safe_psql('postgres', 'CREATE USER test;'); +$node->safe_psql('postgres', 'CREATE USER testalt;'); ++$node->safe_psql('postgres', 'CREATE USER testparam;'); + +my $webserver = PostgreSQL::Test::OAuthServer->new(); +$webserver->run(); + ++END ++{ ++ my $exit_code = $?; ++ ++ $webserver->stop() if defined $webserver; # might have been SKIP'd ++ ++ $? = $exit_code; ++} ++ +my $port = $webserver->port(); +my $issuer = "127.0.0.1:$port"; + +unlink($node->data_dir . '/pg_hba.conf'); +$node->append_conf('pg_hba.conf', qq{ -+local all test oauth issuer="$issuer" scope="openid postgres" -+local all testalt oauth issuer="$issuer/alternate" scope="openid postgres alt" ++local all test oauth issuer="$issuer" scope="openid postgres" ++local all testalt oauth issuer="$issuer/alternate" scope="openid postgres alt" ++local all testparam oauth issuer="$issuer/param" scope="openid postgres" +}); +$node->reload; + @@ src/test/modules/oauth_validator/t/001_server.pl (new) +$log_start = $node->wait_for_log(qr/reloading configuration files/); + +my $user = "test"; -+$node->connect_ok("user=$user dbname=postgres oauth_client_id=f02c6361-0635", "connect", -+ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@); -+ -+$log_end = $node->wait_for_log(qr/connection authorized/, $log_start); -+$node->log_check("user $user: validator receives correct parameters", $log_start, -+ log_like => [ -+ qr/oauth_validator: token="9243959234", role="$user"/, -+ qr/oauth_validator: issuer="\Q$issuer\E", scope="openid postgres"/, -+ ]); -+$node->log_check("user $user: validator sets authenticated identity", $log_start, -+ log_like => [ -+ qr/connection authenticated: identity="test" method=oauth/, -+ ]); -+$log_start = $log_end; ++if ($node->connect_ok("user=$user dbname=postgres oauth_client_id=f02c6361-0635", "connect", ++ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@)) ++{ ++ $log_end = $node->wait_for_log(qr/connection authorized/, $log_start); ++ $node->log_check("user $user: validator receives correct parameters", $log_start, ++ log_like => [ ++ qr/oauth_validator: token="9243959234", role="$user"/, ++ qr/oauth_validator: issuer="\Q$issuer\E", scope="openid postgres"/, ++ ]); ++ $node->log_check("user $user: validator sets authenticated identity", $log_start, ++ log_like => [ ++ qr/connection authenticated: identity="test" method=oauth/, ++ ]); ++ $log_start = $log_end; ++} + +# The /alternate issuer uses slightly different parameters. +$user = "testalt"; -+$node->connect_ok("user=$user dbname=postgres oauth_client_id=f02c6361-0636", "connect", -+ expected_stderr => qr@Visit https://example\.org/ and enter the code: postgresuser@); -+ -+$log_end = $node->wait_for_log(qr/connection authorized/, $log_start); -+$node->log_check("user $user: validator receives correct parameters", $log_start, -+ log_like => [ -+ qr/oauth_validator: token="9243959234-alt", role="$user"/, -+ qr|oauth_validator: issuer="\Q$issuer/alternate\E", scope="openid postgres alt"|, -+ ]); -+$node->log_check("user $user: validator sets authenticated identity", $log_start, -+ log_like => [ -+ qr/connection authenticated: identity="testalt" method=oauth/, -+ ]); -+$log_start = $log_end; -+ -+$webserver->stop(); ++if ($node->connect_ok("user=$user dbname=postgres oauth_client_id=f02c6361-0636", "connect", ++ expected_stderr => qr@Visit https://example\.org/ and enter the code: postgresuser@)) ++{ ++ $log_end = $node->wait_for_log(qr/connection authorized/, $log_start); ++ $node->log_check("user $user: validator receives correct parameters", $log_start, ++ log_like => [ ++ qr/oauth_validator: token="9243959234-alt", role="$user"/, ++ qr|oauth_validator: issuer="\Q$issuer/alternate\E", scope="openid postgres alt"|, ++ ]); ++ $node->log_check("user $user: validator sets authenticated identity", $log_start, ++ log_like => [ ++ qr/connection authenticated: identity="testalt" method=oauth/, ++ ]); ++ $log_start = $log_end; ++} ++ ++# ++# Further tests rely on support for specific behaviors in oauth_server.py. To ++# trigger these behaviors, we ask for the special issuer .../param (which is set ++# up in HBA for the testparam user) and encode magic instructions into the ++# oauth_client_id. ++# ++ ++my $common_connstr = "user=testparam dbname=postgres "; ++ ++sub connstr ++{ ++ my (%params) = @_; ++ ++ my $json = encode_json(\%params); ++ my $encoded = encode_base64($json, ""); ++ ++ return "$common_connstr oauth_client_id=$encoded"; ++} ++ ++# Make sure the param system works end-to-end first. ++$node->connect_ok( ++ connstr(), ++ "connect to /param", ++ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ ++); ++ ++$node->connect_ok( ++ connstr(stage => 'token', retries => 1), ++ "token retry", ++ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ ++); ++$node->connect_ok( ++ connstr(stage => 'token', retries => 2), ++ "token retry (twice)", ++ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ ++); ++$node->connect_ok( ++ connstr(stage => 'all', retries => 1, interval => 2), ++ "token retry (two second interval)", ++ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ ++); ++$node->connect_ok( ++ connstr(stage => 'all', retries => 1, interval => JSON::PP::null), ++ "token retry (default interval)", ++ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ ++); ++ ++$node->connect_ok( ++ connstr(stage => 'all', content_type => 'application/json;charset=utf-8'), ++ "content type with charset", ++ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ ++); ++$node->connect_ok( ++ connstr(stage => 'all', content_type => "application/json \t;\t charset=utf-8"), ++ "content type with charset (whitespace)", ++ expected_stderr => qr@Visit https://example\.com/ and enter the code: postgresuser@ ++); ++ ++$node->connect_fails( ++ connstr(stage => 'device', content_type => 'text/plain'), ++ "bad device authz response: wrong content type", ++ expected_stderr => qr/failed to parse device authorization: unexpected content type/ ++); ++$node->connect_fails( ++ connstr(stage => 'token', content_type => 'text/plain'), ++ "bad token response: wrong content type", ++ expected_stderr => qr/failed to parse access token response: unexpected content type/ ++); ++$node->connect_fails( ++ connstr(stage => 'token', content_type => 'application/jsonx'), ++ "bad token response: wrong content type (correct prefix)", ++ expected_stderr => qr/failed to parse access token response: unexpected content type/ ++); ++ ++$node->connect_fails( ++ connstr(stage => 'all', interval => ~0, retries => 1, retry_code => "slow_down"), ++ "bad token response: server overflows the device authz interval", ++ expected_stderr => qr/failed to obtain access token: slow_down interval overflow/ ++); ++ +$node->stop; + +done_testing(); @@ src/test/modules/oauth_validator/t/oauth_server.py (new) @@ +#! /usr/bin/env python3 + ++import base64 +import http.server +import json +import os +import sys ++import time ++import urllib.parse ++from collections import defaultdict + + +class OAuthHandler(http.server.BaseHTTPRequestHandler): @@ src/test/modules/oauth_validator/t/oauth_server.py (new) + Switches the behavior of the provider depending on the issuer URI. + """ + self._alt_issuer = self.path.startswith("/alternate/") ++ self._parameterized = self.path.startswith("/param/") ++ + if self._alt_issuer: + self.path = self.path.removeprefix("/alternate") ++ elif self._parameterized: ++ self.path = self.path.removeprefix("/param") + + def do_GET(self): ++ self._response_code = 200 + self._check_issuer() + + if self.path == "/.well-known/openid-configuration": @@ src/test/modules/oauth_validator/t/oauth_server.py (new) + + self._send_json(resp) + ++ def _parse_params(self) -> dict[str, str]: ++ """ ++ Parses apart the form-urlencoded request body and returns the resulting ++ dict. For use by do_POST(). ++ """ ++ size = int(self.headers["Content-Length"]) ++ form = self.rfile.read(size) ++ ++ assert self.headers["Content-Type"] == "application/x-www-form-urlencoded" ++ return urllib.parse.parse_qs(form.decode("utf-8"), strict_parsing=True) ++ ++ @property ++ def client_id(self) -> str: ++ """ ++ Returns the client_id sent in the POST body. self._parse_params() must ++ have been called first. ++ """ ++ return self._params["client_id"][0] ++ + def do_POST(self): ++ self._response_code = 200 + self._check_issuer() + ++ self._params = self._parse_params() ++ if self._parameterized: ++ # Pull encoded test parameters out of the peer's client_id field. ++ # This is expected to be Base64-encoded JSON. ++ js = base64.b64decode(self.client_id) ++ self._test_params = json.loads(js) ++ + if self.path == "/authorize": + resp = self.authorization() + elif self.path == "/token": + resp = self.token() + else: -+ self.send_error(404, "Not Found") ++ self.send_error(404) + return + + self._send_json(resp) + ++ def _should_modify(self) -> bool: ++ """ ++ Returns True if the client has requested a modification to this stage of ++ the exchange. ++ """ ++ if not hasattr(self, "_test_params"): ++ return False ++ ++ stage = self._test_params.get("stage") ++ ++ return ( ++ stage == "all" ++ or (stage == "device" and self.path == "/authorize") ++ or (stage == "token" and self.path == "/token") ++ ) ++ ++ def _content_type(self) -> str: ++ """ ++ Returns "application/json" unless the test has requested something ++ different. ++ """ ++ if self._should_modify() and "content_type" in self._test_params: ++ return self._test_params["content_type"] ++ ++ return "application/json" ++ ++ def _interval(self) -> int: ++ """ ++ Returns 0 unless the test has requested something different. ++ """ ++ if self._should_modify() and "interval" in self._test_params: ++ return self._test_params["interval"] ++ ++ return 0 ++ ++ def _retry_code(self) -> str: ++ """ ++ Returns "authorization_pending" unless the test has requested something ++ different. ++ """ ++ if self._should_modify() and "retry_code" in self._test_params: ++ return self._test_params["retry_code"] ++ ++ return "authorization_pending" ++ + def _send_json(self, js: JsonObject) -> None: + """ + Sends the provided JSON dict as an application/json response. ++ self._response_code can be modified to send JSON error responses. + """ -+ + resp = json.dumps(js).encode("ascii") ++ self.log_message("sending JSON response: %s", resp) + -+ self.send_response(200, "OK") -+ self.send_header("Content-Type", "application/json") ++ self.send_response(self._response_code) ++ self.send_header("Content-Type", self._content_type()) + self.send_header("Content-Length", str(len(resp))) + self.end_headers() + @@ src/test/modules/oauth_validator/t/oauth_server.py (new) + + def config(self) -> JsonObject: + port = self.server.socket.getsockname()[1] ++ + issuer = f"http://localhost:{port}" + if self._alt_issuer: + issuer += "/alternate" ++ elif self._parameterized: ++ issuer += "/param" + + return { + "issuer": issuer, @@ src/test/modules/oauth_validator/t/oauth_server.py (new) + "grant_types_supported": ["urn:ietf:params:oauth:grant-type:device_code"], + } + ++ @property ++ def _token_state(self): ++ """ ++ A cached _TokenState object for the connected client (as determined by ++ the request's client_id), or a new one if it doesn't already exist. ++ ++ This relies on the existence of a defaultdict attached to the server; ++ see main() below. ++ """ ++ return self.server.token_state[self.client_id] ++ ++ def _remove_token_state(self): ++ """ ++ Removes any cached _TokenState for the current client_id. Call this ++ after the token exchange ends to get rid of unnecessary state. ++ """ ++ if self.client_id in self.server.token_state: ++ del self.server.token_state[self.client_id] ++ + def authorization(self) -> JsonObject: + uri = "https://example.com/" + if self._alt_issuer: + uri = "https://example.org/" + -+ return { ++ resp = { + "device_code": "postgres", + "user_code": "postgresuser", -+ "interval": 0, + "verification_uri": uri, + "expires-in": 5, + } + ++ interval = self._interval() ++ if interval is not None: ++ resp["interval"] = interval ++ self._token_state.min_delay = interval ++ else: ++ self._token_state.min_delay = 5 # default ++ ++ return resp ++ + def token(self) -> JsonObject: ++ if self._should_modify() and "retries" in self._test_params: ++ retries = self._test_params["retries"] ++ ++ # Check to make sure the token interval is being respected. ++ now = time.monotonic() ++ if self._token_state.last_try is not None: ++ delay = now - self._token_state.last_try ++ assert ( ++ delay > self._token_state.min_delay ++ ), f"client waited only {delay} seconds between token requests (expected {self._token_state.min_delay})" ++ ++ self._token_state.last_try = now ++ ++ # If we haven't reached the required number of retries yet, return a ++ # "pending" response. ++ if self._token_state.retries < retries: ++ self._token_state.retries += 1 ++ ++ self._response_code = 400 ++ return {"error": self._retry_code()} ++ ++ # Clean up any retry tracking state now that the exchange is ending. ++ self._remove_token_state() ++ + token = "9243959234" + if self._alt_issuer: + token += "-alt" @@ src/test/modules/oauth_validator/t/oauth_server.py (new) +def main(): + s = http.server.HTTPServer(("127.0.0.1", 0), OAuthHandler) + ++ # Attach a "cache" dictionary to the server to allow the OAuthHandlers to ++ # track state across token requests. The use of defaultdict ensures that new ++ # entries will be created automatically. ++ class _TokenState: ++ retries = 0 ++ min_delay = None ++ last_try = None ++ ++ s.token_state = defaultdict(_TokenState) ++ + # Give the parent the port number to contact (this is also the signal that + # we're ready to receive requests). + port = s.socket.getsockname()[1] @@ src/test/modules/oauth_validator/validator.c (new) + /* Check to make sure our private state still exists. */ + if (state->private_data != PRIVATE_COOKIE) + elog(ERROR, "oauth_validator: private state cookie changed to %p", -+ state->private_data); ++ state->private_data); + + res = palloc(sizeof(ValidatorModuleResult)); + @@ src/test/perl/PostgreSQL/Test/OAuthServer.pm (new) + +use warnings; +use strict; -+use threads; +use Scalar::Util; +use Socket; +use IO::Select; ++use Test::More; + +local *server_socket; + @@ src/test/perl/PostgreSQL/Test/OAuthServer.pm (new) + my $port; + + my $pid = open(my $read_fh, "-|", $ENV{PYTHON}, "t/oauth_server.py") -+ // die "failed to start OAuth server: $!"; ++ or die "failed to start OAuth server: $!"; + + read($read_fh, $port, 7) // die "failed to read port number: $!"; + chomp $port; @@ src/test/perl/PostgreSQL/Test/OAuthServer.pm (new) + $self->{'port'} = $port; + $self->{'child'} = $read_fh; + -+ print("# OAuth provider (PID $pid) is listening on port $port\n"); ++ diag("OAuth provider (PID $pid) is listening on port $port\n"); +} + +sub stop +{ + my $self = shift; + -+ print("# Sending SIGTERM to OAuth provider PID: $self->{'pid'}\n"); ++ diag("Sending SIGTERM to OAuth provider PID: $self->{'pid'}\n"); + + kill(15, $self->{'pid'}); + $self->{'pid'} = undef; @@ src/tools/pgindent/typedefs.list: VacuumRelation ValidIOData ValidateIndexState +ValidatorModuleState ++ValidatorModuleResult ValuesScan ValuesScanState Var 6: 84c6893325 ! 6: d163d2ca0a Review comments @@ Commit message Fixes and tidy-ups following a review of v21, a few items are (listed in no specific order): - * Implement a version check for libcurl in autoconf, the - equivalent check for Meson is still a TODO. + * Implement a version check for libcurl in autoconf, the equivalent + check for Meson is still a TODO. [ed: moved to an earlier commit] * Address a few TODOs in the code - * libpq JSON support memory management fixups [ed: these have been moved - to an earlier commit] - - ## config/programs.m4 ## -@@ config/programs.m4: if test "$pgac_cv_ldap_safe" != yes; then - *** also uses LDAP will crash on exit.]) - fi]) - -+# PGAC_CHECK_LIBCURL -+# ------------------ -+# Check for libcurl 8.4.0 or higher since earlier versions can be compiled -+# with a codepatch containing exit(), and PostgreSQL does not allow any lib -+# linked to libpq which can call exit. - -+# PGAC_CHECK_LIBCURL -+AC_DEFUN([PGAC_CHECK_LIBCURL], -+[AC_CACHE_CHECK([for compatible libcurl], [pgac_cv_check_libcurl], -+[AC_COMPILE_IFELSE([AC_LANG_PROGRAM( -+[#include -+#if LIBCURL_VERSION_MAJOR <= 8 && LIBCURL_VERSION_MINOR < 4 -+choke me -+#endif], [])], -+[pgac_cv_check_libcurl=yes], -+[pgac_cv_check_libcurl=no])]) -+ -+if test "$pgac_cv_check_libcurl" != yes; then -+ AC_MSG_ERROR([ -+*** The installed version of libcurl is too old to use with PostgreSQL. -+*** libcurl version 8.4.0 or later is required.]) -+fi]) - - # PGAC_CHECK_READLINE - # ------------------- - - ## configure ## -@@ configure: else - as_fn_error $? "library 'curl' is required for --with-oauth=curl" "$LINENO" 5 - fi - -+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for compatible libcurl" >&5 -+$as_echo_n "checking for compatible libcurl... " >&6; } -+if ${pgac_cv_check_libcurl+:} false; then : -+ $as_echo_n "(cached) " >&6 -+else -+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext -+/* end confdefs.h. */ -+#include -+#if LIBCURL_VERSION_MAJOR <= 8 && LIBCURL_VERSION_MINOR < 4 -+choke me -+#endif -+int -+main () -+{ -+ -+ ; -+ return 0; -+} -+_ACEOF -+if ac_fn_c_try_compile "$LINENO"; then : -+ pgac_cv_check_libcurl=yes -+else -+ pgac_cv_check_libcurl=no -+fi -+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext -+fi -+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_check_libcurl" >&5 -+$as_echo "$pgac_cv_check_libcurl" >&6; } -+ -+if test "$pgac_cv_check_libcurl" != yes; then -+ as_fn_error $? " -+*** The installed version of libcurl is too old to use with PostgreSQL. -+*** libcurl version 8.4.0 or later is required." "$LINENO" 5 -+fi - fi - - # for contrib/sepgsql - - ## configure.ac ## -@@ configure.ac: AC_SUBST(LDAP_LIBS_BE) - - if test "$with_oauth" = curl ; then - AC_CHECK_LIB(curl, curl_multi_init, [], [AC_MSG_ERROR([library 'curl' is required for --with-oauth=curl])]) -+ PGAC_CHECK_LIBCURL - fi - - # for contrib/sepgsql + * libpq JSON support memory management fixups [ed: moved to an earlier + commit] ## src/backend/libpq/auth-oauth.c ## -@@ src/backend/libpq/auth-oauth.c: generate_error_response(struct oauth_ctx *ctx, char **output, int *outputlen) - initStringInfo(&buf); - - /* -- * TODO: note that escaping here should be belt-and-suspenders, since -- * escapable characters aren't valid in either the issuer URI or the scope -- * list, but the HBA doesn't enforce that yet. -+ * Escaping the string here is belt-and-suspenders defensive programming -+ * since escapable characters aren't valid in either the issuer URI or the -+ * scope list, but the HBA doesn't enforce that yet. - */ - appendStringInfoString(&buf, "{ \"status\": \"invalid_token\", "); - @@ src/backend/libpq/auth-oauth.c: validate_token_format(const char *header) if (!header || strlen(header) <= 7) { @@ src/backend/libpq/auth-oauth.c: validate(Port *port, const char *auth) } - ## src/include/common/oauth-common.h ## -@@ - * oauth-common.h - * Declarations for helper functions used for OAuth/OIDC authentication - * -- * Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group -+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group - * Portions Copyright (c) 1994, Regents of the University of California - * - * src/include/common/oauth-common.h - - ## src/interfaces/libpq/Makefile ## -@@ src/interfaces/libpq/Makefile: backend_src = $(top_srcdir)/src/backend - # which seems to insert references to that even in pure C code. Excluding - # __tsan_func_exit is necessary when using ThreadSanitizer data race detector - # which use this function for instrumentation of function exit. -+# libcurl registers an exit handler in the memory debugging code when running -+# with LeakSanitizer. - # Skip the test when profiling, as gcc may insert exit() calls for that. - # Also skip the test on platforms where libpq infrastructure may be provided - # by statically-linked libraries, as we can't expect them to honor this -@@ src/interfaces/libpq/Makefile: backend_src = $(top_srcdir)/src/backend - libpq-refs-stamp: $(shlib) - ifneq ($(enable_coverage), yes) - ifeq (,$(filter solaris,$(PORTNAME))) -- @if nm -A -u $< 2>/dev/null | grep -v -e __cxa_atexit -e __tsan_func_exit | grep exit; then \ -+ @if nm -A -u $< 2>/dev/null | grep -v -e __cxa_atexit -e __tsan_func_exit -e _atexit | grep exit; then \ - echo 'libpq must not be calling any function which invokes exit'; exit 1; \ - fi - endif - ## src/interfaces/libpq/fe-auth-oauth-curl.c ## -@@ - * fe-auth-oauth-curl.c - * The libcurl implementation of OAuth/OIDC authentication. - * -- * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group -+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group - * Portions Copyright (c) 1994, Regents of the University of California - * - * IDENTIFICATION @@ #include "libpq-int.h" #include "mb/pg_wchar.h" @@ src/interfaces/libpq/fe-auth-oauth-curl.c /* * Parsed JSON Representations * -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: free_token(struct token *tok) - /* States for the overall async machine. */ - typedef enum - { -- OAUTH_STEP_INIT, -+ OAUTH_STEP_INIT = 0, - OAUTH_STEP_DISCOVERY, - OAUTH_STEP_DEVICE_AUTHORIZATION, - OAUTH_STEP_TOKEN_REQUEST, -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: typedef enum - } OAuthStep; - - /* -- * The async_ctx holds onto state that needs to persist across multiple calls to -- * pg_fe_run_oauth_flow(). Almost everything interacts with this in some way. -+ * The async_ctx holds onto state that needs to persist across multiple calls -+ * to pg_fe_run_oauth_flow(). Almost everything interacts with this in some -+ * way. - */ - struct async_ctx - { -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: struct async_ctx - int timerfd; /* a timerfd for signaling async timeouts */ - #endif - pgsocket mux; /* the multiplexer socket containing all -- * descriptors tracked by cURL, plus the -+ * descriptors tracked by libcurl, plus the - * timerfd */ -- CURLM *curlm; /* top-level multi handle for cURL operations */ -+ CURLM *curlm; /* top-level multi handle for libcurl -+ * operations */ - CURL *curl; /* the (single) easy handle for serial - * requests */ - -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: struct async_ctx - * actx_error[_str] to manipulate this. This must be filled - * with something useful on an error. - * -- * - curl_err: an optional static error buffer used by cURL to put -+ * - curl_err: an optional static error buffer used by libcurl to put - * detailed information about failures. Unfortunately - * untranslatable. - * -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: struct async_ctx - */ - const char *errctx; /* not freed; must point to static allocation */ - PQExpBufferData errbuf; -- char curl_err[CURL_ERROR_SIZE]; -+ PQExpBufferData curl_err; - - /* - * These documents need to survive over multiple calls, and are therefore @@ src/interfaces/libpq/fe-auth-oauth-curl.c: struct async_ctx struct device_authz authz; @@ src/interfaces/libpq/fe-auth-oauth-curl.c: struct async_ctx }; /* -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: free_curl_async_ctx(PGconn *conn, void *ctx) - - if (err) - libpq_append_conn_error(conn, -- "cURL easy handle removal failed: %s", -+ "libcurl easy handle removal failed: %s", - curl_multi_strerror(err)); - } - -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: free_curl_async_ctx(PGconn *conn, void *ctx) - - if (err) - libpq_append_conn_error(conn, -- "cURL multi handle cleanup failed: %s", -+ "libcurl multi handle cleanup failed: %s", - curl_multi_strerror(err)); - } - -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: free_curl_async_ctx(PGconn *conn, void *ctx) - appendPQExpBufferStr(&(ACTX)->errbuf, S) - - /* -- * Macros for getting and setting state for the connection's two cURL handles, -- * so you don't have to write out the error handling every time. -+ * Macros for getting and setting state for the connection's two libcurl -+ * handles, so you don't have to write out the error handling every time. - */ - - #define CHECK_MSETOPT(ACTX, OPT, VAL, FAILACTION) \ @@ src/interfaces/libpq/fe-auth-oauth-curl.c: parse_oauth_json(struct async_ctx *actx, const struct json_field *fields) - actx_error(actx, "no content type was provided"); - goto cleanup; - } -- else if (strcasecmp(content_type, "application/json") != 0) -+ -+ /* -+ * We only check the media-type and not the parameters, so we need to -+ * perform a length limited comparison and not compare the whole string. -+ */ -+ if (pg_strncasecmp(content_type, "application/json", strlen("application/json")) != 0) - { -- actx_error(actx, "unexpected content type \"%s\"", content_type); -- goto cleanup; -+ actx_error(actx, "unexpected content type: \"%s\"", content_type); -+ return false; - } - - if (strlen(resp->data) != resp->len) - { - actx_error(actx, "response contains embedded NULLs"); -- goto cleanup; -+ return false; + return false; } - makeJsonLexContextCstringLen(&lex, resp->data, resp->len, PG_UTF8, true); @@ src/interfaces/libpq/fe-auth-oauth-curl.c: parse_oauth_json(struct async_ctx *ac ctx.errbuf = &actx->errbuf; ctx.fields = fields; -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: parse_device_authz(struct async_ctx *actx, struct device_authz *authz) - authz->interval = parse_interval(authz->interval_str); - else - { -- /* TODO: handle default interval of 5 seconds */ -+ /* -+ * RFC 8628 specify 5 seconds as the default value if the server -+ * doesn't provide an interval. -+ */ -+ authz->interval = 5; - } - - return true; -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: parse_access_token(struct async_ctx *actx, struct token *tok) - } - - /* -- * cURL Multi Setup/Callbacks -+ * libcurl Multi Setup/Callbacks - */ - - /* -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: setup_multiplexer(struct async_ctx *actx) - - /* - * Adds and removes sockets from the multiplexer set, as directed by the -- * cURL multi handle. -+ * libcurl multi handle. - */ - static int - register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx, -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx, - break; - - default: -- actx_error(actx, "unknown cURL socket operation (%d)", what); -+ actx_error(actx, "unknown libcurl socket operation: %d", what); - return -1; - } - -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx, - break; - - default: -- actx_error(actx, "unknown cURL socket operation (%d)", what); -+ actx_error(actx, "unknown libcurl socket operation: %d", what); - return -1; - } - -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx, - /* - * EV_RECEIPT should guarantee one EV_ERROR result for every change, - * whether successful or not. Failed entries contain a non-zero errno -- * in the `data` field. -+ * in the data field. - */ - Assert(ev_out[i].flags & EV_ERROR); - -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx, - - /* - * Adds or removes timeouts from the multiplexer set, as directed by the -- * cURL multi handle. Rather than continually adding and removing the timer, -- * we keep it in the set at all times and just disarm it when it's not -- * needed. -+ * libcurl multi handle. Rather than continually adding and removing the timer, -+ * we keep it in the set at all times and just disarm it when it's not needed. - */ - static int - register_timer(CURLM *curlm, long timeout, void *ctx) -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: register_timer(CURLM *curlm, long timeout, void *ctx) - else if (timeout == 0) - { - /* -- * A zero timeout means cURL wants us to call back immediately. That's -- * not technically an option for timerfd, but we can make the timeout -- * ridiculously short. -+ * A zero timeout means libcurl wants us to call back immediately. -+ * That's not technically an option for timerfd, but we can make the -+ * timeout ridiculously short. - * - * TODO: maybe just signal drive_request() to immediately call back in - * this case? -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: register_timer(CURLM *curlm, long timeout, void *ctx) - return 0; - } - -+static int -+debug_callback(CURL *handle, curl_infotype *type, char *data, size_t size, -+ void *clientp) -+{ -+ struct async_ctx *actx = (struct async_ctx *) clientp; -+ -+ /* For now we only store TEXT debug information, extending is a TODO */ -+ if (type == CURLINFO_TEXT) -+ appendBinaryPQExpBuffer(&actx->curl_err, data, size); -+ -+ return 0; -+} -+ - /* -- * Initializes the two cURL handles in the async_ctx. The multi handle, -+ * Initializes the two libcurl handles in the async_ctx. The multi handle, - * actx->curlm, is what drives the asynchronous engine and tells us what to do - * next. The easy handle, actx->curl, encapsulates the state for a single - * request/response. It's added to the multi handle as needed, during -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: register_timer(CURLM *curlm, long timeout, void *ctx) - static bool - setup_curl_handles(struct async_ctx *actx) - { -- curl_version_info_data *curl_info; -+ curl_version_info_data *curl_info; - - /* - * Create our multi handle. This encapsulates the entire conversation with -- * cURL for this connection. -+ * libcurl for this connection. - */ - actx->curlm = curl_multi_init(); - if (!actx->curlm) - { - /* We don't get a lot of feedback on the failure reason. */ -- actx_error(actx, "failed to create cURL multi handle"); -+ actx_error(actx, "failed to create libcurl multi handle"); - return false; - } - -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: setup_curl_handles(struct async_ctx *actx) - actx->curl = curl_easy_init(); - if (!actx->curl) - { -- actx_error(actx, "failed to create cURL handle"); -+ actx_error(actx, "failed to create libcurl handle"); - return false; - } - -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: setup_curl_handles(struct async_ctx *actx) - /* No alternative resolver, TODO: warn about timeouts */ - } - -- /* TODO investigate using conn->Pfdebug and CURLOPT_DEBUGFUNCTION here */ -+ /* -+ * Set a callback for retrieving error information from libcurl, the -+ * function only takes effect when CURLOPT_VERBOSE has been set so make -+ * sure the order is kept. -+ */ -+ CHECK_SETOPT(actx, CURLOPT_DEBUGDATA, actx, return false); -+ CHECK_SETOPT(actx, CURLOPT_DEBUGFUNCTION, debug_callback, return false); - CHECK_SETOPT(actx, CURLOPT_VERBOSE, 1L, return false); -- CHECK_SETOPT(actx, CURLOPT_ERRORBUFFER, actx->curl_err, return false); - - /* - * Only HTTP[S] is allowed. TODO: disallow HTTP without user opt-in @@ src/interfaces/libpq/fe-auth-oauth-curl.c: setup_curl_handles(struct async_ctx *actx) * pretty strict when it comes to provider behavior, so we have to check * what comes back anyway.) @@ src/interfaces/libpq/fe-auth-oauth-curl.c: setup_curl_handles(struct async_ctx * CHECK_SETOPT(actx, CURLOPT_HTTPHEADER, actx->headers, return false); return true; -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: setup_curl_handles(struct async_ctx *actx) - */ - - /* -- * Response callback from cURL; appends the response body into actx->work_data. -- * See start_request(). -+ * Response callback from libcurl which appends the response body into -+ * actx->work_data (see start_request()). The maximum size of the data is -+ * defined by CURL_MAX_WRITE_SIZE which by default is 16kb (and can only be -+ * changed by recompiling libcurl). - */ - static size_t - append_data(char *buf, size_t size, size_t nmemb, void *userdata) @@ src/interfaces/libpq/fe-auth-oauth-curl.c: append_data(char *buf, size_t size, size_t nmemb, void *userdata) PQExpBuffer resp = userdata; size_t len = size * nmemb; @@ src/interfaces/libpq/fe-auth-oauth-curl.c: drive_request(struct async_ctx *actx) { /* We'll come back again. */ return PGRES_POLLING_READING; -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: drive_request(struct async_ctx *actx) - if (msg->msg != CURLMSG_DONE) - { - /* -- * Future cURL versions may define new message types; we don't -+ * Future libcurl versions may define new message types; we don't - * know how to handle them, so we'll ignore them. - */ - continue; -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: drive_request(struct async_ctx *actx) - err = curl_multi_remove_handle(actx->curlm, msg->easy_handle); - if (err) - { -- actx_error(actx, "cURL easy handle removal failed: %s", -+ actx_error(actx, "libcurl easy handle removal failed: %s", - curl_multi_strerror(err)); - return PGRES_POLLING_FAILED; - } @@ src/interfaces/libpq/fe-auth-oauth-curl.c: start_device_authz(struct async_ctx *actx, PGconn *conn) appendPQExpBuffer(work_buffer, "client_id=%s", conn->oauth_client_id); if (conn->oauth_scope) @@ src/interfaces/libpq/fe-auth-oauth-curl.c: finish_token_request(struct async_ctx + return false; } -+ - /* -- * The top-level, nonblocking entry point for the cURL implementation. This will -- * be called several times to pump the async engine. -+ * The top-level, nonblocking entry point for the libcurl implementation. This -+ * will be called several times to pump the async engine. - * - * The architecture is based on PQconnectPoll(). The first half drives the - * connection state forward as necessary, returning if we're not ready to -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn, pgsocket *altsock) - struct token tok = {0}; - - /* -- * XXX This is not safe. cURL has stringent requirements for the thread -+ * XXX This is not safe. libcurl has stringent requirements for the thread - * context in which you call curl_global_init(), because it's going to try - * initializing a bunch of other libraries (OpenSSL, Winsock...). And we - * probably need to consider both the TLS backend libcurl is compiled -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn, pgsocket *altsock) - * Recent versions of libcurl have improved the thread-safety situation, - * but you apparently can't check at compile time whether the - * implementation is thread-safe, and there's a chicken-and-egg problem -- * where you can't check the thread safety until you've initialized cURL, -- * which you can't do before you've made sure it's thread-safe... -+ * where you can't check the thread safety until you've initialized -+ * libcurl, which you can't do before you've made sure it's thread-safe... - * - * We know we've already initialized Winsock by this point, so we should -- * be able to safely skip that bit. But we have to tell cURL to initialize -- * everything else, because other pieces of our client executable may -- * already be using cURL for their own purposes. If we initialize libcurl -- * first, with only a subset of its features, we could break those other -- * clients nondeterministically, and that would probably be a nightmare to -- * debug. -+ * be able to safely skip that bit. But we have to tell libcurl to -+ * initialize everything else, because other pieces of our client -+ * executable may already be using libcurl for their own purposes. If we -+ * initialize libcurl first, with only a subset of its features, we could -+ * break those other clients nondeterministically, and that would probably -+ * be a nightmare to debug. - */ - curl_global_init(CURL_GLOBAL_ALL - & ~CURL_GLOBAL_WIN32); /* we already initialized Winsock */ -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn, pgsocket *altsock) - - initPQExpBuffer(&actx->work_data); - initPQExpBuffer(&actx->errbuf); -+ initPQExpBuffer(&actx->curl_err); - if (!setup_multiplexer(actx)) - goto error_return; @@ src/interfaces/libpq/fe-auth-oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn, pgsocket *altsock) * errors; anything else and we bail. */ @@ src/interfaces/libpq/fe-auth-oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn, pg goto error_return; } -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: pg_fe_run_oauth_flow(PGconn *conn, pgsocket *altsock) - */ - if (strcmp(err->error, "slow_down") == 0) - { -- actx->authz.interval += 5; /* TODO check for overflow? */ -+ int prev_interval = actx->authz.interval; -+ -+ actx->authz.interval += 5; -+ if (actx->authz.interval < prev_interval) -+ { -+ actx_error(actx, "slow_down interval overflow"); -+ goto error_return; -+ } - } - - /* -@@ src/interfaces/libpq/fe-auth-oauth-curl.c: error_return: - else - appendPQExpBufferStr(&conn->errorMessage, actx->errbuf.data); - -- if (actx->curl_err[0]) -- { -- size_t len; -- -- appendPQExpBuffer(&conn->errorMessage, " (%s)", actx->curl_err); -- -- /* Sometimes libcurl adds a newline to the error buffer. :( */ -- len = conn->errorMessage.len; -- if (len >= 2 && conn->errorMessage.data[len - 2] == '\n') -- { -- conn->errorMessage.data[len - 2] = ')'; -- conn->errorMessage.data[len - 1] = '\0'; -- conn->errorMessage.len--; -- } -- } -+ if (actx->curl_err.len > 0) -+ appendPQExpBuffer(&conn->errorMessage, " (%s)", actx->curl_err.data); - - appendPQExpBufferStr(&conn->errorMessage, "\n"); - ## src/interfaces/libpq/fe-auth-oauth.c ## -@@ - * fe-auth-oauth.c - * The front-end (client) implementation of OAuth/OIDC authentication. - * -- * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group -+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group - * Portions Copyright (c) 1994, Regents of the University of California - * - * IDENTIFICATION @@ src/interfaces/libpq/fe-auth-oauth.c: handle_oauth_sasl_error(PGconn *conn, char *msg, int msglen) return false; } @@ src/interfaces/libpq/fe-auth-oauth.c: handle_oauth_sasl_error(PGconn *conn, char initPQExpBuffer(&ctx.errbuf); sem.semstate = &ctx; - - ## src/interfaces/libpq/fe-auth-oauth.h ## -@@ - * - * Definitions for OAuth authentication implementations - * -- * Portions Copyright (c) 2023, PostgreSQL Global Development Group -+ * Portions Copyright (c) 2024, PostgreSQL Global Development Group - * - * src/interfaces/libpq/fe-auth-oauth.h - * - - ## src/test/modules/oauth_validator/validator.c ## -@@ src/test/modules/oauth_validator/validator.c: validate_token(ValidatorModuleState *state, const char *token, const char *role) - /* Check to make sure our private state still exists. */ - if (state->private_data != PRIVATE_COOKIE) - elog(ERROR, "oauth_validator: private state cookie changed to %p", -- state->private_data); -+ state->private_data); - - res = palloc(sizeof(ValidatorModuleResult)); - - - ## src/test/perl/PostgreSQL/Test/OAuthServer.pm ## -@@ src/test/perl/PostgreSQL/Test/OAuthServer.pm: package PostgreSQL::Test::OAuthServer; - - use warnings; - use strict; --use threads; - use Scalar::Util; - use Socket; - use IO::Select; -+use Test::More; - - local *server_socket; - -@@ src/test/perl/PostgreSQL/Test/OAuthServer.pm: sub run - my $port; - - my $pid = open(my $read_fh, "-|", $ENV{PYTHON}, "t/oauth_server.py") -- // die "failed to start OAuth server: $!"; -+ or die "failed to start OAuth server: $!"; - -- read($read_fh, $port, 7) // die "failed to read port number: $!"; -+ read($read_fh, $port, 7) or die "failed to read port number: $!"; - chomp $port; - die "server did not advertise a valid port" - unless Scalar::Util::looks_like_number($port); -@@ src/test/perl/PostgreSQL/Test/OAuthServer.pm: sub run - $self->{'port'} = $port; - $self->{'child'} = $read_fh; - -- print("# OAuth provider (PID $pid) is listening on port $port\n"); -+ diag("OAuth provider (PID $pid) is listening on port $port\n"); - } - - sub stop - { - my $self = shift; - -- print("# Sending SIGTERM to OAuth provider PID: $self->{'pid'}\n"); -+ diag("Sending SIGTERM to OAuth provider PID: $self->{'pid'}\n"); - - kill(15, $self->{'pid'}); - $self->{'pid'} = undef; - - ## src/tools/pgindent/typedefs.list ## -@@ src/tools/pgindent/typedefs.list: VacuumStmt - ValidIOData - ValidateIndexState - ValidatorModuleState -+ValidatorModuleResult - ValuesScan - ValuesScanState - Var -: ---------- > 7: 9117bf8be2 DO NOT MERGE: Add pytest suite for OAuth