Re: Docs: Encourage strong server verification with SCRAM

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Jacob Champion <jchampion(at)timescale(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>
Subject: Re: Docs: Encourage strong server verification with SCRAM
Date: 2023-05-23 21:02:50
Message-ID: ZG0p+sk/1WkYJ2+l@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings,

* Jacob Champion (jchampion(at)timescale(dot)com) wrote:
> As touched on in past threads, our SCRAM implementation is slightly
> nonstandard and doesn't always protect the entirety of the
> authentication handshake:
>
> - the username in the startup packet is not covered by the SCRAM
> crypto and can be tampered with if channel binding is not in effect,
> or by an attacker holding only the server's key

Encouraging channel binding when using TLS certainly makes a lot of
sense, particularly in environments where the trust anchors are not
strongly managed (that is- if you trust the huge number of root
certificates that a system may have installed). Perhaps explicitly
encouraging users to *not* trust every root server installed on their
client for their PG connections would also be a good idea. We should
probably add language to that effect to this section.

We do default to having channel binding being used though. Breaking
down exactly the cases at risk (perhaps including what version certain
things were introduced) and what direct action administrators can take
to ensure their systems are as secure as possible (and maybe mention
what things that doesn't address still, so they're aware of what risks
still exist) would be good.

> - low iteration counts accepted by the client make it easier than it
> probably should be for a MITM to brute-force passwords (note that
> PG16's scram_iterations GUC, being server-side, does not mitigate
> this)

This would be good to improve on.

> - by default, a SCRAM exchange can be exited by the server prior to
> sending its verifier, skipping the client's server authentication step
> (this is mitigated by requiring channel binding, and PG16 adds
> require_auth=scram-sha-256 to help as well)

Yeah, encouraging this would also be good and should be mentioned as
something to do when one is using SCRAM. Clearly that would go into a
PG16 version of this.

> These aren't currently considered security vulnerabilities, but it'd
> be good for the documentation to call them out, considering mutual
> authentication is one of the design goals of the SCRAM RFC. (I'd also
> like to shore up these problems, eventually, to make SCRAM-based
> mutual authn viable with Postgres. But that work has stalled a bit on
> my end.)

Improving the documentation certainly is a good plan.

> Here's a patch to explicitly warn people away from SCRAM as a form of
> server authentication, and nudge them towards a combination with
> verified TLS or gssenc. I've tried to keep the text version-agnostic,
> to make a potential backport easier. Is this a good place for the
> warning to go? Should I call out that GSS can't use channel binding,
> or promote the use of TLS versus GSS for SCRAM, or just keep it
> simple?

Adding channel binding to GSS (which does support it, we just don't
implement it today..) when using TLS or another encryption method for
transport would be a good improvement to make, particularly right now as
we don't yet support encryption with SSPI, meaning that you need to use
TLS to get session encryption when one of the systems is on Windows. I
do hope to add support for encryption with SSPI during this next release
cycle and would certainly welcome help from anyone else who is
interested in this.

I have to admit that the patch as presented strikes me as a warning
without really providing steps for how to address the issues mentioned
though; there's a reference to what was just covered at the bottom but
nothing really new. The current documentation leads with the warnings
and then goes into steps to take to address the risk, and I'd say it
would make more sense to put this wording about SCRAM not being a
complete solution to this issue above the steps that one can take to
reduce the spoofing risk, similar to how the documentation currently is.

Perhaps more succinctly- maybe we should be making adjustments to the
current language instead of just adding a new paragraph.

Thanks,

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2023-05-23 21:26:42 Re: memory leak in trigger handling (since PG12)
Previous Message Tomas Vondra 2023-05-23 20:57:31 Re: memory leak in trigger handling (since PG12)