[PATCH] Pull general SASL framework out of SCRAM

From: Jacob Champion <pchampion(at)vmware(dot)com>
To: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Cc: "michael(at)paquier(dot)xyz" <michael(at)paquier(dot)xyz>
Subject: [PATCH] Pull general SASL framework out of SCRAM
Date: 2021-06-22 22:37:29
Message-ID: 3d2a6f5d50e741117d6baf83eb67ebf1a8a35a11.camel@vmware.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(This is split off from my work on OAUTHBEARER [1].)

Currently, the SASL logic is tightly coupled to the SCRAM
implementation. This patch untangles the two, by introducing callback
structs for both the frontend and backend.

In the original thread, Michael Paquier commented:

> + /* TODO: SASL_EXCHANGE_FAILURE with output is forbidden in SASL */
> if (result == SASL_EXCHANGE_SUCCESS)
> sendAuthRequest(port,
> AUTH_REQ_SASL_FIN,
> output,
> outputlen);
> Perhaps that's an issue we need to worry on its own? I didn't recall
> this part..

Yeah, it was non-obvious to me on the first read too. It's a
consequence of a couple parts of the SASL spec [2]:

> The protocol may include an optional additional data field in this
> outcome message. This field can only include additional data when
> the outcome is successful.

and

> SASL mechanism specifications MUST supply the following information:
>
> [...]
>
> b) An indication of whether the server is expected to provide
> additional data when indicating a successful outcome. If so,
> if the server sends the additional data as a challenge, the
> specification MUST indicate that the response to this challenge
> is an empty response.

There isn't a corresponding provision for data for a *failed* outcome,
so any such data would have to be sent as a separate, mechanism-
specific, challenge. This is what OAUTHBEARER has to do, with an
annoying "failure handshake".

(Note that our protocol implementation provides an "additional data"
field for the initial client response, but *not* for the authentication
outcome. That seems odd to me, but it is what it is, I suppose.)

Regarding that specific TODO -- I think it'd be good for the framework
to fail hard if a mechanism tries to send data during a failure
outcome, as it probably means the mechanism isn't implemented to spec.

--Jacob

[1] https://www.postgresql.org/message-id/d1b467a78e0e36ed85a09adf979d04cf124a9d4b.camel@vmware.com
[2] https://datatracker.ietf.org/doc/html/rfc4422

Attachment Content-Type Size
auth-generalize-SASL-mechanisms.patch text/x-patch 16.3 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2021-06-22 22:59:37 [PATCH] Make jsonapi usable from libpq
Previous Message Peter Geoghegan 2021-06-22 22:37:06 Re: disfavoring unparameterized nested loops