Re: Add "password_protocol" connection parameter to libpq - Mailing list pgsql-hackers

From Jeff Davis
Subject Re: Add "password_protocol" connection parameter to libpq
Date
Msg-id 130e2ca6a081e8352d21d386efd54d7329849e45.camel@j-davis.com
Whole thread Raw
In response to Re: Add "password_protocol" connection parameter to libpq  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Add "password_protocol" connection parameter to libpq
List pgsql-hackers
On Tue, 2019-09-17 at 16:04 +0900, Michael Paquier wrote:
> basically maps into checking after FE_SCRAM_FINISHED.  Instead of
> those two flags, wouldn't it be cleaner to add an extra routine to
> fe-auth-scram.c which does the same sanity checks, say
> pg_fe_scram_check_state()?  This needs to be careful about three
> things, taking in input an opaque pointer to fe_scram_state if
> channel
> binding is required:
> - The data is not NULL.
> - The sasl mechanism selected is SCRAM-SHA-256-PLUS.
> - The state is FE_SCRAM_FINISHED.

Yes, I think this does come out a bit cleaner, thank you.

> What do you think?  There is no need to save down the connection
> parameter value into fe_scram_state.

I'm not sure I understand this comment, but I removed the extra boolean
flags.

> Nit here as there are only two mechanisms handled: I would rather
> cause the error if the selected mechanism does not match
> SCRAM-SHA-256-PLUS, instead of complaining if the selected mechanism
> matches SCRAM-SHA-256.  Either way is actually fine :)

Done.

> +    printfPQExpBuffer(&conn->errorMessage,
> +                      libpq_gettext("Channel binding required but
> not
> offered by server\n"));
> +                   result = false;
> Should that be "completed by server" instead?

Done.

> is required".  Or you have in mind that this error message is better?

I felt it would be a more useful error message.

> I think that pgindent would complain with the comment block in
> check_expected_areq(). 

Changed.

Regards,
    Jeff Davis


Attachment

pgsql-hackers by date:

Previous
From: Taylor Vesely
Date:
Subject: Re: Zedstore - compressed in-core columnar storage
Next
From: Michael Paquier
Date:
Subject: Re: subscriptionCheck failures on nightjar