HaveIBeenPwnedRestApiPasswordChecker implementations are not thread-safe

[garvit-joshi]

HaveIBeenPwnedRestApiPasswordChecker stores a single MessageDigest instance as a field and reuses it across all invocations of check(). Since MessageDigest is not thread-safe, concurrent calls can produce incorrect hash values.

To Reproduce

1. Register HaveIBeenPwnedRestApiPasswordChecker as a singleton bean
2. Call check() concurrently from multiple threads
3. Hash computation becomes unreliable due to shared mutable state in MessageDigest

Expected behavior

The checker should produce correct results under concurrent access. A new MessageDigest instance should be created per invocation instead of reusing a shared instance.

[garvit-joshi]

Hi Team,

Added a small reproducible demo:

import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.HexFormat;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;

public class MessageDigestThreadSafetyDemo {

	// Shared MessageDigest instance (NOT thread-safe!)
	private static final MessageDigest SHARED_MD;

	static {
		try {
			SHARED_MD = MessageDigest.getInstance("SHA-1");
		} catch (NoSuchAlgorithmException e) {
			throw new RuntimeException(e);
		}
	}

	static void main(String[] args) throws Exception {
		String input = "Hello World";
		String expectedHash = computeHashSafely(input);
		System.out.println("Expected hash: " + expectedHash);

		int threadCount = 10;
		int iterationsPerThread = 1000;
		CountDownLatch latch = new CountDownLatch(threadCount);
		AtomicInteger failures = new AtomicInteger(0);

		for (int t = 0; t < threadCount; t++) {
			Thread.startVirtualThread(() -> {
				try {
					for (int i = 0; i < iterationsPerThread; i++) {
						String hash = computeHashUnsafely(input);
						if (!expectedHash.equals(hash)) {
							System.out.println("wrong hash: " + hash);
							failures.incrementAndGet();
						}
					}
				} finally {
					latch.countDown();
				}
			});
		}

		latch.await();
		System.out.println("Total operations: " + (threadCount * iterationsPerThread));
		System.out.println("Failures (wrong hash): " + failures.get());
	}

	// UNSAFE: Uses shared MessageDigest instance
	private static String computeHashUnsafely(String input) {
		byte[] hash = SHARED_MD.digest(input.getBytes());
		return HexFormat.of().formatHex(hash);
	}

	// SAFE: Creates new instance each time
	private static String computeHashSafely(String input) throws NoSuchAlgorithmException {
		MessageDigest md = MessageDigest.getInstance("SHA-1");
		byte[] hash = md.digest(input.getBytes());
		return HexFormat.of().formatHex(hash);
	}
}

outputs:

Expected hash: 0a4d55a8d778e5022fab701977c5d840bbc486d0
wrong hash: 5dc0012ad54717e3c566b0a84c7085693dba31bd
wrong hash: b38bee8defe04cfba4c5d7f032b6a7d2e4afa520
wrong hash: da39a3ee5e6b4b0d3255bfef95601890afd80709
wrong hash: 16a816dad565af32c5618209370704742658e04f
.
.
.
wrong hash: da39a3ee5e6b4b0d3255bfef95601890afd80709
Total operations: 10000
Failures (wrong hash): 400

[garvit-joshi]

Hi @rwinch ,
I have made a PR for this issue.

Could you please check #18235

[rwinch]

Closing in favor of the PR