since-v7.diff.txt

text/plain

Filename: since-v7.diff.txt
Type: text/plain
Part: 0
Message: Re: [PoC] Federated Authn/z with OAUTHBEARER
-:  ----------- > 1:  5f87f11b18e Add minor-version counterpart to (PG_)MAJORVERSION
1:  942ad5391e2 ! 2:  4c9cc7f69af oauth: Move the builtin flow into a separate module
    @@ Commit message
         the search path came from a different build of Postgres.
     
         This ABI is considered "private". The module has no SONAME or version
    -    symlinks, and it's named libpq-oauth-<major>.so to avoid mixing and
    -    matching across major Postgres versions. (Future improvements may
    -    promote this "OAuth flow plugin" to a first-class concept, at which
    -    point we would need a public API to replace this anyway.)
    +    symlinks, and it's named libpq-oauth-<major>-<minor>.so to avoid mixing
    +    and matching across Postgres versions, in case internal struct order
    +    needs to change. (Future improvements may promote this "OAuth flow
    +    plugin" to a first-class concept, at which point we would need a public
    +    API to replace this anyway.)
     
         Additionally, NLS support for error messages in b3f0be788a was
         incomplete, because the new error macros weren't being scanned by
    @@ src/interfaces/libpq-oauth/Makefile (new)
     +
     +# This is an internal module; we don't want an SONAME and therefore do not set
     +# SO_MAJOR_VERSION.
    -+NAME = pq-oauth-$(MAJORVERSION)
    ++NAME = pq-oauth-$(MAJORVERSION)-$(MINORVERSION)
     +
    -+# Force the name "libpq-oauth" for both the static and shared libraries.
    ++# Force the name "libpq-oauth" for both the static and shared libraries. The
    ++# staticlib doesn't need version information in its name.
     +override shlib := lib$(NAME)$(DLSUFFIX)
    -+override stlib := lib$(NAME).a
    ++override stlib := libpq-oauth.a
     +
     +override CPPFLAGS := -I$(libpq_srcdir) -I$(top_builddir)/src/port $(LIBCURL_CPPFLAGS) $(CPPFLAGS)
     +
     +OBJS = \
    -+	$(WIN32RES) \
    -+	oauth-curl.o
    ++	$(WIN32RES)
    ++
    ++OBJS_STATIC = oauth-curl.o
     +
     +# The shared library needs additional glue symbols.
    -+$(shlib): OBJS += oauth-utils.o
    -+$(shlib): oauth-utils.o
    ++OBJS_SHLIB = \
    ++	oauth-curl_shlib.o \
    ++	oauth-utils.o \
    ++
    ++oauth-utils.o: override CPPFLAGS += -DUSE_DYNAMIC_OAUTH
    ++oauth-curl_shlib.o: override CPPFLAGS_SHLIB += -DUSE_DYNAMIC_OAUTH
    ++
    ++# Add shlib-/stlib-specific objects.
    ++$(shlib): override OBJS += $(OBJS_SHLIB)
    ++$(shlib): $(OBJS_SHLIB)
    ++
    ++$(stlib): override OBJS += $(OBJS_STATIC)
    ++$(stlib): $(OBJS_STATIC)
     +
     +SHLIB_LINK_INTERNAL = $(libpq_pgport_shlib)
     +SHLIB_LINK = $(LIBCURL_LDFLAGS) $(LIBCURL_LDLIBS)
    @@ src/interfaces/libpq-oauth/Makefile (new)
     +# Shared library stuff
     +include $(top_srcdir)/src/Makefile.shlib
     +
    ++# Use src/common/Makefile's trick for tracking dependencies of shlib-specific
    ++# objects.
    ++%_shlib.o: %.c %.o
    ++	$(CC) $(CFLAGS) $(CFLAGS_SL) $(CPPFLAGS) $(CPPFLAGS_SHLIB) -c $< -o $@
    ++
     +# Ignore the standard rules for SONAME-less installation; we want both the
     +# static and shared libraries to go into libdir.
     +install: all installdirs $(stlib) $(shlib)
    @@ src/interfaces/libpq-oauth/Makefile (new)
     +	rm -f '$(DESTDIR)$(libdir)/$(shlib)'
     +
     +clean distclean: clean-lib
    -+	rm -f $(OBJS) oauth-utils.o
    ++	rm -f $(OBJS) $(OBJS_STATIC) $(OBJS_SHLIB)
     
      ## src/interfaces/libpq-oauth/README (new) ##
     @@
    @@ src/interfaces/libpq-oauth/README (new)
     += Load-Time ABI =
     +
     +This module ABI is an internal implementation detail, so it's subject to change
    -+across major releases; the name of the module (libpq-oauth-MAJOR) reflects this.
    ++across releases; the name of the module (libpq-oauth-MAJOR-MINOR) reflects this.
     +The module exports the following symbols:
     +
     +- PostgresPollingStatusType pg_fe_run_oauth_flow(PGconn *conn);
    @@ src/interfaces/libpq-oauth/README (new)
     +
     +At the moment, pg_fe_run_oauth_flow() relies on libpq's pg_g_threadlock and
     +libpq_gettext(), which must be injected by libpq using this initialization
    -+function before the flow is run. It also relies on libpq to expose
    -+conn->errorMessage, via the errmsg_impl.
    ++function before the flow is run.
     +
    -+This dependency injection is done to ensure that the module ABI is decoupled
    -+from the internals of `struct pg_conn`. This way we can safely search the
    -+standard dlopen() paths (e.g. RPATH, LD_LIBRARY_PATH, the SO cache) for an
    -+implementation module to use, even if that module wasn't compiled at the same
    -+time as libpq.
    ++It also relies on libpq to expose conn->errorMessage, via the errmsg_impl. This
    ++is done to decouple the module ABI from the offset of errorMessage, which can
    ++change positions depending on configure-time options. This way we can safely
    ++search the standard dlopen() paths (e.g. RPATH, LD_LIBRARY_PATH, the SO cache)
    ++for an implementation module to use, even if that module wasn't compiled at the
    ++same time as libpq.
     +
     += Static Build =
     +
     +The static library libpq.a does not perform any dynamic loading. If the builtin
    -+flow is enabled, the application is expected to link against libpq-oauth-*.a
    ++flow is enabled, the application is expected to link against libpq-oauth.a
     +directly to provide the necessary symbols.
     
      ## src/interfaces/libpq-oauth/exports.txt (new) ##
    @@ src/interfaces/libpq-oauth/meson.build (new)
     +libpq_oauth_so_sources = files(
     +  'oauth-utils.c',
     +)
    ++libpq_oauth_so_c_args = ['-DUSE_DYNAMIC_OAUTH']
     +
     +export_file = custom_target('libpq-oauth.exports',
     +  kwargs: gen_export_kwargs,
    @@ src/interfaces/libpq-oauth/meson.build (new)
     +# port needs to be in include path due to pthread-win32.h
     +libpq_oauth_inc = include_directories('.', '../libpq', '../../port')
     +
    -+# This is an internal module; we don't want an SONAME and therefore do not set
    -+# SO_MAJOR_VERSION.
    -+libpq_oauth_name = 'libpq-oauth-@0@'.format(pg_version_major)
    -+
    -+libpq_oauth_st = static_library(libpq_oauth_name,
    ++libpq_oauth_st = static_library('libpq-oauth',
     +  libpq_oauth_sources,
     +  include_directories: [libpq_oauth_inc, postgres_inc],
     +  c_pch: pch_postgres_fe_h,
    @@ src/interfaces/libpq-oauth/meson.build (new)
     +  kwargs: default_lib_args,
     +)
     +
    ++# This is an internal module; we don't want an SONAME and therefore do not set
    ++# SO_MAJOR_VERSION.
    ++libpq_oauth_name = 'libpq-oauth-@0@-@1@'.format(pg_version_major, pg_version_minor)
    ++
     +libpq_oauth_so = shared_module(libpq_oauth_name,
     +  libpq_oauth_sources + libpq_oauth_so_sources,
     +  include_directories: [libpq_oauth_inc, postgres_inc],
    ++  c_args: libpq_so_c_args,
     +  c_pch: pch_postgres_fe_h,
     +  dependencies: [frontend_shlib_code, libpq, libpq_oauth_deps],
     +  link_depends: export_file,
    @@ src/interfaces/libpq/fe-auth-oauth-curl.c => src/interfaces/libpq-oauth/oauth-cu
     -#include "libpq-int.h"
      #include "mb/pg_wchar.h"
     +#include "oauth-curl.h"
    ++#ifdef USE_DYNAMIC_OAUTH
     +#include "oauth-utils.h"
    ++#endif
      
      /*
       * It's generally prudent to set a maximum response size to buffer in memory,
    @@ src/interfaces/libpq-oauth/oauth-curl.c: prompt_user(struct async_ctx *actx, PGc
      
      	if (!res)
      	{
    +@@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow_impl(PGconn *conn)
    + {
    + 	fe_oauth_state *state = conn->sasl_state;
    + 	struct async_ctx *actx;
    ++	PQExpBuffer errbuf;
    + 
    + 	if (!initialize_curl(conn))
    + 		return PGRES_POLLING_FAILED;
    +@@ src/interfaces/libpq-oauth/oauth-curl.c: pg_fe_run_oauth_flow_impl(PGconn *conn)
    + 
    + error_return:
    + 
    ++	/*
    ++	 * For the dynamic module build, we can't safely rely on the offset of
    ++	 * conn->errorMessage, since it depends on build options like USE_SSL et
    ++	 * al. libpq gives us a translator function instead.
    ++	 */
    ++#ifdef USE_DYNAMIC_OAUTH
    ++	errbuf = conn_errorMessage(conn);
    ++#else
    ++	errbuf = &conn->errorMessage;
    ++#endif
    ++
    + 	/*
    + 	 * Assemble the three parts of our error: context, body, and detail. See
    + 	 * also the documentation for struct async_ctx.
    + 	 */
    + 	if (actx->errctx)
    + 	{
    +-		appendPQExpBufferStr(&conn->errorMessage,
    +-							 libpq_gettext(actx->errctx));
    +-		appendPQExpBufferStr(&conn->errorMessage, ": ");
    ++		appendPQExpBufferStr(errbuf, libpq_gettext(actx->errctx));
    ++		appendPQExpBufferStr(errbuf, ": ");
    + 	}
    + 
    + 	if (PQExpBufferDataBroken(actx->errbuf))
    +-		appendPQExpBufferStr(&conn->errorMessage,
    +-							 libpq_gettext("out of memory"));
    ++		appendPQExpBufferStr(errbuf, libpq_gettext("out of memory"));
    + 	else
    +-		appendPQExpBufferStr(&conn->errorMessage, actx->errbuf.data);
    ++		appendPQExpBufferStr(errbuf, actx->errbuf.data);
    + 
    + 	if (actx->curl_err[0])
    + 	{
    + 		size_t		len;
    + 
    +-		appendPQExpBuffer(&conn->errorMessage,
    +-						  " (libcurl: %s)", actx->curl_err);
    ++		appendPQExpBuffer(errbuf, " (libcurl: %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')
    ++		len = errbuf->len;
    ++		if (len >= 2 && errbuf->data[len - 2] == '\n')
    + 		{
    +-			conn->errorMessage.data[len - 2] = ')';
    +-			conn->errorMessage.data[len - 1] = '\0';
    +-			conn->errorMessage.len--;
    ++			errbuf->data[len - 2] = ')';
    ++			errbuf->data[len - 1] = '\0';
    ++			errbuf->len--;
    + 		}
    + 	}
    + 
    +-	appendPQExpBufferChar(&conn->errorMessage, '\n');
    ++	appendPQExpBufferChar(errbuf, '\n');
    + 
    + 	return PGRES_POLLING_FAILED;
    + }
     
      ## src/interfaces/libpq-oauth/oauth-curl.h (new) ##
     @@
    @@ src/interfaces/libpq-oauth/oauth-utils.c (new)
     +#include "libpq-int.h"
     +#include "oauth-utils.h"
     +
    ++#ifndef USE_DYNAMIC_OAUTH
    ++#error oauth-utils.c is not supported in static builds
    ++#endif
    ++
     +static libpq_gettext_func libpq_gettext_impl;
    -+static conn_errorMessage_func conn_errorMessage;
     +
     +pgthreadlock_t pg_g_threadlock;
    ++conn_errorMessage_func conn_errorMessage;
     +
     +/*-
     + * Initializes libpq-oauth by setting necessary callbacks.
    @@ src/interfaces/libpq-oauth/oauth-utils.h (new)
     +										 libpq_gettext_func gettext_impl,
     +										 conn_errorMessage_func errmsg_impl);
     +
    ++/* Callback to safely obtain conn->errorMessage from a PGconn. */
    ++extern conn_errorMessage_func conn_errorMessage;
    ++
     +/* Duplicated APIs, copied from libpq. */
     +extern void libpq_append_conn_error(PGconn *conn, const char *fmt,...) pg_attribute_printf(2, 3);
     +extern bool oauth_unsafe_debugging_enabled(void);
    @@ src/interfaces/libpq/Makefile: ifeq ($(with_ssl),openssl)
     +# libpq.so doesn't link against libcurl, but libpq.a needs libpq-oauth, and
     +# libpq-oauth needs libcurl. Put both into *.private.
     +PKG_CONFIG_REQUIRES_PRIVATE += libcurl
    -+%.pc: override SHLIB_LINK_INTERNAL += -lpq-oauth-$(MAJORVERSION)
    ++%.pc: override SHLIB_LINK_INTERNAL += -lpq-oauth
     +endif
     +
      all: all-lib libpq-refs-stamp
    @@ src/interfaces/libpq/fe-auth-oauth.c: cleanup_user_oauth_flow(PGconn *conn)
     +	 */
     +	const char *const module_name =
     +#if defined(__darwin__)
    -+		LIBDIR "/libpq-oauth-" PG_MAJORVERSION DLSUFFIX;
    ++		LIBDIR "/libpq-oauth-" PG_MAJORVERSION "-" PG_MINORVERSION DLSUFFIX;
     +#else
    -+		"libpq-oauth-" PG_MAJORVERSION DLSUFFIX;
    ++		"libpq-oauth-" PG_MAJORVERSION "-" PG_MINORVERSION DLSUFFIX;
     +#endif
     +
     +	state->builtin_flow = dlopen(module_name, RTLD_NOW | RTLD_LOCAL);
    @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth
     -
     -#else
     -		libpq_append_conn_error(conn, "no custom OAuth flows are available, and libpq was not built with libcurl support");
    -+		libpq_append_conn_error(conn, "no custom OAuth flows are available, and the builtin flow is not installed");
    ++		libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)");
      		goto fail;
     -
     -#endif
    @@ src/interfaces/libpq/meson.build: libpq = declare_dependency(
     +  # libpq-oauth needs libcurl. Put both into *.private.
     +  private_deps += [
     +    libpq_oauth_deps,
    -+    '-lpq-oauth-@0@'.format(pg_version_major),
    ++    '-lpq-oauth',
     +  ]
     +endif
     +
    @@ src/test/modules/oauth_validator/t/002_client.pl: if ($ENV{with_libcurl} ne 'yes
      		flags => ["--no-hook"],
      		expected_stderr =>
     -		  qr/no custom OAuth flows are available, and libpq was not built with libcurl support/
    -+		  qr/no custom OAuth flows are available, and the builtin flow is not installed/
    ++		  qr/no OAuth flows are available \(try installing the libpq-oauth package\)/
      	);
      }