Thread-unsafe MD5 on big-endian systems with no OpenSSL
While browsing through all our global variables for the multithreading
effort, I noticed that our MD5 implementation in src/common/md5.c uses a
static buffer on big-endian systems, which makes it not thread-safe.
That's a bug because that function is also used in libpq.
This was introduced in commit b67b57a966, which replaced the old MD5
fallback implementation with the one from pgcrypto. The thread-safety
didn't matter for pgcrypto, but for libpq it does.
This only affects big-endian systems that are compiled without OpenSSL.
--
Heikki Linnakangas
Neon (https://neon.tech)
Attachments:
0001-Make-fallback-MD5-implementation-thread-safe-on-big-.patchtext/x-patch; charset=UTF-8; name=0001-Make-fallback-MD5-implementation-thread-safe-on-big-.patchDownload
From 5ec1ec90ba4b1828b4ecc3d6907489f879e1af12 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue, 6 Aug 2024 15:20:06 +0300
Subject: [PATCH 1/1] Make fallback MD5 implementation thread-safe on
big-endian systems
Replace a static scratch buffer with a local variable, because a
static buffer makes the function not thread-safe. This function is
used in client-code in libpq, so it needs to be thread-safe. It was
until commit b67b57a966, which replaced the implementation with the
one from pgcrypto.
Backpatch to v14, where we switched to the new implementation.
---
src/common/md5.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/common/md5.c b/src/common/md5.c
index f1d47722f8..32b696a66d 100644
--- a/src/common/md5.c
+++ b/src/common/md5.c
@@ -150,10 +150,6 @@ static const uint8 md5_paddat[MD5_BUFLEN] = {
0, 0, 0, 0, 0, 0, 0, 0,
};
-#ifdef WORDS_BIGENDIAN
-static uint32 X[16];
-#endif
-
static void
md5_calc(const uint8 *b64, pg_md5_ctx *ctx)
{
@@ -167,6 +163,7 @@ md5_calc(const uint8 *b64, pg_md5_ctx *ctx)
#else
/* 4 byte words */
/* what a brute force but fast! */
+ uint32 X[16];
uint8 *y = (uint8 *) X;
y[0] = b64[3];
--
2.39.2
On Tue, Aug 6, 2024 at 8:23 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
While browsing through all our global variables for the multithreading
effort, I noticed that our MD5 implementation in src/common/md5.c uses a
static buffer on big-endian systems, which makes it not thread-safe.
That's a bug because that function is also used in libpq.This was introduced in commit b67b57a966, which replaced the old MD5
fallback implementation with the one from pgcrypto. The thread-safety
didn't matter for pgcrypto, but for libpq it does.This only affects big-endian systems that are compiled without OpenSSL.
LGTM.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Aug 6, 2024, at 23:05, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Aug 6, 2024 at 8:23 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:This only affects big-endian systems that are compiled without OpenSSL.
LGTM.
Nice catch, looks fine to me as well.
--
Michael
On 06/08/2024 18:11, Michael Paquier wrote:
On Aug 6, 2024, at 23:05, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Aug 6, 2024 at 8:23 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:This only affects big-endian systems that are compiled without OpenSSL.
LGTM.
Nice catch, looks fine to me as well.
Committed, thanks
--
Heikki Linnakangas
Neon (https://neon.tech)