Greetings, * Justin Pryzby (pry...@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 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.
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=allowed" 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
signature.asc
Description: PGP signature
