Re: Removing the fixed-size buffer restriction in hba.c

Nathan Bossart <nathandbossart@gmail.com>

From: Nathan Bossart <nathandbossart@gmail.com>

To: Fabrízio de Royes Mello <fabriziomello@gmail.com>

Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@lists.postgresql.org

Date: 2023-07-24T22:58:31Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Raise fixed token-length limit in hba.c.
- 341996248e4d 14.9 landed
- 313ceda2fe2e 15.4 landed
- 288b4288c3db 13.12 landed
- 1d031ad54d37 11.21 landed
- 0660f74e861d 12.16 landed
Eliminate fixed token-length limit in hba.c.
- de3f0e3fe0e7 16.0 landed
- 38df84c65ea4 17.0 landed

On Mon, Jul 24, 2023 at 03:07:07PM -0300, Fabrízio de Royes Mello wrote:
> On Mon, Jul 24, 2023 at 2:53 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> We got a complaint at [1] about how a not-so-unreasonable LDAP
>> configuration can hit the "authentication file token too long,
>> skipping" error case in hba.c's next_token().  I think we've
>> seen similar complaints before, although a desultory archives
>> search didn't turn one up.
>>
>> A minimum-change response would be to increase the MAX_TOKEN
>> constant from 256 to (say) 1K or 10K.  But it wouldn't be all
>> that hard to replace the fixed-size buffer with a StringInfo,
>> as attached.
>>
> 
> +1 for replacing it with StringInfo. And the patch LGTM!

I'd suggest noting that next_token() clears buf, but otherwise it looks
reasonable to me, too.

>> Given the infrequency of complaints, I'm inclined to apply
>> the more thorough fix only in HEAD, and to just raise MAX_TOKEN
>> in the back branches.  Thoughts?
>>
> 
> It makes sense to change it only in HEAD.

I wouldn't be opposed to back-patching the more thorough fix, but I don't
feel too strongly about it.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com