From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Daniel Fone <daniel(at)fone(dot)net(dot)nz> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: pgcrypto support for bcrypt $2b$ hashes |
Date: | 2021-09-28 13:33:11 |
Message-ID: | 8FC2FB2B-E2A6-4AF2-9452-0741C4563DE8@yesql.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 28 Sep 2021, at 05:15, Daniel Fone <daniel(at)fone(dot)net(dot)nz> wrote:
>
> Hi Daniel,
>
> Thanks for the feedback.
>
>> On 26/09/2021, at 12:09 AM, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>>
>> But 2b and 2a hashes aren't equal, although very similar. 2a should have the
>> many-buggy to one-correct collision safety and 2b hashes shouldn't. The fact
>> that your hashes work isn't conclusive evidence.
>
> I was afraid this might be a bit naive. Re-reading the crypt_blowfish release notes, it’s principally the changes introducing $2y$ into 1.2 that we need, with support for OpenBSD $2b$ introduced in 1.3. Do I understand this correctly?
Yeah, we'd want a port of 1.3 into pgcrypto essentially.
>> Upgrading our crypt_blowfish.c to the upstream 1.3 version would be the correct
>> fix IMO, but since we have a few local modifications it's not a drop-in. I
>> don't think it would be too hairy, but one needs to be very careful when
>> dealing with crypto.
>
> My C experience is limited, but I can make an initial attempt if the effort would be worthwhile. Is this realistically a patch that a newcomer to the codebase should attempt?
I don't see why not, the best first patches are those scratching an itch. If
you feel up for it then give it a go, I - and the rest of pgsql-hackers - can
help if you need to bounce ideas. Many of the changes in the pgcrypto BF code
is whitespace and formatting, which are performed via pgindent. I would
suggest to familiarize yourself with pgindent in order to tease them out
easier. Another set of changes are around error handling and reporting, which
is postgres specific.
>> Actually it is, in table F.16 in the below documentation page we refer to our
>> supported level as "Blowfish-based, variant 2a”.
>
> Sorry I wasn’t clear. My point was that the docs only mention $2a$, and $2x$ isn’t mentioned even though pgcrypto supports it. As part of the upgrade to 1.3, perhaps the docs can be updated to mention variants x, y, and b as well.
Aha, now I see what you mean, yes you are right. I think the docs should be
updated regardless of the above as a first step to properly match what's in the
tree. Unless there are objections I propose to apply the attached.
--
Daniel Gustafsson https://vmware.com/
Attachment | Content-Type | Size |
---|---|---|
pgcrypto_blowfish.diff | application/octet-stream | 496 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2021-09-28 13:36:34 | Re: Couldn't we mark enum_in() as immutable? |
Previous Message | Magnus Hagander | 2021-09-28 13:23:07 | Re: PROXY protocol support |