Re: SCRAM with channel binding downgrade attack

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>

To: Heikki Linnakangas <hlinnaka@iki.fi>

Cc: Robert Haas <robertmhaas@gmail.com>, Bruce Momjian <bruce@momjian.us>, Magnus Hagander <magnus@hagander.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Stephen Frost <sfrost@snowman.net>

Date: 2018-05-25T23:32:20Z

Lists: pgsql-hackers

On Fri, May 25, 2018 at 06:24:07PM +0300, Heikki Linnakangas wrote:
> On 25 May 2018 17:44:16 EEST, Robert Haas <robertmhaas@gmail.com> wrote:
>> It seems to me that this is really another sort of thing altogether.
>> Whether or not you want to insist on channel binding is a completely
>> separate thing from which channel binding methods you're willing to
>> use.  It seems to me like the most logical thing would be to make
>> these two separate connection options. 
> 
> Works for me.

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.

At the end, the core of the proposed patch relies on the fact that it
checks when receiving AUTH_REQ_OK that a full set of SCRAM messages has
happened with the server, up to the point where the client has checked
the server proof that both ends know the same password.  So do people
here think that this is a sane apprach?  Are there other ideas?
--
Michael

Commits

doc: update PG 11 release notes
- 6eb612fea9d0 12.0 cited
Fix misspelled pg_trgm contrib name in PostgreSQL 11 release notes
- 3abc5a67edfd 12.0 landed
Doc: clarify release note text about v11's new window function features.
- 510421c45fb4 11.0 landed
- 11a3aeeb5e17 12.0 landed
Improve wording of release notes item
- bee6a683a5c3 11.0 landed
Fix typos in release notes
- fb6accd27b99 11.0 landed
Doc: preliminary list of PG11 major features.
- 4aad161c9a6d 11.0 landed
Make numeric power() handle NaNs according to the modern POSIX spec.
- d1fc750b5199 11.0 landed
Various improvements of skipping index scan during vacuum technics
- 8e12f4a250d2 11.0 cited
Revert back-branch changes in power()'s behavior for NaN inputs.
- f74d83b3034f 10.4 cited
Avoid wrong results for power() with NaN input on more platforms.
- 6bdf1303b34b 11.0 cited
Avoid wrong results for power() with NaN input on some platforms.
- 61b200e2f582 11.0 cited
Skip full index scan during cleanup of B-tree indexes when possible
- 857f9c36cda5 11.0 cited
Rewrite the code that applies scan/join targets to paths.
- 11cf92f6e2e1 11.0 cited
Postpone generate_gather_paths for topmost scan/join rel.
- 3f90ec8597c3 11.0 cited
Add casts from jsonb
- c0cbe00fee6d 11.0 cited
Make plpgsql use its DTYPE_REC code paths for composite-type variables.
- 4b93f57999a2 11.0 cited
Don't allow VACUUM VERBOSE ANALYZE VERBOSE.
- 921059bd66c7 11.0 cited
Pass InitPlan values to workers via Gather (Merge).
- e89a71fb449a 11.0 cited
Account for the effect of lossy pages when costing bitmap scans.
- 5edc63bda68a 11.0 cited
Allow no-op GiST support functions to be omitted.
- d3a4f89d8a3e 11.0 cited
Rearm statement_timeout after each executed query.
- f8e5f156b30e 11.0 cited
Push limit through subqueries to underlying sort, where possible.
- 1f6d515a67ec 11.0 cited