Skip to content

Commit b93213e

Browse files
jchampiodanielgustafsson
authored and
Commitfest Bot
committed
oauth: Move the builtin flow into a separate module
The additional packaging footprint of the OAuth Curl dependency, as well as the existence of libcurl in the address space even if OAuth isn't ever used by a client, has raised some concerns. Split off this dependency into a separate loadable module called libpq-oauth. When configured using --with-libcurl, libpq.so searches for this new module via dlopen(). End users may choose not to install the libpq-oauth module, in which case the default flow is disabled. For static applications using libpq.a, the libpq-oauth staticlib is a mandatory link-time dependency for --with-libcurl builds. libpq.pc has been updated accordingly. The default flow relies on some libpq internals. Some of these can be safely duplicated (such as the SIGPIPE handlers), but others need to be shared between libpq and libpq-oauth for thread-safety. To avoid exporting these internals to all libpq clients forever, these dependencies are instead injected from the libpq side via an initialization function. This also lets libpq communicate the offsets of PGconn struct members to libpq-oauth, so that we can function without crashing if the module on the search path came from a different build of Postgres. (A minor-version upgrade could swap the libpq-oauth module out from under a long-running libpq client before it does its first load of the OAuth flow.) 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 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.) Additionally, NLS support for error messages in b3f0be7 was incomplete, because the new error macros weren't being scanned by xgettext. Fix that now. Per request from Tom Lane and Bruce Momjian. Based on an initial patch by Daniel Gustafsson, who also contributed docs changes. The "bare" dlopen() concept came from Thomas Munro. Many many people reviewed the design and implementation; thank you! Co-authored-by: Daniel Gustafsson <[email protected]> Reviewed-by: Andres Freund <[email protected]> Reviewed-by: Christoph Berg <[email protected]> Reviewed-by: Jelte Fennema-Nio <[email protected]> Reviewed-by: Peter Eisentraut <[email protected]> Reviewed-by: Wolfgang Walther <[email protected]> Discussion: https://postgr.es/m/641687.1742360249%40sss.pgh.pa.us
1 parent 991407a commit b93213e

25 files changed

+1080
-140
lines changed

config/programs.m4

+16-1
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,20 @@ AC_DEFUN([PGAC_CHECK_LIBCURL],
286286
[
287287
AC_CHECK_HEADER(curl/curl.h, [],
288288
[AC_MSG_ERROR([header file <curl/curl.h> is required for --with-libcurl])])
289-
AC_CHECK_LIB(curl, curl_multi_init, [],
289+
AC_CHECK_LIB(curl, curl_multi_init, [
290+
AC_DEFINE([HAVE_LIBCURL], [1], [Define to 1 if you have the `curl' library (-lcurl).])
291+
AC_SUBST(LIBCURL_LDLIBS, -lcurl)
292+
],
290293
[AC_MSG_ERROR([library 'curl' does not provide curl_multi_init])])
291294
295+
pgac_save_CPPFLAGS=$CPPFLAGS
296+
pgac_save_LDFLAGS=$LDFLAGS
297+
pgac_save_LIBS=$LIBS
298+
299+
CPPFLAGS="$LIBCURL_CPPFLAGS $CPPFLAGS"
300+
LDFLAGS="$LIBCURL_LDFLAGS $LDFLAGS"
301+
LIBS="$LIBCURL_LDLIBS $LIBS"
302+
292303
# Check to see whether the current platform supports threadsafe Curl
293304
# initialization.
294305
AC_CACHE_CHECK([for curl_global_init thread safety], [pgac_cv__libcurl_threadsafe_init],
@@ -338,4 +349,8 @@ AC_DEFUN([PGAC_CHECK_LIBCURL],
338349
*** lookups. Rebuild libcurl with the AsynchDNS feature enabled in order
339350
*** to use it with libpq.])
340351
fi
352+
353+
CPPFLAGS=$pgac_save_CPPFLAGS
354+
LDFLAGS=$pgac_save_LDFLAGS
355+
LIBS=$pgac_save_LIBS
341356
])# PGAC_CHECK_LIBCURL

configure

+39-11
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,7 @@ UUID_LIBS
655655
LDAP_LIBS_BE
656656
LDAP_LIBS_FE
657657
with_ssl
658+
LIBCURL_LDLIBS
658659
PTHREAD_CFLAGS
659660
PTHREAD_LIBS
660661
PTHREAD_CC
@@ -711,6 +712,8 @@ with_libxml
711712
LIBNUMA_LIBS
712713
LIBNUMA_CFLAGS
713714
with_libnuma
715+
LIBCURL_LDFLAGS
716+
LIBCURL_CPPFLAGS
714717
LIBCURL_LIBS
715718
LIBCURL_CFLAGS
716719
with_libcurl
@@ -9053,19 +9056,27 @@ $as_echo "yes" >&6; }
90539056

90549057
fi
90559058

9056-
# We only care about -I, -D, and -L switches;
9057-
# note that -lcurl will be added by PGAC_CHECK_LIBCURL below.
9059+
# Curl's flags are kept separate from the standard CPPFLAGS/LDFLAGS. We use
9060+
# them only for libpq-oauth.
9061+
LIBCURL_CPPFLAGS=
9062+
LIBCURL_LDFLAGS=
9063+
9064+
# We only care about -I, -D, and -L switches. Note that -lcurl will be added
9065+
# to LIBCURL_LDLIBS by PGAC_CHECK_LIBCURL, below.
90589066
for pgac_option in $LIBCURL_CFLAGS; do
90599067
case $pgac_option in
9060-
-I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
9068+
-I*|-D*) LIBCURL_CPPFLAGS="$LIBCURL_CPPFLAGS $pgac_option";;
90619069
esac
90629070
done
90639071
for pgac_option in $LIBCURL_LIBS; do
90649072
case $pgac_option in
9065-
-L*) LDFLAGS="$LDFLAGS $pgac_option";;
9073+
-L*) LIBCURL_LDFLAGS="$LIBCURL_LDFLAGS $pgac_option";;
90669074
esac
90679075
done
90689076

9077+
9078+
9079+
90699080
# OAuth requires python for testing
90709081
if test "$with_python" != yes; then
90719082
{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: *** OAuth support tests require --with-python to run" >&5
@@ -12704,9 +12715,6 @@ fi
1270412715

1270512716
fi
1270612717

12707-
# XXX libcurl must link after libgssapi_krb5 on FreeBSD to avoid segfaults
12708-
# during gss_acquire_cred(). This is possibly related to Curl's Heimdal
12709-
# dependency on that platform?
1271012718
if test "$with_libcurl" = yes ; then
1271112719

1271212720
ac_fn_c_check_header_mongrel "$LINENO" "curl/curl.h" "ac_cv_header_curl_curl_h" "$ac_includes_default"
@@ -12754,17 +12762,26 @@ fi
1275412762
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_curl_curl_multi_init" >&5
1275512763
$as_echo "$ac_cv_lib_curl_curl_multi_init" >&6; }
1275612764
if test "x$ac_cv_lib_curl_curl_multi_init" = xyes; then :
12757-
cat >>confdefs.h <<_ACEOF
12758-
#define HAVE_LIBCURL 1
12759-
_ACEOF
1276012765

12761-
LIBS="-lcurl $LIBS"
12766+
12767+
$as_echo "#define HAVE_LIBCURL 1" >>confdefs.h
12768+
12769+
LIBCURL_LDLIBS=-lcurl
12770+
1276212771

1276312772
else
1276412773
as_fn_error $? "library 'curl' does not provide curl_multi_init" "$LINENO" 5
1276512774
fi
1276612775

1276712776

12777+
pgac_save_CPPFLAGS=$CPPFLAGS
12778+
pgac_save_LDFLAGS=$LDFLAGS
12779+
pgac_save_LIBS=$LIBS
12780+
12781+
CPPFLAGS="$LIBCURL_CPPFLAGS $CPPFLAGS"
12782+
LDFLAGS="$LIBCURL_LDFLAGS $LDFLAGS"
12783+
LIBS="$LIBCURL_LDLIBS $LIBS"
12784+
1276812785
# Check to see whether the current platform supports threadsafe Curl
1276912786
# initialization.
1277012787
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for curl_global_init thread safety" >&5
@@ -12868,6 +12885,10 @@ $as_echo "$pgac_cv__libcurl_async_dns" >&6; }
1286812885
*** to use it with libpq." "$LINENO" 5
1286912886
fi
1287012887

12888+
CPPFLAGS=$pgac_save_CPPFLAGS
12889+
LDFLAGS=$pgac_save_LDFLAGS
12890+
LIBS=$pgac_save_LIBS
12891+
1287112892
fi
1287212893

1287312894
if test "$with_gssapi" = yes ; then
@@ -14516,6 +14537,13 @@ done
1451614537

1451714538
fi
1451814539

14540+
if test "$with_libcurl" = yes ; then
14541+
# Error out early if this platform can't support libpq-oauth.
14542+
if test "$ac_cv_header_sys_event_h" != yes -a "$ac_cv_header_sys_epoll_h" != yes; then
14543+
as_fn_error $? "client OAuth is not supported on this platform" "$LINENO" 5
14544+
fi
14545+
fi
14546+
1451914547
##
1452014548
## Types, structures, compiler characteristics
1452114549
##

configure.ac

+19-7
Original file line numberDiff line numberDiff line change
@@ -1033,19 +1033,27 @@ if test "$with_libcurl" = yes ; then
10331033
# to explicitly set TLS 1.3 ciphersuites).
10341034
PKG_CHECK_MODULES(LIBCURL, [libcurl >= 7.61.0])
10351035

1036-
# We only care about -I, -D, and -L switches;
1037-
# note that -lcurl will be added by PGAC_CHECK_LIBCURL below.
1036+
# Curl's flags are kept separate from the standard CPPFLAGS/LDFLAGS. We use
1037+
# them only for libpq-oauth.
1038+
LIBCURL_CPPFLAGS=
1039+
LIBCURL_LDFLAGS=
1040+
1041+
# We only care about -I, -D, and -L switches. Note that -lcurl will be added
1042+
# to LIBCURL_LDLIBS by PGAC_CHECK_LIBCURL, below.
10381043
for pgac_option in $LIBCURL_CFLAGS; do
10391044
case $pgac_option in
1040-
-I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
1045+
-I*|-D*) LIBCURL_CPPFLAGS="$LIBCURL_CPPFLAGS $pgac_option";;
10411046
esac
10421047
done
10431048
for pgac_option in $LIBCURL_LIBS; do
10441049
case $pgac_option in
1045-
-L*) LDFLAGS="$LDFLAGS $pgac_option";;
1050+
-L*) LIBCURL_LDFLAGS="$LIBCURL_LDFLAGS $pgac_option";;
10461051
esac
10471052
done
10481053

1054+
AC_SUBST(LIBCURL_CPPFLAGS)
1055+
AC_SUBST(LIBCURL_LDFLAGS)
1056+
10491057
# OAuth requires python for testing
10501058
if test "$with_python" != yes; then
10511059
AC_MSG_WARN([*** OAuth support tests require --with-python to run])
@@ -1354,9 +1362,6 @@ failure. It is possible the compiler isn't looking in the proper directory.
13541362
Use --without-zlib to disable zlib support.])])
13551363
fi
13561364

1357-
# XXX libcurl must link after libgssapi_krb5 on FreeBSD to avoid segfaults
1358-
# during gss_acquire_cred(). This is possibly related to Curl's Heimdal
1359-
# dependency on that platform?
13601365
if test "$with_libcurl" = yes ; then
13611366
PGAC_CHECK_LIBCURL
13621367
fi
@@ -1654,6 +1659,13 @@ if test "$PORTNAME" = "win32" ; then
16541659
AC_CHECK_HEADERS(crtdefs.h)
16551660
fi
16561661

1662+
if test "$with_libcurl" = yes ; then
1663+
# Error out early if this platform can't support libpq-oauth.
1664+
if test "$ac_cv_header_sys_event_h" != yes -a "$ac_cv_header_sys_epoll_h" != yes; then
1665+
AC_MSG_ERROR([client-side OAuth is not supported on this platform])
1666+
fi
1667+
fi
1668+
16571669
##
16581670
## Types, structures, compiler characteristics
16591671
##

doc/src/sgml/installation.sgml

+8
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,14 @@
313313
</para>
314314
</listitem>
315315

316+
<listitem>
317+
<para>
318+
You need <productname>Curl</productname> to build an optional module
319+
which implements the <link linkend="libpq-oauth">OAuth Device
320+
Authorization flow</link> for client applications.
321+
</para>
322+
</listitem>
323+
316324
<listitem>
317325
<para>
318326
You need <productname>LZ4</productname>, if you want to support

doc/src/sgml/libpq.sgml

+22-8
Original file line numberDiff line numberDiff line change
@@ -10226,15 +10226,20 @@ void PQinitSSL(int do_ssl);
1022610226
<title>OAuth Support</title>
1022710227

1022810228
<para>
10229-
libpq implements support for the OAuth v2 Device Authorization client flow,
10229+
<application>libpq</application> implements support for the OAuth v2 Device Authorization client flow,
1023010230
documented in
1023110231
<ulink url="https://datatracker.ietf.org/doc/html/rfc8628">RFC 8628</ulink>,
10232-
which it will attempt to use by default if the server
10232+
as an optional module. See the <link linkend="configure-option-with-libcurl">
10233+
installation documentation</link> for information on how to enable support
10234+
for Device Authorization as a builtin flow.
10235+
</para>
10236+
<para>
10237+
When support is enabled and the optional module installed, <application>libpq</application>
10238+
will use the builtin flow by default if the server
1023310239
<link linkend="auth-oauth">requests a bearer token</link> during
1023410240
authentication. This flow can be utilized even if the system running the
1023510241
client application does not have a usable web browser, for example when
10236-
running a client via <application>SSH</application>. Client applications may implement their own flows
10237-
instead; see <xref linkend="libpq-oauth-authdata-hooks"/>.
10242+
running a client via <acronym>SSH</acronym>.
1023810243
</para>
1023910244
<para>
1024010245
The builtin flow will, by default, print a URL to visit and a user code to
@@ -10251,6 +10256,11 @@ Visit https://example.com/device and enter the code: ABCD-EFGH
1025110256
they match expectations, before continuing. Permissions should not be given
1025210257
to untrusted third parties.
1025310258
</para>
10259+
<para>
10260+
Client applications may implement their own flows to customize interaction
10261+
and integration with applications. See <xref linkend="libpq-oauth-authdata-hooks"/>
10262+
for more information on how add a custom flow to <application>libpq</application>.
10263+
</para>
1025410264
<para>
1025510265
For an OAuth client flow to be usable, the connection string must at minimum
1025610266
contain <xref linkend="libpq-connect-oauth-issuer"/> and
@@ -10366,7 +10376,9 @@ typedef struct _PGpromptOAuthDevice
1036610376
</synopsis>
1036710377
</para>
1036810378
<para>
10369-
The OAuth Device Authorization flow included in <application>libpq</application>
10379+
The OAuth Device Authorization flow which
10380+
<link linkend="configure-option-with-libcurl">can be included</link>
10381+
in <application>libpq</application>
1037010382
requires the end user to visit a URL with a browser, then enter a code
1037110383
which permits <application>libpq</application> to connect to the server
1037210384
on their behalf. The default prompt simply prints the
@@ -10378,7 +10390,8 @@ typedef struct _PGpromptOAuthDevice
1037810390
This callback is only invoked during the builtin device
1037910391
authorization flow. If the application installs a
1038010392
<link linkend="libpq-oauth-authdata-oauth-bearer-token">custom OAuth
10381-
flow</link>, this authdata type will not be used.
10393+
flow</link>, or <application>libpq</application> was not built with
10394+
support for the builtin flow, this authdata type will not be used.
1038210395
</para>
1038310396
<para>
1038410397
If a non-NULL <structfield>verification_uri_complete</structfield> is
@@ -10400,8 +10413,9 @@ typedef struct _PGpromptOAuthDevice
1040010413
</term>
1040110414
<listitem>
1040210415
<para>
10403-
Replaces the entire OAuth flow with a custom implementation. The hook
10404-
should either directly return a Bearer token for the current
10416+
Adds a custom implementation of a flow, replacing the builtin flow if
10417+
it is <link linkend="configure-option-with-libcurl">installed</link>.
10418+
The hook should either directly return a Bearer token for the current
1040510419
user/issuer/scope combination, if one is available without blocking, or
1040610420
else set up an asynchronous callback to retrieve one.
1040710421
</para>

meson.build

+25-7
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ os_deps = []
107107
backend_both_deps = []
108108
backend_deps = []
109109
libpq_deps = []
110+
libpq_oauth_deps = []
110111

111112
pg_sysroot = ''
112113

@@ -860,13 +861,13 @@ endif
860861
###############################################################
861862

862863
libcurlopt = get_option('libcurl')
864+
oauth_flow_supported = false
865+
863866
if not libcurlopt.disabled()
864867
# Check for libcurl 7.61.0 or higher (corresponding to RHEL8 and the ability
865868
# to explicitly set TLS 1.3 ciphersuites).
866869
libcurl = dependency('libcurl', version: '>= 7.61.0', required: libcurlopt)
867870
if libcurl.found()
868-
cdata.set('USE_LIBCURL', 1)
869-
870871
# Check to see whether the current platform supports thread-safe Curl
871872
# initialization.
872873
libcurl_threadsafe_init = false
@@ -938,6 +939,22 @@ if not libcurlopt.disabled()
938939
endif
939940
endif
940941

942+
# Check that the current platform supports our builtin flow. This requires
943+
# libcurl and one of either epoll or kqueue.
944+
oauth_flow_supported = (
945+
libcurl.found()
946+
and (cc.check_header('sys/event.h', required: false,
947+
args: test_c_args, include_directories: postgres_inc)
948+
or cc.check_header('sys/epoll.h', required: false,
949+
args: test_c_args, include_directories: postgres_inc))
950+
)
951+
952+
if oauth_flow_supported
953+
cdata.set('USE_LIBCURL', 1)
954+
elif libcurlopt.enabled()
955+
error('client-side OAuth is not supported on this platform')
956+
endif
957+
941958
else
942959
libcurl = not_found_dep
943960
endif
@@ -3272,17 +3289,18 @@ libpq_deps += [
32723289

32733290
gssapi,
32743291
ldap_r,
3275-
# XXX libcurl must link after libgssapi_krb5 on FreeBSD to avoid segfaults
3276-
# during gss_acquire_cred(). This is possibly related to Curl's Heimdal
3277-
# dependency on that platform?
3278-
libcurl,
32793292
libintl,
32803293
ssl,
32813294
]
32823295

3296+
libpq_oauth_deps += [
3297+
libcurl,
3298+
]
3299+
32833300
subdir('src/interfaces/libpq')
3284-
# fe_utils depends on libpq
3301+
# fe_utils and libpq-oauth depends on libpq
32853302
subdir('src/fe_utils')
3303+
subdir('src/interfaces/libpq-oauth')
32863304

32873305
# for frontend binaries
32883306
frontend_code = declare_dependency(

src/Makefile.global.in

+3
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,9 @@ perl_embed_ldflags = @perl_embed_ldflags@
347347

348348
AWK = @AWK@
349349
LN_S = @LN_S@
350+
LIBCURL_CPPFLAGS = @LIBCURL_CPPFLAGS@
351+
LIBCURL_LDFLAGS = @LIBCURL_LDFLAGS@
352+
LIBCURL_LDLIBS = @LIBCURL_LDLIBS@
350353
MSGFMT = @MSGFMT@
351354
MSGFMT_FLAGS = @MSGFMT_FLAGS@
352355
MSGMERGE = @MSGMERGE@

src/interfaces/Makefile

+12
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,19 @@ include $(top_builddir)/src/Makefile.global
1414

1515
SUBDIRS = libpq ecpg
1616

17+
ifeq ($(with_libcurl), yes)
18+
SUBDIRS += libpq-oauth
19+
else
20+
ALWAYS_SUBDIRS += libpq-oauth
21+
endif
22+
1723
$(recurse)
24+
$(recurse_always)
1825

1926
all-ecpg-recurse: all-libpq-recurse
2027
install-ecpg-recurse: install-libpq-recurse
28+
29+
ifeq ($(with_libcurl), yes)
30+
all-libpq-oauth-recurse: all-libpq-recurse
31+
install-libpq-oauth-recurse: install-libpq-recurse
32+
endif

0 commit comments

Comments
 (0)