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 |
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 |