Thread: Re: pgsql: Add support for OAUTHBEARER SASL mechanism
> Add support for OAUTHBEARER SASL mechanism
Debian still has this experimental port with a GNU userland and a
FreeBSD kernel called kfreebsd. I don't expect anyone to particularly
care about it, but it found an actual bug:
/build/reproducible-path/postgresql-18-18~~devel.20250331/build/../src/interfaces/libpq/fe-auth-oauth-curl.c: In
function‘register_socket’:
/build/reproducible-path/postgresql-18-18~~devel.20250331/build/../src/interfaces/libpq/fe-auth-oauth-curl.c:1317:20:
error:‘actx’ undeclared (first use in this function); did you mean ‘ctx’?
1317 | actx_error(actx, "libpq does not support multiplexer sockets on this platform");
| ^~~~
This should not be a compile-time error; actx is not defined outside
the #ifdef blocks there:
/*
* Adds and removes sockets from the multiplexer set, as directed by the
* libcurl multi handle.
*/
static int
register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx,
void *socketp)
{
#ifdef HAVE_SYS_EPOLL_H
struct async_ctx *actx = ctx;
...
#endif
#ifdef HAVE_SYS_EVENT_H
struct async_ctx *actx = ctx;
...
#endif
actx_error(actx, "libpq does not support multiplexer sockets on this platform");
return -1;
}
https://buildd.debian.org/status/fetch.php?pkg=postgresql-18&arch=hurd-amd64&ver=18%7E%7Edevel.20250331-1&stamp=1743455288&raw=0
Christoph
Re: To Daniel Gustafsson > > Add support for OAUTHBEARER SASL mechanism > > Debian still has this experimental port with a GNU userland and a > FreeBSD kernel called kfreebsd. Sorry this part was nonsense, kfreebsd was actually terminated and obviously I didn't even read the port's name. The failing port (still experimental and care-is-optional) is hurd-amd64. The bug is the same, though. Christoph
On Mon, Mar 31, 2025 at 2:54 PM Christoph Berg <myon@debian.org> wrote: > > > Add support for OAUTHBEARER SASL mechanism > > Debian still has this experimental port with a GNU userland and a > FreeBSD kernel called kfreebsd. I don't expect anyone to particularly > care about it, but it found an actual bug: > > /build/reproducible-path/postgresql-18-18~~devel.20250331/build/../src/interfaces/libpq/fe-auth-oauth-curl.c: In function‘register_socket’: > /build/reproducible-path/postgresql-18-18~~devel.20250331/build/../src/interfaces/libpq/fe-auth-oauth-curl.c:1317:20: error:‘actx’ undeclared (first use in this function); did you mean ‘ctx’? > 1317 | actx_error(actx, "libpq does not support multiplexer sockets on this platform"); > | ^~~~ > > This should not be a compile-time error; actx is not defined outside > the #ifdef blocks there: Ah, sorry about that. Thank you for reporting it! (That means that Windows builds --with-libcurl are similarly broken, I think. Not that Windows packagers will want to use --with-libcurl -- it doesn't do anything -- but it should build.) I don't have hurd-amd64 to test, but I'm working on a patch that will build and pass tests if I manually munge pg_config.h. We were skipping the useless tests via a $windows_os check; I think I should use check_pg_config() instead. We could change how this works a bit for the proposed libpq-oauth.so plugin, and only build it if we have a workable implementation. I do like having these other platforms compile the Curl code, though, since we'd prefer to keep the build clean for a future Windows implementation... --Jacob
On Mon, Mar 31, 2025 at 4:09 PM Jacob Champion <jacob.champion@enterprisedb.com> wrote: > I don't have hurd-amd64 to test, but I'm working on a patch that will > build and pass tests if I manually munge pg_config.h. We were skipping > the useless tests via a $windows_os check; I think I should use > check_pg_config() instead. Proposed fix attached. Thanks, --Jacob
Attachment
Re: Jacob Champion > (That means that Windows builds --with-libcurl are similarly broken, I > think. Not that Windows packagers will want to use --with-libcurl -- > it doesn't do anything -- but it should build.) Does --with-libcurl still do anything useful if this feature test fails? From what you are saying, the answer is "no", and I can see more "not on this platform" error messages in other callbacks. This should be documented in doc/src/sgml/installation.sgml. > We could change how this works a bit for the proposed libpq-oauth.so > plugin, and only build it if we have a workable implementation. I do > like having these other platforms compile the Curl code, though, since > we'd prefer to keep the build clean for a future Windows > implementation... I would prefer to get an error from configure if the feature doesn't do anything on my platform. The current way is confusing. If future users of libcurl change that, the configure test can still be changed. With the libpq-oauth split, this makes even more sense because building a library that always throws an error isn't very useful. (Don't build that file at all if the feature doesn't work.) Since oauth/curl have some security implications, would it make more sense to call the switch --enable-oauth (-Doauth) so users could control better what features their libpq is going to have? Perhaps some other feature (pg_service as URL?) is going to need libcurl as well, but it should be configurable separately. Christoph
> On 1 Apr 2025, at 15:03, Christoph Berg <myon@debian.org> wrote: > With the libpq-oauth split, this makes even more sense because > building a library that always throws an error isn't very useful. > (Don't build that file at all if the feature doesn't work.) After the split, configure/meson should fail if the libcurl dependency isn't satisfied or if the platform isn't supported. > Since oauth/curl have some security implications, would it make more > sense to call the switch --enable-oauth (-Doauth) so users could > control better what features their libpq is going to have? Perhaps > some other feature (pg_service as URL?) is going to need libcurl as > well, but it should be configurable separately. Perhaps --with-oauth-client for the opt-in libpq-oauth? -- Daniel Gustafsson
> On 1 Apr 2025, at 02:06, Jacob Champion <jacob.champion@enterprisedb.com> wrote: > > On Mon, Mar 31, 2025 at 4:09 PM Jacob Champion > <jacob.champion@enterprisedb.com> wrote: >> I don't have hurd-amd64 to test, but I'm working on a patch that will >> build and pass tests if I manually munge pg_config.h. We were skipping >> the useless tests via a $windows_os check; I think I should use >> check_pg_config() instead. > > Proposed fix attached. Thanks, I agree that this is the right fix. While this is all subject to change, I will go ahead with this patch in the meantime to make the tree properly buildable. -- Daniel Gustafsson
On Tue, Apr 1, 2025 at 6:12 AM Daniel Gustafsson <daniel@yesql.se> wrote: > > > On 1 Apr 2025, at 15:03, Christoph Berg <myon@debian.org> wrote: > > > With the libpq-oauth split, this makes even more sense because > > building a library that always throws an error isn't very useful. > > (Don't build that file at all if the feature doesn't work.) > > After the split, configure/meson should fail if the libcurl dependency isn't > satisfied or if the platform isn't supported. Yeah, after sleeping on it I agree. If I want a "canary" buildfarm animal to opt into compilation on unsupported platforms, I can instead look into a manual #define or something; it doesn't have to be a supported configure-time thing. > > Since oauth/curl have some security implications, would it make more > > sense to call the switch --enable-oauth (-Doauth) so users could > > control better what features their libpq is going to have? Perhaps > > some other feature (pg_service as URL?) is going to need libcurl as > > well, but it should be configurable separately. > > Perhaps --with-oauth-client for the opt-in libpq-oauth? It started as -Doauth way back when, but was changed as part of the discussion at [1]. Peter, do you have any objections to switching back to an OAuth-related name? --Jacob [1] https://postgr.es/m/6bde5f56-9e7a-4148-b81c-eb6532cb3651%40eisentraut.org