Horton

Re: SCRAM with channel binding downgrade attack

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Michael Paquier <michael@paquier.xyz>, Bruce Momjian <bruce@momjian.us>, Magnus Hagander <magnus@hagander.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Stephen Frost <sfrost@snowman.net>
Date: 2018-05-25T14:44:16Z
Lists: pgsql-hackers 
On Wed, May 23, 2018 at 2:46 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> "tls-unique" and "tls-server-end-point" are overly technical to users. They
> don't care which one is used, there's no difference in security.
> Furthermore, if we add another channel binding type in the future, perhaps
> because someone finds a vulnerability in those types and a new one is added
> to address it, users would surely like to be automatically switched over to
> the new binding type. If they have "tls-unique" hardcoded in connection
> strings, that won't happen.

+1.

> So I think the options should be "prefer", "disable", and "require". That's
> also symmetrical with sslmode, which is nice.

+1.

> We could provide "tls-unique" and "tls-server-end-point" in addition to
> those, but I'd consider those to be developer only settings, useful only for
> testing the protocol.

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.  If it's discovered that
tls-unique sucks bigtime for some reason, people are going to want to
turn it off whether they are preferring channel binding or requiring
it.

> It would be nice to have a libpq setting like "authenticate_server=require",
> which would mean "I want man-in-the-middle protection". With that, a
> connection would be allowed, if either the server's SSL certificate is
> verified as with "sslmode=verify-full", *or* SCRAM authentication with
> channel binding was used. Or perhaps cram it into sslmode,
> "sslmode=verify-full-or-scram-channel-binding", just with a nicer name. (We
> can do that after v11 though, I think.)

IMHO we could do all of this after v11.  This seems like late
development being jammed through after beta1 to me.  But I just work
here.

Semantically, I see the value of an option that means basically
mitm_protection=true, but in practice I'm not sure there is any real
advantage over having the user just specify either ssmode=verify-full
or channelbinding=require depending on the technology they wish to
use.  They probably have a specific technology in mind; it's hard to
see how they're going to get an environment configured otherwise.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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.