Thread: Postgres 11 release notes

Postgres 11 release notes

From

Bruce Momjian

Date:

11 May 2018, 21:08:52

I have committed the first draft of the Postgres 11 release notes.  I
will add more markup soon.  You can view the most current version here:

    http://momjian.us/pgsql_docs/release-11.html

I expect a torrent of feedback.  ;-)

On a personal note, I want to apologize for not adequately championing
two features that I think have strategic significance but didn't make it
into Postgres 11:  parallel FDW access and improved multi-variate
statistics.  I hope to do better job during Postgres 12.

-- 
  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 +

Re: Postgres 11 release notes

From

Fabien COELHO

Date:

11 May 2018, 22:29:39

Hello Bruce,

>     http://momjian.us/pgsql_docs/release-11.html

> I expect a torrent of feedback.  ;-)

Here is some, for things I know about:

>> Add major scripting features to pgbench (Fabien Coelho)

I assume that you are refering to "bc7fa0c1". I do not think that anything 
qualifies as "major".

I would rather describe it as "Add support for NULL and boolean, as well 
as assorted operators and functions, to pgbench expressions". Or something 
in better English.

>> Add \if macro support to pgbench (Fabien Coelho)

I would remove the word "macro", as it is not a pre-compilation feature, 
it is just a somehow simple standard conditional.

> On a personal note, I want to apologize for not adequately championing
> two features that I think have strategic significance but didn't make it
> into Postgres 11:  parallel FDW access and improved multi-variate
> statistics.

I'm planning to try to help with the later.

-- 
Fabien.

Re: Postgres 11 release notes

From

Teodor Sigaev

Date:

11 May 2018, 22:49:50

Bruce Momjian wrote:
> I have committed the first draft of the Postgres 11 release notes.  I
> will add more markup soon.  You can view the most current version here:
> 
>     http://momjian.us/pgsql_docs/release-11.html
> 
> I expect a torrent of feedback.  ;-)
Hi!

Seems, you miss:
857f9c36cda520030381bd8c2af20adf0ce0e1d4 Skip full index scan during cleanup of 
B-tree indexes when possible
c0cbe00fee6d0a5e0ec72c6d68a035e674edc4cc Add explicit cast from scalar jsonb to 
all numeric and bool types.

Pls, edit:
Add functionjson(b)_to_tsvector to create usable text search queries matching 
JSON/JSONB values (Dmitry Dolgov)
to
Add function json(b)_to_tsvector to create usable vectors to search in json(b) 
(Dmitry Dolgov)
or somehow else. Your wording is about query but actually that functions are 
about how to create tsvector from json(b)

Thank you!

-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/

Re: Postgres 11 release notes

From

Chapman Flack

Date:

11 May 2018, 23:40:54

On 05/11/2018 11:08 AM, Bruce Momjian wrote:

>     http://momjian.us/pgsql_docs/release-11.html
> 
> I expect a torrent of feedback.  ;-)

Very superficial things:

Add predicate locking for Hash, GiST, and GIN indexes

  s/likelyhood/likelihood/

Add extension jsonb_plpython

  There are two such entries; one looks like it should be plperl.

Improve the speed of aggregate computations

  This entry is missing a bullet.

-Chap

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

12 May 2018, 00:01:13

On Fri, May 11, 2018 at 06:29:39PM +0200, Fabien COELHO wrote:
> 
> Hello Bruce,
> 
> >    http://momjian.us/pgsql_docs/release-11.html
> 
> >I expect a torrent of feedback.  ;-)
> 
> Here is some, for things I know about:
> 
> >>Add major scripting features to pgbench (Fabien Coelho)
> 
> I assume that you are refering to "bc7fa0c1". I do not think that anything
> qualifies as "major".
> 
> I would rather describe it as "Add support for NULL and boolean, as well as
> assorted operators and functions, to pgbench expressions". Or something in
> better English.

OK, new wording is:

        Add pgbench expressions support for NULLs, booleans, and some
        functions and operators (Fabien Coelho)

> >>Add \if macro support to pgbench (Fabien Coelho)
> 
> I would remove the word "macro", as it is not a pre-compilation feature, it
> is just a somehow simple standard conditional.

New wording is:

        Add \if conditional support to pgbench (Fabien Coelho)

> >On a personal note, I want to apologize for not adequately championing
> >two features that I think have strategic significance but didn't make it
> >into Postgres 11:  parallel FDW access and improved multi-variate
> >statistics.
> 
> I'm planning to try to help with the later.

Great, thanks.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

12 May 2018, 00:44:06

On Fri, May 11, 2018 at 07:49:50PM +0300, Teodor Sigaev wrote:
> 
> 
> Bruce Momjian wrote:
> >I have committed the first draft of the Postgres 11 release notes.  I
> >will add more markup soon.  You can view the most current version here:
> >
> >    http://momjian.us/pgsql_docs/release-11.html
> >
> >I expect a torrent of feedback.  ;-)
> Hi!
> 
> Seems, you miss:
> 857f9c36cda520030381bd8c2af20adf0ce0e1d4 Skip full index scan during cleanup
> of B-tree indexes when possible

I read that and thought it was too details to be in the release notes. 
It is not that it is unimportant, but it is hard to see how people would
notice the difference or change their behavior based on this change.

> c0cbe00fee6d0a5e0ec72c6d68a035e674edc4cc Add explicit cast from scalar jsonb
> to all numeric and bool types.
> 
> Pls, edit:
> Add functionjson(b)_to_tsvector to create usable text search queries
> matching JSON/JSONB values (Dmitry Dolgov)
> to
> Add function json(b)_to_tsvector to create usable vectors to search in
> json(b) (Dmitry Dolgov)
> or somehow else. Your wording is about query but actually that functions are
> about how to create tsvector from json(b)

OK, how is this text?

        Add function <function>json(b)_to_tsvector()</function> to create
        text search query for matching <type>JSON</type>/<type>JSONB
        </type>values (Dmitry Dolgov)

I have made this change but can adjust it some more.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

12 May 2018, 00:50:39

On Fri, May 11, 2018 at 01:40:54PM -0400, Chapman Flack wrote:
> On 05/11/2018 11:08 AM, Bruce Momjian wrote:
> 
> >     http://momjian.us/pgsql_docs/release-11.html
> > 
> > I expect a torrent of feedback.  ;-)
> 
> Very superficial things:
> 
> 
> Add predicate locking for Hash, GiST, and GIN indexes
> 
>   s/likelyhood/likelihood/
> 
> Add extension jsonb_plpython
> 
>   There are two such entries; one looks like it should be plperl.
> 
> Improve the speed of aggregate computations
> 
>   This entry is missing a bullet.

Great, all fixed, and the URL contents are updated with this and
previous suggestions, plus more markup.

-- 
  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 +

Re: Postgres 11 release notes

From

Andres Freund

Date:

12 May 2018, 00:50:51

On 2018-05-11 14:44:06 -0400, Bruce Momjian wrote:
> On Fri, May 11, 2018 at 07:49:50PM +0300, Teodor Sigaev wrote:
> > 
> > 
> > Bruce Momjian wrote:
> > >I have committed the first draft of the Postgres 11 release notes.  I
> > >will add more markup soon.  You can view the most current version here:
> > >
> > >    http://momjian.us/pgsql_docs/release-11.html
> > >
> > >I expect a torrent of feedback.  ;-)
> > Hi!
> > 
> > Seems, you miss:
> > 857f9c36cda520030381bd8c2af20adf0ce0e1d4 Skip full index scan during cleanup
> > of B-tree indexes when possible
> 
> I read that and thought it was too details to be in the release notes. 
> It is not that it is unimportant, but it is hard to see how people would
> notice the difference or change their behavior based on this change.

It's a *huge* performance problem in larger installations
currently. When you have a multi-TB relation and correspondingly large
relation, the VM allows to make the heap cleanups cheap, but then the
index scan takes just about forever.  I know at least one large PG user
that moved off postgres because of it.  This won't solve all of those
concerns, but it definitely is crucial to know for such users.

People would notice by vacuums of large relations not taking forever
anymore. And the behaviour change would be to a) upgrade b) tune the
associated reloption/GUC.

Greetings,

Andres Freund

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

12 May 2018, 00:59:04

On Fri, May 11, 2018 at 11:50:51AM -0700, Andres Freund wrote:
> On 2018-05-11 14:44:06 -0400, Bruce Momjian wrote:
> > On Fri, May 11, 2018 at 07:49:50PM +0300, Teodor Sigaev wrote:
> > > 
> > > 
> > > Bruce Momjian wrote:
> > > >I have committed the first draft of the Postgres 11 release notes.  I
> > > >will add more markup soon.  You can view the most current version here:
> > > >
> > > >    http://momjian.us/pgsql_docs/release-11.html
> > > >
> > > >I expect a torrent of feedback.  ;-)
> > > Hi!
> > > 
> > > Seems, you miss:
> > > 857f9c36cda520030381bd8c2af20adf0ce0e1d4 Skip full index scan during cleanup
> > > of B-tree indexes when possible
> > 
> > I read that and thought it was too details to be in the release notes. 
> > It is not that it is unimportant, but it is hard to see how people would
> > notice the difference or change their behavior based on this change.
> 
> It's a *huge* performance problem in larger installations
> currently. When you have a multi-TB relation and correspondingly large
> relation, the VM allows to make the heap cleanups cheap, but then the
> index scan takes just about forever.  I know at least one large PG user
> that moved off postgres because of it.  This won't solve all of those
> concerns, but it definitely is crucial to know for such users.
> 
> People would notice by vacuums of large relations not taking forever
> anymore. And the behaviour change would be to a) upgrade b) tune the
> associated reloption/GUC.

OK, so what is the text that people will understand?  This?

    Prevent manual VACUUMs on append-only tables from performing
    needless index scans

You can see why I was hesitant to include it, based on this text, but I
am happy to add it.

-- 
  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 +

Re: Postgres 11 release notes

From

Andres Freund

Date:

12 May 2018, 00:59:12

Hi,

Quick notes:
>       <para>
>        Add Just-In-Time (<acronym>JIT</acronym>) compilation of plans
>        run the by the executor
>(Andres Freund)
>       </para>
>      </listitem>

It's currently not yet compilation of entire plans, but only parts. I
think that's a relevant distinction because I'd like to add that as a
feature to v12 ;). How about just adding 'parts of' or such?  I'd also
rephrase the plan and 'run by the executor a bit'. How about:

  Add Just-In-Time (<acronym>JIT</acronym>) compilation of parts of
  queries, to accelerate their execution.


>      <listitem>
><!--
>2018-03-20 [5b2526c83] Add configure infrastructure (- -with-llvm) to enable LLV
>-->
>
>       <para>
>        Add configure flag <option>--with-llvm</option> to test for
>        <acronym>LLVM</acronym> support (Andres Freund)
>       </para>
>      </listitem>
>
>      <listitem>
><!--
>2018-03-20 [6869b4f25] Add C++ support to configure.
>-->
>
>       <para>
>        Have configure check for the availability of a C++ compiler
>        (Andres Freund)
>       </para>
>      </listitem>
>
>      <listitem>

I wonder if we shouldn't omit those, considering them part of the JIT
entry?  Not quite sure what our policy there is.

Greetings,

Andres Freund

Re: Postgres 11 release notes

From

Alvaro Herrera

Date:

12 May 2018, 01:00:58

Bruce Momjian wrote:

> OK, so what is the text that people will understand?  This?
> 
>     Prevent manual VACUUMs on append-only tables from performing
>     needless index scans

Make vacuum cheaper by avoiding scans of btree indexes when not
necessary
?

Why "manual vacuum"?  It's a problem with vacuums invoked from
autovacuum too.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Postgres 11 release notes

From

Alvaro Herrera

Date:

12 May 2018, 01:02:36

Andres Freund wrote:

> >       <para>
> >        Add configure flag <option>--with-llvm</option> to test for
> >        <acronym>LLVM</acronym> support (Andres Freund)
> >       </para>
> >       <para>
> >        Have configure check for the availability of a C++ compiler
> >        (Andres Freund)
> >       </para>
> 
> I wonder if we shouldn't omit those, considering them part of the JIT
> entry?  Not quite sure what our policy there is.

I don't see why users would be interested in these items at all, so +1
for omitting them.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Postgres 11 release notes

From

Andres Freund

Date:

12 May 2018, 01:04:10

On 2018-05-11 14:59:04 -0400, Bruce Momjian wrote:
> On Fri, May 11, 2018 at 11:50:51AM -0700, Andres Freund wrote:
> > On 2018-05-11 14:44:06 -0400, Bruce Momjian wrote:
> > > On Fri, May 11, 2018 at 07:49:50PM +0300, Teodor Sigaev wrote:
> > > > 
> > > > 
> > > > Bruce Momjian wrote:
> > > > >I have committed the first draft of the Postgres 11 release notes.  I
> > > > >will add more markup soon.  You can view the most current version here:
> > > > >
> > > > >    http://momjian.us/pgsql_docs/release-11.html
> > > > >
> > > > >I expect a torrent of feedback.  ;-)
> > > > Hi!
> > > > 
> > > > Seems, you miss:
> > > > 857f9c36cda520030381bd8c2af20adf0ce0e1d4 Skip full index scan during cleanup
> > > > of B-tree indexes when possible
> > > 
> > > I read that and thought it was too details to be in the release notes. 
> > > It is not that it is unimportant, but it is hard to see how people would
> > > notice the difference or change their behavior based on this change.
> > 
> > It's a *huge* performance problem in larger installations
> > currently. When you have a multi-TB relation and correspondingly large
> > relation, the VM allows to make the heap cleanups cheap, but then the
> > index scan takes just about forever.  I know at least one large PG user
> > that moved off postgres because of it.  This won't solve all of those
> > concerns, but it definitely is crucial to know for such users.
> > 
> > People would notice by vacuums of large relations not taking forever
> > anymore. And the behaviour change would be to a) upgrade b) tune the
> > associated reloption/GUC.
> 
> OK, so what is the text that people will understand?  This?
> 
>     Prevent manual VACUUMs on append-only tables from performing
>     needless index scans

I don't think the table being 'append-only' is necessary?  Nor does it
have to be a manual vacuum. And 'needless index scan' sounds less than
it is/was, namely a full scan of the index. Perhaps something like:

  Allow vacuum to skip doing a full scan of btree indexes after VACUUM,
  if not necessary.

or something like that?


> You can see why I was hesitant to include it, based on this text, but I
> am happy to add it.

I can't. Even if the above were accurate it'd be relevant information.

Greetings,

Andres Freund

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

12 May 2018, 01:11:31

On Fri, May 11, 2018 at 04:00:58PM -0300, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> 
> > OK, so what is the text that people will understand?  This?
> > 
> >     Prevent manual VACUUMs on append-only tables from performing
> >     needless index scans
> 
> Make vacuum cheaper by avoiding scans of btree indexes when not
> necessary
> ?
> 
> Why "manual vacuum"?  It's a problem with vacuums invoked from
> autovacuum too.

Uh, from the commit it says:

    When workload on particular table is append-only, then autovacuum
    isn't intended to touch this table. However, user may run vacuum
    manually in order to fill visibility map and get benefits of
    --------
    index-only scans.  Then ambulkdelete wouldn't be called for
    indexes of such table (because no heap tuples were deleted), only
                                   ---------------------------
    amvacuumcleanup would be called In this case, amvacuumcleanup
    would perform full index scan for two objectives: put recyclable
    pages into free space map and update index statistics.

Why would autovacuum run on a table with no expired index entries?

Personally, I think the fact that autovacuum doesn't run on suvch tables
and therefore doesn't automatically do index-only scans is a problem. I
tried to fix it years ago, but failed and gave up.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

12 May 2018, 01:14:24

On Fri, May 11, 2018 at 11:59:12AM -0700, Andres Freund wrote:
> Hi,
> 
> Quick notes:
> >       <para>
> >        Add Just-In-Time (<acronym>JIT</acronym>) compilation of plans
> >        run the by the executor
> >(Andres Freund)
> >       </para>
> >      </listitem>
> 
> It's currently not yet compilation of entire plans, but only parts. I
> think that's a relevant distinction because I'd like to add that as a
> feature to v12 ;). How about just adding 'parts of' or such?  I'd also
> rephrase the plan and 'run by the executor a bit'. How about:
> 
>   Add Just-In-Time (<acronym>JIT</acronym>) compilation of parts of
>   queries, to accelerate their execution.

OK, new text:

        Add Just-In-Time (<acronym>JIT</acronym>) compilation of some
        parts of query plans to improve execution speed (Andres Freund)

> >      <listitem>
> ><!--
> >2018-03-20 [5b2526c83] Add configure infrastructure (- -with-llvm) to enable LLV
> >-->
> >
> >       <para>
> >        Add configure flag <option>--with-llvm</option> to test for
> >        <acronym>LLVM</acronym> support (Andres Freund)
> >       </para>
> >      </listitem>
> >
> >      <listitem>
> ><!--
> >2018-03-20 [6869b4f25] Add C++ support to configure.
> >-->
> >
> >       <para>
> >        Have configure check for the availability of a C++ compiler
> >        (Andres Freund)
> >       </para>
> >      </listitem>
> >
> >      <listitem>
> 
> I wonder if we shouldn't omit those, considering them part of the JIT
> entry?  Not quite sure what our policy there is.

OK, removed.

-- 
  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 +

Re: Postgres 11 release notes

From

Peter Geoghegan

Date:

12 May 2018, 01:17:47

On Fri, May 11, 2018 at 12:04 PM, Andres Freund <andres@anarazel.de> wrote:
> I don't think the table being 'append-only' is necessary?  Nor does it
> have to be a manual vacuum. And 'needless index scan' sounds less than
> it is/was, namely a full scan of the index. Perhaps something like:
>
>   Allow vacuum to skip doing a full scan of btree indexes after VACUUM,
>   if not necessary.
>
> or something like that?

I suggest "Allow vacuuming to avoid full index scans for indexes when
there are no dead tuples found in a table. Where necessary, the
behavior can be adjusted via the new configuration parameter
vacuum_cleanup_index_scale_factor."

Also:

* "Allow indexes to be built in parallel" should specify that it only
works for B-Tree index builds.

* Suggest replacement sort item be phrased as: "Remove the
configuration parameter replacement_sort_tuples. <newline> The
replacement selection sort algorithm is no longer used."

-- 
Peter Geoghegan

Re: Postgres 11 release notes

From

Teodor Sigaev

Date:

12 May 2018, 01:20:17

>> 857f9c36cda520030381bd8c2af20adf0ce0e1d4 Skip full index scan during cleanup
>> of B-tree indexes when possible
> 
> I read that and thought it was too details to be in the release notes.
> It is not that it is unimportant, but it is hard to see how people would
> notice the difference or change their behavior based on this change.
Hm, it could be something like:
Add possibility to miss scan indexes on append-only table, it decreases 
vacuum time on such tables.

> 
>> c0cbe00fee6d0a5e0ec72c6d68a035e674edc4cc Add explicit cast from scalar jsonb
>> to all numeric and bool types.
>>
>> Pls, edit:
>> Add functionjson(b)_to_tsvector to create usable text search queries
>> matching JSON/JSONB values (Dmitry Dolgov)
>> to
>> Add function json(b)_to_tsvector to create usable vectors to search in
>> json(b) (Dmitry Dolgov)
>> or somehow else. Your wording is about query but actually that functions are
>> about how to create tsvector from json(b)
> 
> OK, how is this text?
> 
>          Add function <function>json(b)_to_tsvector()</function> to create
>          text search query for matching <type>JSON</type>/<type>JSONB
>          </type>values (Dmitry Dolgov)
Not query - *_to_tsvector functions produce a tsvector. So:

Add function <function>json(b)_to_tsvector()</function> to use 
customizable full text search over json(b) columns.


-- 
Teodor Sigaev                      E-mail: teodor@sigaev.ru
                                       WWW: http://www.sigaev.ru/

Re: Postgres 11 release notes

From

Andres Freund

Date:

12 May 2018, 01:22:08

On 2018-05-11 15:11:31 -0400, Bruce Momjian wrote:
> On Fri, May 11, 2018 at 04:00:58PM -0300, Alvaro Herrera wrote:
> > Bruce Momjian wrote:
> > 
> > > OK, so what is the text that people will understand?  This?
> > > 
> > >     Prevent manual VACUUMs on append-only tables from performing
> > >     needless index scans
> > 
> > Make vacuum cheaper by avoiding scans of btree indexes when not
> > necessary
> > ?
> > 
> > Why "manual vacuum"?  It's a problem with vacuums invoked from
> > autovacuum too.
> 
> Uh, from the commit it says:
> 
>     When workload on particular table is append-only, then autovacuum
>     isn't intended to touch this table. However, user may run vacuum
>     manually in order to fill visibility map and get benefits of
>     --------
>     index-only scans.  Then ambulkdelete wouldn't be called for
>     indexes of such table (because no heap tuples were deleted), only
>                                    ---------------------------
>     amvacuumcleanup would be called In this case, amvacuumcleanup
>     would perform full index scan for two objectives: put recyclable
>     pages into free space map and update index statistics.
> 
> Why would autovacuum run on a table with no expired index entries?

Anti-Wraparound is one case. One where it's really painful to take
forever on lots of unchanged tables. Lots of hot updates another.

Btw, is it just me, or do the commit and docs confuse say stalled when
stale is intended?


> Personally, I think the fact that autovacuum doesn't run on suvch tables
> and therefore doesn't automatically do index-only scans is a problem. I
> tried to fix it years ago, but failed and gave up.

I'm really unhappy about that too.

Greetings,

Andres Freund

Re: Postgres 11 release notes

From

Justin Pryzby

Date:

12 May 2018, 01:26:12

On Fri, May 11, 2018 at 12:22:08PM -0700, Andres Freund wrote:
> Btw, is it just me, or do the commit and docs confuse say stalled when
> stale is intended?

Should be fixed since yesterday's 8e12f4a250d250a89153da2eb9b91c31bb80c483 ?

Justin

Re: Postgres 11 release notes

From

Peter Geoghegan

Date:

12 May 2018, 01:28:57

On Fri, May 11, 2018 at 12:17 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> * Suggest replacement sort item be phrased as: "Remove the
> configuration parameter replacement_sort_tuples. <newline> The
> replacement selection sort algorithm is no longer used."

Also, it should be moved to "Migration to Version 11", since the only
issue here is compatibility with older versions.

-- 
Peter Geoghegan

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

12 May 2018, 03:03:56

On Fri, May 11, 2018 at 12:17:47PM -0700, Peter Geoghegan wrote:
> On Fri, May 11, 2018 at 12:04 PM, Andres Freund <andres@anarazel.de> wrote:
> > I don't think the table being 'append-only' is necessary?  Nor does it
> > have to be a manual vacuum. And 'needless index scan' sounds less than
> > it is/was, namely a full scan of the index. Perhaps something like:
> >
> >   Allow vacuum to skip doing a full scan of btree indexes after VACUUM,
> >   if not necessary.
> >
> > or something like that?
> 
> I suggest "Allow vacuuming to avoid full index scans for indexes when
> there are no dead tuples found in a table. Where necessary, the
> behavior can be adjusted via the new configuration parameter
> vacuum_cleanup_index_scale_factor."
> 
> Also:
> 
> * "Allow indexes to be built in parallel" should specify that it only
> works for B-Tree index builds.
> 
> * Suggest replacement sort item be phrased as: "Remove the
> configuration parameter replacement_sort_tuples. <newline> The
> replacement selection sort algorithm is no longer used."

All done.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

12 May 2018, 03:04:05

On Fri, May 11, 2018 at 12:28:57PM -0700, Peter Geoghegan wrote:
> On Fri, May 11, 2018 at 12:17 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> > * Suggest replacement sort item be phrased as: "Remove the
> > configuration parameter replacement_sort_tuples. <newline> The
> > replacement selection sort algorithm is no longer used."
> 
> Also, it should be moved to "Migration to Version 11", since the only
> issue here is compatibility with older versions.

Done.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

12 May 2018, 03:06:52

On Fri, May 11, 2018 at 10:20:17PM +0300, Teodor Sigaev wrote:
> >>857f9c36cda520030381bd8c2af20adf0ce0e1d4 Skip full index scan during cleanup
> >>of B-tree indexes when possible
> >
> >I read that and thought it was too details to be in the release notes.
> >It is not that it is unimportant, but it is hard to see how people would
> >notice the difference or change their behavior based on this change.
> Hm, it could be something like:
> Add possibility to miss scan indexes on append-only table, it decreases
> vacuum time on such tables.
> 
> >
> >>c0cbe00fee6d0a5e0ec72c6d68a035e674edc4cc Add explicit cast from scalar jsonb
> >>to all numeric and bool types.
> >>
> >>Pls, edit:
> >>Add functionjson(b)_to_tsvector to create usable text search queries
> >>matching JSON/JSONB values (Dmitry Dolgov)
> >>to
> >>Add function json(b)_to_tsvector to create usable vectors to search in
> >>json(b) (Dmitry Dolgov)
> >>or somehow else. Your wording is about query but actually that functions are
> >>about how to create tsvector from json(b)
> >
> >OK, how is this text?
> >
> >         Add function <function>json(b)_to_tsvector()</function> to create
> >         text search query for matching <type>JSON</type>/<type>JSONB
> >         </type>values (Dmitry Dolgov)
> Not query - *_to_tsvector functions produce a tsvector. So:
> 
> Add function <function>json(b)_to_tsvector()</function> to use customizable
> full text search over json(b) columns.

Done and URL updated.

-- 
  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 +

Re: Postgres 11 release notes

From

Peter Geoghegan

Date:

13 May 2018, 02:22:55

On Fri, May 11, 2018 at 2:06 PM, Bruce Momjian <bruce@momjian.us> wrote:
> Done and URL updated.

I have some feedback on "Allow NOT NULL to be added to columns without
requiring a table rewrite". I suggest that this be phrased as "Avoid a
table rewrite when ALTER TABLE ADD COLUMN sets a default value for a
column". Referencing the fact that Postgres was previously able to do
this with a NULL default doesn't seem to add anything.


-- 
Peter Geoghegan

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

13 May 2018, 06:47:08

On Sat, May 12, 2018 at 01:22:55PM -0700, Peter Geoghegan wrote:
> On Fri, May 11, 2018 at 2:06 PM, Bruce Momjian <bruce@momjian.us> wrote:
> > Done and URL updated.
> 
> I have some feedback on "Allow NOT NULL to be added to columns without
> requiring a table rewrite". I suggest that this be phrased as "Avoid a
> table rewrite when ALTER TABLE ADD COLUMN sets a default value for a
> column". Referencing the fact that Postgres was previously able to do
> this with a NULL default doesn't seem to add anything.

Agreed, done:

        Allow <command>ALTER TABLE</command> to add a non-null default
        column without a table rewrite (Andrew Dunstan, Serge Rielau)

-- 
  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 +

Re: Postgres 11 release notes

From

Đặng Minh Hướng

Date:

13 May 2018, 14:43:25

Hello Bruce,

Thanks for the greate work!

2018-05-12 0:08 GMT+09:00 Bruce Momjian <bruce@momjian.us>:

I have committed the first draft of the Postgres 11 release notes. I
will add more markup soon. You can view the most current version here:

http://momjian.us/pgsql_docs/release-11.html

I expect a torrent of feedback. ;-)

I think the behavior change (from PostgreSQL 11) of power() with NaN below also need to report to users as the major change.

related commit: 61b200e2f582d0886d9de947e182483339d881fd

6bdf1303b34bc630e8945ae3407ec7e8395c8fe5

Discussion: https://postgr.es/m/75DB81BEEA95B445AE6D576A0A5C9E936A73E741@BPXM05GP.gisp.nec.co.jp

Thanks and best regards,

---

Dang Minh Huong

Re: Postgres 11 release notes

From

Amit Langote

Date:

13 May 2018, 15:53:26

On Sat, May 12, 2018 at 12:08 AM, Bruce Momjian <bruce@momjian.us> wrote:
> I have committed the first draft of the Postgres 11 release notes.  I
> will add more markup soon.  You can view the most current version here:
>
>         http://momjian.us/pgsql_docs/release-11.html

Thank you for compiling this, Bruce!

> I expect a torrent of feedback.  ;-)

The following item

+<!--
+2017-11-23 [05b6ec39d] Show partition info from psql \d+
+-->
+
+       <para>
+        Have psql \d+ show a partition count of zero (Amit Langote)
+       </para>

missed mentioning Ashutosh Bapat as an author.

Maybe, the item name and description text could be improved as follows:

+       <para>
+        Have psql \d+ always show the partition information (Amit
Langote, Ashutosh Bapat)
+       </para>
+
+       <para>
+        Previously partition information would not be displayed for a
partitioned table if
+        it had no partitions.  Also indicate which partitions are
themselves partitioned.
+       </para>


Thanks,
Amit

Re: Postgres 11 release notes

From

Amit Kapila

Date:

13 May 2018, 17:10:19

On Fri, May 11, 2018 at 8:38 PM, Bruce Momjian <bruce@momjian.us> wrote:
> I have committed the first draft of the Postgres 11 release notes.  I
> will add more markup soon.  You can view the most current version here:
>
>         http://momjian.us/pgsql_docs/release-11.html
>
> I expect a torrent of feedback.  ;-)
>

Thanks for compiling the first draft.  I think there are few things
which we should add to the Parallel Queries section. (a) commit
e89a71fb449af2ef74f47be1175f99956cf21524 -
Pass InitPlan values to workers via Gather (Merge).  We can write
something like "Allow the part of the plan that references an initplan
to run in parallel" or  "Parallelise queries containing initplans",
(b) "Parallelise queries that contains expensive functions in target
lists" (Commits - 3f90ec8597c3515e0d3190613b31491686027e4b and
11cf92f6e2e13c0a6e3f98be3e629e6bd90b74d5).

Both of these features can parallelize queries in many workloads.  The
feature (a) has sped up some of the TPC-H queries especially the cases
where InitPlan itself contains Gather node and feature (b) is also
quite helpful with PostGIS functions, see
https://carto.com/blog/inside/postgres-parallel/

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: Postgres 11 release notes

From

Amit Kapila

Date:

13 May 2018, 17:25:35

On Sun, May 13, 2018 at 4:40 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Fri, May 11, 2018 at 8:38 PM, Bruce Momjian <bruce@momjian.us> wrote:
>> I have committed the first draft of the Postgres 11 release notes.  I
>> will add more markup soon.  You can view the most current version here:
>>
>>         http://momjian.us/pgsql_docs/release-11.html
>>
>> I expect a torrent of feedback.  ;-)
>>
>
> Thanks for compiling the first draft.
>

One more comment on below item:

Allow entire hash index pages to be scanned (Ashutosh Sharma)

Previously each hash index entry has to be locked and scanned separately.

I think it is better to write the second line as "Previously for each
hash index entry, we need to refind the scan position within the page
and cuts down on lock/unlock traffic.".

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: Postgres 11 release notes

From

Andrew Dunstan

Date:

14 May 2018, 17:47:54


On 05/12/2018 08:47 PM, Bruce Momjian wrote:
> On Sat, May 12, 2018 at 01:22:55PM -0700, Peter Geoghegan wrote:
>> On Fri, May 11, 2018 at 2:06 PM, Bruce Momjian <bruce@momjian.us> wrote:
>>> Done and URL updated.
>> I have some feedback on "Allow NOT NULL to be added to columns without
>> requiring a table rewrite". I suggest that this be phrased as "Avoid a
>> table rewrite when ALTER TABLE ADD COLUMN sets a default value for a
>> column". Referencing the fact that Postgres was previously able to do
>> this with a NULL default doesn't seem to add anything.
> Agreed, done:
>
>         Allow <command>ALTER TABLE</command> to add a non-null default
>         column without a table rewrite (Andrew Dunstan, Serge Rielau)
>


I think "a column with a non-null default" would be a bit clearer than
"a non-null default column".

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Postgres 11 release notes

From

Dilip Kumar

Date:

14 May 2018, 18:04:54

On Fri, May 11, 2018 at 8:38 PM, Bruce Momjian <bruce@momjian.us> wrote:

I have committed the first draft of the Postgres 11 release notes. I
will add more markup soon. You can view the most current version here:

http://momjian.us/pgsql_docs/release-11.html

I expect a torrent of feedback. ;-)

On a personal note, I want to apologize for not adequately championing
two features that I think have strategic significance but didn't make it
into Postgres 11: parallel FDW access and improved multi-variate
statistics. I hope to do better job during Postgres 12.

--
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 +

I think the below commit is missed in the release notes?

5edc63bda68a77c4d38f0cbeae1c4271f9ef4100

Committer: Robert Haas <rhaas@postgresql.org> 2017-11-10 13:50:50

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

Dilip Kumar, reviewed by Alexander Kumenkov, Amul Sul, and me.

Some final adjustments by me.

As part of this commit, we are also accounting for the lossy pages during bitmap costing. This will consider

the effect of work_mem while selecting the bitmap heap scan path.

Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

15 May 2018, 02:04:58

On Sun, May 13, 2018 at 03:43:08PM +0900, Michael Paquier wrote:
> On Fri, May 11, 2018 at 11:08:52AM -0400, Bruce Momjian wrote:
> > I have committed the first draft of the Postgres 11 release notes.  I
> > will add more markup soon.  You can view the most current version
> > here:
> 
> Thanks for gathering all the commits in one piece, Bruce.
> 
> > I expect a torrent of feedback.  ;-)
> 
> I looked at the entries where my name shows up.  Here is some feedback
> with HEAD at 8c6227a2 (latest as of writing this message).
> 
> <para>
>  Add information_schema columns related to table constraints and
>  triggers (Michael Paquier)
> </para>
> The author of this entry is Peter Eisentraut, not me.

Thanks, I got "Reviewed-by" and "Author" mixed up.

> <para>
>  Channel binding requires the server end
>  of the <acronym>TLS</acronym> connection to
>  prove that it knows the password.  The options are <link
>  linkend="libpq-scram-channel-binding"><option>scram_channel_binding=tls-unique</option></link>
>  and <option>scram_channel_binding=tls-server-end-point</option>.
> </para>
> This is not actually correct.  Channel binding is an MITM prevention
> mechanism which makes sure that after the SSL handshake the backend and
> the frontend are still connected to the same things.  "tls-unique" makes
> sure that a connection is uniquely used using a hash of the TLS finish
>  message, and end-point makes sure that the endpoints are the same using
>  a hash of the server certificate.

So, channel binding has had me confused since I first heard about it.  I
have done some research and reworded the commit with the attached first patch.

Also, I have created a second patch which actually explains the two
SCRAM channel binding options and how the work.

One question I do have is how do we prevent a fake server in the middle
from pretending it is a PG 10 server and therefore avoiding channel
binding protections?  I don't see any channel binding options in
pg_hba.conf, and while libpq has options, they are explained with "This
parameter is mainly intended for protocol testing."

> <para>
>  WHAT DOES THIS DOC TEXT MEAN?  "An empty value specifies that
>  the client will not use channel binding.  The default value
>  is tls-unique."
> </para>
> This means that the client can choose to not use channel binding (which
> sends a 'n' flag if you refer to the communication protocol of SCRAM),
> even if the server has advertised to the client channel binding.  So
> this provides a way to disable the feature at will, an on/off switch if
> you want.  If a v10 libpq tries to connect to a v11 server, then it
> won't use channel binding automatically.  That may be worth adding to
> the documentation as well.

I have updated the docs in the second patch to explain this.

> <para>
>  Allow access to file system functions to be controlled by
>  <command>GRANT</command>/<command>REVOKE</command> permissions,
>  rather than super-user checks (Michael Paquier)
> </para>
> Author is Stephen Frost here.

Done.

> <para>
>  Use <command>GRANT</command>/<command>REVOKE</command>
>  to control access to <link
>  linkend="lo-import"><function>lo_import()</function></link>
>  and <function>lo_export()</function> (Michael Paquier)
> </para>
> Tom Lane is a co-author here I think.

Done.

> <para>
>  Add libpq parameter to allow physical and logical replication
>  connections (Michael Paquier)
> </para>
> This commit has just added documentation which was missing and
> incomplete.  I would suggest to remove it from the release notes as no
> new feature has been added.

Removed.

> <para>
>  Add <link
>  linkend="app-pgreceivewal"><application>pg_receivewal</application></link>
>  option <option>--no-sync</option> to prevent synchronous
>  <acronym>WAL</acronym> writes (Michael Paquier)
> </para>
> Perhaps this should be rewritten?  --no-sync just disables any fsync
> calls for WAL segments, which is useful for tests, not recommended for
> production environments.

Done.

> <para>
>  Prevent <application>pg_rewind</application> from running as
>  <literal>root</literal> (Magnus Hagander)
> </para>
> This one's authorship is actually mine, after a bug I found :)

Done, thanks much.

-- 
  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 +

On Wed, May 16, 2018 at 4:08 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2018-May-15, Bruce Momjian wrote:

> On Wed, May 16, 2018 at 09:01:35AM +0900, Tatsuo Ishii wrote:
> > There's a small typo.
> >
> > > Add support for with huge(large) pages on Windows (Takayuki Tsunakawa, Thomas Munro)
> >
> > I think a space between "huge" and "(large)" is needed.
>
> Done, URL updated.

I'm not sure why we say "huge (large) pages". The natural term for
Windows is "large-pages",
https://msdn.microsoft.com/en-us/library/windows/desktop/aa366720(v=vs.85).aspx
so I think we should use that terminology. Maybe something like

Add support for <firstterm>large pages</firstterm> on Windows (Takayuki Tsunakawa, Thomas Munro)
This is controlled by the <link>huge_pages</link> configuration parameter.

I assume the point here is that we the term huge pages is what's used in PostgreSQL. For example, they are still configured using the huge_pages GUC.

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: Postgres 11 release notes

From

Alvaro Herrera

Date:

16 May 2018, 22:24:57

On 2018-May-16, Magnus Hagander wrote:

> On Wed, May 16, 2018 at 4:08 PM, Alvaro Herrera <alvherre@2ndquadrant.com>
> wrote:

> > I'm not sure why we say "huge (large) pages".  The natural term for
> > Windows is "large-pages",
> > https://msdn.microsoft.com/en-us/library/windows/desktop/
> > aa366720(v=vs.85).aspx
> > so I think we should use that terminology.  Maybe something like
> >
> > Add support for <firstterm>large pages</firstterm> on Windows (Takayuki
> > Tsunakawa, Thomas Munro)
> > This is controlled by the <link>huge_pages</link> configuration parameter.
>
> I assume the point here is that we the term huge pages is what's used
> in PostgreSQL. For example, they are still configured using the
> huge_pages GUC.

That's exactly what my proposed wording says :-)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Postgres 11 release notes

From

Magnus Hagander

Date:

16 May 2018, 22:28:43

On Wed, May 16, 2018 at 6:24 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

On 2018-May-16, Magnus Hagander wrote:

> On Wed, May 16, 2018 at 4:08 PM, Alvaro Herrera <alvherre@2ndquadrant.com>
> wrote:

> > I'm not sure why we say "huge (large) pages". The natural term for
> > Windows is "large-pages",
> > https://msdn.microsoft.com/en-us/library/windows/desktop/
> > aa366720(v=vs.85).aspx
> > so I think we should use that terminology. Maybe something like
> >
> > Add support for <firstterm>large pages</firstterm> on Windows (Takayuki
> > Tsunakawa, Thomas Munro)
> > This is controlled by the <link>huge_pages</link> configuration parameter.
>
> I assume the point here is that we the term huge pages is what's used
> in PostgreSQL. For example, they are still configured using the
> huge_pages GUC.

That's exactly what my proposed wording says :-)

Uh. Nevermind. Nothing to see here. For some reason I only saw the first paragraph.

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: NaNs in numeric_power (was Re: Postgres 11 release notes)

From

David Rowley

Date:

17 May 2018, 03:49:18

On 16 May 2018 at 09:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> David Rowley <david.rowley@2ndquadrant.com> writes:
>> On 16 May 2018 at 02:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I'm not particularly fussed about getting credit for that.  However,
>>> looking again at how that patch series turned out --- ie, that
>>> we ensured POSIX behavior for NaNs only in HEAD --- I wonder
>>> whether we shouldn't do what was mentioned in the commit log for
>>> 6bdf1303, and teach numeric_pow() about these same special cases.
>>> It seems like it would be more consistent to change both functions
>>> for v11, rather than letting that other shoe drop in some future
>>> major release.
>
>> I'm inclined to agree. It's hard to imagine these two functions
>> behaving differently in regards to NaN input is useful to anyone.
>
> Here's a proposed patch for that.

Looks good to me.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: NaNs in numeric_power (was Re: Postgres 11 release notes)

From

Dean Rasheed

Date:

17 May 2018, 04:30:09

On 16 May 2018 at 14:44, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
>> In the case 1 ^ NaN = 1, what should the result scale be?
>
> The result is exact, so I don't see a reason to be worried about its
> scale.  Messing with the scale would also require expending even
> more code on what is, in the end, a corner case that no users have
> expressed concern about.
>

OK, fine. Not really worth worrying about.

Regards,
Dean

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

17 May 2018, 06:13:21

On Wed, May 16, 2018 at 01:22:45PM +0900, Michael Paquier wrote:
> On Mon, May 14, 2018 at 08:45:44PM -0400, Bruce Momjian wrote:
> > What TLS does is to mix the offered ciphers into the negotiation hash so
> > a man-in-the-middle can't pretend it doesn't support something.  Could
> > we do something like that here?
> 
> I have to admit that I don't quite follow here, the shape of channel
> binding data is decided by RFC 5929, so we need to stick with it.

OK, let me explain.  First, there are two man-in-the-middle types of
attacks.  The first one allows the two legitimate ends of the TLS
connection to negotiate the shared secret between themselves, but
injects or changes some of the packets before the shared secret is
agreed upon, perhaps to downgrade the strength of the protocol options. 
TLS prevents this type of man-in-the-middle attack by hashing the shared
secret with a hash of all of the TLS negotiation messages previously
sent.  Each side who knows the shared secret can verify that no messages
were changed;  see:

https://security.stackexchange.com/questions/115284/how-can-an-attacker-downgrade-modify-the-cipher-suites-when-they-are-maced-fre

The second type of man-in-the-middle attack is where the
man-in-the-middle is negotiating the shared secret separately with the
two end points.  There is no way to detect this without having some
shared secret between the two valid end points.  One shared secret is
the password hashed with the shared secret, which is what tls-unique
uses.  The second uses the server certificate hashed with the shared
secret.  This is now documented in Postgres:

   In <acronym>SCRAM</acronym> with <literal>tls-unique</literal>
   channel binding, the shared secret negotiated during the SSL session
   is mixed into the user-supplied password hash.  The shared secret
   is partly chosen by the server, but not directly transmitted, making
   it impossible for a fake server to create an SSL connection with the
   client that has the same shared secret it has with the real server.
   <acronym>SCRAM</acronym> with <literal>tls-server-end-point</literal>
   mixes a hash of the server's certificate into the user-supplied password
   hash.  While a fake server can retransmit the real server's certificate,
   it doesn't have access to the private key matching that certificate, and
   therefore cannot prove it is the owner, causing SSL connection failure.

TLS prevents a man-in-the-middle from injecting invalid packets. 
However, it does not prevent a man-in-the-middle attack with separeate
shared secret negotiation unless certificates are used.

The current problem under discussion is the same as that of the
man-in-the-middle packet change/injection attack in that the
man-in-the-middle can change the options reported as supported by the
Postgres server, before the password is sent.  The most efficient fix
for this would be for the all Postgres protocol messages up to the point
where the authentication was chosen to be hashed with the password and
sent to the server.  In that way, if a man-in-the-middle changed the
server-reported supported authentication, the server would identify this
and refuse the connection.

The problem is that current and past-version PG authentication methods
don't have any protocol downgrade protection, and it is hard to add it
unless you just remove support for old protocols.  I think the only
reasonable solution is to allow the client and/or server to force
channel binding.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

17 May 2018, 06:20:49

On Wed, May 16, 2018 at 08:59:23PM +0900, Michael Paquier wrote:
> On Wed, May 16, 2018 at 01:09:07PM +0300, Heikki Linnakangas wrote:
> > I have to agree with Bruce, that it's pretty useless to implement channel
> > binding, if there is no way to require it in libpq. IMHO that must be
> > fixed.
> 
> Wouldn't we want to also do something for the case where a client is
> willing to use SCRAM but that the server forces back MD5?  In which
> case, one possibility is a connection parameter like the following,
> named say authmethod:
> - An empty value is equivalent to the current behavior, and is the
> default.
> - 'scram' means that client is willing to use SCRAM, which would cause a
> failure if server attempts to enforce MD5.
> - 'scram-plus' means that client enforces SCRAM and channel binding.
> 
> Or we could just have a channel_binding_mode, which has a "require"
> value like sslmode, and "prefer" mode, which is the default and the
> current behavior...  Still what to do with MD5 requests in this case?

Just to reiterate, MD5 and SCRAM-less-binding is designed to avoid
packet _replay_.  It assumes no man-in-the-middle has adjusted what is
supported by the two endpoints.

SCRAM-with-binding is the first password method that attempts to avoid
man-in-the-middle attacks, and therefore is much less likely to be able
to trust what the endpoints supports.  I think it is really the
channel_binding_mode that we want to control at the client.  The lesser
modes are much more reasonable to use an automatic best-supported
negotiation, which is what we do now.

FYI, I think the server could also require channel binding for SCRAM. We
already have scram-sha-256 in pg_hba.conf, and I think
scram-sha-256-plus would be reasonable.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

17 May 2018, 06:24:43

On Wed, May 16, 2018 at 09:55:05AM +0530, Amit Kapila wrote:
> On Wed, May 16, 2018 at 12:17 AM, Bruce Momjian <bruce@momjian.us> wrote:
> > On Tue, May 15, 2018 at 08:45:07AM +0530, Amit Kapila wrote:
> >> No, it is not like that.  We divide the scan among workers and each
> >> worker should perform projection of the rows it scanned (after
> >> applying filter).  Now, if the expensive functions are part of target
> >> lists, then we can push the computation of expensive functions (as
> >> part of target list) in workers which will divide the work.
> >>
> >> >  Really?  Do
> >> > we run each column in its own worker or do we split the result set into
> >> > parts and run those in parallel?  How do we know, just the function call
> >> > costs?
> >> >
> >>
> >> The function's cost can be determined via pg_proc->procost.  For this
> >> particular case, you can refer the call graph -
> >> create_pathtarget->set_pathtarget_cost_width->cost_qual_eval_node->cost_qual_eval_walker->get_func_cost
> >>
> >> >  I can admit I never saw that coming.
> >> >
> >>
> >> I think the use case becomes interesting with parallel query because
> >> now you can divide such cost among workers.
> >>
> >> Feel free to ask more questions if above doesn't clarify the usage of
> >> these features.
> >
> > OK, I have added the following release note item for both of these:
> >
> > 2017-11-16 [e89a71fb4] Pass InitPlan values to workers via Gather (Merge).
> > 2018-03-29 [3f90ec859] Postpone generate_gather_paths for topmost scan/join rel
> > 2018-03-29 [11cf92f6e] Rewrite the code that applies scan/join targets to paths
> >
> >         Allow single-evaluation queries, e.g. <literal>FROM</literal>
> >         clause queries, and functions in the target list to be
> >         parallelized (Amit Kapila, Robert Haas)
> >
> 
> Sorry, but it is not clear to me what you intend to say by "e.g.
> <literal>FROM</literal> clause queries"?   What I could imagine is
> something like "e.g. queries in <literal>WHERE</literal> clause that
> return aggregate value"

Oh, sorry, changed to:

        Allow single-evaluation queries, e.g. <literal>WHERE</literal>
        clause aggregate queries, and functions in the target list to be
        parallelized (Amit Kapila, Robert Haas)

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

17 May 2018, 06:26:44

On Wed, May 16, 2018 at 01:50:04PM +0300, Heikki Linnakangas wrote:
> >* Allow bytes to be specified for server variable sizes (Beena Emerson)
> >
> >The specification is "B".
> 
> I had to dig the commit in the git history to figure out what this is. I'd
> suggest rewording this:
> 
> * Allow server options related to memory and file sizes, to be specified as
> number of bytes.
> 
> The new unit is "B". This is in addition to "kB", "MB", "GB" and "TB", which
> were accepted previously.

OK, good, done.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

17 May 2018, 06:29:18

On Wed, May 16, 2018 at 10:08:18AM -0400, Alvaro Herrera wrote:
> On 2018-May-15, Bruce Momjian wrote:
> 
> > On Wed, May 16, 2018 at 09:01:35AM +0900, Tatsuo Ishii wrote:
> > > There's a small typo.
> > > 
> > > > Add support for with huge(large) pages on Windows (Takayuki Tsunakawa, Thomas Munro) 
> > > 
> > > I think a space between "huge" and "(large)" is needed.
> > 
> > Done, URL updated.
> 
> I'm not sure why we say "huge (large) pages".  The natural term for
> Windows is "large-pages",
> https://msdn.microsoft.com/en-us/library/windows/desktop/aa366720(v=vs.85).aspx
> so I think we should use that terminology.  Maybe something like
> 
> Add support for <firstterm>large pages</firstterm> on Windows (Takayuki Tsunakawa, Thomas Munro)
> This is controlled by the <link>huge_pages</link> configuration parameter.

OK, done.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

17 May 2018, 07:09:22

On Thu, May 17, 2018 at 09:56:49AM +0900, Michael Paquier wrote:
> On Wed, May 16, 2018 at 08:20:49PM -0400, Bruce Momjian wrote:
> > SCRAM-with-binding is the first password method that attempts to avoid
> > man-in-the-middle attacks, and therefore is much less likely to be able
> > to trust what the endpoints supports.  I think it is really the
> > channel_binding_mode that we want to control at the client.  The lesser
> > modes are much more reasonable to use an automatic best-supported
> > negotiation, which is what we do now.
> 
> Noted.  Which means that the parameter is ignored when using a non-SSL
> connection, as well as when the server tries to enforce the use of
> anything else than SCRAM.

Uh, a man-in-the-middle could prevent SSL or ask for a different
password authentication method and then channel binding would not be
used.  I think when you say you want channel binding, you have to fail
if you don't get it.

> > FYI, I think the server could also require channel binding for SCRAM. We
> > already have scram-sha-256 in pg_hba.conf, and I think
> > scram-sha-256-plus would be reasonable.
> 
> Noted as well.  There is of course the question of v10 libpq versions
> which don't support channel binding, but if an admin is willing to set
> up scram-sha-256-plus in pg_hba.conf then he can request his users to
> update his drivers/libs as well.

Yes, I don't see a way around it.  Once you accept that someone in the
middle can change what you request undetected, then you can't do 
fallback.  Imagine a man-in-the-middle with TLS where the
man-in-the-middle allows the two end-points to negotiate the shared
secret, but the man-in-the-middle forces a weak cipher.  This is what is
happening with Postgres when the man-in-the-middle forces a weaker
authentication method.

-- 
  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 +

Re: Postgres 11 release notes

From

Amit Kapila

Date:

17 May 2018, 08:27:47

On Thu, May 17, 2018 at 5:54 AM, Bruce Momjian <bruce@momjian.us> wrote:
>
> Oh, sorry, changed to:
>
>         Allow single-evaluation queries, e.g. <literal>WHERE</literal>
>         clause aggregate queries, and functions in the target list to be
>         parallelized (Amit Kapila, Robert Haas)
>

LGTM.  Thanks.


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

RE: NaNs in numeric_power (was Re: Postgres 11 release notes)

From

Huong Dangminh

Date:

17 May 2018, 15:51:31

Hi, 

> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> David Rowley <david.rowley@2ndquadrant.com> writes:
> > On 16 May 2018 at 02:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I'm not particularly fussed about getting credit for that.  However,
> >> looking again at how that patch series turned out --- ie, that we
> >> ensured POSIX behavior for NaNs only in HEAD --- I wonder whether we
> >> shouldn't do what was mentioned in the commit log for 6bdf1303, and
> >> teach numeric_pow() about these same special cases.
> >> It seems like it would be more consistent to change both functions
> >> for v11, rather than letting that other shoe drop in some future
> >> major release.
> 
> > I'm inclined to agree. It's hard to imagine these two functions
> > behaving differently in regards to NaN input is useful to anyone.
> 

Thank you. The patch looks fine to me.
Also, I have done the "make check" in Windows and Linux environment with no problem.


Thanks and best regards,
---
Dang Minh Huong
NEC Solution Innovators, Ltd.
http://www.nec-solutioninnovators.co.jp/en/

Re: Postgres 11 release notes

From

Magnus Hagander

Date:

17 May 2018, 19:38:36

On Thu, May 17, 2018 at 2:56 AM, Michael Paquier <michael@paquier.xyz> wrote:

On Wed, May 16, 2018 at 08:20:49PM -0400, Bruce Momjian wrote:
> SCRAM-with-binding is the first password method that attempts to avoid
> man-in-the-middle attacks, and therefore is much less likely to be able
> to trust what the endpoints supports. I think it is really the
> channel_binding_mode that we want to control at the client. The lesser
> modes are much more reasonable to use an automatic best-supported
> negotiation, which is what we do now.

Noted. Which means that the parameter is ignored when using a non-SSL
connection, as well as when the server tries to enforce the use of
anything else than SCRAM.

(apologies if this was covered earlier, as I'm entering late into the discussion)

"ignored" in combination with a security parameter is generally a very very red flag.

If the client requests channel binding and ends up using a non encrypted connection, surely the correct thing to do is fail the connection, rather than downgrade the authentication?

We should really make sure we don't re-implement something as silly as our current "sslmode=prefer", because it makes no sense. From the client side perspective, there really only needs to be two choices -- "enforce channel binding at level <x>" or "meh, I don't care". In the "meh, I don't care" mode, go with whatever the server picks (through enforcement in pg_hba.conf for example).

> FYI, I think the server could also require channel binding for SCRAM. We
> already have scram-sha-256 in pg_hba.conf, and I think
> scram-sha-256-plus would be reasonable.

Noted as well. There is of course the question of v10 libpq versions
which don't support channel binding, but if an admin is willing to set
up scram-sha-256-plus in pg_hba.conf then he can request his users to
update his drivers/libs as well.

Yes. And they *should* fail if they don't upgrade. That's what requirement means... :)

What's the take of others? Magnus, Stephen or Heikki perhaps (you've
been the most involved with SCRAM early talks)?

Saw it by luck. It would probably be better if it wasn't hidden deep in a thread about release notes.

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

17 May 2018, 19:55:34

On Wed, May 16, 2018 at 09:09:22PM -0400, Bruce Momjian wrote:
> > > FYI, I think the server could also require channel binding for SCRAM. We
> > > already have scram-sha-256 in pg_hba.conf, and I think
> > > scram-sha-256-plus would be reasonable.
> > 
> > Noted as well.  There is of course the question of v10 libpq versions
> > which don't support channel binding, but if an admin is willing to set
> > up scram-sha-256-plus in pg_hba.conf then he can request his users to
> > update his drivers/libs as well.
> 
> Yes, I don't see a way around it.  Once you accept that someone in the
> middle can change what you request undetected, then you can't do 
> fallback.  Imagine a man-in-the-middle with TLS where the
> man-in-the-middle allows the two end-points to negotiate the shared
> secret, but the man-in-the-middle forces a weak cipher.  This is what is
> happening with Postgres when the man-in-the-middle forces a weaker
> authentication method.

Technically, you can do automatic fallback if the fallback has
man-in-the-middle and downgrade protection.  Technically, because TLS is
already active when we start authentication negotiation, we don't need
downgrade protection as long as we have man-in-the-middle protection.

Unfortunately, only SCRAM with channel binding and sslmode=verify-full
have man-in-the-middle protection.  Therefore, downgrading from SCRAM
with channel binding to SCRAM without channel binding, MD5, or
'password' is only safe if sslmode=verify-full is enabled.  Technically
MD5, or 'password' has weak or non-existent replay protection, but if we
are requiring TLS, then that doesn't really matter, assuming we have
man-in-the-middle protection.

I don't know if falling back from SCRAM with channel binding to a lesser
authentication methods only if sslmode=verify-full is enabled is really
helpful to anyone since it requires certificate installation.

TLS has similar downgrade issues:

    http://www.educatedguesswork.org/2012/07/problems_with_secure_upgrade_t.html

but many of its downgrade options have downgrade protection, and you
don't lose man-in-the-middle protection by downgrading. 
Man-in-the-middle protection via certificate checking happens
independent of the TLS version being used, which is not the case for
Postgres authentication downgrade options.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

17 May 2018, 19:56:21

On Thu, May 17, 2018 at 09:48:54PM +0900, Michael Paquier wrote:
> On Wed, May 16, 2018 at 09:09:22PM -0400, Bruce Momjian wrote:
> > On Thu, May 17, 2018 at 09:56:49AM +0900, Michael Paquier wrote:
> >> On Wed, May 16, 2018 at 08:20:49PM -0400, Bruce Momjian wrote:
> >>> SCRAM-with-binding is the first password method that attempts to avoid
> >>> man-in-the-middle attacks, and therefore is much less likely to be able
> >>> to trust what the endpoints supports.  I think it is really the
> >>> channel_binding_mode that we want to control at the client.  The lesser
> >>> modes are much more reasonable to use an automatic best-supported
> >>> negotiation, which is what we do now.
> >> 
> >> Noted.  Which means that the parameter is ignored when using a non-SSL
> >> connection, as well as when the server tries to enforce the use of
> >> anything else than SCRAM.
> > 
> > Uh, a man-in-the-middle could prevent SSL or ask for a different
> > password authentication method and then channel binding would not be
> > used.  I think when you say you want channel binding, you have to fail
> > if you don't get it.
> 
> I am not exactly sure what is the result we are looking for here, so I
> am adding for now an open item which refers to this part of the thread.
> Please note that I am fine to spend cycles if needed to address any
> issues and/or concerns.  Let's the discussion continue for now.

Agreed, and I just posted a more detailed email about when
authentication downgrades are possible.

-- 
  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 +

SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

17 May 2018, 20:05:25

On Thu, May 17, 2018 at 03:38:36PM +0200, Magnus Hagander wrote:
>     What's the take of others?  Magnus, Stephen or Heikki perhaps (you've
>     been the most involved with SCRAM early talks)?
> 
> Saw it by luck. It would probably be better if it wasn't hidden deep in a
> thread about release notes.

Agreed.  The problem was so glaring that I assumed I was not
understanding it.  I have modified this email subject so users will
hopefully read back in this thread to see the details.

-- 
  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 +

Re: NaNs in numeric_power (was Re: Postgres 11 release notes)

From

Tom Lane

Date:

17 May 2018, 21:22:45

Huong Dangminh <huo-dangminh@ys.jp.nec.com> writes:
> Thank you. The patch looks fine to me.
> Also, I have done the "make check" in Windows and Linux environment with no problem.

Pushed, thanks for reviewing/testing.

            regards, tom lane

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

18 May 2018, 00:23:00

On Thu, May 17, 2018 at 10:35:53PM +0900, Michael Paquier wrote:
> Hi Bruce,
> 
> Here is some bonus feedback.
> 
> On Fri, May 11, 2018 at 11:08:52AM -0400, Bruce Momjian wrote:
> > I expect a torrent of feedback.  ;-)
> 
> I have just noticed that this entry does not have the correct author
> (guess who?):

Fixed.  (I think I guessed right.)

>    <para>
>     Add libpq option to support channel binding when using <link
>     linkend="auth-password"><acronym>SCRAM</acronym></link>
>     authentication (Peter Eisentraut)
>    </para>
> 
> I think that there should be two different entries in the release notes
> for channel binding:
> 1) The new connection parameter in libpq which allows to control the
> channel binding name, as well as to decide if it should be disabled.
> I would think that this is better placed within the section for client
> interface changes.

Well, I tend to put items in the first section that applies, and in this
case, you are right that the API is libpq but the feature is
authentication.  We know we are going to need to adjust this feature,
so let's see where it ends up and let's revisit it.

> 2) Channel binding itself, which should be part of the authentication
> section.
> 
>    <para>
>     Have libpq's <link
>     linkend="libpq-pqhost"><function>PQhost()</function></link>
>     always return the actual connected host (Hari Babu)
>    </para>
> Should this be added as well in the section "Client interfaces"?

Well, again, using the rules above, the PQhost item goes into the first
section where it fits, and incompatibility is the first such section. 
There are other items in incompatibility that could be moved to lower,
but again, we want the incompatibilities to all be in the same place.

-- 
  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 +

Re: Postgres 11 release notes

From

Michael Paquier

Date:

18 May 2018, 07:19:37

On Thu, May 17, 2018 at 02:23:00PM -0400, Bruce Momjian wrote:
> On Thu, May 17, 2018 at 10:35:53PM +0900, Michael Paquier wrote:
> > Hi Bruce,
> >
> > Here is some bonus feedback.
> >
> > On Fri, May 11, 2018 at 11:08:52AM -0400, Bruce Momjian wrote:
> > > I expect a torrent of feedback.  ;-)
> >
> > I have just noticed that this entry does not have the correct author
> > (guess who?):
>
> Fixed.  (I think I guessed right.)

Thanks, Bruce.  It looks that your guess is right.
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

18 May 2018, 07:46:46

On Thu, May 17, 2018 at 10:05:25AM -0400, Bruce Momjian wrote:
> Agreed. The problem was so glaring that I assumed I was not
> understanding it. I have modified this email subject so users will
> hopefully read back in this thread to see the details.

Okay, let's use this new thread then for the follow-up discussion. For
people wondering, this basically refers to the following message from
Bruce and Heikki:
https://www.postgresql.org/message-id/d01b31f5-0b3e-b69a-1504-a79649d81f46@iki.fi

There are actually two problems which have been touched in this
discussion:
1) A client using libpq may be forced by a rogue server to not use
channel binding even if it is willing to do so. For example, a v11
libpq with a v10 server enters in this category. An idea here would be
to provide an extra connection parameter which allows the client to
reject an access to a server if channel binding is not supported. One
idea is something I mentioned here:
https://www.postgresql.org/message-id/20180516115923.GB14835%40paquier.xyz
So we could have channel_binding_mode, which has a 'prefer' mode, which
is the actual default we have in the tree, as well as a 'require' mode,
which allows the client to fail connection if a server does not publish
the SCRAM-PLUS mechanism after the initial authentication message
exchange.
2) Allow clients to connect to servers only if they use channel
binding. This goes with a server-side hba configuration, like
scram-sha-256-plus to not allow access to clients in a SCRAM
authentication if they don't have channel binding support.

From a security point of view, 1) is important for libpq, but I am not
much enthusiast about 2) as a whole. The backend has proper support for
channel binding, hence other drivers speaking the protocol could have
their own restriction mechanisms.

I actually touched a bit what's being discussed here as part of the
channel binding thread:
https://www.postgresql.org/message-id/CAB7nPqTRW9qbPSYpWtKJD9A%2BQBd6dS0ePkgrrjbkkG0C10zsBA%40mail.gmail.com
However per what I read this does not cover the need to allow the client
to allow that channel binding *has* to be used depending on its
requirements.

I'd like to mention as well that I touched the topic during the
unconference of PGConf Asia last December:
https://wiki.postgresql.org/wiki/PGConf.ASIA2017_Developer_Unconference#SCRAM_improvements

Any ideas raised there did not get much enthusiam from the
participants. Magnus and Bruce have participated in the conference,
perhaps you were not at my unconf session..
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

18 May 2018, 08:48:48

On Fri, May 18, 2018 at 10:46:46AM +0900, Michael Paquier wrote:
> On Thu, May 17, 2018 at 10:05:25AM -0400, Bruce Momjian wrote:
> > Agreed.  The problem was so glaring that I assumed I was not
> > understanding it.  I have modified this email subject so users will
> > hopefully read back in this thread to see the details.
> 
> Okay, let's use this new thread then for the follow-up discussion.  For
> people wondering, this basically refers to the following message from
> Bruce and Heikki:
> https://www.postgresql.org/message-id/d01b31f5-0b3e-b69a-1504-a79649d81f46@iki.fi

FYI, it was only glaring to me because I have spent so many months
recently studying security.

-- 
  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 +

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

18 May 2018, 09:03:49

On Fri, May 18, 2018 at 10:46:46AM +0900, Michael Paquier wrote:
> From a security point of view, 1) is important for libpq, but I am not
> much enthusiast about 2) as a whole.  The backend has proper support for
> channel binding, hence other drivers speaking the protocol could have
> their own restriction mechanisms.

So, I have been playing with libpq to address point 1), and added a new
connection parameter called channel_binding_mode which can be set to
'prefer', which is what libpq uses now, and 'require'.  The patch has
two important parts:
1) If a server does not support channel binding, still it is sending
back a SCRAM authentication, but the client still wants to enforce the
use of channel binding, then libpq reacts as follows:
$ psql -d "dbname=foo channel_binding_mode=require"
psql: channel binding required for SASL authentication but no valid
mechanism could be selected
This requires pg_SASL_init() to be patched after the SASL mechanism has
been selected.  That error can be triggered with a v10 server with whom
a SCRAM authentication is done, as well as with a v11 server where SSL
is not used.  Some people may use sslmode=prefer in combination to
channel_binding_mode=require, in which case an error should be raised if
the SSL connection cannot be achieved first.  That addresses a bit of
the craziness of sslmode=prefer...
2) If client wants to use channel binding, but the server is trying to
enforce another protocol than SCRAM, like MD5, trust, gssapi or such,
then the following error happens:
$ psql -d "dbname=foo channel_binding_mode=require"
psql: channel binding required for authentication but no valid protocol are used
In this case, it seems to me that the best bet is to patch
pg_fe_sendauth() and filter by message types.

In the attached, I have added the parameter and some documentation.  I
have not added tests, but some things could be tested in the SSL suite:
- Check for incorrect values in the parameter.
- Test connection without SCRAM with "require"
- Test connection without SSL but SCRAM with "require"
I have not put much thoughts into the documentation, but the patch is
rather simple so hopefully that helps in making progress in the
discussion.
--
Michael

Attachment

Re: Postgres 11 release notes

From

Andrey Borodin

Date:

18 May 2018, 11:56:25

H, Bruce!

11 мая 2018 г., в 20:08, Bruce Momjian <bruce@momjian.us> написал(а):

I expect a torrent of feedback. ;-)

I'm not sure it is usefull in release notes since it is more about API, and not user-facing change. Just in case.

GiST opclasses now can omit compress and decompress functions. If compress function is omited, IndexOnlyScan is enabled for opclass without any extra change.

https://github.com/postgres/postgres/commit/d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128

Best regards, Andrey Borodin.

Re: Postgres 11 release notes

From

Amit Kapila

Date:

18 May 2018, 16:28:16

On Tue, May 15, 2018 at 2:11 AM, Bruce Momjian <bruce@momjian.us> wrote:
> On Mon, May 14, 2018 at 05:34:54PM +0530, Dilip Kumar wrote:
>>
>> On Fri, May 11, 2018 at 8:38 PM, Bruce Momjian <bruce@momjian.us> wrote:
>>
>>     I have committed the first draft of the Postgres 11 release notes.  I
>>     will add more markup soon.  You can view the most current version here:
>>
>>             http://momjian.us/pgsql_docs/release-11.html
>>
>>     I expect a torrent of feedback.  ;-)
>>
>>     On a personal note, I want to apologize for not adequately championing
>>     two features that I think have strategic significance but didn't make it
>>     into Postgres 11:  parallel FDW access and improved multi-variate
>>     statistics.  I hope to do better job during Postgres 12.
>>
>>     --
>>       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 +
>>
>>
>>
>> I think the below commit is missed in the release notes?
>>
>> 5edc63bda68a77c4d38f0cbeae1c4271f9ef4100
>>
>> Committer: Robert Haas <rhaas@postgresql.org>  2017-11-10 13:50:50
>>
>>     Account for the effect of lossy pages when costing bitmap scans.
>>
>>     Dilip Kumar, reviewed by Alexander Kumenkov, Amul Sul, and me.
>>     Some final adjustments by me.
>>
>> As part of this commit, we are also accounting for the lossy pages during
>> bitmap costing.  This will consider
>> the effect of work_mem while selecting the bitmap heap scan path.
>
> Uh, that seems very exotic to mention, sorry.  Others have opinions?
>

This patch improves the plan selection in many cases, see the
discussion of this patch[1][2].  The change in plan leads to
significant performance improvements in a number of TPC-H queries at
various settings.  I think it is worth considering to add this in the
release notes.

[1] - https://www.postgresql.org/message-id/CAFiTN-uL%3DrQtvt9zFnLV9khXODhEyJTvC4TB135HSK1%3DYdFAxQ%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/9daaf288-e67f-f349-965f-3a8c6e0819ae%40postgrespro.ru


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

18 May 2018, 19:30:22

On Fri, May 18, 2018 at 12:03:49PM +0900, Michael Paquier wrote:
> On Fri, May 18, 2018 at 10:46:46AM +0900, Michael Paquier wrote:
> > From a security point of view, 1) is important for libpq, but I am not
> > much enthusiast about 2) as a whole.  The backend has proper support for
> > channel binding, hence other drivers speaking the protocol could have
> > their own restriction mechanisms.
> 
> So, I have been playing with libpq to address point 1), and added a new
> connection parameter called channel_binding_mode which can be set to
> 'prefer', which is what libpq uses now, and 'require'.  The patch has
> two important parts:

Good work, but I think the existance of both scram_channel_binding and
channel_binding_mode in libpq is confusing.  I am thinking we should
have one channel binding parameter that can take values "prefer",
"tls-unique", "tls-server-end-point", and "require".  I don't see the
point to having "none" and "allow" that sslmode supports.   "tls-unique"
and "tls-server-end-point" would _require_ those channel binding modes; 
the setting would never be ignored, e.g. if no SSL.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

18 May 2018, 19:53:02

On Fri, May 18, 2018 at 10:56:25AM +0500, Andrey Borodin wrote:
> H, Bruce!
> 
> 
>     11 мая 2018 г., в 20:08, Bruce Momjian <bruce@momjian.us> написал(а):
> 
>     I expect a torrent of feedback.  ;-)
> 
> 
> I'm not sure it is usefull in release notes since it is more about API, and not
> user-facing change. Just in case.
> GiST opclasses now can omit compress and decompress functions. If compress
> function is omited, IndexOnlyScan is enabled for opclass without any extra
> change.
> https://github.com/postgres/postgres/commit/
> d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128

Uh, we do have this for SP-GiST:

        Allow SP-GiST indexes to optionally use compression (Teodor Sigaev,
        Heikki Linnakangas, Alexander Korotkov, Nikita Glukhov)

I am unclear how far downt the API stack I should go in documenting
changes like this.

-- 
  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 +

Re: Postgres 11 release notes

From

Tom Lane

Date:

18 May 2018, 20:28:12

While we're all griping about omissions from the release notes ...
I think you should have documented that we fixed plpgsql to cope
(or cope better, at least) with intrasession changes in the rowtypes
of composite-type variables.  See bug #15203 for the latest in a long
line of user complaints about that --- so it's not like this is
something users don't care about.

            regards, tom lane

Re: Postgres 11 release notes

From

"Jonathan S. Katz"

Date:

18 May 2018, 21:56:22

> On May 11, 2018, at 11:08 AM, Bruce Momjian <bruce@momjian.us> wrote:
> 
> I have committed the first draft of the Postgres 11 release notes.  I
> will add more markup soon.  You can view the most current version here:
> 
>     http://momjian.us/pgsql_docs/release-11.html
> 
> I expect a torrent of feedback.  ;-)

Per feedback from many asynchronous threads, attached is the proposed
patch for the list of major features. I also expect a torrent of feedback. I
will have a corresponding press release for Beta 1 in the very near future.

This being my first doc patch proposal, I tried to follow the formatting I
saw in the existing SGML. Please let me know if I can make improvements.

Thanks,

Jonathan

Attachment

pg11features.diff

Re: Postgres 11 release notes

From

Amit Langote

Date:

18 May 2018, 21:58:04

Hi Bruce.

On Tue, May 15, 2018 at 12:46 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2018/05/15 5:30, Bruce Momjian wrote:
>> I like it, done.
>
> Thank you.

I wonder what you think about including this little performance item:

https://www.postgresql.org/message-id/E1eotSQ-0005V0-LV@gemulon.postgresql.org

especially considering the part of the commit message which states

...Still, testing shows
that this makes single-row inserts significantly faster on a table
with many partitions without harming the bulk-insert case.

I recall seeing those inserts being as much as 2x faster as partition
count grows beyond hundreds.  One might argue that we should think
about publicizing this only after we've dealt with the
lock-all-partitions issue that's also mentioned in the commit message
which is still a significant portion of the time spent and I'm totally
fine with that.

Thanks,
Amit

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

19 May 2018, 18:35:57

On Fri, May 18, 2018 at 09:30:22AM -0400, Bruce Momjian wrote:
> Good work, but I think the existance of both scram_channel_binding and
> channel_binding_mode in libpq is confusing.  I am thinking we should
> have one channel binding parameter that can take values "prefer",
> "tls-unique", "tls-server-end-point", and "require".  I don't see the
> point to having "none" and "allow" that sslmode supports.   "tls-unique"
> and "tls-server-end-point" would _require_ those channel binding modes;
> the setting would never be ignored, e.g. if no SSL.

I can see the point you are coming at.  Do you think that we should
worry about users willing to use specifically tls-server-end-point
(which is something actually in the backend protocol only for JDBC) and
manage a cluster of both v10 and v11 servers?  In this case we assume
that "prefer" means always using tls-unique as channel binding, but
there is no way to say "I would prefer channel binding if available,
only with end-point".  It would not be that hard to let the application
layer on top of libpq handle that complexity with channel binding
handling, though it makes the life of libpq consumers a bit harder.

Hence, I would actually eliminate "require", as that would be actually
the same as "tls-unique", and the possibility to use an empty value from
the list of options available but add "none" as that's actually the same
as the current empty value.  This gives as list:
- tls-unique
- tls-server-end-point
- none
- prefer, which has the meaning of preferring tls-unique
- And prefer-end-point?  But thinking why end-point has been added it
would not be worth it, and for tests only the option requiring end-point
is enough.

The previous patch has actually problems with servers using "trust",
"password" and any protocol which can send directly AUTH_REQ_OK as those
could still enforce a downgrade to not use channel binding, so we
actually need to make sure that AUTH_REQ_SASL_FIN has been received when
channel binding is required when looking at a AUTH_REQ_OK message.
--
Michael

Attachment

signature.asc

Re: Postgres 11 release notes

From

David Rowley

Date:

21 May 2018, 13:34:18

On 19 May 2018 at 03:58, Amit Langote <amitlangote09@gmail.com> wrote:
> I wonder what you think about including this little performance item:
>
> https://www.postgresql.org/message-id/E1eotSQ-0005V0-LV@gemulon.postgresql.org
>
> especially considering the part of the commit message which states
>
> ...Still, testing shows
> that this makes single-row inserts significantly faster on a table
> with many partitions without harming the bulk-insert case.
>
> I recall seeing those inserts being as much as 2x faster as partition
> count grows beyond hundreds.  One might argue that we should think
> about publicizing this only after we've dealt with the
> lock-all-partitions issue that's also mentioned in the commit message
> which is still a significant portion of the time spent and I'm totally
> fine with that.

While I do think that was a good change, I do think there's much still
left to do to speed up usage of partitioned tables with many
partitions.

I've been working a bit in this area over the past few weeks and with
PG11 I measured a single INSERT into a 10k RANGE partitioned table at
just 84 tps (!), while inserting the same row into a non-partitioned
table was about 11.1k tps. I have patches locally that take this up to
~9.8k tps, which I'll submit for PG12. I'm unsure if we should be
shouting anything from the rooftops about the work done in this area
for PG11, since it's still got a long way to go still before the
feature is usable with higher numbers of partitions. I do think your
change was a good one to make, but I just don't want users to think
that we're done here when we all know that much work remains.

If we're going to add an item in the release notes about this then I
wouldn't object, providing it could be done in a way that indicates
we've not finished here yet, but if that's the case then maybe it's
better to say nothing at all.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: Postgres 11 release notes

From

Haribabu Kommi

Date:

21 May 2018, 14:28:36

On Sat, May 12, 2018 at 1:08 AM, Bruce Momjian <bruce@momjian.us> wrote:

I have committed the first draft of the Postgres 11 release notes. I
will add more markup soon. You can view the most current version here:

http://momjian.us/pgsql_docs/release-11.html

Thanks for preparing the release notes.

>Have pg_dump dump all aspects of a database (Haribabu Kommi)

>pg_dump and pg_restore, without --clean, no longer dump/restore database

> comments and security labels.

There is small change in option name, the option to print database comments is --create not --clean.

>Have libpq's PQhost() always return the actual connected host (Hari Babu)

>Previously PQhost() often returned the supplied host parameters, which could contain

>several hosts. The same is true of PQport(), which now returns the actual port number,

>not the multiple supplied port numbers. ACCURATE?

Now PQhost() can return hostaddr also if host is not available. How about changing as below?

Have libpq's PQhost() always return the actual connected host/hostaddr (Haribabu Kommi)

hostaddr is returned, when there is no host available with the connected host.

Regards,

Haribabu Kommi

Fujitsu Australia

Re: Postgres 11 release notes

From

Amit Langote

Date:

21 May 2018, 20:58:50

On Mon, May 21, 2018 at 4:34 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
> On 19 May 2018 at 03:58, Amit Langote <amitlangote09@gmail.com> wrote:
>> I wonder what you think about including this little performance item:
>>
>> https://www.postgresql.org/message-id/E1eotSQ-0005V0-LV@gemulon.postgresql.org
>>
>> especially considering the part of the commit message which states
>>
>> ...Still, testing shows
>> that this makes single-row inserts significantly faster on a table
>> with many partitions without harming the bulk-insert case.
>>
>> I recall seeing those inserts being as much as 2x faster as partition
>> count grows beyond hundreds.  One might argue that we should think
>> about publicizing this only after we've dealt with the
>> lock-all-partitions issue that's also mentioned in the commit message
>> which is still a significant portion of the time spent and I'm totally
>> fine with that.
>
> While I do think that was a good change, I do think there's much still
> left to do to speed up usage of partitioned tables with many
> partitions.
>
> I've been working a bit in this area over the past few weeks and with
> PG11 I measured a single INSERT into a 10k RANGE partitioned table at
> just 84 tps (!), while inserting the same row into a non-partitioned
> table was about 11.1k tps. I have patches locally that take this up to
> ~9.8k tps, which I'll submit for PG12. I'm unsure if we should be
> shouting anything from the rooftops about the work done in this area
> for PG11, since it's still got a long way to go still before the
> feature is usable with higher numbers of partitions. I do think your
> change was a good one to make, but I just don't want users to think
> that we're done here when we all know that much work remains.
>
> If we're going to add an item in the release notes about this then I
> wouldn't object, providing it could be done in a way that indicates
> we've not finished here yet, but if that's the case then maybe it's
> better to say nothing at all.

You're right, it surely isn't time yet to make fanfare about this.  I
will look forward to being able to review your patches. :-)

Thanks,
Amit

Re: Postgres 11 release notes

From

Tom Lane

Date:

21 May 2018, 22:38:34

"Jonathan S. Katz" <jkatz@postgresql.org> writes:
> Per feedback from many asynchronous threads, attached is the proposed
> patch for the list of major features. I also expect a torrent of feedback. I
> will have a corresponding press release for Beta 1 in the very near future.
> This being my first doc patch proposal, I tried to follow the formatting I
> saw in the existing SGML. Please let me know if I can make improvements.

Pushed with some very minor editorialization.

            regards, tom lane

Re: Postgres 11 release notes

From

"Jonathan S. Katz"

Date:

21 May 2018, 22:52:31

> On May 21, 2018, at 12:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> "Jonathan S. Katz" <jkatz@postgresql.org> writes:
>> Per feedback from many asynchronous threads, attached is the proposed
>> patch for the list of major features. I also expect a torrent of feedback. I
>> will have a corresponding press release for Beta 1 in the very near future.
>> This being my first doc patch proposal, I tried to follow the formatting I
>> saw in the existing SGML. Please let me know if I can make improvements.
>
> Pushed with some very minor editorialization.

Thanks! I do agree with your editorial, the main goal now is to give exposure to features for testing.

Jonathan

RE: Postgres 11 release notes

From

"Ideriha, Takeshi"

Date:

22 May 2018, 08:36:00

>-----Original Message-----
>From: Bruce Momjian [mailto:bruce@momjian.us]
>I have committed the first draft of the Postgres 11 release notes.  I will add more
>markup soon.  You can view the most current version here:

Hi, there is a small typo.

I think "These function" should be "These functions".
(At the next sentece "these functions" is used.) 

Regards,
Ideriha, Takeshi

Attachment

release_note_typo.patch

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

22 May 2018, 14:22:19

On Sat, May 19, 2018 at 09:35:57PM +0900, Michael Paquier wrote:
> The previous patch has actually problems with servers using "trust",
> "password" and any protocol which can send directly AUTH_REQ_OK as those
> could still enforce a downgrade to not use channel binding, so we
> actually need to make sure that AUTH_REQ_SASL_FIN has been received when
> channel binding is required when looking at a AUTH_REQ_OK message.

Okay, so I have digested the previous comments with the attached.
scram_channel_binding is modified as follows (from commit message):
- "prefer", is the default and behaves so as channel binding is used if
available.  If the cluster does not support it then it is not used. This
does not protect from downgrade attacks.
- "disable", which is the equivalent of the empty value previously,
disables channel binding.
- "tls-unique" and "tls-server-end-point" make channel binding a
requirement and use the channel binding of the same name for
connection.  Note that in this case, if the connection is attempted
without SSL or if server does not support channel binding then libpq
refuses the connection as well.

In order to make sure that a client is not tricked by a "trust"
connection downgrade which sends back directly AUTH_REQ_OK, one way I
have thought about is to check if the client has achieved with a server
a full SASL exchange when receiving this message type, which is
something that we know about as the exchange state is saved in
PGconn->sasl_state.

The patch includes documentation and tests, which check that connections
are refused without SSL and or if the server downgrades authentication
requests.

Thanks,
--
Michael

On Wed, May 23, 2018 at 11:34 AM, Bruce Momjian <bruce@momjian.us> wrote:

On Mon, May 21, 2018 at 06:28:36PM +1000, Haribabu Kommi wrote:
> On Sat, May 12, 2018 at 1:08 AM, Bruce Momjian <bruce@momjian.us> wrote:
>
> I have committed the first draft of the Postgres 11 release notes. I
> will add more markup soon. You can view the most current version here:
>
> http://momjian.us/pgsql_docs/release-11.html
>
>
> Thanks for preparing the release notes.
>
> >Have pg_dump dump all aspects of a database (Haribabu Kommi)
> >
> >pg_dump and pg_restore, without --clean, no longer dump/restore database
> > comments and security labels.
>
> There is small change in option name, the option to print database comments is
> --create not --clean.

Uh, what about security labels?

Yes, security labels also gets dumped only with --create option.

> >Have libpq's PQhost() always return the actual connected host (Hari Babu)
> >
> >Previously PQhost() often returned the supplied host parameters, which could
> contain
> >several hosts. The same is true of PQport(), which now returns the actual port
> number,
> >not the multiple supplied port numbers. ACCURATE?
>
> Now PQhost() can return hostaddr also if host is not available. How about
> changing as below?
>
> Have libpq's PQhost() always return the actual connected host/hostaddr
> (Haribabu Kommi)
>
> hostaddr is returned, when there is no host available with the connected host.

OK, here is the new text:

Have libpq's <link
linkend="libpq-pqhost"><function>PQhost()</function></link>
always return the actual connected host (Hari Babu)

Can you update the author name as (Haribabu Kommi) to make it consistent

with other entries.

Previously <function>PQhost()</function> often returned the
supplied host parameters, which could contain several hosts.
-> It will now also return the host's IP address if the host name was
-> not supplied. The same is true of <function>PQport()</function>,
which now returns the actual port number, not the multiple supplied
port numbers.

That looks good to me.

Regards,

Haribabu Kommi

Fujitsu Australia

Re: Postgres 11 release notes

From

David Rowley

Date:

23 May 2018, 08:06:15

On 23 May 2018 at 13:18, Bruce Momjian <bruce@momjian.us> wrote:
> On Mon, May 21, 2018 at 07:34:18PM +1200, David Rowley wrote:
>> I've been working a bit in this area over the past few weeks and with
>> PG11 I measured a single INSERT into a 10k RANGE partitioned table at
>> just 84 tps (!), while inserting the same row into a non-partitioned
>> table was about 11.1k tps. I have patches locally that take this up to
>> ~9.8k tps, which I'll submit for PG12. I'm unsure if we should be
>
> Yikes!  I think the question is whether we need to _remove_ the item I
> just posted that is already in the release notes:
>
>         Allow faster partition elimination during query processing (Amit
>         Langote, David Rowley, Dilip Kumar)
>
>         This speeds access to partitioned tables with many partitions.

Well, partition elimination/pruning and tuple routing are not the same
thing. Pruning saves us scanning partitions that can't contain
matching tuples, whereas routing finds a home for a specific tuple.

Amit's work to improve partition elimination certainly is much faster
than constraint exclusion. It's not the last thing we'll ever do to
speed up the planning of queries for partitioned tables but it is a
very good start, and without it, run-time pruning would not be
possible.

I'd say the release notes in this regard don't claim anything that's
untrue. They look fine to me. Thanks for working on them!

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

23 May 2018, 08:40:27

On Wed, May 23, 2018 at 11:55:29AM +1000, Haribabu Kommi wrote:
> Yes, security labels also gets dumped only with --create option.

OK.

> Can you update the author name as (Haribabu Kommi) to make it consistent
> with other entries.

Done.  Applied patch attached.

-- 
  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 +

Attachment

diff

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

23 May 2018, 08:41:21

On Wed, May 23, 2018 at 02:06:15PM +1200, David Rowley wrote:
> On 23 May 2018 at 13:18, Bruce Momjian <bruce@momjian.us> wrote:
> > On Mon, May 21, 2018 at 07:34:18PM +1200, David Rowley wrote:
> >> I've been working a bit in this area over the past few weeks and with
> >> PG11 I measured a single INSERT into a 10k RANGE partitioned table at
> >> just 84 tps (!), while inserting the same row into a non-partitioned
> >> table was about 11.1k tps. I have patches locally that take this up to
> >> ~9.8k tps, which I'll submit for PG12. I'm unsure if we should be
> >
> > Yikes!  I think the question is whether we need to _remove_ the item I
> > just posted that is already in the release notes:
> >
> >         Allow faster partition elimination during query processing (Amit
> >         Langote, David Rowley, Dilip Kumar)
> >
> >         This speeds access to partitioned tables with many partitions.
> 
> Well, partition elimination/pruning and tuple routing are not the same
> thing. Pruning saves us scanning partitions that can't contain
> matching tuples, whereas routing finds a home for a specific tuple.

I just suspected they would use the same algorithm.

> Amit's work to improve partition elimination certainly is much faster
> than constraint exclusion. It's not the last thing we'll ever do to
> speed up the planning of queries for partitioned tables but it is a
> very good start, and without it, run-time pruning would not be
> possible.
> 
> I'd say the release notes in this regard don't claim anything that's
> untrue. They look fine to me. Thanks for working on them!

OK.

-- 
  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 +

Re: Postgres 11 release notes

From

Tom Lane

Date:

23 May 2018, 09:31:08

Bruce Momjian <bruce@momjian.us> writes:
> On Fri, May 18, 2018 at 10:28:12AM -0400, Tom Lane wrote:
>> While we're all griping about omissions from the release notes ...
>> I think you should have documented that we fixed plpgsql to cope
>> (or cope better, at least) with intrasession changes in the rowtypes
>> of composite-type variables.

> It was fixed in this commit, right?
>     commit 4b93f57999a2ca9b9c9e573ea32ab1aeaa8bf496

That was the main one, anyway.

            regards, tom lane

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

23 May 2018, 11:42:08

On Tue, May 22, 2018 at 08:19:34PM -0400, Bruce Momjian wrote:
> Since tls-unique and tls-server-end-point provide the same level of
> security, I don't see any value in allowing prefer-tls-server-end-point,
> as you stated above.

Thanks.  We are on the same line for this portion then.
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Heikki Linnakangas

Date:

23 May 2018, 12:46:14

On 22/05/18 11:22, Michael Paquier wrote:
> On Sat, May 19, 2018 at 09:35:57PM +0900, Michael Paquier wrote:
>> The previous patch has actually problems with servers using "trust",
>> "password" and any protocol which can send directly AUTH_REQ_OK as those
>> could still enforce a downgrade to not use channel binding, so we
>> actually need to make sure that AUTH_REQ_SASL_FIN has been received when
>> channel binding is required when looking at a AUTH_REQ_OK message.
> 
> Okay, so I have digested the previous comments with the attached.
> scram_channel_binding is modified as follows (from commit message):
> - "prefer", is the default and behaves so as channel binding is used if
> available.  If the cluster does not support it then it is not used. This
> does not protect from downgrade attacks.
> - "disable", which is the equivalent of the empty value previously,
> disables channel binding.
> - "tls-unique" and "tls-server-end-point" make channel binding a
> requirement and use the channel binding of the same name for
> connection.  Note that in this case, if the connection is attempted
> without SSL or if server does not support channel binding then libpq
> refuses the connection as well.

"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.

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

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 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.)

> In order to make sure that a client is not tricked by a "trust"
> connection downgrade which sends back directly AUTH_REQ_OK, one way I
> have thought about is to check if the client has achieved with a server
> a full SASL exchange when receiving this message type, which is
> something that we know about as the exchange state is saved in
> PGconn->sasl_state.

It'd be nice if the client could tell the server that it requires 
authentication, so that the server would go through the SCRAM 
authentication even with "trust". With channel binding, SCRAM not only 
authenticates the client to the server, but also the server to the 
client. But that's not urgent, I think we can live without it for v11.

- Heikki

Re: SCRAM with channel binding downgrade attack

From

Magnus Hagander

Date:

23 May 2018, 12:59:43

On Wed, May 23, 2018 at 8:46 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

On 22/05/18 11:22, Michael Paquier wrote:
On Sat, May 19, 2018 at 09:35:57PM +0900, Michael Paquier wrote:
The previous patch has actually problems with servers using "trust",
"password" and any protocol which can send directly AUTH_REQ_OK as those
could still enforce a downgrade to not use channel binding, so we
actually need to make sure that AUTH_REQ_SASL_FIN has been received when
channel binding is required when looking at a AUTH_REQ_OK message.

Okay, so I have digested the previous comments with the attached.
scram_channel_binding is modified as follows (from commit message):
- "prefer", is the default and behaves so as channel binding is used if
available. If the cluster does not support it then it is not used. This
does not protect from downgrade attacks.
- "disable", which is the equivalent of the empty value previously,
disables channel binding.
- "tls-unique" and "tls-server-end-point" make channel binding a
requirement and use the channel binding of the same name for
connection. Note that in this case, if the connection is attempted
without SSL or if server does not support channel binding then libpq
refuses the connection as well.

"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.

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

As a general point, I still don't like being symmetrical with sslmode=prefer, because that's a silly setting for sslmode :) It basically says "I don't care about the security guarantees, I just want the overhead please". Now, granted, the over head for SCRAM channel-binding is certainly a lot less than it is for TLS. But if we just want to go with symmetry, it would perhaps make more sense to implement the "allow" mode which makes more sense on the TLS side as well.

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 would be nice to have a libpq setting like "authenticate_server=require", which would mean "I want man-in-the-middle protection".

That seems like a bad name for such a thing. Shouldn't it be "authenticate_server=no_man_in_the_middle" (not those words but you get the idea). Specifically, protecting against man in the middle attack does not equal authenticating server -- you can still be connected to the wrong server just with no second attacker between you and the first attacker.

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.)

sslmode=verify-full is very different from SCRAM with channel binding, isn't it? As in, SCRAM with channel binding at no point proves which server you're talking to -- only that you are talking to the SSL endpoint? It could be a rogue SSL endpoint unless you do certificate validation.

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

23 May 2018, 14:56:19

On Wed, May 23, 2018 at 08:59:43AM +0200, Magnus Hagander wrote:
> On Wed, May 23, 2018 at 8: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.
>>
>> So I think the options should be "prefer", "disable", and "require".
>> That's also symmetrical with sslmode, which is nice.

OK, being able to introduce a new default if necessary is a good point.
Let's introduce a "require" mode then, which uses tls-unique
underground, while "tls-unique" and "tls-server-end-point" are
documented as developer-oriented.

> As a general point, I still don't like being symmetrical with
> sslmode=prefer, because that's a silly setting for sslmode :) It basically
> says "I don't care about the security guarantees, I just want the overhead
> please". Now, granted, the over head for SCRAM channel-binding is certainly
> a lot less than it is for TLS. But if we just want to go with symmetry, it
> would perhaps make more sense to implement the "allow" mode which makes
> more sense on the TLS side as well.

Something that you may be forgetting here is that we still want to be
able to connect to a v10 backend with default options even with a
post-v11 libpq.  Or we will get complaints.

>> It would be nice to have a libpq setting like
>> "authenticate_server=require", which would mean "I want man-in-the-middle
>> protection".
>
> That seems like a bad name for such a thing. Shouldn't it be
> "authenticate_server=no_man_in_the_middle" (not those words but you get the
> idea). Specifically, protecting against man in the middle attack does not
> equal authenticating server -- you can still be connected to the wrong
> server just with no second attacker between you and the first
> attacker.

Still that's not something we want for v11, right?

>> 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.)
>
> sslmode=verify-full is very different from SCRAM with channel binding,
> isn't it? As in, SCRAM with channel binding at no point proves which server
> you're talking to -- only that you are talking to the SSL endpoint? It
> could be a rogue SSL endpoint unless you do certificate validation.

And the reverse is true as well, say the CA is compromised..
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Magnus Hagander

Date:

23 May 2018, 15:01:10

On Wed, May 23, 2018 at 10:56 AM, Michael Paquier <michael@paquier.xyz> wrote:

On Wed, May 23, 2018 at 08:59:43AM +0200, Magnus Hagander wrote:
> On Wed, May 23, 2018 at 8: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.
>>
>> So I think the options should be "prefer", "disable", and "require".
>> That's also symmetrical with sslmode, which is nice.

OK, being able to introduce a new default if necessary is a good point.
Let's introduce a "require" mode then, which uses tls-unique
underground, while "tls-unique" and "tls-server-end-point" are
documented as developer-oriented.

> As a general point, I still don't like being symmetrical with
> sslmode=prefer, because that's a silly setting for sslmode :) It basically
> says "I don't care about the security guarantees, I just want the overhead
> please". Now, granted, the over head for SCRAM channel-binding is certainly
> a lot less than it is for TLS. But if we just want to go with symmetry, it
> would perhaps make more sense to implement the "allow" mode which makes
> more sense on the TLS side as well.

Something that you may be forgetting here is that we still want to be
able to connect to a v10 backend with default options even with a
post-v11 libpq. Or we will get complaints.

Right. Having a mode called "allow" and having that be default would work fine in this case, wouldn't it?

(That is, my suggestion is to implement "allow", "disable" and "require", and to make "allow" the default)

>> It would be nice to have a libpq setting like
>> "authenticate_server=require", which would mean "I want man-in-the-middle
>> protection".
>
> That seems like a bad name for such a thing. Shouldn't it be
> "authenticate_server=no_man_in_the_middle" (not those words but you get the
> idea). Specifically, protecting against man in the middle attack does not
> equal authenticating server -- you can still be connected to the wrong
> server just with no second attacker between you and the first
> attacker.

Still that's not something we want for v11, right?

Agreed.

>> 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.)
>
> sslmode=verify-full is very different from SCRAM with channel binding,
> isn't it? As in, SCRAM with channel binding at no point proves which server
> you're talking to -- only that you are talking to the SSL endpoint? It
> could be a rogue SSL endpoint unless you do certificate validation.

And the reverse is true as well, say the CA is compromised..

Well, sure. But scram channel binding doesn't protect you there, so you're screwed either way if that happens.

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: SCRAM with channel binding downgrade attack

From

Heikki Linnakangas

Date:

23 May 2018, 15:08:35

On 23/05/18 09:59, Magnus Hagander wrote:
>> 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.)
> 
> sslmode=verify-full is very different from SCRAM with channel binding,
> isn't it? As in, SCRAM with channel binding at no point proves which server
> you're talking to -- only that you are talking to the SSL endpoint? It
> could be a rogue SSL endpoint unless you do certificate validation.

SCRAM, even without channel binding, does prove that you're talking to 
the correct server. Or to be precise, it proves to the client, that the 
server also knows the password, so assuming that you're using strong 
passwords and not sharing them across servers, you know that you're 
talking to the correct server.

Channel binding adds the guarantee that the SSL endpoint belongs to the 
same server you're authenticating with, i.e. there is no man in the middle.

- Heikki

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

23 May 2018, 15:10:36

On Wed, May 23, 2018 at 05:56:19PM +0900, Michael Paquier wrote:
> OK, being able to introduce a new default if necessary is a good point.
> Let's introduce a "require" mode then, which uses tls-unique
> underground, while "tls-unique" and "tls-server-end-point" are
> documented as developer-oriented.

By the way, if somebody could review the latest version of the patch
before I write a new version and agrees with the concept introduced
would be nice..  Adding one option is simple enough, making sure that we
agree that the patch is on good tracks is harder.
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Magnus Hagander

Date:

23 May 2018, 15:15:28

On Wed, May 23, 2018 at 11:08 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

On 23/05/18 09:59, Magnus Hagander wrote:
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.)

sslmode=verify-full is very different from SCRAM with channel binding,
isn't it? As in, SCRAM with channel binding at no point proves which server
you're talking to -- only that you are talking to the SSL endpoint? It
could be a rogue SSL endpoint unless you do certificate validation.

SCRAM, even without channel binding, does prove that you're talking to the correct server. Or to be precise, it proves to the client, that the server also knows the password, so assuming that you're using strong passwords and not sharing them across servers, you know that you're talking to the correct server.

Right. It provides a very different guarantee from what ssl certs provide. They are not replaceable, or mutually exclusive. Trying to force those into a single configuration parameter doesn't make a lot of sense IMO.

Channel binding adds the guarantee that the SSL endpoint belongs to the same server you're authenticating with, i.e. there is no man in the middle.

Yeah, it does protect you against things like pgbouncer (a real one or a rogue one- the rogue one being the mitm attacker). But again, only if you never share a password, which would be a nice world to live in :)

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

24 May 2018, 04:41:16

On Wed, May 23, 2018 at 11:15:28AM +0200, Magnus Hagander wrote:
> On Wed, May 23, 2018 at 11:08 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>     SCRAM, even without channel binding, does prove that you're talking to the
>     correct server. Or to be precise, it proves to the client, that the server
>     also knows the password, so assuming that you're using strong passwords and
>     not sharing them across servers, you know that you're talking to the
>     correct server.
> 
> Right. It provides a very different guarantee from what ssl certs provide. They
> are not replaceable, or mutually exclusive. Trying to force those into a single
> configuration parameter doesn't make a lot of sense IMO.

True.  sslmode is checking the the SSL endpoint with which you have a
shared secret has access to the private key of a server certificate that
is signed by a trusted CA, and perhaps the certificate's subject name
also matches the hostname.

With channel binding, you are proving that the SSL endpoint with which
you have a shared secret has access to the user password hash.

I can imagine someone wanting both checks so merging them into a single
options seems unwise, as Magnus mentioned.

-- 
  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 +

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

24 May 2018, 05:46:27

On Wed, May 23, 2018 at 06:41:16PM -0400, Bruce Momjian wrote:
> I can imagine someone wanting both checks so merging them into a single
> options seems unwise, as Magnus mentioned.

FWIW, definitely agreed.
--
Michael

Attachment

signature.asc

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

24 May 2018, 08:06:52

On Tue, May 22, 2018 at 11:31:08PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Fri, May 18, 2018 at 10:28:12AM -0400, Tom Lane wrote:
> >> While we're all griping about omissions from the release notes ...
> >> I think you should have documented that we fixed plpgsql to cope
> >> (or cope better, at least) with intrasession changes in the rowtypes
> >> of composite-type variables.
> 
> > It was fixed in this commit, right?
> >     commit 4b93f57999a2ca9b9c9e573ea32ab1aeaa8bf496
> 
> That was the main one, anyway.

OK, added.  Applied patch attached.

-- 
  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 +

Attachment

diff

Re: SCRAM with channel binding downgrade attack

From

Robert Haas

Date:

25 May 2018, 20:44:16

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

Re: SCRAM with channel binding downgrade attack

From

Heikki Linnakangas

Date:

25 May 2018, 21:24:07


On 25 May 2018 17:44:16 EEST, Robert Haas <robertmhaas@gmail.com> wrote:
>On Wed, May 23, 2018 at 2:46 AM, Heikki Linnakangas <hlinnaka@iki.fi>
>wrote:
>> 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.

Works for me.

- Heikki

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

26 May 2018, 05:32:20

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

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

26 May 2018, 19:08:50

On Sat, May 26, 2018 at 08:32:20AM +0900, Michael Paquier wrote:
> 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.

scram_channel_binding_method?

> 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?

Do we really know someone is going to want to actually specify the
channel binding type?  If it is only testing, maybe we don't need to
document this parameter.

-- 
  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 +

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

26 May 2018, 20:42:38

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.

> Do we really know someone is going to want to actually specify the
> channel binding type?  If it is only testing, maybe we don't need to
> document this parameter.

Keeping everything documented is useful as well for new developers, as
they need to guess less from the code.  So I would prefer seeing both
connection parameters documented, and mentioning directly in the docs if
a parameter is for developers or not.
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

28 May 2018, 07:23:28

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.

>> Do we really know someone is going to want to actually specify the
>> channel binding type?  If it is only testing, maybe we don't need to
>> document this parameter.
>
> Keeping everything documented is useful as well for new developers, as
> they need to guess less from the code.  So I would prefer seeing both
> connection parameters documented, and mentioning directly in the docs if
> a parameter is for developers or not.

So done this way.  Feel free to pick me up at PGcon this week if you
wish to discuss this issue.  Docs, tests and a commit message are
added.
--
Michael

Attachment

Re: SCRAM with channel binding downgrade attack

From

Heikki Linnakangas

Date:

28 May 2018, 15:00:33

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

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

28 May 2018, 15:20:02

On Mon, May 28, 2018 at 12:00:33PM +0300, Heikki Linnakangas wrote:
> 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.

Yeah, I know..  Do you think that it would be better to add an extra
switch/case at the beginning of pg_fe_sendauth which filters and checks
per message types then?
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Heikki Linnakangas

Date:

28 May 2018, 15:26:37

On 28/05/18 12:20, Michael Paquier wrote:
> On Mon, May 28, 2018 at 12:00:33PM +0300, Heikki Linnakangas wrote:
>> 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.
> 
> Yeah, I know..  Do you think that it would be better to add an extra
> switch/case at the beginning of pg_fe_sendauth which filters and checks
> per message types then?

Sounds good.

- Heikki

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

29 May 2018, 01:08:50

On Mon, May 28, 2018 at 12:26:37PM +0300, Heikki Linnakangas wrote:
> Sounds good.

Okay.  Done this way as attached.  If the backend forces anything else
than SCRAM then the client gets an immediate error if channel binding is
required, without waiting for the password prompt.
--
Michael

Attachment

Re: SCRAM with channel binding downgrade attack

From

Heikki Linnakangas

Date:

02 June 2018, 23:08:56

On 28/05/18 15:08, Michael Paquier wrote:
> On Mon, May 28, 2018 at 12:26:37PM +0300, Heikki Linnakangas wrote:
>> Sounds good.
> 
> Okay.  Done this way as attached.  If the backend forces anything else
> than SCRAM then the client gets an immediate error if channel binding is
> required, without waiting for the password prompt.

Thanks! The comments and error messages need some copy-editing:

>          /*
>           * Select the mechanism to use.  Pick SCRAM-SHA-256-PLUS over anything
>          * else if a channel binding mode is not 'disable'.  Pick SCRAM-SHA-256
>          * if nothing else has already been picked.  If we add more mechanisms, a
>           * more refined priority mechanism might become necessary.
>           */

"else if a channel binding mode is not 'disable'" sounds a bit awkward. 
A double negative. (Also, "a" -> "the")

> +    /*
> +     * If client is willing to enforce the use the channel binding but
> +     * it has not been previously selected, because it was either not
> +     * published by the server or could not be selected, then complain
> +     * back.  If SSL is not used for this connection, still complain
> +     * similarly, as the client may want channel binding but forgot
> +     * to set it up to do so which could be the case with sslmode=prefer
> +     * for example.  This protects from any kind of downgrade attacks
> +     * from rogue servers attempting to bypass channel binding.
> +     */

Instead of "is willing to enforce the use the channel binding", perhaps 
simply "requires channel binding".

> +  printfPQExpBuffer(&conn->errorMessage,
> +    libpq_gettext("channel binding required for SASL authentication but no valid mechanism could be selected\n"));

Channel binding is a property of SCRAM authentication specifically, it's 
not applicable to other SASL mechanisms. So I'd suggest something like:

"server does not support channel binding, and 
channel_binding_mode=require was used"

> +    /*
> +     * Complain if channel binding mechanism is chosen but no channel
> +     * binding type is defined.
> +     */
> +    if (strcmp(selected_mechanism, SCRAM_SHA_256_PLUS_NAME) == 0 &&
> +        conn->scram_channel_binding_type == NULL)
> +    {
> +        printfPQExpBuffer(&conn->errorMessage,
> +                          libpq_gettext("SASL authentication mechanism using channel binding supported but no
channelbinding type defined\n"));

> +        goto error;
> +    }

It's not immediately clear to me from the error message or the comment, 
when this error is thrown. Can this even happen?

> +    /*
> +     * Before processing any message, perform security-related sanity
> +     * checks.  If the client is willing to perform channel binding with
> +     * SCRAM authentication, only a subset of messages can be used so
> +     * as there is no transmission of any password data or such.
> +     */

I'd suggest something like:

"If SCRAM with channel binding is required, refuse all other 
authentication methods. Requiring channel binding implies that the 
client doesn't trust the server, until the SCRAM authentication is 
completed, so it's important that we don't divulge the plaintext 
password, or perform some weaker authentication, even if the server asks 
for it. In all other authentication methods, it's currently assumed that 
the client trusts the server, either implicitly or because the SSL 
certificate was already verified during the SSL handshake."

> +  printfPQExpBuffer(&conn->errorMessage,
> +    libpq_gettext("channel binding required for authentication but invalid protocol used\n"));

If I understand correctly, you get this error, e.g. if you have 
"password" or "sspi" in pg_hba.conf, and the client uses 
"channel_binding_mode=require". "Invalid protocol" doesn't sound right 
for that. Perhaps "channel binding required, but the server requested %s 
authentication". Is it possible to have an error hint here? Perhaps add 
"HINT: change the authentication method to scram-sha-256 in the server's 
pg_hba.conf file".

- Heikki

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

05 June 2018, 12:41:38

On Sat, Jun 02, 2018 at 01:08:56PM -0400, Heikki Linnakangas wrote:
> On 28/05/18 15:08, Michael Paquier wrote:
>> On Mon, May 28, 2018 at 12:26:37PM +0300, Heikki Linnakangas wrote:
>> > Sounds good.
>>
>> Okay.  Done this way as attached.  If the backend forces anything else
>> than SCRAM then the client gets an immediate error if channel binding is
>> required, without waiting for the password prompt.
>
> Thanks! The comments and error messages need some copy-editing:

Thanks Heikki for the input.  I have reworked all the points you raised
in the attached.

>>          /*
>>           * Select the mechanism to use.  Pick SCRAM-SHA-256-PLUS over anything
>>          * else if a channel binding mode is not 'disable'.  Pick SCRAM-SHA-256
>>          * if nothing else has already been picked.  If we add more mechanisms, a
>>           * more refined priority mechanism might become necessary.
>>           */
>
> "else if a channel binding mode is not 'disable'" sounds a bit awkward. A
> double negative. (Also, "a" -> "the")

Okay.  I have completely rewritten this block.  Hopefully that's clearer.

>> +    /*
>> +     * If client is willing to enforce the use the channel binding but
>> +     * it has not been previously selected, because it was either not
>> +     * published by the server or could not be selected, then complain
>> +     * back.  If SSL is not used for this connection, still complain
>> +     * similarly, as the client may want channel binding but forgot
>> +     * to set it up to do so which could be the case with sslmode=prefer
>> +     * for example.  This protects from any kind of downgrade attacks
>> +     * from rogue servers attempting to bypass channel binding.
>> +     */
>
> Instead of "is willing to enforce the use the channel binding", perhaps
> simply "requires channel binding".

Check.

>> +  printfPQExpBuffer(&conn->errorMessage,
>> +    libpq_gettext("channel binding required for SASL authentication but no valid mechanism could be selected\n"));
>
> Channel binding is a property of SCRAM authentication specifically, it's not
> applicable to other SASL mechanisms. So I'd suggest something like:
>
> "server does not support channel binding, and channel_binding_mode=require
> was used"

Changed as such, except that this is using scram_channel_binding_mode in
the error message.

>> +    /*
>> +     * Complain if channel binding mechanism is chosen but no channel
>> +     * binding type is defined.
>> +     */
>> +    if (strcmp(selected_mechanism, SCRAM_SHA_256_PLUS_NAME) == 0 &&
>> +        conn->scram_channel_binding_type == NULL)
>> +    {
>> +        printfPQExpBuffer(&conn->errorMessage,
>> +         libpq_gettext("SASL authentication mechanism using channel binding supported but no channel binding type
defined\n"));
>> +        goto error;
>> +    }
>
> It's not immediately clear to me from the error message or the comment, when
> this error is thrown. Can this even happen?

No, let's not make this happen then.  I have added NULL handling in
connectOptions2 related to the initialization of the options so both
scram_channel_binding_mode and scram_channel_binding_type will never be
NULL like sslmode.

>> +    /*
>> +     * Before processing any message, perform security-related sanity
>> +     * checks.  If the client is willing to perform channel binding with
>> +     * SCRAM authentication, only a subset of messages can be used so
>> +     * as there is no transmission of any password data or such.
>> +     */
>
> I'd suggest something like:
>
> "If SCRAM with channel binding is required, refuse all other authentication
> methods. Requiring channel binding implies that the client doesn't trust the
> server, until the SCRAM authentication is completed, so it's important that
> we don't divulge the plaintext password, or perform some weaker
> authentication, even if the server asks for it. In all other authentication
> methods, it's currently assumed that the client trusts the server, either
> implicitly or because the SSL certificate was already verified during the
> SSL handshake."

Check.  Thanks for the suggestion.

>> +  printfPQExpBuffer(&conn->errorMessage,
>> +    libpq_gettext("channel binding required for authentication but invalid protocol used\n"));
>
> If I understand correctly, you get this error, e.g. if you have "password"
> or "sspi" in pg_hba.conf, and the client uses
> "channel_binding_mode=require". "Invalid protocol" doesn't sound right for
> that. Perhaps "channel binding required, but the server requested %s
> authentication".

Right, even "md5" enters in this category if the user has a MD5
verifier, but not if it has a SCRAM verifier.  I have done that with
get_auth_request_str(), which maps authentication requests to the
probable hba entry.  It feels like cheating to map "trust" to
AUTH_REQ_OK as all methods use it as final message though, even if it is
not triggered by this patch.  So I have used no mapping name for it.

> Is it possible to have an error hint here? Perhaps add
> "HINT: change the authentication method to scram-sha-256 in the server's
> pg_hba.conf file".

Hm.  A HINT would require something similar to PQresultErrorField for
error hints, which is a fight I not much willing to do just for this
patch.  Spawning a new error message with a newline would also be
considered as a new error message.  So I discard this one.

I have also added a test with a "password" server which stresses this
code path actually, and I just indented the patch.  What do you think?
--
Michael

On Wed, Jun 06, 2018 at 11:53:06PM +0300, Heikki Linnakangas wrote:
> That would certainly be good. We've always had that problem, even with md5
> -> plaintext password downgrade, and it would be nice to fix it. It's quite
> late in the release cycle already, do you think we should address that now?
> I could go either way..

I would be inclined to treat that as new development as this is no new
problem.  Still that's linked with what is discussed on this thread with
scram_channel_bindin_mode.

> What should the option look like? Perhaps something like:
>
> allowed_authentication_methods=md5,SCRAM-SHA-256,SCRAM-SHA-256-PLUS

That's actually a discussion I had with somebody after my talk at
PGCon, and I suggested a comma-separate list of authorized protocols as
well, except that those could just map to the hba entries, and that each
entry could just be lower-case :)
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

07 June 2018, 10:47:21

On Wed, Jun 06, 2018 at 11:46:11PM +0300, Heikki Linnakangas wrote:
> Ok. Perhaps add a comment pointing out that as the code stands,
> get_auth_request_str() is never called with AUTH_REQ_OK. So that if someone
> starts calling it with that, maybe they'll know to revisit this.

That makes sense.  Done.

>> +         <varlistentry>
>> +          <term><literal>prefer</literal> (default)</term>
>> +          <listitem>
>> +           <para>
>> +            Use channel binding if available.  If the cluster does not
>> +            support it, then it is not used.  This is the default.
>> +           </para>
>> +          </listitem>
>> +         </varlistentry>
>
> I'd suggest s/cluster/server/. Here, and elsewhere in the docs changes.

Done.

> There are some funny behaviors with different combinations of
> "scram_channel_binding_mode=require" and different sslmode settings. For
> example, with sslmode=disable, the client will attempt to connect, but it's
> guaranteed to fail at authentication, because channel binding is only
> possible with SSL. Perhaps it should throw an error without even attempting
> to connect? Or at least, the "server does not support channel binding" is
> misleading, as it was the client that insisted on an impossible combination;
> the server might well support channel binding, if SSL was used.
>
> And with sslmode=allow, if the server allows a non-SSL connection, then the
> client won't use SSL, and will fail, as with sslmode=disable. But if the
> server requires SSL, then it will work. Perhaps sslmode=allow should be
> "promoted" to "sslmode=require", if scram_channel_binding_mode=require? Or
> don't let the user select a silly combination like that at all, and throw an
> error.

I was not really planning to make the code more complicated with
cross-option checks (that's complicated enough), but isn't what you are
looking for here to make sslmode=require a mandatory thing if
channel_binding_mode=require is set?  It is possible to add checks like
that in connectOptions2 for example after setting all the values.
Promoting automatically sslmode="allow" to "require" if
scram_channel_binding_mode=require feels like a trap for users as well.

> If scram_channel_binding_mode=require, but the server doesn't support SSL at
> all, you get:
>
> psql: server does not support channel binding, and
> scram_channel_binding_mode=require was used
>
> Perhaps it would be more clear to say "server does not support SSL" or
> something like that. I could imagine an admin spending a long time looking
> for an "enable channel binding" option in server settings, not realizing
> that it's actually "ssl=off" that's the problem.

It really depends on the connection context as a v10 server allows SSL
without channel binding, so just saying that server does not support SSL
is not correct either.  It seems to me that if you want an operator to
take correct actions then we want to see if the backend connected to is
a v10 server or something newer, or actually uses SSL in the connection
context.  If that's a v10 or older, then libpq should still complain
about channel binding not supported.  If it is a v11 or newer, then
libpq should complain about SSL not being supported.  By adding the
cross-option check in connectOptions2, the error message about SSL not
being supported is moot if requiring sslmode=require for
channel_binding_mode=require.  I have added a more complicated logic in
pg_SASL_init() for that (grep for "server does not support channel
binding") as it looks safer to me to have that for future sanity checks,
so please let me know your thoughts.  And it could be perfectly possible
that an attacking server tries to make the client think that it is a v11
server but tries to downgrade to SCRAM without channel binding.

Still, the tests reach their limit here, as you would normally bump into
HBA entries which don't match normally, still it is easy to add a test
which checks that sslmode!=require fails with scram_channel_binding=require.

> As the patch stands, if the server is configured for "trust" authentication,
> and the client uses "scram_channel_binding_mode=require", you get this
> error:
>
> psql: channel binding required for authentication but SASL exchange is not
> finished
>
> "SASL exchange is not finished" is quite technical. Can we have something
> like "... but server was configured for \"trust\" authentication"?

OK, here is a suggestion then as in the attached:
"channel binding required, but the server requested authentication \"trust\""

> So, in general, would be good to go through the different combinations of
> scram_channel_binding_mode, sslmode, server configured for SSL or not,
> server configured for different authentication methods, one more time. To
> make sure the error message in each case makes sense, and points to what the
> admin needs to change to make it work.

In the updated patch attached, you get the following failures with all
those configurations (making sslmode=require a requirement with
channel_binding_mode=require reduces the set of combinations a lot):
1) Server supporting SSL.
1-1) v10 server with SCRAM, failure:
$ psql "sslmode=require scram_channel_binding_mode=require host=localhost"
psql: server does not support channel binding, and scram_channel_binding_mode=require was used
1-2) v10 server with md5, password or trust, failures:
$ psql "sslmode=require scram_channel_binding_mode=require host=localhost"
psql: SSL enabled and channel binding required, but the server requested authentication "trust
$ psql "sslmode=require scram_channel_binding_mode=require host=localhost"
psql: SSL enabled and channel binding required, but the server requested
authentication "md5"
1-3) v11 server with SCRAM, works up to password processing...  Of course.
1-4) v11 server with md5, password or trust:
$ psql "sslmode=require scram_channel_binding_mode=require host=localhost"
psql: SSL enabled and channel binding required, but the server requested
authentication "trust"
$ psql "sslmode=require scram_channel_binding_mode=require host=localhost"
psql: SSL enabled and channel binding required, but the server requested
authentication "md5"

So I have added an extra hint to tell that SSL is enabled and added an
assertion as well as this is enforced by sslmode=require.

2) Server not supporting SSL, with failures for all cases as sslmode
overrides any settings of scram_channel_binding_mode=require
2-1) v10 server with SCRAM.
2-2) v10 server with md5, password or trust, failures.
2-3) v11 server with SCRAM: failure.
2-4) v11 server with md5, password or trust: failures.
$ psql "sslmode=require scram_channel_binding_mode=require host=localhost"
psql: server does not support SSL, but SSL was required

Thanks,
--
Michael

Attachment

Re: Postgres 11 release notes

From

"David G. Johnston"

Date:

10 June 2018, 02:35:19

On Fri, May 11, 2018 at 8:08 AM, Bruce Momjian <bruce@momjian.us> wrote:

I have committed the first draft of the Postgres 11 release notes. I
will add more markup soon. You can view the most current version here:

http://momjian.us/pgsql_docs/release-11.html

Some thoughts:

As a major item:

""""

JIT compilation of some SQL code, including support for fast evaluation of expressions

""""

leaves me wondering what the general benefit is to the user. Also, spelling out "Just-In-Time (JIT)" here seems warranted.

"""

No longer retain WAL that spans two checkpoints (Simon Riggs)

The retention of WAL records for only one checkpoint is required.

"""

Maybe: "Servers now only ensure that a single checkpoint record is present in the locally retained WAL. Previously it would ensure at least two checkpoints were available for recovery."

"""

Also, if any table mentioned in VACUUM uses a column list, then ANALYZE keyword must be supplied; previously ANALYZE was implied in such cases.

"""

Should that be mentioned in the compatibility notes?

"""

Add the ability to define PL/pgSQL record types as not null, constant, or with initial values (Tom Lane)

"""

Found the commit message for this one - should we back-patch a documentation change indicating that this doesn't work prior to v11?

David J.

Re: Postgres 11 release notes

From

"Jonathan S. Katz"

Date:

10 June 2018, 02:58:29

On Jun 9, 2018, at 4:35 PM, David G. Johnston <david.g.johnston@gmail.com> wrote:

On Fri, May 11, 2018 at 8:08 AM, Bruce Momjian <bruce@momjian.us> wrote:
I have committed the first draft of the Postgres 11 release notes. I
will add more markup soon. You can view the most current version here:

http://momjian.us/pgsql_docs/release-11.html

Some thoughts:

As a major item:
""""
JIT compilation of some SQL code, including support for fast evaluation of expressions
""""

leaves me wondering what the general benefit is to the user. Also, spelling out "Just-In-Time (JIT)" here seems warranted.

+1 for spelling it out.

Looking through past release notes[1][2], we’ve typically left the context for

the features press releases[3].

Now, as the beta press release (feature-explanation oriented) typically differs from

the major version press release (use-case/context oriented), perhaps we could

incorporate some of the elements from the beta press release into the

“Major Features” section. It would be a departure from the release notes of recent

memory, but perhaps it would be more helpful.

Thoughts?

Jonathan

[1] https://www.postgresql.org/docs/11/static/release-10.html

[2] https://www.postgresql.org/docs/11/static/release-9-6.html

[3] https://www.postgresql.org/about/news/1855/

Re: Postgres 11 release notes

From

Justin Pryzby

Date:

10 June 2018, 05:20:58

On Sat, Jun 09, 2018 at 01:35:19PM -0700, David G. Johnston wrote:
> On Fri, May 11, 2018 at 8:08 AM, Bruce Momjian <bruce@momjian.us> wrote:
> 
> > I have committed the first draft of the Postgres 11 release notes.  I
> > will add more markup soon.  You can view the most current version here:
> >
> >         http://momjian.us/pgsql_docs/release-11.html
> 
> """
> Also, if any table mentioned in VACUUM uses a column list, then ANALYZE
> keyword must be supplied; previously ANALYZE was implied in such cases.
> """
> 
> Should that be mentioned in the compatibility notes?

+1

Note also from 921059bd66c7fb1230c705d3b1a65940800c4cbb
                           
 

This is okay in v10 but rejected in v11b1:
                                                     
 
        postgres=# VACUUM ANALYZE VERBOSE t;
                                                     
 
        ERROR:  syntax error at or near "VERBOSE"
                                                     
 

Justin

Re: SCRAM with channel binding downgrade attack

From

Peter Eisentraut

Date:

11 June 2018, 20:49:01

On 6/6/18 18:04, Michael Paquier wrote:
> On Wed, Jun 06, 2018 at 11:53:06PM +0300, Heikki Linnakangas wrote:
>> That would certainly be good. We've always had that problem, even with md5
>> -> plaintext password downgrade, and it would be nice to fix it. It's quite
>> late in the release cycle already, do you think we should address that now?
>> I could go either way..
> 
> I would be inclined to treat that as new development as this is no new
> problem.

I agree.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: SCRAM with channel binding downgrade attack

From

Magnus Hagander

Date:

11 June 2018, 20:54:45

On Mon, Jun 11, 2018 at 4:49 PM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:

On 6/6/18 18:04, Michael Paquier wrote:
> On Wed, Jun 06, 2018 at 11:53:06PM +0300, Heikki Linnakangas wrote:
>> That would certainly be good. We've always had that problem, even with md5
>> -> plaintext password downgrade, and it would be nice to fix it. It's quite
>> late in the release cycle already, do you think we should address that now?
>> I could go either way..
>
> I would be inclined to treat that as new development as this is no new
> problem.

I agree.

Agreed as well.

I'm wondering if that means we should then also not do it specifically for scram in this version. Otherwise we're likely to end up with a parameter that only has a "lifetime" of one version, and that seems like a bad idea. If nothing else we should clearly think out what the path is to make sure that doesn't happen. (e.g. we don't want a scram_channel_binding_mode=require in this version, if the next one is going to replace it with something like heikkis suggested allowed_authentication_methods=SCRAM-SHA-256-PLUS or whatever we end up coming up with there).

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

12 June 2018, 10:49:26

On Mon, Jun 11, 2018 at 04:54:45PM +0200, Magnus Hagander wrote:
> I'm wondering if that means we should then also not do it specifically for
> scram in this version. Otherwise we're likely to end up with a parameter
> that only has a "lifetime" of one version, and that seems like a bad idea.
> If nothing else we should clearly think out what the path is to make sure
> that doesn't happen. (e.g. we don't want a
> scram_channel_binding_mode=require in this version, if the next one is
> going to replace it with something like heikkis suggested
> allowed_authentication_methods=SCRAM-SHA-256-PLUS or whatever we end up
> coming up with there).

Conceptually, it depends on if we consider SCRAM and
SCRAM+channel_binding as two separate authentication protocols.  However
it seems to me that as both are the same thing as they use the same
protocol so it would be confusing for the user to be able to define both
SCRAM-SHA-256 and SCRAM-SHA-256-PLUS within the same list, so I would
tend to think that we should have in this future
"allowed_authentication_methods" only scram-sha-256 listed as an option,
which counts for both SCRAM with and without channel binding.

Thinking harder about this thread, it could be as well possible in the
future that we add support for channel binding for some other SASL
mechanism, in which case I would tend to rename
scram_channel_binding_type and scram_channel_binding_mode to simply
channel_binding_type and channel_binding_mode, without any concepts of
SCRAM attached to it.  So in short, I'd like to keep both enforcement
mechanisms as separate concepts.  One future compatibility issue is how
to deal with for example cases like allowed_authentication_methods="md5"
and channel_binding_mode=require where an allowed authentication does
not have channel binding?  And it seems to me that this should result in
an error.

Opinions of others are welcome.
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Magnus Hagander

Date:

14 June 2018, 17:43:30

On Tue, Jun 12, 2018 at 6:49 AM, Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Jun 11, 2018 at 04:54:45PM +0200, Magnus Hagander wrote:
> I'm wondering if that means we should then also not do it specifically for
> scram in this version. Otherwise we're likely to end up with a parameter
> that only has a "lifetime" of one version, and that seems like a bad idea.
> If nothing else we should clearly think out what the path is to make sure
> that doesn't happen. (e.g. we don't want a
> scram_channel_binding_mode=require in this version, if the next one is
> going to replace it with something like heikkis suggested
> allowed_authentication_methods=SCRAM-SHA-256-PLUS or whatever we end up
> coming up with there).

Conceptually, it depends on if we consider SCRAM and
SCRAM+channel_binding as two separate authentication protocols. However
it seems to me that as both are the same thing as they use the same
protocol so it would be confusing for the user to be able to define both
SCRAM-SHA-256 and SCRAM-SHA-256-PLUS within the same list, so I would
tend to think that we should have in this future
"allowed_authentication_methods" only scram-sha-256 listed as an option,
which counts for both SCRAM with and without channel binding.

One could argue they are equally the same protocol if we have SCRAM-SHA-512 or whatever in the future. So how would those be handled?

Thinking harder about this thread, it could be as well possible in the
future that we add support for channel binding for some other SASL
mechanism, in which case I would tend to rename
scram_channel_binding_type and scram_channel_binding_mode to simply
channel_binding_type and channel_binding_mode, without any concepts of
SCRAM attached to it. So in short, I'd like to keep both enforcement
mechanisms as separate concepts. One future compatibility issue is how
to deal with for example cases like allowed_authentication_methods="md5"
and channel_binding_mode=require where an allowed authentication does
not have channel binding? And it seems to me that this should result in
an error.

Yeah, not embedding scram in the name seems smarter. But you might still need to define which one, so channel_binding_mode=require wouldn't be enough in that case -- you'd need to have channel_binding_mode=require-scram-sha-256-plus, wouldn't you?

And yes, in your suggested example, it should absolutely fail early, as there is no way to actually connect with that setting. Arguably we should also fail on e.g. sslmode=require over a unix socket, though, which we don't. But that is probably not somethign to copy :)

I still think that the fact that we are still discussing what is basically the *basic concepts* of how this would be set up after we have released beta1 is a clear sign that this should not go into 11.

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: SCRAM with channel binding downgrade attack

From

Robert Haas

Date:

16 June 2018, 03:23:27

On Thu, Jun 14, 2018 at 7:43 AM, Magnus Hagander <magnus@hagander.net> wrote:
> I still think that the fact that we are still discussing what is basically
> the *basic concepts* of how this would be set up after we have released
> beta1 is a clear sign that this should not go into 11.

+1.

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

Re: Postgres 11 release notes

From

Darafei "Komяpa" Praliaskouski

Date:

16 June 2018, 18:56:05

> I'm not sure it is usefull in release notes since it is more about API, and not
> user-facing change. Just in case.
> GiST opclasses now can omit compress and decompress functions. If compress
> function is omited, IndexOnlyScan is enabled for opclass without any extra
> change.
> https://github.com/postgres/postgres/commit/
> d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128

Uh, we do have this for SP-GiST:

Allow SP-GiST indexes to optionally use compression (Teodor Sigaev,
Heikki Linnakangas, Alexander Korotkov, Nikita Glukhov)

I am unclear how far downt the API stack I should go in documenting
changes like this.

It is also a bit misleading - the idea in that change is that now index representation can be a lossy version of actual data type (a box instead of polygon as a referende, so a changelog entry can tell "Allow SP-GiST index creation for polygon datatype."). There is no "decompression" for such thing. "compression" sounds like gzip for me in user-facing context.

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

17 June 2018, 19:21:27

On Fri, Jun 15, 2018 at 05:23:27PM -0400, Robert Haas wrote:
> On Thu, Jun 14, 2018 at 7:43 AM, Magnus Hagander <magnus@hagander.net> wrote:
>> I still think that the fact that we are still discussing what is basically
>> the *basic concepts* of how this would be set up after we have released
>> beta1 is a clear sign that this should not go into 11.
>
> +1.

Yes, that sounds right.
--
Michael

Attachment

signature.asc

Re: Postgres 11 release notes

From

Alexander Korotkov

Date:

19 June 2018, 15:15:28

On Sat, Jun 16, 2018 at 3:57 PM Darafei "Komяpa" Praliaskouski
<me@komzpa.net> wrote:
>>
>> > I'm not sure it is usefull in release notes since it is more about API, and not
>> > user-facing change. Just in case.
>> > GiST opclasses now can omit compress and decompress functions. If compress
>> > function is omited, IndexOnlyScan is enabled for opclass without any extra
>> > change.
>> > https://github.com/postgres/postgres/commit/
>> > d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128
>>
>> Uh, we do have this for SP-GiST:
>>
>>         Allow SP-GiST indexes to optionally use compression (Teodor Sigaev,
>>         Heikki Linnakangas, Alexander Korotkov, Nikita Glukhov)
>>
>> I am unclear how far downt the API stack I should go in documenting
>> changes like this.
>
>
> It is also a bit misleading - the idea in that change is that now index representation can be a lossy version of
actualdata type (a box instead of polygon as a referende, so a changelog entry can tell "Allow SP-GiST index creation
forpolygon datatype.").  There is no "decompression" for such thing. "compression" sounds like gzip for me in
user-facingcontext. 

+1 that current wording looks confusing.  But I think we need to
highlight that we have general SP-GiST improvement, not just support
for particular datatype.  So, I propose following wording: "Allow
SP-GiST to use lossy representation of leaf keys, and add SP-GiST
support for polygon type using that".

I would also like to highlight, that there is a set of typos found my
Liudmila Mantrova in documentation including release notes [1].  As
you know, I'm not native English speaker, but fixes proposed by
Liudmila looks correct for me.

1. https://www.postgresql.org/message-id/275fc450-bbc9-a715-04bb-3b7104fecfcd%40postgrespro.ru

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: Postgres 11 release notes

From

Alexander Korotkov

Date:

19 June 2018, 16:40:38

On Tue, Jun 19, 2018 at 12:15 PM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> On Sat, Jun 16, 2018 at 3:57 PM Darafei "Komяpa" Praliaskouski
> <me@komzpa.net> wrote:
> >>
> >> > I'm not sure it is usefull in release notes since it is more about API, and not
> >> > user-facing change. Just in case.
> >> > GiST opclasses now can omit compress and decompress functions. If compress
> >> > function is omited, IndexOnlyScan is enabled for opclass without any extra
> >> > change.
> >> > https://github.com/postgres/postgres/commit/
> >> > d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128
> >>
> >> Uh, we do have this for SP-GiST:
> >>
> >>         Allow SP-GiST indexes to optionally use compression (Teodor Sigaev,
> >>         Heikki Linnakangas, Alexander Korotkov, Nikita Glukhov)
> >>
> >> I am unclear how far downt the API stack I should go in documenting
> >> changes like this.
> >
> >
> > It is also a bit misleading - the idea in that change is that now index representation can be a lossy version of
actualdata type (a box instead of polygon as a referende, so a changelog entry can tell "Allow SP-GiST index creation
forpolygon datatype.").  There is no "decompression" for such thing. "compression" sounds like gzip for me in
user-facingcontext. 
>
> +1 that current wording looks confusing.  But I think we need to
> highlight that we have general SP-GiST improvement, not just support
> for particular datatype.  So, I propose following wording: "Allow
> SP-GiST to use lossy representation of leaf keys, and add SP-GiST
> support for polygon type using that".

Oh, I missed that we have separate release notes entry for polygons
indexing.  Then second part of sentence isn't needed, it should be
just "Allow SP-GiST to use lossy representation of leaf keys".  If no
objections, I'm going to commit that altogether with fixes by
Liudmila.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment

spgist-compress-release-notes.patch

Re: Postgres 11 release notes

From

Daniel Gustafsson

Date:

19 June 2018, 21:18:16

> On 19 Jun 2018, at 12:40, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
>
> On Tue, Jun 19, 2018 at 12:15 PM Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
>> On Sat, Jun 16, 2018 at 3:57 PM Darafei "Komяpa" Praliaskouski
>> <me@komzpa.net> wrote:
>>>>
>>>>> I'm not sure it is usefull in release notes since it is more about API, and not
>>>>> user-facing change. Just in case.
>>>>> GiST opclasses now can omit compress and decompress functions. If compress
>>>>> function is omited, IndexOnlyScan is enabled for opclass without any extra
>>>>> change.
>>>>> https://github.com/postgres/postgres/commit/
>>>>> d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128
>>>>
>>>> Uh, we do have this for SP-GiST:
>>>>
>>>>        Allow SP-GiST indexes to optionally use compression (Teodor Sigaev,
>>>>        Heikki Linnakangas, Alexander Korotkov, Nikita Glukhov)
>>>>
>>>> I am unclear how far downt the API stack I should go in documenting
>>>> changes like this.
>>>
>>>
>>> It is also a bit misleading - the idea in that change is that now index representation can be a lossy version of
actualdata type (a box instead of polygon as a referende, so a changelog entry can tell "Allow SP-GiST index creation
forpolygon datatype.").  There is no "decompression" for such thing. "compression" sounds like gzip for me in
user-facingcontext. 
>>
>> +1 that current wording looks confusing.  But I think we need to
>> highlight that we have general SP-GiST improvement, not just support
>> for particular datatype.  So, I propose following wording: "Allow
>> SP-GiST to use lossy representation of leaf keys, and add SP-GiST
>> support for polygon type using that".
>
> Oh, I missed that we have separate release notes entry for polygons
> indexing.  Then second part of sentence isn't needed, it should be
> just "Allow SP-GiST to use lossy representation of leaf keys".  If no
> objections, I'm going to commit that altogether with fixes by
> Liudmila.

Speaking of typos, I think I spotted two more: s/features is/feature is/.
Fixed in the attached diff.

cheers ./daniel

Attachment

relnotes11typo.diff

Re: Postgres 11 release notes

From

Alexander Korotkov

Date:

19 June 2018, 21:37:06

On Tue, Jun 19, 2018 at 6:18 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> > On 19 Jun 2018, at 12:40, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
> >
> > On Tue, Jun 19, 2018 at 12:15 PM Alexander Korotkov
> > <a.korotkov@postgrespro.ru> wrote:
> >> On Sat, Jun 16, 2018 at 3:57 PM Darafei "Komяpa" Praliaskouski
> >> <me@komzpa.net> wrote:
> >>>>
> >>>>> I'm not sure it is usefull in release notes since it is more about API, and not
> >>>>> user-facing change. Just in case.
> >>>>> GiST opclasses now can omit compress and decompress functions. If compress
> >>>>> function is omited, IndexOnlyScan is enabled for opclass without any extra
> >>>>> change.
> >>>>> https://github.com/postgres/postgres/commit/
> >>>>> d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128
> >>>>
> >>>> Uh, we do have this for SP-GiST:
> >>>>
> >>>>        Allow SP-GiST indexes to optionally use compression (Teodor Sigaev,
> >>>>        Heikki Linnakangas, Alexander Korotkov, Nikita Glukhov)
> >>>>
> >>>> I am unclear how far downt the API stack I should go in documenting
> >>>> changes like this.
> >>>
> >>>
> >>> It is also a bit misleading - the idea in that change is that now index representation can be a lossy version of
actualdata type (a box instead of polygon as a referende, so a changelog entry can tell "Allow SP-GiST index creation
forpolygon datatype.").  There is no "decompression" for such thing. "compression" sounds like gzip for me in
user-facingcontext. 
> >>
> >> +1 that current wording looks confusing.  But I think we need to
> >> highlight that we have general SP-GiST improvement, not just support
> >> for particular datatype.  So, I propose following wording: "Allow
> >> SP-GiST to use lossy representation of leaf keys, and add SP-GiST
> >> support for polygon type using that".
> >
> > Oh, I missed that we have separate release notes entry for polygons
> > indexing.  Then second part of sentence isn't needed, it should be
> > just "Allow SP-GiST to use lossy representation of leaf keys".  If no
> > objections, I'm going to commit that altogether with fixes by
> > Liudmila.
>
> Speaking of typos, I think I spotted two more: s/features is/feature is/.
> Fixed in the attached diff.

Committed, thanks!

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: Postgres 11 release notes

From

Alexander Korotkov

Date:

21 June 2018, 18:56:10

On Tue, Jun 19, 2018 at 1:40 PM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> On Tue, Jun 19, 2018 at 12:15 PM Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
> > On Sat, Jun 16, 2018 at 3:57 PM Darafei "Komяpa" Praliaskouski
> > <me@komzpa.net> wrote:
> > >>
> > >> > I'm not sure it is usefull in release notes since it is more about API, and not
> > >> > user-facing change. Just in case.
> > >> > GiST opclasses now can omit compress and decompress functions. If compress
> > >> > function is omited, IndexOnlyScan is enabled for opclass without any extra
> > >> > change.
> > >> > https://github.com/postgres/postgres/commit/
> > >> > d3a4f89d8a3e500bd7c0b7a8a8a5ce1b47859128
> > >>
> > >> Uh, we do have this for SP-GiST:
> > >>
> > >>         Allow SP-GiST indexes to optionally use compression (Teodor Sigaev,
> > >>         Heikki Linnakangas, Alexander Korotkov, Nikita Glukhov)
> > >>
> > >> I am unclear how far downt the API stack I should go in documenting
> > >> changes like this.
> > >
> > >
> > > It is also a bit misleading - the idea in that change is that now index representation can be a lossy version of
actualdata type (a box instead of polygon as a referende, so a changelog entry can tell "Allow SP-GiST index creation
forpolygon datatype.").  There is no "decompression" for such thing. "compression" sounds like gzip for me in
user-facingcontext. 
> >
> > +1 that current wording looks confusing.  But I think we need to
> > highlight that we have general SP-GiST improvement, not just support
> > for particular datatype.  So, I propose following wording: "Allow
> > SP-GiST to use lossy representation of leaf keys, and add SP-GiST
> > support for polygon type using that".
>
> Oh, I missed that we have separate release notes entry for polygons
> indexing.  Then second part of sentence isn't needed, it should be
> just "Allow SP-GiST to use lossy representation of leaf keys".  If no
> objections, I'm going to commit that altogether with fixes by
> Liudmila.

Wording improvement is pushed.  Typo fixes are already pushed by Magnus.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

23 June 2018, 09:01:53

On Sun, Jun 17, 2018 at 10:21:27PM +0900, Michael Paquier wrote:
> On Fri, Jun 15, 2018 at 05:23:27PM -0400, Robert Haas wrote:
> > On Thu, Jun 14, 2018 at 7:43 AM, Magnus Hagander <magnus@hagander.net> wrote:
> >> I still think that the fact that we are still discussing what is basically
> >> the *basic concepts* of how this would be set up after we have released
> >> beta1 is a clear sign that this should not go into 11.
> > 
> > +1.
> 
> Yes, that sounds right.

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.

This is different from downgrade attacks like SCRAM to MD5 or MD5 to
'password' because the way the password is transmitted is not integral
to preventing man-in-the-middle attacks.  Channel binding's sole value
is to prevent such attacks, so if it cannot prevent them, it has no use
and will just confuse people until we make it useful in a later release.

-- 
  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 +

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

23 June 2018, 19:30:19

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.
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

23 June 2018, 20:07:01

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 +

Re: SCRAM with channel binding downgrade attack

From

Peter Eisentraut

Date:

27 June 2018, 22:55:28

On 6/14/18 13:43, Magnus Hagander wrote:
> I still think that the fact that we are still discussing what is
> basically the *basic concepts* of how this would be set up after we have
> released beta1 is a clear sign that this should not go into 11.

Other than some naming and handling of some nonsensical combinations,
what is unclear?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: SCRAM with channel binding downgrade attack

From

Alvaro Herrera

Date:

27 June 2018, 23:24:15

Going over this thread a little bit I'm confused about what is being
proposed.  I think I understand that we no longer think we have have
SCRAM channel binding.  I hope that doesn't mean we don't have SCRAM
itself.  However, in terms of the Postgres release proper, what do we
need to do?  There is still an open item about this, and I had the
impression that if we simply demoted channel binding from a pg11 major
feature to barely a footnote that somebody can implement it with some
hypothetical future JDBC driver that supports the option, then we're
done.

Am I mistaken?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: SCRAM with channel binding downgrade attack

From

Magnus Hagander

Date:

28 June 2018, 13:33:09

On Wed, Jun 27, 2018 at 6:55 PM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:

On 6/14/18 13:43, Magnus Hagander wrote:
> I still think that the fact that we are still discussing what is
> basically the *basic concepts* of how this would be set up after we have
> released beta1 is a clear sign that this should not go into 11.

Other than some naming and handling of some nonsensical combinations,
what is unclear?

Should there be one or more parameters? How should they interact? At which level should they be controlled? Limited to SCRAM or other channel bindings? Are the different levels of SCRAM to be considered different protocols or the same protocol with a tweak? etc.

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: SCRAM with channel binding downgrade attack

From

Magnus Hagander

Date:

28 June 2018, 13:35:57

On Wed, Jun 27, 2018 at 7:24 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

Going over this thread a little bit I'm confused about what is being
proposed. I think I understand that we no longer think we have have
SCRAM channel binding. I hope that doesn't mean we don't have SCRAM
itself. However, in terms of the Postgres release proper, what do we
need to do? There is still an open item about this, and I had the
impression that if we simply demoted channel binding from a pg11 major
feature to barely a footnote that somebody can implement it with some
hypothetical future JDBC driver that supports the option, then we're
done.

Am I mistaken?

No, we absolutely still have SCRAM channel binding.

*libpq* has no way to *enforce* it, meaning it always acts like our default SSL config which is "use it if available but if it's not then silently accept the downgrade". From a security perspective, it's just as bad as our default ssl config, but unlike ssl you can't configure a requirement in 11.

There is nothing preventing a third party driver like jdbc or npgsql to implement a way to enforce it. I would generally recommend they wait for the outcome of the discussion about parameters and names in order to implement the same semantics, but they don't have to wait for the next postgres release.

It doesn't affect the having of SCRAM at all. That one is still there, and has been since 10.

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: SCRAM with channel binding downgrade attack

From

Peter Eisentraut

Date:

28 June 2018, 14:04:05

On 6/28/18 09:35, Magnus Hagander wrote:
> No, we absolutely still have SCRAM channel binding.
> 
> *libpq* has no way to *enforce* it, meaning it always acts like our
> default SSL config which is "use it if available but if it's not then
> silently accept the downgrade". From a security perspective, it's just
> as bad as our default ssl config, but unlike ssl you can't configure a
> requirement in 11.

Isn't this similar to what happened whenever we added a new or better
password method?  A MITM that didn't want to bother cracking MD5 could
just alter the stream and request "password" authentication.  Same with
MD5->SCRAM, SCRAM->SCRAM+CB, and even a hypothetical future change in
the SCRAM hashing method.  Clearly, we need a more comprehensive
solution for this.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: SCRAM with channel binding downgrade attack

From

Peter Eisentraut

Date:

28 June 2018, 14:05:23

On 6/28/18 09:33, Magnus Hagander wrote:
> Should there be one or more parameters? How should they interact? At
> which level should they be controlled? Limited to SCRAM or other channel
> bindings? Are the different levels of SCRAM to be considered different
> protocols or the same protocol with a tweak? etc.

OK, I'm fine with postponing this.

But before we drop the SCRAM business completely off the open items, I
think we need to consider how TLS 1.3 affects this.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

SCRAM with channel binding downgrade attack

From

Peter Eisentraut

Date:

28 June 2018, 14:05:47

On 6/28/18 09:35, Magnus Hagander wrote:
> No, we absolutely still have SCRAM channel binding.
> 
> *libpq* has no way to *enforce* it, meaning it always acts like our
> default SSL config which is "use it if available but if it's not then
> silently accept the downgrade". From a security perspective, it's just
> as bad as our default ssl config, but unlike ssl you can't configure a
> requirement in 11.

Isn't this similar to what happened whenever we added a new or better
password method?  A MITM that didn't want to bother cracking MD5 could
just alter the stream and request "password" authentication.  Same with
MD5->SCRAM, SCRAM->SCRAM+CB, and even a hypothetical future change in
the SCRAM hashing method.  Clearly, we need a more comprehensive
solution for this.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: SCRAM with channel binding downgrade attack

From

Magnus Hagander

Date:

28 June 2018, 14:07:44

On Thu, Jun 28, 2018 at 10:04 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:

On 6/28/18 09:35, Magnus Hagander wrote:
> No, we absolutely still have SCRAM channel binding.
>
> *libpq* has no way to *enforce* it, meaning it always acts like our
> default SSL config which is "use it if available but if it's not then
> silently accept the downgrade". From a security perspective, it's just
> as bad as our default ssl config, but unlike ssl you can't configure a
> requirement in 11.

Isn't this similar to what happened whenever we added a new or better
password method? A MITM that didn't want to bother cracking MD5 could
just alter the stream and request "password" authentication. Same with
MD5->SCRAM, SCRAM->SCRAM+CB, and even a hypothetical future change in
the SCRAM hashing method. Clearly, we need a more comprehensive
solution for this.

That is sort of the gist of the discussion, yes. It is.

So if you just enabled scram channel binding, an attacker could just turn off scram completely.

That's why we need a solution that covers the full problem, which is why it needs to be thought of as one problem so we don't end up with a fragmented solution.

Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

Re: SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

28 June 2018, 18:48:29

On Thu, Jun 28, 2018 at 09:35:57AM +0200, Magnus Hagander wrote:
> No, we absolutely still have SCRAM channel binding.
> 
> *libpq* has no way to *enforce* it, meaning it always acts like our default SSL
> config which is "use it if available but if it's not then silently accept the
> downgrade". From a security perspective, it's just as bad as our default ssl
> config, but unlike ssl you can't configure a requirement in 11.

I think we are much more likely to be able to force channel binding by
default since there is no need to configure a certificate authority.

-- 
  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 +

Re: SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

28 June 2018, 18:51:47

On Thu, Jun 28, 2018 at 10:04:05AM +0200, Peter Eisentraut wrote:
> On 6/28/18 09:35, Magnus Hagander wrote:
> > No, we absolutely still have SCRAM channel binding.
> > 
> > *libpq* has no way to *enforce* it, meaning it always acts like our
> > default SSL config which is "use it if available but if it's not then
> > silently accept the downgrade". From a security perspective, it's just
> > as bad as our default ssl config, but unlike ssl you can't configure a
> > requirement in 11.
> 
> Isn't this similar to what happened whenever we added a new or better
> password method?  A MITM that didn't want to bother cracking MD5 could
> just alter the stream and request "password" authentication.  Same with
> MD5->SCRAM, SCRAM->SCRAM+CB, and even a hypothetical future change in
> the SCRAM hashing method.  Clearly, we need a more comprehensive
> solution for this.

The issue is that our different password methods were designed to do a
best-effort at preventing _passive_ snoopers from decrypting or reusing
passwords.  With channel binding, we are trying to prevent _active_
network attacks, and our fallback behavior eliminates the protection
that channel binding provides.

-- 
  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 +

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

29 June 2018, 07:37:55

On Thu, Jun 28, 2018 at 10:05:23AM +0200, Peter Eisentraut wrote:
> But before we drop the SCRAM business completely off the open items, I
> think we need to consider how TLS 1.3 affects this.

The set of APIs that we use to the SSL abstraction layer is very
internal, so it would not be an issue if we add some in stable branches,
no?  My point is that from OpenSSL point of view, TLS 1.3 stuff has been
added in 1.1.1 which is now in beta 6 stage, so we could consider as
well all this part once OpenSSL is released.  That's compatibility work
I wanted to work on anyway.  Impossible to say down to which versions of
Postgres things could be applied easily though without a deep
investigation of the new compatibility breakages that upstream OpenSSL
has very-likely introduced in upstream.

Still it does not sound completely strange either to me to wait for
OpenSSL to release as we won't be able to have a full solution designed
before that.
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Michael Paquier

Date:

29 June 2018, 08:16:01

On Fri, Jun 29, 2018 at 10:37:55AM +0900, Michael Paquier wrote:
> The set of APIs that we use to the SSL abstraction layer is very
> internal, so it would not be an issue if we add some in stable branches,
> no?  My point is that from OpenSSL point of view, TLS 1.3 stuff has been
> added in 1.1.1 which is now in beta 6 stage, so we could consider as
> well all this part once OpenSSL is released.  That's compatibility work
> I wanted to work on anyway.  Impossible to say down to which versions of
> Postgres things could be applied easily though without a deep
> investigation of the new compatibility breakages that upstream OpenSSL
> has very-likely introduced in upstream.
>
> Still it does not sound completely strange either to me to wait for
> OpenSSL to release as we won't be able to have a full solution designed
> before that.

Actually, I got curious about that part and just compiled Postgres with
OpenSSL 1.1.1 beta 6 that I compiled manually, and channel binding is
generating consistent data for both tls-unique and tls-server-end-point
even if TLS v1.3 is used, while tests in src/test/ssl/ are all able to
pass.  So that's less dramatic than what I thought after the melodrama
of upgrading the code to 1.1.0.

The thread where this is discussed is also kind of interesting as the
last email points to having tls-unique deprecated for all the TLS
versions:
https://www.ietf.org/mail-archive/web/tls/current/msg18265.html

I am able to find easily drafts of TLS 1.3, but I am not seeing an RFC
associated to it, which would be the base document to rely on I
guess...  So that's really hard to make any decision in this area
without the real deal.  As far as I can see tls-unique could be
deprecated and replaced, but from OpenSSL point of view it technically
works.
--
Michael

Attachment

signature.asc

Re: SCRAM with channel binding downgrade attack

From

Peter Eisentraut

Date:

30 June 2018, 02:42:36

On 29.06.18 04:16, Michael Paquier wrote:
> I am able to find easily drafts of TLS 1.3, but I am not seeing an RFC
> associated to it, which would be the base document to rely on I
> guess...  So that's really hard to make any decision in this area
> without the real deal.  As far as I can see tls-unique could be
> deprecated and replaced, but from OpenSSL point of view it technically
> works.

At this point, I think we can leave it as is until they came out with a
new channel binding type.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

TLS 1.3 and OpenSSL

From

Peter Eisentraut

Date:

30 June 2018, 02:47:00

On 29.06.18 03:37, Michael Paquier wrote:
> The set of APIs that we use to the SSL abstraction layer is very
> internal, so it would not be an issue if we add some in stable branches,
> no?  My point is that from OpenSSL point of view, TLS 1.3 stuff has been
> added in 1.1.1 which is now in beta 6 stage, so we could consider as
> well all this part once OpenSSL is released.  That's compatibility work
> I wanted to work on anyway.  Impossible to say down to which versions of
> Postgres things could be applied easily though without a deep
> investigation of the new compatibility breakages that upstream OpenSSL
> has very-likely introduced in upstream.

One thing we should look into is that OpenSSL maintains separate cipher
lists for TLS <=1.2 and TLS 1.3.  So the current ssl_ciphers GUC only
affects TLS <=1.2 connections.  We would probably need to add a separate
setting for TLS 1.3.

Here is the relevant man page:
https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_cipher_list.html

This isn't critical, since most people probably run well with the
defaults, but someone once wanted the ssl_ciphers GUC, so they'll
eventually want one for TLS 1.3 as well.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Postgres 11 release notes

From

Brad DeJong

Date:

01 July 2018, 00:11:05

I suggest the following edits for the PostgreSQL beta 2 release announcement.

I did not provide the edits as a patch because I can't find the source of /about/news/1867.

- This release contains ... as well as bug fixes that were reported ...

+ This release contains ... as well as fixes for bugs that were reported ...

- we do not advise for you to run

+ we do not advise you to run

+ we do not advise running

- CHANGES SINCE BETA 2

+ CHANGES SINCE BETA 1

- There have been many bug fixes for PostgreSQL 11 reported during

- the Beta 1 period and applied to the Beta 2 release. Several bug fixes

- reported for version 10 or earlier that also affected version 11 are

- included in the Beta 2 release.

+ Fixes for many of the bugs reported during the PostgreSQL 11 Beta 1

+ period have been applied to the Beta 2 release. Fixes for several bugs

+ reported for version 10 or earlier that affected version 11 have also

+ been included in the Beta 2 release.

or, if you prefer active voice

+ PostgreSQL 11 Beta 2 contains fixes for many of the bugs that were

+ reported during the Beta 1 period and for several bugs reported

+ for version 10 or earlier that affected version 11.

- ... eliminating a needless additional partition constraint checks ...

+ ... eliminating needless additional partition constraint checks ...

- including returning NULL if slot is not advanced

+ including returning NULL if the slot is not advanced

- Fix returning accurate results with ...

This phrasing makes it sound like you didn't want the accurate results so you fixed them. :)

Not being familiar with the fix in question, I do not have a suggestion as to how it should be rephrased.

Re: Postgres 11 release notes

From

Daniel Gustafsson

Date:

01 July 2018, 00:48:35

> On 30 Jun 2018, at 20:11, Brad DeJong <bpd0018@gmail.com> wrote:
>
> I suggest the following edits for the PostgreSQL beta 2 release announcement.
> I did not provide the edits as a patch because I can't find the source of /about/news/1867.

The news item is stored in the postgresql.org database, and not in the code, so
there is no file to patch.  CC:ing -www where website changes are discussed.

cheers ./daniel

Re: Postgres 11 release notes

From

Daniel Gustafsson

Date:

01 July 2018, 00:48:35

> On 30 Jun 2018, at 20:11, Brad DeJong <bpd0018@gmail.com> wrote:
>
> I suggest the following edits for the PostgreSQL beta 2 release announcement.
> I did not provide the edits as a patch because I can't find the source of /about/news/1867.

The news item is stored in the postgresql.org database, and not in the code, so
there is no file to patch.  CC:ing -www where website changes are discussed.

cheers ./daniel

Re: Postgres 11 release notes

From

"Jonathan S. Katz"

Date:

01 July 2018, 00:56:34

> On Jun 30, 2018, at 2:48 PM, Daniel Gustafsson <daniel@yesql.se> wrote:
>
>> On 30 Jun 2018, at 20:11, Brad DeJong <bpd0018@gmail.com> wrote:
>>
>> I suggest the following edits for the PostgreSQL beta 2 release announcement.
>> I did not provide the edits as a patch because I can't find the source of /about/news/1867.
>
> The news item is stored in the postgresql.org database, and not in the code, so
> there is no file to patch.  CC:ing -www where website changes are discussed.

The "CHANGES SINCE BETA 1” was applied earlier thanks to a separate report.

I applied wording fixes for the 3 bulleted items listed. I did not see any reason to
change the opening line as well as intro paragraph to the “CHANGES SINCE BETA 1”
section.

Thanks for reporting!

Jonathan

Re: Postgres 11 release notes

From

"Jonathan S. Katz"

Date:

01 July 2018, 00:56:34

> On Jun 30, 2018, at 2:48 PM, Daniel Gustafsson <daniel@yesql.se> wrote:
>
>> On 30 Jun 2018, at 20:11, Brad DeJong <bpd0018@gmail.com> wrote:
>>
>> I suggest the following edits for the PostgreSQL beta 2 release announcement.
>> I did not provide the edits as a patch because I can't find the source of /about/news/1867.
>
> The news item is stored in the postgresql.org database, and not in the code, so
> there is no file to patch.  CC:ing -www where website changes are discussed.

The "CHANGES SINCE BETA 1” was applied earlier thanks to a separate report.

I applied wording fixes for the 3 bulleted items listed. I did not see any reason to
change the opening line as well as intro paragraph to the “CHANGES SINCE BETA 1”
section.

Thanks for reporting!

Jonathan

Re: Postgres 11 release notes

From

Yaroslav

Date:

01 July 2018, 03:48:38

Hello.

I've noticed the following in v11 release notes:
"Window functions now support all options shown in the SQL:2011 standard,
including RANGE distance PRECEDING/FOLLOWING, GROUPS mode, and frame
exclusion options"

But, as far as I see, IGNORE NULLS option for lead, lag, etc. is still not
supported. May be, the item could be reformulated?

--
WBR, Yaroslav Schekin




-----
WBR, Yaroslav Schekin.
--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html

Re: Postgres 11 release notes

From

Tom Lane

Date:

01 July 2018, 03:52:46

Yaroslav <ladayaroslav@yandex.ru> writes:
> I've noticed the following in v11 release notes:
> "Window functions now support all options shown in the SQL:2011 standard,
> including RANGE distance PRECEDING/FOLLOWING, GROUPS mode, and frame
> exclusion options"

> But, as far as I see, IGNORE NULLS option for lead, lag, etc. is still not
> supported. May be, the item could be reformulated?

Mmm, yeah, I suppose it should say "all framing options" rather than
implying that we've implemented every other window-related frammish
there is in the spec.

            regards, tom lane

Re: Postgres 11 release notes

From

"Jonathan S. Katz"

Date:

02 July 2018, 01:57:38

> On Jun 30, 2018, at 5:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Yaroslav <ladayaroslav@yandex.ru> writes:
>> I've noticed the following in v11 release notes:
>> "Window functions now support all options shown in the SQL:2011 standard,
>> including RANGE distance PRECEDING/FOLLOWING, GROUPS mode, and frame
>> exclusion options"
> 
>> But, as far as I see, IGNORE NULLS option for lead, lag, etc. is still not
>> supported. May be, the item could be reformulated?
> 
> Mmm, yeah, I suppose it should say "all framing options" rather than
> implying that we've implemented every other window-related frammish
> there is in the spec.

+1. Attached patch that does exactly that.

Jonathan

Attachment

0001-window-docs.patch

Re: Postgres 11 release notes

From

Tatsuro Yamada

Date:

09 July 2018, 11:18:14

Hi Bruce,

 > I expect a torrent of feedback.;-)

Could you add this fix to the release note because this change affects
an extension developer using the hook function.
It would be better to know the change by reading the release note, not compilation error.

<!--
2017-01-11 [4d41b2e09] Add QueryEnvironment to ExplainOneQuery_hook's parameter list.
-->
        <para>
         Add QueryEnvironment to ExplainOneQuery_hook's parameter list
         (Tatsuro Yamada, Thomas Munro)
        </para>

Discussion: https://postgr.es/m/890e8dd9-c1c7-a422-6892-874f5eaee048@lab.ntt.co.jp

I guess "E.1.3.11. Source Code" or "E.1.3.12. Additional Modules" sections are
suitable for putting the message in the release note.

Thanks,
Tatsuro Yamada
NTT Open Source Software Center

Re: Postgres 11 release notes

From

Andres Freund

Date:

09 July 2018, 11:28:15

On 2018-07-09 14:18:14 +0900, Tatsuro Yamada wrote:
> Hi Bruce,
> 
> > I expect a torrent of feedback.;-)
> 
> Could you add this fix to the release note because this change affects
> an extension developer using the hook function.
> It would be better to know the change by reading the release note, not compilation error.
> 
> <!--
> 2017-01-11 [4d41b2e09] Add QueryEnvironment to ExplainOneQuery_hook's parameter list.
> -->
>        <para>
>         Add QueryEnvironment to ExplainOneQuery_hook's parameter list
>         (Tatsuro Yamada, Thomas Munro)
>        </para>
> 
> Discussion: https://postgr.es/m/890e8dd9-c1c7-a422-6892-874f5eaee048@lab.ntt.co.jp
> 
> I guess "E.1.3.11. Source Code" or "E.1.3.12. Additional Modules" sections are
> suitable for putting the message in the release note.

We adapt plenty of functions signatures without listing them
individually in the release notes. I don't think randomly selecting one
relatively uncommonly used hook is a good way to attack that.

Greetings,

Andres Freund

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

09 July 2018, 20:36:30

On Sun, Jul  8, 2018 at 10:28:15PM -0700, Andres Freund wrote:
> On 2018-07-09 14:18:14 +0900, Tatsuro Yamada wrote:
> > Hi Bruce,
> > 
> > > I expect a torrent of feedback.;-)
> > 
> > Could you add this fix to the release note because this change affects
> > an extension developer using the hook function.
> > It would be better to know the change by reading the release note, not compilation error.
> > 
> > <!--
> > 2017-01-11 [4d41b2e09] Add QueryEnvironment to ExplainOneQuery_hook's parameter list.
> > -->
> >        <para>
> >         Add QueryEnvironment to ExplainOneQuery_hook's parameter list
> >         (Tatsuro Yamada, Thomas Munro)
> >        </para>
> > 
> > Discussion: https://postgr.es/m/890e8dd9-c1c7-a422-6892-874f5eaee048@lab.ntt.co.jp
> > 
> > I guess "E.1.3.11. Source Code" or "E.1.3.12. Additional Modules" sections are
> > suitable for putting the message in the release note.
> 
> We adapt plenty of functions signatures without listing them
> individually in the release notes. I don't think randomly selecting one
> relatively uncommonly used hook is a good way to attack that.

Agreed.  Ideally we would have a wiki page that lists _all_ the hooks,
and what release they were added.

-- 
  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 +

Re: Postgres 11 release notes

From

David Fetter

Date:

09 July 2018, 20:52:11

On Mon, Jul 09, 2018 at 10:36:30AM -0400, Bruce Momjian wrote:
> On Sun, Jul  8, 2018 at 10:28:15PM -0700, Andres Freund wrote:
> > On 2018-07-09 14:18:14 +0900, Tatsuro Yamada wrote:
> > > Hi Bruce,
> > > 
> > > > I expect a torrent of feedback.;-)
> > > 
> > > Could you add this fix to the release note because this change affects
> > > an extension developer using the hook function.
> > > It would be better to know the change by reading the release note, not compilation error.
> > > 
> > > <!--
> > > 2017-01-11 [4d41b2e09] Add QueryEnvironment to ExplainOneQuery_hook's parameter list.
> > > -->
> > >        <para>
> > >         Add QueryEnvironment to ExplainOneQuery_hook's parameter list
> > >         (Tatsuro Yamada, Thomas Munro)
> > >        </para>
> > > 
> > > Discussion: https://postgr.es/m/890e8dd9-c1c7-a422-6892-874f5eaee048@lab.ntt.co.jp
> > > 
> > > I guess "E.1.3.11. Source Code" or "E.1.3.12. Additional Modules" sections are
> > > suitable for putting the message in the release note.
> > 
> > We adapt plenty of functions signatures without listing them
> > individually in the release notes. I don't think randomly selecting one
> > relatively uncommonly used hook is a good way to attack that.
> 
> Agreed.  Ideally we would have a wiki page that lists _all_ the hooks,
> and what release they were added.

If we're talking about ideals, all our public APIs including the hooks
should be in the official documentation and have man pages.

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Re: Postgres 11 release notes

From

Tatsuro Yamada

Date:

10 July 2018, 07:09:09

On 2018/07/09 23:52, David Fetter wrote:
> On Mon, Jul 09, 2018 at 10:36:30AM -0400, Bruce Momjian wrote:
>> On Sun, Jul  8, 2018 at 10:28:15PM -0700, Andres Freund wrote:
>>> On 2018-07-09 14:18:14 +0900, Tatsuro Yamada wrote:
>>>> Hi Bruce,
>>>>
>>>>> I expect a torrent of feedback.;-)
>>>>
>>>> Could you add this fix to the release note because this change affects
>>>> an extension developer using the hook function.
>>>> It would be better to know the change by reading the release note, not compilation error.
>>>>
>>>> <!--
>>>> 2017-01-11 [4d41b2e09] Add QueryEnvironment to ExplainOneQuery_hook's parameter list.
>>>> -->
>>>>         <para>
>>>>          Add QueryEnvironment to ExplainOneQuery_hook's parameter list
>>>>          (Tatsuro Yamada, Thomas Munro)
>>>>         </para>
>>>>
>>>> Discussion: https://postgr.es/m/890e8dd9-c1c7-a422-6892-874f5eaee048@lab.ntt.co.jp
>>>>
>>>> I guess "E.1.3.11. Source Code" or "E.1.3.12. Additional Modules" sections are
>>>> suitable for putting the message in the release note.
>>>
>>> We adapt plenty of functions signatures without listing them
>>> individually in the release notes. I don't think randomly selecting one
>>> relatively uncommonly used hook is a good way to attack that.
>>
>> Agreed.  Ideally we would have a wiki page that lists _all_ the hooks,
>> and what release they were added.

Did you mean the page is pdf file?
If so, it is not updated from 2012 because it's a presentation material.
I couldn't find other lists by using "hooks" in search box on wiki.

   https://wiki.postgresql.org/images/e/e3/Hooks_in_postgresql.pdf


> If we're talking about ideals, all our public APIs including the hooks
> should be in the official documentation and have man pages.

I think so too.
It is useful information for newbe developer and extension developer.

Thanks,
Tatsuro Yamada

Re: Postgres 11 release notes

From

Andres Freund

Date:

10 July 2018, 07:17:27

On 2018-07-09 16:52:11 +0200, David Fetter wrote:
> On Mon, Jul 09, 2018 at 10:36:30AM -0400, Bruce Momjian wrote:
> > On Sun, Jul  8, 2018 at 10:28:15PM -0700, Andres Freund wrote:
> > > On 2018-07-09 14:18:14 +0900, Tatsuro Yamada wrote:
> > > > Hi Bruce,
> > > > 
> > > > > I expect a torrent of feedback.;-)
> > > > 
> > > > Could you add this fix to the release note because this change affects
> > > > an extension developer using the hook function.
> > > > It would be better to know the change by reading the release note, not compilation error.
> > > > 
> > > > <!--
> > > > 2017-01-11 [4d41b2e09] Add QueryEnvironment to ExplainOneQuery_hook's parameter list.
> > > > -->
> > > >        <para>
> > > >         Add QueryEnvironment to ExplainOneQuery_hook's parameter list
> > > >         (Tatsuro Yamada, Thomas Munro)
> > > >        </para>
> > > > 
> > > > Discussion: https://postgr.es/m/890e8dd9-c1c7-a422-6892-874f5eaee048@lab.ntt.co.jp
> > > > 
> > > > I guess "E.1.3.11. Source Code" or "E.1.3.12. Additional Modules" sections are
> > > > suitable for putting the message in the release note.
> > > 
> > > We adapt plenty of functions signatures without listing them
> > > individually in the release notes. I don't think randomly selecting one
> > > relatively uncommonly used hook is a good way to attack that.
> > 
> > Agreed.  Ideally we would have a wiki page that lists _all_ the hooks,
> > and what release they were added.
> 
> If we're talking about ideals, all our public APIs including the hooks
> should be in the official documentation and have man pages.

FWIW, at this point in time I'd pretty strenuously object to a rule
requiring all hooks / all public functions to be documented. I think the
development velocity penalty would be far greater than the benefit.

That's however *NOT* to say, that we shouldn't document individual API
that we expect to be somewhat stable / frequently externally used (say
the PL interface, C trigger interface).

Greetings,

Andres Freund

Re: Postgres 11 release notes

From

Tom Lane

Date:

12 July 2018, 21:14:55

"Jonathan S. Katz" <jonathan.katz@excoventures.com> writes:
> On Jun 30, 2018, at 5:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Mmm, yeah, I suppose it should say "all framing options" rather than
>> implying that we've implemented every other window-related frammish
>> there is in the spec.

> +1. Attached patch that does exactly that.

Pushed, thanks.

            regards, tom lane

Re: Postgres 11 release notes

From

Masahiko Sawada

Date:

10 August 2018, 11:06:57

Hi,

I found that the release note says "Add pgtrgm function
strict_word_similarity() to compute the similarity of whole words" but
I think "pgtrgm" should be "pg_trgm". Attached patch fixes it.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachment

fix_11_release_note.patch

Re: Postgres 11 release notes

From

Alexander Korotkov

Date:

10 August 2018, 16:10:51

On Fri, Aug 10, 2018 at 8:08 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I found that the release note says "Add pgtrgm function
strict_word_similarity() to compute the similarity of whole words" but
I think "pgtrgm" should be "pg_trgm". Attached patch fixes it.

Right. Pushed, thanks!

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com

The Russian Postgres Company

Re: Postgres 11 release notes

From

Adrien Nayrat

Date:

12 August 2018, 15:21:26

Hello,

It seems this feature is missing in releases notes ?

commit 1f6d515a67ec98194c23a5db25660856c9aab944
Author: Robert Haas <rhaas@postgresql.org>
Date:   Mon Aug 21 14:43:01 2017 -0400

    Push limit through subqueries to underlying sort, where possible.

    Douglas Doole, reviewed by Ashutosh Bapat and by me.  Minor formatting
    change by me.

    Discussion:
http://postgr.es/m/CADE5jYLuugnEEUsyW6Q_4mZFYTxHxaVCQmGAsF0yiY8ZDggi-w@mail.gmail.com

Regards,

Attachment

signature.asc

Re: Postgres 11 release notes

From

Masahiko Sawada

Date:

13 August 2018, 06:10:45

On Fri, Aug 10, 2018 at 7:10 PM, Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> On Fri, Aug 10, 2018 at 8:08 AM Masahiko Sawada <sawada.mshk@gmail.com>
> wrote:
>>
>> I found that the release note says "Add pgtrgm function
>> strict_word_similarity() to compute the similarity of whole words" but
>> I think "pgtrgm" should be "pg_trgm". Attached patch fixes it.
>
>
> Right.  Pushed, thanks!
>

Thank you!

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

26 August 2018, 00:41:11

On Sun, Aug 12, 2018 at 11:21:26AM +0200, Adrien Nayrat wrote:
> Hello,
> 
> It seems this feature is missing in releases notes ?
> 
> commit 1f6d515a67ec98194c23a5db25660856c9aab944
> Author: Robert Haas <rhaas@postgresql.org>
> Date:   Mon Aug 21 14:43:01 2017 -0400
> 
>     Push limit through subqueries to underlying sort, where possible.
> 
>     Douglas Doole, reviewed by Ashutosh Bapat and by me.  Minor formatting
>     change by me.
> 
>     Discussion:
> http://postgr.es/m/CADE5jYLuugnEEUsyW6Q_4mZFYTxHxaVCQmGAsF0yiY8ZDggi-w@mail.gmail.com

I looked into this and it is usually a detail we don't have in the
release notes.  It isn't generally interesting enough to user.

-- 
  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 +

Re: Postgres 11 release notes

From

Andres Freund

Date:

26 August 2018, 00:42:35


On August 25, 2018 11:41:11 AM PDT, Bruce Momjian <bruce@momjian.us> wrote:
>On Sun, Aug 12, 2018 at 11:21:26AM +0200, Adrien Nayrat wrote:
>> Hello,
>>
>> It seems this feature is missing in releases notes ?
>>
>> commit 1f6d515a67ec98194c23a5db25660856c9aab944
>> Author: Robert Haas <rhaas@postgresql.org>
>> Date:   Mon Aug 21 14:43:01 2017 -0400
>>
>>     Push limit through subqueries to underlying sort, where possible.
>>
>>     Douglas Doole, reviewed by Ashutosh Bapat and by me.  Minor
>formatting
>>     change by me.
>>
>>     Discussion:
>>
>http://postgr.es/m/CADE5jYLuugnEEUsyW6Q_4mZFYTxHxaVCQmGAsF0yiY8ZDggi-w@mail.gmail.com
>
>I looked into this and it is usually a detail we don't have in the
>release notes.  It isn't generally interesting enough to user.

This seems quite relevant. Both because it addresses concerns, but also can lead to a worse plan.

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

26 August 2018, 00:47:20

On Sat, Aug 25, 2018 at 11:42:35AM -0700, Andres Freund wrote:
> 
> 
> On August 25, 2018 11:41:11 AM PDT, Bruce Momjian <bruce@momjian.us> wrote:
> >On Sun, Aug 12, 2018 at 11:21:26AM +0200, Adrien Nayrat wrote:
> >> Hello,
> >> 
> >> It seems this feature is missing in releases notes ?
> >> 
> >> commit 1f6d515a67ec98194c23a5db25660856c9aab944
> >> Author: Robert Haas <rhaas@postgresql.org>
> >> Date:   Mon Aug 21 14:43:01 2017 -0400
> >> 
> >>     Push limit through subqueries to underlying sort, where possible.
> >> 
> >>     Douglas Doole, reviewed by Ashutosh Bapat and by me.  Minor
> >formatting
> >>     change by me.
> >> 
> >>     Discussion:
> >>
> >http://postgr.es/m/CADE5jYLuugnEEUsyW6Q_4mZFYTxHxaVCQmGAsF0yiY8ZDggi-w@mail.gmail.com
> >
> >I looked into this and it is usually a detail we don't have in the
> >release notes.  It isn't generally interesting enough to user.
> 
> This seems quite relevant. Both because it addresses concerns, but also can lead to a worse plan.

Well, our normal logical is whether the average user would adjust their
behavior based on this change, or whether it is user visible.  I thought
it was a contrived-enough query that this was not the case, but I would
be interested to hear what others think.

-- 
  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 +

Re: Postgres 11 release notes

From

Andres Freund

Date:

26 August 2018, 01:17:17

Hi,

On 2018-08-25 14:47:20 -0400, Bruce Momjian wrote:
> Well, our normal logical is whether the average user would adjust their
> behavior based on this change, or whether it is user visible.  I thought
> it was a contrived-enough query that this was not the case, but I would
> be interested to hear what others think.

I think that's less "our" logic and more yours, that has become
established because you've done most of the major release notes for a
long time. I'm not trying to say that that's wrong or anything, just
that a lot of those principles grew organically without explicit
decisions.

I also think that the need of userbase have shifted - it used to be that
a lot more people couldn't migrate to PG for lack of features, but these
days we have a much larger userbase that has to migrate from version to
version on a regular basis.

Planner changes, in my experience, are *the* major hurdle in doing so
(leaving aside things like the 8.3 casting changes), because they have
the potential to drastically change performance. In a largely
unpredictable way.  So one of the first things you do after encountering
an issue while testing an issue during a migration, is to look through
the release notes whether that query could be affected by a documented
change.

Greetings,

Andres Freund

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

26 August 2018, 03:18:56

On Sat, Aug 25, 2018 at 12:17:17PM -0700, Andres Freund wrote:
> Hi,
> 
> On 2018-08-25 14:47:20 -0400, Bruce Momjian wrote:
> > Well, our normal logical is whether the average user would adjust their
> > behavior based on this change, or whether it is user visible.  I thought
> > it was a contrived-enough query that this was not the case, but I would
> > be interested to hear what others think.
> 
> I think that's less "our" logic and more yours, that has become
> established because you've done most of the major release notes for a
> long time. I'm not trying to say that that's wrong or anything, just

I don't do my work in a vacuum.  My behavior is based on what feedback I
have gotten from the community on what to include/exclude.

-- 
  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 +

Re: Postgres 11 release notes

From

Peter Geoghegan

Date:

26 August 2018, 03:24:51

On Sat, Aug 25, 2018 at 2:18 PM, Bruce Momjian <bruce@momjian.us> wrote:
>> I think that's less "our" logic and more yours, that has become
>> established because you've done most of the major release notes for a
>> long time. I'm not trying to say that that's wrong or anything, just
>
> I don't do my work in a vacuum.  My behavior is based on what feedback I
> have gotten from the community on what to include/exclude.

FWIW, I agree with what Andres said about planner changes and the release notes.

-- 
Peter Geoghegan

Re: Postgres 11 release notes

From

Amit Kapila

Date:

26 August 2018, 17:23:05

On Sun, Aug 26, 2018 at 12:17 AM Bruce Momjian <bruce@momjian.us> wrote:
>
> On Sat, Aug 25, 2018 at 11:42:35AM -0700, Andres Freund wrote:
> >
> >
> > On August 25, 2018 11:41:11 AM PDT, Bruce Momjian <bruce@momjian.us> wrote:
> > >On Sun, Aug 12, 2018 at 11:21:26AM +0200, Adrien Nayrat wrote:
> > >> Hello,
> > >>
> > >> It seems this feature is missing in releases notes ?
> > >>
> > >> commit 1f6d515a67ec98194c23a5db25660856c9aab944
> > >> Author: Robert Haas <rhaas@postgresql.org>
> > >> Date:   Mon Aug 21 14:43:01 2017 -0400
> > >>
> > >>     Push limit through subqueries to underlying sort, where possible.
> > >>
> > >>     Douglas Doole, reviewed by Ashutosh Bapat and by me.  Minor
> > >formatting
> > >>     change by me.
> > >>
> > >>     Discussion:
> > >>
> > >http://postgr.es/m/CADE5jYLuugnEEUsyW6Q_4mZFYTxHxaVCQmGAsF0yiY8ZDggi-w@mail.gmail.com
> > >
> > >I looked into this and it is usually a detail we don't have in the
> > >release notes.  It isn't generally interesting enough to user.
> >
> > This seems quite relevant. Both because it addresses concerns, but also can lead to a worse plan.

+1.

>
> Well, our normal logical is whether the average user would adjust their
> behavior based on this change, or whether it is user visible.
>

In general, I think this is okay because otherwise providing a lot of
information which most users won't care doesn't serve any purpose.
OTOH, I think it is not easy to decide which features to ignore.  I
think it won't be important to mention some of the code refactoring
commits or minor improvements in the code here and there, but we can't
say that about any of the performance or planner improvements.  Sure,
we might not want to mention everything, but it is better we discuss
such features before ignoring it.  I agree that it will be more work
for preparing release notes, but in the end, we might save some time
by not arguing later about which feature is important and which is
not.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Re: Postgres 11 release notes

From

Michael Banck

Date:

12 September 2018, 23:34:07

Hi,

On Fri, May 11, 2018 at 11:08:52AM -0400, Bruce Momjian wrote:
> I have committed the first draft of the Postgres 11 release notes.  I
> will add more markup soon.  You can view the most current version here:
> 
>     http://momjian.us/pgsql_docs/release-11.html

The first item of section 'E.1.3.10. Server Applications' is mine and
has this TODO (though maybe that should be better marked up?) text:

|IS IT CLEAR FROM THE DOCS THAT THE REPLICATION SLOT IS NOT TEMPORARY?

So I guess we need to decide on that before release. The current doc is
AFAIK:

| This option causes the replication slot specified by the
| option <literal>--slot</literal> to be created before starting the
| backup.  In this case, an error is raised if the slot already exists.

I guess it does not hurt to have something like "to be (permanently)
created before starting", but what do others think?  Is it clear enough?

Michael

-- 
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: michael.banck@credativ.de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer

Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz

Re: Postgres 11 release notes

From

"Jonathan S. Katz"

Date:

15 September 2018, 21:06:04

Hi,

On 9/12/18 1:34 PM, Michael Banck wrote:
> Hi,
>
> On Fri, May 11, 2018 at 11:08:52AM -0400, Bruce Momjian wrote:
>> I have committed the first draft of the Postgres 11 release notes.  I
>> will add more markup soon.  You can view the most current version here:
>>
>>     http://momjian.us/pgsql_docs/release-11.html
>
> The first item of section 'E.1.3.10. Server Applications' is mine and
> has this TODO (though maybe that should be better marked up?) text:
>
> |IS IT CLEAR FROM THE DOCS THAT THE REPLICATION SLOT IS NOT TEMPORARY?
>
> So I guess we need to decide on that before release. The current doc is
> AFAIK:
>
> | This option causes the replication slot specified by the
> | option <literal>--slot</literal> to be created before starting the
> | backup.  In this case, an error is raised if the slot already exists.
>
> I guess it does not hurt to have something like "to be (permanently)
> created before starting", but what do others think?  Is it clear enough?

Attached are two patches.

The first modifies the major improvements section to align with what we
have discovered during the beta period + the general advocacy push. It
also modifies the "last updated date" + narrows down a prospective
release month.

The second modifies/removes the questions/placeholders in the release
notes, such as Michael's comment above (which I agreed with and added a
one sentence explanation after).

Thanks,

Jonathan

Attachment

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

16 September 2018, 21:14:06

On Sat, Sep 15, 2018 at 11:06:04AM -0400, Jonathan Katz wrote:
>         <listitem>
>          <para>
> -         Partitioning by a hash key
> +         Partitioning by a hash key (a.k.a. "hash partitioning")

I question whether a.k.a (also known as) is familiar enough to our
readers to appear in the releaes notes.  I think the "a.k.a." can be
simply removed.

-- 
  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 +

Re: Postgres 11 release notes

From

"Jonathan S. Katz"

Date:

16 September 2018, 21:17:02

On 9/16/18 11:14 AM, Bruce Momjian wrote:
> On Sat, Sep 15, 2018 at 11:06:04AM -0400, Jonathan Katz wrote:
>>         <listitem>
>>          <para>
>> -         Partitioning by a hash key
>> +         Partitioning by a hash key (a.k.a. "hash partitioning")
>
> I question whether a.k.a (also known as) is familiar enough to our
> readers to appear in the releaes notes.  I think the "a.k.a." can be
> simply removed.

Fine by me. I've reattached the patches.

Thanks,

Jonathan

Attachment

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

16 September 2018, 21:36:45

On Sat, Sep 15, 2018 at 11:06:04AM -0400, Jonathan Katz wrote:
> @@ -2414,12 +2408,8 @@ same commits as above
>          The option <option>--create-slot</option> creates
>          the named replication slot (<option>--slot</option>)
>          when the <acronym>WAL</acronym> streaming method
> -        (<option>--wal-method=stream</option>) is used.
> -       </para>
> -
> -       <para>
> -        IS IT CLEAR FROM THE DOCS THAT THE REPLICATION SLOT IS NOT
> -        TEMPORARY?
> +        (<option>--wal-method=stream</option>) is used. The replication slot is
> +        available until it is explicitly dropped.
>         </para>
>        </listitem>

I have clarified the documentation for this option.  This additional
release note text is not necessary.

-- 
  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 +

Re: Postgres 11 release notes

From

"Jonathan S. Katz"

Date:

16 September 2018, 22:07:17

On 9/16/18 11:36 AM, Bruce Momjian wrote:
> On Sat, Sep 15, 2018 at 11:06:04AM -0400, Jonathan Katz wrote:
>> @@ -2414,12 +2408,8 @@ same commits as above
>>          The option <option>--create-slot</option> creates
>>          the named replication slot (<option>--slot</option>)
>>          when the <acronym>WAL</acronym> streaming method
>> -        (<option>--wal-method=stream</option>) is used.
>> -       </para>
>> -
>> -       <para>
>> -        IS IT CLEAR FROM THE DOCS THAT THE REPLICATION SLOT IS NOT
>> -        TEMPORARY?
>> +        (<option>--wal-method=stream</option>) is used. The replication slot is
>> +        available until it is explicitly dropped.
>>         </para>
>>        </listitem>
>
> I have clarified the documentation for this option.  This additional
> release note text is not necessary.

Thanks for updating it in the docs.

I can make arguments both ways about including/not including that in the
release notes, but I've attached a patch that drops the clarification.

Thanks,

Jonathan

Attachment

Re: Postgres 11 release notes

From

Adrien Nayrat

Date:

20 September 2018, 12:47:38

On 8/25/18 11:24 PM, Peter Geoghegan wrote:
> On Sat, Aug 25, 2018 at 2:18 PM, Bruce Momjian <bruce@momjian.us> wrote:
>>> I think that's less "our" logic and more yours, that has become
>>> established because you've done most of the major release notes for a
>>> long time. I'm not trying to say that that's wrong or anything, just
>>
>> I don't do my work in a vacuum.  My behavior is based on what feedback I
>> have gotten from the community on what to include/exclude.
>
> FWIW, I agree with what Andres said about planner changes and the release notes.
>

Hello,

Here is the patch that mention 1f6d515a67. Note I am not sure about my explanation.


Thanks

--
Adrien

On 9/27/18 9:21 PM, Amit Langote wrote:
> On 2018/09/27 23:24, Alvaro Herrera wrote:
>> On 2018-Sep-27, Amit Langote wrote:
>>
>>> Sorry I couldn't reply sooner, but the following of your proposed text
>>> needs to be updated a bit:
>>>
>>> +       <listitem>
>>> +        <para>
>>> +         Having a "default" partition for storing data that does not match a
>>> +         partition key
>>> +        </para>
>>> +       </listitem>
>>>
>>> I think "does not match a partition key" is not accurate.  Description of
>>> default partitions further below in the release notes says this:
>>>
>>> "The default partition can store rows that don't match any of the other
>>> defined partitions, and is searched accordingly."
>>>
>>> So, we could perhaps write it as:
>>>
>>> Having a "default" partition for storing data that does not match any of
>>> the remaining partitions
>>
>> Yeah, I agree that "a partition key" is not the right term to use there
>> (and that term is used in the press release text also).  However I don't
>> think "remaining" is the right word there either, because it sounds as
>> if you're removing something.
>>
>> For the Spanish translation of the press release, we ended up using the
>> equivalent of "for the data that does not match any other partition".
>
> Yeah, "any other partition" is what the existing description uses too, so:
>
> Having a "default" partition for storing data that does not match any
> other partition

Sorry for the slow turnaround on this. Excellent suggestion. I have
updated the patches, please see attached.

Thanks,

Jonathan

Attachment

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

06 October 2018, 03:22:01

On Fri, Oct  5, 2018 at 03:28:41PM -0400, Jonathan Katz wrote:
> On 9/27/18 9:21 PM, Amit Langote wrote:
> > Yeah, "any other partition" is what the existing description uses too, so:
> > 
> > Having a "default" partition for storing data that does not match any
> > other partition
> 
> Sorry for the slow turnaround on this. Excellent suggestion. I have
> updated the patches, please see attached.

Patch applied:

    https://git.postgresql.org/pg/commitdiff/6eb612fea9d080f2ae77ecb7091e73dc9f298c97

-- 
  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 +

Re: Postgres 11 release notes

From

"Jonathan S. Katz"

Date:

06 October 2018, 03:24:03

On 10/5/18 5:22 PM, Bruce Momjian wrote:
> On Fri, Oct  5, 2018 at 03:28:41PM -0400, Jonathan Katz wrote:
>> On 9/27/18 9:21 PM, Amit Langote wrote:
>>> Yeah, "any other partition" is what the existing description uses too, so:
>>>
>>> Having a "default" partition for storing data that does not match any
>>> other partition
>>
>> Sorry for the slow turnaround on this. Excellent suggestion. I have
>> updated the patches, please see attached.
>
> Patch applied:
>
>     https://git.postgresql.org/pg/commitdiff/6eb612fea9d080f2ae77ecb7091e73dc9f298c97
>

Thank you!

Jonathan

Attachment

signature.asc

Re: Postgres 11 release notes

From

Adrien NAYRAT

Date:

08 October 2018, 16:05:03

On 9/20/18 8:47 AM, Adrien Nayrat wrote:
> On 8/25/18 11:24 PM, Peter Geoghegan wrote:
>> On Sat, Aug 25, 2018 at 2:18 PM, Bruce Momjian <bruce@momjian.us> wrote:
>>>> I think that's less "our" logic and more yours, that has become
>>>> established because you've done most of the major release notes for a
>>>> long time. I'm not trying to say that that's wrong or anything, just
>>>
>>> I don't do my work in a vacuum.  My behavior is based on what feedback I
>>> have gotten from the community on what to include/exclude.
>>
>> FWIW, I agree with what Andres said about planner changes and the release notes.
>>
> 
> Hello,
> 
> Here is the patch that mention 1f6d515a67. Note I am not sure about my explanation.
> 
> 
> Thanks
> 

Hi,

It seems this change has not been added to release notes? Should I 
change something in my path?

Thanks,

Re: SCRAM with channel binding downgrade attack

From

Peter Eisentraut

Date:

08 October 2018, 16:42:39

On 05/10/2018 19:01, Bruce Momjian wrote:
> On Fri, Oct  5, 2018 at 04:53:34PM +0200, Peter Eisentraut wrote:
>> On 23/05/2018 08:46, Heikki Linnakangas 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. 
>>
>> A question was raised about this in a recent user group meeting.
>>
>> When someone steals the server certificate from the real database server
>> and sets up a MITM with that certificate, this would pass
>> tls-server-end-point channel binding, because both the MITM and the real
>> server have the same certificate.  But with tls-unique they would have
>> different channel binding data, so the channel binding would detect this.
>>
>> Is that not correct?
> 
> Not correct.  First, they need to steal the server certificate and
> _private_ key that goes with the certificate to impersonate the owner of
> the certificate.

Right, I meant to imply that.

> If that happens, with tls-server-end-point, a MITM
> could replay what the real server sends to the MITM.  You are right that
> tls-unique makes it harder for a MITM to reproduce the TLS shared key
> which is mixed with the password hash to prove the server knows the
> password hash.

So you appear to be saying the above *is* correct?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

08 October 2018, 18:20:03

On Mon, Oct  8, 2018 at 12:05:03PM +0200, Adrien NAYRAT wrote:
> On 9/20/18 8:47 AM, Adrien Nayrat wrote:
> >On 8/25/18 11:24 PM, Peter Geoghegan wrote:
> >>On Sat, Aug 25, 2018 at 2:18 PM, Bruce Momjian <bruce@momjian.us> wrote:
> >>>>I think that's less "our" logic and more yours, that has become
> >>>>established because you've done most of the major release notes for a
> >>>>long time. I'm not trying to say that that's wrong or anything, just
> >>>
> >>>I don't do my work in a vacuum.  My behavior is based on what feedback I
> >>>have gotten from the community on what to include/exclude.
> >>
> >>FWIW, I agree with what Andres said about planner changes and the release notes.
> >>
> >
> >Hello,
> >
> >Here is the patch that mention 1f6d515a67. Note I am not sure about my explanation.
> >
> >
> >Thanks
> >
> 
> Hi,
> 
> It seems this change has not been added to release notes? Should I change
> something in my path?

Uh, where are you looking?  We don't rebuild the beta docs until the
next beta release, but you can see the changes in the developer docs:

    https://www.postgresql.org/docs/devel/static/release-11.html

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

08 October 2018, 18:20:03

On Mon, Oct  8, 2018 at 12:05:03PM +0200, Adrien NAYRAT wrote:
> On 9/20/18 8:47 AM, Adrien Nayrat wrote:
> >On 8/25/18 11:24 PM, Peter Geoghegan wrote:
> >>On Sat, Aug 25, 2018 at 2:18 PM, Bruce Momjian <bruce@momjian.us> wrote:
> >>>>I think that's less "our" logic and more yours, that has become
> >>>>established because you've done most of the major release notes for a
> >>>>long time. I'm not trying to say that that's wrong or anything, just
> >>>
> >>>I don't do my work in a vacuum.  My behavior is based on what feedback I
> >>>have gotten from the community on what to include/exclude.
> >>
> >>FWIW, I agree with what Andres said about planner changes and the release notes.
> >>
> >
> >Hello,
> >
> >Here is the patch that mention 1f6d515a67. Note I am not sure about my explanation.
> >
> >
> >Thanks
> >
> 
> Hi,
> 
> It seems this change has not been added to release notes? Should I change
> something in my path?

Uh, where are you looking?  We don't rebuild the beta docs until the
next beta release, but you can see the changes in the developer docs:

    https://www.postgresql.org/docs/devel/static/release-11.html

-- 
  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 +

Re: SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

08 October 2018, 18:23:48

On Mon, Oct  8, 2018 at 12:42:39PM +0200, Peter Eisentraut wrote:
> On 05/10/2018 19:01, Bruce Momjian wrote:
> > On Fri, Oct  5, 2018 at 04:53:34PM +0200, Peter Eisentraut wrote:
> >> On 23/05/2018 08:46, Heikki Linnakangas 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. 
> >>
> >> A question was raised about this in a recent user group meeting.
> >>
> >> When someone steals the server certificate from the real database server
> >> and sets up a MITM with that certificate, this would pass
> >> tls-server-end-point channel binding, because both the MITM and the real
> >> server have the same certificate.  But with tls-unique they would have
> >> different channel binding data, so the channel binding would detect this.
> >>
> >> Is that not correct?
> > 
> > Not correct.  First, they need to steal the server certificate and
> > _private_ key that goes with the certificate to impersonate the owner of
> > the certificate.
> 
> Right, I meant to imply that.

Right --- that is often confused so I wanted to clarify.

> > If that happens, with tls-server-end-point, a MITM
> > could replay what the real server sends to the MITM.  You are right that
> > tls-unique makes it harder for a MITM to reproduce the TLS shared key
> > which is mixed with the password hash to prove the server knows the
> > password hash.
> 
> So you appear to be saying the above *is* correct?

Yep, I am afraid so.  tls-unique seems technically superior, but I guess
not operationally superior, e.g., Java can't access the TLS shared
secret.  Supporting both causes the kind of channel binding mode
confusion that we have been dealing with, and the new security trend is
not to support too many options because their interaction is often
surprising, and insecure.

-- 
  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 +

Re: SCRAM with channel binding downgrade attack

From

Bruce Momjian

Date:

08 October 2018, 18:23:48

On Mon, Oct  8, 2018 at 12:42:39PM +0200, Peter Eisentraut wrote:
> On 05/10/2018 19:01, Bruce Momjian wrote:
> > On Fri, Oct  5, 2018 at 04:53:34PM +0200, Peter Eisentraut wrote:
> >> On 23/05/2018 08:46, Heikki Linnakangas 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. 
> >>
> >> A question was raised about this in a recent user group meeting.
> >>
> >> When someone steals the server certificate from the real database server
> >> and sets up a MITM with that certificate, this would pass
> >> tls-server-end-point channel binding, because both the MITM and the real
> >> server have the same certificate.  But with tls-unique they would have
> >> different channel binding data, so the channel binding would detect this.
> >>
> >> Is that not correct?
> > 
> > Not correct.  First, they need to steal the server certificate and
> > _private_ key that goes with the certificate to impersonate the owner of
> > the certificate.
> 
> Right, I meant to imply that.

Right --- that is often confused so I wanted to clarify.

> > If that happens, with tls-server-end-point, a MITM
> > could replay what the real server sends to the MITM.  You are right that
> > tls-unique makes it harder for a MITM to reproduce the TLS shared key
> > which is mixed with the password hash to prove the server knows the
> > password hash.
> 
> So you appear to be saying the above *is* correct?

Yep, I am afraid so.  tls-unique seems technically superior, but I guess
not operationally superior, e.g., Java can't access the TLS shared
secret.  Supporting both causes the kind of channel binding mode
confusion that we have been dealing with, and the new security trend is
not to support too many options because their interaction is often
surprising, and insecure.

-- 
  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 +

Re: Postgres 11 release notes

From

Adrien NAYRAT

Date:

08 October 2018, 18:44:32

On 10/8/18 5:20 PM, Bruce Momjian wrote:
> Uh, where are you looking?  We don't rebuild the beta docs until the
> next beta release, but you can see the changes in the developer docs:
> 
>     https://www.postgresql.org/docs/devel/static/release-11.html

I looked in doc/src/sgml/release-11.sgml

And even on https://www.postgresql.org/docs/devel/static/release-11.html 
I don't see mention of 1f6d515a67

Thanks

Re: Postgres 11 release notes

From

Adrien NAYRAT

Date:

08 October 2018, 18:44:32

On 10/8/18 5:20 PM, Bruce Momjian wrote:
> Uh, where are you looking?  We don't rebuild the beta docs until the
> next beta release, but you can see the changes in the developer docs:
> 
>     https://www.postgresql.org/docs/devel/static/release-11.html

I looked in doc/src/sgml/release-11.sgml

And even on https://www.postgresql.org/docs/devel/static/release-11.html 
I don't see mention of 1f6d515a67

Thanks

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

09 October 2018, 04:28:02

On Mon, Oct  8, 2018 at 05:44:32PM +0200, Adrien NAYRAT wrote:
> On 10/8/18 5:20 PM, Bruce Momjian wrote:
> >Uh, where are you looking?  We don't rebuild the beta docs until the
> >next beta release, but you can see the changes in the developer docs:
> >
> >    https://www.postgresql.org/docs/devel/static/release-11.html
> 
> I looked in doc/src/sgml/release-11.sgml
> 
> And even on https://www.postgresql.org/docs/devel/static/release-11.html I
> don't see mention of 1f6d515a67

I did not think there was sufficient interest in adding this item.  If
we add it we need to discuss adding all similar items.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

09 October 2018, 04:28:02

On Mon, Oct  8, 2018 at 05:44:32PM +0200, Adrien NAYRAT wrote:
> On 10/8/18 5:20 PM, Bruce Momjian wrote:
> >Uh, where are you looking?  We don't rebuild the beta docs until the
> >next beta release, but you can see the changes in the developer docs:
> >
> >    https://www.postgresql.org/docs/devel/static/release-11.html
> 
> I looked in doc/src/sgml/release-11.sgml
> 
> And even on https://www.postgresql.org/docs/devel/static/release-11.html I
> don't see mention of 1f6d515a67

I did not think there was sufficient interest in adding this item.  If
we add it we need to discuss adding all similar items.

-- 
  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 +

Re: Postgres 11 release notes

From

Andres Freund

Date:

10 October 2018, 22:55:33

Hi,

On 2018-10-08 21:28:02 -0400, Bruce Momjian wrote:
> On Mon, Oct  8, 2018 at 05:44:32PM +0200, Adrien NAYRAT wrote:
> > On 10/8/18 5:20 PM, Bruce Momjian wrote:
> > >Uh, where are you looking?  We don't rebuild the beta docs until the
> > >next beta release, but you can see the changes in the developer docs:
> > >
> > >    https://www.postgresql.org/docs/devel/static/release-11.html
> > 
> > I looked in doc/src/sgml/release-11.sgml
> > 
> > And even on https://www.postgresql.org/docs/devel/static/release-11.html I
> > don't see mention of 1f6d515a67
> 
> I did not think there was sufficient interest in adding this item.  If
> we add it we need to discuss adding all similar items.

Several people argued for introducing it, you're the only one that was
against it. Adrien, Amit Kapila, Peter Geoghegan, and I all said we
think that kind of thing should be included.

Greetings,

Andres Freund

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

11 October 2018, 01:06:01

On Wed, Oct 10, 2018 at 12:55:33PM -0700, Andres Freund wrote:
> Hi,
> 
> On 2018-10-08 21:28:02 -0400, Bruce Momjian wrote:
> > On Mon, Oct  8, 2018 at 05:44:32PM +0200, Adrien NAYRAT wrote:
> > > On 10/8/18 5:20 PM, Bruce Momjian wrote:
> > > >Uh, where are you looking?  We don't rebuild the beta docs until the
> > > >next beta release, but you can see the changes in the developer docs:
> > > >
> > > >    https://www.postgresql.org/docs/devel/static/release-11.html
> > > 
> > > I looked in doc/src/sgml/release-11.sgml
> > > 
> > > And even on https://www.postgresql.org/docs/devel/static/release-11.html I
> > > don't see mention of 1f6d515a67
> > 
> > I did not think there was sufficient interest in adding this item.  If
> > we add it we need to discuss adding all similar items.
> 
> Several people argued for introducing it, you're the only one that was
> against it. Adrien, Amit Kapila, Peter Geoghegan, and I all said we
> think that kind of thing should be included.

Then you need to start a new thread and argue that we need to include
more optimizer items in the release notes.  My point was that I was
using the existing criteria, and if we want to change it, we have to do
it consistently, not just jam in one item.

Once we agree we can go back and see what we missed in PG 11.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

11 October 2018, 01:13:56

On Wed, Oct 10, 2018 at 06:06:01PM -0400, Bruce Momjian wrote:
> On Wed, Oct 10, 2018 at 12:55:33PM -0700, Andres Freund wrote:
> > Several people argued for introducing it, you're the only one that was
> > against it. Adrien, Amit Kapila, Peter Geoghegan, and I all said we
> > think that kind of thing should be included.
> 
> Then you need to start a new thread and argue that we need to include
> more optimizer items in the release notes.  My point was that I was
> using the existing criteria, and if we want to change it, we have to do
> it consistently, not just jam in one item.
> 
> Once we agree we can go back and see what we missed in PG 11.

Years ago someone said they wanted more optimizer items in the release
notes, so I did it, and then many more people said they didn't want it
--- I don't want to repeat the experience.

-- 
  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 +

Re: Postgres 11 release notes

From

Bruce Momjian

Date:

11 October 2018, 02:57:20

On Wed, Oct 10, 2018 at 06:13:56PM -0400, Bruce Momjian wrote:
> On Wed, Oct 10, 2018 at 06:06:01PM -0400, Bruce Momjian wrote:
> > On Wed, Oct 10, 2018 at 12:55:33PM -0700, Andres Freund wrote:
> > > Several people argued for introducing it, you're the only one that was
> > > against it. Adrien, Amit Kapila, Peter Geoghegan, and I all said we
> > > think that kind of thing should be included.
> > 
> > Then you need to start a new thread and argue that we need to include
> > more optimizer items in the release notes.  My point was that I was
> > using the existing criteria, and if we want to change it, we have to do
> > it consistently, not just jam in one item.
> > 
> > Once we agree we can go back and see what we missed in PG 11.
> 
> Years ago someone said they wanted more optimizer items in the release
> notes, so I did it, and then many more people said they didn't want it
> --- I don't want to repeat the experience.

Thinking some more, if this is this _only_ optimizer improvement you
think I missed, let's just add it and call it done, and we will not
re-litigate the criteria used for the release notes.

-- 
  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 +