> On 25 Oct 2024, at 20:22, Jacob Champion <jacob.champ...@enterprisedb.com>
> wrote:
> I have combed almost all of Daniel's feedback backwards into the main
> patch (just the new bzero code remains, with the open question
> upthread),
Re-reading I can't see a vector there, I guess I am just scarred from what
seemed to be harmless leaks in auth codepaths and treat every bit as
potentially important. Feel free to drop from the patchset for now.
> Next up is, hopefully, url-encoding. I hadn't realized what an
> absolute mess that would be [1].
Everything and anything involving urls is a hot mess =/
Looking more at the patchset I think we need to apply conditional compilation
of the backend for oauth like how we do with other opt-in schemes in configure
and meson. The attached .txt has a diff for making --with-oauth a requirement
for compiling support into backend libpq.
--
Daniel Gustafsson
diff --git a/configure b/configure
index 39fe5a0542..b40cd836f1 100755
--- a/configure
+++ b/configure
@@ -8439,9 +8439,6 @@ fi
if test x"$with_oauth" = x"curl"; then
-$as_echo "#define USE_OAUTH 1" >>confdefs.h
-
-
$as_echo "#define USE_OAUTH_CURL 1" >>confdefs.h
# OAuth requires python for testing
diff --git a/configure.ac b/configure.ac
index f7e1400b6e..82217e652e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -927,8 +927,7 @@ if test x"$with_oauth" = x"" ; then
fi
if test x"$with_oauth" = x"curl"; then
- AC_DEFINE([USE_OAUTH], 1, [Define to 1 to build with OAuth 2.0 support.
(--with-oauth)])
- AC_DEFINE([USE_OAUTH_CURL], 1, [Define to 1 to use libcurl for OAuth
support.])
+ AC_DEFINE([USE_OAUTH_CURL], 1, [Define to 1 to build with OAuth 2.0 support.
(--with-oauth=curl)])
# OAuth requires python for testing
if test "$with_python" != yes; then
AC_MSG_WARN([*** OAuth support tests requires --with-python to run])
diff --git a/src/backend/libpq/Makefile b/src/backend/libpq/Makefile
index 98eb2a8242..ba502b0442 100644
--- a/src/backend/libpq/Makefile
+++ b/src/backend/libpq/Makefile
@@ -15,7 +15,6 @@ include $(top_builddir)/src/Makefile.global
# be-fsstubs is here for historical reasons, probably belongs elsewhere
OBJS = \
- auth-oauth.o \
auth-sasl.o \
auth-scram.o \
auth.o \
@@ -30,6 +29,10 @@ OBJS = \
pqmq.o \
pqsignal.o
+ifneq ($(with_oauth),)
+OBJS += auth-oauth.o
+endif
+
ifeq ($(with_ssl),openssl)
OBJS += be-secure-openssl.o
endif
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 0cf3e31c9f..57638f922e 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -29,7 +29,6 @@
#include "libpq/auth.h"
#include "libpq/crypt.h"
#include "libpq/libpq.h"
-#include "libpq/oauth.h"
#include "libpq/pqformat.h"
#include "libpq/sasl.h"
#include "libpq/scram.h"
@@ -201,6 +200,14 @@ static int CheckRADIUSAuth(Port *port);
static int PerformRadiusTransaction(const char *server, const char
*secret, const char *portstr, const char *identifier, const char *user_name,
const char *passwd);
+/*----------------------------------------------------------------
+ * OAuth Authentication
+ *----------------------------------------------------------------
+ */
+#ifdef USE_OAUTH
+#include "libpq/oauth.h"
+#endif
+
/*----------------------------------------------------------------
* Global authentication functions
*----------------------------------------------------------------
@@ -614,8 +621,13 @@ ClientAuthentication(Port *port)
case uaTrust:
status = STATUS_OK;
break;
+
case uaOAuth:
+#ifdef USE_OAUTH
status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL,
NULL);
+#else
+ Assert(false);
+#endif
break;
}
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index c623b8463d..719c7a881e 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1749,7 +1749,11 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel)
else if (strcmp(token->string, "radius") == 0)
parsedline->auth_method = uaRADIUS;
else if (strcmp(token->string, "oauth") == 0)
+#ifdef USE_OAUTH
parsedline->auth_method = uaOAuth;
+#else
+ unsupauth = "oauth";
+#endif
else
{
ereport(elevel,
diff --git a/src/backend/libpq/meson.build b/src/backend/libpq/meson.build
index c85527fb01..1c76dd80cc 100644
--- a/src/backend/libpq/meson.build
+++ b/src/backend/libpq/meson.build
@@ -1,7 +1,6 @@
# Copyright (c) 2022-2024, PostgreSQL Global Development Group
backend_sources += files(
- 'auth-oauth.c',
'auth-sasl.c',
'auth-scram.c',
'auth.c',
@@ -17,6 +16,10 @@ backend_sources += files(
'pqsignal.c',
)
+if oauth.found()
+ backend_sources += files('auth-oauth.c')
+endif
+
if ssl.found()
backend_sources += files('be-secure-openssl.c')
endif
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 2f4e3a8f63..a60df328bb 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -703,10 +703,7 @@
/* Define to select named POSIX semaphores. */
#undef USE_NAMED_POSIX_SEMAPHORES
-/* Define to 1 to build with OAuth 2.0 support. (--with-oauth) */
-#undef USE_OAUTH
-
-/* Define to 1 to use libcurl for OAuth support. */
+/* Define to 1 to build with OAuth 2.0 support. (--with-oauth=curl) */
#undef USE_OAUTH_CURL
/* Define to 1 to build with OpenSSL support. (--with-ssl=openssl) */
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index e49eb13e43..9bd655e306 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -172,6 +172,14 @@
#define USE_SSL
#endif
+/*
+ * USE_OAUTH code should be compiled only when compiling with OAuth support
+ * enabled.
+ */
+#ifdef USE_OAUTH_CURL
+#define USE_OAUTH
+#endif
+
/*
* This is the default directory in which AF_UNIX socket files are
* placed. Caution: changing this risks breaking your existing client
diff --git a/src/test/modules/oauth_validator/t/001_server.pl
b/src/test/modules/oauth_validator/t/001_server.pl
index 3f06f5be76..46684a56e0 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -19,7 +19,7 @@ if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~
/\boauth\b/)
if ($ENV{with_oauth} ne 'curl')
{
- plan skip_all => 'client-side OAuth not supported by this build';
+ plan skip_all => 'OAuth not supported by this build';
}
if ($ENV{with_python} ne 'yes')
