Horton

Re: SCRAM with channel binding downgrade attack

Bruce Momjian <bruce@momjian.us>

View thread View on postgr.es
From: Bruce Momjian <bruce@momjian.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Robert Haas <robertmhaas@gmail.com>, Magnus Hagander <magnus@hagander.net>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Postgres hackers <pgsql-hackers@postgresql.org>, Stephen Frost <sfrost@snowman.net>
Date: 2018-06-23T14:07:01Z
Lists: pgsql-hackers 
On Sat, Jun 23, 2018 at 10:30:19PM +0900, Michael Paquier wrote:
> On Fri, Jun 22, 2018 at 11:01:53PM -0400, Bruce Momjian wrote:
> > Uh, as I am understanding it, if we don't allow clients to force channel
> > binding, then channel binding is useless because it cannot prevent
> > man-in-the-middle attacks.  I am sure some users will try to use it, and
> > not understand that it serves no purpose.  If we then allow clients to
> > force channel binding in PG 12, they will then need to fix their
> > clients.
> > 
> > I suggest that if we don't allow users to use channel binding
> > effectively that we should remove all documentation about this
> > feature.
> 
> Well, I don't agree with this position as the protocol put in place for
> SCRAM with or without channel binding perfectly allows a client to
> enforce the use channel binding.  While that's missing for libpq, other
> clients like JDBC or npgsql could perfectly implement that before this
> gets in Postgres core in the shape they want.  So I think that the docs
> should be kept.

Yes, the code is useful, but the _feature_ is not useful until some
interface allows the forcing of channel binding.  People are worried
about users having to change their API in PG 12, but the point is that
to use this feature people will have to change their API in PG 12
anyway, and it doesn't do anything useful without an interface we don't
ship, and hasn't been written, so why confuse people that it is a
feature in PG 11?

Channel binding is listed as a _major_ feature in PG 11 in the release
notes, and you can bet people are going to look at how to use it:

      Channel binding for SCRAM authentication, to prevent potential
      man-in-the-middle attacks on database connections

It should perhaps be marked in the source code section, and listed as
not useful by PG 11's libpq or any of the interfaces built on it.  We
are also going to need to communicate to people who have already looked
at the release notes that this features is not useful in PG 11 using
libpq.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

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.