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

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

From
"Koshi Shibagaki (Fujitsu)"
Date:
Hi
This is Shibagaki.

When FIPS mode is enabled, some encryption algorithms cannot be used.
Since PostgreSQL15, pgcrypto requires OpenSSL[1], digest() and other functions
also follow this policy.

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.

Thank you


[1] https://github.com/postgres/postgres/commit/db7d1a7b0530e8cbd045744e1c75b0e63fb6916f
[2] https://peter.eisentraut.org/blog/2023/12/05/postgresql-and-fips-mode

crypt() and gen_salt() are performed on in example below.

/////

-- OS RHEL8.6

$openssl version
OpenSSL 1.1.1k  FIPS 25 Mar 2021

$fips-mode-setup --check
FIPS mode is enabled.

$./pgsql17/bin/psql
psql (17devel)
Type "help" for help.

postgres=# SHOW server_version;
 server_version
----------------
 17devel
(1 row)

postgres=# SELECT digest('data','md5');
ERROR:  Cannot use "md5": Cipher cannot be initialized

postgres=# SELECT crypt('new password',gen_salt('md5')); -- md5 is not available when fips mode is turned on. This is a
normalbehavior 
ERROR:  crypt(3) returned NULL

postgres=# SELECT crypt('new password',gen_salt('des')); -- however, des is avalable. This may break a FIPS rule
     crypt
---------------
 32REGk7H6dSnE
(1 row)

/////

FYI - OpenSSL itself cannot use DES algorithm while encrypting files. This is an expected behavior.

-----------------------------------------------
Fujitsu Limited
Shibagaki Koshi
shibagaki.koshi@fujitsu.com





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

From
Peter Eisentraut
Date:
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.




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

From
Daniel Gustafsson
Date:
> On 15 Feb 2024, at 16:49, Peter Eisentraut <peter@eisentraut.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.

Something like the below untested pseudocode.

diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index 96447c5757..3d4391ebe1 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -187,6 +187,14 @@ pg_crypt(PG_FUNCTION_ARGS)
                *resbuf;
     text       *res;

+#if defined FIPS_mode
+    if (FIPS_mode())
+#else
+    if (EVP_default_properties_is_fips_enabled(OSSL_LIB_CTX_get0_global_default()))
+#endif
+        ereport(ERROR,
+                (errmsg("not available when using OpenSSL in FIPS mode")));
+
     buf0 = text_to_cstring(arg0);
     buf1 = text_to_cstring(arg1);

Greenplum implemented similar functionality but with a GUC, fips_mode=<bool>.
The problem with that is that it gives the illusion that enabling such a GUC
gives any guarantees about FIPS which isn't really the case since postgres
isn't FIPS certified.

--
Daniel Gustafsson




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

From
"Koshi Shibagaki (Fujitsu)"
Date:
Dear Peter

Thanks for the replying

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

Indeed, all the algorithm could not be used in FIPS and huge engineering might 
be needed for the replacement. If the benefit is smaller than the cost, we 
should consider another way - e.g., prohibit to call these functions in FIPS 
mode as in the pseudocode Daniel sent. Replacing OpenSSL is a way, the objective
is to eliminate the user's error in choosing an encryption algorithm.


-----------------------------------------------
Fujitsu Limited
Shibagaki Koshi
shibagaki.koshi@fujitsu.com




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

From
"Koshi Shibagaki (Fujitsu)"
Date:
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?

-----------------------------------------------
Fujitsu Limited
Shibagaki Koshi
shibagaki.koshi@fujitsu.com





On 2/16/24 04:16, Daniel Gustafsson wrote:
>> On 15 Feb 2024, at 16:49, Peter Eisentraut <peter@eisentraut.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.
> 
> Something like the below untested pseudocode.
> 
> diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
> index 96447c5757..3d4391ebe1 100644
> --- a/contrib/pgcrypto/pgcrypto.c
> +++ b/contrib/pgcrypto/pgcrypto.c
> @@ -187,6 +187,14 @@ pg_crypt(PG_FUNCTION_ARGS)
>                  *resbuf;
>       text       *res;
>   
> +#if defined FIPS_mode
> +    if (FIPS_mode())
> +#else
> +    if (EVP_default_properties_is_fips_enabled(OSSL_LIB_CTX_get0_global_default()))
> +#endif
> +        ereport(ERROR,
> +                (errmsg("not available when using OpenSSL in FIPS mode")));

Makes sense +1

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




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

From
Peter Eisentraut
Date:
On 16.02.24 10:16, Daniel Gustafsson wrote:
>> 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.

I wonder on what level this kind of check would be done.  For example, 
the password hashing done for SCRAM is not FIPS-compliant either, but 
surely we don't want to disallow that.  Maybe this should be done on the 
level of block ciphers.  So if someone wanted to add a "crypt-aes" 
module, that would then continue to work.



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

From
Daniel Gustafsson
Date:
> On 16 Feb 2024, at 13:57, Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 16.02.24 10:16, Daniel Gustafsson wrote:
>>> 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.
>
> I wonder on what level this kind of check would be done.  For example, the password hashing done for SCRAM is not
FIPS-complianteither, but surely we don't want to disallow that. 

Can you elaborate?  When building with OpenSSL all SCRAM hashing will use the
OpenSSL implementation of pg_hmac and pg_cryptohash, so it would be subject to
OpenSSL FIPS configuration no?

> Maybe this should be done on the level of block ciphers.  So if someone wanted to add a "crypt-aes" module, that
wouldthen continue to work. 

That's a fair point, we can check individual ciphers.  I'll hack up a version
doing this.

--
Daniel Gustafsson




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

From
Peter Eisentraut
Date:
On 16.02.24 14:30, Daniel Gustafsson wrote:
>> On 16 Feb 2024, at 13:57, Peter Eisentraut <peter@eisentraut.org> wrote:
>>
>> On 16.02.24 10:16, Daniel Gustafsson wrote:
>>>> 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.
>>
>> I wonder on what level this kind of check would be done.  For example, the password hashing done for SCRAM is not
FIPS-complianteither, but surely we don't want to disallow that.
 
> 
> Can you elaborate?  When building with OpenSSL all SCRAM hashing will use the
> OpenSSL implementation of pg_hmac and pg_cryptohash, so it would be subject to
> OpenSSL FIPS configuration no?

Yes, but the overall methods of composing all this into secrets and 
protocol messages etc. are not covered by FIPS.

>> Maybe this should be done on the level of block ciphers.  So if someone wanted to add a "crypt-aes" module, that
wouldthen continue to work.
 
> 
> That's a fair point, we can check individual ciphers.  I'll hack up a version
> doing this.

Like, if we did a "crypt-aes", would that be FIPS-compliant?  I don't know.




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

From
Daniel Gustafsson
Date:
> On 16 Feb 2024, at 15:49, Peter Eisentraut <peter@eisentraut.org> wrote:

> Like, if we did a "crypt-aes", would that be FIPS-compliant?  I don't know.

If I remember my FIPS correct: Only if it used a FIPS certified implementation,
like the one in OpenSSL when the fips provider has been loaded.  The cipher
must be allowed *and* the implementation must be certified.

--
Daniel Gustafsson




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

From
"Koshi Shibagaki (Fujitsu)"
Date:
Let me confirm the discussion in threads. I think there are two topics.
1. prohibit the use of ciphers disallowed in FIPS mode at the level of block
cipher (crypt-bf, etc...) in crypt() and gen_salt()
2. adding new "crypt-aes" module.

If this is correct, I would like to make a patch for the first topic, as I think
I can handle it.
Daniel, please let me know if you have been making a patch based on the idea.


Also, I think the second one should be discussed in a separate thread, so could
you split it into a separate thread?

Thank you

-----------------------------------------------
Fujitsu Limited
Shibagaki Koshi
shibagaki.koshi@fujitsu.com





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

From
Daniel Gustafsson
Date:
> On 20 Feb 2024, at 10:56, Koshi Shibagaki (Fujitsu) <shibagaki.koshi@fujitsu.com> wrote:

> Let me confirm the discussion in threads. I think there are two topics.
> 1. prohibit the use of ciphers disallowed in FIPS mode at the level of block
> cipher (crypt-bf, etc...) in crypt() and gen_salt()

That level might be overkill given that any cipher not in the FIPS certfied
module mustn't be used, but it's also not the wrong place to put it IMHO.

> 2. adding new "crypt-aes" module.

I think this was a hypothetical scenario and not a concrete proposal.

> If this is correct, I would like to make a patch for the first topic, as I think
> I can handle it.
> Daniel, please let me know if you have been making a patch based on the idea.

I haven't yet started on that so feel free to take a stab at it, I'd be happy
to review it.  Note that there are different API's for doing this in OpenSSL
1.0.2 and OpenSSL 3.x, so a solution must take both into consideration.

--
Daniel Gustafsson




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

From
Peter Eisentraut
Date:
On 20.02.24 11:09, Daniel Gustafsson wrote:
>> On 20 Feb 2024, at 10:56, Koshi Shibagaki (Fujitsu) <shibagaki.koshi@fujitsu.com> wrote:
> 
>> Let me confirm the discussion in threads. I think there are two topics.
>> 1. prohibit the use of ciphers disallowed in FIPS mode at the level of block
>> cipher (crypt-bf, etc...) in crypt() and gen_salt()
> 
> That level might be overkill given that any cipher not in the FIPS certfied
> module mustn't be used, but it's also not the wrong place to put it IMHO.

I think we are going about this the wrong way.  It doesn't make sense to 
ask OpenSSL what a piece of code that doesn't use OpenSSL should do. 
(And would that even give a sensible answer?  Like, you can configure 
OpenSSL to load the fips module, but you can also load the legacy module 
alongside it(??).)  And as you say, even if this code supported modern 
block ciphers, it wouldn't be FIPS compliant.

I think there are several less weird ways to address this:

* Just document it.

* Make a pgcrypto-level GUC setting.

* Split out these functions into a separate extension.

* Deprecate these functions.

Or some combination of these.




On Tue, Feb 20, 2024 at 4:49 PM Peter Eisentraut <peter@eisentraut.org> wrote:
> I think there are several less weird ways to address this:
>
> * Just document it.
>
> * Make a pgcrypto-level GUC setting.
>
> * Split out these functions into a separate extension.
>
> * Deprecate these functions.
>
> Or some combination of these.

I don't think the first two of these proposals help anything. AIUI,
FIPS mode is supposed to be a system wide toggle that affects
everything on the machine. The third one might help if you can be
compliant by just choosing not to install that extension, and the
fourth one solves the problem by sledgehammer.

Does Linux provide some way of asking whether "fips=1" was specified
at kernel boot time?

--
Robert Haas
EDB: http://www.enterprisedb.com



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

From
Daniel Gustafsson
Date:
> On 20 Feb 2024, at 12:27, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Tue, Feb 20, 2024 at 4:49 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>> I think there are several less weird ways to address this:
>>
>> * Just document it.
>>
>> * Make a pgcrypto-level GUC setting.
>>
>> * Split out these functions into a separate extension.
>>
>> * Deprecate these functions.
>>
>> Or some combination of these.
>
> I don't think the first two of these proposals help anything. AIUI,
> FIPS mode is supposed to be a system wide toggle that affects
> everything on the machine. The third one might help if you can be
> compliant by just choosing not to install that extension, and the
> fourth one solves the problem by sledgehammer.

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.

> Does Linux provide some way of asking whether "fips=1" was specified
> at kernel boot time?

There is a crypto.fips_enabled sysctl but I have no idea how portable that is
across distributions etc.

--
Daniel Gustafsson




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

From
Daniel Gustafsson
Date:
> On 20 Feb 2024, at 12:18, Peter Eisentraut <peter@eisentraut.org> wrote:

> I think we are going about this the wrong way.  It doesn't make sense to ask OpenSSL what a piece of code that
doesn'tuse OpenSSL should do. 

Given that pgcrypto cannot be built without OpenSSL, and ideally we should be
using the OpenSSL implementations for everything, I don't think it's too far
fetched.

--
Daniel Gustafsson




On Tue, Feb 20, 2024 at 5:09 PM Daniel Gustafsson <daniel@yesql.se> 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.

That generally seems fine, although I'm fuzzy on what our policy
actually is. We have fallback implementations for some things and not
others, IIRC.

> > Does Linux provide some way of asking whether "fips=1" was specified
> > at kernel boot time?
>
> There is a crypto.fips_enabled sysctl but I have no idea how portable that is
> across distributions etc.

My guess would be that it's pretty portable, but my guesses about
Linux might not be very good. Still, if we wanted to go this route, it
probably wouldn't be too hard to figure out how portable this is.

--
Robert Haas
EDB: http://www.enterprisedb.com



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

From
Peter Eisentraut
Date:
On 20.02.24 12:27, Robert Haas wrote:
> I don't think the first two of these proposals help anything. AIUI,
> FIPS mode is supposed to be a system wide toggle that affects
> everything on the machine. The third one might help if you can be
> compliant by just choosing not to install that extension, and the
> fourth one solves the problem by sledgehammer.
> 
> Does Linux provide some way of asking whether "fips=1" was specified
> at kernel boot time?

What you are describing only happens on Red Hat systems, I think.  They 
have built additional integration around this, which is great.  But 
that's not something you can rely on being the case on all systems, not 
even all Linux systems.



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

From
Daniel Gustafsson
Date:
> On 20 Feb 2024, at 13:24, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Tue, Feb 20, 2024 at 5:09 PM Daniel Gustafsson <daniel@yesql.se> 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.
>
> That generally seems fine, although I'm fuzzy on what our policy
> actually is. We have fallback implementations for some things and not
> others, IIRC.

I'm not sure there is a well-formed policy, but IIRC the idea with cryptohash
was to provide in-core functionality iff OpenSSL isn't used, and only use the
OpenSSL implementations if it is.  Since pgcrypto cannot be built without
OpenSSL (since db7d1a7b0530e8cbd045744e1c75b0e63fb6916f) I don't think it's a
problem to continue the work from that commit and replace more with OpenSSL
implementations.

--
Daniel Gustafsson




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

From
Peter Eisentraut
Date:
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.




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

From
Daniel Gustafsson
Date:
> On 20 Feb 2024, at 13:40, Peter Eisentraut <peter@eisentraut.org> wrote:
>
> 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
cryptolibrary underneath.  Effectively, the definition of what, say, blowfish crypt does, is whatever is in that source
file,and transitively, whatever OpenBSD does.   

I don't disagree, but if the OP is willing to take a stab at it then..

> (Fun question: Does OpenBSD care about FIPS?)

No, LibreSSL ripped out FIPS support early on.

>  Of course, you could reimplement the same algorithms independently, using OpenSSL or whatever.  But I don't think
thiswill really improve the state of the world in aggregate, because to a large degree we are relying on the upstream
tokeep these implementations maintained, and if we rewrite them, we become the upstream. 

As a sidenote, we are already trailing behind upstream on this, the patch in
[0] sits on my TODO, but given the lack of complaints over the years it's not
been bumped to the top.

--
Daniel Gustafsson

[0]
https://www.postgresql.org/message-id/flat/CAA-7PziyARoKi_9e2xdC75RJ068XPVk1CHDDdscu2BGrPuW9TQ%40mail.gmail.com#b20783dd6c72e95a8a0f6464d1228ed5




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

From
Daniel Gustafsson
Date:
> On 26 Oct 2024, at 20:10, Joe Conway <mail@joeconway.com> wrote:

> Rather than depend on figuring out if we are in FIPS_mode in a portable way, I think the GUC is simpler and
sufficient.Why not do that and just use a better name, e.g. legacy_crypto_enabled or something similar (bike-shedding
welcomed)as in the attached. 

I'm not very enthusiastic about adding a GUC to match a system property like
that for the same reason why we avoid GUCs with transitive dependencies.

Re-reading the thread and thinking about I think the best solution would be to
split these functions off into their own extension.

--
Daniel Gustafsson




On 10/29/24 05:57, Daniel Gustafsson wrote:
>> On 26 Oct 2024, at 20:10, Joe Conway <mail@joeconway.com> wrote:
> 
>> Rather than depend on figuring out if we are in FIPS_mode in a portable way, I think the GUC is simpler and
sufficient.Why not do that and just use a better name, e.g. legacy_crypto_enabled or something similar (bike-shedding
welcomed)as in the attached.
 
> 
> I'm not very enthusiastic about adding a GUC to match a system property like
> that for the same reason why we avoid GUCs with transitive dependencies.
> 
> Re-reading the thread and thinking about I think the best solution would be to
> split these functions off into their own extension.


Seems like that would be an issue for backward comparability and upgrades.

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



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

From
Daniel Gustafsson
Date:
> On 29 Oct 2024, at 13:53, Joe Conway <mail@joeconway.com> wrote:
>
> On 10/29/24 05:57, Daniel Gustafsson wrote:
>>> On 26 Oct 2024, at 20:10, Joe Conway <mail@joeconway.com> wrote:
>>> Rather than depend on figuring out if we are in FIPS_mode in a portable way, I think the GUC is simpler and
sufficient.Why not do that and just use a better name, e.g. legacy_crypto_enabled or something similar (bike-shedding
welcomed)as in the attached. 
>> I'm not very enthusiastic about adding a GUC to match a system property like
>> that for the same reason why we avoid GUCs with transitive dependencies.
>> Re-reading the thread and thinking about I think the best solution would be to
>> split these functions off into their own extension.
>
> Seems like that would be an issue for backward comparability and upgrades.

That's undoubtedly a downside of this proposal which the GUC proposal doesn't have.
--
Daniel Gustafsson




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

From
Jacob Champion
Date:
On Sat, Oct 26, 2024 at 11:10 AM Joe Conway <mail@joeconway.com> wrote:
> Rather than depend on figuring out if we are in FIPS_mode in a portable
> way, I think the GUC is simpler and sufficient. Why not do that and just
> use a better name, e.g. legacy_crypto_enabled or something similar
> (bike-shedding welcomed) as in the attached.

While it's definitely simpler, I read the original request as "forbid
non-FIPS crypto if the system tells us to", and I don't think this
proposal does that.

--Jacob



On 10/29/24 12:52, Jacob Champion wrote:
> On Sat, Oct 26, 2024 at 11:10 AM Joe Conway <mail@joeconway.com> wrote:
>> Rather than depend on figuring out if we are in FIPS_mode in a portable
>> way, I think the GUC is simpler and sufficient. Why not do that and just
>> use a better name, e.g. legacy_crypto_enabled or something similar
>> (bike-shedding welcomed) as in the attached.
> 
> While it's definitely simpler, I read the original request as "forbid
> non-FIPS crypto if the system tells us to", and I don't think this
> proposal does that.


No, you just have to be able to configure the system in such a way that 
the non-FIPs crypto is not available. This is entirely sufficient for that.


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



On 10/29/24 10:08, Daniel Gustafsson wrote:
>> On 29 Oct 2024, at 13:53, Joe Conway <mail@joeconway.com> wrote:
>> On 10/29/24 05:57, Daniel Gustafsson wrote:
>>>> On 26 Oct 2024, at 20:10, Joe Conway <mail@joeconway.com> wrote:

>>>> Rather than depend on figuring out if we are in FIPS_mode in a
>>>> portable way, I think the GUC is simpler and sufficient. Why
>>>> not do that and just use a better name, e.g.
>>>> legacy_crypto_enabled or something similar (bike-shedding
>>>> welcomed) as in the attached.

>>> I'm not very enthusiastic about adding a GUC to match a system property like
>>> that for the same reason why we avoid GUCs with transitive dependencies.
>>> Re-reading the thread and thinking about I think the best solution would be to
>>> split these functions off into their own extension.

>> Seems like that would be an issue for backward comparability and upgrades.

> That's undoubtedly a downside of this proposal which the GUC proposal doesn't have.

Any other opinions out there?

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



On Tue, Nov 19, 2024 at 12:30 PM Joe Conway <mail@joeconway.com> wrote:
> Any other opinions out there?

Why should we accept your patch (which adds a legacy_crypto_enabled
GUC) instead of adopting the approach originally proposed (i.e. use
the OpenSSL version of the functions)? It seems to me that the two
proposals accomplish the same thing, except that one of them also adds
a GUC.

--
Robert Haas
EDB: http://www.enterprisedb.com



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

From
Daniel Gustafsson
Date:
> On 19 Nov 2024, at 18:30, Joe Conway <mail@joeconway.com> wrote:

> Any other opinions out there?

Couldn't installations who would be satisfied with a GUC gate revoke privileges
from the relevant functions already today and achieve almost the same result?

--
Daniel Gustafsson




On 11/19/24 18:12, Robert Haas wrote:
> On Tue, Nov 19, 2024 at 12:30 PM Joe Conway <mail@joeconway.com> wrote:
>> Any other opinions out there?
> 
> Why should we accept your patch (which adds a legacy_crypto_enabled
> GUC) instead of adopting the approach originally proposed (i.e. use
> the OpenSSL version of the functions)?


Because that idea was rejected earlier in the thread by multiple people 
other than me?

   ¯\_(ツ)_/¯


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



On 11/20/24 12:14, Daniel Gustafsson wrote:
>> On 19 Nov 2024, at 18:30, Joe Conway <mail@joeconway.com> wrote:
> 
>> Any other opinions out there?
> 
> Couldn't installations who would be satisfied with a GUC gate revoke privileges
> from the relevant functions already today and achieve almost the same result?

I think that would qualify as a "mitigation" but not "FIPS compliant".

When the OS is made FIPS compliant, for example, you run something on 
the command line and then you need to reboot (RHEL at least). I believe 
that is considered configuration for FIPS.

A postmaster GUC (requiring restart) would be a way to configure 
Postgres to eliminate these two non-FIPS functions that could not be 
undone without another restart (similar to the OS example above).

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



On Thu, Nov 21, 2024 at 2:06 PM Joe Conway <mail@joeconway.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?

--
Robert Haas
EDB: http://www.enterprisedb.com



On 11/21/24 15:43, Robert Haas wrote:
> On Thu, Nov 21, 2024 at 2:06 PM Joe Conway <mail@joeconway.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



On Thu, Nov 21, 2024 at 4:39 PM Joe Conway <mail@joeconway.com> wrote:
> 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.

OK, I see the problem now. I don't interpret those messages as
opposing the idea of making this use OpenSSL, but they do say it would
be hard to implement, which is a problem.

--
Robert Haas
EDB: http://www.enterprisedb.com



On 11/22/24 09:11, Daniel Gustafsson wrote:
>> On 21 Nov 2024, at 22:39, Joe Conway <mail@joeconway.com> wrote:
> 
>> I mean, perhaps I am misreading and/or interpreting all of that differently to you, but from my reading of the
entirethread there was clearly no consensus to using openssl to provide those two functions.
 
> 
> My interpretation (or perhaps, my opinion) is that it would be ideal to
> reimplement these functions using OpenSSL *if possible* but the cost/benefit
> ratio is probably tilted such that it will never happen.
> 
>> [..] we don't drag this out past pg18 feature freeze
> 
> Agreed.
> 
>> If you have a better patch you would like to propose to fix this problem,
>> please do.
> 
> I'm still not thrilled about having a transitive dependency GUC, so attached is
> a (very lightly tested POC) version of your patch which expands it from boolean
> to enum with on/off/fips; the fips value being "disable if openssl is in fips
> mode, else enable".  I'm not sure if that's better, but at least it gives users
> a way to control the FIPS mode setting in one place and have crypto consumers
> follow the set value (or they can explicitly turn it off if they just want them
> disabled even without FIPS).

Works for me.

I do wonder if the GUC should be PGC_POSTMASTER (as I had suggested it 
ought to be in an earlier post) rather than PGC_SUSET (which was the way 
my posted patch had it). But perhaps PGC_SUSET is sufficient, and it 
makes testing easier.

One other question this spawned -- do we document the minimum supported 
version of OpenSSL anywhere? I remembered it had recently been 
increased, but could only find confirmation in the git logs that 1.1.1 
was now the minimum.

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



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

From
Daniel Gustafsson
Date:
> On 23 Nov 2024, at 17:13, Joe Conway <mail@joeconway.com> wrote:

> I do wonder if the GUC should be PGC_POSTMASTER (as I had suggested it ought to be in an earlier post) rather than
PGC_SUSET(which was the way my posted patch had it). But perhaps PGC_SUSET is sufficient, and it makes testing easier. 

I copied PGC_SUSET from your patch, since I think it seems sufficient for this.

> One other question this spawned -- do we document the minimum supported version of OpenSSL anywhere? I remembered it
hadrecently been increased, but could only find confirmation in the git logs that 1.1.1 was now the minimum. 

It's documented under installation requirements in the docs, where the 17 docs
currently state 1.0.2 as the minimum:

    https://www.postgresql.org/docs/devel/install-requirements.html

--
Daniel Gustafsson




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

From
Daniel Gustafsson
Date:
> On 24 Nov 2024, at 14:48, Daniel Gustafsson <daniel@yesql.se> wrote:

Any other opinions or should we proceed with the proposed GUC?

--
Daniel Gustafsson




On 12/3/24 08:59, Daniel Gustafsson wrote:
>> On 24 Nov 2024, at 14:48, Daniel Gustafsson <daniel@yesql.se> wrote:
> 
> Any other opinions or should we proceed with the proposed GUC?


I'm good with it. Did you want to commit or would you rather I do it?

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



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

From
Daniel Gustafsson
Date:
> On 3 Dec 2024, at 15:04, Joe Conway <mail@joeconway.com> wrote:
> 
> On 12/3/24 08:59, Daniel Gustafsson wrote:
>>> On 24 Nov 2024, at 14:48, Daniel Gustafsson <daniel@yesql.se> wrote:
>> Any other opinions or should we proceed with the proposed GUC?
> 
> I'm good with it. Did you want to commit or would you rather I do it?

No worries, I can make it happen.  ssnce there has been no objections since
this patch was posted I'll get it commmitted shortly.

--
Daniel Gustafsson




On Wed, Dec 4, 2024 at 8:54 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> Looking over this again I realized it's a bit silly to fall back on FIPS_mode()
> when EVP_default_properties_is_fips_enabled isn't available since that would
> only be OpenSSL versions before 3.0 (and since we don't support 1.0.2 then no
> such version can have FIPS).  Sharing back a v3 which is what I think we should
> go with.

The comment suggests to me that if the user happened to be using
OpenSSL 1.1.1 and CheckLegacyCryptoMode() was called, the expected
outcome would be an error, but it will just return.

Am I confused?

--
Robert Haas
EDB: http://www.enterprisedb.com



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

From
Daniel Gustafsson
Date:
> On 4 Dec 2024, at 15:28, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Dec 4, 2024 at 8:54 AM Daniel Gustafsson <daniel@yesql.se> wrote:
>> Looking over this again I realized it's a bit silly to fall back on FIPS_mode()
>> when EVP_default_properties_is_fips_enabled isn't available since that would
>> only be OpenSSL versions before 3.0 (and since we don't support 1.0.2 then no
>> such version can have FIPS).  Sharing back a v3 which is what I think we should
>> go with.
>
> The comment suggests to me that if the user happened to be using
> OpenSSL 1.1.1 and CheckLegacyCryptoMode() was called, the expected
> outcome would be an error, but it will just return.

I think I know what you mean, but just to be clear so I know what to reword,
the comment in the code or the above quoted email?

If the GUC is set to fips it will mimic the OpenSSL setting (disallow when
OpenSSL is in FIPS mode and allow when OpenSSL isn't in FIPS mode), and thus
allow internal crypto since OpenSSL 1.1.1 cannot operate in FIPS mode.  If the
GUC is set to on or off it will allow or disallow built-in crypto without
considering the OpenSSL state.

--
Daniel Gustafsson




On Wed, Dec 4, 2024 at 9:33 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> > The comment suggests to me that if the user happened to be using
> > OpenSSL 1.1.1 and CheckLegacyCryptoMode() was called, the expected
> > outcome would be an error, but it will just return.
>
> I think I know what you mean, but just to be clear so I know what to reword,
> the comment in the code or the above quoted email?
>
> If the GUC is set to fips it will mimic the OpenSSL setting (disallow when
> OpenSSL is in FIPS mode and allow when OpenSSL isn't in FIPS mode), and thus
> allow internal crypto since OpenSSL 1.1.1 cannot operate in FIPS mode.  If the
> GUC is set to on or off it will allow or disallow built-in crypto without
> considering the OpenSSL state.

Never mind, I was being stupid.

*wanders off to find a paper bag*

--
Robert Haas
EDB: http://www.enterprisedb.com



On 12/4/24 09:33, Daniel Gustafsson wrote:
> since OpenSSL 1.1.1 cannot operate in FIPS mode.

I don't think that is correct. The RHEL 8 openssl which was FIPS 140-2 
validated is 1.1.1k. See:


https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp4642.pdf

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



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

From
Daniel Gustafsson
Date:
> On 4 Dec 2024, at 15:40, Joe Conway <mail@joeconway.com> wrote:
>
> On 12/4/24 09:33, Daniel Gustafsson wrote:
>> since OpenSSL 1.1.1 cannot operate in FIPS mode.
>
> I don't think that is correct. The RHEL 8 openssl which was FIPS 140-2 validated is 1.1.1k. See:
>
>
https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp4642.pdf

Does RHEL publish the source of their fork somewhere?  In OpenSSL 1.1.1 the
code for FIPS_mode is:

int FIPS_mode(void)
{
    /* This version of the library does not support FIPS mode. */
    return 0;
}

Do you know if RHEL patched OpenSSL to allow FIPS_mode() to return other than 0
or if that function is useless regardless?

--
Daniel Gustafsson




On 12/4/24 09:45, Daniel Gustafsson wrote:
>> On 4 Dec 2024, at 15:40, Joe Conway <mail@joeconway.com> wrote:
>> 
>> On 12/4/24 09:33, Daniel Gustafsson wrote:
>>> since OpenSSL 1.1.1 cannot operate in FIPS mode.
>> 
>> I don't think that is correct. The RHEL 8 openssl which was FIPS 140-2 validated is 1.1.1k. See:
>> 
>>
https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp4642.pdf
> 
> Does RHEL publish the source of their fork somewhere?  In OpenSSL 1.1.1 the
> code for FIPS_mode is:
> 
> int FIPS_mode(void)
> {
>      /* This version of the library does not support FIPS mode. */
>      return 0;
> }
> 
> Do you know if RHEL patched OpenSSL to allow FIPS_mode() to return other than 0
> or if that function is useless regardless?

Yes the RHEL and OpenSUSE rpms for openssl are heavily patched for the 
FIPS versions, as is the Ubuntu one. It has been a while but last time I 
looked at all of this they were all using very similar patches to allow 
the "system wide" FIPS mode rather than depending on the app to 
explicitly go into FIPS_mode().

I can look for links, but investigating it involved (for example) 
installing the source rpm and then wading through hundreds of patches in 
the SOURCE directory.

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



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

From
Daniel Gustafsson
Date:
> On 4 Dec 2024, at 15:52, Joe Conway <mail@joeconway.com> wrote:
>
> On 12/4/24 09:45, Daniel Gustafsson wrote:
>>> On 4 Dec 2024, at 15:40, Joe Conway <mail@joeconway.com> wrote:
>>> On 12/4/24 09:33, Daniel Gustafsson wrote:
>>>> since OpenSSL 1.1.1 cannot operate in FIPS mode.
>>> I don't think that is correct. The RHEL 8 openssl which was FIPS 140-2 validated is 1.1.1k. See:
>>>
https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp4642.pdf
>> Does RHEL publish the source of their fork somewhere?  In OpenSSL 1.1.1 the
>> code for FIPS_mode is:
>> int FIPS_mode(void)
>> {
>>     /* This version of the library does not support FIPS mode. */
>>     return 0;
>> }
>> Do you know if RHEL patched OpenSSL to allow FIPS_mode() to return other than 0
>> or if that function is useless regardless?
>
> Yes the RHEL and OpenSUSE rpms for openssl are heavily patched for the FIPS versions, as is the Ubuntu one. It has
beena while but last time I looked at all of this they were all using very similar patches to allow the "system wide"
FIPSmode rather than depending on the app to explicitly go into FIPS_mode(). 
>
> I can look for links, but investigating it involved (for example) installing the source rpm and then wading through
hundredsof patches in the SOURCE directory. 

While I dislike not having a "follow-the-lib" setting on the GUC and rely on
the transitive dependency, maybe that's the only option if we can't reliably
detect the operating mode.  Sigh, as if OpenSSL wasn't messy enough even
without vendor patches =)

--
Daniel Gustafsson




On 12/4/24 10:01, Daniel Gustafsson wrote:
>> On 4 Dec 2024, at 15:52, Joe Conway <mail@joeconway.com> wrote:
>> 
>> On 12/4/24 09:45, Daniel Gustafsson wrote:
>>>> On 4 Dec 2024, at 15:40, Joe Conway <mail@joeconway.com> wrote:
>>>> On 12/4/24 09:33, Daniel Gustafsson wrote:
>>>>> since OpenSSL 1.1.1 cannot operate in FIPS mode.
>>>> I don't think that is correct. The RHEL 8 openssl which was FIPS 140-2 validated is 1.1.1k. See:
>>>>
https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp4642.pdf
>>> Does RHEL publish the source of their fork somewhere?  In OpenSSL 1.1.1 the
>>> code for FIPS_mode is:
>>> int FIPS_mode(void)
>>> {
>>>     /* This version of the library does not support FIPS mode. */
>>>     return 0;
>>> }
>>> Do you know if RHEL patched OpenSSL to allow FIPS_mode() to return other than 0
>>> or if that function is useless regardless?
>> 
>> Yes the RHEL and OpenSUSE rpms for openssl are heavily patched for the FIPS versions, as is the Ubuntu one. It has
beena while but last time I looked at all of this they were all using very similar patches to allow the "system wide"
FIPSmode rather than depending on the app to explicitly go into FIPS_mode().
 
>> 
>> I can look for links, but investigating it involved (for example) installing the source rpm and then wading through
hundredsof patches in the SOURCE directory.
 

I can send you the source RPM for openssl 1.1.1c which was an earlier 
FIPS validated version, but the main FIPS patch contains:

8<-------------
diff -up openssl-1.1.1b/crypto/o_fips.c.fips openssl-1.1.1b/crypto/o_fips.c
--- openssl-1.1.1b/crypto/o_fips.c.fips    2019-02-26 15:15:30.000000000 +0100
+++ openssl-1.1.1b/crypto/o_fips.c    2019-02-28 11:30:06.817745466 +0100
@@ -8,17 +8,28 @@
   */

  #include "internal/cryptlib.h"
+#include "internal/fips_int.h"

  int FIPS_mode(void)
  {
+#ifdef OPENSSL_FIPS
+    return FIPS_module_mode();
+#else
      /* This version of the library does not support FIPS mode. */
      return 0;
+#endif
  }
8<-------------

> While I dislike not having a "follow-the-lib" setting on the GUC and rely on
> the transitive dependency, maybe that's the only option if we can't reliably
> detect the operating mode.  Sigh, as if OpenSSL wasn't messy enough even
> without vendor patches =)

Yep, that was my concern. I believe the RHEL, OpenSUSE, and Ubuntu 
solutions are very similar, but they may be different enough that it 
will be painful to reliably detect FIPS_mode. The RHEL and SUSE source 
RPMs were findable. I think to get the Ubuntu FIPS deb I had to pay for 
a subscription. But as I said it has been a while so I don't remember 
exactly (like 6+ years).

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



On 12/4/24 10:57, Joe Conway wrote:
> On 12/4/24 10:01, Daniel Gustafsson wrote:
>>> On 4 Dec 2024, at 15:52, Joe Conway <mail@joeconway.com> wrote:
>>> 
>>> On 12/4/24 09:45, Daniel Gustafsson wrote:
>>>>> On 4 Dec 2024, at 15:40, Joe Conway <mail@joeconway.com> wrote:
>>>>> On 12/4/24 09:33, Daniel Gustafsson wrote:
>>>>>> since OpenSSL 1.1.1 cannot operate in FIPS mode.
>>>>> I don't think that is correct. The RHEL 8 openssl which was FIPS 140-2 validated is 1.1.1k. See:
>>>>>
https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp4642.pdf
>>>> Does RHEL publish the source of their fork somewhere?  In OpenSSL 1.1.1 the
>>>> code for FIPS_mode is:
>>>> int FIPS_mode(void)
>>>> {
>>>>     /* This version of the library does not support FIPS mode. */
>>>>     return 0;
>>>> }
>>>> Do you know if RHEL patched OpenSSL to allow FIPS_mode() to return other than 0
>>>> or if that function is useless regardless?
>>> 
>>> Yes the RHEL and OpenSUSE rpms for openssl are heavily patched for the FIPS versions, as is the Ubuntu one. It has
beena while but last time I looked at all of this they were all using very similar patches to allow the "system wide"
FIPSmode rather than depending on the app to explicitly go into FIPS_mode().
 
>>> 
>>> I can look for links, but investigating it involved (for example) installing the source rpm and then wading through
hundredsof patches in the SOURCE directory.
 
> 
> I can send you the source RPM for openssl 1.1.1c which was an earlier
> FIPS validated version, but the main FIPS patch contains:
> 
> 8<-------------
> diff -up openssl-1.1.1b/crypto/o_fips.c.fips openssl-1.1.1b/crypto/o_fips.c
> --- openssl-1.1.1b/crypto/o_fips.c.fips    2019-02-26 15:15:30.000000000 +0100
> +++ openssl-1.1.1b/crypto/o_fips.c    2019-02-28 11:30:06.817745466 +0100
> @@ -8,17 +8,28 @@
>     */
> 
>    #include "internal/cryptlib.h"
> +#include "internal/fips_int.h"
> 
>    int FIPS_mode(void)
>    {
> +#ifdef OPENSSL_FIPS
> +    return FIPS_module_mode();
> +#else
>        /* This version of the library does not support FIPS mode. */
>        return 0;
> +#endif
>    }
> 8<-------------

FWIW, here is a link to a 1.1.1k source RPM:
  https://yum.oracle.com/repo/OracleLinux/OL8/baseos/latest/x86_64/getPackageSource/openssl-1.1.1k-4.el8.src.rpm

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



On 12/9/24 07:23, Daniel Gustafsson wrote:
>> On 4 Dec 2024, at 16:57, Joe Conway <mail@joeconway.com> wrote:
> 
>> I can send you the source RPM for openssl 1.1.1c which was an earlier FIPS validated version, but the main FIPS
patchcontains:
 
> 
> AFAICT the forks of 1.1.1 which offer FIPS certification all patch the common
> OpenSSL API FIPS_mode() rather than invent a new one, so the earlier approach
> should work fine. PFA an updated version which I propose we go ahead with.

That sounds correct from my memory of it.

I have not done any actual testing (yet), but on quick scan this part 
looks suspicious:
8<-------------------
+_PG_init(void)
+{
+    DefineCustomEnumVariable("pgcrypto.legacy_crypto_enabled",
+                             "Sets if builtin crypto functions are enabled.",
+                             "Yes enables builtin crypto, No unconditionally disables and 
OpenSSL "
+                             "will disable if OpenSSL is in FIPS mode",
+                             &legacy_crypto_enabled,
8<-------------------

Rather than:
  "Yes enables builtin crypto, No unconditionally disables and OpenSSL "
                                                               ^^^^^^^
  "will disable if OpenSSL is in FIPS mode"

I think that should say:
  "Yes enables builtin crypto, No unconditionally disables and fips "
                                                               ^^^^
  "will disable if OpenSSL is in FIPS mode"

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



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

From
Peter Eisentraut
Date:
On 09.12.24 22:37, Daniel Gustafsson wrote:
>> On 9 Dec 2024, at 15:11, Joe Conway <mail@joeconway.com> wrote:
>>
>> On 12/9/24 07:23, Daniel Gustafsson wrote:
>>>> On 4 Dec 2024, at 16:57, Joe Conway <mail@joeconway.com> wrote:
>>>> I can send you the source RPM for openssl 1.1.1c which was an earlier FIPS validated version, but the main FIPS
patchcontains:
 
>>> AFAICT the forks of 1.1.1 which offer FIPS certification all patch the common
>>> OpenSSL API FIPS_mode() rather than invent a new one, so the earlier approach
>>> should work fine. PFA an updated version which I propose we go ahead with.
>>
>> That sounds correct from my memory of it.
>>
>> I have not done any actual testing (yet), but on quick scan this part looks suspicious:
> 
> Not only suspicious but plain wrong, fixed in the attached, thanks!

I think these function names are wrong:

+      <varname>pgcrypto.legacy_crypto_enabled</varname> determines if the
+      built in legacy crypto functions <literal>pg_gen_salt</literal>,
+      <literal>pg_gen_salt_rounds</literal>, and 
<literal>pg_crypt</literal>
+      are available for use.

Those are the C-level functions.  The SQL-level functions are called 
gen_salt and crypt.