Letting the client choose the protocol to use during a SASL exchange

On Tue, Apr 04, 2017 at 03:02:30PM +0900, Michael Paquier wrote:

There is still one open item pending for SCRAM that has not been
treated which is mentioned here:
/messages/by-id/b081887e-1712-3aa4-7dbe-e012333d50e4@iki.fi

When doing an authentication with SASL, the server decides what is the
mechanism that the client has to use. As SCRAM-SHA-256 is only one of
such mechanisms, it would be nice to have something more generic and
have the server return to the client a list of protocols that the
client can choose from. And also the server achnowledge which protocol
is going to be used.

Note that RFC4422 has some content on the matter
https://tools.ietf.org/html/rfc4422#section-3.1:
Mechanism negotiation is protocol specific.

Commonly, a protocol will specify that the server advertises
supported and available mechanisms to the client via some facility
provided by the protocol, and the client will then select the "best"
mechanism from this list that it supports and finds suitable.

So once the server sends back the list of mechanisms that are
supported, the client is free to use what it wants.

On HEAD, a 'R' message with AUTH_REQ_SASL followed by
SCRAM_SHA256_NAME is sent to let the client know what is the mechanism
to use for the SASL exchange. In the future, this should be extended
so as a list of names is sent, for example a comma-separated list, but
we are free to choose the format we want here. With this list at hand,
the client can then choose the protocol it thinks is the best. Still,
there is a gap with our current implementation because the server
expects the first message from the client to have a SCRAM format, but
that's true only if SCRAM-SHA-256 is used as mechanism.

In order to cover this gap, it seems to me that we need to have an
intermediate state before the server is switched to FE_SCRAM_INIT so
as the mechanism used is negotiated between the two parties. Once the
protocol negotiation is done, the server can then move on with the
mechanism to use. This would be important in the future to allow more
SASL mechanisms to work. I am adding an open item for that.

If any SCRAM open item is a beta blocker, it's this one. (But SASLprep is
also in or near that status.) Post-beta wire protocol changes are bad,
considering beta is normally the time for projects like pgjdbc and npgsql to
start adapting to such changes.

[Action required within three days. This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item. Heikki,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.

[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com

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

On 04/06/2017 08:13 AM, Noah Misch wrote:

If any SCRAM open item is a beta blocker, it's this one. (But SASLprep is
also in or near that status.) Post-beta wire protocol changes are bad,
considering beta is normally the time for projects like pgjdbc and npgsql to
start adapting to such changes.

[Action required within three days. This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.

I will work on this next week. I haven't given it much thought yet, but
I think it's going to be pretty straightforward. It won't require much
code yet, as we only support one SASL mechanism. We just need to ensure
that we don't paint ourselves in the corner with the protocol.

- Heikki

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

On 06/04/17 19:05, Heikki Linnakangas wrote:

On 04/06/2017 08:13 AM, Noah Misch wrote:

If any SCRAM open item is a beta blocker, it's this one. (But
SASLprep is
also in or near that status.) Post-beta wire protocol changes are bad,
considering beta is normally the time for projects like pgjdbc and
npgsql to
start adapting to such changes.

[Action required within three days. This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.

I will work on this next week. I haven't given it much thought yet,
but I think it's going to be pretty straightforward. It won't require
much code yet, as we only support one SASL mechanism. We just need to
ensure that we don't paint ourselves in the corner with the protocol.

I think this could easily extended from the current message. SCRAM
does not force any concrete way of negotiating the protocols supported.
The current SASL message sends the only SCRAM method supported. I think
it should be enough to make this a CSV, which for a single value, is the
same as we have today (so no code change required, only documentation).
The other message that needs to be changed is the password one, the
first time the client sends the SCRAM "client-first-message", which
needs to contain the algorithm selected. As of today, that could be a
constant (at least in the code) and validate is value.

So I guess code changes would be minimal. I could be wrong, of course.

I'm working myself on Java's (pgjdbc) implementation, and I will
hopefully have a prototype by next week to try it.

�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

On 4 April 2017 at 02:02, Michael Paquier <michael.paquier@gmail.com> wrote:

Hi all,

There is still one open item pending for SCRAM that has not been
treated which is mentioned here:
/messages/by-id/b081887e-1712-3aa4-7dbe-e012333d50e4@iki.fi

When doing an authentication with SASL, the server decides what is the
mechanism that the client has to use. As SCRAM-SHA-256 is only one of
such mechanisms, it would be nice to have something more generic and
have the server return to the client a list of protocols that the
client can choose from. And also the server achnowledge which protocol
is going to be used.

Note that RFC4422 has some content on the matter
https://tools.ietf.org/html/rfc4422#section-3.1:
Mechanism negotiation is protocol specific.

Commonly, a protocol will specify that the server advertises
supported and available mechanisms to the client via some facility
provided by the protocol, and the client will then select the "best"
mechanism from this list that it supports and finds suitable.

So once the server sends back the list of mechanisms that are
supported, the client is free to use what it wants.

On HEAD, a 'R' message with AUTH_REQ_SASL followed by
SCRAM_SHA256_NAME is sent to let the client know what is the mechanism
to use for the SASL exchange. In the future, this should be extended
so as a list of names is sent, for example a comma-separated list, but
we are free to choose the format we want here. With this list at hand,
the client can then choose the protocol it thinks is the best. Still,
there is a gap with our current implementation because the server
expects the first message from the client to have a SCRAM format, but
that's true only if SCRAM-SHA-256 is used as mechanism.

How would we provide the list of protocols? Surely the protocol is
defined by pg_hba.conf, which makes it dependent upon username,
database and ip range. If the list were accurate, it would allow an
attacker to discover how best to attack. If the list were inaccurate
it would just be an annoyance.

At minimum, providing the list of protocols means an extra round trip
to the server.

So while RFC4422 might say what "commonly" happens, I don't think it
applies sensibly to PostgreSQL, especially when we only have one
method.

ISTM that if you have a valid role to connect to then you'll also know
what authentication mechanism to use so you should be able to specify
the mechanism in your connection message and save the extra trip.

In order to cover this gap, it seems to me that we need to have an
intermediate state before the server is switched to FE_SCRAM_INIT so
as the mechanism used is negotiated between the two parties. Once the
protocol negotiation is done, the server can then move on with the
mechanism to use. This would be important in the future to allow more
SASL mechanisms to work. I am adding an open item for that.

So I don't see a gap or an open item on that point. I see a potential
future feature.

For extensibility, we may also want to revisit the choice of defining
'scram' in pg_hba.conf instead of 'sasl'...

I'd like to see us replace "scram" with "sasl=scram-sha-256".

So when we extend it in future, we already have the syntax in place to
support "sasl=newmethod", rather than introduce syntax that we already
know will become outdated in future.

--
Simon Riggs 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

Simon Riggs <simon@2ndquadrant.com> writes:

How would we provide the list of protocols? Surely the protocol is
defined by pg_hba.conf, which makes it dependent upon username,
database and ip range. If the list were accurate, it would allow an
attacker to discover how best to attack. If the list were inaccurate
it would just be an annoyance.

At minimum, providing the list of protocols means an extra round trip
to the server.

Yeah, that's a problem.

ISTM that if you have a valid role to connect to then you'll also know
what authentication mechanism to use so you should be able to specify
the mechanism in your connection message and save the extra trip.

I do not buy that in the least. It has never been the case before now
that clients know in advance what the auth challenge method will be.
If we put that requirement on them for SCRAM, we're just going to be
exporting a lot of pain and end-user-visible inconsistency.

Perhaps we could turn this around: have the client send (in the connection
request packet) a list of auth protocols it thinks it is able to handle.
(I'm envisioning this as being more or less fixed for any one version of
any one client, since it would basically mean "I have code to do X, Y, or
Z".) Then the server can pick one that is allowed by pg_hba.conf, or it
can just ignore the list and send what it wants anyway, probably leading
to client disconnect.

We could avoid this being a protocol break by having the server's default
assumption being that the client can handle all pre-SCRAM auth protocols.

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

On 06/04/17 22:05, Tom Lane wrote:

Simon Riggs <simon@2ndquadrant.com> writes:

How would we provide the list of protocols? Surely the protocol is
defined by pg_hba.conf, which makes it dependent upon username,
database and ip range. If the list were accurate, it would allow an
attacker to discover how best to attack. If the list were inaccurate
it would just be an annoyance.
At minimum, providing the list of protocols means an extra round trip
to the server.

Yeah, that's a problem.

I don't see it. The message AuthenticationSASL.String could contain
a CSV of the SCRAM protocols supported. This is specially important to
support channel binding (which is just another protocol name for this
matter), which is the really enhanced security mechanism of SCRAM. Since
this message is sent regardless, and the client replies with
PasswordMessage, no extra round trip is required. However,
PasswordMessage needs to also include a field with the name of the
selected protocol (it is the client who picks). Or a different message
would need to be created, but no extra round-trips more than those
required by SCRAM itself (4 messages for SCRAM + 1 extra for the server
to tell the client it needs to use SCRAM).

ISTM that if you have a valid role to connect to then you'll also know
what authentication mechanism to use so you should be able to specify
the mechanism in your connection message and save the extra trip.

I do not buy that in the least. It has never been the case before now
that clients know in advance what the auth challenge method will be.
If we put that requirement on them for SCRAM, we're just going to be
exporting a lot of pain and end-user-visible inconsistency.

Perhaps we could turn this around: have the client send (in the connection
request packet) a list of auth protocols it thinks it is able to handle.

Per the SCRAM RFC, it is the server who advertises and the client
who picks.

Regards,

�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

On 6 April 2017 at 16:05, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Perhaps we could turn this around: have the client send (in the connection
request packet) a list of auth protocols it thinks it is able to handle.
(I'm envisioning this as being more or less fixed for any one version of
any one client, since it would basically mean "I have code to do X, Y, or
Z".) Then the server can pick one that is allowed by pg_hba.conf,

+1

Much better plan.

or it
can just ignore the list and send what it wants anyway, probably leading
to client disconnect.

It would need to follow one of the requested protocols, but mark the
request as doomed. Otherwise we'd be revealing information. That's
what SCRAM does now.

Since the list is currently length one, we can add more later when we
get a list potentially > 1.

We could avoid this being a protocol break by having the server's default
assumption being that the client can handle all pre-SCRAM auth protocols.

+1

--
Simon Riggs 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

On Fri, Apr 7, 2017 at 5:15 AM, Álvaro Hernández Tortosa <aht@8kdata.com> wrote:

I don't see it. The message AuthenticationSASL.String could contain a
CSV of the SCRAM protocols supported. This is specially important to support
channel binding (which is just another protocol name for this matter), which
is the really enhanced security mechanism of SCRAM. Since this message is
sent regardless, and the client replies with PasswordMessage, no extra round
trip is required. However, PasswordMessage needs to also include a field
with the name of the selected protocol (it is the client who picks). Or a
different message would need to be created, but no extra round-trips more
than those required by SCRAM itself (4 messages for SCRAM + 1 extra for the
server to tell the client it needs to use SCRAM).

Yes, it seems to me that the list of protocols to send should be done
by sendAuthRequest(). Then the client parses the received string, and
sends an extra 'p' message with its choice before sending the first
SCRAM message. So there is no need for any extra round trips.
--
Michael

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

On 6 April 2017 at 16:15, Álvaro Hernández Tortosa <aht@8kdata.com> wrote:

Per the SCRAM RFC, it is the server who advertises and the client who picks.

Yes, but what does the RFC say about how to handle servers with an pg_hba.conf?

How and what will we advertise?

--
Simon Riggs 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

On Fri, Apr 7, 2017 at 7:29 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 6 April 2017 at 16:15, Álvaro Hernández Tortosa <aht@8kdata.com> wrote:

Per the SCRAM RFC, it is the server who advertises and the client who picks.

Yes, but what does the RFC say about how to handle servers with an pg_hba.conf?

How and what will we advertise?

Did you read the first email of this thread? The RFC says that the
protocol implementers are free to do what they want as this is
protocol-specific. At least that's what I understand.
--
Michael

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

On 04/06/2017 11:16 PM, Simon Riggs wrote:

or it
can just ignore the list and send what it wants anyway, probably leading
to client disconnect.

It would need to follow one of the requested protocols, but mark the
request as doomed. Otherwise we'd be revealing information. That's
what SCRAM does now.

It's not a secret today, what authentication method the server requires.
You can't really hide it, anyway, as the client could probe with
different lists of supported methods, and see which method the server
picks in each case.

- Heikki

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

On 04/06/2017 11:05 PM, Tom Lane wrote:

Perhaps we could turn this around: have the client send (in the connection
request packet) a list of auth protocols it thinks it is able to handle.
(I'm envisioning this as being more or less fixed for any one version of
any one client, since it would basically mean "I have code to do X, Y, or
Z".) Then the server can pick one that is allowed by pg_hba.conf, or it
can just ignore the list and send what it wants anyway, probably leading
to client disconnect.

That list of supported authentication methods would need to be included
in the startup message. Unfortunately, there is no way to add options to
the startup message, without breaking compatibility with old servers. If
there is an option in the startup message that the server doesn't
understand, it will treat it as a GUC, and you get an "unrecognized
configuration parameter" after authentication.

It would be nice to change that, so that the server would ignore
parameters that it doesn't understand that begin with "optional_"
prefix, for example. But it won't help us right 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

On 7 April 2017 at 16:33, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

That list of supported authentication methods would need to be included in
the startup message. Unfortunately, there is no way to add options to the
startup message, without breaking compatibility with old servers. If there
is an option in the startup message that the server doesn't understand, it
will treat it as a GUC, and you get an "unrecognized configuration
parameter" after authentication.

sasl.mechanisms = 'SCRAM_SHA256'

:p

No, I'm not seriously suggesting we abuse that.

Personally I think it's reasonable enough to let the server send a 'B'
message with supported auth modes. I'm not overly concerned about the
small information leak that provides. We're unlikely to be able to
convincingly fake execution of any and all SASL auth methods the
client may request, and since they may require any arbitrary number of
message exchanges we'd basically land up blackholeing clients that try
an unsupported auth-method.

No thanks. It's one area I'd rather honestly say "nope, we don't
support that". In which case the client can easily enough probe for
all known methods, and we might as well just tell it up front.

This is hardly new. Most servers with neotiable auth, like SMTP, IMAP,
etc, have the server supply a list of auth mechs.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, 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

On 04/07/2017 11:57 AM, Craig Ringer wrote:

On 7 April 2017 at 16:33, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

That list of supported authentication methods would need to be included in
the startup message. Unfortunately, there is no way to add options to the
startup message, without breaking compatibility with old servers. If there
is an option in the startup message that the server doesn't understand, it
will treat it as a GUC, and you get an "unrecognized configuration
parameter" after authentication.

sasl.mechanisms = 'SCRAM_SHA256'

:p

No, I'm not seriously suggesting we abuse that.

Hmm, that's not such a bad idea, actually. It only goes back to 9.2,
though. Before that, the prefix needed to be listed in
custom_variable_classes, or you got an error. 9.2 is the oldest
supported version, but libpq should still be able to connect to older
versions.

- Heikki

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

On 04/07/2017 01:13 AM, Michael Paquier wrote:

On Fri, Apr 7, 2017 at 5:15 AM, Álvaro Hernández Tortosa <aht@8kdata.com> wrote:

I don't see it. The message AuthenticationSASL.String could contain a
CSV of the SCRAM protocols supported. This is specially important to support
channel binding (which is just another protocol name for this matter), which
is the really enhanced security mechanism of SCRAM. Since this message is
sent regardless, and the client replies with PasswordMessage, no extra round
trip is required. However, PasswordMessage needs to also include a field
with the name of the selected protocol (it is the client who picks). Or a
different message would need to be created, but no extra round-trips more
than those required by SCRAM itself (4 messages for SCRAM + 1 extra for the
server to tell the client it needs to use SCRAM).

Yes, it seems to me that the list of protocols to send should be done
by sendAuthRequest(). Then the client parses the received string, and
sends an extra 'p' message with its choice before sending the first
SCRAM message. So there is no need for any extra round trips.

I started writing down the protocol docs, based on the above idea. See
attached. The AuthenticationSASL message now contains a list of mechanisms.

Does that seem clear to you? If so, I'll change the code to match the
attached docs.

I added two new message formats to the docs, SASLResponse and
SASLInitialResponse. Those use the same type byte as PasswordMessage,
'p', but I decided to describe them as separate messages for
documentation purposes, since the content is completely different
depending on whether the message is sent as part of SASL, GSS, md5, or
password authentication. IOW, this is not a change in the
implementation, only in the way it's documented.

While working on this, and reading the RFCs more carefully, I noticed
one detail we should change, to be spec-complicant. The SASL spec
specifies that a SASL authentication exchange consists of
challenge-response pairs. There must be a response to each challenge. If
the last message in the authentication mechanism (SCRAM in this case)
goes in the server->client direction, then that message must sent as
"additional data" in the server->client message that tells the client
that the authentication was successful. That's AuthenticationOK in the
PostgreSQL protocol. In the current implementation, the
server-final-message is sent as an AuthenticationSASLContinue message,
and the client doesn't respond to that.

We should change that, so that the server-final-message is sent as
"additional data" in the AuthenticationOK message. The attached docs
patch describes that, rather than what the current implementation does.

(For your convenience, I built the HTML docs with this patch, and put
them up at http://hlinnaka.iki.fi/temp/scram-wip-docs/protocol.html for
viewing)

- Heikki

On 10/04/17 14:57, Heikki Linnakangas wrote:

On 04/07/2017 01:13 AM, Michael Paquier wrote:

On Fri, Apr 7, 2017 at 5:15 AM, Álvaro Hernández Tortosa
<aht@8kdata.com> wrote:

I don't see it. The message AuthenticationSASL.String could
contain a
CSV of the SCRAM protocols supported. This is specially important to
support
channel binding (which is just another protocol name for this
matter), which
is the really enhanced security mechanism of SCRAM. Since this
message is
sent regardless, and the client replies with PasswordMessage, no
extra round
trip is required. However, PasswordMessage needs to also include a
field
with the name of the selected protocol (it is the client who picks).
Or a
different message would need to be created, but no extra round-trips
more
than those required by SCRAM itself (4 messages for SCRAM + 1 extra
for the
server to tell the client it needs to use SCRAM).

Yes, it seems to me that the list of protocols to send should be done
by sendAuthRequest(). Then the client parses the received string, and
sends an extra 'p' message with its choice before sending the first
SCRAM message. So there is no need for any extra round trips.

I started writing down the protocol docs, based on the above idea. See
attached. The AuthenticationSASL message now contains a list of
mechanisms.

Does that seem clear to you? If so, I'll change the code to match the
attached docs.

I added two new message formats to the docs, SASLResponse and
SASLInitialResponse. Those use the same type byte as PasswordMessage,
'p', but I decided to describe them as separate messages for
documentation purposes, since the content is completely different
depending on whether the message is sent as part of SASL, GSS, md5, or
password authentication. IOW, this is not a change in the
implementation, only in the way it's documented.

While working on this, and reading the RFCs more carefully, I noticed
one detail we should change, to be spec-complicant. The SASL spec
specifies that a SASL authentication exchange consists of
challenge-response pairs. There must be a response to each challenge.
If the last message in the authentication mechanism (SCRAM in this
case) goes in the server->client direction, then that message must
sent as "additional data" in the server->client message that tells the
client that the authentication was successful. That's AuthenticationOK
in the PostgreSQL protocol. In the current implementation, the
server-final-message is sent as an AuthenticationSASLContinue message,
and the client doesn't respond to that.

We should change that, so that the server-final-message is sent as
"additional data" in the AuthenticationOK message. The attached docs
patch describes that, rather than what the current implementation does.

(For your convenience, I built the HTML docs with this patch, and put
them up at http://hlinnaka.iki.fi/temp/scram-wip-docs/protocol.html
for viewing)

- Heikki

Thanks for posting the patched HTML. In my opinion, all looks good
except that:

- I will add an extra String (a CSV) to AuthenticationSASL message for
channel binding names, so that message format can remain without changes
when channel binding is implemented. It can be empty.

- If the username used is the one sent in the startup message, rather
than leaving it empty in the client-first-message, I would force it to
be the same as the used in the startuo message. Otherwise we may confuse
some client implementations which would probably consider that as an
error; for one, my implementation would currently throw an error if
username is empty, and I think that's correct.

Álvaro

--

Álvaro Hernández Tortosa

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

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

Thanks for posting the patched HTML. In my opinion, all looks good
except that:

- I will add an extra String (a CSV) to AuthenticationSASL message for
channel binding names, so that message format can remain without changes
when channel binding is implemented. It can be empty.

Note that SCRAM-SHA-256 with channel binding has a different SASL
mechanism name, SRAM-SHA-256-PLUS. No need for a separate flag or string
for channel binding. When support for channel binding is added to the
server, it will advertise two SASL mechanisms in the AuthenticationSASL
message, SCRAM-SHA-256 and SCRAM-SHA-256-PLUS. (Or just
SCRAM-SHA-256-PLUS, if channel-binding is required).

- If the username used is the one sent in the startup message, rather
than leaving it empty in the client-first-message, I would force it to
be the same as the used in the startuo message.

The problem with that is that the SCRAM spec dictates that the username
must be encoded in UTF-8, but PostgreSQL supports non-UTF-8 usernames.

Or did you mean that, if the username is sent, it must match the one in
the startup packet, but an empty string would always be allowed? That
would be reasonable.

Otherwise we may confuse
some client implementations which would probably consider that as an
error; for one, my implementation would currently throw an error if
username is empty, and I think that's correct.

I'm not sure I follow. The username is sent from client to server, and
currently, the server will ignore it. If you're writing a client
library, it can send whatever it wants. (Although again I would
recommend an empty string, to avoid confusion. Sending the same username
as in the startup packet, as long as it's in UTF-8, seems reasonable too.)

Thanks for reviewing this! I'll start hacking on code changes to go with
these docs.

- Heikki

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

On 10/04/17 21:41, Heikki Linnakangas wrote:

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

Thanks for posting the patched HTML. In my opinion, all looks good
except that:

- I will add an extra String (a CSV) to AuthenticationSASL message for
channel binding names, so that message format can remain without changes
when channel binding is implemented. It can be empty.

Note that SCRAM-SHA-256 with channel binding has a different SASL
mechanism name, SRAM-SHA-256-PLUS. No need for a separate flag or
string for channel binding. When support for channel binding is added
to the server, it will advertise two SASL mechanisms in the
AuthenticationSASL message, SCRAM-SHA-256 and SCRAM-SHA-256-PLUS. (Or
just SCRAM-SHA-256-PLUS, if channel-binding is required).

Channel binding needs to specify actually three things:
- The mechanism, which is indeed suffixed "-PLUS".
- The channel binding name, which is described here:
https://tools.ietf.org/html/rfc5056. Types are also IANA-registered (see
https://www.iana.org/assignments/channel-binding-types/channel-binding-types.xhtml).
SCRAM mandates to implement 'tls-unique', but other channel binding
types could be supported (like 'tls-server-end-point' for example).
- The channel binding data, which is channel binding mechanism
dependent, and is sent as part of the client last message.

What I'm talking about here is the second one, the channel binding
type (name).

- If the username used is the one sent in the startup message, rather
than leaving it empty in the client-first-message, I would force it to
be the same as the used in the startuo message.

The problem with that is that the SCRAM spec dictates that the
username must be encoded in UTF-8, but PostgreSQL supports non-UTF-8
usernames.

Or did you mean that, if the username is sent, it must match the one
in the startup packet, but an empty string would always be allowed?
That would be reasonable.

Otherwise we may confuse
some client implementations which would probably consider that as an
error; for one, my implementation would currently throw an error if
username is empty, and I think that's correct.

I'm not sure I follow. The username is sent from client to server, and
currently, the server will ignore it. If you're writing a client
library, it can send whatever it wants. (Although again I would
recommend an empty string, to avoid confusion. Sending the same
username as in the startup packet, as long as it's in UTF-8, seems
reasonable too.)

OK, understood. I will not let then the SCRAM implementation I'm
writing to allow for empty string as the user name, but in the pgjdbc
glue code send "ignore" as the user name or something like that ;P

Thanks for reviewing this! I'll start hacking on code changes to go
with these docs.

Thanks for writing the 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

On Tue, Apr 11, 2017 at 4:41 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

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

- If the username used is the one sent in the startup message, rather
than leaving it empty in the client-first-message, I would force it to
be the same as the used in the startuo message.

The problem with that is that the SCRAM spec dictates that the username must
be encoded in UTF-8, but PostgreSQL supports non-UTF-8 usernames.

Or did you mean that, if the username is sent, it must match the one in the
startup packet, but an empty string would always be allowed? That would be
reasonable.

That sounds sensible from here.

Otherwise we may confuse
some client implementations which would probably consider that as an
error; for one, my implementation would currently throw an error if
username is empty, and I think that's correct.

I'm not sure I follow. The username is sent from client to server, and
currently, the server will ignore it. If you're writing a client library, it
can send whatever it wants. (Although again I would recommend an empty
string, to avoid confusion. Sending the same username as in the startup
packet, as long as it's in UTF-8, seems reasonable too.)

Thanks for reviewing this! I'll start hacking on code changes to go with
these docs.

Sounds good to me, thanks! I looked at the doc patch for now, and here
are some nits.

+  <para>
+    SCRAM-SHA-256 is the only implemented SASL mechanism, at the moment. It is
+    described in detail in [RFC7677] and [RFC5741]. (called just
SCRAM from now on)
+  </para>
Perhaps those should be links to the RFCs.

+<para>
+  Client sends a SASLResponse messsage, with SCRAM
+  <literal>client-final-message</literal> as the content.
+</para>
Typo. Message needs two 's'.

+  <literal>server-final-message</> in the "additional data" field.
+</para>
+
+</sect2>
+
I think that you are missing a </sect1> markup here.

+<listitem>
+<para>
+                Authentication method specific "additional data". If this
+                message contains no additional data, this field is omitted.
+</para>
+</listitem>
So that's for the first message generated by the client in the
exchange. Better than my previous idea. Shouldn't double quotes be
replaced by proper markups?
-- 
Michael

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