diff --git a/src/interfaces/libpq-oauth/Makefile b/src/interfaces/libpq-oauth/Makefile index a5f2d65fcad..e90482566b1 100644 --- a/src/interfaces/libpq-oauth/Makefile +++ b/src/interfaces/libpq-oauth/Makefile @@ -16,13 +16,10 @@ include $(top_builddir)/src/Makefile.global PGFILEDESC = "libpq-oauth - device authorization OAuth support" # This is an internal module; we don't want an SONAME and therefore do not set -# SO_MAJOR_VERSION. -NAME = pq-oauth-$(MAJORVERSION) - -# Force the name "libpq-oauth" for both the static and shared libraries. The -# staticlib doesn't need version information in its name. +# SO_MAJOR_VERSION. This requires an explicit override for the shared library +# name. +NAME = pq-oauth override shlib := lib$(NAME)$(DLSUFFIX) -override stlib := libpq-oauth.a override CPPFLAGS := -I$(libpq_srcdir) -I$(top_builddir)/src/port $(CPPFLAGS) $(LIBCURL_CPPFLAGS) override CFLAGS += $(PTHREAD_CFLAGS) diff --git a/src/interfaces/libpq-oauth/README b/src/interfaces/libpq-oauth/README index 553962d644e..a27a8238d4b 100644 --- a/src/interfaces/libpq-oauth/README +++ b/src/interfaces/libpq-oauth/README @@ -10,48 +10,33 @@ results in a failed connection. = 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. -The module exports the following symbols: +As of v19, this module ABI is public and cannot change incompatibly without also +changing the entry points. Both libpq and libpq-oauth must gracefully handle +situations where the other library is of a different release version, past or +future, since upgrades to the libraries may happen in either order. -- PostgresPollingStatusType pg_fe_run_oauth_flow(PGconn *conn); -- void pg_fe_cleanup_oauth_flow(PGconn *conn); +(Don't assume that package version dependencies from libpq-oauth to libpq will +simplify the situation! Since libpq delay-loads libpq-oauth, we still have to +handle cases where a long-running client application has a libpq that's older +than a newly upgraded plugin.) -pg_fe_run_oauth_flow and pg_fe_cleanup_oauth_flow are implementations of -conn->async_auth and conn->cleanup_async_auth, respectively. +The module exports the following symbol: -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: +- int pg_start_oauthbearer(PGconn *conn, PGoauthBearerRequestV2 *request); -- void libpq_oauth_init(pgthreadlock_t threadlock, - libpq_gettext_func gettext_impl, - conn_errorMessage_func errmsg_impl, - conn_oauth_client_id_func clientid_impl, - conn_oauth_client_secret_func clientsecret_impl, - conn_oauth_discovery_uri_func discoveryuri_impl, - conn_oauth_issuer_id_func issuerid_impl, - conn_oauth_scope_func scope_impl, - conn_sasl_state_func saslstate_impl, - set_conn_altsock_func setaltsock_impl, - set_conn_oauth_token_func settoken_impl); +The module then behaves as if it had received a PQAUTHDATA_OAUTH_BEARER_TOKEN_V2 +request via the PQauthDataHook API, and it either fills in an existing token or +populates the necessary callbacks for a token to be obtained asynchronously. +(See the documentation for PGoauthBearerRequest.) The function returns zero on +success and nonzero on failure. -It also relies on access to several members of the PGconn struct. Not only can -these change positions across minor versions, but the offsets aren't necessarily -stable within a single minor release (conn->errorMessage, for instance, can -change offsets depending on configure-time options). Therefore the necessary -accessors (named conn_*) and mutators (set_conn_*) are injected here. With this -approach, 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 -- which becomes especially -important during "live upgrade" situations where a running libpq application has -the libpq-oauth module updated out from under it before it's first loaded from -disk. +Additionally, libpq-oauth relies on libpq's libpq_gettext(), which must be +injected by libpq using this initialization function before the flow is run: + +- void libpq_oauth_init(libpq_gettext_func gettext_impl); = 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 -directly to provide the necessary symbols. (libpq.a and libpq-oauth.a must be -part of the same build. Unlike the dynamic module, there are no translation -shims provided.) +flow is enabled, the application is expected to link against libpq-oauth.a to +provide the necessary symbol, or else implement pg_start_oauthbearer() itself. diff --git a/src/interfaces/libpq-oauth/exports.txt b/src/interfaces/libpq-oauth/exports.txt index 6891a83dbf9..7bc12b860d7 100644 --- a/src/interfaces/libpq-oauth/exports.txt +++ b/src/interfaces/libpq-oauth/exports.txt @@ -1,4 +1,3 @@ # src/interfaces/libpq-oauth/exports.txt libpq_oauth_init 1 -pg_fe_run_oauth_flow 2 -pg_fe_cleanup_oauth_flow 3 +pg_start_oauthbearer 2 diff --git a/src/interfaces/libpq-oauth/meson.build b/src/interfaces/libpq-oauth/meson.build index 27aca2bc324..685a00acf7a 100644 --- a/src/interfaces/libpq-oauth/meson.build +++ b/src/interfaces/libpq-oauth/meson.build @@ -38,10 +38,8 @@ endif # 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) - if build_shared_lib -libpq_oauth_so = shared_module(libpq_oauth_name, +libpq_oauth_so = shared_module('libpq-oauth', libpq_oauth_sources + libpq_oauth_so_sources, include_directories: [libpq_oauth_inc, postgres_inc], c_args: libpq_oauth_so_c_args, diff --git a/src/interfaces/libpq-oauth/oauth-curl.c b/src/interfaces/libpq-oauth/oauth-curl.c index 2c147f98d0d..052ecd32da2 100644 --- a/src/interfaces/libpq-oauth/oauth-curl.c +++ b/src/interfaces/libpq-oauth/oauth-curl.c @@ -29,7 +29,6 @@ #endif #include "common/jsonapi.h" -#include "fe-auth-oauth.h" #include "mb/pg_wchar.h" #include "oauth-curl.h" @@ -44,23 +43,10 @@ #else /* !USE_DYNAMIC_OAUTH */ -/* - * Static builds may rely on PGconn offsets directly. Keep these aligned with - * the bank of callbacks in oauth-utils.h. - */ +/* Static builds may make use of libpq internals directly. */ +#include "fe-auth-oauth.h" #include "libpq-int.h" -#define conn_errorMessage(CONN) (&CONN->errorMessage) -#define conn_oauth_client_id(CONN) (CONN->oauth_client_id) -#define conn_oauth_client_secret(CONN) (CONN->oauth_client_secret) -#define conn_oauth_discovery_uri(CONN) (CONN->oauth_discovery_uri) -#define conn_oauth_issuer_id(CONN) (CONN->oauth_issuer_id) -#define conn_oauth_scope(CONN) (CONN->oauth_scope) -#define conn_sasl_state(CONN) (CONN->sasl_state) - -#define set_conn_altsock(CONN, VAL) do { CONN->altsock = VAL; } while (0) -#define set_conn_oauth_token(CONN, VAL) do { CONN->oauth_token = VAL; } while (0) - #endif /* USE_DYNAMIC_OAUTH */ /* One final guardrail against accidental inclusion... */ @@ -227,6 +213,15 @@ enum OAuthStep */ struct async_ctx { + /* relevant connection options cached from the PGconn */ + char *client_id; /* oauth_client_id */ + char *client_secret; /* oauth_client_secret (may be NULL) */ + + /* options cached from the PGoauthBearerRequest (we don't own these) */ + const char *discovery_uri; + const char *issuer_id; + const char *scope; + enum OAuthStep step; /* where are we in the flow? */ int timerfd; /* descriptor for signaling async timeouts */ @@ -339,29 +334,72 @@ free_async_ctx(struct async_ctx *actx) if (actx->timerfd >= 0) close(actx->timerfd); + free(actx->client_id); + free(actx->client_secret); + free(actx); } /* - * Release resources used for the asynchronous exchange and disconnect the - * altsock. - * - * This is called either at the end of a successful authentication, or during - * pqDropConnection(), so we won't leak resources even if PQconnectPoll() never - * calls us back. + * Release resources used for the asynchronous exchange. */ -void -pg_fe_cleanup_oauth_flow(PGconn *conn) +static void +pg_fe_cleanup_oauth_flow(PGconn *conn, PGoauthBearerRequest *request) { - fe_oauth_state *state = conn_sasl_state(conn); + struct async_ctx *actx = request->user; - if (state->async_ctx) + /* request->cleanup is only set after actx has been allocated. */ + Assert(actx); + + free_async_ctx(actx); + request->user = NULL; + + /* libpq has made its own copy of the token; clear ours now. */ + if (request->token) { - free_async_ctx(state->async_ctx); - state->async_ctx = NULL; + explicit_bzero(request->token, strlen(request->token)); + free(request->token); + request->token = NULL; + } +} + +/* + * Builds an error message from actx and stores it in req->error. The allocation + * is backed by actx->work_data (which will be reset first). + */ +static void +append_actx_error(PGoauthBearerRequestV2 *req, struct async_ctx *actx) +{ + PQExpBuffer errbuf = &actx->work_data; + + resetPQExpBuffer(errbuf); + + /* + * Assemble the three parts of our error: context, body, and detail. See + * also the documentation for struct async_ctx. + */ + if (actx->errctx) + appendPQExpBuffer(errbuf, "%s: ", actx->errctx); + + if (PQExpBufferDataBroken(actx->errbuf)) + appendPQExpBufferStr(errbuf, libpq_gettext("out of memory")); + else + appendPQExpBufferStr(errbuf, actx->errbuf.data); + + if (actx->curl_err[0]) + { + appendPQExpBuffer(errbuf, " (libcurl: %s)", actx->curl_err); + + /* Sometimes libcurl adds a newline to the error buffer. :( */ + if (errbuf->len >= 2 && errbuf->data[errbuf->len - 2] == '\n') + { + errbuf->data[errbuf->len - 2] = ')'; + errbuf->data[errbuf->len - 1] = '\0'; + errbuf->len--; + } } - set_conn_altsock(conn, PGINVALID_SOCKET); + req->error = errbuf->data; } /* @@ -2197,7 +2235,7 @@ static bool check_issuer(struct async_ctx *actx, PGconn *conn) { const struct provider *provider = &actx->provider; - const char *oauth_issuer_id = conn_oauth_issuer_id(conn); + const char *oauth_issuer_id = actx->issuer_id; Assert(oauth_issuer_id); /* ensured by setup_oauth_parameters() */ Assert(provider->issuer); /* ensured by parse_provider() */ @@ -2300,8 +2338,8 @@ check_for_device_flow(struct async_ctx *actx) static bool add_client_identification(struct async_ctx *actx, PQExpBuffer reqbody, PGconn *conn) { - const char *oauth_client_id = conn_oauth_client_id(conn); - const char *oauth_client_secret = conn_oauth_client_secret(conn); + const char *oauth_client_id = actx->client_id; + const char *oauth_client_secret = actx->client_secret; bool success = false; char *username = NULL; @@ -2384,11 +2422,10 @@ cleanup: static bool start_device_authz(struct async_ctx *actx, PGconn *conn) { - const char *oauth_scope = conn_oauth_scope(conn); + const char *oauth_scope = actx->scope; const char *device_authz_uri = actx->provider.device_authorization_endpoint; PQExpBuffer work_buffer = &actx->work_data; - Assert(conn_oauth_client_id(conn)); /* ensured by setup_oauth_parameters() */ Assert(device_authz_uri); /* ensured by check_for_device_flow() */ /* Construct our request body. */ @@ -2476,7 +2513,6 @@ start_token_request(struct async_ctx *actx, PGconn *conn) const char *device_code = actx->authz.device_code; PQExpBuffer work_buffer = &actx->work_data; - Assert(conn_oauth_client_id(conn)); /* ensured by setup_oauth_parameters() */ Assert(token_uri); /* ensured by parse_provider() */ Assert(device_code); /* ensured by parse_device_authz() */ @@ -2655,7 +2691,7 @@ prompt_user(struct async_ctx *actx, PGconn *conn) * function will not try to reinitialize Curl on successive calls. */ static bool -initialize_curl(PGconn *conn) +initialize_curl(PGoauthBearerRequestV2 *req) { /* * Don't let the compiler play tricks with this variable. In the @@ -2689,8 +2725,7 @@ initialize_curl(PGconn *conn) goto done; else if (init_successful == PG_BOOL_NO) { - libpq_append_conn_error(conn, - "curl_global_init previously failed during OAuth setup"); + req->error = libpq_gettext("curl_global_init previously failed during OAuth setup"); goto done; } @@ -2708,8 +2743,7 @@ initialize_curl(PGconn *conn) */ if (curl_global_init(CURL_GLOBAL_ALL & ~CURL_GLOBAL_WIN32) != CURLE_OK) { - libpq_append_conn_error(conn, - "curl_global_init failed during OAuth setup"); + req->error = libpq_gettext("curl_global_init failed during OAuth setup"); init_successful = PG_BOOL_NO; goto done; } @@ -2730,11 +2764,11 @@ initialize_curl(PGconn *conn) * In a downgrade situation, the damage is already done. Curl global * state may be corrupted. Be noisy. */ - libpq_append_conn_error(conn, "libcurl is no longer thread-safe\n" - "\tCurl initialization was reported thread-safe when libpq\n" - "\twas compiled, but the currently installed version of\n" - "\tlibcurl reports that it is not. Recompile libpq against\n" - "\tthe installed version of libcurl."); + req->error = libpq_gettext("libcurl is no longer thread-safe\n" + "\tCurl initialization was reported thread-safe when libpq\n" + "\twas compiled, but the currently installed version of\n" + "\tlibcurl reports that it is not. Recompile libpq against\n" + "\tthe installed version of libcurl."); init_successful = PG_BOOL_NO; goto done; } @@ -2764,54 +2798,16 @@ done: * provider. */ static PostgresPollingStatusType -pg_fe_run_oauth_flow_impl(PGconn *conn) +pg_fe_run_oauth_flow_impl(PGconn *conn, PGoauthBearerRequestV2 *request, + int *altsock) { - fe_oauth_state *state = conn_sasl_state(conn); - struct async_ctx *actx; + struct async_ctx *actx = request->v1.user; char *oauth_token = NULL; - PQExpBuffer errbuf; - - if (!initialize_curl(conn)) - return PGRES_POLLING_FAILED; - - if (!state->async_ctx) - { - /* - * Create our asynchronous state, and hook it into the upper-level - * OAuth state immediately, so any failures below won't leak the - * context allocation. - */ - actx = calloc(1, sizeof(*actx)); - if (!actx) - { - libpq_append_conn_error(conn, "out of memory"); - return PGRES_POLLING_FAILED; - } - - actx->mux = PGINVALID_SOCKET; - actx->timerfd = -1; - - /* Should we enable unsafe features? */ - actx->debugging = oauth_unsafe_debugging_enabled(); - - state->async_ctx = actx; - - initPQExpBuffer(&actx->work_data); - initPQExpBuffer(&actx->errbuf); - - if (!setup_multiplexer(actx)) - goto error_return; - - if (!setup_curl_handles(actx)) - goto error_return; - } - - actx = state->async_ctx; do { /* By default, the multiplexer is the altsock. Reassign as desired. */ - set_conn_altsock(conn, actx->mux); + *altsock = actx->mux; switch (actx->step) { @@ -2876,7 +2872,7 @@ pg_fe_run_oauth_flow_impl(PGconn *conn) if (!expired) { - set_conn_altsock(conn, actx->timerfd); + *altsock = actx->timerfd; return PGRES_POLLING_READING; } @@ -2893,7 +2889,7 @@ pg_fe_run_oauth_flow_impl(PGconn *conn) { case OAUTH_STEP_INIT: actx->errctx = libpq_gettext("failed to fetch OpenID discovery document"); - if (!start_discovery(actx, conn_oauth_discovery_uri(conn))) + if (!start_discovery(actx, actx->discovery_uri)) goto error_return; actx->step = OAUTH_STEP_DISCOVERY; @@ -2933,10 +2929,10 @@ pg_fe_run_oauth_flow_impl(PGconn *conn) goto error_return; /* - * Hook any oauth_token into the PGconn immediately so that - * the allocation isn't lost in case of an error. + * Hook any oauth_token into the request struct immediately so + * that the allocation isn't lost in case of an error. */ - set_conn_oauth_token(conn, oauth_token); + request->v1.token = oauth_token; if (!actx->user_prompted) { @@ -2965,7 +2961,7 @@ pg_fe_run_oauth_flow_impl(PGconn *conn) * the client wait directly on the timerfd rather than the * multiplexer. */ - set_conn_altsock(conn, actx->timerfd); + *altsock = actx->timerfd; actx->step = OAUTH_STEP_WAIT_INTERVAL; actx->running = 1; @@ -2991,48 +2987,21 @@ pg_fe_run_oauth_flow_impl(PGconn *conn) return oauth_token ? PGRES_POLLING_OK : PGRES_POLLING_READING; error_return: - errbuf = conn_errorMessage(conn); - - /* - * Assemble the three parts of our error: context, body, and detail. See - * also the documentation for struct async_ctx. - */ - if (actx->errctx) - appendPQExpBuffer(errbuf, "%s: ", actx->errctx); - - if (PQExpBufferDataBroken(actx->errbuf)) - appendPQExpBufferStr(errbuf, libpq_gettext("out of memory")); - else - appendPQExpBufferStr(errbuf, actx->errbuf.data); - - if (actx->curl_err[0]) - { - appendPQExpBuffer(errbuf, " (libcurl: %s)", actx->curl_err); - - /* Sometimes libcurl adds a newline to the error buffer. :( */ - if (errbuf->len >= 2 && errbuf->data[errbuf->len - 2] == '\n') - { - errbuf->data[errbuf->len - 2] = ')'; - errbuf->data[errbuf->len - 1] = '\0'; - errbuf->len--; - } - } - - appendPQExpBufferChar(errbuf, '\n'); + append_actx_error(request, actx); return PGRES_POLLING_FAILED; } /* - * The top-level entry point. This is a convenient place to put necessary - * wrapper logic before handing off to the true implementation, above. + * The top-level entry point for the flow. This is a convenient place to put + * necessary wrapper logic before handing off to the true implementation, above. */ -PostgresPollingStatusType -pg_fe_run_oauth_flow(PGconn *conn) +static PostgresPollingStatusType +pg_fe_run_oauth_flow(PGconn *conn, struct PGoauthBearerRequest *request, + int *altsock) { PostgresPollingStatusType result; - fe_oauth_state *state = conn_sasl_state(conn); - struct async_ctx *actx; + struct async_ctx *actx = request->user; #ifndef WIN32 sigset_t osigset; bool sigpipe_pending; @@ -3059,20 +3028,16 @@ pg_fe_run_oauth_flow(PGconn *conn) masked = (pq_block_sigpipe(&osigset, &sigpipe_pending) == 0); #endif - result = pg_fe_run_oauth_flow_impl(conn); + result = pg_fe_run_oauth_flow_impl(conn, + (PGoauthBearerRequestV2 *) request, + altsock); /* * To assist with finding bugs in comb_multiplexer() and * drain_timer_events(), when we're in debug mode, track the total number * of calls to this function and print that at the end of the flow. - * - * Be careful that state->async_ctx could be NULL if early initialization - * fails during the first call. */ - actx = state->async_ctx; - Assert(actx || result == PGRES_POLLING_FAILED); - - if (actx && actx->debugging) + if (actx->debugging) { actx->dbg_num_calls++; if (result == PGRES_POLLING_OK || result == PGRES_POLLING_FAILED) @@ -3093,3 +3058,104 @@ pg_fe_run_oauth_flow(PGconn *conn) return result; } + +/* + * Callback registration for OAUTHBEARER. libpq calls this once per OAuth + * connection. + */ +int +pg_start_oauthbearer(PGconn *conn, PGoauthBearerRequestV2 *request) +{ + struct async_ctx *actx; + PQconninfoOption *conninfo = NULL; + + if (!initialize_curl(request)) + return -1; + + /* + * Create our asynchronous state, and hook it into the upper-level OAuth + * state immediately, so any failures below won't leak the context + * allocation. + */ + actx = calloc(1, sizeof(*actx)); + if (!actx) + goto oom; + + actx->mux = PGINVALID_SOCKET; + actx->timerfd = -1; + + /* + * Now we have a valid (but still useless) actx, so we can fill in the + * request object. From this point onward, failures will result in a call + * to pg_fe_cleanup_oauth_flow(). Further cleanup logic belongs there. + */ + request->v1.async = pg_fe_run_oauth_flow; + request->v1.cleanup = pg_fe_cleanup_oauth_flow; + request->v1.user = actx; + + /* + * Now finish filling in the actx. + */ + + /* Should we enable unsafe features? */ + actx->debugging = oauth_unsafe_debugging_enabled(); + + initPQExpBuffer(&actx->work_data); + initPQExpBuffer(&actx->errbuf); + + /* Pull relevant connection options. */ + conninfo = PQconninfo(conn); + if (!conninfo) + goto oom; + + for (PQconninfoOption *opt = conninfo; opt->keyword; opt++) + { + if (!opt->val) + continue; /* simplifies the strdup logic below */ + + if (strcmp(opt->keyword, "oauth_client_id") == 0) + { + actx->client_id = strdup(opt->val); + if (!actx->client_id) + goto oom; + } + else if (strcmp(opt->keyword, "oauth_client_secret") == 0) + { + actx->client_secret = strdup(opt->val); + if (!actx->client_secret) + goto oom; + } + } + + PQconninfoFree(conninfo); + conninfo = NULL; /* keeps `goto oom` safe */ + + actx->discovery_uri = request->v1.openid_configuration; + actx->issuer_id = request->issuer; + actx->scope = request->v1.scope; + + Assert(actx->client_id); /* ensured by setup_oauth_parameters() */ + Assert(actx->issuer_id); /* ensured by setup_oauth_parameters() */ + Assert(actx->discovery_uri); /* ensured by oauth_exchange() */ + + if (!setup_multiplexer(actx)) + { + append_actx_error(request, actx); + return -1; + } + + if (!setup_curl_handles(actx)) + { + append_actx_error(request, actx); + return -1; + } + + return 0; + +oom: + if (conninfo) + PQconninfoFree(conninfo); + + request->error = libpq_gettext("out of memory"); + return -1; +} diff --git a/src/interfaces/libpq-oauth/oauth-curl.h b/src/interfaces/libpq-oauth/oauth-curl.h index 686e7b4df3f..1d4dd766217 100644 --- a/src/interfaces/libpq-oauth/oauth-curl.h +++ b/src/interfaces/libpq-oauth/oauth-curl.h @@ -17,8 +17,8 @@ #include "libpq-fe.h" -/* Exported async-auth callbacks. */ -extern PGDLLEXPORT PostgresPollingStatusType pg_fe_run_oauth_flow(PGconn *conn); -extern PGDLLEXPORT void pg_fe_cleanup_oauth_flow(PGconn *conn); +/* Exported flow callback. */ +extern PGDLLEXPORT int pg_start_oauthbearer(PGconn *conn, + PGoauthBearerRequestV2 *request); #endif /* OAUTH_CURL_H */ diff --git a/src/interfaces/libpq-oauth/oauth-utils.c b/src/interfaces/libpq-oauth/oauth-utils.c index 4ebe7d0948c..ccb0d9bf2c5 100644 --- a/src/interfaces/libpq-oauth/oauth-utils.c +++ b/src/interfaces/libpq-oauth/oauth-utils.c @@ -35,85 +35,18 @@ pgthreadlock_t pg_g_threadlock; static libpq_gettext_func libpq_gettext_impl; -conn_errorMessage_func conn_errorMessage; -conn_oauth_client_id_func conn_oauth_client_id; -conn_oauth_client_secret_func conn_oauth_client_secret; -conn_oauth_discovery_uri_func conn_oauth_discovery_uri; -conn_oauth_issuer_id_func conn_oauth_issuer_id; -conn_oauth_scope_func conn_oauth_scope; -conn_sasl_state_func conn_sasl_state; - -set_conn_altsock_func set_conn_altsock; -set_conn_oauth_token_func set_conn_oauth_token; - /*- * Initializes libpq-oauth by setting necessary callbacks. * - * The current implementation relies on the following private implementation - * details of libpq: - * - * - pg_g_threadlock: protects libcurl initialization if the underlying Curl - * installation is not threadsafe - * - * - libpq_gettext: translates error messages using libpq's message domain - * - * The implementation also needs access to several members of the PGconn struct, - * which are not guaranteed to stay in place across minor versions. Accessors - * (named conn_*) and mutators (named set_conn_*) are injected here. + * The current implementation relies on libpq_gettext to translate error + * messages using libpq's message domain, so libpq injects it here. We also use + * this chance to initialize our threadlock. */ void -libpq_oauth_init(pgthreadlock_t threadlock_impl, - libpq_gettext_func gettext_impl, - conn_errorMessage_func errmsg_impl, - conn_oauth_client_id_func clientid_impl, - conn_oauth_client_secret_func clientsecret_impl, - conn_oauth_discovery_uri_func discoveryuri_impl, - conn_oauth_issuer_id_func issuerid_impl, - conn_oauth_scope_func scope_impl, - conn_sasl_state_func saslstate_impl, - set_conn_altsock_func setaltsock_impl, - set_conn_oauth_token_func settoken_impl) +libpq_oauth_init(libpq_gettext_func gettext_impl) { - pg_g_threadlock = threadlock_impl; + pg_g_threadlock = PQgetThreadLock(); libpq_gettext_impl = gettext_impl; - conn_errorMessage = errmsg_impl; - conn_oauth_client_id = clientid_impl; - conn_oauth_client_secret = clientsecret_impl; - conn_oauth_discovery_uri = discoveryuri_impl; - conn_oauth_issuer_id = issuerid_impl; - conn_oauth_scope = scope_impl; - conn_sasl_state = saslstate_impl; - set_conn_altsock = setaltsock_impl; - set_conn_oauth_token = settoken_impl; -} - -/* - * Append a formatted string to the error message buffer of the given - * connection, after translating it. This is a copy of libpq's internal API. - */ -void -libpq_append_conn_error(PGconn *conn, const char *fmt,...) -{ - int save_errno = errno; - bool done; - va_list args; - PQExpBuffer errorMessage = conn_errorMessage(conn); - - Assert(fmt[strlen(fmt) - 1] != '\n'); - - if (PQExpBufferBroken(errorMessage)) - return; /* already failed */ - - /* Loop in case we have to retry after enlarging the buffer. */ - do - { - errno = save_errno; - va_start(args, fmt); - done = appendPQExpBufferVA(errorMessage, libpq_gettext(fmt), args); - va_end(args); - } while (!done); - - appendPQExpBufferChar(errorMessage, '\n'); } #ifdef ENABLE_NLS diff --git a/src/interfaces/libpq-oauth/oauth-utils.h b/src/interfaces/libpq-oauth/oauth-utils.h index 9f4d5b692d2..293e9936989 100644 --- a/src/interfaces/libpq-oauth/oauth-utils.h +++ b/src/interfaces/libpq-oauth/oauth-utils.h @@ -15,53 +15,13 @@ #ifndef OAUTH_UTILS_H #define OAUTH_UTILS_H -#include "fe-auth-oauth.h" #include "libpq-fe.h" #include "pqexpbuffer.h" -/* - * A bank of callbacks to safely access members of PGconn, which are all passed - * to libpq_oauth_init() by libpq. - * - * Keep these aligned with the definitions in fe-auth-oauth.c as well as the - * static declarations in oauth-curl.c. - */ -#define DECLARE_GETTER(TYPE, MEMBER) \ - typedef TYPE (*conn_ ## MEMBER ## _func) (PGconn *conn); \ - extern conn_ ## MEMBER ## _func conn_ ## MEMBER; - -#define DECLARE_SETTER(TYPE, MEMBER) \ - typedef void (*set_conn_ ## MEMBER ## _func) (PGconn *conn, TYPE val); \ - extern set_conn_ ## MEMBER ## _func set_conn_ ## MEMBER; - -DECLARE_GETTER(PQExpBuffer, errorMessage); -DECLARE_GETTER(char *, oauth_client_id); -DECLARE_GETTER(char *, oauth_client_secret); -DECLARE_GETTER(char *, oauth_discovery_uri); -DECLARE_GETTER(char *, oauth_issuer_id); -DECLARE_GETTER(char *, oauth_scope); -DECLARE_GETTER(fe_oauth_state *, sasl_state); - -DECLARE_SETTER(pgsocket, altsock); -DECLARE_SETTER(char *, oauth_token); - -#undef DECLARE_GETTER -#undef DECLARE_SETTER - typedef char *(*libpq_gettext_func) (const char *msgid); /* Initializes libpq-oauth. */ -extern PGDLLEXPORT void libpq_oauth_init(pgthreadlock_t threadlock, - libpq_gettext_func gettext_impl, - conn_errorMessage_func errmsg_impl, - conn_oauth_client_id_func clientid_impl, - conn_oauth_client_secret_func clientsecret_impl, - conn_oauth_discovery_uri_func discoveryuri_impl, - conn_oauth_issuer_id_func issuerid_impl, - conn_oauth_scope_func scope_impl, - conn_sasl_state_func saslstate_impl, - set_conn_altsock_func setaltsock_impl, - set_conn_oauth_token_func settoken_impl); +extern PGDLLEXPORT void libpq_oauth_init(libpq_gettext_func gettext_impl); /* * Duplicated APIs, copied from libpq (primarily libpq-int.h, which we cannot @@ -75,7 +35,6 @@ typedef enum PG_BOOL_NO /* No (false) */ } PGTernaryBool; -extern void libpq_append_conn_error(PGconn *conn, const char *fmt,...) pg_attribute_printf(2, 3); extern bool oauth_unsafe_debugging_enabled(void); extern int pq_block_sigpipe(sigset_t *osigset, bool *sigpipe_pending); extern void pq_reset_sigpipe(sigset_t *osigset, bool sigpipe_pending, bool got_epipe); diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c index 2aef327c68b..f93184f04db 100644 --- a/src/interfaces/libpq/fe-auth-oauth.c +++ b/src/interfaces/libpq/fe-auth-oauth.c @@ -78,7 +78,7 @@ oauth_init(PGconn *conn, const char *password, * This handles only mechanism state tied to the connection lifetime; state * stored in state->async_ctx is freed up either immediately after the * authentication handshake succeeds, or before the mechanism is cleaned up on - * failure. See pg_fe_cleanup_oauth_flow() and cleanup_user_oauth_flow(). + * failure. See pg_fe_cleanup_oauth_flow() and cleanup_oauth_flow(). */ static void oauth_free(void *opaq) @@ -680,30 +680,54 @@ cleanup: * it's added to conn->errorMessage here. */ static void -report_user_flow_error(PGconn *conn, const PGoauthBearerRequestV2 *request) +report_flow_error(PGconn *conn, const PGoauthBearerRequestV2 *request) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("user-defined OAuth flow failed")); + fe_oauth_state *state = conn->sasl_state; + const char *errmsg = request->error; - if (request->error) + /* + * User-defined flows are called out explicitly so that the user knows who + * to blame. Builtin flows don't need that extra message length; we expect + * them to always fill in request->error on failure anyway. + */ + if (state->builtin) { - appendPQExpBufferStr(&conn->errorMessage, ": "); - appendPQExpBufferStr(&conn->errorMessage, request->error); + if (!errmsg) + { + /* + * Don't turn a bug here into a crash in production, but don't + * bother translating either. + */ + Assert(false); + errmsg = "builtin flow failed but did not provide an error message"; + } + + appendPQExpBufferStr(&conn->errorMessage, errmsg); + } + else + { + appendPQExpBufferStr(&conn->errorMessage, + libpq_gettext("user-defined OAuth flow failed")); + if (errmsg) + { + appendPQExpBufferStr(&conn->errorMessage, ": "); + appendPQExpBufferStr(&conn->errorMessage, errmsg); + } } appendPQExpBufferChar(&conn->errorMessage, '\n'); } /* - * Callback implementation of conn->async_auth() for a user-defined OAuth flow. - * Delegates the retrieval of the token to the application's async callback. + * Callback implementation of conn->async_auth() for OAuth flows. Delegates the + * retrieval of the token to the PGoauthBearerRequestV2.async() callback. * - * This will be called multiple times as needed; the application is responsible - * for setting an altsock to signal and returning the correct PGRES_POLLING_* + * This will be called multiple times as needed; the callback is responsible for + * setting an altsock to signal and returning the correct PGRES_POLLING_* * statuses for use by PQconnectPoll(). */ static PostgresPollingStatusType -run_user_oauth_flow(PGconn *conn) +run_oauth_flow(PGconn *conn) { fe_oauth_state *state = conn->sasl_state; PGoauthBearerRequestV2 *request = state->async_ctx; @@ -711,6 +735,7 @@ run_user_oauth_flow(PGconn *conn) if (!request->v1.async) { + Assert(!state->builtin); /* be very noisy if our code does this */ libpq_append_conn_error(conn, "user-defined OAuth flow provided neither a token nor an async callback"); return PGRES_POLLING_FAILED; @@ -722,7 +747,7 @@ run_user_oauth_flow(PGconn *conn) if (status == PGRES_POLLING_FAILED) { - report_user_flow_error(conn, request); + report_flow_error(conn, request); return status; } else if (status == PGRES_POLLING_OK) @@ -734,6 +759,7 @@ run_user_oauth_flow(PGconn *conn) */ if (!request->v1.token) { + Assert(!state->builtin); libpq_append_conn_error(conn, "user-defined OAuth flow did not provide a token"); return PGRES_POLLING_FAILED; @@ -752,6 +778,7 @@ run_user_oauth_flow(PGconn *conn) /* The hook wants the client to poll the altsock. Make sure it set one. */ if (conn->altsock == PGINVALID_SOCKET) { + Assert(!state->builtin); libpq_append_conn_error(conn, "user-defined OAuth flow did not provide a socket for polling"); return PGRES_POLLING_FAILED; @@ -761,12 +788,16 @@ run_user_oauth_flow(PGconn *conn) } /* - * Cleanup callback for the async user flow. Delegates most of its job to + * Cleanup callback for the async flow. Delegates most of its job to * PGoauthBearerRequest.cleanup(), then disconnects the altsock and frees the * request itself. + * + * This is called either at the end of a successful authentication, or during + * pqDropConnection(), so we won't leak resources even if PQconnectPoll() never + * calls us back. */ static void -cleanup_user_oauth_flow(PGconn *conn) +cleanup_oauth_flow(PGconn *conn) { fe_oauth_state *state = conn->sasl_state; PGoauthBearerRequestV2 *request = state->async_ctx; @@ -786,12 +817,16 @@ cleanup_user_oauth_flow(PGconn *conn) * * There are three potential implementations of use_builtin_flow: * - * 1) If the OAuth client is disabled at configuration time, return false. + * 1) If the OAuth client is disabled at configuration time, return zero. * Dependent clients must provide their own flow. * 2) If the OAuth client is enabled and USE_DYNAMIC_OAUTH is defined, dlopen() * the libpq-oauth plugin and use its implementation. * 3) Otherwise, use flow callbacks that are statically linked into the * executable. + * + * For caller convenience, the return value follows the convention of + * PQauthDataHook: zero means no implementation is provided, negative indicates + * failure, and positive indicates success. */ #if !defined(USE_LIBCURL) @@ -800,10 +835,10 @@ cleanup_user_oauth_flow(PGconn *conn) * This configuration doesn't support the builtin flow. */ -bool -use_builtin_flow(PGconn *conn, fe_oauth_state *state) +static int +use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *request) { - return false; + return 0; } #elif defined(USE_DYNAMIC_OAUTH) @@ -814,36 +849,6 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state) typedef char *(*libpq_gettext_func) (const char *msgid); -/* - * Define accessor/mutator shims to inject into libpq-oauth, so that it doesn't - * depend on the offsets within PGconn. (These have changed during minor version - * updates in the past.) - */ - -#define DEFINE_GETTER(TYPE, MEMBER) \ - typedef TYPE (*conn_ ## MEMBER ## _func) (PGconn *conn); \ - static TYPE conn_ ## MEMBER(PGconn *conn) { return conn->MEMBER; } - -/* Like DEFINE_GETTER, but returns a pointer to the member. */ -#define DEFINE_GETTER_P(TYPE, MEMBER) \ - typedef TYPE (*conn_ ## MEMBER ## _func) (PGconn *conn); \ - static TYPE conn_ ## MEMBER(PGconn *conn) { return &conn->MEMBER; } - -#define DEFINE_SETTER(TYPE, MEMBER) \ - typedef void (*set_conn_ ## MEMBER ## _func) (PGconn *conn, TYPE val); \ - static void set_conn_ ## MEMBER(PGconn *conn, TYPE val) { conn->MEMBER = val; } - -DEFINE_GETTER_P(PQExpBuffer, errorMessage); -DEFINE_GETTER(char *, oauth_client_id); -DEFINE_GETTER(char *, oauth_client_secret); -DEFINE_GETTER(char *, oauth_discovery_uri); -DEFINE_GETTER(char *, oauth_issuer_id); -DEFINE_GETTER(char *, oauth_scope); -DEFINE_GETTER(fe_oauth_state *, sasl_state); - -DEFINE_SETTER(pgsocket, altsock); -DEFINE_SETTER(char *, oauth_token); - /* * Loads the libpq-oauth plugin via dlopen(), initializes it, and plugs its * callbacks into the connection's async auth handlers. @@ -852,27 +857,19 @@ DEFINE_SETTER(char *, oauth_token); * handle the use case where the build supports loading a flow but a user does * not want to install it. Troubleshooting of linker/loader failures can be done * via PGOAUTHDEBUG. + * + * The lifetime of *request ends shortly after this call, so it must be copied + * to longer-lived storage. */ -bool -use_builtin_flow(PGconn *conn, fe_oauth_state *state) +static int +use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *request) { static bool initialized = false; static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER; int lockerr; - void (*init) (pgthreadlock_t threadlock, - libpq_gettext_func gettext_impl, - conn_errorMessage_func errmsg_impl, - conn_oauth_client_id_func clientid_impl, - conn_oauth_client_secret_func clientsecret_impl, - conn_oauth_discovery_uri_func discoveryuri_impl, - conn_oauth_issuer_id_func issuerid_impl, - conn_oauth_scope_func scope_impl, - conn_sasl_state_func saslstate_impl, - set_conn_altsock_func setaltsock_impl, - set_conn_oauth_token_func settoken_impl); - PostgresPollingStatusType (*flow) (PGconn *conn); - void (*cleanup) (PGconn *conn); + void (*init) (libpq_gettext_func gettext_impl); + int (*start_flow) (PGconn *conn, PGoauthBearerRequestV2 *request); /* * On macOS only, load the module using its absolute install path; the @@ -885,9 +882,9 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state) */ const char *const module_name = #if defined(__darwin__) - LIBDIR "/libpq-oauth-" PG_MAJORVERSION DLSUFFIX; + LIBDIR "/libpq-oauth" DLSUFFIX; #else - "libpq-oauth-" PG_MAJORVERSION DLSUFFIX; + "libpq-oauth" DLSUFFIX; #endif state->builtin_flow = dlopen(module_name, RTLD_NOW | RTLD_LOCAL); @@ -903,22 +900,25 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state) if (oauth_unsafe_debugging_enabled()) fprintf(stderr, "failed dlopen for libpq-oauth: %s\n", dlerror()); - return false; + return 0; } if ((init = dlsym(state->builtin_flow, "libpq_oauth_init")) == NULL - || (flow = dlsym(state->builtin_flow, "pg_fe_run_oauth_flow")) == NULL - || (cleanup = dlsym(state->builtin_flow, "pg_fe_cleanup_oauth_flow")) == NULL) + || (start_flow = dlsym(state->builtin_flow, "pg_start_oauthbearer")) == NULL) { /* - * This is more of an error condition than the one above, but due to - * the dlerror() threadsafety issue, lock it behind PGOAUTHDEBUG too. + * This is more of an error condition than the one above, but the + * cause is still locked behind PGOAUTHDEBUG due to the dlerror() + * threadsafety issue. */ if (oauth_unsafe_debugging_enabled()) fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror()); dlclose(state->builtin_flow); - return false; + state->builtin_flow = NULL; + + request->error = libpq_gettext("could not find entry point for libpq-oauth"); + return -1; } /* @@ -937,57 +937,45 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state) /* Should not happen... but don't continue if it does. */ Assert(false); - libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr); - return false; + appendPQExpBuffer(&conn->errorMessage, + "use_builtin_flow: failed to lock mutex (%d)\n", + lockerr); + + request->error = ""; /* satisfy report_flow_error() */ + return -1; } if (!initialized) { - init(pg_g_threadlock, + init( #ifdef ENABLE_NLS - libpq_gettext, + libpq_gettext #else - NULL, + NULL #endif - conn_errorMessage, - conn_oauth_client_id, - conn_oauth_client_secret, - conn_oauth_discovery_uri, - conn_oauth_issuer_id, - conn_oauth_scope, - conn_sasl_state, - set_conn_altsock, - set_conn_oauth_token); + ); initialized = true; } pthread_mutex_unlock(&init_mutex); - /* Set our asynchronous callbacks. */ - conn->async_auth = flow; - conn->cleanup_async_auth = cleanup; - - return true; + return (start_flow(conn, request) == 0) ? 1 : -1; } #else /* - * Use the builtin flow in libpq-oauth.a (see libpq-oauth/oauth-curl.h). + * For static builds, we can just call pg_start_oauthbearer() directly. It's + * provided by libpq-oauth.a. */ -extern PostgresPollingStatusType pg_fe_run_oauth_flow(PGconn *conn); -extern void pg_fe_cleanup_oauth_flow(PGconn *conn); +extern int pg_start_oauthbearer(PGconn *conn, PGoauthBearerRequestV2 *request); -bool -use_builtin_flow(PGconn *conn, fe_oauth_state *state) +static int +use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *request) { - /* Set our asynchronous callbacks. */ - conn->async_auth = pg_fe_run_oauth_flow; - conn->cleanup_async_auth = pg_fe_cleanup_oauth_flow; - - return true; + return (pg_start_oauthbearer(conn, request) == 0) ? 1 : -1; } #endif /* USE_LIBCURL */ @@ -1000,11 +988,11 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state) * If the application has registered a custom flow handler using * PQAUTHDATA_OAUTH_BEARER_TOKEN[_V2], it may either return a token immediately * (e.g. if it has one cached for immediate use), or set up for a series of - * asynchronous callbacks which will be managed by run_user_oauth_flow(). + * asynchronous callbacks which will be managed by run_oauth_flow(). * * If the default handler is used instead, a Device Authorization flow is used - * for the connection if support has been compiled in. (See - * fe-auth-oauth-curl.c for implementation details.) + * for the connection if support has been compiled in. (See oauth-curl.c for + * implementation details.) * * If neither a custom handler nor the builtin flow is available, the connection * fails here. @@ -1026,11 +1014,17 @@ setup_token_request(PGconn *conn, fe_oauth_state *state) /* * The client may have overridden the OAuth flow. Try the v2 hook first, - * then fall back to the v1 implementation. + * then fall back to the v1 implementation. If neither is available, try + * the builtin flow. */ res = PQauthDataHook(PQAUTHDATA_OAUTH_BEARER_TOKEN_V2, conn, &request); if (res == 0) res = PQauthDataHook(PQAUTHDATA_OAUTH_BEARER_TOKEN, conn, &request); + if (res == 0) + { + state->builtin = true; + res = use_builtin_flow(conn, state, &request); + } if (res > 0) { @@ -1065,22 +1059,21 @@ setup_token_request(PGconn *conn, fe_oauth_state *state) *request_copy = request; - conn->async_auth = run_user_oauth_flow; - conn->cleanup_async_auth = cleanup_user_oauth_flow; + conn->async_auth = run_oauth_flow; + conn->cleanup_async_auth = cleanup_oauth_flow; state->async_ctx = request_copy; - } - else if (res < 0) - { - report_user_flow_error(conn, &request); - goto fail; - } - else if (!use_builtin_flow(conn, state)) - { - libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)"); - goto fail; + + return true; } - return true; + /* + * Failure cases: either we tried to set up a flow and failed, or there + * was no flow to try. + */ + if (res < 0) + report_flow_error(conn, &request); + else + libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)"); fail: if (request.v1.cleanup) diff --git a/src/interfaces/libpq/fe-auth-oauth.h b/src/interfaces/libpq/fe-auth-oauth.h index 5c8a24b76fa..511284614f7 100644 --- a/src/interfaces/libpq/fe-auth-oauth.h +++ b/src/interfaces/libpq/fe-auth-oauth.h @@ -27,11 +27,6 @@ enum fe_oauth_step FE_OAUTH_SERVER_ERROR, }; -/* - * This struct is exported to the libpq-oauth module. If changes are needed - * during backports to stable branches, please keep ABI compatibility (no - * changes to existing members, add new members at the end, etc.). - */ typedef struct { enum fe_oauth_step step; @@ -39,12 +34,12 @@ typedef struct PGconn *conn; void *async_ctx; + bool builtin; void *builtin_flow; } fe_oauth_state; extern void pqClearOAuthToken(PGconn *conn); extern bool oauth_unsafe_debugging_enabled(void); -extern bool use_builtin_flow(PGconn *conn, fe_oauth_state *state); /* Mechanisms in fe-auth-oauth.c */ extern const pg_fe_sasl_mech pg_oauth_mech; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 41d54182887..49ad84a62d4 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3589,13 +3589,6 @@ colormaprange compare_context config_handle config_var_value -conn_errorMessage_func -conn_oauth_client_id_func -conn_oauth_client_secret_func -conn_oauth_discovery_uri_func -conn_oauth_issuer_id_func -conn_oauth_scope_func -conn_sasl_state_func contain_aggs_of_level_context contain_placeholder_references_context convert_testexpr_context @@ -4169,8 +4162,6 @@ security_class_t sem_t sepgsql_context_info_t sequence_magic -set_conn_altsock_func -set_conn_oauth_token_func set_join_pathlist_hook_type set_rel_pathlist_hook_type shared_ts_iter