Date: Fri, 7 Jan 2022 15:56:39 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Bruce Momjian <bruce@momjian.us>,
	Daniil Zakhlystov <usernamedt@yandex-team.ru>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: libpq compression (part 2)
Message-ID: <20220107205638.GP15820@tamriel.snowman.net>
References: <ABAA09C6-BB95-47A5-890D-90353533F9AC@yandex-team.ru>
 <20220101172504.GW24477@telsasoft.com>
 <YdiKaL/AhNV9OtvK@momjian.us>
 <20220107192110.GG14051@telsasoft.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="OiITrshLgfui8fEl"
Content-Disposition: inline
In-Reply-To: <20220107192110.GG14051@telsasoft.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: 
 <https://www.postgresql.org/message-id/20220107205638.GP15820%40tamriel.snowman.net>
Precedence: bulk


--OiITrshLgfui8fEl
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Justin Pryzby (pryzby@telsasoft.com) wrote:
> 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 creati=
ng a new
> > > CF entry. http://cfbot.cputube.org/daniil-zakhlystov.html
> > >=20
> > > +zpq_should_compress(ZpqStream * zpq, char msg_type, uint32 msg_len)
> > > +{
> > > +   return zpq_choose_compressor(zpq, msg_type, msg_len) =3D=3D -1;
> > >=20
> > > I think this is backwards , and should say !=3D -1 ?
> > >=20
> > > As written, the server GUC libpq_compression defaults to "on", and th=
e client
> > > doesn't request compression.  I think the server GUC should default t=
o off.
> > > I failed to convince Kontantin about this last year.  The reason is t=
hat 1)
> > > it's a new feature; 2) with security implications.  An admin should n=
eed to
> > > "opt in" to this.  I still wonder if this should be controlled by a n=
ew "TYPE"
> > > in pg_hba (rather than a GUC); that would make it exclusive of SSL.
> >=20
> > 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.
>=20
> It's discussed in last year's thread.  The thinking is that there tends t=
o be
> *fewer* exploitable opportunities between application->DB than between
> browser->app.

Yes, this was discussed previously and addressed.

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

I'm not entirely convinced of this but also am happy enough as long as
the capability exists, no matter if it's off or on by default.

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

I'm definitely against us deciding that we know better than admins if
this is an acceptable trade-off in their environment, or not.  Letting
users/admins control it is fine, but I don't feel we should forbid it.

As for the details of how we allow control over it, I suppose there's a
number of options.  Having it in the HBA doesn't seem terrible, though I
suspect most will just want to enable it across the board and having to
have "compression=3Dallowed" or whatever added to every hba line seems
likely to be annoying.  Maybe a global GUC and then allow the hba to
override?

Thanks,

Stephen

--OiITrshLgfui8fEl
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJh2KkGAAoJEO1sijiDR2RVu8EP/3LlRTC5y17Rux86lzEH2xWE
a3c7LAu0cCJWwuvUst0LtHULrhcsStbHJ4giQlMPWAlym8GMw6Sh6oV2a389UlI2
p5eRe8+Pdssa9f4av8NOJhg3o85+MH3lwVckz43s50/LwLBkD1CDqAa/c9BfXdek
nomCVClV7apJe0VeVxx1u6Mhh/9eoz/m7D4vQklmXOaURZgX0/38coucHpYRiFo2
/8Fvt7fSMAUkjvJAXxUPLip69Zoqg9xW0tSgeuwPS4WTWsisIl2H6dZbcLVYDL9Z
lVVVQO51AMFp/HJYzdzKQ9yCo4qJNgvg/AEDDVmZWYbHOl4a9er89WVoV1mhutLS
u7GFyhtfm5nlV9EJC2PqVctE9VQwJgM22+RvTtWqD7mmRYvmHFIkO/+L64rgOmtT
c+lqejU5thf8oZ5ns+Z83akQK8wSPZoEmzeU92ss24xT0Tar80al7vku7Sm2NYVc
fSV95JH6+LZMjAa+kOciRpxNHt+5zSDT5cITGNzFTDKtlC0DRxbUAbFiQOATuqDv
3rqb2FrpHZnh6/q8c24YkWA0NxS4AJHtHoClhjztA1QZQSPI1e5VkkxpVzsGa0qL
Kb8rZDW9iYthQwfydVymJ/M2c4sA99SKemS4Y/NkbX7sqiENCKDrlutIDW1JLVlm
BtWMTLeNsvtzcNhnk9Ub
=7bxI
-----END PGP SIGNATURE-----

--OiITrshLgfui8fEl--