Re: Some thoughts about SCRAM implementation

On 12/04/17 18:38, Robert Haas wrote:
> On Tue, Apr 11, 2017 at 9:20 AM, Álvaro Hernández Tortosa
> <aht(at)8kdata(dot)com> wrote:
>> LOL. Do you really want a half-baked Java programmer writing a patch in
>> C for PostgreSQL? I once tried that and Magnus said my code was Java code
>> that happened to compile with a C compiler.... ^_^
>>
>> Having said that, I take the bait, I like challenges and putting my
>> words behind my code :)
> Excellent, because that's how stuff gets done around here. Saying
> that you want something and hoping other people will do it is fine,
> but being will to put some effort into it is a lot more likely to get
> it done. Not to be harsh, but showing up 3 days after feature freeze
> to complain that a feature pending commit for 14 months is missing
> something that you really need isn't really the right way to make
> something happen. I'm pretty sure that the lack of channel binding
> was discussed quite a bit earlier than now, so I think there was
> adequate opportunity to protest, contribute a patch, etc. It's not
> that I don't have sympathy for the way you feel about this: seeing
> features you care about fall out of a release sucks, and I've
> experienced a lot of that suckage quite recently, so the pain is fresh
> in my mind. But it's something we all have to live with for the
> overall good of the product. We used to not have a firm feature
> freeze, and that was way worse.

Hi Robert. Not harsh, no worries, but please note a couple of
things, let me give you a bit of context about my recent comments:

- I haven't complained about not having channel binding support. I was
just giving my own recommendations for the PostgreSQL community, in the
belief that contributing this opinion could let -hackes to make a more
informed decision about whether to include or not a given feature.

- I haven't said either that I need it. I don't use SCRAM, much less
with channel binding. Rather than looking after myself here, I'm trying
to sit on the foot of potential users and speak up for them. I might be
wrong, of course, but this is the only role I'm trying to play here.

- I know how PostgreSQL release cycles work and not meaning to hack them
anyway. I just thought raising the fact that something that I believe
might be requested by enterprise users was on the other hand a minor
addition to a feature, and thus a really high value-to-effort ratio.
Indeed, I bet adding channel binding is an order of magnitude easier
than adding saslprep (which is optional too, btw). Ultimately I'm happy
with SCRAM in PG 10, whether it has cbinding or not, as it is a great
addition and makes PG10 an even better software.

- Even though I don't really care about SCRAM, and without having any
prior knowledge about SCRAM, I volunteered some time ago to study SCRAM,
give a lightning talk about SCRAM and later write a client
implementation for the jdbc driver. And I have already devoted a very
fair amount of time in doing so, and will keep doing that until all code
is done. Code WIP is here FYI: https://github.com/ahachete/scram. So
it's not that I haven't already put my code behind my words.

- Given all that, I still want to volunteer to not only do the client
jdbc part and consequently help debugging the server implementation (as
I believe it is so far the only non-libpq implementation), but also try
to also stand by my words by writing channel binding code for PostgreSQL
server. This is a huge effort for me, I only did C programming on the
year 2001. But still, I want to help from my limited capabilities as
much as I can.

- Having thoroughly studied the RFC and companion documentation, I
thought I was in a knowledge position as to give some recommendations
that other hackers may not know about (unless a deep study of SCRAM
would have been done). That's it, recommendation, ideas.

>
> In this case, I think it is abundantly clear that SCRAM without
> channel binding is still a good feature.

I agree. I may have exaggerated before, downplaying SCRAM without
channel binding. I think it is great. Period. I also still think channel
binding is very small code addition yet provides even better value. I
apologize for not picking my previous words more carefully.

> One piece of evidence for
> that is that the standard uses the suffix -PLUS to denote the
> availability of channel binding. That clearly conveys that channel
> binding has value, but also that not having it does not make the whole
> thing useless. Otherwise, they would have required it to be part of
> every implementation, or they would have made you add -CRAPPY if you
> didn't have it. The discussion elsewhere on this thread has
> adequately underlined the value of what we've already got, so I won't
> further belabor the point here.
>
> Furthermore, I think that the state of this feature as it currently
> exists in the tree is actually kind of concerning. There are
> currently four open items pertaining to SCRAM at least two of which
> look to my mind an awful lot like stuff that should have ideally been
> handled pre-feature-freeze: \password support, and protocol
> negotiation. I'm grateful for the hard work that has gone into this
> feature, but these are pretty significant loose ends. \password
> support is a basic usability issue. Protocol negotiation affects
> anyone who may want to make their PG driver work with this feature,
> and certainly can't be changed after final release, and ideally not
> even after beta. We really, really need to get that stuff nailed down
> ASAP or we're going to have big problems. So I think we should focus
> on those things, not this.

Absolutely agreed. That's why I have also tried to give all my
comments (from the perspective of the SCRAM jdbc driver implementator)
as to the protocol asap, and will continue to do so.

Thanks,

Álvaro

--

Álvaro Hernández Tortosa

-----------
<8K>data