Horton

Re: [PoC] Federated Authn/z with OAUTHBEARER

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Daniel Gustafsson <daniel@yesql.se>
Date: 2024-07-29T12:01:58Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. meson: Fix install-quiet after clean

  2. oauth: Run Autoconf tests with correct compiler flags

  3. Link libpq with libdl if the platform needs that.

  4. Doc: correct spelling of meson switch.

  5. oauth: Correct SSL dependency for libpq-oauth.a

  6. oauth: Fix Autoconf build on macOS

  7. oauth: Move the builtin flow into a separate module

  8. Remove a stray "pgrminclude" annotation

  9. oauth: Simplify copy of PGoauthBearerRequest

  10. oauth: Improve validator docs on interruptibility

  11. oauth: Disallow synchronous DNS in libcurl

  12. oauth: Fix postcondition for set_timer on macOS

  13. oauth: Use IPv4-only issuer in oauth_validator tests

  14. Work around OAuth/EVFILT_TIMER quirk on NetBSD.

  15. oauth: Fix incorrect const markers in struct

  16. Add missing entry to oauth_validator test .gitignore

  17. cirrus: Temporarily fix libcurl link error

  18. Add support for OAUTHBEARER SASL mechanism

  19. libpq: Handle asynchronous actions during SASL

  20. require_auth: prepare for multiple SASL mechanisms

  21. Move PG_MAX_AUTH_TOKEN_LENGTH to libpq/auth.h

  22. Make SASL max message length configurable

  23. jsonapi: fully initialize dummy lexer

  24. common/jsonapi: support libpq as a client

  25. Remove fe_memutils from libpgcommon_shlib

  26. Revert ECPG's use of pnstrdup()

  27. Explicitly require password for SCRAM exchange

  28. Refactor SASL exchange to return tri-state status

I have some comments about the first three patches, that deal with 
memory management.

v24-0001-Revert-ECPG-s-use-of-pnstrdup.patch

This looks right.

I suppose another approach would be to put a full replacement for 
strndup() into src/port/.  But since there is currently only one user, 
and most other users should be using pnstrdup(), the presented approach 
seems ok.

We should take the check for exit() calls from libpq and expand it to 
cover the other libraries as well.  Maybe there are other problems like 
this?


v24-0002-Remove-fe_memutils-from-libpgcommon_shlib.patch

I don't quite understand how this problem can arise.  The description says

"""
libpq appears to have no need for this, and the exit() references cause
our libpq-refs-stamp test to fail if the linker doesn't strip out the
unused code.
"""

But under what circumstances does "the linker doesn't strip out" happen? 
  If this happens accidentally, then we should have seen some buildfarm 
failures or something?

Also, one could look further and notice that restricted_token.c and 
sprompt.c both a) are not needed by libpq and b) can trigger exit() 
calls.  Then it's not clear why those are not affected.


v24-0003-common-jsonapi-support-libpq-as-a-client.patch

I'm reminded of thread [0].  I think there is quite a bit of confusion 
about the pqexpbuffer vs. stringinfo APIs, and they are probably used 
incorrectly quite a bit.  There are now also programs that use both of 
them!  This patch now introduces another layer on top of them.  I fear, 
at the end, nobody is going to understand any of this anymore.  Also, 
changing all the programs to link in libpq for pqexpbuffer seems like 
the opposite direction from what was suggested in [0].

I think we need to do some deeper thinking here about how we want the 
memory management on the client side to work.  Maybe we could just use 
one API but have some flags or callbacks to control the out-of-memory 
behavior.

[0]: 
https://www.postgresql.org/message-id/flat/16d0beac-a141-e5d3-60e9-323da75f49bf%40eisentraut.org