Re: Commitfest 2021-11 Patch Triage - Part 2

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Commitfest 2021-11 Patch Triage - Part 2
Date: 2021-11-12 16:04:08
Message-ID: 4E2714D0-1A6B-4921-A75B-AFC666DBD568@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> 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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-11-12 16:54:20 Re: [PATCH] postgres_fdw: suppress explicit casts in text:text comparisons (was: column option to override foreign types)
Previous Message Andrew Dunstan 2021-11-12 15:42:14 Re: Test::More version