Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-13 Thread Michael Paquier
On Tue, Jul 13, 2021 at 10:41:01PM +, Jacob Champion wrote: > Just to make sure -- do we want to export the fe-auth-sasl.h header as > opposed to forward-declaring the pg_fe_sasl_mech struct? Installing fe-auth-sasl.h has the advantage to make the internals of the callbacks available to applic

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-13 Thread Jacob Champion
On Tue, 2021-07-13 at 22:41 +, Jacob Champion wrote: > On Tue, 2021-07-13 at 19:31 +0900, Michael Paquier wrote: > > On Tue, Jul 13, 2021 at 12:41:27PM +0300, Mikhail Kulagin wrote: > > > > > > I think the new fe-auth-sasl.h file should be installed too. > > > Correction proposal in the attach

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-13 Thread Jacob Champion
On Tue, 2021-07-13 at 19:31 +0900, Michael Paquier wrote: > On Tue, Jul 13, 2021 at 12:41:27PM +0300, Mikhail Kulagin wrote: > > I got an error while building one of the extensions. > > /home/mkulagin/pg-install/postgresql-master/include/internal/libpq-int.h:44:10: > > fatal error: fe-auth-sasl.h:

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-13 Thread Michael Paquier
On Tue, Jul 13, 2021 at 12:41:27PM +0300, Mikhail Kulagin wrote: > I got an error while building one of the extensions. > /home/mkulagin/pg-install/postgresql-master/include/internal/libpq-int.h:44:10: > fatal error: fe-auth-sasl.h: No such file or directory > #include "fe-auth-s

RE: [PATCH] Pull general SASL framework out of SCRAM

2021-07-13 Thread Mikhail Kulagin
Hello, hackers! I got an error while building one of the extensions. /home/mkulagin/pg-install/postgresql-master/include/internal/libpq-int.h:44:10: fatal error: fe-auth-sasl.h: No such file or directory #include "fe-auth-sasl.h" ^~~~ I

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-12 Thread Michael Paquier
On Tue, Jul 13, 2021 at 12:01:46AM +, Jacob Champion wrote: > Ah, right. I think the (!done && !success) case is probably indicative > of an API smell, but that's probably something to clean up in a future > pass. Yeah, agreed. I feel that it would should be cleaner to replace those two boole

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-12 Thread Jacob Champion
On Sun, 2021-07-11 at 13:16 +0900, Michael Paquier wrote: > On Fri, Jul 09, 2021 at 11:31:48PM +, Jacob Champion wrote: > > On Thu, 2021-07-08 at 16:27 +0900, Michael Paquier wrote: > > > + * outputlen: The length (0 or higher) of the client response > > > buffer, > > > + *

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-10 Thread Michael Paquier
On Fri, Jul 09, 2021 at 11:31:48PM +, Jacob Champion wrote: > On Thu, 2021-07-08 at 16:27 +0900, Michael Paquier wrote: >> + * outputlen: The length (0 or higher) of the client response >> buffer, >> + * invalid if output is NULL. > > nitpick: maybe "ignor

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-09 Thread Jacob Champion
On Thu, 2021-07-08 at 16:27 +0900, Michael Paquier wrote: > I agree that this looks like an improvement in terms of the > expectations behind a SASL mechanism, so I have done the attached to > strengthen a bit all those checks. However, I don't really see a > point in back-patching any of that, as

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-08 Thread Michael Paquier
On Wed, Jul 07, 2021 at 03:07:14PM +, Jacob Champion wrote: > That's correct. But the client may not simply ignore the challenge and > keep the exchange open waiting for a new one, as pg_SASL_continue() > currently allows. That's what my TODO is referring to. I have been looking more at your t

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-07 Thread Jacob Champion
On Wed, 2021-07-07 at 14:08 +0900, Michael Paquier wrote: > On Tue, Jul 06, 2021 at 06:20:49PM +, Jacob Champion wrote: > > On Mon, 2021-07-05 at 17:17 +0900, Michael Paquier wrote: > > > > > Hmm. Does the RFCs tell us anything about that? > > > > Just in general terms: > > > > >Each au

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-06 Thread Michael Paquier
On Tue, Jul 06, 2021 at 06:20:49PM +, Jacob Champion wrote: > On Mon, 2021-07-05 at 17:17 +0900, Michael Paquier wrote: > Each name must be null-terminated, not just null-separated. That way > the list of names ends with an empty string: > > name-one\0 <- added by the mechanis

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-06 Thread Jacob Champion
On Mon, 2021-07-05 at 17:17 +0900, Michael Paquier wrote: > On Wed, Jun 30, 2021 at 10:30:12PM +, Jacob Champion wrote: > > Done in v3, with a second patch for the code motion. > > I have gone through that, tweaking the documentation you have added as > that's the meat of the patch, reworking

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-07-05 Thread Michael Paquier
On Wed, Jun 30, 2021 at 10:30:12PM +, Jacob Champion wrote: > Done in v3, with a second patch for the code motion. I have gone through that, tweaking the documentation you have added as that's the meat of the patch, reworking a bit the declarations of the callbacks (no need for several typedef

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-06-30 Thread Jacob Champion
On Sat, 2021-06-26 at 09:47 +0900, Michael Paquier wrote: > On Fri, Jun 25, 2021 at 11:40:33PM +, Jacob Champion wrote: > > I can definitely move it (into, say, auth-sasl.c?). I'll probably do > > that in a second commit, though, since keeping it in place during the > > refactor makes the revie

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-06-25 Thread Michael Paquier
On Fri, Jun 25, 2021 at 11:40:33PM +, Jacob Champion wrote: > I can definitely move it (into, say, auth-sasl.c?). I'll probably do > that in a second commit, though, since keeping it in place during the > refactor makes the review easier IMO. auth-sasl.c is a name consistent with the existing

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-06-25 Thread Jacob Champion
On Wed, 2021-06-23 at 16:38 +0900, Michael Paquier wrote: > On Tue, Jun 22, 2021 at 10:37:29PM +, Jacob Champion wrote: > > 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 b

Re: [PATCH] Pull general SASL framework out of SCRAM

2021-06-23 Thread Michael Paquier
On Tue, Jun 22, 2021 at 10:37:29PM +, Jacob Champion wrote: > 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. The approach to define and have a set callbacks feels nat

[PATCH] Pull general SASL framework out of SCRAM

2021-06-22 Thread Jacob Champion
(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