From: Tom Lane <tgl@sss.pgh.pa.us>
To: Marcos David <mdavid@palantir.com>
cc: "pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #16106: Patch - Radius secrets always gets lowercased
In-reply-to: <BC05948D-9509-4F30-A350-7E2C36570CF1@palantir.com>
References: <16106-7d319e4295d08e70@postgresql.org>
 <19993.1573503787@sss.pgh.pa.us>
 <BC05948D-9509-4F30-A350-7E2C36570CF1@palantir.com>
Comments: In-reply-to Marcos David <mdavid@palantir.com>
	message dated "Tue, 12 Nov 2019 15:57:57 +0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <7941.1573576426.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Tue, 12 Nov 2019 11:33:46 -0500
Message-ID: <7942.1573576426@sss.pgh.pa.us>
Precedence: bulk

Marcos David <mdavid@palantir.com> writes:
> On 11/11/2019, 20:24, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
>> I'm hesitant to back-patch a change like this, because in theory
>> it could change a working configuration into a non-working one.
>> But it'd be sensible to do in HEAD.

> We only noticed this because  we were upgrading from 9.6 and it seems th=
is bug was introduced in 10 in this commit:
> https://github.com/postgres/postgres/commit/6b76f1bb58f53aec25cfec763912=
70ea36ad1170

Oh!  Hm, if it can be painted as a regression, that changes the calculus
a bit.  In that case I'd be inclined to go ahead and back-patch.

> I don't think patch would break anything in current configs since the se=
cret would currently need to be lowercased anyway for the radius auth to w=
ork.

The case I was imagining was where the secret was entered in the PG
configuration with some uppercase letters, but the server actually
expects lowercase, so the forced lowercasing makes it work.  I admit
that's a bit of a stretch, but if it had always worked like that
then it's at least possible someone was relying on the behavior.
But if we changed the behavior from correct to less correct, that's
another story.

BTW, it looks to me like it should work to double-quote the
secret, although doing so is really tedious because there is an
additional layer of double-quoting required by the pg_hba syntax:

host ... radiussecrets=3D"""ServerSecret"",""OtherServersSecret"""

However, while you can defeat the downcasing that way, you can't
bypass the truncation to NAMEDATALEN.  So it's arguably broken
even if this point had been documented, which it was not,
at least not in any adequate way (the reference to quoting in
the docs is mighty unclear to my eyes, and for sure it doesn't
give a working example).

Unfortunately, we've missed the window to get it into this week's
releases, but I'll see about getting this committed after the release
cycle finishes.

			regards, tom lane