Horton

Re: SCRAM with channel binding downgrade attack

Heikki Linnakangas <hlinnaka@iki.fi>

View thread View on postgr.es
From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Michael Paquier <michael@paquier.xyz>, Postgres hackers <pgsql-hackers@postgresql.org>
Cc: Robert Haas <robertmhaas@gmail.com>, Magnus Hagander <magnus@hagander.net>, Stephen Frost <sfrost@snowman.net>, Bruce Momjian <bruce@momjian.us>
Date: 2018-05-28T09:00:33Z
Lists: pgsql-hackers 
On 28/05/18 04:23, Michael Paquier wrote:
> On Sat, May 26, 2018 at 11:42:38PM +0900, Michael Paquier wrote:
>> On Sat, May 26, 2018 at 09:08:50AM -0400, Bruce Momjian wrote:
>>> On Sat, May 26, 2018 at 08:32:20AM +0900, Michael Paquier wrote:
>>>>
>>>> OK, I can live with that as well.  So we'll go in the direction of two
>>>> parameters then:
>>>> - scram_channel_binding, which can use "prefer" (default), "require" or
>>>> "disable".
>>>> - scram_channel_binding_name, developer option to choose the type of
>>>> channel binding, with "tls-unique" (default) and "tls-server-end-point".
>>>> We could also remove the prefix "scram_".  Ideas of names are welcome.
>>>
>>> scram_channel_binding_method?
>>
>> Or scram_channel_binding_type.  The first sentence of RFC 5929 uses this
>> term.
> 
> I just went with scram_channel_binding_mode (require, disable and
> prefer) and scram_channel_binding_type as parameter names, in the shape
> of the attached patch.

Thanks! Getting better.

There's one pretty fatal bug in this patch: If you use 
"scram_channel_binding=require", we only fail the connection after going 
through the motions of authenticating with whatever authentication the 
server asked for. That's bad, because it means that the client will 
merrily respond to a AUTH_REQ_PASSWORD request from the server, by 
sending the password in cleartext.

That's not a new problem, but it makes the MITM protection fairly 
pointless, if a fake server can acquire the user's password by simply 
asking for it. The client will report a failed connection, but with the 
user's password, Mallory won't need to act as a MITM anymore.

- Heikki

Commits

  1. doc: update PG 11 release notes

  2. Fix misspelled pg_trgm contrib name in PostgreSQL 11 release notes

  3. Doc: clarify release note text about v11's new window function features.

  4. Improve wording of release notes item

  5. Fix typos in release notes

  6. Doc: preliminary list of PG11 major features.

  7. Make numeric power() handle NaNs according to the modern POSIX spec.

  8. Various improvements of skipping index scan during vacuum technics

  9. Revert back-branch changes in power()'s behavior for NaN inputs.

  10. Avoid wrong results for power() with NaN input on more platforms.

  11. Avoid wrong results for power() with NaN input on some platforms.

  12. Skip full index scan during cleanup of B-tree indexes when possible

  13. Rewrite the code that applies scan/join targets to paths.

  14. Postpone generate_gather_paths for topmost scan/join rel.

  15. Add casts from jsonb

  16. Make plpgsql use its DTYPE_REC code paths for composite-type variables.

  17. Don't allow VACUUM VERBOSE ANALYZE VERBOSE.

  18. Pass InitPlan values to workers via Gather (Merge).

  19. Account for the effect of lossy pages when costing bitmap scans.

  20. Allow no-op GiST support functions to be omitted.

  21. Rearm statement_timeout after each executed query.

  22. Push limit through subqueries to underlying sort, where possible.