since-v6.diff.txt
text/plain
Filename: since-v6.diff.txt
Type: text/plain
Part: 0
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 <jacob.champion@enterprisedb.com>
## 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",