[PATCH] Pull general SASL framework out of SCRAM - Mailing list pgsql-hackers

From Jacob Champion
Subject [PATCH] Pull general SASL framework out of SCRAM
Date
Msg-id 3d2a6f5d50e741117d6baf83eb67ebf1a8a35a11.camel@vmware.com
Whole thread Raw
Responses Re: [PATCH] Pull general SASL framework out of SCRAM
List 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

pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: disfavoring unparameterized nested loops
Next
From: Jacob Champion
Date:
Subject: [PATCH] Make jsonapi usable from libpq