Re: Commitfest 2021-11 Patch Triage - Part 2

> On 9 Nov 2021, at 18:43, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>>> 2773: libpq compression
>>> =======================
>>> This patch intended to provide libpq connection compression to "replace SSL
>>> compression" which was doomed when the patch was written, and have since been
>>> removed altogether. The initial approach didn't get much traction but there
>>> was significant discussion and work, which has since fizzled out. The patch
>>> has been updated but there hasn't been meaningful review the past months, the
>>> last comments seem to imply there being a fair amount of questionmarks left in
>>> here. Robert, having been very involved in this do you have any thoughts on
>>> where we are and where to go (if at all IYO)?
>>
>> I'm not Robert, but I still have an opinion here, and that it's that this
>> feature would at best be an attractive nuisance. If you need compression
>> on a database session, it probably means that the connection is over the
>> open internet, which means that you need encryption even more. And we
>> know that compression and encryption do not play well together. The
>> reason compression was taken out of the latest TLS standards is not that
>> they wouldn't have liked to have it, nor that applying compression in a
>> separate code layer would be any safer. I fear offering this would
>> merely lead people to build CVE-worthy setups.
>
> I've got an opinion on this also and I don't agree that needing
> compression means you're on the open internet or that we shouldn't allow
> users the choice to decide if they want to use compression and
> encryption together or not. Yes, there's potential risks there, but I
> don't think those risks would lead to CVEs against PG for supporting a
> mode where we allow compression and then also allow encryption- if
> that's a problem then it's an issue for the encryption library/method
> being used and isn't the fault of us for allowing that combination.

I'm not so sure, if the parts aren't vulnerable separately on their own but the
combination of them + libpq allows for weakening encryption in a TLS stream I'd
say the CVE is on us.

> Further, the general issue with encryption+compression attacks is when
> the attacker is able to inject chosen plaintext content into what ends
> up being compressed and encrypted, and then is able to observe the
> compressed+encrypted traffic, and that's not nearly as easy to do when
> you're talking about a database connection vs. a case where an attacker
> is able to convince a victim to go to a malicious website and through
> that is able to do cross-site requests to inject the known plaintext
> into requests to a trusted webpage and see the results coming back.

I agree with this, the known weaknesses rely on mechanisms that aren't readily
applicable to postgres connections. I don't remember if the patch already does
this, but if not we should ensure we only negotiate compression after
authentication like how OpenSSH does [0].

--
Daniel Gustafsson https://vmware.com/

[0] https://www.openssh.com/txt/draft-miller-secsh-compression-delayed-00.txt