[HACKERS] Some thoughts about SCRAM implementation - Mailing list pgsql-hackers

    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