Replace current implementations in crypt() and gen_salt() to OpenSSL
Hi
This is Shibagaki.
When FIPS mode is enabled, some encryption algorithms cannot be used.
Since PostgreSQL15, pgcrypto requires OpenSSL[1]https://github.com/postgres/postgres/commit/db7d1a7b0530e8cbd045744e1c75b0e63fb6916f, digest() and other functions
also follow this policy.
However, crypt() and gen_salt() do not use OpenSSL as mentioned in [2]https://peter.eisentraut.org/blog/2023/12/05/postgresql-and-fips-mode.
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 normal behavior
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
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.
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
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
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
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.
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-compliant either, 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 would then continue to work.
That's a fair point, we can check individual ciphers. I'll hack up a version
doing this.
--
Daniel Gustafsson
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-compliant either, 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 would then 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.
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
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
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
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
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
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't use 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
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.
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
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.
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 crypto library 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 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.
As a sidenote, we are already trailing behind upstream on this, the patch in
[0]: /messages/by-id/CAA-7PziyARoKi_9e2xdC75RJ068XPVk1CHDDdscu2BGrPuW9TQ@mail.gmail.com
been bumped to the top.
--
Daniel Gustafsson
[0]: /messages/by-id/CAA-7PziyARoKi_9e2xdC75RJ068XPVk1CHDDdscu2BGrPuW9TQ@mail.gmail.com
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 crypto library
underneath. Effectively, the definition of what, say, blowfish
crypt does, is whatever is in that source file, and transitively,
whatever OpenBSD does.
Someone asked me about this issue a few days ago which led me back to
this unresolved thread.
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.
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.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-pgcrypto.patchtext/x-patch; charset=UTF-8; name=v1-0001-pgcrypto.patchDownload
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd..c754ed7 100644
*** a/contrib/pgcrypto/expected/crypt-des.out
--- b/contrib/pgcrypto/expected/crypt-des.out
*************** FROM ctest;
*** 28,31 ****
--- 28,38 ----
t
(1 row)
+ -- check disabling of legacy crypto functions
+ SET pgcrypto.legacy_crypto_enabled = false;
+ UPDATE ctest SET salt = gen_salt('des');
+ ERROR: legacy crypto functions not enabled
+ UPDATE ctest SET res = crypt(data, salt);
+ ERROR: legacy crypto functions not enabled
+ RESET pgcrypto.legacy_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index 96447c5..a3e41ba 100644
*** a/contrib/pgcrypto/pgcrypto.c
--- b/contrib/pgcrypto/pgcrypto.c
***************
*** 38,43 ****
--- 38,44 ----
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+ #include "utils/guc.h"
#include "utils/uuid.h"
#include "varatt.h"
*************** typedef int (*PFN) (const char *name, vo
*** 49,54 ****
--- 50,70 ----
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+ /* exported vars */
+ bool legacy_crypto_enabled = true;
+
+ /*
+ * Entrypoint of this module.
+ */
+ void
+ _PG_init(void)
+ {
+ DefineCustomBoolVariable("pgcrypto.legacy_crypto_enabled",
+ "True if legacy builtin crypto functions are enabled",
+ NULL, &legacy_crypto_enabled, true, PGC_SUSET,
+ 0, NULL, NULL, NULL);
+ }
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2..92fe167 100644
*** a/contrib/pgcrypto/px-crypt.c
--- b/contrib/pgcrypto/px-crypt.c
***************
*** 34,39 ****
--- 34,41 ----
#include "px-crypt.h"
#include "px.h"
+ extern bool legacy_crypto_enabled;
+
static char *
run_crypt_des(const char *psw, const char *salt,
char *buf, unsigned len)
*************** px_crypt(const char *psw, const char *sa
*** 91,96 ****
--- 93,102 ----
{
const struct px_crypt_algo *c;
+ if (!legacy_crypto_enabled)
+ ereport(ERROR,
+ (errmsg("legacy crypto functions not enabled")));
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
*************** px_gen_salt(const char *salt_type, char
*** 135,140 ****
--- 141,150 ----
char *p;
char rbuf[16];
+ if (!legacy_crypto_enabled)
+ ereport(ERROR,
+ (errmsg("legacy crypto functions not enabled")));
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e..dea3276 100644
*** a/contrib/pgcrypto/sql/crypt-des.sql
--- b/contrib/pgcrypto/sql/crypt-des.sql
*************** UPDATE ctest SET res = crypt(data, salt)
*** 18,21 ****
--- 18,27 ----
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+ -- check disabling of legacy crypto functions
+ SET pgcrypto.legacy_crypto_enabled = false;
+ UPDATE ctest SET salt = gen_salt('des');
+ UPDATE ctest SET res = crypt(data, salt);
+ RESET pgcrypto.legacy_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f..ba133d6 100644
*** a/doc/src/sgml/pgcrypto.sgml
--- b/doc/src/sgml/pgcrypto.sgml
*************** gen_random_uuid() returns uuid
*** 1222,1227 ****
--- 1222,1264 ----
</sect3>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-legacy_crypto_enabled">
+ <term>
+ <varname>pgcrypto.legacy_crypto_enabled</varname> (<type>bool</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.legacy_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <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. Setting this to <literal>false</literal>
+ disables these functions. <literal>true</literal> (the default) enables
+ these functions to work normally.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, these parameters are set
+ in <filename>postgresql.conf</filename>, although superusers can alter them
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-author">
<title>Author</title>
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
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
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
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 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 entire thread 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).
--
Daniel Gustafsson
Attachments:
v2-0001-Make-it-possible-to-disable-built-in-crypto.patchapplication/octet-stream; name=v2-0001-Make-it-possible-to-disable-built-in-crypto.patch; x-unix-mode=0644Download
From 243326cc90d7824ac099b2d7b4c607d158cb76c9 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Fri, 22 Nov 2024 14:49:31 +0100
Subject: [PATCH v2] Make it possible to disable built-in crypto
---
contrib/pgcrypto/expected/crypt-des.out | 7 +++++
contrib/pgcrypto/openssl.c | 29 ++++++++++++++++++
contrib/pgcrypto/pgcrypto.c | 30 +++++++++++++++++++
contrib/pgcrypto/px-crypt.c | 4 +++
contrib/pgcrypto/px.h | 10 +++++++
contrib/pgcrypto/sql/crypt-des.sql | 6 ++++
doc/src/sgml/pgcrypto.sgml | 39 +++++++++++++++++++++++++
7 files changed, 125 insertions(+)
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd580a..8993a2d1044 100644
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -28,4 +28,11 @@ FROM ctest;
t
(1 row)
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.legacy_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 26454bc3e29..c29940f3d06 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -794,3 +794,32 @@ ResOwnerReleaseOSSLCipher(Datum res)
{
free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
}
+
+/*
+ * CheckLegacyCryptoMode
+ *
+ * Function for erroring out in case built-in crypto is executed when the user
+ * has disabled it. If legacy_crypto_enabled is set to LGC_OFF or LGC_FIPS and
+ * OpenSSL is operating in FIPS mode the function will error out, else the
+ * query executing built-in crypto can proceed.
+ */
+void
+CheckLegacyCryptoMode(void)
+{
+ if (legacy_crypto_enabled == LGC_ON)
+ return;
+
+ if (legacy_crypto_enabled == LGC_OFF)
+ ereport(ERROR,
+ errmsg("use of built-in crypto functions is disabled"));
+
+ Assert(legacy_crypto_enabled == LGC_FIPS);
+
+#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("use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode"));
+}
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ebd76eed702..fe0e2b9242a 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -38,16 +38,46 @@
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+#include "utils/guc.h"
#include "varatt.h"
PG_MODULE_MAGIC;
/* private stuff */
+static const struct config_enum_entry legacy_crypto_options[] = {
+ {"on", LGC_ON, false},
+ {"off", LGC_OFF, false},
+ {"fips", LGC_FIPS, false},
+ {NULL, 0, false}
+};
+
typedef int (*PFN) (const char *name, void **res);
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+int legacy_crypto_enabled = LGC_ON;
+
+/*
+ * Entrypoint of this module.
+ */
+void
+_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,
+ LGC_ON,
+ legacy_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+}
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2c1bc..b97ea4fd36b 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -91,6 +91,8 @@ px_crypt(const char *psw, const char *salt, char *buf, unsigned len)
{
const struct px_crypt_algo *c;
+ CheckLegacyCryptoMode();
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
@@ -135,6 +137,8 @@ px_gen_salt(const char *salt_type, char *buf, int rounds)
char *p;
char rbuf[16];
+ CheckLegacyCryptoMode();
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 471bb4ec727..8f21dad54ec 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -89,6 +89,12 @@
#define PXE_PGP_UNSUPPORTED_PUBALGO -122
#define PXE_PGP_MULTIPLE_SUBKEYS -123
+typedef enum LegacyCryptoOptions
+{
+ LGC_ON,
+ LGC_OFF,
+ LGC_FIPS,
+} LegacyCryptoOptions;
typedef struct px_digest PX_MD;
typedef struct px_alias PX_Alias;
@@ -96,6 +102,8 @@ typedef struct px_hmac PX_HMAC;
typedef struct px_cipher PX_Cipher;
typedef struct px_combo PX_Combo;
+extern int legacy_crypto_enabled;
+
struct px_digest
{
unsigned (*result_size) (PX_MD *h);
@@ -182,6 +190,8 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
+void CheckLegacyCryptoMode(void);
+
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
#else
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e6555..5b75dc5bb49 100644
--- a/contrib/pgcrypto/sql/crypt-des.sql
+++ b/contrib/pgcrypto/sql/crypt-des.sql
@@ -18,4 +18,10 @@ UPDATE ctest SET res = crypt(data, salt);
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+UPDATE ctest SET res = crypt(data, salt);
+RESET pgcrypto.legacy_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f0cde..c54b0e8459f 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1222,6 +1222,45 @@ gen_random_uuid() returns uuid
</sect3>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-legacy_crypto_enabled">
+ <term>
+ <varname>pgcrypto.legacy_crypto_enabled</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.legacy_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <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. Setting this to <literal>off</literal>
+ disables these functions. <literal>on</literal> (the default) enables
+ these functions to work normally. <literal>fips</literal> disables these
+ functions if <application>OpenSSL</application> is detected to operate
+ in FIPS mode.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, these parameters are set
+ in <filename>postgresql.conf</filename>, although superusers can alter them
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-author">
<title>Author</title>
--
2.39.3 (Apple Git-146)
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 entire thread 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
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 had recently 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
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
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 3 Dec 2024, at 22:56, Daniel Gustafsson <daniel@yesql.se> wrote:
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.
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.
--
Daniel Gustafsson
Attachments:
v3-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patchapplication/octet-stream; name=v3-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patch; x-unix-mode=0644Download
From 79f88c79bf1217915fb4417a67c4e370fd08c002 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Fri, 22 Nov 2024 14:49:31 +0100
Subject: [PATCH v3] pgcrypto: Make it possible to disable built-in crypto
When using OpenSSL and/or the underlying operating system in FIPS
mode no non-FIPS certified crypto implementations should be used.
While that is already possible by just not invoking the built-in
crypto in pgcrypto, this adds a GUC which prohibit the code from
being called. This doesn't change the FIPS status of PostgreSQL
but can make it easier for sites which target FIPS compliance to
ensure that violations cannot occur.
Reviewed-by: Joe Conway <mail@joeconway.com>
Discussion: https://postgr.es/m/TYCPR01MB11684E5B636E17548D4A42248FA4D2@TYCPR01MB11684.jpnprd01.prod.outlook.com
---
contrib/pgcrypto/expected/crypt-des.out | 7 +++++
contrib/pgcrypto/openssl.c | 34 +++++++++++++++++++++
contrib/pgcrypto/pgcrypto.c | 30 +++++++++++++++++++
contrib/pgcrypto/px-crypt.c | 4 +++
contrib/pgcrypto/px.h | 10 +++++++
contrib/pgcrypto/sql/crypt-des.sql | 6 ++++
doc/src/sgml/pgcrypto.sgml | 39 +++++++++++++++++++++++++
7 files changed, 130 insertions(+)
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd580a..8993a2d1044 100644
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -28,4 +28,11 @@ FROM ctest;
t
(1 row)
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.legacy_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 448db331a0f..c6990177945 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -794,3 +794,37 @@ ResOwnerReleaseOSSLCipher(Datum res)
{
free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
}
+
+/*
+ * CheckLegacyCryptoMode
+ *
+ * Function for erroring out in case built-in crypto is executed when the user
+ * has disabled it. If legacy_crypto_enabled is set to LGC_OFF or LGC_FIPS and
+ * OpenSSL is operating in FIPS mode the function will error out, else the
+ * query executing built-in crypto can proceed.
+ */
+void
+CheckLegacyCryptoMode(void)
+{
+ if (legacy_crypto_enabled == LGC_ON)
+ return;
+
+ if (legacy_crypto_enabled == LGC_OFF)
+ ereport(ERROR,
+ errmsg("use of built-in crypto functions is disabled"));
+
+ Assert(legacy_crypto_enabled == LGC_FIPS);
+
+#if defined EVP_default_properties_is_fips_enabled
+ /*
+ * EVP_default_properties_is_fips_enabled was aded in OpenSSL 3.0, before
+ * that FIPS_mode() was used to test for FIPS being enabled. The last
+ * version before 3.0 which supported FIPS was 1.0.2 and since we require
+ * 1.1.1 or newer there is no need test with FIPS_mode() at all since it
+ * is guaranteed to always return false.
+ */
+ if (EVP_default_properties_is_fips_enabled(NULL);
+ ereport(ERROR,
+ errmsg("use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode"));
+#endif
+}
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ebd76eed702..fe0e2b9242a 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -38,16 +38,46 @@
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+#include "utils/guc.h"
#include "varatt.h"
PG_MODULE_MAGIC;
/* private stuff */
+static const struct config_enum_entry legacy_crypto_options[] = {
+ {"on", LGC_ON, false},
+ {"off", LGC_OFF, false},
+ {"fips", LGC_FIPS, false},
+ {NULL, 0, false}
+};
+
typedef int (*PFN) (const char *name, void **res);
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+int legacy_crypto_enabled = LGC_ON;
+
+/*
+ * Entrypoint of this module.
+ */
+void
+_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,
+ LGC_ON,
+ legacy_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+}
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2c1bc..b97ea4fd36b 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -91,6 +91,8 @@ px_crypt(const char *psw, const char *salt, char *buf, unsigned len)
{
const struct px_crypt_algo *c;
+ CheckLegacyCryptoMode();
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
@@ -135,6 +137,8 @@ px_gen_salt(const char *salt_type, char *buf, int rounds)
char *p;
char rbuf[16];
+ CheckLegacyCryptoMode();
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 471bb4ec727..8f21dad54ec 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -89,6 +89,12 @@
#define PXE_PGP_UNSUPPORTED_PUBALGO -122
#define PXE_PGP_MULTIPLE_SUBKEYS -123
+typedef enum LegacyCryptoOptions
+{
+ LGC_ON,
+ LGC_OFF,
+ LGC_FIPS,
+} LegacyCryptoOptions;
typedef struct px_digest PX_MD;
typedef struct px_alias PX_Alias;
@@ -96,6 +102,8 @@ typedef struct px_hmac PX_HMAC;
typedef struct px_cipher PX_Cipher;
typedef struct px_combo PX_Combo;
+extern int legacy_crypto_enabled;
+
struct px_digest
{
unsigned (*result_size) (PX_MD *h);
@@ -182,6 +190,8 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
+void CheckLegacyCryptoMode(void);
+
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
#else
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e6555..5b75dc5bb49 100644
--- a/contrib/pgcrypto/sql/crypt-des.sql
+++ b/contrib/pgcrypto/sql/crypt-des.sql
@@ -18,4 +18,10 @@ UPDATE ctest SET res = crypt(data, salt);
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+UPDATE ctest SET res = crypt(data, salt);
+RESET pgcrypto.legacy_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f0cde..c54b0e8459f 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1222,6 +1222,45 @@ gen_random_uuid() returns uuid
</sect3>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-legacy_crypto_enabled">
+ <term>
+ <varname>pgcrypto.legacy_crypto_enabled</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.legacy_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <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. Setting this to <literal>off</literal>
+ disables these functions. <literal>on</literal> (the default) enables
+ these functions to work normally. <literal>fips</literal> disables these
+ functions if <application>OpenSSL</application> is detected to operate
+ in FIPS mode.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, these parameters are set
+ in <filename>postgresql.conf</filename>, although superusers can alter them
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-author">
<title>Author</title>
--
2.39.3 (Apple Git-146)
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
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:
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
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:
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:
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
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.pdfDoes 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.
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.pdfDoes 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.
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.pdfDoes 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.
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 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 patch contains:
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.
--
Daniel Gustafsson
Attachments:
v4-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patchapplication/octet-stream; name=v4-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patch; x-unix-mode=0644Download
From 3fb17a2ff4298e52128ea80a677c1b7afda0400b Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Fri, 22 Nov 2024 14:49:31 +0100
Subject: [PATCH v4] pgcrypto: Make it possible to disable built-in crypto
When using OpenSSL and/or the underlying operating system in FIPS
mode no non-FIPS certified crypto implementations should be used.
While that is already possible by just not invoking the built-in
crypto in pgcrypto, this adds a GUC which prohibit the code from
being called. This doesn't change the FIPS status of PostgreSQL
but can make it easier for sites which target FIPS compliance to
ensure that violations cannot occur.
Reviewed-by: Joe Conway <mail@joeconway.com>
Discussion: https://postgr.es/m/TYCPR01MB11684E5B636E17548D4A42248FA4D2@TYCPR01MB11684.jpnprd01.prod.outlook.com
---
contrib/pgcrypto/expected/crypt-des.out | 7 ++++
contrib/pgcrypto/openssl.c | 43 +++++++++++++++++++++++++
contrib/pgcrypto/pgcrypto.c | 30 +++++++++++++++++
contrib/pgcrypto/px-crypt.c | 4 +++
contrib/pgcrypto/px.h | 10 ++++++
contrib/pgcrypto/sql/crypt-des.sql | 6 ++++
doc/src/sgml/pgcrypto.sgml | 39 ++++++++++++++++++++++
7 files changed, 139 insertions(+)
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd580a..8993a2d1044 100644
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -28,4 +28,11 @@ FROM ctest;
t
(1 row)
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.legacy_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 448db331a0f..c5683aa70e7 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -794,3 +794,46 @@ ResOwnerReleaseOSSLCipher(Datum res)
{
free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
}
+
+/*
+ * CheckLegacyCryptoMode
+ *
+ * Function for erroring out in case built-in crypto is executed when the user
+ * has disabled it. If legacy_crypto_enabled is set to LGC_OFF or LGC_FIPS and
+ * OpenSSL is operating in FIPS mode the function will error out, else the
+ * query executing built-in crypto can proceed.
+ */
+void
+CheckLegacyCryptoMode(void)
+{
+ int fips_enabled;
+
+ if (legacy_crypto_enabled == LGC_ON)
+ return;
+
+ if (legacy_crypto_enabled == LGC_OFF)
+ ereport(ERROR,
+ errmsg("use of built-in crypto functions is disabled"));
+
+ Assert(legacy_crypto_enabled == LGC_FIPS);
+
+ /*
+ * EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0, before
+ * that FIPS_mode() was used to test for FIPS being enabled. The last
+ * upstream OpenSSL version before 3.0 which supported FIPS was 1.0.2, but
+ * there are forks of 1.1.1 which are FIPS certified so we still need to
+ * test with FIPS_mode() even though we don't support 1.0.2.
+ */
+ fips_enabled =
+#if defined(EVP_default_properties_is_fips_enabled)
+ EVP_default_properties_is_fips_enabled(NULL);
+#elif defined(FIPS_mode)
+ FIPS_mode();
+#else
+ 0;
+#endif
+
+ if (fips_enabled)
+ ereport(ERROR,
+ errmsg("use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode"));
+}
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ebd76eed702..fe0e2b9242a 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -38,16 +38,46 @@
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+#include "utils/guc.h"
#include "varatt.h"
PG_MODULE_MAGIC;
/* private stuff */
+static const struct config_enum_entry legacy_crypto_options[] = {
+ {"on", LGC_ON, false},
+ {"off", LGC_OFF, false},
+ {"fips", LGC_FIPS, false},
+ {NULL, 0, false}
+};
+
typedef int (*PFN) (const char *name, void **res);
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+int legacy_crypto_enabled = LGC_ON;
+
+/*
+ * Entrypoint of this module.
+ */
+void
+_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,
+ LGC_ON,
+ legacy_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+}
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2c1bc..b97ea4fd36b 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -91,6 +91,8 @@ px_crypt(const char *psw, const char *salt, char *buf, unsigned len)
{
const struct px_crypt_algo *c;
+ CheckLegacyCryptoMode();
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
@@ -135,6 +137,8 @@ px_gen_salt(const char *salt_type, char *buf, int rounds)
char *p;
char rbuf[16];
+ CheckLegacyCryptoMode();
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 471bb4ec727..8f21dad54ec 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -89,6 +89,12 @@
#define PXE_PGP_UNSUPPORTED_PUBALGO -122
#define PXE_PGP_MULTIPLE_SUBKEYS -123
+typedef enum LegacyCryptoOptions
+{
+ LGC_ON,
+ LGC_OFF,
+ LGC_FIPS,
+} LegacyCryptoOptions;
typedef struct px_digest PX_MD;
typedef struct px_alias PX_Alias;
@@ -96,6 +102,8 @@ typedef struct px_hmac PX_HMAC;
typedef struct px_cipher PX_Cipher;
typedef struct px_combo PX_Combo;
+extern int legacy_crypto_enabled;
+
struct px_digest
{
unsigned (*result_size) (PX_MD *h);
@@ -182,6 +190,8 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
+void CheckLegacyCryptoMode(void);
+
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
#else
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e6555..5b75dc5bb49 100644
--- a/contrib/pgcrypto/sql/crypt-des.sql
+++ b/contrib/pgcrypto/sql/crypt-des.sql
@@ -18,4 +18,10 @@ UPDATE ctest SET res = crypt(data, salt);
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+UPDATE ctest SET res = crypt(data, salt);
+RESET pgcrypto.legacy_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f0cde..c54b0e8459f 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1222,6 +1222,45 @@ gen_random_uuid() returns uuid
</sect3>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-legacy_crypto_enabled">
+ <term>
+ <varname>pgcrypto.legacy_crypto_enabled</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.legacy_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <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. Setting this to <literal>off</literal>
+ disables these functions. <literal>on</literal> (the default) enables
+ these functions to work normally. <literal>fips</literal> disables these
+ functions if <application>OpenSSL</application> is detected to operate
+ in FIPS mode.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, these parameters are set
+ in <filename>postgresql.conf</filename>, although superusers can alter them
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-author">
<title>Author</title>
--
2.39.3 (Apple Git-146)
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 patch contains:
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
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 patch contains: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!
--
Daniel Gustafsson
Attachments:
v5-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patchapplication/octet-stream; name=v5-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patch; x-unix-mode=0644Download
From e0170539edf31744de7a7f42540914ff910e8ed5 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Fri, 22 Nov 2024 14:49:31 +0100
Subject: [PATCH v5] pgcrypto: Make it possible to disable built-in crypto
When using OpenSSL and/or the underlying operating system in FIPS
mode no non-FIPS certified crypto implementations should be used.
While that is already possible by just not invoking the built-in
crypto in pgcrypto, this adds a GUC which prohibit the code from
being called. This doesn't change the FIPS status of PostgreSQL
but can make it easier for sites which target FIPS compliance to
ensure that violations cannot occur.
Reviewed-by: Joe Conway <mail@joeconway.com>
Discussion: https://postgr.es/m/TYCPR01MB11684E5B636E17548D4A42248FA4D2@TYCPR01MB11684.jpnprd01.prod.outlook.com
---
contrib/pgcrypto/expected/crypt-des.out | 7 ++++
contrib/pgcrypto/openssl.c | 43 +++++++++++++++++++++++++
contrib/pgcrypto/pgcrypto.c | 30 +++++++++++++++++
contrib/pgcrypto/px-crypt.c | 4 +++
contrib/pgcrypto/px.h | 10 ++++++
contrib/pgcrypto/sql/crypt-des.sql | 6 ++++
doc/src/sgml/pgcrypto.sgml | 39 ++++++++++++++++++++++
7 files changed, 139 insertions(+)
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd580..8993a2d104 100644
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -28,4 +28,11 @@ FROM ctest;
t
(1 row)
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.legacy_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 448db331a0..c5683aa70e 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -794,3 +794,46 @@ ResOwnerReleaseOSSLCipher(Datum res)
{
free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
}
+
+/*
+ * CheckLegacyCryptoMode
+ *
+ * Function for erroring out in case built-in crypto is executed when the user
+ * has disabled it. If legacy_crypto_enabled is set to LGC_OFF or LGC_FIPS and
+ * OpenSSL is operating in FIPS mode the function will error out, else the
+ * query executing built-in crypto can proceed.
+ */
+void
+CheckLegacyCryptoMode(void)
+{
+ int fips_enabled;
+
+ if (legacy_crypto_enabled == LGC_ON)
+ return;
+
+ if (legacy_crypto_enabled == LGC_OFF)
+ ereport(ERROR,
+ errmsg("use of built-in crypto functions is disabled"));
+
+ Assert(legacy_crypto_enabled == LGC_FIPS);
+
+ /*
+ * EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0, before
+ * that FIPS_mode() was used to test for FIPS being enabled. The last
+ * upstream OpenSSL version before 3.0 which supported FIPS was 1.0.2, but
+ * there are forks of 1.1.1 which are FIPS certified so we still need to
+ * test with FIPS_mode() even though we don't support 1.0.2.
+ */
+ fips_enabled =
+#if defined(EVP_default_properties_is_fips_enabled)
+ EVP_default_properties_is_fips_enabled(NULL);
+#elif defined(FIPS_mode)
+ FIPS_mode();
+#else
+ 0;
+#endif
+
+ if (fips_enabled)
+ ereport(ERROR,
+ errmsg("use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode"));
+}
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ebd76eed70..e3d34d3a10 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -38,16 +38,46 @@
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+#include "utils/guc.h"
#include "varatt.h"
PG_MODULE_MAGIC;
/* private stuff */
+static const struct config_enum_entry legacy_crypto_options[] = {
+ {"on", LGC_ON, false},
+ {"off", LGC_OFF, false},
+ {"fips", LGC_FIPS, false},
+ {NULL, 0, false}
+};
+
typedef int (*PFN) (const char *name, void **res);
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+int legacy_crypto_enabled = LGC_ON;
+
+/*
+ * Entrypoint of this module.
+ */
+void
+_PG_init(void)
+{
+ DefineCustomEnumVariable("pgcrypto.legacy_crypto_enabled",
+ "Sets if builtin crypto functions are enabled.",
+ "\"on\" enables builtin crypto, \"off\" unconditionally disables and \"fips\" "
+ "will disable builtin crypto if OpenSSL is in FIPS mode",
+ &legacy_crypto_enabled,
+ LGC_ON,
+ legacy_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+}
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2c1b..b97ea4fd36 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -91,6 +91,8 @@ px_crypt(const char *psw, const char *salt, char *buf, unsigned len)
{
const struct px_crypt_algo *c;
+ CheckLegacyCryptoMode();
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
@@ -135,6 +137,8 @@ px_gen_salt(const char *salt_type, char *buf, int rounds)
char *p;
char rbuf[16];
+ CheckLegacyCryptoMode();
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 471bb4ec72..8f21dad54e 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -89,6 +89,12 @@
#define PXE_PGP_UNSUPPORTED_PUBALGO -122
#define PXE_PGP_MULTIPLE_SUBKEYS -123
+typedef enum LegacyCryptoOptions
+{
+ LGC_ON,
+ LGC_OFF,
+ LGC_FIPS,
+} LegacyCryptoOptions;
typedef struct px_digest PX_MD;
typedef struct px_alias PX_Alias;
@@ -96,6 +102,8 @@ typedef struct px_hmac PX_HMAC;
typedef struct px_cipher PX_Cipher;
typedef struct px_combo PX_Combo;
+extern int legacy_crypto_enabled;
+
struct px_digest
{
unsigned (*result_size) (PX_MD *h);
@@ -182,6 +190,8 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
+void CheckLegacyCryptoMode(void);
+
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
#else
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e655..5b75dc5bb4 100644
--- a/contrib/pgcrypto/sql/crypt-des.sql
+++ b/contrib/pgcrypto/sql/crypt-des.sql
@@ -18,4 +18,10 @@ UPDATE ctest SET res = crypt(data, salt);
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+UPDATE ctest SET res = crypt(data, salt);
+RESET pgcrypto.legacy_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f0cd..c54b0e8459 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1222,6 +1222,45 @@ gen_random_uuid() returns uuid
</sect3>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-legacy_crypto_enabled">
+ <term>
+ <varname>pgcrypto.legacy_crypto_enabled</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.legacy_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <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. Setting this to <literal>off</literal>
+ disables these functions. <literal>on</literal> (the default) enables
+ these functions to work normally. <literal>fips</literal> disables these
+ functions if <application>OpenSSL</application> is detected to operate
+ in FIPS mode.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, these parameters are set
+ in <filename>postgresql.conf</filename>, although superusers can alter them
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-author">
<title>Author</title>
--
2.39.3 (Apple Git-146)
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 patch contains: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.
On 12 Dec 2024, at 12:07, Peter Eisentraut <peter@eisentraut.org> wrote:
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 patch contains: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.
Nice catch, they have been copied around since v1 of the patch without anyone
noticing. Fixed, and I also added parentheses to match the docs page in
question (<function>crypt()</function> instead of <function>crypt</function>).
--
Daniel Gustafsson
Attachments:
v6-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patchapplication/octet-stream; name=v6-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patch; x-unix-mode=0644Download
From 93df67e8d130d9aab62247899b2bb54502c1f008 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Thu, 12 Dec 2024 12:54:39 +0100
Subject: [PATCH v6] pgcrypto: Make it possible to disable built-in crypto
When using OpenSSL and/or the underlying operating system in FIPS
mode no non-FIPS certified crypto implementations should be used.
While that is already possible by just not invoking the built-in
crypto in pgcrypto, this adds a GUC which prohibit the code from
being called. This doesn't change the FIPS status of PostgreSQL
but can make it easier for sites which target FIPS compliance to
ensure that violations cannot occur.
Co-authored-by: Daniel Gustafsson <daniel@yesql.se>
Co-authored-by: Joe Conway <mail@joeconway.com>
Reviewed-by: Joe Conway <mail@joeconway.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/TYCPR01MB11684E5B636E17548D4A42248FA4D2@TYCPR01MB11684.jpnprd01.prod.outlook.com
---
contrib/pgcrypto/expected/crypt-des.out | 7 ++++
contrib/pgcrypto/openssl.c | 43 +++++++++++++++++++++++++
contrib/pgcrypto/pgcrypto.c | 30 +++++++++++++++++
contrib/pgcrypto/px-crypt.c | 4 +++
contrib/pgcrypto/px.h | 10 ++++++
contrib/pgcrypto/sql/crypt-des.sql | 6 ++++
doc/src/sgml/pgcrypto.sgml | 39 ++++++++++++++++++++++
7 files changed, 139 insertions(+)
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd580..8993a2d104 100644
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -28,4 +28,11 @@ FROM ctest;
t
(1 row)
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.legacy_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 448db331a0..c5683aa70e 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -794,3 +794,46 @@ ResOwnerReleaseOSSLCipher(Datum res)
{
free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
}
+
+/*
+ * CheckLegacyCryptoMode
+ *
+ * Function for erroring out in case built-in crypto is executed when the user
+ * has disabled it. If legacy_crypto_enabled is set to LGC_OFF or LGC_FIPS and
+ * OpenSSL is operating in FIPS mode the function will error out, else the
+ * query executing built-in crypto can proceed.
+ */
+void
+CheckLegacyCryptoMode(void)
+{
+ int fips_enabled;
+
+ if (legacy_crypto_enabled == LGC_ON)
+ return;
+
+ if (legacy_crypto_enabled == LGC_OFF)
+ ereport(ERROR,
+ errmsg("use of built-in crypto functions is disabled"));
+
+ Assert(legacy_crypto_enabled == LGC_FIPS);
+
+ /*
+ * EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0, before
+ * that FIPS_mode() was used to test for FIPS being enabled. The last
+ * upstream OpenSSL version before 3.0 which supported FIPS was 1.0.2, but
+ * there are forks of 1.1.1 which are FIPS certified so we still need to
+ * test with FIPS_mode() even though we don't support 1.0.2.
+ */
+ fips_enabled =
+#if defined(EVP_default_properties_is_fips_enabled)
+ EVP_default_properties_is_fips_enabled(NULL);
+#elif defined(FIPS_mode)
+ FIPS_mode();
+#else
+ 0;
+#endif
+
+ if (fips_enabled)
+ ereport(ERROR,
+ errmsg("use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode"));
+}
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ebd76eed70..e3d34d3a10 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -38,16 +38,46 @@
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+#include "utils/guc.h"
#include "varatt.h"
PG_MODULE_MAGIC;
/* private stuff */
+static const struct config_enum_entry legacy_crypto_options[] = {
+ {"on", LGC_ON, false},
+ {"off", LGC_OFF, false},
+ {"fips", LGC_FIPS, false},
+ {NULL, 0, false}
+};
+
typedef int (*PFN) (const char *name, void **res);
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+int legacy_crypto_enabled = LGC_ON;
+
+/*
+ * Entrypoint of this module.
+ */
+void
+_PG_init(void)
+{
+ DefineCustomEnumVariable("pgcrypto.legacy_crypto_enabled",
+ "Sets if builtin crypto functions are enabled.",
+ "\"on\" enables builtin crypto, \"off\" unconditionally disables and \"fips\" "
+ "will disable builtin crypto if OpenSSL is in FIPS mode",
+ &legacy_crypto_enabled,
+ LGC_ON,
+ legacy_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+}
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2c1b..b97ea4fd36 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -91,6 +91,8 @@ px_crypt(const char *psw, const char *salt, char *buf, unsigned len)
{
const struct px_crypt_algo *c;
+ CheckLegacyCryptoMode();
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
@@ -135,6 +137,8 @@ px_gen_salt(const char *salt_type, char *buf, int rounds)
char *p;
char rbuf[16];
+ CheckLegacyCryptoMode();
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 471bb4ec72..8f21dad54e 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -89,6 +89,12 @@
#define PXE_PGP_UNSUPPORTED_PUBALGO -122
#define PXE_PGP_MULTIPLE_SUBKEYS -123
+typedef enum LegacyCryptoOptions
+{
+ LGC_ON,
+ LGC_OFF,
+ LGC_FIPS,
+} LegacyCryptoOptions;
typedef struct px_digest PX_MD;
typedef struct px_alias PX_Alias;
@@ -96,6 +102,8 @@ typedef struct px_hmac PX_HMAC;
typedef struct px_cipher PX_Cipher;
typedef struct px_combo PX_Combo;
+extern int legacy_crypto_enabled;
+
struct px_digest
{
unsigned (*result_size) (PX_MD *h);
@@ -182,6 +190,8 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
+void CheckLegacyCryptoMode(void);
+
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
#else
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e655..5b75dc5bb4 100644
--- a/contrib/pgcrypto/sql/crypt-des.sql
+++ b/contrib/pgcrypto/sql/crypt-des.sql
@@ -18,4 +18,10 @@ UPDATE ctest SET res = crypt(data, salt);
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+UPDATE ctest SET res = crypt(data, salt);
+RESET pgcrypto.legacy_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f0cd..7fe47f5f08 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1222,6 +1222,45 @@ gen_random_uuid() returns uuid
</sect3>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-legacy_crypto_enabled">
+ <term>
+ <varname>pgcrypto.legacy_crypto_enabled</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.legacy_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>pgcrypto.legacy_crypto_enabled</varname> determines if the
+ built in legacy crypto functions <literal>gen_salt()</literal>,
+ <literal>pg_gen_salt_rounds()</literal>, and <literal>crypt()</literal>
+ are available for use. Setting this to <literal>off</literal>
+ disables these functions. <literal>on</literal> (the default) enables
+ these functions to work normally. <literal>fips</literal> disables these
+ functions if <application>OpenSSL</application> is detected to operate
+ in FIPS mode.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, these parameters are set
+ in <filename>postgresql.conf</filename>, although superusers can alter them
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-author">
<title>Author</title>
--
2.39.3 (Apple Git-146)
Circling back to wrap up this thread I retested with, and without, OpenSSL FIPS
and tweaked the docs and code a little to ensure it detects the right function
to use. The attached is what I propose we go ahead with.
--
Daniel Gustafsson
Attachments:
v7-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patchapplication/octet-stream; name=v7-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patch; x-unix-mode=0644Download
From dad91c86f90e341a39e544dd664acef9b586c517 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Mon, 13 Jan 2025 15:13:20 +0100
Subject: [PATCH v7] pgcrypto: Make it possible to disable built-in crypto
When using OpenSSL and/or the underlying operating system in FIPS
mode no non-FIPS certified crypto implementations should be used.
While that is already possible by just not invoking the built-in
crypto in pgcrypto, this adds a GUC which prohibit the code from
being called. This doesn't change the FIPS status of PostgreSQL
but can make it easier for sites which target FIPS compliance to
ensure that violations cannot occur.
Author: Daniel Gustafsson <daniel@yesql.se>
Author: Joe Conway <mail@joeconway.com>
Reviewed-by: Joe Conway <mail@joeconway.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/16b4a157-9ea1-44d0-b7b3-4c85df5de97b@joeconway.com
---
contrib/pgcrypto/expected/crypt-des.out | 7 +++++
contrib/pgcrypto/openssl.c | 41 +++++++++++++++++++++++++
contrib/pgcrypto/pgcrypto.c | 30 ++++++++++++++++++
contrib/pgcrypto/px-crypt.c | 4 +++
contrib/pgcrypto/px.h | 10 ++++++
contrib/pgcrypto/sql/crypt-des.sql | 6 ++++
doc/src/sgml/pgcrypto.sgml | 39 +++++++++++++++++++++++
7 files changed, 137 insertions(+)
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd580a..8993a2d1044 100644
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -28,4 +28,11 @@ FROM ctest;
t
(1 row)
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.legacy_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 448db331a0f..61bc1721eba 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -794,3 +794,44 @@ ResOwnerReleaseOSSLCipher(Datum res)
{
free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
}
+
+/*
+ * CheckLegacyCryptoMode
+ *
+ * Function for erroring out in case built-in crypto is executed when the user
+ * has disabled it. If legacy_crypto_enabled is set to LGC_OFF or LGC_FIPS and
+ * OpenSSL is operating in FIPS mode the function will error out, else the
+ * query executing built-in crypto can proceed.
+ */
+void
+CheckLegacyCryptoMode(void)
+{
+ int fips_enabled;
+
+ if (legacy_crypto_enabled == LGC_ON)
+ return;
+
+ if (legacy_crypto_enabled == LGC_OFF)
+ ereport(ERROR,
+ errmsg("use of built-in crypto functions is disabled"));
+
+ Assert(legacy_crypto_enabled == LGC_FIPS);
+
+ /*
+ * EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0, before
+ * that FIPS_mode() was used to test for FIPS being enabled. The last
+ * upstream OpenSSL version before 3.0 which supported FIPS was 1.0.2, but
+ * there are forks of 1.1.1 which are FIPS certified so we still need to
+ * test with FIPS_mode() even though we don't support 1.0.2.
+ */
+ fips_enabled =
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_default_properties_is_fips_enabled(NULL);
+#else
+ FIPS_mode();
+#endif
+
+ if (fips_enabled)
+ ereport(ERROR,
+ errmsg("use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode"));
+}
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ebd76eed702..e3d34d3a10f 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -38,16 +38,46 @@
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+#include "utils/guc.h"
#include "varatt.h"
PG_MODULE_MAGIC;
/* private stuff */
+static const struct config_enum_entry legacy_crypto_options[] = {
+ {"on", LGC_ON, false},
+ {"off", LGC_OFF, false},
+ {"fips", LGC_FIPS, false},
+ {NULL, 0, false}
+};
+
typedef int (*PFN) (const char *name, void **res);
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+int legacy_crypto_enabled = LGC_ON;
+
+/*
+ * Entrypoint of this module.
+ */
+void
+_PG_init(void)
+{
+ DefineCustomEnumVariable("pgcrypto.legacy_crypto_enabled",
+ "Sets if builtin crypto functions are enabled.",
+ "\"on\" enables builtin crypto, \"off\" unconditionally disables and \"fips\" "
+ "will disable builtin crypto if OpenSSL is in FIPS mode",
+ &legacy_crypto_enabled,
+ LGC_ON,
+ legacy_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+}
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2c1bc..b97ea4fd36b 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -91,6 +91,8 @@ px_crypt(const char *psw, const char *salt, char *buf, unsigned len)
{
const struct px_crypt_algo *c;
+ CheckLegacyCryptoMode();
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
@@ -135,6 +137,8 @@ px_gen_salt(const char *salt_type, char *buf, int rounds)
char *p;
char rbuf[16];
+ CheckLegacyCryptoMode();
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 471bb4ec727..8f21dad54ec 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -89,6 +89,12 @@
#define PXE_PGP_UNSUPPORTED_PUBALGO -122
#define PXE_PGP_MULTIPLE_SUBKEYS -123
+typedef enum LegacyCryptoOptions
+{
+ LGC_ON,
+ LGC_OFF,
+ LGC_FIPS,
+} LegacyCryptoOptions;
typedef struct px_digest PX_MD;
typedef struct px_alias PX_Alias;
@@ -96,6 +102,8 @@ typedef struct px_hmac PX_HMAC;
typedef struct px_cipher PX_Cipher;
typedef struct px_combo PX_Combo;
+extern int legacy_crypto_enabled;
+
struct px_digest
{
unsigned (*result_size) (PX_MD *h);
@@ -182,6 +190,8 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
+void CheckLegacyCryptoMode(void);
+
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
#else
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e6555..5b75dc5bb49 100644
--- a/contrib/pgcrypto/sql/crypt-des.sql
+++ b/contrib/pgcrypto/sql/crypt-des.sql
@@ -18,4 +18,10 @@ UPDATE ctest SET res = crypt(data, salt);
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+-- check disabling of legacy crypto functions
+SET pgcrypto.legacy_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+UPDATE ctest SET res = crypt(data, salt);
+RESET pgcrypto.legacy_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f0cde..29dfac92ed8 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1222,6 +1222,45 @@ gen_random_uuid() returns uuid
</sect3>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-legacy_crypto_enabled">
+ <term>
+ <varname>pgcrypto.legacy_crypto_enabled</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.legacy_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>pgcrypto.legacy_crypto_enabled</varname> determines if the
+ built in legacy crypto functions <literal>gen_salt()</literal>,
+ <literal>pg_gen_salt_rounds()</literal>, and <literal>crypt()</literal>
+ are available for use. Setting this to <literal>off</literal>
+ disables these functions. <literal>on</literal> (the default) enables
+ these functions to work normally. <literal>fips</literal> disables these
+ functions if <application>OpenSSL</application> is detected to operate
+ in FIPS mode.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set
+ in <filename>postgresql.conf</filename>, although superusers can alter it
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-author">
<title>Author</title>
--
2.39.3 (Apple Git-146)
Dear Daniel,
Thanks for working on the project. I have few cosmetic comments.
```
+ built in legacy crypto functions <literal>gen_salt()</literal>,
```
According to other lines, `<literal>gen_salt()</literal>` should be `<function>gen_salt()</function>`.
```
+ <literal>pg_gen_salt_rounds()</literal>, and <literal>crypt()</literal>
```
Similar with [1]/messages/by-id/1f32ff67-255d-4c0c-8433-c8c721842aa3@eisentraut.org, `pg_gen_salt_rounds` is not an SQL function.
I think we do not have to mention the function because it's just another implementation of gen_salt().
Also, use <function> instead of <literal>.
```
+void
+_PG_init(void)
+{
+ DefineCustomEnumVariable("pgcrypto.legacy_crypto_enabled",
+ "Sets if builtin crypto functions are enabled.",
+ "\"on\" enables builtin crypto, \"off\" unconditionally disables and \"fips\" "
+ "will disable builtin crypto if OpenSSL is in FIPS mode",
+ &legacy_crypto_enabled,
+ LGC_ON,
+ legacy_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+}
```I think we must call MarkGUCPrefixReserved() to catch the mis-spell.
Also: I'm not sure whether we should bump the version of pgcrypto. It should be done when
the API is changed, but the patch does not do. Thought?
[1]: /messages/by-id/1f32ff67-255d-4c0c-8433-c8c721842aa3@eisentraut.org
Best regards,
Hayato Kuroda
FUJITSU LIMITED
On 14 Jan 2025, at 13:12, Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote:
Similar with [1], `pg_gen_salt_rounds` is not an SQL function.
I think we do not have to mention the function because it's just another implementation of gen_salt().
Also, use <function> instead of <literal>.
Fixed.
I think we must call MarkGUCPrefixReserved() to catch the mis-spell.
Good point, fixed.
Also: I'm not sure whether we should bump the version of pgcrypto. It should be done when
the API is changed, but the patch does not do. Thought?
I don't think this constitutes a change which warrants a version bump so I've
left that out for now.
The attached includes a rename from "legacy_crypto" to "builtin_crypto". While
legacy might apply now, there is work ongoing to modernize the algorithms
supported by crypt [0]c763235a2757e2f5f9e3e27268b9028349cef659.camel@oopsware.de so legacy might not be applicable soon (this GUC would
however still be relevant as the proposed code isn't FIPS certified). Builtin
seems like a more future-proof choice in terms of naming.
--
Daniel Gustafsson
[0]: c763235a2757e2f5f9e3e27268b9028349cef659.camel@oopsware.de
Attachments:
v8-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patchapplication/octet-stream; name=v8-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patch; x-unix-mode=0644Download
From f6132e8b1fbee4b528eb5f4a158a3fbcffa74aa4 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Wed, 15 Jan 2025 14:56:28 +0100
Subject: [PATCH v8] pgcrypto: Make it possible to disable built-in crypto
When using OpenSSL and/or the underlying operating system in FIPS
mode no non-FIPS certified crypto implementations should be used.
While that is already possible by just not invoking the built-in
crypto in pgcrypto, this adds a GUC which prohibit the code from
being called. This doesn't change the FIPS status of PostgreSQL
but can make it easier for sites which target FIPS compliance to
ensure that violations cannot occur.
Author: Daniel Gustafsson <daniel@yesql.se>
Author: Joe Conway <mail@joeconway.com>
Reviewed-by: Joe Conway <mail@joeconway.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/16b4a157-9ea1-44d0-b7b3-4c85df5de97b@joeconway.com
---
contrib/pgcrypto/expected/crypt-des.out | 7 +++++
contrib/pgcrypto/openssl.c | 41 +++++++++++++++++++++++++
contrib/pgcrypto/pgcrypto.c | 31 +++++++++++++++++++
contrib/pgcrypto/px-crypt.c | 4 +++
contrib/pgcrypto/px.h | 10 ++++++
contrib/pgcrypto/sql/crypt-des.sql | 6 ++++
doc/src/sgml/pgcrypto.sgml | 38 +++++++++++++++++++++++
7 files changed, 137 insertions(+)
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd580a..70ca7b357b2 100644
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -28,4 +28,11 @@ FROM ctest;
t
(1 row)
+-- check disabling of built in crypto functions
+SET pgcrypto.builtin_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.builtin_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 448db331a0f..fc930e4679c 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -794,3 +794,44 @@ ResOwnerReleaseOSSLCipher(Datum res)
{
free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
}
+
+/*
+ * CheckBuiltinCryptoMode
+ *
+ * Function for erroring out in case built-in crypto is executed when the user
+ * has disabled it. If builtin_crypto_enabled is set to BC_OFF or BC_FIPS and
+ * OpenSSL is operating in FIPS mode the function will error out, else the
+ * query executing built-in crypto can proceed.
+ */
+void
+CheckBuiltinCryptoMode(void)
+{
+ int fips_enabled;
+
+ if (builtin_crypto_enabled == BC_ON)
+ return;
+
+ if (builtin_crypto_enabled == BC_OFF)
+ ereport(ERROR,
+ errmsg("use of built-in crypto functions is disabled"));
+
+ Assert(builtin_crypto_enabled == BC_FIPS);
+
+ /*
+ * EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0, before
+ * that FIPS_mode() was used to test for FIPS being enabled. The last
+ * upstream OpenSSL version before 3.0 which supported FIPS was 1.0.2, but
+ * there are forks of 1.1.1 which are FIPS certified so we still need to
+ * test with FIPS_mode() even though we don't support 1.0.2.
+ */
+ fips_enabled =
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_default_properties_is_fips_enabled(NULL);
+#else
+ FIPS_mode();
+#endif
+
+ if (fips_enabled)
+ ereport(ERROR,
+ errmsg("use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode"));
+}
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ebd76eed702..d02a621c3c6 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -38,16 +38,47 @@
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+#include "utils/guc.h"
#include "varatt.h"
PG_MODULE_MAGIC;
/* private stuff */
+static const struct config_enum_entry builtin_crypto_options[] = {
+ {"on", BC_ON, false},
+ {"off", BC_OFF, false},
+ {"fips", BC_FIPS, false},
+ {NULL, 0, false}
+};
+
typedef int (*PFN) (const char *name, void **res);
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+int builtin_crypto_enabled = BC_ON;
+
+/*
+ * Entrypoint of this module.
+ */
+void
+_PG_init(void)
+{
+ DefineCustomEnumVariable("pgcrypto.builtin_crypto_enabled",
+ "Sets if builtin crypto functions are enabled.",
+ "\"on\" enables builtin crypto, \"off\" unconditionally disables and \"fips\" "
+ "will disable builtin crypto if OpenSSL is in FIPS mode",
+ &builtin_crypto_enabled,
+ BC_ON,
+ builtin_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+ MarkGUCPrefixReserved("pgcrypto");
+}
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2c1bc..96ce9384aff 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -91,6 +91,8 @@ px_crypt(const char *psw, const char *salt, char *buf, unsigned len)
{
const struct px_crypt_algo *c;
+ CheckBuiltinCryptoMode();
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
@@ -135,6 +137,8 @@ px_gen_salt(const char *salt_type, char *buf, int rounds)
char *p;
char rbuf[16];
+ CheckBuiltinCryptoMode();
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 471bb4ec727..c574f722fdf 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -89,6 +89,12 @@
#define PXE_PGP_UNSUPPORTED_PUBALGO -122
#define PXE_PGP_MULTIPLE_SUBKEYS -123
+typedef enum BuiltinCryptoOptions
+{
+ BC_ON,
+ BC_OFF,
+ BC_FIPS,
+} BuiltinCryptoOptions;
typedef struct px_digest PX_MD;
typedef struct px_alias PX_Alias;
@@ -96,6 +102,8 @@ typedef struct px_hmac PX_HMAC;
typedef struct px_cipher PX_Cipher;
typedef struct px_combo PX_Combo;
+extern int builtin_crypto_enabled;
+
struct px_digest
{
unsigned (*result_size) (PX_MD *h);
@@ -182,6 +190,8 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
+void CheckBuiltinCryptoMode(void);
+
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
#else
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e6555..58cc9d2720a 100644
--- a/contrib/pgcrypto/sql/crypt-des.sql
+++ b/contrib/pgcrypto/sql/crypt-des.sql
@@ -18,4 +18,10 @@ UPDATE ctest SET res = crypt(data, salt);
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+-- check disabling of built in crypto functions
+SET pgcrypto.builtin_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+UPDATE ctest SET res = crypt(data, salt);
+RESET pgcrypto.builtin_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f0cde..d1034eff78b 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1222,6 +1222,44 @@ gen_random_uuid() returns uuid
</sect3>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-builtin_crypto_enabled">
+ <term>
+ <varname>pgcrypto.builtin_crypto_enabled</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.builtin_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>pgcrypto.builtin_crypto_enabled</varname> determines if the
+ built in crypto functions <function>gen_salt()</function>, and
+ <function>crypt()</function> are available for use. Setting this to
+ <literal>off</literal> disables these functions. <literal>on</literal>
+ (the default) enables these functions to work normally.
+ <literal>fips</literal> disables these functions if
+ <application>OpenSSL</application> is detected to operate in FIPS mode.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set
+ in <filename>postgresql.conf</filename>, although superusers can alter it
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-author">
<title>Author</title>
--
2.39.3 (Apple Git-146)
On 1/15/25 09:24, Daniel Gustafsson wrote:
On 14 Jan 2025, at 13:12, Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote:
Similar with [1], `pg_gen_salt_rounds` is not an SQL function.
I think we do not have to mention the function because it's just another implementation of gen_salt().
Also, use <function> instead of <literal>.Fixed.
I think we must call MarkGUCPrefixReserved() to catch the mis-spell.
Good point, fixed.
Also: I'm not sure whether we should bump the version of pgcrypto. It should be done when
the API is changed, but the patch does not do. Thought?I don't think this constitutes a change which warrants a version bump so I've
left that out for now.The attached includes a rename from "legacy_crypto" to "builtin_crypto". While
legacy might apply now, there is work ongoing to modernize the algorithms
supported by crypt [0] so legacy might not be applicable soon (this GUC would
however still be relevant as the proposed code isn't FIPS certified). Builtin
seems like a more future-proof choice in terms of naming.
I wonder if it makes any sense to test the "fips" value of the GUC here:
8<----------------
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -28,4 +28,11 @@ FROM ctest;
t
(1 row)+-- check disabling of built in crypto functions
+SET pgcrypto.builtin_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.builtin_crypto_enabled;
DROP TABLE ctest;
8<----------------The usual case would be "fips disabled", in which case it ought to work
same as "on".
Maybe we could document that the test should fail if fips is enabled?
FWIW I have not tested at all on a fips enabled machine. I will see
about doing that...
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v9-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patchtext/x-patch; charset=UTF-8; name=v9-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patchDownload
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd..edc559d 100644
*** a/contrib/pgcrypto/expected/crypt-des.out
--- b/contrib/pgcrypto/expected/crypt-des.out
*************** FROM ctest;
*** 28,31 ****
--- 28,42 ----
t
(1 row)
+ -- check disabling of built in crypto functions
+ SET pgcrypto.builtin_crypto_enabled = off;
+ UPDATE ctest SET salt = gen_salt('des');
+ ERROR: use of built-in crypto functions is disabled
+ UPDATE ctest SET res = crypt(data, salt);
+ ERROR: use of built-in crypto functions is disabled
+ RESET pgcrypto.builtin_crypto_enabled;
+ SET pgcrypto.builtin_crypto_enabled = fips;
+ UPDATE ctest SET salt = gen_salt('des');
+ UPDATE ctest SET res = crypt(data, salt);
+ RESET pgcrypto.builtin_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 448db33..fc930e4 100644
*** a/contrib/pgcrypto/openssl.c
--- b/contrib/pgcrypto/openssl.c
*************** ResOwnerReleaseOSSLCipher(Datum res)
*** 794,796 ****
--- 794,837 ----
{
free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
}
+
+ /*
+ * CheckBuiltinCryptoMode
+ *
+ * Function for erroring out in case built-in crypto is executed when the user
+ * has disabled it. If builtin_crypto_enabled is set to BC_OFF or BC_FIPS and
+ * OpenSSL is operating in FIPS mode the function will error out, else the
+ * query executing built-in crypto can proceed.
+ */
+ void
+ CheckBuiltinCryptoMode(void)
+ {
+ int fips_enabled;
+
+ if (builtin_crypto_enabled == BC_ON)
+ return;
+
+ if (builtin_crypto_enabled == BC_OFF)
+ ereport(ERROR,
+ errmsg("use of built-in crypto functions is disabled"));
+
+ Assert(builtin_crypto_enabled == BC_FIPS);
+
+ /*
+ * EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0, before
+ * that FIPS_mode() was used to test for FIPS being enabled. The last
+ * upstream OpenSSL version before 3.0 which supported FIPS was 1.0.2, but
+ * there are forks of 1.1.1 which are FIPS certified so we still need to
+ * test with FIPS_mode() even though we don't support 1.0.2.
+ */
+ fips_enabled =
+ #if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_default_properties_is_fips_enabled(NULL);
+ #else
+ FIPS_mode();
+ #endif
+
+ if (fips_enabled)
+ ereport(ERROR,
+ errmsg("use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode"));
+ }
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ebd76ee..d02a621 100644
*** a/contrib/pgcrypto/pgcrypto.c
--- b/contrib/pgcrypto/pgcrypto.c
***************
*** 38,53 ****
--- 38,84 ----
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+ #include "utils/guc.h"
#include "varatt.h"
PG_MODULE_MAGIC;
/* private stuff */
+ static const struct config_enum_entry builtin_crypto_options[] = {
+ {"on", BC_ON, false},
+ {"off", BC_OFF, false},
+ {"fips", BC_FIPS, false},
+ {NULL, 0, false}
+ };
+
typedef int (*PFN) (const char *name, void **res);
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+ int builtin_crypto_enabled = BC_ON;
+
+ /*
+ * Entrypoint of this module.
+ */
+ void
+ _PG_init(void)
+ {
+ DefineCustomEnumVariable("pgcrypto.builtin_crypto_enabled",
+ "Sets if builtin crypto functions are enabled.",
+ "\"on\" enables builtin crypto, \"off\" unconditionally disables and \"fips\" "
+ "will disable builtin crypto if OpenSSL is in FIPS mode",
+ &builtin_crypto_enabled,
+ BC_ON,
+ builtin_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+ MarkGUCPrefixReserved("pgcrypto");
+ }
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2..96ce938 100644
*** a/contrib/pgcrypto/px-crypt.c
--- b/contrib/pgcrypto/px-crypt.c
*************** px_crypt(const char *psw, const char *sa
*** 91,96 ****
--- 91,98 ----
{
const struct px_crypt_algo *c;
+ CheckBuiltinCryptoMode();
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
*************** px_gen_salt(const char *salt_type, char
*** 135,140 ****
--- 137,144 ----
char *p;
char rbuf[16];
+ CheckBuiltinCryptoMode();
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 471bb4e..c574f72 100644
*** a/contrib/pgcrypto/px.h
--- b/contrib/pgcrypto/px.h
***************
*** 89,94 ****
--- 89,100 ----
#define PXE_PGP_UNSUPPORTED_PUBALGO -122
#define PXE_PGP_MULTIPLE_SUBKEYS -123
+ typedef enum BuiltinCryptoOptions
+ {
+ BC_ON,
+ BC_OFF,
+ BC_FIPS,
+ } BuiltinCryptoOptions;
typedef struct px_digest PX_MD;
typedef struct px_alias PX_Alias;
*************** typedef struct px_hmac PX_HMAC;
*** 96,101 ****
--- 102,109 ----
typedef struct px_cipher PX_Cipher;
typedef struct px_combo PX_Combo;
+ extern int builtin_crypto_enabled;
+
struct px_digest
{
unsigned (*result_size) (PX_MD *h);
*************** void px_set_debug_handler(void (*handle
*** 182,187 ****
--- 190,197 ----
void px_memset(void *ptr, int c, size_t len);
+ void CheckBuiltinCryptoMode(void);
+
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
#else
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e..bd61938 100644
*** a/contrib/pgcrypto/sql/crypt-des.sql
--- b/contrib/pgcrypto/sql/crypt-des.sql
*************** UPDATE ctest SET res = crypt(data, salt)
*** 18,21 ****
--- 18,32 ----
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+ -- check disabling of built in crypto functions
+ SET pgcrypto.builtin_crypto_enabled = off;
+ UPDATE ctest SET salt = gen_salt('des');
+ UPDATE ctest SET res = crypt(data, salt);
+ RESET pgcrypto.builtin_crypto_enabled;
+
+ SET pgcrypto.builtin_crypto_enabled = fips;
+ UPDATE ctest SET salt = gen_salt('des');
+ UPDATE ctest SET res = crypt(data, salt);
+ RESET pgcrypto.builtin_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f..d1034ef 100644
*** a/doc/src/sgml/pgcrypto.sgml
--- b/doc/src/sgml/pgcrypto.sgml
*************** gen_random_uuid() returns uuid
*** 1222,1227 ****
--- 1222,1265 ----
</sect3>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-builtin_crypto_enabled">
+ <term>
+ <varname>pgcrypto.builtin_crypto_enabled</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.builtin_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>pgcrypto.builtin_crypto_enabled</varname> determines if the
+ built in crypto functions <function>gen_salt()</function>, and
+ <function>crypt()</function> are available for use. Setting this to
+ <literal>off</literal> disables these functions. <literal>on</literal>
+ (the default) enables these functions to work normally.
+ <literal>fips</literal> disables these functions if
+ <application>OpenSSL</application> is detected to operate in FIPS mode.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set
+ in <filename>postgresql.conf</filename>, although superusers can alter it
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-author">
<title>Author</title>
Thank you for moving this discussion forward.
Maybe we could document that the test should fail if fips is enabled?
FWIW I have not tested at all on a fips enabled machine. I will see about doing
that...
I tested all on a fips enabled machine and test failed.
Since all tests have been made to run even with FIPS enabled in PostgreSQL 17,
it would be ideal for this test to follow suit.
Following the modification that made the citext tests compatible with FIPS mode[1]https://github.com/postgres/postgres/commit/3c551ebede46194237f82062b54b92e474b5c743,
how about make multiple expected output?
diff --git a/contrib/pgcrypto/expected/crypt-des_1.out b/contrib/pgcrypto/expected/crypt-des_1.out
new file mode 100644
index 0000000000..f8106b0ee2
--- /dev/null
+++ b/contrib/pgcrypto/expected/crypt-des_1.out
@@ -0,0 +1,44 @@
+--
+-- crypt() and gen_salt(): crypt-des
+--
+SELECT crypt('', 'NB');
+ crypt
+---------------
+ NBPx/38Y48kHg
+(1 row)
+
+SELECT crypt('foox', 'NB');
+ crypt
+---------------
+ NB53EGGqrrb5E
+(1 row)
+
+-- We are supposed to pass in a 2-character salt.
+-- error since salt is too short:
+SELECT crypt('password', 'a');
+ERROR: invalid salt
+CREATE TABLE ctest (data text, res text, salt text);
+INSERT INTO ctest VALUES ('password', '', '');
+UPDATE ctest SET salt = gen_salt('des');
+UPDATE ctest SET res = crypt(data, salt);
+SELECT res = crypt(data, res) AS "worked"
+FROM ctest;
+ worked
+--------
+ t
+(1 row)
+
+-- check disabling of built in crypto functions
+SET pgcrypto.builtin_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.builtin_crypto_enabled;
+SET pgcrypto.builtin_crypto_enabled = fips;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode
+RESET pgcrypto.builtin_crypto_enabled;
+DROP TABLE ctest;[1]: https://github.com/postgres/postgres/commit/3c551ebede46194237f82062b54b92e474b5c743
Koshi Shibagaki
FUJITSU LIMITED
https://www.fujitsu.com/
On 20 Jan 2025, at 01:26, Koshi Shibagaki (Fujitsu) <shibagaki.koshi@fujitsu.com> wrote:
Thank you for moving this discussion forward.
Maybe we could document that the test should fail if fips is enabled?
FWIW I have not tested at all on a fips enabled machine. I will see about doing
that...I tested all on a fips enabled machine and test failed.
Since all tests have been made to run even with FIPS enabled in PostgreSQL 17,
it would be ideal for this test to follow suit.
Following the modification that made the citext tests compatible with FIPS mode[1],
how about make multiple expected output?
Agreed, I'll roll that into the next version of the patch.
--
Daniel Gustafsson
On 2025-Jan-15, Daniel Gustafsson wrote:
On 14 Jan 2025, at 13:12, Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote:
Also: I'm not sure whether we should bump the version of pgcrypto.
It should be done when the API is changed, but the patch does not
do. Thought?I don't think this constitutes a change which warrants a version bump
so I've left that out for now.
Extension versions need to be changed only when the SQL definition of
the module is modified. If the patch does not require a .sql script
that would run with ALTER EXTENSION UPDATE, then you should not modify
the extension version.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
<Schwern> It does it in a really, really complicated way
<crab> why does it need to be complicated?
<Schwern> Because it's MakeMaker.
On 20 Jan 2025, at 01:26, Koshi Shibagaki (Fujitsu) <shibagaki.koshi@fujitsu.com> wrote:
Thank you for moving this discussion forward.
Maybe we could document that the test should fail if fips is enabled?
FWIW I have not tested at all on a fips enabled machine. I will see about doing
that...I tested all on a fips enabled machine and test failed.
Did the patch as posted fail, or did it fail when you changed the GUC to follow
the fips mode? I assume it's the latter since the code in question doesn't
care about FIPS at all (hence this patch). Re-testing it again against OpenSSL
3.4 with FIPS enabled as well as disabled I can't reproduce any failure.
Since all tests have been made to run even with FIPS enabled in PostgreSQL 17,
it would be ideal for this test to follow suit.
The work which was done was to ensure that the tests passes regardless of if
FIPS is enabled or not, they were not designed to test FIPS.
After thinking about I don't think we need an alternative output file since it
won't add any testing:
+SET pgcrypto.builtin_crypto_enabled = fips; +UPDATE ctest SET salt = gen_salt('des'); +ERROR: use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode +UPDATE ctest SET res = crypt(data, salt);
If we add such an alternative output we also need the other case where FIPS is
disabled and the functions work, which means we add no test coverage at all as
both options are allowed to pass.
--
Daniel Gustafsson
On 1/21/25 06:39, Daniel Gustafsson wrote:
On 20 Jan 2025, at 01:26, Koshi Shibagaki (Fujitsu) <shibagaki.koshi@fujitsu.com> wrote:
Thank you for moving this discussion forward.
Maybe we could document that the test should fail if fips is enabled?
FWIW I have not tested at all on a fips enabled machine. I will see about doing
that...I tested all on a fips enabled machine and test failed.
Did the patch as posted fail, or did it fail when you changed the GUC to follow
the fips mode? I assume it's the latter since the code in question doesn't
care about FIPS at all (hence this patch). Re-testing it again against OpenSSL
3.4 with FIPS enabled as well as disabled I can't reproduce any failure.Since all tests have been made to run even with FIPS enabled in PostgreSQL 17,
it would be ideal for this test to follow suit.The work which was done was to ensure that the tests passes regardless of if
FIPS is enabled or not, they were not designed to test FIPS.After thinking about I don't think we need an alternative output file since it
won't add any testing:+SET pgcrypto.builtin_crypto_enabled = fips; +UPDATE ctest SET salt = gen_salt('des'); +ERROR: use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode +UPDATE ctest SET res = crypt(data, salt);If we add such an alternative output we also need the other case where FIPS is
disabled and the functions work, which means we add no test coverage at all as
both options are allowed to pass.
I was thinking the same -- perhaps just an SQL comment that says
something like: "-- expected to fail when in fips mode"
or similar.
I also wonder if it is worthwhile to expose a SQL callable function to
indicate whether the backend is in fips mode or not. I think that would
be a useful addition, but I guess one could derive the answer based on
whether these functions work or not when "builtin_crypto_enabled = fips"
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On 21 Jan 2025, at 18:51, Joe Conway <mail@joeconway.com> wrote:
On 1/21/25 06:39, Daniel Gustafsson wrote:
If we add such an alternative output we also need the other case where FIPS is
disabled and the functions work, which means we add no test coverage at all as
both options are allowed to pass.I was thinking the same -- perhaps just an SQL comment that says something like: "-- expected to fail when in fips mode"
or similar.
But it isn't actually expected to fail when in FIPS mode since the test sets
the GUC to off. It would only fail (due to different error message) if the GUC
in the testfile was changed and I don't we need to cater for manual alterings
of the testdata.
I also wonder if it is worthwhile to expose a SQL callable function to indicate whether the backend is in fips mode or not. I think that would be a useful addition, but I guess one could derive the answer based on whether these functions work or not when "builtin_crypto_enabled = fips"
It could indeed be useful, but I doubt we can make it portable to check for
anything but the state of OpenSSL. If the operating system has a FIPS mode
then we won't capture that. That might not be a problem since if the OS is in
FIPS mode then OpenSSL most likely will be too but we can't guarantee it.
Definitely worth thinking about, I can see it being useful especially in DBaaS
environments to ensure that the libraries are in the expected state.
--
Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
It could indeed be useful, but I doubt we can make it portable to check for
anything but the state of OpenSSL. If the operating system has a FIPS mode
then we won't capture that. That might not be a problem since if the OS is in
FIPS mode then OpenSSL most likely will be too but we can't guarantee it.
Not our problem, I think. The OS vendor would have to have fallen
down on the job quite badly to offer an OS-level "FIPS mode" while
shipping an OpenSSL that doesn't honor that. It would not be optional
for OpenSSL to be in that mode if the OS is.
(If we end up inventing a FIPS-mode flag, I would fully expect
interested vendors to patch our code to force it on when the
OS-level flag is set, which is exactly what they will have done
to OpenSSL. We should design our behavior with that in mind.)
regards, tom lane
On 21 Jan 2025, at 21:59, Tom Lane <tgl@sss.pgh.pa.us> wrote:
(If we end up inventing a FIPS-mode flag, I would fully expect
interested vendors to patch our code to force it on when the
OS-level flag is set, which is exactly what they will have done
to OpenSSL. We should design our behavior with that in mind.)
This patch is essentially a FIPS-mode flag as it's designed to block the
built-in non-certified code in pgcrypto which ensures that OpenSSL is used for
all crypto operations. When setting this GUC to "fips" it will match the
OpenSSL setting, disable built-in crypto when OpenSSL has FIPS enabled and
allow it when OpenSSL has FIPS disabled. Setting it to off will disable
built-in crypto regardless of FIPS mode in OpenSSL.
--
Daniel Gustafsson
On 1/21/25 15:59, Tom Lane wrote:
Daniel Gustafsson <daniel@yesql.se> writes:
It could indeed be useful, but I doubt we can make it portable to check for
anything but the state of OpenSSL. If the operating system has a FIPS mode
then we won't capture that. That might not be a problem since if the OS is in
FIPS mode then OpenSSL most likely will be too but we can't guarantee it.Not our problem, I think. The OS vendor would have to have fallen
down on the job quite badly to offer an OS-level "FIPS mode" while
shipping an OpenSSL that doesn't honor that. It would not be optional
for OpenSSL to be in that mode if the OS is.(If we end up inventing a FIPS-mode flag, I would fully expect
interested vendors to patch our code to force it on when the
OS-level flag is set, which is exactly what they will have done
to OpenSSL. We should design our behavior with that in mind.)
I think this is a non-issue. Every implementation I have seen, the
OS-level enabling of FIPS mode is just a way to ensure openssl is
automatically put into FIPS mode when the library is loaded, just as if
(and not depending on) the application had invoked FIPS mode manually.
All matters here is that the loaded openssl thinks it is in FIPS mode.
I think that could be done with a subset of the proposed
CheckBuiltinCryptoMode() function. E.g. something like (completely
untested):
8<----------------------
+ /*
+ * CheckFIPSMode
+ *
+ * Function to determine if OpenSSL is operating in FIPS mode
+ */
+ int
+ CheckFIPSMode(void)
+ {
+ int fips_enabled;
+
+ /*
+ * EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0,
before
+ * that FIPS_mode() was used to test for FIPS being enabled. The last
+ * upstream OpenSSL version before 3.0 which supported FIPS was
1.0.2, but
+ * there are forks of 1.1.1 which are FIPS certified so we still need to
+ * test with FIPS_mode() even though we don't support 1.0.2.
+ */
+ fips_enabled =
+ #if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_default_properties_is_fips_enabled(NULL);
+ #else
+ FIPS_mode();
+ #endif
+
+ return fips_enabled;
+ }
8<-----------------The we could call CheckFIPSMode() from CheckBuiltinCryptoMode() as well
as from a SQL-level wrapper.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On 21 Jan 2025, at 22:13, Joe Conway <mail@joeconway.com> wrote:
I think this is a non-issue. Every implementation I have seen, the OS-level enabling of FIPS mode is just a way to ensure openssl is automatically put into FIPS mode when the library is loaded, just as if (and not depending on) the application had invoked FIPS mode manually. All matters here is that the loaded openssl thinks it is in FIPS mode.
Good point. The attached v9 adds a 0001 which expose a SQL function (along
with version bump and docs) for returning the FIPS mode, and 0002 is the
previous patch except it use the function from 0001.
--
Daniel Gustafsson
Attachments:
v9-0001-pgcrypto-Add-function-to-check-FIPS-mode.patchapplication/octet-stream; name=v9-0001-pgcrypto-Add-function-to-check-FIPS-mode.patch; x-unix-mode=0644Download
From a1386937bc29ae9da1b98fbc456dad3fcc51c3fb Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Tue, 21 Jan 2025 23:53:37 +0100
Subject: [PATCH v9 1/2] pgcrypto: Add function to check FIPS mode
This adds a SQL callable function for reading and returning the status
of FIPS configuration of OpenSSL. If OpenSSL is operating with FIPS
enabled it will return true, otherwise false. As this adds a function
to the SQL file, bump the extension version to 1.4.
Author: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/8f979145-e206-475a-a31b-73c977a4134c@joeconway.com
---
contrib/pgcrypto/Makefile | 2 +-
contrib/pgcrypto/meson.build | 1 +
contrib/pgcrypto/openssl.c | 26 +++++++++++++++++++++++++
contrib/pgcrypto/pgcrypto--1.3--1.4.sql | 9 +++++++++
contrib/pgcrypto/pgcrypto.c | 8 ++++++++
contrib/pgcrypto/pgcrypto.control | 2 +-
contrib/pgcrypto/px.h | 2 ++
doc/src/sgml/pgcrypto.sgml | 16 +++++++++++++++
8 files changed, 64 insertions(+), 2 deletions(-)
create mode 100644 contrib/pgcrypto/pgcrypto--1.3--1.4.sql
diff --git a/contrib/pgcrypto/Makefile b/contrib/pgcrypto/Makefile
index 85f1c946813..d40216dd420 100644
--- a/contrib/pgcrypto/Makefile
+++ b/contrib/pgcrypto/Makefile
@@ -36,7 +36,7 @@ MODULE_big = pgcrypto
EXTENSION = pgcrypto
DATA = pgcrypto--1.3.sql pgcrypto--1.2--1.3.sql pgcrypto--1.1--1.2.sql \
- pgcrypto--1.0--1.1.sql
+ pgcrypto--1.0--1.1.sql pgcrypto--1.3--1.4.sql
PGFILEDESC = "pgcrypto - cryptographic functions"
REGRESS = init md5 sha1 hmac-md5 hmac-sha1 blowfish rijndael \
diff --git a/contrib/pgcrypto/meson.build b/contrib/pgcrypto/meson.build
index 0bcbe4cfe5a..7a4e8e76d64 100644
--- a/contrib/pgcrypto/meson.build
+++ b/contrib/pgcrypto/meson.build
@@ -93,6 +93,7 @@ install_data(
'pgcrypto--1.1--1.2.sql',
'pgcrypto--1.2--1.3.sql',
'pgcrypto--1.3.sql',
+ 'pgcrypto--1.3--1.4.sql',
'pgcrypto.control',
kwargs: contrib_data_args,
)
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 448db331a0f..b2984045980 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -794,3 +794,29 @@ ResOwnerReleaseOSSLCipher(Datum res)
{
free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
}
+
+/*
+ * CheckFIPSMode
+ *
+ * Returns the FIPS mode of the underlying OpenSSL installation.
+ */
+bool
+CheckFIPSMode(void)
+{
+ int fips_enabled = 0;
+ /*
+ * EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0, before
+ * that FIPS_mode() was used to test for FIPS being enabled. The last
+ * upstream OpenSSL version before 3.0 which supported FIPS was 1.0.2, but
+ * there are forks of 1.1.1 which are FIPS certified so we still need to
+ * test with FIPS_mode() even though we don't support 1.0.2.
+ */
+ fips_enabled =
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_default_properties_is_fips_enabled(NULL);
+#else
+ FIPS_mode();
+#endif
+
+ return (fips_enabled == 1);
+}
diff --git a/contrib/pgcrypto/pgcrypto--1.3--1.4.sql b/contrib/pgcrypto/pgcrypto--1.3--1.4.sql
new file mode 100644
index 00000000000..b942bde8427
--- /dev/null
+++ b/contrib/pgcrypto/pgcrypto--1.3--1.4.sql
@@ -0,0 +1,9 @@
+/* contrib/pgcrypto/pgcrypto--1.3--1.4.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pgcrypto UPDATE TO '1.4'" to load this file. \quit
+
+CREATE FUNCTION fips_mode()
+RETURNS bool
+AS 'MODULE_PATHNAME', 'pg_check_fipsmode'
+LANGUAGE C VOLATILE STRICT PARALLEL SAFE;
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ebd76eed702..ee2a010e402 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -450,6 +450,14 @@ pg_random_uuid(PG_FUNCTION_ARGS)
return gen_random_uuid(fcinfo);
}
+PG_FUNCTION_INFO_V1(pg_check_fipsmode);
+
+Datum
+pg_check_fipsmode(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_BOOL(CheckFIPSMode());
+}
+
static void *
find_provider(text *name,
PFN provider_lookup,
diff --git a/contrib/pgcrypto/pgcrypto.control b/contrib/pgcrypto/pgcrypto.control
index d2151d3bc4b..fcdd0b46f5b 100644
--- a/contrib/pgcrypto/pgcrypto.control
+++ b/contrib/pgcrypto/pgcrypto.control
@@ -1,6 +1,6 @@
# pgcrypto extension
comment = 'cryptographic functions'
-default_version = '1.3'
+default_version = '1.4'
module_pathname = '$libdir/pgcrypto'
relocatable = true
trusted = true
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 471bb4ec727..c2c2fc31245 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -182,6 +182,8 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
+bool CheckFIPSMode(void);
+
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
#else
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f0cde..838d7532a52 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1149,6 +1149,22 @@ gen_random_uuid() returns uuid
</para>
</sect2>
+ <sect2 id="pgcrypto-openssl-support-funcs">
+ <title>OpenSSL Support Functions</title>
+
+ <indexterm>
+ <primary>fips_mode</primary>
+ </indexterm>
+
+<synopsis>
+fips_mode() returns boolean
+</synopsis>
+ <para>
+ Returns <literal>true</literal> if <productname>OpenSSL</productname> is
+ running with FIPS mode enabled, otherwise <literal>false</literal>.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-notes">
<title>Notes</title>
--
2.39.3 (Apple Git-146)
v9-0002-pgcrypto-Make-it-possible-to-disable-built-in-cry.patchapplication/octet-stream; name=v9-0002-pgcrypto-Make-it-possible-to-disable-built-in-cry.patch; x-unix-mode=0644Download
From 0f4c2e5aad4641b33c310650e522d8c385c3b2e4 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Tue, 21 Jan 2025 23:53:39 +0100
Subject: [PATCH v9 2/2] pgcrypto: Make it possible to disable built-in crypto
When using OpenSSL and/or the underlying operating system in FIPS
mode no non-FIPS certified crypto implementations should be used.
While that is already possible by just not invoking the built-in
crypto in pgcrypto, this adds a GUC which prohibit the code from
being called. This doesn't change the FIPS status of PostgreSQL
but can make it easier for sites which target FIPS compliance to
ensure that violations cannot occur.
Author: Daniel Gustafsson <daniel@yesql.se>
Author: Joe Conway <mail@joeconway.com>
Reviewed-by: Joe Conway <mail@joeconway.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/16b4a157-9ea1-44d0-b7b3-4c85df5de97b@joeconway.com
---
contrib/pgcrypto/expected/crypt-des.out | 7 +++++
contrib/pgcrypto/openssl.c | 26 +++++++++++++++++
contrib/pgcrypto/pgcrypto.c | 31 ++++++++++++++++++++
contrib/pgcrypto/px-crypt.c | 4 +++
contrib/pgcrypto/px.h | 9 ++++++
contrib/pgcrypto/sql/crypt-des.sql | 6 ++++
doc/src/sgml/pgcrypto.sgml | 38 +++++++++++++++++++++++++
7 files changed, 121 insertions(+)
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd580a..70ca7b357b2 100644
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -28,4 +28,11 @@ FROM ctest;
t
(1 row)
+-- check disabling of built in crypto functions
+SET pgcrypto.builtin_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.builtin_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index b2984045980..e3e452d1251 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -804,6 +804,7 @@ bool
CheckFIPSMode(void)
{
int fips_enabled = 0;
+
/*
* EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0, before
* that FIPS_mode() was used to test for FIPS being enabled. The last
@@ -820,3 +821,28 @@ CheckFIPSMode(void)
return (fips_enabled == 1);
}
+
+/*
+ * CheckBuiltinCryptoMode
+ *
+ * Function for erroring out in case built-in crypto is executed when the user
+ * has disabled it. If builtin_crypto_enabled is set to BC_OFF or BC_FIPS and
+ * OpenSSL is operating in FIPS mode the function will error out, else the
+ * query executing built-in crypto can proceed.
+ */
+void
+CheckBuiltinCryptoMode(void)
+{
+ if (builtin_crypto_enabled == BC_ON)
+ return;
+
+ if (builtin_crypto_enabled == BC_OFF)
+ ereport(ERROR,
+ errmsg("use of built-in crypto functions is disabled"));
+
+ Assert(builtin_crypto_enabled == BC_FIPS);
+
+ if (CheckFIPSMode() == true)
+ ereport(ERROR,
+ errmsg("use of non-FIPS certified crypto not allowed when OpenSSL is in FIPS mode"));
+}
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ee2a010e402..b7e5383b9a6 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -38,16 +38,47 @@
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+#include "utils/guc.h"
#include "varatt.h"
PG_MODULE_MAGIC;
/* private stuff */
+static const struct config_enum_entry builtin_crypto_options[] = {
+ {"on", BC_ON, false},
+ {"off", BC_OFF, false},
+ {"fips", BC_FIPS, false},
+ {NULL, 0, false}
+};
+
typedef int (*PFN) (const char *name, void **res);
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+int builtin_crypto_enabled = BC_ON;
+
+/*
+ * Entrypoint of this module.
+ */
+void
+_PG_init(void)
+{
+ DefineCustomEnumVariable("pgcrypto.builtin_crypto_enabled",
+ "Sets if builtin crypto functions are enabled.",
+ "\"on\" enables builtin crypto, \"off\" unconditionally disables and \"fips\" "
+ "will disable builtin crypto if OpenSSL is in FIPS mode",
+ &builtin_crypto_enabled,
+ BC_ON,
+ builtin_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+ MarkGUCPrefixReserved("pgcrypto");
+}
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2c1bc..96ce9384aff 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -91,6 +91,8 @@ px_crypt(const char *psw, const char *salt, char *buf, unsigned len)
{
const struct px_crypt_algo *c;
+ CheckBuiltinCryptoMode();
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
@@ -135,6 +137,8 @@ px_gen_salt(const char *salt_type, char *buf, int rounds)
char *p;
char rbuf[16];
+ CheckBuiltinCryptoMode();
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index c2c2fc31245..37013cd9f82 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -89,6 +89,12 @@
#define PXE_PGP_UNSUPPORTED_PUBALGO -122
#define PXE_PGP_MULTIPLE_SUBKEYS -123
+typedef enum BuiltinCryptoOptions
+{
+ BC_ON,
+ BC_OFF,
+ BC_FIPS,
+} BuiltinCryptoOptions;
typedef struct px_digest PX_MD;
typedef struct px_alias PX_Alias;
@@ -96,6 +102,8 @@ typedef struct px_hmac PX_HMAC;
typedef struct px_cipher PX_Cipher;
typedef struct px_combo PX_Combo;
+extern int builtin_crypto_enabled;
+
struct px_digest
{
unsigned (*result_size) (PX_MD *h);
@@ -183,6 +191,7 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
bool CheckFIPSMode(void);
+void CheckBuiltinCryptoMode(void);
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e6555..58cc9d2720a 100644
--- a/contrib/pgcrypto/sql/crypt-des.sql
+++ b/contrib/pgcrypto/sql/crypt-des.sql
@@ -18,4 +18,10 @@ UPDATE ctest SET res = crypt(data, salt);
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+-- check disabling of built in crypto functions
+SET pgcrypto.builtin_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+UPDATE ctest SET res = crypt(data, salt);
+RESET pgcrypto.builtin_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 838d7532a52..dc19ffd8bc1 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1165,6 +1165,44 @@ fips_mode() returns boolean
</para>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-builtin_crypto_enabled">
+ <term>
+ <varname>pgcrypto.builtin_crypto_enabled</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.builtin_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>pgcrypto.builtin_crypto_enabled</varname> determines if the
+ built in crypto functions <function>gen_salt()</function>, and
+ <function>crypt()</function> are available for use. Setting this to
+ <literal>off</literal> disables these functions. <literal>on</literal>
+ (the default) enables these functions to work normally.
+ <literal>fips</literal> disables these functions if
+ <application>OpenSSL</application> is detected to operate in FIPS mode.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set
+ in <filename>postgresql.conf</filename>, although superusers can alter it
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-notes">
<title>Notes</title>
--
2.39.3 (Apple Git-146)
On 1/21/25 17:57, Daniel Gustafsson wrote:
On 21 Jan 2025, at 22:13, Joe Conway <mail@joeconway.com> wrote:
I think this is a non-issue. Every implementation I have seen, the OS-level enabling of FIPS mode is just a way to ensure openssl is automatically put into FIPS mode when the library is loaded, just as if (and not depending on) the application had invoked FIPS mode manually. All matters here is that the loaded openssl thinks it is in FIPS mode.
Good point. The attached v9 adds a 0001 which expose a SQL function (along
with version bump and docs) for returning the FIPS mode, and 0002 is the
previous patch except it use the function from 0001.
I found it necessary to add:
#include <openssl/crypto.h>
in
contrib/pgcrypto/openssl.c
to avoid a symbol not defined warning.
Otherwise looks good here:
8<---------------
SET pgcrypto.builtin_crypto_enabled = on;
select crypt('secret data', gen_salt('des'));
crypt
---------------
OCAkiP03AAbPA
(1 row)
SET pgcrypto.builtin_crypto_enabled = off;
select crypt('secret data', gen_salt('des'));
ERROR: use of built-in crypto functions is disabled
SET pgcrypto.builtin_crypto_enabled = fips;
select crypt('secret data', gen_salt('des'));
ERROR: use of non-FIPS certified crypto not allowed when OpenSSL is in
FIPS mode
select fips_mode();
fips_mode
-----------
t
(1 row)
8<---------------
Although come to think of it, probably:
"use of non-FIPS certified crypto"
^^^^^^^^^
should rather say:
"use of non-FIPS validated crypto"
^^^^^^^^^
FWIW, I tested with non-FIPS (OpenSSL 3.0.13 30 Jan 2024) on Linux Mint
22.1 and FIPS (aws-lc [1]https://github.com/aws/aws-lc/tree/fips-2022-11-02[2]https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp4759.pdf) on Amazon Linux 2023.
[1]: https://github.com/aws/aws-lc/tree/fips-2022-11-02
[2]: https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp4759.pdf
https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp4759.pdf
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On 22 Jan 2025, at 19:59, Joe Conway <mail@joeconway.com> wrote:
I found it necessary to add:
#include <openssl/crypto.h>
in
contrib/pgcrypto/openssl.c
to avoid a symbol not defined warning.
Makes sense, it doesn't reproduce in my tree but reading OpenSSL code it seems
very plausible (and clearly happens in your environment).
Although come to think of it, probably:
"use of non-FIPS certified crypto"
^^^^^^^^^
should rather say:
"use of non-FIPS validated crypto"
^^^^^^^^^
That's probably better yes. I was under the impression that the terminology
used was "FIPS certified" but reading the OpenSSL and FIPS documentation they
too use "FIPS validated" so I've switched to that as per your comment.
FWIW, I tested with non-FIPS (OpenSSL 3.0.13 30 Jan 2024) on Linux Mint 22.1 and FIPS (aws-lc [1][2]) on Amazon Linux 2023.
Thanks. My testing has been with a range of plain upstream OpenSSL trees from
1.1.1 to 3.4 (compiled on macOS).
Rebased v10 with the above fixed attached.
--
Daniel Gustafsson
Attachments:
v10-0002-pgcrypto-Make-it-possible-to-disable-built-in-cr.patchapplication/octet-stream; name=v10-0002-pgcrypto-Make-it-possible-to-disable-built-in-cr.patch; x-unix-mode=0644Download
From 06594cb71bc0e9eed56d18fe3e991a0abe045a95 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Tue, 21 Jan 2025 23:53:39 +0100
Subject: [PATCH v10 2/2] pgcrypto: Make it possible to disable built-in crypto
When using OpenSSL and/or the underlying operating system in FIPS
mode no non-FIPS certified crypto implementations should be used.
While that is already possible by just not invoking the built-in
crypto in pgcrypto, this adds a GUC which prohibit the code from
being called. This doesn't change the FIPS status of PostgreSQL
but can make it easier for sites which target FIPS compliance to
ensure that violations cannot occur.
Author: Daniel Gustafsson <daniel@yesql.se>
Author: Joe Conway <mail@joeconway.com>
Reviewed-by: Joe Conway <mail@joeconway.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/16b4a157-9ea1-44d0-b7b3-4c85df5de97b@joeconway.com
---
contrib/pgcrypto/expected/crypt-des.out | 7 +++++
contrib/pgcrypto/openssl.c | 29 ++++++++++++++++++-
contrib/pgcrypto/pgcrypto.c | 31 ++++++++++++++++++++
contrib/pgcrypto/px-crypt.c | 4 +++
contrib/pgcrypto/px.h | 9 ++++++
contrib/pgcrypto/sql/crypt-des.sql | 6 ++++
doc/src/sgml/pgcrypto.sgml | 38 +++++++++++++++++++++++++
7 files changed, 123 insertions(+), 1 deletion(-)
diff --git a/contrib/pgcrypto/expected/crypt-des.out b/contrib/pgcrypto/expected/crypt-des.out
index a462dcd580a..70ca7b357b2 100644
--- a/contrib/pgcrypto/expected/crypt-des.out
+++ b/contrib/pgcrypto/expected/crypt-des.out
@@ -28,4 +28,11 @@ FROM ctest;
t
(1 row)
+-- check disabling of built in crypto functions
+SET pgcrypto.builtin_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+ERROR: use of built-in crypto functions is disabled
+UPDATE ctest SET res = crypt(data, salt);
+ERROR: use of built-in crypto functions is disabled
+RESET pgcrypto.builtin_crypto_enabled;
DROP TABLE ctest;
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index b2984045980..75f40a2d034 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -31,6 +31,7 @@
#include "postgres.h"
+#include <openssl/crypto.h>
#include <openssl/evp.h>
#include <openssl/err.h>
#include <openssl/rand.h>
@@ -804,11 +805,12 @@ bool
CheckFIPSMode(void)
{
int fips_enabled = 0;
+
/*
* EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0, before
* that FIPS_mode() was used to test for FIPS being enabled. The last
* upstream OpenSSL version before 3.0 which supported FIPS was 1.0.2, but
- * there are forks of 1.1.1 which are FIPS certified so we still need to
+ * there are forks of 1.1.1 which are FIPS validated so we still need to
* test with FIPS_mode() even though we don't support 1.0.2.
*/
fips_enabled =
@@ -820,3 +822,28 @@ CheckFIPSMode(void)
return (fips_enabled == 1);
}
+
+/*
+ * CheckBuiltinCryptoMode
+ *
+ * Function for erroring out in case built-in crypto is executed when the user
+ * has disabled it. If builtin_crypto_enabled is set to BC_OFF or BC_FIPS and
+ * OpenSSL is operating in FIPS mode the function will error out, else the
+ * query executing built-in crypto can proceed.
+ */
+void
+CheckBuiltinCryptoMode(void)
+{
+ if (builtin_crypto_enabled == BC_ON)
+ return;
+
+ if (builtin_crypto_enabled == BC_OFF)
+ ereport(ERROR,
+ errmsg("use of built-in crypto functions is disabled"));
+
+ Assert(builtin_crypto_enabled == BC_FIPS);
+
+ if (CheckFIPSMode() == true)
+ ereport(ERROR,
+ errmsg("use of non-FIPS validated crypto not allowed when OpenSSL is in FIPS mode"));
+}
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ee2a010e402..b7e5383b9a6 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -38,16 +38,47 @@
#include "px-crypt.h"
#include "px.h"
#include "utils/builtins.h"
+#include "utils/guc.h"
#include "varatt.h"
PG_MODULE_MAGIC;
/* private stuff */
+static const struct config_enum_entry builtin_crypto_options[] = {
+ {"on", BC_ON, false},
+ {"off", BC_OFF, false},
+ {"fips", BC_FIPS, false},
+ {NULL, 0, false}
+};
+
typedef int (*PFN) (const char *name, void **res);
static void *find_provider(text *name, PFN provider_lookup, const char *desc,
int silent);
+int builtin_crypto_enabled = BC_ON;
+
+/*
+ * Entrypoint of this module.
+ */
+void
+_PG_init(void)
+{
+ DefineCustomEnumVariable("pgcrypto.builtin_crypto_enabled",
+ "Sets if builtin crypto functions are enabled.",
+ "\"on\" enables builtin crypto, \"off\" unconditionally disables and \"fips\" "
+ "will disable builtin crypto if OpenSSL is in FIPS mode",
+ &builtin_crypto_enabled,
+ BC_ON,
+ builtin_crypto_options,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+ MarkGUCPrefixReserved("pgcrypto");
+}
+
/* SQL function: hash(bytea, text) returns bytea */
PG_FUNCTION_INFO_V1(pg_digest);
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index 0913ff2c1bc..96ce9384aff 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -91,6 +91,8 @@ px_crypt(const char *psw, const char *salt, char *buf, unsigned len)
{
const struct px_crypt_algo *c;
+ CheckBuiltinCryptoMode();
+
for (c = px_crypt_list; c->id; c++)
{
if (!c->id_len)
@@ -135,6 +137,8 @@ px_gen_salt(const char *salt_type, char *buf, int rounds)
char *p;
char rbuf[16];
+ CheckBuiltinCryptoMode();
+
for (g = gen_list; g->name; g++)
if (pg_strcasecmp(g->name, salt_type) == 0)
break;
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index c2c2fc31245..37013cd9f82 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -89,6 +89,12 @@
#define PXE_PGP_UNSUPPORTED_PUBALGO -122
#define PXE_PGP_MULTIPLE_SUBKEYS -123
+typedef enum BuiltinCryptoOptions
+{
+ BC_ON,
+ BC_OFF,
+ BC_FIPS,
+} BuiltinCryptoOptions;
typedef struct px_digest PX_MD;
typedef struct px_alias PX_Alias;
@@ -96,6 +102,8 @@ typedef struct px_hmac PX_HMAC;
typedef struct px_cipher PX_Cipher;
typedef struct px_combo PX_Combo;
+extern int builtin_crypto_enabled;
+
struct px_digest
{
unsigned (*result_size) (PX_MD *h);
@@ -183,6 +191,7 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
bool CheckFIPSMode(void);
+void CheckBuiltinCryptoMode(void);
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
diff --git a/contrib/pgcrypto/sql/crypt-des.sql b/contrib/pgcrypto/sql/crypt-des.sql
index a85ec1e6555..58cc9d2720a 100644
--- a/contrib/pgcrypto/sql/crypt-des.sql
+++ b/contrib/pgcrypto/sql/crypt-des.sql
@@ -18,4 +18,10 @@ UPDATE ctest SET res = crypt(data, salt);
SELECT res = crypt(data, res) AS "worked"
FROM ctest;
+-- check disabling of built in crypto functions
+SET pgcrypto.builtin_crypto_enabled = off;
+UPDATE ctest SET salt = gen_salt('des');
+UPDATE ctest SET res = crypt(data, salt);
+RESET pgcrypto.builtin_crypto_enabled;
+
DROP TABLE ctest;
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 838d7532a52..dc19ffd8bc1 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1165,6 +1165,44 @@ fips_mode() returns boolean
</para>
</sect2>
+ <sect2 id="pgcrypto-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is one configuration parameter that controls the behavior of
+ <filename>pgcrypto</filename>.
+ </para>
+
+ <variablelist>
+ <varlistentry id="pgcrypto-configuration-parameters-builtin_crypto_enabled">
+ <term>
+ <varname>pgcrypto.builtin_crypto_enabled</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>pgcrypto.builtin_crypto_enabled</varname> configuration
+ parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>pgcrypto.builtin_crypto_enabled</varname> determines if the
+ built in crypto functions <function>gen_salt()</function>, and
+ <function>crypt()</function> are available for use. Setting this to
+ <literal>off</literal> disables these functions. <literal>on</literal>
+ (the default) enables these functions to work normally.
+ <literal>fips</literal> disables these functions if
+ <application>OpenSSL</application> is detected to operate in FIPS mode.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set
+ in <filename>postgresql.conf</filename>, although superusers can alter it
+ on-the-fly within their own sessions.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-notes">
<title>Notes</title>
--
2.39.3 (Apple Git-146)
v10-0001-pgcrypto-Add-function-to-check-FIPS-mode.patchapplication/octet-stream; name=v10-0001-pgcrypto-Add-function-to-check-FIPS-mode.patch; x-unix-mode=0644Download
From c222ac8a5c75b55f6f890616e775f925f712fb4d Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Tue, 21 Jan 2025 23:53:37 +0100
Subject: [PATCH v10 1/2] pgcrypto: Add function to check FIPS mode
This adds a SQL callable function for reading and returning the status
of FIPS configuration of OpenSSL. If OpenSSL is operating with FIPS
enabled it will return true, otherwise false. As this adds a function
to the SQL file, bump the extension version to 1.4.
Author: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/8f979145-e206-475a-a31b-73c977a4134c@joeconway.com
---
contrib/pgcrypto/Makefile | 2 +-
contrib/pgcrypto/meson.build | 1 +
contrib/pgcrypto/openssl.c | 26 +++++++++++++++++++++++++
contrib/pgcrypto/pgcrypto--1.3--1.4.sql | 9 +++++++++
contrib/pgcrypto/pgcrypto.c | 8 ++++++++
contrib/pgcrypto/pgcrypto.control | 2 +-
contrib/pgcrypto/px.h | 2 ++
doc/src/sgml/pgcrypto.sgml | 16 +++++++++++++++
8 files changed, 64 insertions(+), 2 deletions(-)
create mode 100644 contrib/pgcrypto/pgcrypto--1.3--1.4.sql
diff --git a/contrib/pgcrypto/Makefile b/contrib/pgcrypto/Makefile
index 85f1c946813..d40216dd420 100644
--- a/contrib/pgcrypto/Makefile
+++ b/contrib/pgcrypto/Makefile
@@ -36,7 +36,7 @@ MODULE_big = pgcrypto
EXTENSION = pgcrypto
DATA = pgcrypto--1.3.sql pgcrypto--1.2--1.3.sql pgcrypto--1.1--1.2.sql \
- pgcrypto--1.0--1.1.sql
+ pgcrypto--1.0--1.1.sql pgcrypto--1.3--1.4.sql
PGFILEDESC = "pgcrypto - cryptographic functions"
REGRESS = init md5 sha1 hmac-md5 hmac-sha1 blowfish rijndael \
diff --git a/contrib/pgcrypto/meson.build b/contrib/pgcrypto/meson.build
index 0bcbe4cfe5a..7a4e8e76d64 100644
--- a/contrib/pgcrypto/meson.build
+++ b/contrib/pgcrypto/meson.build
@@ -93,6 +93,7 @@ install_data(
'pgcrypto--1.1--1.2.sql',
'pgcrypto--1.2--1.3.sql',
'pgcrypto--1.3.sql',
+ 'pgcrypto--1.3--1.4.sql',
'pgcrypto.control',
kwargs: contrib_data_args,
)
diff --git a/contrib/pgcrypto/openssl.c b/contrib/pgcrypto/openssl.c
index 448db331a0f..b2984045980 100644
--- a/contrib/pgcrypto/openssl.c
+++ b/contrib/pgcrypto/openssl.c
@@ -794,3 +794,29 @@ ResOwnerReleaseOSSLCipher(Datum res)
{
free_openssl_cipher((OSSLCipher *) DatumGetPointer(res));
}
+
+/*
+ * CheckFIPSMode
+ *
+ * Returns the FIPS mode of the underlying OpenSSL installation.
+ */
+bool
+CheckFIPSMode(void)
+{
+ int fips_enabled = 0;
+ /*
+ * EVP_default_properties_is_fips_enabled was added in OpenSSL 3.0, before
+ * that FIPS_mode() was used to test for FIPS being enabled. The last
+ * upstream OpenSSL version before 3.0 which supported FIPS was 1.0.2, but
+ * there are forks of 1.1.1 which are FIPS certified so we still need to
+ * test with FIPS_mode() even though we don't support 1.0.2.
+ */
+ fips_enabled =
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ EVP_default_properties_is_fips_enabled(NULL);
+#else
+ FIPS_mode();
+#endif
+
+ return (fips_enabled == 1);
+}
diff --git a/contrib/pgcrypto/pgcrypto--1.3--1.4.sql b/contrib/pgcrypto/pgcrypto--1.3--1.4.sql
new file mode 100644
index 00000000000..b942bde8427
--- /dev/null
+++ b/contrib/pgcrypto/pgcrypto--1.3--1.4.sql
@@ -0,0 +1,9 @@
+/* contrib/pgcrypto/pgcrypto--1.3--1.4.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION pgcrypto UPDATE TO '1.4'" to load this file. \quit
+
+CREATE FUNCTION fips_mode()
+RETURNS bool
+AS 'MODULE_PATHNAME', 'pg_check_fipsmode'
+LANGUAGE C VOLATILE STRICT PARALLEL SAFE;
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index ebd76eed702..ee2a010e402 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -450,6 +450,14 @@ pg_random_uuid(PG_FUNCTION_ARGS)
return gen_random_uuid(fcinfo);
}
+PG_FUNCTION_INFO_V1(pg_check_fipsmode);
+
+Datum
+pg_check_fipsmode(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_BOOL(CheckFIPSMode());
+}
+
static void *
find_provider(text *name,
PFN provider_lookup,
diff --git a/contrib/pgcrypto/pgcrypto.control b/contrib/pgcrypto/pgcrypto.control
index d2151d3bc4b..fcdd0b46f5b 100644
--- a/contrib/pgcrypto/pgcrypto.control
+++ b/contrib/pgcrypto/pgcrypto.control
@@ -1,6 +1,6 @@
# pgcrypto extension
comment = 'cryptographic functions'
-default_version = '1.3'
+default_version = '1.4'
module_pathname = '$libdir/pgcrypto'
relocatable = true
trusted = true
diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h
index 471bb4ec727..c2c2fc31245 100644
--- a/contrib/pgcrypto/px.h
+++ b/contrib/pgcrypto/px.h
@@ -182,6 +182,8 @@ void px_set_debug_handler(void (*handler) (const char *));
void px_memset(void *ptr, int c, size_t len);
+bool CheckFIPSMode(void);
+
#ifdef PX_DEBUG
void px_debug(const char *fmt,...) pg_attribute_printf(1, 2);
#else
diff --git a/doc/src/sgml/pgcrypto.sgml b/doc/src/sgml/pgcrypto.sgml
index 396c67f0cde..838d7532a52 100644
--- a/doc/src/sgml/pgcrypto.sgml
+++ b/doc/src/sgml/pgcrypto.sgml
@@ -1149,6 +1149,22 @@ gen_random_uuid() returns uuid
</para>
</sect2>
+ <sect2 id="pgcrypto-openssl-support-funcs">
+ <title>OpenSSL Support Functions</title>
+
+ <indexterm>
+ <primary>fips_mode</primary>
+ </indexterm>
+
+<synopsis>
+fips_mode() returns boolean
+</synopsis>
+ <para>
+ Returns <literal>true</literal> if <productname>OpenSSL</productname> is
+ running with FIPS mode enabled, otherwise <literal>false</literal>.
+ </para>
+ </sect2>
+
<sect2 id="pgcrypto-notes">
<title>Notes</title>
--
2.39.3 (Apple Git-146)
On 1/22/25 17:49, Daniel Gustafsson wrote:
On 22 Jan 2025, at 19:59, Joe Conway <mail@joeconway.com> wrote:
I found it necessary to add:
#include <openssl/crypto.h>
in
contrib/pgcrypto/openssl.c
to avoid a symbol not defined warning.Makes sense, it doesn't reproduce in my tree but reading OpenSSL code it seems
very plausible (and clearly happens in your environment).Although come to think of it, probably:
"use of non-FIPS certified crypto"
^^^^^^^^^
should rather say:
"use of non-FIPS validated crypto"
^^^^^^^^^That's probably better yes. I was under the impression that the terminology
used was "FIPS certified" but reading the OpenSSL and FIPS documentation they
too use "FIPS validated" so I've switched to that as per your comment.FWIW, I tested with non-FIPS (OpenSSL 3.0.13 30 Jan 2024) on Linux Mint 22.1 and FIPS (aws-lc [1][2]) on Amazon Linux 2023.
Thanks. My testing has been with a range of plain upstream OpenSSL trees from
1.1.1 to 3.4 (compiled on macOS).Rebased v10 with the above fixed attached.
LGTM
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On 23 Jan 2025, at 21:42, Joe Conway <mail@joeconway.com> wrote:
LGTM
After staring at it a bit more and fixing a few small details I committed this.
--
Daniel Gustafsson
On 1/24/25 08:55, Daniel Gustafsson wrote:
On 23 Jan 2025, at 21:42, Joe Conway <mail@joeconway.com> wrote:
LGTM
After staring at it a bit more and fixing a few small details I committed this.
\o/
Thanks for getting this over the finish line!
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com