Re: [PoC] Let libpq reject unexpected authentication requests

From: Jacob Champion <jchampion(at)timescale(dot)com>
To: Aleksander Alekseev <aleksander(at)timescale(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Subject: Re: [PoC] Let libpq reject unexpected authentication requests
Date: 2022-11-12 00:11:07
Message-ID: ba30fb31-719e-a438-2a1a-413a6b09cf5c@timescale.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 11/11/22 05:52, Aleksander Alekseev wrote:
> I noticed that this patchset stuck a bit so I decided to take a look.

Thanks!

> Assigning a negative number to uint32 doesn't necessarily work on all
> platforms. I suggest using PG_UINT32_MAX.

Hmm -- on which platforms is "-1 converted to unsigned" not equivalent
to the maximum value? Are they C-compliant?

> The commit message IMO has a better description of "require". I
> suggest adding the part about "This doesn't add any additional
> security ..." to the documentation.

Sounds good; see what you think of v12.

> ```
> + * hard-coded certificate via sslcert, so we don't actually set any
> certificates
> + * here; we just it to record whether or not the server has actually asked for
> ```
>
> Something is off with the wording here in the "we just it to ..." part.

Fixed.

> The patchset seems to be in very good shape except for these few
> nitpicks. I'm inclined to change its status to "Ready for Committer"
> as soon as the new version will pass cfbot unless there are going to
> be any objections from the community.

Thank you! I expect a maintainer will need to weigh in on the
cost/benefit of 0003 either way.

--Jacob

Attachment Content-Type Size
since-v11.diff.txt text/plain 2.2 KB
v12-0001-libpq-let-client-reject-unexpected-auth-methods.patch text/x-patch 33.2 KB
v12-0002-Add-sslcertmode-option-for-client-certificates.patch text/x-patch 17.1 KB
v12-0003-require_auth-decouple-SASL-and-SCRAM.patch text/x-patch 8.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2022-11-12 04:12:06 Re: Typo about subxip in comments
Previous Message Jeff Davis 2022-11-11 23:10:20 Re: Add test module for Custom WAL Resource Manager feature