Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On 10/29/24 12:52, Jacob Champion wrote: On Sat, Oct 26, 2024 at 11:10 AM Joe Conway 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
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On Sat, Oct 26, 2024 at 11:10 AM Joe Conway 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
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
> On 29 Oct 2024, at 13:53, Joe Conway wrote: > > On 10/29/24 05:57, Daniel Gustafsson wrote: >>> On 26 Oct 2024, at 20:10, Joe Conway wrote: >>> Rather than depend on figuring out if we are in FIPS_mode in a portable >>> way, I think the GUC is simpler and sufficient. Why not do that and just >>> use a better name, e.g. legacy_crypto_enabled or something similar >>> (bike-shedding welcomed) as in the attached. >> I'm not very enthusiastic about adding a GUC to match a system property like >> that for the same reason why we avoid GUCs with transitive dependencies. >> Re-reading the thread and thinking about I think the best solution would be >> to >> split these functions off into their own extension. > > Seems like that would be an issue for backward comparability and upgrades. That's undoubtedly a downside of this proposal which the GUC proposal doesn't have. -- Daniel Gustafsson
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On 10/29/24 05:57, Daniel Gustafsson wrote: On 26 Oct 2024, at 20:10, Joe Conway wrote: Rather than depend on figuring out if we are in FIPS_mode in a portable way, I think the GUC is simpler and sufficient. Why not do that and just use a better name, e.g. legacy_crypto_enabled or something similar (bike-shedding welcomed) as in the attached. I'm not very enthusiastic about adding a GUC to match a system property like that for the same reason why we avoid GUCs with transitive dependencies. Re-reading the thread and thinking about I think the best solution would be to split these functions off into their own extension. Seems like that would be an issue for backward comparability and upgrades. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
> On 26 Oct 2024, at 20:10, Joe Conway 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
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On 20 Feb 2024, at 13:40, Peter Eisentraut 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=.
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.comdiff --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 di
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
> On 20 Feb 2024, at 13:40, Peter Eisentraut 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] sits on my TODO, but given the lack of complaints over the years it's not been bumped to the top. -- Daniel Gustafsson [0] https://www.postgresql.org/message-id/flat/CAA-7PziyARoKi_9e2xdC75RJ068XPVk1CHDDdscu2BGrPuW9TQ%40mail.gmail.com#b20783dd6c72e95a8a0f6464d1228ed5
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On 20.02.24 12:39, Daniel Gustafsson wrote: A fifth option is to throw away our in-tree implementations and use the OpenSSL API's for everything, which is where this thread started. If the effort to payoff ratio is palatable to anyone then patches are for sure welcome. The problem is that, as I understand it, these crypt routines are not designed in a way that you can just plug in a crypto library underneath. Effectively, the definition of what, say, blowfish crypt does, is whatever is in that source file, and transitively, whatever OpenBSD does. (Fun question: Does OpenBSD care about FIPS?) Of course, you could reimplement the same algorithms independently, using OpenSSL or whatever. But I don't think this will really improve the state of the world in aggregate, because to a large degree we are relying on the upstream to keep these implementations maintained, and if we rewrite them, we become the upstream.
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
> On 20 Feb 2024, at 13:24, Robert Haas wrote: > > On Tue, Feb 20, 2024 at 5:09 PM 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. > > That generally seems fine, although I'm fuzzy on what our policy > actually is. We have fallback implementations for some things and not > others, IIRC. I'm not sure there is a well-formed policy, but IIRC the idea with cryptohash was to provide in-core functionality iff OpenSSL isn't used, and only use the OpenSSL implementations if it is. Since pgcrypto cannot be built without OpenSSL (since db7d1a7b0530e8cbd045744e1c75b0e63fb6916f) I don't think it's a problem to continue the work from that commit and replace more with OpenSSL implementations. -- Daniel Gustafsson
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On 20.02.24 12:27, Robert Haas wrote: I don't think the first two of these proposals help anything. AIUI, FIPS mode is supposed to be a system wide toggle that affects everything on the machine. The third one might help if you can be compliant by just choosing not to install that extension, and the fourth one solves the problem by sledgehammer. Does Linux provide some way of asking whether "fips=1" was specified at kernel boot time? What you are describing only happens on Red Hat systems, I think. They have built additional integration around this, which is great. But that's not something you can rely on being the case on all systems, not even all Linux systems.
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On Tue, Feb 20, 2024 at 5:09 PM 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. That generally seems fine, although I'm fuzzy on what our policy actually is. We have fallback implementations for some things and not others, IIRC. > > Does Linux provide some way of asking whether "fips=1" was specified > > at kernel boot time? > > There is a crypto.fips_enabled sysctl but I have no idea how portable that is > across distributions etc. My guess would be that it's pretty portable, but my guesses about Linux might not be very good. Still, if we wanted to go this route, it probably wouldn't be too hard to figure out how portable this is. -- Robert Haas EDB: http://www.enterprisedb.com
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
> On 20 Feb 2024, at 12:18, Peter Eisentraut 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
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
> On 20 Feb 2024, at 12:27, Robert Haas wrote: > > On Tue, Feb 20, 2024 at 4:49 PM Peter Eisentraut wrote: >> I think there are several less weird ways to address this: >> >> * Just document it. >> >> * Make a pgcrypto-level GUC setting. >> >> * Split out these functions into a separate extension. >> >> * Deprecate these functions. >> >> Or some combination of these. > > I don't think the first two of these proposals help anything. AIUI, > FIPS mode is supposed to be a system wide toggle that affects > everything on the machine. The third one might help if you can be > compliant by just choosing not to install that extension, and the > fourth one solves the problem by sledgehammer. A fifth option is to throw away our in-tree implementations and use the OpenSSL API's for everything, which is where this thread started. If the effort to payoff ratio is palatable to anyone then patches are for sure welcome. > Does Linux provide some way of asking whether "fips=1" was specified > at kernel boot time? There is a crypto.fips_enabled sysctl but I have no idea how portable that is across distributions etc. -- Daniel Gustafsson
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On Tue, Feb 20, 2024 at 4:49 PM Peter Eisentraut wrote: > I think there are several less weird ways to address this: > > * Just document it. > > * Make a pgcrypto-level GUC setting. > > * Split out these functions into a separate extension. > > * Deprecate these functions. > > Or some combination of these. I don't think the first two of these proposals help anything. AIUI, FIPS mode is supposed to be a system wide toggle that affects everything on the machine. The third one might help if you can be compliant by just choosing not to install that extension, and the fourth one solves the problem by sledgehammer. Does Linux provide some way of asking whether "fips=1" was specified at kernel boot time? -- Robert Haas EDB: http://www.enterprisedb.com
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On 20.02.24 11:09, Daniel Gustafsson wrote: On 20 Feb 2024, at 10:56, Koshi Shibagaki (Fujitsu) 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.
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
> On 20 Feb 2024, at 10:56, Koshi Shibagaki (Fujitsu) > wrote: > Let me confirm the discussion in threads. I think there are two topics. > 1. prohibit the use of ciphers disallowed in FIPS mode at the level of block > cipher (crypt-bf, etc...) in crypt() and gen_salt() That level might be overkill given that any cipher not in the FIPS certfied module mustn't be used, but it's also not the wrong place to put it IMHO. > 2. adding new "crypt-aes" module. I think this was a hypothetical scenario and not a concrete proposal. > If this is correct, I would like to make a patch for the first topic, as I > think > I can handle it. > Daniel, please let me know if you have been making a patch based on the idea. I haven't yet started on that so feel free to take a stab at it, I'd be happy to review it. Note that there are different API's for doing this in OpenSSL 1.0.2 and OpenSSL 3.x, so a solution must take both into consideration. -- Daniel Gustafsson
RE: Replace current implementations in crypt() and gen_salt() to OpenSSL
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.ko...@fujitsu.com
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
> On 16 Feb 2024, at 15:49, Peter Eisentraut wrote: > Like, if we did a "crypt-aes", would that be FIPS-compliant? I don't know. If I remember my FIPS correct: Only if it used a FIPS certified implementation, like the one in OpenSSL when the fips provider has been loaded. The cipher must be allowed *and* the implementation must be certified. -- Daniel Gustafsson
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On 16.02.24 14:30, Daniel Gustafsson wrote: On 16 Feb 2024, at 13:57, Peter Eisentraut 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.
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
> On 16 Feb 2024, at 13:57, Peter Eisentraut 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
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On 16.02.24 10:16, Daniel Gustafsson wrote: 2. The crypt() and gen_salt() methods built on top of them (modes of operation, kind of) are not FIPS-compliant. I wonder if it's worth trying to make pgcrypto disallow non-FIPS compliant ciphers when the compiled against OpenSSL is running with FIPS mode enabled, or raise a WARNING when used? It seems rather unlikely that someone running OpenSSL with FIPS=yes want to use our DES cipher without there being an error or misconfiguration somewhere. I wonder on what level this kind of check would be done. For example, the password hashing done for SCRAM is not FIPS-compliant either, but surely we don't want to disallow that. Maybe this should be done on the level of block ciphers. So if someone wanted to add a "crypt-aes" module, that would then continue to work.
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
On 2/16/24 04:16, Daniel Gustafsson wrote:
On 15 Feb 2024, at 16:49, Peter Eisentraut wrote:
1. All the block ciphers currently supported by crypt() and gen_salt() are not
FIPS-compliant.
2. The crypt() and gen_salt() methods built on top of them (modes of operation,
kind of) are not FIPS-compliant.
I wonder if it's worth trying to make pgcrypto disallow non-FIPS compliant
ciphers when the compiled against OpenSSL is running with FIPS mode enabled, or
raise a WARNING when used? It seems rather unlikely that someone running
OpenSSL with FIPS=yes want to use our DES cipher without there being an error
or misconfiguration somewhere.
Something like the below untested pseudocode.
diff --git a/contrib/pgcrypto/pgcrypto.c b/contrib/pgcrypto/pgcrypto.c
index 96447c5757..3d4391ebe1 100644
--- a/contrib/pgcrypto/pgcrypto.c
+++ b/contrib/pgcrypto/pgcrypto.c
@@ -187,6 +187,14 @@ pg_crypt(PG_FUNCTION_ARGS)
*resbuf;
text *res;
+#if defined FIPS_mode
+ if (FIPS_mode())
+#else
+ if
(EVP_default_properties_is_fips_enabled(OSSL_LIB_CTX_get0_global_default()))
+#endif
+ ereport(ERROR,
+ (errmsg("not available when using OpenSSL in FIPS
mode")));
Makes sense +1
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
RE: Replace current implementations in crypt() and gen_salt() to OpenSSL
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.ko...@fujitsu.com
RE: Replace current implementations in crypt() and gen_salt() to OpenSSL
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.ko...@fujitsu.com
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
> On 15 Feb 2024, at 16:49, Peter Eisentraut 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=.
The problem with that is that it gives the illusion that enabling such a GUC
gives any guarantees about FIPS which isn't really the case since postgres
isn't FIPS certified.
--
Daniel Gustafsson
Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
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.
