1: 90f05c5d963 < -: ----------- libpq: Introduce PQAUTHDATA_OAUTH_BEARER_TOKEN_V2 2: d8fbfc8f7a0 ! 1: f5b861f7efd libpq-oauth: Use the PGoauthBearerRequestV2 API @@ src/interfaces/libpq/fe-auth-oauth.c: 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 - * PGoauthBearerRequestV2.cleanup(), then disconnects the altsock and frees the + * PGoauthBearerRequest.cleanup(), then disconnects the altsock and frees the * request itself. + * + * This is called either at the end of a successful authentication, or during @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st + || (start_flow = dlsym(state->builtin_flow, "pg_start_oauthbearer")) == NULL) { /* - * This is more of an error condition than the one above, but due to -@@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_state *state) +- * 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; -+ return 0; ++ state->builtin_flow = NULL; ++ ++ request->error = libpq_gettext("could not find entry point for libpq-oauth"); ++ return -1; } /* @@ src/interfaces/libpq/fe-auth-oauth.c: 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); +- libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr); - return false; -+ return 0; ++ appendPQExpBuffer(&conn->errorMessage, ++ "use_builtin_flow: failed to lock mutex (%d)\n", ++ lockerr); ++ ++ request->error = ""; /* satisfy report_flow_error() */ ++ return -1; } if (!initialized) @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth + conn->async_auth = run_oauth_flow; + conn->cleanup_async_auth = cleanup_oauth_flow; state->async_ctx = request_copy; - } - else if (res < 0) - { +- } +- else if (res < 0) +- { - report_user_flow_error(conn, &request); -+ report_flow_error(conn, &request); - goto fail; - } +- 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)"); - goto fail; ++ libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)"); + + fail: + if (request.v1.cleanup) 3: 96bd5970668 = 2: f59d6431b04 libpq-oauth: Never link against libpq's encoding functions 4: 0e7fae1e152 ! 3: 75ac7ddc120 WIP: test out poisoning @@ Metadata Author: Jacob Champion ## Commit message ## - WIP: test out poisoning + libpq: Poison the v2 part of a v1 Bearer request + + The new PGoauthBearerRequestV2 API (which has similarities to the + "subclass" pointer architecture in use by the backend, for Nodes) + carries the risk of a developer ignoring the type of hook in use and + just casting directly to the V2 struct. This will appear to work fine in + 19, but crash (or worse) when speaking to libpq 18. + + However, we're in a unique position to catch this problem, because we + have tight control over the struct. Add poisoning code to the v1 path + which does the following: + + - masks the v2 request->issuer pointer, to hopefully point at nonsense + memory + - abort()s if the v2 request->error is assigned by the hook + - attempts to cover both with VALGRIND_MAKE_MEM_NOACCESS for the + duration of the callback (a potential AddressSanitizer implementation + is left for future work) + + The struct is unpoisoned after the call, so we can switch back to the v2 + internal implementation when necessary. + + Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com ## src/interfaces/libpq/fe-auth-oauth.h ## @@ src/interfaces/libpq/fe-auth-oauth.h: typedef struct @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth } @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth_state *state) - return true; + libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)"); fail: - if (request.v1.cleanup) -: ----------- > 4: 7a43398c90a libpq: Allow developers to reimplement libpq-oauth 5: fbb89bcd980 ! 5: b3daf3140f8 WIP: Introduce third-party OAuth flow plugins? @@ Commit message This experimental commit promotes the pg_start_oauthbearer API to a public header (libpq-oauth.h) and adds a PGOAUTHMODULE environment variable that overrides the load path for the plugin, allowing users to - provide their own. The libpq_oauth_init function is now optional, and - will remain undocumented. (Modules that don't provide it are marked as - user-defined.) + provide their own. It's probably not a good solution; it provides almost + nothing over LD_LIBRARY_PATH (other than not being LD_LIBRARY_PATH). This is a relatively small amount of implementation change, but unfortunately the tests have a large amount of code motion to be able to @@ src/interfaces/libpq-oauth/oauth-curl.h (deleted) - -#endif /* OAUTH_CURL_H */ - ## src/interfaces/libpq/fe-auth-oauth.h ## -@@ src/interfaces/libpq/fe-auth-oauth.h: typedef struct - - bool v1; - bool builtin; -- void *builtin_flow; -+ void *flow_module; - } fe_oauth_state; - - extern void pqClearOAuthToken(PGconn *conn); - ## src/interfaces/libpq/libpq-oauth.h (new) ## @@ +/*------------------------------------------------------------------------- @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st "libpq-oauth" DLSUFFIX; #endif -- state->builtin_flow = dlopen(module_name, RTLD_NOW | RTLD_LOCAL); -- if (!state->builtin_flow) + /*- + * Additionally, the user may override the module path explicitly to be + * able to provide their own module, via PGOAUTHMODULE. @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st + else + state->builtin = true; + -+ state->flow_module = dlopen(module_name, RTLD_NOW | RTLD_LOCAL); -+ if (!state->flow_module) + state->flow_module = dlopen(module_name, RTLD_NOW | RTLD_LOCAL); + if (!state->flow_module) { /* * For end users, this probably isn't an error condition, it just @@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_st + return -1; } -- if ((init = dlsym(state->builtin_flow, "libpq_oauth_init")) == NULL -- || (start_flow = dlsym(state->builtin_flow, "pg_start_oauthbearer")) == NULL) -+ if ((start_flow = dlsym(state->flow_module, "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. - */ - if (oauth_unsafe_debugging_enabled()) -- fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror()); -+ fprintf(stderr, "failed dlsym for %s: %s\n", module_name, dlerror()); -+ -+ dlclose(state->flow_module); -+ state->flow_module = NULL; -+ -+ if (state->builtin) -+ return 0; - -- dlclose(state->builtin_flow); -- return 0; -+ request->error = libpq_gettext("plugin entry point could not be located"); -+ return -1; - } - - /* -@@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re - */ - /* -- * We need to inject necessary function pointers into the module. This -- * only needs to be done once -- even if the pointers are constant, -- * assigning them while another thread is executing the flows feels like -- * tempting fate. -+ * Our libpq-oauth.so provides a special initialization function for libpq + * Our libpq-oauth.so provides a special initialization function for libpq +- * integration. If we don't find this, assume that a custom module is in +- * use instead. + * integration. It's not a problem if we don't find this; it just means + * that a user-defined module is being used. */ -- if ((lockerr = pthread_mutex_lock(&init_mutex)) != 0) -+ init = dlsym(state->flow_module, "libpq_oauth_init"); -+ -+ if (!init) -+ state->builtin = false; /* adjust our error messages */ -+ else - { -- /* Should not happen... but don't continue if it does. */ -- Assert(false); -+ /* -+ * We need to inject necessary function pointers into the module. This -+ * only needs to be done once -- even if the pointers are constant, -+ * assigning them while another thread is executing the flows feels -+ * like tempting fate. -+ */ -+ if ((lockerr = pthread_mutex_lock(&init_mutex)) != 0) -+ { -+ /* Should not happen... but don't continue if it does. */ -+ Assert(false); - -- libpq_append_conn_error(conn, "failed to lock mutex (%d)", lockerr); -- return 0; -- } -+ appendPQExpBuffer(&conn->errorMessage, -+ "use_builtin_flow: failed to lock mutex (%d)\n", -+ lockerr); - -- if (!initialized) -- { -- init( -+ request->error = ""; /* satisfy report_flow_error() */ -+ return -1; -+ } -+ -+ if (!initialized) -+ { -+ init( - #ifdef ENABLE_NLS -- libpq_gettext -+ libpq_gettext - #else -- NULL -+ NULL - #endif -- ); -+ ); + init = dlsym(state->flow_module, "libpq_oauth_init"); + if (!init) +@@ src/interfaces/libpq/fe-auth-oauth.c: use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re + * threadsafety issue. + */ + if (oauth_unsafe_debugging_enabled()) +- fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror()); ++ fprintf(stderr, "failed dlsym for %s: %s\n", module_name, dlerror()); -- initialized = true; -- } -+ initialized = true; -+ } + dlclose(state->flow_module); + state->flow_module = NULL; -- pthread_mutex_unlock(&init_mutex); -+ pthread_mutex_unlock(&init_mutex); -+ } +- request->error = libpq_gettext("could not find entry point for libpq-oauth"); ++ request->error = state->builtin ? ++ libpq_gettext("could not find entry point for libpq-oauth") : ++ libpq_gettext("could not find entry point for custom plugin"); + return -1; + } - return (start_flow(conn, request) == 0) ? 1 : -1; - } @@ src/interfaces/libpq/fe-auth-oauth.c: extern int pg_start_oauthbearer(PGconn *conn, PGoauthBearerRequestV2 *request); static int use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *request) @@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth if (res > 0) { -@@ src/interfaces/libpq/fe-auth-oauth.c: setup_token_request(PGconn *conn, fe_oauth_state *state) - conn->async_auth = run_oauth_flow; - conn->cleanup_async_auth = cleanup_oauth_flow; - state->async_ctx = request_copy; -+ -+ return true; - } -- else if (res < 0) -- { -+ -+ /* -+ * 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); -- goto fail; -- } - else -- { - libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)"); -- goto fail; -- } -- -- return true; - - fail: - do_cleanup(state, &request); ## src/test/modules/oauth_validator/meson.build ## @@ src/test/modules/oauth_validator/meson.build: test_install_libs += magic_validator @@ src/test/modules/oauth_validator/t/002_client.pl: sub test } test( -@@ src/test/modules/oauth_validator/t/002_client.pl: test( +@@ src/test/modules/oauth_validator/t/002_client.pl: $common_connstr = "$base_connstr oauth_issuer=$issuer oauth_client_id=myID"; # Make sure the v1 hook continues to work. test( "v1 synchronous hook can provide a token",