On 20 Feb 2024, at 13:40, Peter Eisentraut <pe...@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
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>
