Re: WIP: SCRAM authentication

* Heikki Linnakangas (hlinnaka(at)iki(dot)fi) wrote:
> There have been numerous threads on replacing our MD5 authentication
> method, so I started hacking on that to see what it might look like.
> Just to be clear, this is 9.6 material. Attached is a WIP patch
> series that adds support for SCRAM. There's no need to look at the
> details yet, but it demonstrates what the protocol changes and the
> code structure would be like.

Great! Very glad that you're working on this.

> I'm not wedded to SCRAM - SRP or JPAKE or something else might be
> better. But replacing the algorithm, or adding more of them, should
> be straightforward with this.

Excellent.

> 3. Allow storing multiple verifiers in pg_authid
> ------------------------------------------------
>
> Replace the pg_authid.rolpassword text field with an array, and
> rename it to 'rolverifiers'. This allows storing multiple password
> hashes: an MD5 hash for MD5 authentication, and a SCRAM salt and
> stored key for SCRAM authentication, etc. Each element in the array
> is a string that begins with the method's name. For example
> "md5:<MD5 hash>", or "password:<plaintext>".
>
> For dump/reload, and for clients that wish to create the hashes in
> the client-side, there is a new option to CREATE/ALTER USER
> commands: PASSWORD VERIFIERS '{ ... }', that allows replacing the
> array.
>
> The old "ENCRYPTED/UNENCRYPTED PASSWORD 'foo'" options are still
> supported for backwards-compatibility, but it's not clear what it
> should mean now.
>
> TODO:
>
> * Password-checking hook needs to be redesigned, to allow for more
> kinds of hashes.
>
> * With "CREATE USER PASSWORD 'foo'", which hashes/verifiers should
> be generated by default? We currently have a boolean
> password_encryption setting for that. Needs to be a list.

This generally sounds good to me but we definitely need to have that
list of hashes to be used. The MIT KDC for Kerberos (and I believe all
the other Kerberos implementations) have a similar setting for what will
be stored and what will be allowed for hashing and encryption options.
It's very important that we allow users to tweak this list, as we will
want to encourage users to migrate off of the existing md5 storage
mechanism and on to the SCRAM based one eventually.

Unfortunately, the first major release with this will certainly need to
default to including md5 as we can't have a password update or change
break clients right off the bat. What I think would be fantastic would
be a warning, perhaps in the first release or maybe the second, which
deprecates md5 as an auth method and is thrown when a password is set
which includes storing an md5-based password. I'm sure there will be
plenty of discussion about that in the future.

One additional item is that we need to have a way to prefer SCRAM-based
auth while allowing a fall-back to md5 if the client doesn't support it.
This might have to be driven by the client side explicitly saying "I
support SCRAM" from the start to avoid breaking existing clients.

> 4. Implement SCRAM
> ------------------
>
> The protocol and the code is structured so that it would be fairly
> easy to add more built-in SASL mechanisms, or to use a SASL library
> to provide more. But for now I'm focusing on adding exactly one new
> built-in mechanism, to replace MD5 in the long term.
>
> In the protocol, there is a new AuthenticationSASL message,
> alongside the existing AuthenticationMD5, AuthenticationSSPI etc.
> The AuthenticationSASL message contains the name of the SASL
> mechanism used ("SCRAM-SHA-1"). Just like in the GSSAPI/SSPI
> authentication, a number of PasswordMessage and
> AuthenticationSASLContinue messages are exchanged after that,
> carrying the data specified by the SCRAM spec, until the
> authentication succeeds (or not).
>
> TODO:
>
> * Per the SCRAM specification, the client sends the username in the
> handshake. But in the FE/BE protocol, we've already sent it in the
> startup packet. In the patch, libpq always sends an empty username
> in the SCRAM exchange, and the username from the startup packet is
> what matters. We could also require it to be the same, but in SCRAM
> the username to be UTF-8 encoded, while in PostgreSQL the username
> can be in any encoding. That is a source of annoyance in itself, as
> it's not well-defined in PostgreSQL which encoding to use when
> sending a username to the server. But I don't want to try fixing
> that in this patch, so it seems easiest to just require the username
> to be empty.

I don't like having it be empty.. I'm not looking at the spec right at
the moment, but have you confirmed that the username being empty during
the SCRAM discussion doesn't reduce the effectiveness of the
authentication method overall in some way? Is it ever used in
generation of the authentication verifier, etc? One way to address the
risk which you bring up about the different encodings might be to simply
discourage using non-UTF8-compliant encodings by throwing a warning or
refusing to support SCRAM in cases where the role wouldn't be allowed by
SCRAM (eg: in CREATE ROLE or ALTER ROLE when the SCRAM auth verifier
storage is being handled). Another option might be to define a way to
convert from "whatever" to "UTF8 something" for the purposes of the
SCRAM auth method.

> * Need a source of randomness in client, to generate random nonces
> used in the handshake. The SCRAM specification is not explicit about
> it, but I believe it doesn't need to be unpredictable, as long as a
> different nonce is used for each authentication.

I'd *very* much prefer a well defined and understood way (ideally
implemented in some well known and maintained library) rather than
trying to work out something ourselves. Further, it'd be good to review
what others have done in this space with SCRAM as there may be lessons
learned or at least well reviewed approaches to consider.

> * The client does not authenticate the server, even though the SCRAM
> protocol allows that. The client does verify the proof the server
> sends, but nothing stops a malicious server that's impersonating the
> real server from not requesting SCRAM authentication in the first
> place. It could just send AuthenticationOK without any
> authentication at all. To take advantage of the server
> authentication, we'll need to add something similar to the
> "sslmode=verify-ca" option in the client. In that mode, the client
> should refuse the connection if the server doesn't request SCRAM
> authentication (or some other future authentication mechanism that
> authenticates the server to the client).

Agreed, we should have a way for the client to require SCRAM.
Presumably we would do this for libpq-based clients and expect other
implementations to look at the options we build into libpq for their own
versions (eg: JDBC). There's nothing protocol-level to be done here
that I can think of off-hand.

> * Channel binding is not implemented. Not essential, but would be
> nice to have. Together with the new client option mentioned in the
> previous point, it would allow the client to know that there is no
> man-in-the-middle, without having to verify the server's SSL
> certificate.

Agreed, this is definitely one of the good features of SCRAM and should
be included.

Haven't looked at the code at all.

Thanks!

Stephen