Re: [PoC] Federated Authn/z with OAUTHBEARER

Hi,

On 2025-02-20 09:28:36 -0800, Jacob Champion wrote:
> On Thu, Feb 20, 2025 at 8:30 AM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> > I spent a few more hours staring at this, and ran it through a number of CI and
> > local builds, without anything showing up. Pushed to master with the first set
> > of buildfarm animals showing green builds.
>
> Thank you!! And _huge thanks_ to everyone who's reviewed and provided
> feedback. I'm going to start working with Andrew on getting the new
> tests going in the buildfarm.

+1

One question about interruptability. The docs say:

+ <varlistentry>
+ <term>Interruptibility</term>
+ <listitem>
+ <para>
+ Modules must remain interruptible by signals so that the server can
+ correctly handle authentication timeouts and shutdown signals from
+ <application>pg_ctl</application>. For example, a module receiving
+ <symbol>EINTR</symbol>/<symbol>EAGAIN</symbol> from a blocking call
+ should call <function>CHECK_FOR_INTERRUPTS()</function> before retrying.
+ The same should be done during any long-running loops. Failure to follow
+ this guidance may result in unresponsive backend sessions.
+ </para>
+ </listitem>
+ </varlistentry>

Is EINTR checking really sufficient?

I don't think we can generally rely on all blocking system calls to be
interruptible by signals on all platforms?

And, probably worse, isn't relying on getting EINTR rather racy, due to the
chance of the signal arriving between CHECK_FOR_INTERRUPTS() and the blocking
system call?

Afaict the only real way to do safely across platforms is to never call
blocking functions, e.g. by using non-blocking sockets and waiting for
readiness using latches.

And a second one about supporting !CURL_VERSION_ASYNCHDNS:

Is it a good idea to support that? We e.g. rely on libpq connections made by
the backend to be interruptible. Admittedly that's, I think, already not
bulletproof, due to libpq's DNS lookups going through libc if connection
string contains a name that needs to be looked up, but this seems to make that
a bit worse? With the connection string the DNS lookups can at least be
avoided, not that I think we properly document that...

Greetings,

Andres