Re: [PoC] Federated Authn/z with OAUTHBEARER

On 4/27/23 10:35, Jacob Champion wrote:
> Moving forward, the first thing I plan to tackle is asynchronous
> operation, so that polling clients can still operate sanely. If I can
> find a good solution there, the conversations about possible extension
> points should get a lot easier.

Attached is patchset v8, now with concurrency and 300% more cURL! And
many more questions to answer.

This is a full reimplementation of the client-side OAuth flow. It's an
async-first engine built on top of cURL's multi handles. All pending
operations are multiplexed into a single epoll set (the "altsock"),
which is exposed through PQsocket() for the duration of the OAuth flow.
Clients return to the flow on their next call to PQconnectPoll().

Andrey and Mahendrakar: you'll probably be interested in the
conn->async_auth() callback, conn->altsock, and the pg_fe_run_oauth_flow
entry point. This is intended to be the foundation for alternative flows.

I've kept the blocking iddawc implementation for comparison, but if
you're running the tests against it, be aware that the asynchronous
tests will, predictably, hang. Skip them with `py.test -k 'not
asynchronous'`.

= The Good =

- PQconnectPoll() is no longer indefinitely blocked on a single
connection's OAuth handshake. (iddawc doesn't appear to have any
asynchronous primitives in its API, unless I've missed something crucial.)

- We now have a swappable entry point. Alternative flows could be
implemented by applications without forcing clients to redesign their
polling loops (PQconnect* should just work as expected).

- We have full control over corner cases in our default flow. Debugging
failures is much nicer, with explanations of exactly what has gone wrong
and where, compared to iddawc's "I_ERROR" messages.

- cURL is not a lightweight library by any means, but we're no longer
bundling things like web servers that we're not going to use.

= The Bad =

- Unsurprisingly, there's a lot more code now that we're implementing
the flow ourselves. The client patch has tripled in size, and we'd be on
the hook for implementing and staying current with the RFCs.

- The client implementation is currently epoll-/Linux-specific. I think
kqueue shouldn't be too much trouble for the BSDs, but it's even more
code to maintain.

- Some clients in the wild (psycopg2/psycopg) suppress all notifications
during PQconnectPoll(). To accommodate them, I no longer use the
noticeHooks for communicating the user code, but that means we have to
come up with some other way to let applications override the printing to
stderr. Something like the OpenSSL decryption callback, maybe?

= The Ugly =

- Unless someone is aware of some amazing Winsock magic, I'm pretty sure
the multiplexed-socket approach is dead in the water on Windows. I think
the strategy there probably has to be a background thread plus a fake
"self-pipe" (loopback socket) for polling... which may be controversial?

- We have to figure out how to initialize cURL in a thread-safe manner.
Newer versions of libcurl and OpenSSL improve upon this situation, but I
don't think there's a way to check at compile time whether the
initialization strategy is safe or not (and even at runtime, I think
there may be a chicken-and-egg problem with the API, where it's not safe
to check for thread-safe initialization until after you've safely
initialized).

= Next Steps =

There are so many TODOs in the cURL implementation: it's been a while
since I've done any libcurl programming, it all needs to be hardened,
and I need to comb through the relevant specs again. But I don't want to
gold-plate it if this overall approach is unacceptable. So, questions
for the gallery:

1) Would starting up a background thread (pooled or not) be acceptable
on Windows? Alternatively, does anyone know enough Winsock deep magic to
combine multiple pending events into one (selectable!) socket?

2) If a background thread is acceptable on one platform, does it make
more sense to use one on every platform and just have synchronous code
everywhere? Or should we use a threadless async implementation when we can?

3) Is the current conn->async_auth() entry point sufficient for an
application to implement the Microsoft flows discussed upthread?

4) Would we want to try to require a new enough cURL/OpenSSL to avoid
thread safety problems during initialization, or do we need to introduce
some API equivalent to PQinitOpenSSL?

5) Does this maintenance tradeoff (full control over the client vs. a
large amount of RFC-governed code) seem like it could be okay?

Thanks,
--Jacob