since-v47.diff.txt
text/plain
Filename: since-v47.diff.txt
Type: text/plain
Part: 0
1: 6747b7cc795 ! 1: cf86e3bfbbc Add OAUTHBEARER SASL mechanism
@@ config/programs.m4: AC_DEFUN([PGAC_CHECK_STRIP],
+ AC_DEFINE(HAVE_THREADSAFE_CURL_GLOBAL_INIT, 1,
+ [Define to 1 if curl_global_init() is guaranteed to be threadsafe.])
+ fi
++
++ # Warn if a thread-friendly DNS resolver isn't built.
++ AC_CACHE_CHECK([for curl support for asynchronous DNS], [pgac_cv__libcurl_async_dns],
++ [AC_RUN_IFELSE([AC_LANG_PROGRAM([
++#include <curl/curl.h>
++],[
++ curl_version_info_data *info;
++
++ if (curl_global_init(CURL_GLOBAL_ALL))
++ return -1;
++
++ info = curl_version_info(CURLVERSION_NOW);
++ return (info->features & CURL_VERSION_ASYNCHDNS) ? 0 : 1;
++])],
++ [pgac_cv__libcurl_async_dns=yes],
++ [pgac_cv__libcurl_async_dns=no],
++ [pgac_cv__libcurl_async_dns=unknown])])
++ if test x"$pgac_cv__libcurl_async_dns" != xyes ; then
++ AC_MSG_WARN([
++*** The installed version of libcurl does not support asynchronous DNS
++*** lookups. Connection timeouts will not be honored during DNS resolution,
++*** which may lead to hangs in client programs.])
++ fi
+])# PGAC_CHECK_LIBCURL
## configure ##
@@ configure: fi
+
+ fi
+
++ # Warn if a thread-friendly DNS resolver isn't built.
++ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for curl support for asynchronous DNS" >&5
++$as_echo_n "checking for curl support for asynchronous DNS... " >&6; }
++if ${pgac_cv__libcurl_async_dns+:} false; then :
++ $as_echo_n "(cached) " >&6
++else
++ if test "$cross_compiling" = yes; then :
++ pgac_cv__libcurl_async_dns=unknown
++else
++ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
++/* end confdefs.h. */
++
++#include <curl/curl.h>
++
++int
++main ()
++{
++
++ curl_version_info_data *info;
++
++ if (curl_global_init(CURL_GLOBAL_ALL))
++ return -1;
++
++ info = curl_version_info(CURLVERSION_NOW);
++ return (info->features & CURL_VERSION_ASYNCHDNS) ? 0 : 1;
++
++ ;
++ return 0;
++}
++_ACEOF
++if ac_fn_c_try_run "$LINENO"; then :
++ pgac_cv__libcurl_async_dns=yes
++else
++ pgac_cv__libcurl_async_dns=no
++fi
++rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
++ conftest.$ac_objext conftest.beam conftest.$ac_ext
++fi
++
++fi
++{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__libcurl_async_dns" >&5
++$as_echo "$pgac_cv__libcurl_async_dns" >&6; }
++ if test x"$pgac_cv__libcurl_async_dns" != xyes ; then
++ { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING:
++*** The installed version of libcurl does not support asynchronous DNS
++*** lookups. Connection timeouts will not be honored during DNS resolution,
++*** which may lead to hangs in client programs." >&5
++$as_echo "$as_me: WARNING:
++*** The installed version of libcurl does not support asynchronous DNS
++*** lookups. Connection timeouts will not be honored during DNS resolution,
++*** which may lead to hangs in client programs." >&2;}
++ fi
++
+fi
+
if test "$with_gssapi" = yes ; then
@@ doc/src/sgml/libpq.sgml: void PQinitSSL(int do_ssl);
+{
+ const char *verification_uri; /* verification URI to visit */
+ const char *user_code; /* user code to enter */
++ const char *verification_uri_complete; /* optional combination of URI and
++ * code, or NULL */
++ int expires_in; /* seconds until user code expires */
+} PGpromptOAuthDevice;
+</synopsis>
+ </para>
@@ doc/src/sgml/libpq.sgml: void PQinitSSL(int do_ssl);
+ <link linkend="libpq-oauth-authdata-oauth-bearer-token">custom OAuth
+ flow</link>, this authdata type will not be used.
+ </para>
++ <para>
++ If a non-NULL <structfield>verification_uri_complete</structfield> is
++ provided, it may optionally be used for non-textual verification (for
++ example, by displaying a QR code). The URL and user code should still
++ be displayed to the end user in this case, because the code will be
++ manually confirmed by the provider, and the URL lets users continue
++ even if they can't use the non-textual method. Review the RFC's
++ <ulink url="https://datatracker.ietf.org/doc/html/rfc8628#section-3.3.1">notes
++ on non-textual verification</ulink>.
++ </para>
+ </listitem>
+ </varlistentry>
+
@@ meson.build: endif
+ if libcurl_threadsafe_init
+ cdata.set('HAVE_THREADSAFE_CURL_GLOBAL_INIT', 1)
+ endif
++
++ # Warn if a thread-friendly DNS resolver isn't built.
++ libcurl_async_dns = false
++
++ if not meson.is_cross_build()
++ r = cc.run('''
++ #include <curl/curl.h>
++
++ int main(void)
++ {
++ curl_version_info_data *info;
++
++ if (curl_global_init(CURL_GLOBAL_ALL))
++ return -1;
++
++ info = curl_version_info(CURLVERSION_NOW);
++ return (info->features & CURL_VERSION_ASYNCHDNS) ? 0 : 1;
++ }''',
++ name: 'test for curl support for asynchronous DNS',
++ dependencies: libcurl,
++ )
++
++ assert(r.compiled())
++ if r.returncode() == 0
++ libcurl_async_dns = true
++ endif
++ endif
++
++ if not libcurl_async_dns
++ warning('''
++*** The installed version of libcurl does not support asynchronous DNS
++*** lookups. Connection timeouts will not be honored during DNS resolution,
++*** which may lead to hangs in client programs.''')
++ endif
+ endif
+
+else
@@ src/backend/libpq/auth-oauth.c (new)
+ char **output, int *outputlen, const char **logdetail);
+
+static void load_validator_library(const char *libname);
-+static void shutdown_validator_library(int code, Datum arg);
++static void shutdown_validator_library(void *arg);
+
+static ValidatorModuleState *validator_module_state;
+static const OAuthValidatorCallbacks *ValidatorCallbacks;
@@ src/backend/libpq/auth-oauth.c (new)
+load_validator_library(const char *libname)
+{
+ OAuthValidatorModuleInit validator_init;
++ MemoryContextCallback *mcb;
+
+ Assert(libname && *libname);
+
@@ src/backend/libpq/auth-oauth.c (new)
+ if (ValidatorCallbacks->startup_cb != NULL)
+ ValidatorCallbacks->startup_cb(validator_module_state);
+
-+ before_shmem_exit(shutdown_validator_library, 0);
++ /* Shut down the library before cleaning up its state. */
++ mcb = palloc0(sizeof(*mcb));
++ mcb->func = shutdown_validator_library;
++
++ MemoryContextRegisterResetCallback(CurrentMemoryContext, mcb);
+}
+
+/*
+ * Call the validator module's shutdown callback, if one is provided. This is
-+ * invoked via before_shmem_exit().
++ * invoked during memory context reset.
+ */
+static void
-+shutdown_validator_library(int code, Datum arg)
++shutdown_validator_library(void *arg)
+{
+ if (ValidatorCallbacks->shutdown_cb != NULL)
+ ValidatorCallbacks->shutdown_cb(validator_module_state);
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ char *device_code;
+ char *user_code;
+ char *verification_uri;
++ char *verification_uri_complete;
++ char *expires_in_str;
+ char *interval_str;
+
+ /* Fields below are parsed from the corresponding string above. */
++ int expires_in;
+ int interval;
+};
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ free(authz->device_code);
+ free(authz->user_code);
+ free(authz->verification_uri);
++ free(authz->verification_uri_complete);
++ free(authz->expires_in_str);
+ free(authz->interval_str);
+}
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+{
+ enum OAuthStep step; /* where are we in the flow? */
+
-+#ifdef HAVE_SYS_EPOLL_H
-+ int timerfd; /* a timerfd for signaling async timeouts */
-+#endif
++ int timerfd; /* descriptor for signaling async timeouts */
+ pgsocket mux; /* the multiplexer socket containing all
+ * descriptors tracked by libcurl, plus the
+ * timerfd */
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+
+ if (actx->mux != PGINVALID_SOCKET)
+ close(actx->mux);
-+#ifdef HAVE_SYS_EPOLL_H
+ if (actx->timerfd >= 0)
+ close(actx->timerfd);
-+#endif
+
+ free(actx);
+}
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ /*
+ * We should never start parsing a new field while a previous one is
+ * still active.
-+ *
-+ * TODO: this code relies on assertions too much. We need to exit
-+ * sanely on internal logic errors, to avoid turning bugs into
-+ * vulnerabilities.
+ */
-+ Assert(!ctx->active);
++ if (ctx->active)
++ {
++ Assert(false);
++ oauth_parse_set_error(ctx,
++ "internal error: started field '%s' before field '%s' was finished",
++ name, ctx->active->name);
++ return JSON_SEM_ACTION_FAILED;
++ }
+
+ while (field->name)
+ {
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ struct oauth_parse *ctx = state;
+
+ --ctx->nested;
-+ if (!ctx->nested)
-+ Assert(!ctx->active); /* all fields should be fully processed */
++
++ /*
++ * All fields should be fully processed by the end of the top-level
++ * object.
++ */
++ if (!ctx->nested && ctx->active)
++ {
++ Assert(false);
++ oauth_parse_set_error(ctx,
++ "internal error: field '%s' still active at end of object",
++ ctx->active->name);
++ return JSON_SEM_ACTION_FAILED;
++ }
+
+ return JSON_SUCCESS;
+}
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ if (ctx->active)
+ {
+ /*
-+ * This assumes that no target arrays can contain other arrays, which
-+ * we check in the array_start callback.
++ * Clear the target (which should be an array inside the top-level
++ * object). For this to be safe, no target arrays can contain other
++ * arrays; we check for that in the array_start callback.
+ */
-+ Assert(ctx->nested == 2);
-+ Assert(ctx->active->type == JSON_TOKEN_ARRAY_START);
++ if (ctx->nested != 2 || ctx->active->type != JSON_TOKEN_ARRAY_START)
++ {
++ Assert(false);
++ oauth_parse_set_error(ctx,
++ "internal error: found unexpected array end while parsing field '%s'",
++ ctx->active->name);
++ return JSON_SEM_ACTION_FAILED;
++ }
+
+ ctx->active = NULL;
+ }
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+
+ if (field->type != JSON_TOKEN_ARRAY_START)
+ {
-+ Assert(ctx->nested == 1);
-+ Assert(!*field->target.scalar);
++ /* Ensure that we're parsing the top-level keys... */
++ if (ctx->nested != 1)
++ {
++ Assert(false);
++ oauth_parse_set_error(ctx,
++ "internal error: scalar target found at nesting level %d",
++ ctx->nested);
++ return JSON_SEM_ACTION_FAILED;
++ }
++
++ /* ...and that a result has not already been set. */
++ if (*field->target.scalar)
++ {
++ Assert(false);
++ oauth_parse_set_error(ctx,
++ "internal error: scalar field '%s' would be assigned twice",
++ ctx->active->name);
++ return JSON_SEM_ACTION_FAILED;
++ }
+
+ *field->target.scalar = strdup(token);
+ if (!*field->target.scalar)
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ {
+ struct curl_slist *temp;
+
-+ Assert(ctx->nested == 2);
++ /* The target array should be inside the top-level object. */
++ if (ctx->nested != 2)
++ {
++ Assert(false);
++ oauth_parse_set_error(ctx,
++ "internal error: array member found at nesting level %d",
++ ctx->nested);
++ return JSON_SEM_ACTION_FAILED;
++ }
+
+ /* Note that curl_slist_append() makes a copy of the token. */
+ temp = curl_slist_append(*field->target.array, token);
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+}
+
+/*
++ * Parses a valid JSON number into a double. The input must have come from
++ * pg_parse_json(), so that we know the lexer has validated it; there's no
++ * in-band signal for invalid formats.
++ */
++static double
++parse_json_number(const char *s)
++{
++ double parsed;
++ int cnt;
++
++ /*
++ * The JSON lexer has already validated the number, which is stricter than
++ * the %f format, so we should be good to use sscanf().
++ */
++ cnt = sscanf(s, "%lf", &parsed);
++
++ if (cnt != 1)
++ {
++ /*
++ * Either the lexer screwed up or our assumption above isn't true, and
++ * either way a developer needs to take a look.
++ */
++ Assert(cnt == 1);
++ return 0;
++ }
++
++ return parsed;
++}
++
++/*
+ * Parses the "interval" JSON number, corresponding to the number of seconds to
+ * wait between token endpoint requests.
+ *
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ * the result at a minimum of one. (Zero-second intervals would result in an
+ * expensive network polling loop.) Tests may remove the lower bound with
+ * PGOAUTHDEBUG, for improved performance.
-+ *
-+ * TODO: maybe clamp the upper bound too, based on the libpq timeout and/or the
-+ * code expiration time?
+ */
+static int
+parse_interval(struct async_ctx *actx, const char *interval_str)
+{
+ double parsed;
-+ int cnt;
-+
-+ /*
-+ * The JSON lexer has already validated the number, which is stricter than
-+ * the %f format, so we should be good to use sscanf().
-+ */
-+ cnt = sscanf(interval_str, "%lf", &parsed);
-+
-+ if (cnt != 1)
-+ {
-+ /*
-+ * Either the lexer screwed up or our assumption above isn't true, and
-+ * either way a developer needs to take a look.
-+ */
-+ Assert(cnt == 1);
-+ return 1; /* don't fall through in release builds */
-+ }
+
++ parsed = parse_json_number(interval_str);
+ parsed = ceil(parsed);
+
+ if (parsed < 1)
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+}
+
+/*
++ * Parses the "expires_in" JSON number, corresponding to the number of seconds
++ * remaining in the lifetime of the device code request.
++ *
++ * 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.
++ */
++static int
++parse_expires_in(struct async_ctx *actx, const char *expires_in_str)
++{
++ double parsed;
++
++ parsed = parse_json_number(expires_in_str);
++ parsed = round(parsed);
++
++ if (INT_MAX <= parsed)
++ return INT_MAX;
++ else if (parsed <= INT_MIN)
++ return INT_MIN;
++
++ return parsed;
++}
++
++/*
+ * Parses the Device Authorization Response (RFC 8628, Sec. 3.2).
+ */
+static bool
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ {"device_code", JSON_TOKEN_STRING, {&authz->device_code}, REQUIRED},
+ {"user_code", JSON_TOKEN_STRING, {&authz->user_code}, REQUIRED},
+ {"verification_uri", JSON_TOKEN_STRING, {&authz->verification_uri}, REQUIRED},
++ {"expires_in", JSON_TOKEN_NUMBER, {&authz->expires_in_str}, REQUIRED},
+
+ /*
+ * Some services (Google, Azure) spell verification_uri differently.
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ */
+ {"verification_url", JSON_TOKEN_STRING, {&authz->verification_uri}, REQUIRED},
+
-+ /*
-+ * The following fields are technically REQUIRED, but we don't use
-+ * them anywhere yet:
-+ *
-+ * - expires_in
-+ */
-+
++ {"verification_uri_complete", JSON_TOKEN_STRING, {&authz->verification_uri_complete}, OPTIONAL},
+ {"interval", JSON_TOKEN_NUMBER, {&authz->interval_str}, OPTIONAL},
+
+ {0},
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ authz->interval = 5;
+ }
+
++ Assert(authz->expires_in_str); /* ensured by parse_oauth_json() */
++ authz->expires_in = parse_expires_in(actx, authz->expires_in_str);
++
+ return true;
+}
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ {"access_token", JSON_TOKEN_STRING, {&tok->access_token}, REQUIRED},
+ {"token_type", JSON_TOKEN_STRING, {&tok->token_type}, REQUIRED},
+
-+ /*
-+ * The following fields are technically REQUIRED, but we don't use
-+ * them anywhere yet:
++ /*---
++ * We currently have no use for the following OPTIONAL fields:
+ *
-+ * - scope (only required if different than requested -- TODO check)
++ * - expires_in: This will be important for maintaining a token cache,
++ * but we do not yet implement one.
++ *
++ * - refresh_token: Ditto.
++ *
++ * - scope: This is only sent when the authorization server sees fit to
++ * change our scope request. It's not clear what we should do
++ * about this; either it's been done as a matter of policy, or
++ * the user has explicitly denied part of the authorization,
++ * and either way the server-side validator is in a better
++ * place to complain if the change isn't acceptable.
+ */
+
+ {0},
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ * select() on instead of the Postgres socket during OAuth negotiation.
+ *
+ * This is just an epoll set or kqueue abstracting multiple other descriptors.
-+ * A timerfd is always part of the set when using epoll; it's just disabled
-+ * when we're not using it.
++ * For epoll, the timerfd is always part of the set; it's just disabled when
++ * we're not using it. For kqueue, the "timerfd" is actually a second kqueue
++ * instance which is only added to the set when needed.
+ */
+static bool
+setup_multiplexer(struct async_ctx *actx)
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ return false;
+ }
+
++ /*
++ * Originally, we set EVFILT_TIMER directly on the top-level multiplexer.
++ * This makes it difficult to implement timer_expired(), though, so now we
++ * set EVFILT_TIMER on a separate actx->timerfd, which is chained to
++ * actx->mux while the timer is active.
++ */
++ actx->timerfd = kqueue();
++ if (actx->timerfd < 0)
++ {
++ actx_error(actx, "failed to create timer kqueue: %m");
++ return false;
++ }
++
+ return true;
+#endif
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+
+/*
+ * Enables or disables the timer in the multiplexer set. The timeout value is
-+ * in milliseconds (negative values disable the timer). 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.
++ * in milliseconds (negative values disable the timer).
++ *
++ * For epoll, 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. For kqueue,
++ * the timer is removed completely when disabled to prevent stale timeouts from
++ * remaining in the queue.
+ */
+static bool
+set_timer(struct async_ctx *actx, long timeout)
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ actx_error(actx, "setting timerfd to %ld: %m", timeout);
+ return false;
+ }
++
++ return true;
+#endif
+#ifdef HAVE_SYS_EVENT_H
+ struct kevent ev;
+
-+ EV_SET(&ev, 1, EVFILT_TIMER, timeout < 0 ? EV_DELETE : EV_ADD,
++ /* Enable/disable the timer itself. */
++ EV_SET(&ev, 1, EVFILT_TIMER, timeout < 0 ? EV_DELETE : (EV_ADD | EV_ONESHOT),
+ 0, timeout, 0);
-+ if (kevent(actx->mux, &ev, 1, NULL, 0, NULL) < 0 && errno != ENOENT)
++ if (kevent(actx->timerfd, &ev, 1, NULL, 0, NULL) < 0 && errno != ENOENT)
+ {
+ actx_error(actx, "setting kqueue timer to %ld: %m", timeout);
+ return false;
+ }
-+#endif
++
++ /*
++ * Add/remove the timer to/from the mux. (In contrast with epoll, if we
++ * allowed the timer to remain registered here after being disabled, the
++ * mux queue would retain any previous stale timeout notifications and
++ * remain readable.)
++ */
++ EV_SET(&ev, actx->timerfd, EVFILT_READ, timeout < 0 ? EV_DELETE : EV_ADD,
++ 0, 0, 0);
++ if (kevent(actx->mux, &ev, 1, NULL, 0, NULL) < 0 && errno != ENOENT)
++ {
++ actx_error(actx, "could not update timer on kqueue: %m");
++ return false;
++ }
+
+ return true;
++#endif
++
++ actx_error(actx, "libpq does not support timers on this platform");
++ return false;
++}
++
++/*
++ * Returns 1 if the timeout in the multiplexer set has expired since the last
++ * call to set_timer(), 0 if the timer is still running, or -1 (with an
++ * actx_error() report) if the timer cannot be queried.
++ */
++static int
++timer_expired(struct async_ctx *actx)
++{
++#if HAVE_SYS_EPOLL_H
++ struct itimerspec spec = {0};
++
++ if (timerfd_gettime(actx->timerfd, &spec) < 0)
++ {
++ actx_error(actx, "getting timerfd value: %m");
++ return -1;
++ }
++
++ /*
++ * This implementation assumes we're using single-shot timers. If you
++ * change to using intervals, you'll need to reimplement this function
++ * too, possibly with the read() or select() interfaces for timerfd.
++ */
++ Assert(spec.it_interval.tv_sec == 0
++ && spec.it_interval.tv_nsec == 0);
++
++ /* If the remaining time to expiration is zero, we're done. */
++ return (spec.it_value.tv_sec == 0
++ && spec.it_value.tv_nsec == 0);
++#endif
++#ifdef HAVE_SYS_EVENT_H
++ int res;
++
++ /* Is the timer queue ready? */
++ res = PQsocketPoll(actx->timerfd, 1 /* forRead */ , 0, 0);
++ if (res < 0)
++ {
++ actx_error(actx, "checking kqueue for timeout: %m");
++ return -1;
++ }
++
++ return (res > 0);
++#endif
++
++ actx_error(actx, "libpq does not support timers on this platform");
++ return -1;
+}
+
+/*
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ struct async_ctx *actx = ctx;
+
+ /*
-+ * TODO: maybe just signal drive_request() to immediately call back in the
-+ * (timeout == 0) case?
++ * There might be an optimization opportunity here: if timeout == 0, we
++ * could signal drive_request to immediately call
++ * curl_multi_socket_action, rather than returning all the way up the
++ * stack only to come right back. But it's not clear that the additional
++ * code complexity is worth it.
+ */
+ if (!set_timer(actx, timeout))
+ return -1; /* actx_error already called */
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+debug_callback(CURL *handle, curl_infotype type, char *data, size_t size,
+ void *clientp)
+{
-+ const char *const end = data + size;
+ const char *prefix;
++ bool printed_prefix = false;
++ PQExpBufferData buf;
+
+ /* Prefixes are modeled off of the default libcurl debug output. */
+ switch (type)
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ return 0;
+ }
+
++ initPQExpBuffer(&buf);
++
+ /*
+ * Split the output into lines for readability; sometimes multiple headers
-+ * are included in a single call.
++ * are included in a single call. We also don't allow unprintable ASCII
++ * through without a basic <XX> escape.
+ */
-+ while (data < end)
++ for (int i = 0; i < size; i++)
+ {
-+ size_t len = end - data;
-+ char *eol = memchr(data, '\n', len);
++ char c = data[i];
+
-+ if (eol)
-+ len = eol - data + 1;
++ if (!printed_prefix)
++ {
++ appendPQExpBuffer(&buf, "%s ", prefix);
++ printed_prefix = true;
++ }
+
-+ /* TODO: handle unprintables */
-+ fprintf(stderr, "%s %.*s%s", prefix, (int) len, data,
-+ eol ? "" : "\n");
++ if (c >= 0x20 && c <= 0x7E)
++ appendPQExpBufferChar(&buf, c);
++ else if ((type == CURLINFO_HEADER_IN
++ || type == CURLINFO_HEADER_OUT
++ || type == CURLINFO_TEXT)
++ && (c == '\r' || c == '\n'))
++ {
++ /*
++ * Don't bother emitting <0D><0A> for headers and text; it's not
++ * helpful noise.
++ */
++ }
++ else
++ appendPQExpBuffer(&buf, "<%02X>", c);
+
-+ data += len;
++ if (c == '\n')
++ {
++ appendPQExpBufferChar(&buf, c);
++ printed_prefix = false;
++ }
+ }
+
++ if (printed_prefix)
++ appendPQExpBufferChar(&buf, '\n'); /* finish the line */
++
++ fprintf(stderr, "%s", buf.data);
++ termPQExpBuffer(&buf);
+ return 0;
+}
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+static bool
+setup_curl_handles(struct async_ctx *actx)
+{
-+ curl_version_info_data *curl_info;
-+
+ /*
+ * Create our multi handle. This encapsulates the entire conversation with
+ * libcurl for this connection.
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ }
+
+ /*
-+ * Extract information about the libcurl we are linked against.
-+ */
-+ curl_info = curl_version_info(CURLVERSION_NOW);
-+
-+ /*
+ * The multi handle tells us what to wait on using two callbacks. These
+ * will manipulate actx->mux as needed.
+ */
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ * Multi-threaded applications must set CURLOPT_NOSIGNAL. This requires us
+ * to handle the possibility of SIGPIPE ourselves using pq_block_sigpipe;
+ * see pg_fe_run_oauth_flow().
++ *
++ * NB: If libcurl is not built against a friendly DNS resolver (c-ares or
++ * threaded), setting this option prevents DNS lookups from timing out
++ * correctly. We warn about this situation at configure time.
+ */
+ CHECK_SETOPT(actx, CURLOPT_NOSIGNAL, 1L, return false);
-+ if (!curl_info->ares_num)
-+ {
-+ /* No alternative resolver, TODO: warn about timeouts */
-+ }
+
+ if (actx->debugging)
+ {
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ CHECK_SETOPT(actx, popt, protos, return false);
+ }
+
-+ /* TODO: would anyone use this in "real" situations, or just testing? */
++ /*
++ * If we're in debug mode, allow the developer to change the trusted CA
++ * list. For now, this is not something we expose outside of the UNSAFE
++ * mode, because it's not clear that it's useful in production: both libpq
++ * and the user's browser must trust the same authorization servers for
++ * the flow to work at all, so any changes to the roots are likely to be
++ * done system-wide.
++ */
+ if (actx->debugging)
+ {
+ const char *env;
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ * of the authorization server where the authorization request was
+ * sent to. This comparison MUST use simple string comparison as defined
+ * in Section 6.2.1 of [RFC3986].
-+ *
-+ * TODO: Encoding support?
+ */
+ if (strcmp(conn->oauth_issuer_id, provider->issuer) != 0)
+ {
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ PGpromptOAuthDevice prompt = {
+ .verification_uri = actx->authz.verification_uri,
+ .user_code = actx->authz.user_code,
-+ /* TODO: optional fields */
++ .verification_uri_complete = actx->authz.verification_uri_complete,
++ .expires_in = actx->authz.expires_in,
+ };
+
+ res = PQauthDataHook(PQAUTHDATA_PROMPT_OAUTH_DEVICE, conn, &prompt);
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ }
+
+ actx->mux = PGINVALID_SOCKET;
-+#ifdef HAVE_SYS_EPOLL_H
+ actx->timerfd = -1;
-+#endif
+
+ /* Should we enable unsafe features? */
+ actx->debugging = oauth_unsafe_debugging_enabled();
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ /* not done yet */
+ return status;
+ }
++
++ break;
+ }
+
+ case OAUTH_STEP_WAIT_INTERVAL:
-+ /* TODO check that the timer has expired */
++
++ /*
++ * The client application is supposed to wait until our timer
++ * expires before calling PQconnectPoll() again, but that
++ * might not happen. To avoid sending a token request early,
++ * check the timer before continuing.
++ */
++ if (!timer_expired(actx))
++ {
++ conn->altsock = actx->timerfd;
++ return PGRES_POLLING_READING;
++ }
++
++ /* Disable the expired timer. */
++ if (!set_timer(actx, -1))
++ goto error_return;
++
+ break;
+ }
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ if (!set_timer(actx, actx->authz.interval * 1000))
+ goto error_return;
+
-+#ifdef HAVE_SYS_EPOLL_H
-+
+ /*
+ * No Curl requests are running, so we can simplify by having
+ * the client wait directly on the timerfd rather than the
-+ * multiplexer. (This isn't possible for kqueue.)
++ * multiplexer.
+ */
+ conn->altsock = actx->timerfd;
-+#endif
+
+ actx->step = OAUTH_STEP_WAIT_INTERVAL;
+ actx->running = 1;
@@ src/interfaces/libpq/fe-auth-oauth.c (new)
+{
+ struct json_ctx *ctx = state;
+
++ /* Only top-level keys are considered. */
+ if (ctx->nested == 1)
+ {
+ if (strcmp(name, ERROR_STATUS_FIELD) == 0)
@@ src/interfaces/libpq/fe-auth-oauth.c (new)
+
+ if (ctx->target_field)
+ {
-+ Assert(ctx->nested == 1);
++ if (ctx->nested != 1)
++ {
++ /*
++ * ctx->target_field should not have been set for nested keys.
++ * Assert and don't continue any further for production builds.
++ */
++ Assert(false);
++ oauth_json_set_error(ctx, /* don't bother translating */
++ "internal error: target scalar found at nesting level %d during OAUTHBEARER parsing",
++ ctx->nested);
++ return JSON_SEM_ACTION_FAILED;
++ }
+
+ /*
+ * We don't allow duplicate field names; error out if the target has
@@ src/interfaces/libpq/libpq-fe.h: extern int PQenv2encoding(void);
+{
+ const char *verification_uri; /* verification URI to visit */
+ const char *user_code; /* user code to enter */
++ const char *verification_uri_complete; /* optional combination of URI and
++ * code, or NULL */
++ int expires_in; /* seconds until user code expires */
+} PGpromptOAuthDevice;
+
+/* for PGoauthBearerRequest.async() */
@@ src/test/modules/oauth_validator/oauth_hook_client.c (new)
+ printf(" --expected-uri URI fail if received configuration link does not match URI\n");
+ printf(" --misbehave=MODE have the hook fail required postconditions\n"
+ " (MODEs: no-hook, fail-async, no-token, no-socket)\n");
-+ printf(" --no-hook don't install OAuth hooks (connection will fail)\n");
++ printf(" --no-hook don't install OAuth hooks\n");
+ printf(" --hang-forever don't ever return a token (combine with connect_timeout)\n");
+ printf(" --token TOKEN use the provided TOKEN value\n");
++ printf(" --stress-async busy-loop on PQconnectPoll rather than polling\n");
+}
+
+/* --options */
+static bool no_hook = false;
+static bool hang_forever = false;
++static bool stress_async = false;
+static const char *expected_uri = NULL;
+static const char *expected_scope = NULL;
+static const char *misbehave_mode = NULL;
@@ src/test/modules/oauth_validator/oauth_hook_client.c (new)
+ {"token", required_argument, NULL, 1003},
+ {"hang-forever", no_argument, NULL, 1004},
+ {"misbehave", required_argument, NULL, 1005},
++ {"stress-async", no_argument, NULL, 1006},
+ {0}
+ };
+
@@ src/test/modules/oauth_validator/oauth_hook_client.c (new)
+ misbehave_mode = optarg;
+ break;
+
++ case 1006: /* --stress-async */
++ stress_async = true;
++ break;
++
+ default:
+ usage(argv);
+ return 1;
@@ src/test/modules/oauth_validator/oauth_hook_client.c (new)
+ PQsetAuthDataHook(handle_auth_data);
+
+ /* Connect. (All the actual work is in the hook.) */
-+ conn = PQconnectdb(conninfo);
++ if (stress_async)
++ {
++ /*
++ * Perform an asynchronous connection, busy-looping on PQconnectPoll()
++ * without actually waiting on socket events. This stresses code paths
++ * that rely on asynchronous work to be done before continuing with
++ * the next step in the flow.
++ */
++ PostgresPollingStatusType res;
++
++ conn = PQconnectStart(conninfo);
++
++ do
++ {
++ res = PQconnectPoll(conn);
++ } while (res != PGRES_POLLING_FAILED && res != PGRES_POLLING_OK);
++ }
++ else
++ {
++ /* Perform a standard synchronous connection. */
++ conn = PQconnectdb(conninfo);
++ }
++
+ if (PQstatus(conn) != CONNECTION_OK)
+ {
-+ fprintf(stderr, "Connection to database failed: %s\n",
++ fprintf(stderr, "connection to database failed: %s\n",
+ PQerrorMessage(conn));
+ PQfinish(conn);
+ return 1;
@@ src/test/modules/oauth_validator/t/001_server.pl (new)
+ qr/failed to obtain access token: mutual TLS required for client \(invalid_client\)/
+);
+
++# Stress test: make sure our builtin flow operates correctly even if the client
++# application isn't respecting PGRES_POLLING_READING/WRITING signals returned
++# from PQconnectPoll().
++$base_connstr =
++ "$common_connstr port=" . $node->port . " host=" . $node->host;
++my @cmd = (
++ "oauth_hook_client", "--no-hook", "--stress-async",
++ connstr(stage => 'all', retries => 1, interval => 1));
++
++note "running '" . join("' '", @cmd) . "'";
++my ($stdout, $stderr) = run_command(\@cmd);
++
++like($stdout, qr/connection succeeded/, "stress-async: stdout matches");
++unlike($stderr, qr/connection to database failed/, "stress-async: stderr matches");
++
+#
+# This section of tests reconfigures the validator module itself, rather than
+# the OAuth server.
@@ src/test/modules/oauth_validator/t/oauth_server.py (new)
+ "device_code": "postgres",
+ "user_code": "postgresuser",
+ self._uri_spelling: uri,
-+ "expires-in": 5,
++ "expires_in": 5,
+ **self._response_padding,
+ }
+
@@ 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 in shutdown",
++ elog(PANIC, "oauth_validator: private state cookie changed to %p in shutdown",
+ state->private_data);
+}
+
2: 483129c1ca9 < -: ----------- fixup! Add OAUTHBEARER SASL mechanism
3: 75d98784ded < -: ----------- fixup! Add OAUTHBEARER SASL mechanism
4: fd60ceb4c84 < -: ----------- fixup! Add OAUTHBEARER SASL mechanism
5: 595362ef2c1 < -: ----------- fixup! Add OAUTHBEARER SASL mechanism
6: f73c042adc9 < -: ----------- fixup! Add OAUTHBEARER SASL mechanism
7: 298839b69f0 < -: ----------- fixup! Add OAUTHBEARER SASL mechanism
8: 1cf48a8f835 < -: ----------- fixup! Add OAUTHBEARER SASL mechanism
9: 27135876559 < -: ----------- fixup! Add OAUTHBEARER SASL mechanism
-: ----------- > 2: 9171989a75e fixup! Add OAUTHBEARER SASL mechanism
-: ----------- > 3: 1bd03e1de10 fixup! Add OAUTHBEARER SASL mechanism
-: ----------- > 4: 0929bfbc5fc fixup! Add OAUTHBEARER SASL mechanism
-: ----------- > 5: be882ef6eae fixup! Add OAUTHBEARER SASL mechanism
-: ----------- > 6: 954341052b4 fixup! Add OAUTHBEARER SASL mechanism
10: d8c1f298080 = 7: d88a2938e7e XXX fix libcurl link error
11: dbf305d0489 ! 8: 44e5cbc8ad1 DO NOT MERGE: Add pytest suite for OAuth
@@ src/test/python/client/test_oauth.py (new)
+ id="invalid request without description",
+ ),
+ pytest.param(
-+ (400, {"error": "invalid_request", "padding": "x" * 1024 * 1024}),
++ (400, {"error": "invalid_request", "padding": "x" * 256 * 1024}),
+ r"failed to obtain device authorization: response is too large",
+ id="gigantic authz response",
+ ),
@@ src/test/python/client/test_oauth.py (new)
+ id="access denied without description",
+ ),
+ pytest.param(
-+ (400, {"error": "access_denied", "padding": "x" * 1024 * 1024}),
++ (400, {"error": "access_denied", "padding": "x" * 256 * 1024}),
+ r"failed to obtain access token: response is too large",
+ id="gigantic token response",
+ ),
@@ src/test/python/client/test_oauth.py (new)
+ "urn:ietf:params:oauth:grant-type:device_code"
+ ],
+ "device_authorization_endpoint": "https://256.256.256.256/dev",
-+ "filler": "x" * 1024 * 1024,
++ "filler": "x" * 256 * 1024,
+ },
+ ),
+ r"failed to fetch OpenID discovery document: response is too large",
@@ src/test/python/server/oauthtest.c (new)
+
+static void test_startup(ValidatorModuleState *state);
+static void test_shutdown(ValidatorModuleState *state);
-+static ValidatorModuleResult *test_validate(ValidatorModuleState *state,
-+ const char *token,
-+ const char *role);
++static bool test_validate(const ValidatorModuleState *state,
++ const char *token,
++ const char *role,
++ ValidatorModuleResult *result);
+
+static const OAuthValidatorCallbacks callbacks = {
++ PG_OAUTH_VALIDATOR_MAGIC,
++
+ .startup_cb = test_startup,
+ .shutdown_cb = test_shutdown,
+ .validate_cb = test_validate,
@@ src/test/python/server/oauthtest.c (new)
+{
+}
+
-+static ValidatorModuleResult *
-+test_validate(ValidatorModuleState *state, const char *token, const char *role)
++static bool
++test_validate(const ValidatorModuleState *state,
++ const char *token, const char *role,
++ ValidatorModuleResult *res)
+{
-+ ValidatorModuleResult *res;
-+
-+ res = palloc0(sizeof(ValidatorModuleResult)); /* TODO: palloc context? */
-+
+ if (reflect_role)
+ {
+ res->authorized = true;
-+ res->authn_id = pstrdup(role); /* TODO: constify? */
++ res->authn_id = pstrdup(role);
+ }
+ else
+ {
+ if (*expected_bearer && strcmp(token, expected_bearer) == 0)
+ res->authorized = true;
+ if (set_authn_id)
-+ res->authn_id = authn_id;
++ res->authn_id = pstrdup(authn_id);
+ }
+
-+ return res;
++ return true;
+}
## src/test/python/server/test_oauth.py (new) ##