Re: Replace current implementations in crypt() and gen_salt() to OpenSSL

On 11/21/24 15:43, Robert Haas wrote:
> On Thu, Nov 21, 2024 at 2:06 PM Joe Conway <mail(at)joeconway(dot)com> wrote:
>> Because that idea was rejected earlier in the thread by multiple people
>> other than me?
>>
>> ¯\_(ツ)_/¯
>
> I actually don't see that in the thread anywhere. Which messages are
> you talking about?

Well there is this:
> From: Peter Eisentraut <peter(at)eisentraut(dot)org>

> On 15.02.24 13:42, Koshi Shibagaki (Fujitsu) wrote:
>> However, crypt() and gen_salt() do not use OpenSSL as mentioned in [2].
>> Therefore, if we run crypt() and gen_salt() on a machine with FIPS mode enabled,
>> they are not affected by FIPS mode. This means we can use encryption algorithms
>> disallowed in FIPS.
>>
>> I would like to change the proprietary implementations of crypt() and gen_salt()
>> to use OpenSSL API.
>> If it's not a problem, I am going to create a patch, but if you have a better
>> approach, please let me know.
>
> The problems are:
>
> 1. All the block ciphers currently supported by crypt() and gen_salt()
> are not FIPS-compliant.
>
> 2. The crypt() and gen_salt() methods built on top of them (modes of
> operation, kind of) are not FIPS-compliant.
>
> 3. The implementations (crypt-blowfish.c, crypt-des.c, etc.) are not
> structured in a way that OpenSSL calls can easily be patched in.
>
> So if you want FIPS-compliant cryptography, these interfaces look like a
> dead end. I don't know if there are any modern equivalents of these
> functions that we should be supplying instead.

and this

> From: Peter Eisentraut <peter(at)eisentraut(dot)org>
>
> On 20.02.24 12:39, Daniel Gustafsson wrote:
>> A fifth option is to throw away our in-tree implementations and use the OpenSSL
>> API's for everything, which is where this thread started. If the effort to
>> payoff ratio is palatable to anyone then patches are for sure welcome.
>
>
> The problem is that, as I understand it, these crypt routines are not
> designed in a way that you can just plug in a crypto library underneath.
> Effectively, the definition of what, say, blowfish crypt does, is
> whatever is in that source file, and transitively, whatever OpenBSD
> does. (Fun question: Does OpenBSD care about FIPS?) Of course, you
> could reimplement the same algorithms independently, using OpenSSL or
> whatever. But I don't think this will really improve the state of the
> world in aggregate, because to a large degree we are relying on the
> upstream to keep these implementations maintained, and if we rewrite
> them, we become the upstream.

And Daniel proposed this:

> From: Daniel Gustafsson <daniel(at)yesql(dot)se>

>> On 15 Feb 2024, at 16:49, Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
>> 1. All the block ciphers currently supported by crypt() and gen_salt() are not FIPS-compliant.
>>
>> 2. The crypt() and gen_salt() methods built on top of them (modes of operation, kind of) are not FIPS-compliant.
>
> I wonder if it's worth trying to make pgcrypto disallow non-FIPS compliant
> ciphers when the compiled against OpenSSL is running with FIPS mode enabled, or
> raise a WARNING when used? It seems rather unlikely that someone running
> OpenSSL with FIPS=yes want to use our DES cipher without there being an error
> or misconfiguration somewhere.

and then there is this:

> From: "Koshi Shibagaki (Fujitsu)" <shibagaki(dot)koshi(at)fujitsu(dot)com>

> Dear Daniel
>
> Thanks for your reply.
>
>> I wonder if it's worth trying to make pgcrypto disallow non-FIPS compliant
>> ciphers when the compiled against OpenSSL is running with FIPS mode
>> enabled, or raise a WARNING when used? It seems rather unlikely that
>> someone running OpenSSL with FIPS=yes want to use our DES cipher without
>> there being an error or misconfiguration somewhere.
>
> Indeed, users do not use non-FIPS compliant ciphers in crypt() and gen_salt()
> such as DES with FIPS mode enabled.
> However, can we reduce human error by having these functions make the judgment
> as to whether ciphers can or cannot be used?
>
> If pgcrypto checks if FIPS enabled or not as in the pseudocode, it is easier to
> achieve than replacing to OpenSSL.
> Currently, OpenSSL internally determines if it is in FIPS mode or not, but would
> it be a problem to have PostgreSQL take on that role?

I mean, perhaps I am misreading and/or interpreting all of that
differently to you, but from my reading of the entire thread there was
clearly no consensus to using openssl to provide those two functions.

Frankly I don't care which solution is picked as long as we pick
something that allows users of pgcrypto to be FIPS complaint, and we
don't drag this out past pg18 feature freeze because there are too many
opinions and no one is willing to take a stand.

So the patch I posted is my attempt to take a stand. If you have a
better patch you would like to propose to fix this problem, please do.

--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com