since-v8.diff.txt
text/plain
Filename: since-v8.diff.txt
Type: text/plain
Part: 0
1: 6434d90105 = 1: 9c6a340119 common/jsonapi: support FRONTEND clients
2: 13ddf2b6b3 ! 2: 8072d0416e libpq: add OAUTHBEARER SASL mechanism
@@ Commit message
development headers. Pass `curl` or `iddawc` to --with-oauth/-Doauth
during configuration.
+ Thomas Munro wrote the kqueue() implementation for oauth-curl; thanks!
+
Several TODOs:
- don't retry forever if the server won't accept our token
- perform several sanity checks on the OAuth issuer's responses
@@ meson.build: if meson.version().version_compare('>=0.57')
'plperl': perl_dep,
## meson_options.txt ##
-@@ meson_options.txt: option('lz4', type : 'feature', value: 'auto',
+@@ meson_options.txt: option('lz4', type: 'feature', value: 'auto',
option('nls', type: 'feature', value: 'auto',
- description: 'native language support')
+ description: 'Native language support')
+option('oauth', type : 'combo', choices : ['auto', 'none', 'curl', 'iddawc'],
+ value: 'auto',
+ description: 'use LIB for OAuth 2.0 support (curl, iddawc)')
+
- option('pam', type : 'feature', value: 'auto',
- description: 'build with PAM support')
+ option('pam', type: 'feature', value: 'auto',
+ description: 'PAM support')
## src/Makefile.global.in ##
@@ src/Makefile.global.in: with_ldap = @with_ldap@
## src/common/meson.build ##
@@ src/common/meson.build: common_sources_frontend_static += files(
- # For the server build of pgcommon, depend on lwlocknames_h, because at least
- # cryptohash_openssl.c, hmac_openssl.c depend on it. That's arguably a
+ # least cryptohash_openssl.c, hmac_openssl.c depend on it.
+ # controldata_utils.c depends on wait_event_types_h. That's arguably a
# layering violation, but ...
+#
+# XXX Frontend builds need libpq's pqexpbuffer.h, so adjust the include paths
@@ src/common/meson.build: common_sources_frontend_static += files(
pgcommon_variants = {
'_srv': internal_lib_args + {
+ 'include_directories': include_directories('.'),
- 'sources': common_sources + [lwlocknames_h],
+ 'sources': common_sources + [lwlocknames_h] + [wait_event_types_h],
'dependencies': [backend_common_code],
},
'': default_lib_args + {
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+
+#include <curl/curl.h>
+#include <math.h>
++#ifdef HAVE_SYS_EPOLL_H
+#include <sys/epoll.h>
+#include <sys/timerfd.h>
++#endif
++#ifdef HAVE_SYS_EVENT_H
++#include <sys/event.h>
++#endif
+#include <unistd.h>
+
+#include "common/jsonapi.h"
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+#include "libpq-int.h"
+#include "mb/pg_wchar.h"
+
++#ifdef HAVE_SYS_EVENT_H
++/* macOS doesn't define the time unit macros, but uses milliseconds by default. */
++#ifndef NOTE_MSECONDS
++#define NOTE_MSECONDS 0
++#endif
++#endif
++
+/*
+ * Parsed JSON Representations
+ *
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+{
+ OAuthStep step; /* where are we in the flow? */
+
++#ifdef HAVE_SYS_EPOLL_H
+ int timerfd; /* a timerfd for signaling async timeouts */
++#endif
+ pgsocket mux; /* the multiplexer socket containing all descriptors
+ tracked by cURL, plus the timerfd */
+ CURLM *curlm; /* top-level multi handle for cURL operations */
@@ 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)
+ * Sets up the actx->mux, which is the altsock that PQconnectPoll clients will
+ * select() on instead of the Postgres socket during OAuth negotiation.
+ *
-+ * This is just an epoll set abstracting multiple other descriptors. A timerfd
-+ * is always part of the set; it's just disabled when we're not using it.
++ * 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.
+ */
+static bool
+setup_multiplexer(struct async_ctx *actx)
+{
++#ifdef HAVE_SYS_EPOLL_H
+ struct epoll_event ev = {.events = EPOLLIN};
+
+ actx->mux = epoll_create1(EPOLL_CLOEXEC);
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ }
+
+ return true;
++#endif
++#ifdef HAVE_SYS_EVENT_H
++ actx->mux = kqueue();
++ if (actx->mux < 0)
++ {
++ actx_error(actx, "failed to create kqueue: %m");
++ return false;
++ }
++
++ return true;
++#endif
++
++ actx_error(actx, "here's a nickel kid, get yourself a better computer");
++ return false;
+}
+
+/*
-+ * Adds and removes sockets from the multiplexer epoll set, as directed by the
++ * Adds and removes sockets from the multiplexer set, as directed by the
+ * cURL multi handle.
+ */
+static int
+register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx,
+ void *socketp)
+{
++#ifdef HAVE_SYS_EPOLL_H
+ struct async_ctx *actx = ctx;
+ struct epoll_event ev = {0};
+ int res;
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+
+ return -1;
+ }
++#endif
++#ifdef HAVE_SYS_EVENT_H
++ struct async_ctx *actx = ctx;
++ struct kevent ev[2] = {{0}};
++ struct kevent ev_out[2];
++ struct timespec timeout = {0};
++ int nev = 0;
++ int res;
++
++ switch (what)
++ {
++ case CURL_POLL_IN:
++ EV_SET(&ev[nev], socket, EVFILT_READ, EV_ADD, 0, 0, 0);
++ nev++;
++ break;
++
++ case CURL_POLL_OUT:
++ EV_SET(&ev[nev], socket, EVFILT_WRITE, EV_ADD, 0, 0, 0);
++ nev++;
++ break;
++
++ case CURL_POLL_INOUT:
++ EV_SET(&ev[nev], socket, EVFILT_READ, EV_ADD, 0, 0, 0);
++ nev++;
++ EV_SET(&ev[nev], socket, EVFILT_WRITE, EV_ADD, 0, 0, 0);
++ nev++;
++ break;
++
++ case CURL_POLL_REMOVE:
++ /*
++ * We don't know which of these is currently registered, perhaps
++ * both, so we try to remove both. This means we need to tolerate
++ * ENOENT below.
++ */
++ EV_SET(&ev[nev], socket, EVFILT_READ, EV_DELETE, 0, 0, 0);
++ nev++;
++ EV_SET(&ev[nev], socket, EVFILT_WRITE, EV_DELETE, 0, 0, 0);
++ nev++;
++ break;
++
++ default:
++ actx_error(actx, "unknown cURL socket operation (%d)", what);
++ return -1;
++ }
++
++ res = kevent(actx->mux, ev, nev, ev_out, lengthof(ev_out), &timeout);
++ if (res < 0)
++ {
++ actx_error(actx, "could not modify kqueue: %m");
++ return -1;
++ }
++
++ /*
++ * We can't use the simple errno version of kevent, because we need to skip
++ * over ENOENT while still allowing a second change to be processed. So we
++ * need a longer-form error checking loop.
++ */
++ for (int i = 0; i < res; ++i)
++ {
++ if (ev_out[i].flags & EV_ERROR && ev_out[i].data != ENOENT)
++ {
++ errno = ev_out[i].data;
++ switch (what)
++ {
++ case CURL_POLL_REMOVE:
++ actx_error(actx, "could not delete from kqueue: %m");
++ break;
++ default:
++ actx_error(actx, "could not add to kqueue: %m");
++ }
++ return -1;
++ }
++ }
++#endif
+
+ return 0;
+}
+
+/*
-+ * Adds or removes timeouts from the multiplexer epoll set, as directed by the
-+ * cURL multi handle. Rather than continually adding and removing the timerfd,
-+ * we keep it in the epoll set at all times and just disarm it when it's not
++ * 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.
+ */
+static int
+register_timer(CURLM *curlm, long timeout, void *ctx)
+{
++#if HAVE_SYS_EPOLL_H
+ struct async_ctx *actx = ctx;
+ struct itimerspec spec = {0};
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ actx_error(actx, "setting timerfd to %ld: %m", timeout);
+ return -1;
+ }
++#endif
++#ifdef HAVE_SYS_EVENT_H
++ struct async_ctx *actx = ctx;
++ struct kevent ev;
++
++ EV_SET(&ev, 1, EVFILT_TIMER, timeout < 0 ? EV_DELETE : EV_ADD,
++ NOTE_MSECONDS, timeout, 0);
++ if (kevent(actx->mux, &ev, 1, NULL, 0, NULL) < 0 && errno != ENOENT)
++ {
++ actx_error(actx, "setting kqueue timer to %ld: %m", timeout);
++ return -1;
++ }
++#endif
+
+ return 0;
+}
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ }
+
+ actx->mux = PGINVALID_SOCKET;
++#ifdef HAVE_SYS_EPOLL_H
+ actx->timerfd = -1;
++#endif
+
+ state->async_ctx = actx;
+
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ case OAUTH_STEP_TOKEN_REQUEST:
+ {
+ const struct token_error *err;
++#ifdef HAVE_SYS_EPOLL_H
+ struct itimerspec spec = {0};
++#endif
++#ifdef HAVE_SYS_EVENT_H
++ struct kevent ev = {0};
++#endif
+
+ if (!finish_token_request(actx, &tok))
+ goto error_return;
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ * Wait for the required interval before issuing the next request.
+ */
+ Assert(actx->authz.interval > 0);
++#ifdef HAVE_SYS_EPOLL_H
+ spec.it_value.tv_sec = actx->authz.interval;
+
+ if (timerfd_settime(actx->timerfd, 0 /* no flags */, &spec, NULL) < 0)
@@ src/interfaces/libpq/fe-auth-oauth-curl.c (new)
+ }
+
+ *altsock = actx->timerfd;
++#endif
++#ifdef HAVE_SYS_EVENT_H
++ // XXX: I guess this wants to be hidden in a routine
++ EV_SET(&ev, 1, EVFILT_TIMER, EV_ADD, NOTE_MSECONDS,
++ actx->authz.interval * 1000, 0);
++ if (kevent(actx->mux, &ev, 1, NULL, 0, NULL) < 0)
++ {
++ actx_error(actx, "failed to set kqueue timer: %m");
++ goto error_return;
++ }
++ // XXX: why did we change the altsock in the epoll version?
++#endif
+ actx->step = OAUTH_STEP_WAIT_INTERVAL;
+ break;
+ }
3: 0b0b0f2b33 ! 3: 07be9375aa backend: add OAUTHBEARER SASL mechanism
@@ Commit message
correctly is delegated entirely to the oauth_validator_command).
Several TODOs:
- - port to platforms other than "modern Linux"
+ - port to platforms other than "modern Linux/BSD"
- overhaul the communication with oauth_validator_command, which is
currently a bad hack on OpenPipeStream()
- implement more sanity checks on the OAUTHBEARER message format and
@@ src/backend/libpq/auth-oauth.c (new)
+static bool validate(Port *port, const char *auth, const char **logdetail);
+static bool run_validator_command(Port *port, const char *token);
+static bool check_exit(FILE **fh, const char *command);
-+static bool unset_cloexec(int fd);
++static bool set_cloexec(int fd);
+static bool username_ok_for_shell(const char *username);
+
+#define KVSEP 0x01
@@ src/backend/libpq/auth-oauth.c (new)
+ * MUST read all data off of the pipe before writing anything).
+ * TODO: port to Windows using _pipe().
+ */
-+ rc = pipe2(pipefd, O_CLOEXEC);
++ rc = pipe(pipefd);
+ if (rc < 0)
+ {
+ ereport(COMMERROR,
@@ src/backend/libpq/auth-oauth.c (new)
+ rfd = pipefd[0];
+ wfd = pipefd[1];
+
-+ /* Allow the read pipe be passed to the child. */
-+ if (!unset_cloexec(rfd))
++ if (!set_cloexec(wfd))
+ {
+ /* error message was already logged */
+ goto cleanup;
@@ src/backend/libpq/auth-oauth.c (new)
+ }
+
+ /* Execute the command. */
-+ fh = OpenPipeStream(command.data, "re");
-+ /* TODO: handle failures */
++ fh = OpenPipeStream(command.data, "r");
++ if (!fh)
++ {
++ ereport(COMMERROR,
++ (errcode_for_file_access(),
++ errmsg("opening pipe to OAuth validator: %m")));
++ goto cleanup;
++ }
+
+ /* We don't need the read end of the pipe anymore. */
+ close(rfd);
@@ src/backend/libpq/auth-oauth.c (new)
+}
+
+static bool
-+unset_cloexec(int fd)
++set_cloexec(int fd)
+{
+ int flags;
+ int rc;
@@ src/backend/libpq/auth-oauth.c (new)
+ return false;
+ }
+
-+ rc = fcntl(fd, F_SETFD, flags & ~FD_CLOEXEC);
++ rc = fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
+ if (rc < 0)
+ {
+ ereport(COMMERROR,
+ (errcode_for_file_access(),
-+ errmsg("could not unset FD_CLOEXEC for child pipe: %m")));
++ errmsg("could not set FD_CLOEXEC for child pipe: %m")));
+ return false;
+ }
+
@@ src/include/libpq/auth.h
+
extern PGDLLIMPORT char *pg_krb_server_keyfile;
extern PGDLLIMPORT bool pg_krb_caseins_users;
- extern PGDLLIMPORT bool pg_gss_accept_deleg;
-@@ src/include/libpq/auth.h: extern PGDLLIMPORT char *pg_krb_realm;
+ extern PGDLLIMPORT bool pg_gss_accept_delegation;
+@@ src/include/libpq/auth.h: extern PGDLLIMPORT bool pg_gss_accept_delegation;
extern void ClientAuthentication(Port *port);
extern void sendAuthRequest(Port *port, AuthRequest areq, const char *extradata,
int extralen);
4: 0e8ddadcbf ! 4: 71cedc6ff5 Add pytest suite for OAuth
@@ src/test/python/requirements.txt (new)
+construct~=2.10.61
+isort~=5.6
+# TODO: update to psycopg[c] 3.1
-+psycopg2~=2.8.6
++psycopg2~=2.9.6
+pytest~=7.3
+pytest-asyncio~=0.21.0
5: 65c319a6a3 ! 5: 9b02e14829 squash! Add pytest suite for OAuth
@@ Commit message
TODOs:
- The --tap-stream option to pytest-tap is slightly broken during test
failures (it suppresses error information), which impedes debugging.
- - Unsurprisingly, Windows and Mac builds fail on the Linux-specific
- backend changes.
+ - Unsurprisingly, Windows builds fail on the Linux-/BSD-specific backend
+ changes. 32-bit builds on Ubuntu fail during testing as well.
- pyca/cryptography is pinned at an old version. Since we use it for
testing and not security, this isn't a critical problem yet, but it's
not ideal. Newer versions require a Rust compiler to build, and while
@@ meson.build: foreach test_dir : tests
+ test_command = [
+ pytest,
+ # Avoid running these tests against an existing database.
-+ '--temp-instance', test_output / 'tmp_check',
++ '--temp-instance', test_output / 'data',
+
+ # FIXME pytest-tap's stream feature accidentally suppresses errors that
+ # are critical for debugging:
@@ src/test/python/meson.build (new)
@@
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
++pytest_env = {
++ 'with_oauth': oauth_library,
++
++ # Point to the default database; the tests will create their own databases as
++ # needed.
++ 'PGDATABASE': 'postgres',
++
++ # Avoid the need for a Rust compiler on platforms without prebuilt wheels for
++ # pyca/cryptography.
++ 'CRYPTOGRAPHY_DONT_BUILD_RUST': '1',
++}
++
++# Some modules (psycopg2) need OpenSSL at compile time; for platforms where we
++# might have multiple implementations installed (macOS+brew), try to use the
++# same one that libpq is using.
++if ssl.found()
++ pytest_incdir = ssl.get_variable(pkgconfig: 'includedir', default_value: '')
++ if pytest_incdir != ''
++ pytest_env += { 'CPPFLAGS': '-I@0@'.format(pytest_incdir) }
++ endif
++
++ pytest_libdir = ssl.get_variable(pkgconfig: 'libdir', default_value: '')
++ if pytest_libdir != ''
++ pytest_env += { 'LDFLAGS': '-L@0@'.format(pytest_libdir) }
++ endif
++endif
++
+tests += {
+ 'name': 'python',
+ 'sd': meson.current_source_dir(),
@@ src/test/python/meson.build (new)
+ './test_internals.py',
+ './test_pq3.py',
+ ],
-+ 'env': {
-+ 'with_oauth': oauth_library,
-+
-+ # Point to the default database; the tests will create their own databases
-+ # as needed.
-+ 'PGDATABASE': 'postgres',
-+
-+ # Avoid the need for a Rust compiler on platforms without prebuilt wheels
-+ # for pyca/cryptography.
-+ 'CRYPTOGRAPHY_DONT_BUILD_RUST': '1',
-+ },
++ 'env': pytest_env,
+ },
+}
@@ src/test/python/server/conftest.py: import pq3
+ cleanup_prior_instance(datadir)
+ subprocess.run(["pg_ctl", "-D", datadir, "init"], check=True)
+
++ # The CI looks for *.log files to upload, so the file name here isn't
++ # completely arbitrary.
++ log = os.path.join(datadir, "postmaster.log")
+ port = unused_tcp_port_factory()
-+ log = os.path.join(datadir, "logfile")
+
+ subprocess.run(
+ [
@@ src/test/python/server/conftest.py: import pq3
+ "-l",
+ log,
+ "-o",
-+ f"-c port={port} -c listen_addresses=localhost",
++ " ".join(
++ [
++ f"-c port={port}",
++ "-c listen_addresses=localhost",
++ "-c log_connections=on",
++ ]
++ ),
+ "start",
+ ],
+ check=True,
-: ---------- > 6: 7d179f7e53 XXX work around psycopg2 build failures