Re: [HACKERS] Some thoughts about SCRAM implementation

2017-05-22 Thread Michael Paquier 
On Mon, Apr 10, 2017 at 6:39 PM, Álvaro Hernández Tortosa
 wrote:
> - I think channel binding support should be added. SCRAM brings security
> improvements over md5 and other simpler digest algorithms. But where it
> really shines is together with channel binding. This is the only method to
> prevent MITM attacks. Implementing it should not very difficult. There are
> several possible channel binding mechanisms, but the mandatory and probably
> more typical one is 'tls-unique' which basically means getting the byte
> array from the TLSfinish() message and comparing it with the same data sent
> by the client. That's more or less all it takes to implement it. So I would
> go for it.

I was just looking at that during a long flight, and OpenSSL offers
SSL_get_finished() and SSL_get_peer_finished() to get the Finished
message data. So that's a matter of saving it in the client, encode it
in base64 (the spec is clear about that) and send the data as part of
the first challenge response to the server that just compares both
contents. Of course the protocol list and the first client message
need to be extended as well, but most of the facility is already
there. The spec is also clear about the lines to follow should the
client and/or server be built without OpenSSL (aka no channel binding
support).
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-05-05 Thread Heikki Linnakangas 

On 04/10/2017 09:28 PM, Álvaro Hernández Tortosa wrote:

On 10/04/17 13:02, Heikki Linnakangas wrote:

On 04/10/2017 12:39 PM, Álvaro Hernández Tortosa wrote:

* The nonce length is not specified by the RFC. I see typical
implementations use 24 chars for the client and 18 for the server.
Current code uses 10. I think it should not hurt making it at least 16
or 18.


Wouldn't hurt, I guess. IIRC I checked some other implementations,
when I picked 10, but I don't remember which ones anymore. Got a
reference for 24/18?


 First reference is the RFC example itself (non-mandatory, of
course). But then I saw many followed this. As a quick example, GNU SASL
defines:

#define SNONCE_ENTROPY_BYTES 18
https://www.gnu.org/software/gsasl/coverage/lib/scram/server.c.gcov.frameset.html


Ok, I bumped up the nonce lengths to 18 raw bytes. Thanks!

- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-15 Thread Greg Stark 
On 14 April 2017 at 20:20, Peter Eisentraut
 wrote:
>
> Yeah, I think if you're concerned about MITM then you would also be
> concerned about MITM siphoning off your data.  So you should be using
> TLS and then you don't need channel binding.

No. You can use TLS for authentication (by verifying SSL certs in both
directions) in which case TLS will protect against MITM for you. But
if you only use TLS for encryption but still want to use passwords for
authentication then there's no protection against MITM as you don't
know that the party doing the encryption is the same as the one you
authenticated to.

Channel binding is all about tying the authentication mechanism to the
encryption to guarantee that the party doing the encryption is the
same as the party you authenticated to. Otherwise someone could MITM
the TLS connection and relay the raw bytes of of the scram
negotiation. Under our md5 auth that gives them your password, under
scram they won't get the password which is a huge improvement but they
would still have the raw unencrypted data from your traffic.

-- 
greg


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-14 Thread Peter Eisentraut 
On 4/11/17 09:03, Magnus Hagander wrote:
> I would expect most enterprise customers who care about MITM protection
> are already using either TLS or ipsec to cover that already. They have
> benefit from the other parts of SCRAM, but they've already solved those
> problems.

Yeah, I think if you're concerned about MITM then you would also be
concerned about MITM siphoning off your data.  So you should be using
TLS and then you don't need channel binding.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Michael Paquier 
On Thu, Apr 13, 2017 at 3:21 AM, Robert Haas  wrote:
> On Wed, Apr 12, 2017 at 2:09 PM, Heikki Linnakangas  wrote:
>> Yes, we need to nail down the protocol and \password before beta. I am
>> working on them now.
>
> Good to hear.

FWIW, there are patches for each issue.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Robert Haas 
On Wed, Apr 12, 2017 at 2:09 PM, Heikki Linnakangas  wrote:
> That is very much appreciated! You writing a second implementation of the
> client-side support (libpq being the first) is very, very helpful, to
> validate that the protocol is sane, unambiguous, and adequately documented.

+1.

> Yes, we need to nail down the protocol and \password before beta. I am
> working on them now.

Good to hear.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Heikki Linnakangas 

On 04/12/2017 08:38 PM, Álvaro Hernández Tortosa wrote:

- 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.


That is very much appreciated! You writing a second implementation of 
the client-side support (libpq being the first) is very, very helpful, 
to validate that the protocol is sane, unambiguous, and adequately 
documented.



On 12/04/17 18:38, Robert Haas wrote:

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.


Yes, we need to nail down the protocol and \password before beta. I am 
working on them now.


- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Álvaro Hernández Tortosa 



On 12/04/17 18:38, Robert Haas wrote:

On Tue, Apr 11, 2017 at 9:20 AM, Álvaro Hernández Tortosa
 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

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Álvaro Hernández Tortosa 



On 12/04/17 18:09, Tom Lane wrote:

Heikki Linnakangas  writes:

On 04/12/2017 06:26 PM, Bruce Momjian wrote:

How does it do that?

Good question, crypto magic? I don't know the details, but the basic
idea is that you extract a blob of data that uniquely identifies the TLS
connection. Using some OpenSSL functions, in this case. I think it's a
hash of some of the TLS handshake messages that were used when the TLS
connection was established (that's what "tls-unique" means). That data
is then incorporated in the hash calculations of the SCRAM
authentication. If the client and the server are not speaking over the
same TLS connection, they will use different values for the TLS data,
and the SCRAM computations will not match, and you get an authentication
failure.


I believe the above is not correct. Channel binding data is *not* 
used for any hash computations. It is simply a byte array that is 
received as an extra user parameter, and the server then gets it by its 
own way (its own TLS data) and do a byte comparison. That's what the 
RFCs say about it.

... which the user can't tell apart from having fat-fingered the password,
I suppose?  Doesn't sound terribly friendly.  A report of a certificate
mismatch is far more likely to lead people to realize there's a MITM.


So given what I said before, that won't happen. Indeed, SCRAM RFC 
contains a specific error code for this: "channel-bindings-dont-match".



Álvaro


--

Álvaro Hernández Tortosa


---
<8K>data



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Tom Lane 
Stephen Frost  writes:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> ... which the user can't tell apart from having fat-fingered the password,
>> I suppose?  Doesn't sound terribly friendly.  A report of a certificate
>> mismatch is far more likely to lead people to realize there's a MITM.

> We might be able to improve on that.

I'd sure be happier about this being a useful feature if we can.  But
unless someone has a design for that ready-to-go, that's another good
reason to put it off.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Stephen Frost 
Tom, all,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> ... which the user can't tell apart from having fat-fingered the password,
> I suppose?  Doesn't sound terribly friendly.  A report of a certificate
> mismatch is far more likely to lead people to realize there's a MITM.

We might be able to improve on that.

> So this seems more like a hack than like a feature we need so desperately
> as to push it into v10 post-freeze.

Channel binding certainly isn't a 'hack' and is something we should
support, but I agree that it doesn't need to go into v10.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Robert Haas 
On Tue, Apr 11, 2017 at 9:20 AM, Álvaro Hernández Tortosa
 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.

In this case, I think it is abundantly clear that SCRAM without
channel binding is still a good feature.  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.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Tom Lane 
Heikki Linnakangas  writes:
> On 04/12/2017 06:26 PM, Bruce Momjian wrote:
>> How does it do that?

> Good question, crypto magic? I don't know the details, but the basic 
> idea is that you extract a blob of data that uniquely identifies the TLS 
> connection. Using some OpenSSL functions, in this case. I think it's a 
> hash of some of the TLS handshake messages that were used when the TLS 
> connection was established (that's what "tls-unique" means). That data 
> is then incorporated in the hash calculations of the SCRAM 
> authentication. If the client and the server are not speaking over the 
> same TLS connection, they will use different values for the TLS data, 
> and the SCRAM computations will not match, and you get an authentication 
> failure.

... which the user can't tell apart from having fat-fingered the password,
I suppose?  Doesn't sound terribly friendly.  A report of a certificate
mismatch is far more likely to lead people to realize there's a MITM.

So this seems more like a hack than like a feature we need so desperately
as to push it into v10 post-freeze.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Heikki Linnakangas 

On 04/12/2017 06:26 PM, Bruce Momjian wrote:

On Wed, Apr 12, 2017 at 12:13:03PM +0300, Heikki Linnakangas wrote:

That said, I stand by my comment that I don't think it's the enterprises
that need or want the channel binding. If they care about it, they have
already put certificate validation in place, and it won't buy them anything.

Because channel binding also only secures the authentication (SCRAM), not
the actual contents and commands that are then sent across the channel,
AFAIK?


TLS protects the contents and the commands. The point of channel binding is
to defeat a MITM attack, where the client connects to a malicious server,
using TLS, which then connects to the real server, using another TLS
connection. Channel binding will detect that the client and the real server
are not communicating over the same TLS connection, but two different TLS
connections, and make the authentication fail.

SSL certificates, with validation, achieves the same, but channel binding
achieves it without the hassle of certificates.


How does it do that?


Good question, crypto magic? I don't know the details, but the basic 
idea is that you extract a blob of data that uniquely identifies the TLS 
connection. Using some OpenSSL functions, in this case. I think it's a 
hash of some of the TLS handshake messages that were used when the TLS 
connection was established (that's what "tls-unique" means). That data 
is then incorporated in the hash calculations of the SCRAM 
authentication. If the client and the server are not speaking over the 
same TLS connection, they will use different values for the TLS data, 
and the SCRAM computations will not match, and you get an authentication 
failure.


- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Bruce Momjian 
On Wed, Apr 12, 2017 at 12:13:03PM +0300, Heikki Linnakangas wrote:
> >That said, I stand by my comment that I don't think it's the enterprises
> >that need or want the channel binding. If they care about it, they have
> >already put certificate validation in place, and it won't buy them anything.
> >
> >Because channel binding also only secures the authentication (SCRAM), not
> >the actual contents and commands that are then sent across the channel,
> >AFAIK?
> 
> TLS protects the contents and the commands. The point of channel binding is
> to defeat a MITM attack, where the client connects to a malicious server,
> using TLS, which then connects to the real server, using another TLS
> connection. Channel binding will detect that the client and the real server
> are not communicating over the same TLS connection, but two different TLS
> connections, and make the authentication fail.
> 
> SSL certificates, with validation, achieves the same, but channel binding
> achieves it without the hassle of certificates.

How does it do that?

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Craig Ringer 
On 12 Apr. 2017 17:27, "Magnus Hagander"  wrote:



On Wed, Apr 12, 2017 at 11:13 AM, Heikki Linnakangas 
wrote:

> On 04/12/2017 11:22 AM, Magnus Hagander wrote:
>
>> On Wed, Apr 12, 2017 at 3:25 AM, Bruce Momjian  wrote:
>>
>> And which enterprises are using SSL without certificates?  And I thought
>>> channel binding required certificates anyway, e.g.:
>>>
>>> https://en.wikipedia.org/wiki/Salted_Challenge_Response_
>>> Authentication_Mechanism#Channel_binding
>>>
>>> For instance, for the tls-server-end-point channel binding, it is
>>> the
>>> server's TLS certificate.
>>>
>>
>> AFAIK it does require the TLS certifificates, but it does not require TLS
>> certificate *validation*. You can use channel binding with just
>> self-signed
>> certs.
>>
>
> tls-server-end-point channel binding type relies on certificates. But
> SCRAM uses "tls-unique" by default, and it does not use certificates. It's
> a bit weird that the wikipedia article uses tls-server-end-point as the
> example, I don't know why anyone would use tls-server-end-point with SCRAM.


Interesting. But we don't support TLS without certificates, do we? We
support it without client certificates, but we need a server certificate.
So the TLS connection itself still relies on the certificates, justn ot the
channel binding.


Actually, by default we just use the server certificate as a public key
container. libpq pretty much ignores the rest of the certificate's ASN.1
data (or rather, doesn't ask OpenSSL to care about it). It doesn't do any
trust chain checking to find a path to a root cert. It doesn't check the
host name/IP. It doesn't remember the key for later connections, SSH
known-hosts style.

You can freely MITM a sslmode=require libpq connection with your own self
signed cert based proxy and nobody will be the wiser. It provides only
transport encryption. Or just have your MITM not offer SSL; libpq silently
degrades to unencrypted in its default sslmode=prefer mode :(

If you use sslmode=verify-full then you get the behaviour you'd normally
expect - validation of server key against local trusted cert, and
ip/hostname check.

Even then we don't do any sort of smart cert trust path finding like libnss
or Java's JSSE do. We just look at the cert file and expect it to have
signed the server's cert. But at least we can do some kind of validation.
It's painful if you talk to multiple different servers with different
certs, but it works.

I'm not taking a position on the urgency of channel binding by pointing
this out. Just trying to set out current behaviour and limitations.

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Magnus Hagander 
On Wed, Apr 12, 2017 at 11:13 AM, Heikki Linnakangas 
wrote:

> On 04/12/2017 11:22 AM, Magnus Hagander wrote:
>
>> On Wed, Apr 12, 2017 at 3:25 AM, Bruce Momjian  wrote:
>>
>> And which enterprises are using SSL without certificates?  And I thought
>>> channel binding required certificates anyway, e.g.:
>>>
>>> https://en.wikipedia.org/wiki/Salted_Challenge_Response_
>>> Authentication_Mechanism#Channel_binding
>>>
>>> For instance, for the tls-server-end-point channel binding, it is
>>> the
>>> server's TLS certificate.
>>>
>>
>> AFAIK it does require the TLS certifificates, but it does not require TLS
>> certificate *validation*. You can use channel binding with just
>> self-signed
>> certs.
>>
>
> tls-server-end-point channel binding type relies on certificates. But
> SCRAM uses "tls-unique" by default, and it does not use certificates. It's
> a bit weird that the wikipedia article uses tls-server-end-point as the
> example, I don't know why anyone would use tls-server-end-point with SCRAM.


Interesting. But we don't support TLS without certificates, do we? We
support it without client certificates, but we need a server certificate.
So the TLS connection itself still relies on the certificates, justn ot the
channel binding.



> That said, I stand by my comment that I don't think it's the enterprises
>> that need or want the channel binding. If they care about it, they have
>> already put certificate validation in place, and it won't buy them
>> anything.
>>
>> Because channel binding also only secures the authentication (SCRAM), not
>> the actual contents and commands that are then sent across the channel,
>> AFAIK?
>>
>
> TLS protects the contents and the commands. The point of channel binding
> is to defeat a MITM attack, where the client connects to a malicious
> server, using TLS, which then connects to the real server, using another
> TLS connection. Channel binding will detect that the client and the real
> server are not communicating over the same TLS connection, but two
> different TLS connections, and make the authentication fail.
>
> SSL certificates, with validation, achieves the same, but channel binding
> achieves it without the hassle of certificates.


Right. It also achieves some more things, but definitely with more hassle.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Heikki Linnakangas 

On 04/12/2017 11:22 AM, Magnus Hagander wrote:

On Wed, Apr 12, 2017 at 3:25 AM, Bruce Momjian  wrote:


And which enterprises are using SSL without certificates?  And I thought
channel binding required certificates anyway, e.g.:

https://en.wikipedia.org/wiki/Salted_Challenge_Response_
Authentication_Mechanism#Channel_binding

For instance, for the tls-server-end-point channel binding, it is
the
server's TLS certificate.


AFAIK it does require the TLS certifificates, but it does not require TLS
certificate *validation*. You can use channel binding with just self-signed
certs.


tls-server-end-point channel binding type relies on certificates. But 
SCRAM uses "tls-unique" by default, and it does not use certificates. 
It's a bit weird that the wikipedia article uses tls-server-end-point as 
the example, I don't know why anyone would use tls-server-end-point with 
SCRAM.



That said, I stand by my comment that I don't think it's the enterprises
that need or want the channel binding. If they care about it, they have
already put certificate validation in place, and it won't buy them anything.

Because channel binding also only secures the authentication (SCRAM), not
the actual contents and commands that are then sent across the channel,
AFAIK?


TLS protects the contents and the commands. The point of channel binding 
is to defeat a MITM attack, where the client connects to a malicious 
server, using TLS, which then connects to the real server, using another 
TLS connection. Channel binding will detect that the client and the real 
server are not communicating over the same TLS connection, but two 
different TLS connections, and make the authentication fail.


SSL certificates, with validation, achieves the same, but channel 
binding achieves it without the hassle of certificates.


- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-12 Thread Magnus Hagander 
On Wed, Apr 12, 2017 at 3:25 AM, Bruce Momjian  wrote:

> On Tue, Apr 11, 2017 at 08:58:30PM -0400, Stephen Frost wrote:
> > > > But just a bit more is needed to make it really a big
> announcement and
> > > > provide real value to (I guess, mostly but very interesting)
> enterprise
> > > > customers, for which MITM and impersonating are big things. The good
> news is
> > > > that adding channel binding is like inverse Paretto: a 20% of extra
> effort
> > > > (I bet significantly less) leads to 80% improvement.
> > >
> > > I don't see why channel binding is a big deal for enterprises because I
> > > assume they are already using SSL:
> >
> > Channel binding should be used with SSL to ensure that there is no
> > man-in-the-middle attack being performed.  It's necessary when the
> > end-points aren't performing full, mutual, certificate-based
> > verification.
>
> And which enterprises are using SSL without certificates?  And I thought
> channel binding required certificates anyway, e.g.:
>
> https://en.wikipedia.org/wiki/Salted_Challenge_Response_
> Authentication_Mechanism#Channel_binding
>
> For instance, for the tls-server-end-point channel binding, it is
> the
> server's TLS certificate.
>

AFAIK it does require the TLS certifificates, but it does not require TLS
certificate *validation*. You can use channel binding with just self-signed
certs.

That said, I stand by my comment that I don't think it's the enterprises
that need or want the channel binding. If they care about it, they have
already put certificate validation in place, and it won't buy them anything.

Because channel binding also only secures the authentication (SCRAM), not
the actual contents and commands that are then sent across the channel,
AFAIK?



> > > I think the big win for SCRAM is the inability to replay md5 packets
> > > after recording 16k sessions (salt was only 32-bit, so a 50% chance of
> > > replay after 16 sessions), and storage of SHA256 hashes instead of MD5
> > > in pg_authid, though the value of that is mostly a check-box item
> > > because collisions are not a problem for the way we use MD5.
> >
> > There are a lot of wins to having SCRAM implemented.  I disagree
> > strongly that securing PG from attacks based on latent information
> > gathering (backups which include pg_authid) is just a "check-box" item.
>
> Well, they have the entire backup so I don't think cracking the password
> is a huge win, though the password does potentially give them access to
> future data.  And it prevents them from reusing the password on another
> server, but _again_, I still think the big win is to prevent replay
> after 16k packets are sniffed.
>

In particular in multi-installation systems, it's quite likely that people
at least use the same password across multiple postgres instances for
example and it would help against those.

It would also help against somebody who stole a backup, and then wants to
use that hash to log into the production system and *modify* things. From
the backup they can read all the data, but they can't modify anything --
but with  a replayable hash, they can connect and modify data. If the
superuser has a password they can also use that password to escalate to OS
level privileges.

I think these are both big wins. And both of them more important than
channel binding.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-11 Thread Bruce Momjian 
On Tue, Apr 11, 2017 at 08:58:30PM -0400, Stephen Frost wrote:
> > > But just a bit more is needed to make it really a big announcement and
> > > provide real value to (I guess, mostly but very interesting) enterprise
> > > customers, for which MITM and impersonating are big things. The good news 
> > > is
> > > that adding channel binding is like inverse Paretto: a 20% of extra effort
> > > (I bet significantly less) leads to 80% improvement.
> > 
> > I don't see why channel binding is a big deal for enterprises because I
> > assume they are already using SSL:
> 
> Channel binding should be used with SSL to ensure that there is no
> man-in-the-middle attack being performed.  It's necessary when the
> end-points aren't performing full, mutual, certificate-based
> verification.

And which enterprises are using SSL without certificates?  And I thought
channel binding required certificates anyway, e.g.:


https://en.wikipedia.org/wiki/Salted_Challenge_Response_Authentication_Mechanism#Channel_binding

For instance, for the tls-server-end-point channel binding, it is the
server's TLS certificate.

> > I think the big win for SCRAM is the inability to replay md5 packets
> > after recording 16k sessions (salt was only 32-bit, so a 50% chance of
> > replay after 16 sessions), and storage of SHA256 hashes instead of MD5
> > in pg_authid, though the value of that is mostly a check-box item
> > because collisions are not a problem for the way we use MD5.
> 
> There are a lot of wins to having SCRAM implemented.  I disagree
> strongly that securing PG from attacks based on latent information
> gathering (backups which include pg_authid) is just a "check-box" item.

Well, they have the entire backup so I don't think cracking the password
is a huge win, though the password does potentially give them access to
future data.  And it prevents them from reusing the password on another
server, but _again_, I still think the big win is to prevent replay
after 16k packets are sniffed.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-11 Thread Stephen Frost 
Bruce,

* Bruce Momjian (br...@momjian.us) wrote:
> On Tue, Apr 11, 2017 at 02:53:24PM +0200, Álvaro Hernández Tortosa wrote:
> > Let's put ourselves on the foot of potential users. Why would anyone
> > want to use SCRAM? What for? The hashing mechanism is better, no question.
> > And bring some added benefits, true. So its "better". But the real gain
> > comes from using channel binding, which avoids impersonation, MITM attacks.
> > This is the deal breaker. SCRAM without channel binding is like Coke Zero
> > without caffeine and mixed with water. Don't get me wrong, the work behind
> > is great.
> > 
> > But just a bit more is needed to make it really a big announcement and
> > provide real value to (I guess, mostly but very interesting) enterprise
> > customers, for which MITM and impersonating are big things. The good news is
> > that adding channel binding is like inverse Paretto: a 20% of extra effort
> > (I bet significantly less) leads to 80% improvement.
> 
> I don't see why channel binding is a big deal for enterprises because I
> assume they are already using SSL:

Channel binding should be used with SSL to ensure that there is no
man-in-the-middle attack being performed.  It's necessary when the
end-points aren't performing full, mutual, certificate-based
verification.

> I think the big win for SCRAM is the inability to replay md5 packets
> after recording 16k sessions (salt was only 32-bit, so a 50% chance of
> replay after 16 sessions), and storage of SHA256 hashes instead of MD5
> in pg_authid, though the value of that is mostly a check-box item
> because collisions are not a problem for the way we use MD5.

There are a lot of wins to having SCRAM implemented.  I disagree
strongly that securing PG from attacks based on latent information
gathering (backups which include pg_authid) is just a "check-box" item.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-11 Thread Bruce Momjian 
On Tue, Apr 11, 2017 at 02:53:24PM +0200, Álvaro Hernández Tortosa wrote:
> Let's put ourselves on the foot of potential users. Why would anyone
> want to use SCRAM? What for? The hashing mechanism is better, no question.
> And bring some added benefits, true. So its "better". But the real gain
> comes from using channel binding, which avoids impersonation, MITM attacks.
> This is the deal breaker. SCRAM without channel binding is like Coke Zero
> without caffeine and mixed with water. Don't get me wrong, the work behind
> is great.
> 
> But just a bit more is needed to make it really a big announcement and
> provide real value to (I guess, mostly but very interesting) enterprise
> customers, for which MITM and impersonating are big things. The good news is
> that adding channel binding is like inverse Paretto: a 20% of extra effort
> (I bet significantly less) leads to 80% improvement.

I don't see why channel binding is a big deal for enterprises because I
assume they are already using SSL:


https://en.wikipedia.org/wiki/Salted_Challenge_Response_Authentication_Mechanism#Channel_binding

I think the big win for SCRAM is the inability to replay md5 packets
after recording 16k sessions (salt was only 32-bit, so a 50% chance of
replay after 16 sessions), and storage of SHA256 hashes instead of MD5
in pg_authid, though the value of that is mostly a check-box item
because collisions are not a problem for the way we use MD5.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-11 Thread Álvaro Hernández Tortosa 



On 11/04/17 15:18, Heikki Linnakangas wrote:

On 04/11/2017 04:09 PM, Álvaro Hernández Tortosa wrote:

 But I will conserve my remaining courage (thanks Michael!) credits
for future threads ;) I have stated my opinion clearly, I will now go
back to the client library.


Once you're done with the client library, feel free to post a patch to 
implement channel binding. If it really is trivial, we can discuss it. 
Most likely, it won't go into v10, but someone's got to write the 
patch for v11 anyway, and until there's a patch on the table to 
discuss, there's no point in arguing. Yes, I'm trying to cajole you 
into writing the patch for v11, so that I don't have to :-).


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 :)



Álvaro

--

Álvaro Hernández Tortosa


---
<8K>data



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-11 Thread Heikki Linnakangas 

On 04/11/2017 04:09 PM, Álvaro Hernández Tortosa wrote:

 But I will conserve my remaining courage (thanks Michael!) credits
for future threads ;) I have stated my opinion clearly, I will now go
back to the client library.


Once you're done with the client library, feel free to post a patch to 
implement channel binding. If it really is trivial, we can discuss it. 
Most likely, it won't go into v10, but someone's got to write the patch 
for v11 anyway, and until there's a patch on the table to discuss, 
there's no point in arguing. Yes, I'm trying to cajole you into writing 
the patch for v11, so that I don't have to :-).


- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-11 Thread Álvaro Hernández Tortosa 



On 11/04/17 15:03, Magnus Hagander wrote:
On Tue, Apr 11, 2017 at 2:53 PM, Álvaro Hernández Tortosa 
mailto:a...@8kdata.com>> wrote:




On 10/04/17 20:32, Andres Freund wrote:

On 2017-04-10 20:28:27 +0200, Álvaro Hernández Tortosa wrote:


On 10/04/17 13:02, Heikki Linnakangas wrote:

On 04/10/2017 12:39 PM, Álvaro Hernández Tortosa wrote:

- I think channel binding support should be added.
SCRAM brings security
improvements over md5 and other simpler digest
algorithms. But where it
really shines is together with channel binding.
This is the only method
to prevent MITM attacks. Implementing it should
not very difficult.
There are several possible channel binding
mechanisms, but the mandatory
and probably more typical one is 'tls-unique'
which basically means
getting the byte array from the TLSfinish()
message and comparing it
with the same data sent by the client. That's more
or less all it takes
to implement it. So I would go for it.

We missed the boat for PostgreSQL 10. You're right
that it probably
wouldn't be difficult to implement, but until there's
a concrete patch
to discuss, that's a moot point.

 Really? That's a real shame I know we're very
late in the CF cycle
but, again, this would be a real shame.

That can equally be said about about a lot of features.  If we
don't
stop at some point... Also, we're not late in the CF cycle,
the CF cycle
for v10 is over.  It's not like the non-existance of channel
binding
removes previously existing features, or makes SCRAM useless.


Greetings,

Andres Freund


I know this is a lost battle. But please bear with me for a
minute.

Let's put ourselves on the foot of potential users. Why would
anyone want to use SCRAM? What for? The hashing mechanism is
better, no question. And bring some added benefits, true. So its
"better". But the real gain comes from using channel binding,
which avoids impersonation, MITM attacks. This is the deal
breaker. SCRAM without channel binding is like Coke Zero without
caffeine and mixed with water. Don't get me wrong, the work behind
is great.



I think you are seriously undervaluing the SCRAM without channel binding.


I'm not. If I wouldn't appreciate its value, I wouldn't be writing 
a client library for the jdbc driver.



It improves a lot of things over our current md5 method beyond just 
using a stronger hashing algorithm.


Sure, channel binding is great. But that's not a dealbreaker, or even 
close to it.


I think otherwise. It is close to a dealbreaker. And it's so few 
extra code lines that it requires




But just a bit more is needed to make it really a big
announcement and provide real value to (I guess, mostly but very
interesting) enterprise customers, for which MITM and
impersonating are big things. The good news is that adding channel
binding is like inverse Paretto: a 20% of extra effort (I bet
significantly less) leads to 80% improvement.


I would expect most enterprise customers who care about MITM 
protection are already using either TLS or ipsec to cover that 
already. They have benefit from the other parts of SCRAM, but they've 
already solved those problems.


Enterprises use checklists. And discard solutions if they don't 
have "checks" on all the items. One of those is, in my opinion, SCRAM 
with channel binding. I don't want this to happen to PG, specially when 
it's so easy to implement.



But I will conserve my remaining courage (thanks Michael!) credits 
for future threads ;) I have stated my opinion clearly, I will now go 
back to the client library.



Thanks,

Álvaro

--

Álvaro Hernández Tortosa


---
<8K>data

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-11 Thread Magnus Hagander 
On Tue, Apr 11, 2017 at 2:53 PM, Álvaro Hernández Tortosa 
wrote:

>
>
> On 10/04/17 20:32, Andres Freund wrote:
>
>> On 2017-04-10 20:28:27 +0200, Álvaro Hernández Tortosa wrote:
>>
>>>
>>> On 10/04/17 13:02, Heikki Linnakangas wrote:
>>>
 On 04/10/2017 12:39 PM, Álvaro Hernández Tortosa wrote:

> - I think channel binding support should be added. SCRAM brings
> security
> improvements over md5 and other simpler digest algorithms. But where it
> really shines is together with channel binding. This is the only method
> to prevent MITM attacks. Implementing it should not very difficult.
> There are several possible channel binding mechanisms, but the
> mandatory
> and probably more typical one is 'tls-unique' which basically means
> getting the byte array from the TLSfinish() message and comparing it
> with the same data sent by the client. That's more or less all it takes
> to implement it. So I would go for it.
>
 We missed the boat for PostgreSQL 10. You're right that it probably
 wouldn't be difficult to implement, but until there's a concrete patch
 to discuss, that's a moot point.

>>>  Really? That's a real shame I know we're very late in the CF
>>> cycle
>>> but, again, this would be a real shame.
>>>
>> That can equally be said about about a lot of features.  If we don't
>> stop at some point... Also, we're not late in the CF cycle, the CF cycle
>> for v10 is over.  It's not like the non-existance of channel binding
>> removes previously existing features, or makes SCRAM useless.
>>
>>
>> Greetings,
>>
>> Andres Freund
>>
>
> I know this is a lost battle. But please bear with me for a minute.
>
> Let's put ourselves on the foot of potential users. Why would anyone
> want to use SCRAM? What for? The hashing mechanism is better, no question.
> And bring some added benefits, true. So its "better". But the real gain
> comes from using channel binding, which avoids impersonation, MITM attacks.
> This is the deal breaker. SCRAM without channel binding is like Coke Zero
> without caffeine and mixed with water. Don't get me wrong, the work behind
> is great.
>


I think you are seriously undervaluing the SCRAM without channel binding.
It improves a lot of things over our current md5 method beyond just using a
stronger hashing algorithm.

Sure, channel binding is great. But that's not a dealbreaker, or even close
to it.



> But just a bit more is needed to make it really a big announcement and
> provide real value to (I guess, mostly but very interesting) enterprise
> customers, for which MITM and impersonating are big things. The good news
> is that adding channel binding is like inverse Paretto: a 20% of extra
> effort (I bet significantly less) leads to 80% improvement.
>

I would expect most enterprise customers who care about MITM protection are
already using either TLS or ipsec to cover that already. They have benefit
from the other parts of SCRAM, but they've already solved those problems.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-11 Thread Michael Paquier 
On Tue, Apr 11, 2017 at 9:53 PM, Álvaro Hernández Tortosa
 wrote:
> I know this is a lost battle. But please bear with me for a minute.

I admire your courage.

> But just a bit more is needed to make it really a big announcement and
> provide real value to (I guess, mostly but very interesting) enterprise
> customers, for which MITM and impersonating are big things. The good news is
> that adding channel binding is like inverse Paretto: a 20% of extra effort
> (I bet significantly less) leads to 80% improvement.

We'll get that into PG11, don't worry. At least Heikki or I will submit a patch.

> So CF v10 is over. So we're on testing phase. Can't we consider this a
> "missing feature bug"? ^_^

We should really focus on stability. There is still a bit more to do,
and for SCRAM we have added already a lot of infrastructure so this
should be improved first. And then we can work on extending it on a
sane basis.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-11 Thread Álvaro Hernández Tortosa 



On 10/04/17 20:32, Andres Freund wrote:

On 2017-04-10 20:28:27 +0200, Álvaro Hernández Tortosa wrote:


On 10/04/17 13:02, Heikki Linnakangas wrote:

On 04/10/2017 12:39 PM, Álvaro Hernández Tortosa wrote:

- I think channel binding support should be added. SCRAM brings security
improvements over md5 and other simpler digest algorithms. But where it
really shines is together with channel binding. This is the only method
to prevent MITM attacks. Implementing it should not very difficult.
There are several possible channel binding mechanisms, but the mandatory
and probably more typical one is 'tls-unique' which basically means
getting the byte array from the TLSfinish() message and comparing it
with the same data sent by the client. That's more or less all it takes
to implement it. So I would go for it.

We missed the boat for PostgreSQL 10. You're right that it probably
wouldn't be difficult to implement, but until there's a concrete patch
to discuss, that's a moot point.

 Really? That's a real shame I know we're very late in the CF cycle
but, again, this would be a real shame.

That can equally be said about about a lot of features.  If we don't
stop at some point... Also, we're not late in the CF cycle, the CF cycle
for v10 is over.  It's not like the non-existance of channel binding
removes previously existing features, or makes SCRAM useless.


Greetings,

Andres Freund


I know this is a lost battle. But please bear with me for a minute.

Let's put ourselves on the foot of potential users. Why would 
anyone want to use SCRAM? What for? The hashing mechanism is better, no 
question. And bring some added benefits, true. So its "better". But the 
real gain comes from using channel binding, which avoids impersonation, 
MITM attacks. This is the deal breaker. SCRAM without channel binding is 
like Coke Zero without caffeine and mixed with water. Don't get me 
wrong, the work behind is great.


But just a bit more is needed to make it really a big announcement 
and provide real value to (I guess, mostly but very interesting) 
enterprise customers, for which MITM and impersonating are big things. 
The good news is that adding channel binding is like inverse Paretto: a 
20% of extra effort (I bet significantly less) leads to 80% improvement.


So CF v10 is over. So we're on testing phase. Can't we consider 
this a "missing feature bug"? ^_^



Álvaro

--

Álvaro Hernández Tortosa


---
<8K>data



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-10 Thread Robert Haas 
On Mon, Apr 10, 2017 at 2:32 PM, Andres Freund  wrote:
> That can equally be said about about a lot of features.  If we don't
> stop at some point... Also, we're not late in the CF cycle, the CF cycle
> for v10 is over.  It's not like the non-existance of channel binding
> removes previously existing features, or makes SCRAM useless.

+1.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-10 Thread Andres Freund 
On 2017-04-10 20:28:27 +0200, Álvaro Hernández Tortosa wrote:
> 
> 
> On 10/04/17 13:02, Heikki Linnakangas wrote:
> > On 04/10/2017 12:39 PM, Álvaro Hernández Tortosa wrote:
> > > - I think channel binding support should be added. SCRAM brings security
> > > improvements over md5 and other simpler digest algorithms. But where it
> > > really shines is together with channel binding. This is the only method
> > > to prevent MITM attacks. Implementing it should not very difficult.
> > > There are several possible channel binding mechanisms, but the mandatory
> > > and probably more typical one is 'tls-unique' which basically means
> > > getting the byte array from the TLSfinish() message and comparing it
> > > with the same data sent by the client. That's more or less all it takes
> > > to implement it. So I would go for it.
> > 
> > We missed the boat for PostgreSQL 10. You're right that it probably
> > wouldn't be difficult to implement, but until there's a concrete patch
> > to discuss, that's a moot point.
> 
> Really? That's a real shame I know we're very late in the CF cycle
> but, again, this would be a real shame.

That can equally be said about about a lot of features.  If we don't
stop at some point... Also, we're not late in the CF cycle, the CF cycle
for v10 is over.  It's not like the non-existance of channel binding
removes previously existing features, or makes SCRAM useless.


Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-10 Thread Álvaro Hernández Tortosa 



On 10/04/17 13:02, Heikki Linnakangas wrote:

On 04/10/2017 12:39 PM, Álvaro Hernández Tortosa wrote:

- I think channel binding support should be added. SCRAM brings security
improvements over md5 and other simpler digest algorithms. But where it
really shines is together with channel binding. This is the only method
to prevent MITM attacks. Implementing it should not very difficult.
There are several possible channel binding mechanisms, but the mandatory
and probably more typical one is 'tls-unique' which basically means
getting the byte array from the TLSfinish() message and comparing it
with the same data sent by the client. That's more or less all it takes
to implement it. So I would go for it.


We missed the boat for PostgreSQL 10. You're right that it probably 
wouldn't be difficult to implement, but until there's a concrete patch 
to discuss, that's a moot point.


Really? That's a real shame I know we're very late in the CF 
cycle but, again, this would be a real shame.





- One SCRAM parameter, i, the iteration count, is important, and in my
opinion should be configurable. AFAIK it is currently hard coded at
4096, which is the minimum value accepted by the RFC. But it should be
at least 15K (RFC says), and given that it affects computing time of the
authentication, it should be configurable. It's also now specified per
user, which I think its too much granularity (it should suffice to say
this group of users all require this iteration count).


Yeah, a GUC might be a good idea.


Cool.



The iteration count is a part of the stored SCRAM verifier, so it's 
determined when you set the user's password. That's why it's per-user.


Sure. But I think per-user is too much granularity. And if it is 
not, it should be a parameter exposed to the CREATE USER command, such 
that it can be (effectively) set per-user. I maintain the best approach 
could be the suggested pg_scram.conf with the format (or a similar one) 
that I proposed in the OP.





- The SCRAM RFC states that server should advertise supported
mechanisms. I see discussion going into not advertising them. I think it
should be done, I don't see reasons not to do it, and it will become
compliant with the RFC.


The SASL spec [RFC4422] says that mechanism negotiation is protocol 
specific. The SCRAM spec [RFC5802] prescribes how negotiation of 
channel-binding is done, and the current implementation is compliant 
with that. (Which is very straightforward when neither the client nor 
the server supports channel binding).


Yeah. But what I'm saying is that we might want to add an extra 
String to AuthenticationSASL message for channel binding, so that the 
message format needs not to be changed when channel binding is added. 
But the channel binding method name needs to be negotiated, and so there 
needs to be room for it.




The negotiation of the mechanism is being discussed one the "Letting 
the client choose the protocol to use during a SASL exchange" thread. 
I'm just writing a more concrete proposal based on your suggestion of 
sending a list of SASL mechanisms in the AuthenticationSASL message. 
Stay tuned!


Yepp, I will reply there, thanks :)




- I don't see why proposed scram mechanism names for pg_hba.conf are not
following the IANA Registry standard (
https://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml#scram), 


which is uppercase (like in SCRAM-SHA-256).


All the existing authentication mechanisms are spelled in lowercase. 
And uppercase is ugly. It's a matter of taste, for sure.


And I agree with you. It's ugly. But it's standard. I'd say let's 
favor standardization vs our own taste.





- SCRAM also supports the concept of authzid, which is kind of what
pg_ident does: authenticate the user as the user sent, but login as the
user specified here. It could also be supported.


Yeah, it might be handy for something like pgbouncer, which could then 
have one userid+password to authenticate, but then switch to another 
user. But the SCRAM part of that seems to be just a small part of the 
overall feature. In any case, this is clearly Postgres 11 material.


Again, it should be a tiny change, just an extra attribute sent 
over the message. But I guess time was over... just saying... ;)






* The nonce length is not specified by the RFC. I see typical
implementations use 24 chars for the client and 18 for the server.
Current code uses 10. I think it should not hurt making it at least 16
or 18.


Wouldn't hurt, I guess. IIRC I checked some other implementations, 
when I picked 10, but I don't remember which ones anymore. Got a 
reference for 24/18?


First reference is the RFC example itself (non-mandatory, of 
course). But then I saw many followed this. As a quick example, GNU SASL 
defines:


#define SNONCE_ENTROPY_BYTES 18
https://www.gnu.org/software/gsasl/coverage/lib/scram/server.c.gcov.frameset.html




* It seems to me that the errors defined by the RFC, sent on

Re: [HACKERS] Some thoughts about SCRAM implementation

2017-04-10 Thread Heikki Linnakangas 

On 04/10/2017 12:39 PM, Álvaro Hernández Tortosa wrote:

- I think channel binding support should be added. SCRAM brings security
improvements over md5 and other simpler digest algorithms. But where it
really shines is together with channel binding. This is the only method
to prevent MITM attacks. Implementing it should not very difficult.
There are several possible channel binding mechanisms, but the mandatory
and probably more typical one is 'tls-unique' which basically means
getting the byte array from the TLSfinish() message and comparing it
with the same data sent by the client. That's more or less all it takes
to implement it. So I would go for it.


We missed the boat for PostgreSQL 10. You're right that it probably 
wouldn't be difficult to implement, but until there's a concrete patch 
to discuss, that's a moot point.



- One SCRAM parameter, i, the iteration count, is important, and in my
opinion should be configurable. AFAIK it is currently hard coded at
4096, which is the minimum value accepted by the RFC. But it should be
at least 15K (RFC says), and given that it affects computing time of the
authentication, it should be configurable. It's also now specified per
user, which I think its too much granularity (it should suffice to say
this group of users all require this iteration count).


Yeah, a GUC might be a good idea.

The iteration count is a part of the stored SCRAM verifier, so it's 
determined when you set the user's password. That's why it's per-user.



- The SCRAM RFC states that server should advertise supported
mechanisms. I see discussion going into not advertising them. I think it
should be done, I don't see reasons not to do it, and it will become
compliant with the RFC.


The SASL spec [RFC4422] says that mechanism negotiation is protocol 
specific. The SCRAM spec [RFC5802] prescribes how negotiation of 
channel-binding is done, and the current implementation is compliant 
with that. (Which is very straightforward when neither the client nor 
the server supports channel binding).


The negotiation of the mechanism is being discussed one the "Letting the 
client choose the protocol to use during a SASL exchange" thread. I'm 
just writing a more concrete proposal based on your suggestion of 
sending a list of SASL mechanisms in the AuthenticationSASL message. 
Stay tuned!



- I don't see why proposed scram mechanism names for pg_hba.conf are not
following the IANA Registry standard (
https://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml#scram),
which is uppercase (like in SCRAM-SHA-256).


All the existing authentication mechanisms are spelled in lowercase. And 
uppercase is ugly. It's a matter of taste, for sure.



- SCRAM also supports the concept of authzid, which is kind of what
pg_ident does: authenticate the user as the user sent, but login as the
user specified here. It could also be supported.


Yeah, it might be handy for something like pgbouncer, which could then 
have one userid+password to authenticate, but then switch to another 
user. But the SCRAM part of that seems to be just a small part of the 
overall feature. In any case, this is clearly Postgres 11 material.



* The nonce length is not specified by the RFC. I see typical
implementations use 24 chars for the client and 18 for the server.
Current code uses 10. I think it should not hurt making it at least 16
or 18.


Wouldn't hurt, I guess. IIRC I checked some other implementations, when 
I picked 10, but I don't remember which ones anymore. Got a reference 
for 24/18?



* It seems to me that the errors defined by the RFC, sent on the
server-final-message if there were errors, are never sent with the
current implementation. I think they should be sent as per the standard,
and also proceed until the last stage even if errors were detected
earlier (to conform with the RFC).


The server does send "e=invalid-proof", if the password is incorrect (or 
the user doesn't exist). All other errors are reported with 
ereport(ERROR). That is allowed by the spec:



   o  e: This attribute specifies an error that occurred during
  authentication exchange.  It is sent by the server in its final
  message and can help diagnose the reason for the authentication
  exchange failure.  On failed authentication, the entire server-
  final-message is OPTIONAL; specifically, a server implementation
  MAY conclude the SASL exchange with a failure without sending the
  server-final-message.  This results in an application-level error
  response without an extra round-trip.  If the server-final-message
  is sent on authentication failure, then the "e" attribute MUST be
  included.


It's a bit awkward to use the spec's "e=xxx" mechanism for other errors, 
because the "e=xxx" error code can only be sent in the 
server-final-message. If an error is detected before that stage, the 
server would need to continue the authentication to the end, until it 
could report it. That's why.

[HACKERS] Some thoughts about SCRAM implementation

2017-04-10 Thread Álvaro Hernández Tortosa 


Hi!

There's some ongoing discussion about SCRAM (like this thread 
https://www.postgresql.org/message-id/243d8c11-6149-a4bb-0909-136992f74b23%40iki.fi) 
but I wanted to open a new thread that covers these topics and other, 
more general ones. Here are some thoughts based on my perspective on 
SCRAM, which I gained thanks to studying it as part of the effort to 
implement SCRAM in the JDBC driver. FYI, some very early code can be 
found here: https://github.com/ahachete/scram. Although there's still a 
lot to do, I'm doing great progress and I expect to have a working 
version within 2 weeks.


So here's what I think:

- Work so far is great. Thanks Heikki, Michael and others for the effort 
involved. I love to have SCRAM support in Postgres!


- I think channel binding support should be added. SCRAM brings security 
improvements over md5 and other simpler digest algorithms. But where it 
really shines is together with channel binding. This is the only method 
to prevent MITM attacks. Implementing it should not very difficult. 
There are several possible channel binding mechanisms, but the mandatory 
and probably more typical one is 'tls-unique' which basically means 
getting the byte array from the TLSfinish() message and comparing it 
with the same data sent by the client. That's more or less all it takes 
to implement it. So I would go for it.


- One SCRAM parameter, i, the iteration count, is important, and in my 
opinion should be configurable. AFAIK it is currently hard coded at 
4096, which is the minimum value accepted by the RFC. But it should be 
at least 15K (RFC says), and given that it affects computing time of the 
authentication, it should be configurable. It's also now specified per 
user, which I think its too much granularity (it should suffice to say 
this group of users all require this iteration count).


- The SCRAM RFC states that server should advertise supported 
mechanisms. I see discussion going into not advertising them. I think it 
should be done, I don't see reasons not to do it, and it will become 
compliant with the RFC.


- I don't see why proposed scram mechanism names for pg_hba.conf are not 
following the IANA Registry standard ( 
https://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml#scram), 
which is uppercase (like in SCRAM-SHA-256).


- SCRAM also supports the concept of authzid, which is kind of what 
pg_ident does: authenticate the user as the user sent, but login as the 
user specified here. It could also be supported.



Based on these comments, I'd like to propose the following changes:

* Implement channel binding.

* Add a GUC for the channel binding technique to use. It could be named 
as "scram_channel_binding_name" or "sasl_channel_binding_name", for 
example. And probably supported value as of today, 'tls-unique'.


* AuthenticationSASL backend message:
- The String field make it a CSV that would be a list of one or 
more supported IANA SCRAM authentication methods. This info comes from 
pg_scram.conf (see below).
- Add another String to specify the variety of channel binding 
supported (if any), like 'tls-unique'. This info comes from the GUC.


* Treat the configuration of scram in pg_hba.conf, which seems 
complicated, similarly to how pg_ident.conf is treated. Basically in 
pg_hba.conf specify a "scram=scram_entry_name" value and then look for a 
pg_scram.conf file for an entry with that name. The pg_scram.conf might 
have the following structure:


name|scram_mechanisms|i
scramlight|SCRAM-SHA-1  |  4096
scramstrong |SCRAM-SHA-256,SCRAM-SHA-256-PLUS|16384


* The nonce length is not specified by the RFC. I see typical 
implementations use 24 chars for the client and 18 for the server. 
Current code uses 10. I think it should not hurt making it at least 16 
or 18.


* It seems to me that the errors defined by the RFC, sent on the 
server-final-message if there were errors, are never sent with the 
current implementation. I think they should be sent as per the standard, 
and also proceed until the last stage even if errors were detected 
earlier (to conform with the RFC).



Thoughts?

Álvaro


--

Álvaro Hernández Tortosa


---
<8K>data



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Some thoughts about SCRAM implementation

2017-04-10 Thread Álvaro Hernández Tortosa 


Hi!

There's some ongoing discussion about SCRAM (like this thread 
https://www.postgresql.org/message-id/243d8c11-6149-a4bb-0909-136992f74b23%40iki.fi) 
but I wanted to open a new thread that covers these topics and other, 
more general ones. Here are some thoughts based on my perspective on 
SCRAM, which I gained thanks to studying it as part of the effort to 
implement SCRAM in the JDBC driver. FYI, some very early code can be 
found here: https://github.com/ahachete/scram. Although there's still a 
lot to do, I'm doing great progress and I expect to have a working 
version within 2 weeks.


So here's what I think:

- Work so far is great. Thanks Heikki, Michael and others for the effort 
involved. I love to have SCRAM support in Postgres!


- I think channel binding support should be added. SCRAM brings security 
improvements over md5 and other simpler digest algorithms. But where it 
really shines is together with channel binding. This is the only method 
to prevent MITM attacks. Implementing it should not very difficult. 
There are several possible channel binding mechanisms, but the mandatory 
and probably more typical one is 'tls-unique' which basically means 
getting the byte array from the TLSfinish() message and comparing it 
with the same data sent by the client. That's more or less all it takes 
to implement it. So I would go for it.


- One SCRAM parameter, i, the iteration count, is important, and in my 
opinion should be configurable. AFAIK it is currently hard coded at 
4096, which is the minimum value accepted by the RFC. But it should be 
at least 15K (RFC says), and given that it affects computing time of the 
authentication, it should be configurable. It's also now specified per 
user, which I think its too much granularity (it should suffice to say 
this group of users all require this iteration count).


- The SCRAM RFC states that server should advertise supported 
mechanisms. I see discussion going into not advertising them. I think it 
should be done, I don't see reasons not to do it, and it will become 
compliant with the RFC.


- I don't see why proposed scram mechanism names for pg_hba.conf are not 
following the IANA Registry standard ( 
https://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml#scram), 
which is uppercase (like in SCRAM-SHA-256).


- SCRAM also supports the concept of authzid, which is kind of what 
pg_ident does: authenticate the user as the user sent, but login as the 
user specified here. It could also be supported.



Based on these comments, I'd like to propose the following changes:

* Implement channel binding.

* Add a GUC for the channel binding technique to use. It could be named 
as "scram_channel_binding_name" or "sasl_channel_binding_name", for 
example. And probably supported value as of today, 'tls-unique'.


* AuthenticationSASL backend message:
- The String field make it a CSV that would be a list of one or 
more supported IANA SCRAM authentication methods. This info comes from 
pg_scram.conf (see below).
- Add another String to specify the variety of channel binding 
supported (if any), like 'tls-unique'. This info comes from the GUC.


* Treat the configuration of scram in pg_hba.conf, which seems 
complicated, similarly to how pg_ident.conf is treated. Basically in 
pg_hba.conf specify a "scram=scram_entry_name" value and then look for a 
pg_scram.conf file for an entry with that name. The pg_scram.conf might 
have the following structure:


name|scram_mechanisms|i
scramlight|SCRAM-SHA-1  |  4096
scramstrong |SCRAM-SHA-256,SCRAM-SHA-256-PLUS|16384


* The nonce length is not specified by the RFC. I see typical 
implementations use 24 chars for the client and 18 for the server. 
Current code uses 10. I think it should not hurt making it at least 16 
or 18.


* It seems to me that the errors defined by the RFC, sent on the 
server-final-message if there were errors, are never sent with the 
current implementation. I think they should be sent as per the standard, 
and also proceed until the last stage even if errors were detected 
earlier (to conform with the RFC).



Thoughts?

Álvaro


--

Álvaro Hernández Tortosa


---
<8K>data



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers