Date: Fri, 7 Jan 2022 13:21:10 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Bruce Momjian <bruce@momjian.us>
Cc: Daniil Zakhlystov <usernamedt@yandex-team.ru>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: libpq compression (part 2)
Message-ID: <20220107192110.GG14051@telsasoft.com>
References: <ABAA09C6-BB95-47A5-890D-90353533F9AC@yandex-team.ru>
 <20220101172504.GW24477@telsasoft.com>
 <YdiKaL/AhNV9OtvK@momjian.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YdiKaL/AhNV9OtvK@momjian.us>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220107192110.GG14051%40telsasoft.com>
Precedence: bulk

On Fri, Jan 07, 2022 at 01:46:00PM -0500, Bruce Momjian wrote:
> On Sat, Jan  1, 2022 at 11:25:05AM -0600, Justin Pryzby wrote:
> > Thanks for working on this.  The patch looks to be in good shape - I hope more
> > people will help to review and test it.  I took the liberty of creating a new
> > CF entry. http://cfbot.cputube.org/daniil-zakhlystov.html
> > 
> > +zpq_should_compress(ZpqStream * zpq, char msg_type, uint32 msg_len)
> > +{
> > +   return zpq_choose_compressor(zpq, msg_type, msg_len) == -1;
> > 
> > I think this is backwards , and should say != -1 ?
> > 
> > As written, the server GUC libpq_compression defaults to "on", and the client
> > doesn't request compression.  I think the server GUC should default to off.
> > I failed to convince Kontantin about this last year.  The reason is that 1)
> > it's a new feature; 2) with security implications.  An admin should need to
> > "opt in" to this.  I still wonder if this should be controlled by a new "TYPE"
> > in pg_hba (rather than a GUC); that would make it exclusive of SSL.
> 
> I assume this compression happens before it is encrypted for TLS
> transport.  Second, compression was removed from TLS because there were
> too many ways for HTTP to weaken encryption.  I assume the Postgres wire
> protocol doesn't have similar exploit possibilities.

It's discussed in last year's thread.  The thinking is that there tends to be
*fewer* exploitable opportunities between application->DB than between
browser->app.

But it's still a known concern, and should default to off - as I said.

That's also why I wondered if compression should be controlled by pg_hba,
rather than a GUC.  To require/allow an DBA to opt-in to it for specific hosts.
Or to make it exclusive of ssl.  We could choose to not suppose that case at
all, or (depending on the implement) refuse that combination of layers.

-- 
Justin