From: | Joe Conway <mail(at)joeconway(dot)com> |
---|---|
To: | Daniel Gustafsson <daniel(at)yesql(dot)se>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Koshi Shibagaki (Fujitsu)" <shibagaki(dot)koshi(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Replace current implementations in crypt() and gen_salt() to OpenSSL |
Date: | 2025-01-18 17:32:10 |
Message-ID: | e16b6d8e-5f8c-4f6a-85ed-e1fb59ba6b3a@joeconway.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 1/15/25 09:24, Daniel Gustafsson wrote:
>> On 14 Jan 2025, at 13:12, Hayato Kuroda (Fujitsu) <kuroda(dot)hayato(at)fujitsu(dot)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
Attachment | Content-Type | Size |
---|---|---|
v9-0001-pgcrypto-Make-it-possible-to-disable-built-in-cry.patch | text/x-patch | 7.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2025-01-18 18:17:46 | Re: [PATCH] Fix jsonb comparison for raw scalar pseudo arrays |
Previous Message | Robert Treat | 2025-01-18 17:10:23 | Re: Eagerly scan all-visible pages to amortize aggressive vacuum |