BUG #19457: RE: pgp_sym_encrypt silently accepts non-FIPS ciphers (bf, cast5, 3des) when OpenSSL is in FIPS mod
The following bug has been logged on the website:
Bug reference: 19457
Logged by: Shishir Sharma
Email address: ansh01072001@gmail.com
PostgreSQL version: 18.3
Operating system: AlmaLinux 8.10, x86_64
Description:
When PostgreSQL 18.3 is built against OpenSSL operating in FIPS mode, the
pgp_sym_encrypt() (and pgp_pub_encrypt()) functions in the pgcrypto
extension silently succeed when called with non-FIPS-approved cipher
algorithms: bf (Blowfish), cast5 (CAST5), and 3des (Triple-DES).
This is a FIPS compliance gap. Every other non-FIPS algorithm in
pgcrypto is either blocked by OpenSSL (e.g. encrypt(..., 'bf'),
digest(..., 'md5')) or, as of PostgreSQL 18, controlled by the new
pgcrypto.builtin_crypto_enabled GUC (gen_salt(), crypt()). The PGP
code path is the only one left completely unguarded.
The result is that a deployment that has gone to the trouble of
enabling OpenSSL FIPS mode expecting that prohibited algorithms
cannot be used can still encrypt production data with Blowfish,
CAST5, or 3DES via pgp_sym_encrypt(), with no error, no warning, and
no indication that a FIPS violation has occurred.
=== Root Cause ===
The pgp_sym_encrypt() call chain is:
pgp_sym_encrypt()
→ encrypt_internal() [pgp-pgsql.c]
→ pgp_set_cipher_algo() [pgp.c] -- maps 'cast5' →
PGP_SYM_CAST5
→ pgp_encrypt() [pgp-encrypt.c]
→ pgp_cfb_create() [pgp-cfb.c]
→ pgp_load_cipher() [pgp.c] -- maps PGP_SYM_CAST5 →
"cast5-ecb"
→ px_find_cipher() [openssl.c] -- calls
EVP_get_cipherbyname()
The pgcrypto.builtin_crypto_enabled GUC introduced in PostgreSQL 18
(commits 924d89a and 035f99c) protects only px_crypt() and
px_gen_salt() in px-crypt.c. The PGP encryption path never passes
through px-crypt.c and is therefore not covered by the GUC, regardless
of whether it is set to 'on', 'off', or 'fips'.
=== Steps to Reproduce ===
Prerequisites:
- PostgreSQL 18 built with OpenSSL
- OpenSSL configured in FIPS mode (fips_mode() returns true)
- pgcrypto extension installed
- FIPS enabled system
-- Step 1: Verify FIPS mode is active
SELECT fips_mode();
-- Expected: t
-- Step 2: Confirm the GUC is set to 'fips' (the recommended FIPS setting)
SET pgcrypto.builtin_crypto_enabled = 'fips';
SHOW pgcrypto.builtin_crypto_enabled;
-- Expected: fips
-- Step 3: Verify the GUC correctly blocks gen_salt/crypt (it does work
there)
SELECT gen_salt('bf');
-- Expected: ERROR: use of non-FIPS validated crypto not allowed...
-- Actual: ERROR: use of non-FIPS validated crypto not allowed... ✓
-- Step 4: Verify encrypt() correctly blocks non-FIPS ciphers (OpenSSL
blocks these)
SELECT encrypt('secret'::bytea, 'key12345'::bytea, 'bf');
-- Expected: ERROR
-- Actual: ERROR: encrypt error: Cipher cannot be initialized ✓
-- Step 5: THE BUG — pgp_sym_encrypt silently succeeds with non-FIPS ciphers
SELECT pgp_sym_encrypt('secret', 'key', 'cipher-algo=cast5,
compress-algo=0');
-- Expected: ERROR (non-FIPS algorithm should be rejected)
-- Actual: \xc30d0403... (ciphertext returned silently — FIPS VIOLATION)
SELECT pgp_sym_encrypt('secret', 'key', 'cipher-algo=bf, compress-algo=0');
-- Expected: ERROR
-- Actual: \xc30d0404... (ciphertext returned silently — FIPS VIOLATION)
SELECT pgp_sym_encrypt('secret', 'key', 'cipher-algo=3des,
compress-algo=0');
-- Expected: ERROR
-- Actual: \xc30d0402... (ciphertext returned silently — FIPS VIOLATION)
-- Step 6: Confirm FIPS-approved ciphers still work correctly
SELECT pgp_sym_encrypt('secret', 'key', 'cipher-algo=aes256,
compress-algo=0');
-- Actual: \xc30d0409... (correct — AES-256 is FIPS approved) ✓
=== Proposed Fix ===
I am happy to work on this.
Add a FIPS cipher check in pgp_load_cipher() in contrib/pgcrypto/pgp.c.
This function is the single chokepoint for all PGP cipher operations
(encrypt, decrypt, session key encryption/decryption). A whitelist of
FIPS 140-2/140-3 approved ciphers for PGP use would be:
PGP_SYM_AES_128, PGP_SYM_AES_192, PGP_SYM_AES_256
All other ciphers (PGP_SYM_BLOWFISH, PGP_SYM_CAST5, PGP_SYM_DES3,
PGP_SYM_TWOFISH, etc.) should raise an error when CheckFIPSMode()
returns true.
The error message should be consistent with the one used for gen_salt/crypt:
ERROR: use of non-FIPS validated crypto not allowed when OpenSSL is in
FIPS mode
Additionally, the pgcrypto documentation (doc/src/sgml/pgcrypto.sgml)
should be updated to note which cipher-algo values for pgp_sym_encrypt
are not FIPS 140-2/140-3 approved (bf, cast5, 3des) and that they will
fail when OpenSSL FIPS mode is active with the fix applied.
Related upstream work for reference:
- Commit 924d89a: Add fips_mode() SQL function
- Commit 035f99c: Add pgcrypto.builtin_crypto_enabled GUC
Thank you for your time reviewing this.
On 17 Apr 2026, at 06:21, PG Bug reporting form <noreply@postgresql.org> wrote:
I am happy to work on this.
Please do, that would be great. I'd be happy to review this so keep me CC'd.
Add a FIPS cipher check in pgp_load_cipher() in contrib/pgcrypto/pgp.c.
This function is the single chokepoint for all PGP cipher operations
(encrypt, decrypt, session key encryption/decryption). A whitelist of
FIPS 140-2/140-3 approved ciphers for PGP use would be:PGP_SYM_AES_128, PGP_SYM_AES_192, PGP_SYM_AES_256
Maybe an extra flag in the cipher_info struct?
All other ciphers (PGP_SYM_BLOWFISH, PGP_SYM_CAST5, PGP_SYM_DES3,
PGP_SYM_TWOFISH, etc.) should raise an error when CheckFIPSMode()
returns true.
Not just FIPS, it should check CheckBuiltinCryptoMode() to be consistent with
the other builtin checks.
--
Daniel Gustafsson
On Tue, Apr 21, 2026 at 04:04:40PM +0200, Daniel Gustafsson wrote:
Not just FIPS, it should check CheckBuiltinCryptoMode() to be consistent with
the other builtin checks.
I am interesting in getting that fixed for the next point release, so
I have given it a try, finishing with the attached. This would cause
pgp_sym_encrypt() and pgp_sym_decrypt() to complain when the builtin
mode is disabled, making things more consistent with the surroundings.
I agree that this could break environments where builtin_crypto is
off, as the functions would now be blocked, but I am not sure that
this is worth worrying about as builtin_crypto=on is the default.
Daniel, what do you think?
--
Michael
Attachments:
0001-pgcrypto-Respect-builtin_crypto_enabled-for-PGP-ciph.patchtext/plain; charset=us-asciiDownload+250-15
On 24 Apr 2026, at 06:20, Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Apr 21, 2026 at 04:04:40PM +0200, Daniel Gustafsson wrote:
Not just FIPS, it should check CheckBuiltinCryptoMode() to be consistent with
the other builtin checks.I am interesting in getting that fixed for the next point release, so
I have given it a try, finishing with the attached. This would cause
pgp_sym_encrypt() and pgp_sym_decrypt() to complain when the builtin
mode is disabled, making things more consistent with the surroundings.I agree that this could break environments where builtin_crypto is
off, as the functions would now be blocked, but I am not sure that
this is worth worrying about as builtin_crypto=on is the default.
I'm not convinced this is material for a minor release, the feature works as
documented and it was never documented to cover PGP. Re-reading the thread PGP
was never discussed, and while that admittedly seem like an oversight doing
this in a minor release will alter documented behaviour which is generally not
what we want to do.
--
Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
On 24 Apr 2026, at 06:20, Michael Paquier <michael@paquier.xyz> wrote:
I am interesting in getting that fixed for the next point release, so
I have given it a try, finishing with the attached. This would cause
pgp_sym_encrypt() and pgp_sym_decrypt() to complain when the builtin
mode is disabled, making things more consistent with the surroundings.
I'm not convinced this is material for a minor release, the feature works as
documented and it was never documented to cover PGP. Re-reading the thread PGP
was never discussed, and while that admittedly seem like an oversight doing
this in a minor release will alter documented behaviour which is generally not
what we want to do.
I sympathize with that argument, but ... people who are running in
FIPS mode are probably doing so because they have contractual or legal
obligations to meet that standard. A person who could be in hot water
if they are accidentally running disallowed crypto would see this as a
dangerous bug. A person who does not care should not be using FIPS
mode.
regards, tom lane
On 4/24/26 10:38, Tom Lane wrote:
Daniel Gustafsson <daniel@yesql.se> writes:
On 24 Apr 2026, at 06:20, Michael Paquier <michael@paquier.xyz> wrote:
I am interesting in getting that fixed for the next point release, so
I have given it a try, finishing with the attached. This would cause
pgp_sym_encrypt() and pgp_sym_decrypt() to complain when the builtin
mode is disabled, making things more consistent with the surroundings.I'm not convinced this is material for a minor release, the feature works as
documented and it was never documented to cover PGP. Re-reading the thread PGP
was never discussed, and while that admittedly seem like an oversight doing
this in a minor release will alter documented behaviour which is generally not
what we want to do.I sympathize with that argument, but ... people who are running in
FIPS mode are probably doing so because they have contractual or legal
obligations to meet that standard. A person who could be in hot water
if they are accidentally running disallowed crypto would see this as a
dangerous bug. A person who does not care should not be using FIPS
mode.
+1 I think we should consider this as a backpatchable bug.
--
Joe Conway
PostgreSQL Contributors Team
Amazon Web Services: https://aws.amazon.com
On 24 Apr 2026, at 17:18, Joe Conway <mail@joeconway.com> wrote:
On 4/24/26 10:38, Tom Lane wrote:
Daniel Gustafsson <daniel@yesql.se> writes:
On 24 Apr 2026, at 06:20, Michael Paquier <michael@paquier.xyz> wrote:
I am interesting in getting that fixed for the next point release, so
I have given it a try, finishing with the attached. This would cause
pgp_sym_encrypt() and pgp_sym_decrypt() to complain when the builtin
mode is disabled, making things more consistent with the surroundings.I'm not convinced this is material for a minor release, the feature works as
documented and it was never documented to cover PGP. Re-reading the thread PGP
was never discussed, and while that admittedly seem like an oversight doing
this in a minor release will alter documented behaviour which is generally not
what we want to do.I sympathize with that argument, but ... people who are running in
FIPS mode are probably doing so because they have contractual or legal
obligations to meet that standard. A person who could be in hot water
if they are accidentally running disallowed crypto would see this as a
dangerous bug. A person who does not care should not be using FIPS
mode.+1 I think we should consider this as a backpatchable bug.
Sounds good, I'll have a look at the two proposed patches.
--
Daniel Gustafsson
On Fri, Apr 24, 2026 at 05:27:27PM +0200, Daniel Gustafsson wrote:
Sounds good, I'll have a look at the two proposed patches.
Thanks, Daniel.
FWIW, the patch sent by Shishir is mostly a copy-paste of what I have
provided in terms of tests (exactly the same) and documentation
(mostly the same). He has added a paragraph about the set of ciphers
that are allowed in FIPS. Do we actually need to mention these
explicitely? Perhaps a link to an external source would be more
adapted? I am not convinced that this is a good addition for
pgcrypto, but feel free to disagree.
The second difference is this diff, impacting the outputs of the
tests and the decision-making:
@@ -162,6 +163,9 @@ pgp_load_cipher(int code, PX_Cipher **res)
if (i == NULL)
return PXE_PGP_CORRUPT_DATA;
+ if (!i->fips_allowed)
+ CheckBuiltinCryptoMode();
Daniel should have the last word on that, I guess, as it is his
feature, but the semantics I have chosen are harder than that:
- If the GUC is off, block everything.
- If the GUC is on, allow everything.
- If the GUC is fips, block the non-fips ciphers and allow the fips
ciphers.
This behavior would be more consistent and symmetric with the other
functions, at least IMHO.
Thanks,
--
Michael
On 4/24/26 18:32, Michael Paquier wrote:
He has added a paragraph about the set of ciphers that are allowed
in FIPS. Do we actually need to mention these explicitely? Perhaps
a link to an external source would be more adapted? I am not
convinced that this is a good addition for pgcrypto, but feel free
to disagree.
+1 for a link to an external source, specifically the official NIST
reference I would think.
--
Joe Conway
PostgreSQL Contributors Team
Amazon Web Services: https://aws.amazon.com
My last message showed a failed delivery, so resending it.
Daniel should have the last word on that, I guess, as it is his
feature, but the semantics I have chosen are harder than that:
- If the GUC is off, block everything.
- If the GUC is on, allow everything.
- If the GUC is fips, block the non-fips ciphers and allow the fips
ciphers.This behavior would be more consistent and symmetric with the other
functions, at least IMHO.
The intent behind gating the check on fips_allowed was that the GUC
(commit *035f99c*) was designed to block built-in crypto (gen_salt,
crypt) which use PostgreSQL's own implementations. PGP with AES goes
through OpenSSL's FIPS-validated EVP interface, so blocking it under
builtin_crypto_enabled=off felt like it went beyond what the GUC was
designed for.
That said, you and Daniel have far more context on the codebase and its
history than I do, so I'm happy to adjust or defer to whichever
approach you both prefer.
Show quoted text
Import Notes
Reply to msg id not found: CABV8eT0YZzKmedQsYGP9AkHSK+rKr=rjUFrRQnFfeg1UsVKsXg@mail.gmail.com