From 459fcabd9ef28f22a604b74a236660ce0ad6fd82 Mon Sep 17 00:00:00 2001
From: Tobias Brunner <tobias@strongswan.org>
Date: Tue, 21 Jul 2026 11:52:46 +0200
Subject: [PATCH] chunk: Improve constant time comparison for chunks with
 unequal length

While for most uses the length is fixed and public (e.g. PRF/MAC outputs),
there are a few (e.g. in xauth-generic) that compare variable length
data.

The previous code directly leaked a differing length by short-circuiting
before comparing anything.  While we could limit the comparison by the
minimum length (and call `memeq_const()`), that could still leak the
length because the time will plateau once the secret's length is reached.
Similarly, if the comparison was bound by the longer chunk (would prevent
the use of `memeq_const()`), the length could also be revealed once the
input gets longer than the secret and the time increases.

This changes the semantics of the function by declaring the first
argument the expected/reference secret and the second the variable input.
This strictly makes the function constant-time, bound by the secret's
length.  So the length can't be guessed by providing different input (but
if an attacker can trigger the comparison against different secrets, of
potentially known lengths, it might still be possible).  If the chunks
are known to have the same length, the order doesn't matter.

Callers of this function have been updated accordingly.
---
 .../plugins/eap_aka/eap_aka_server.c          |  4 +--
 .../plugins/eap_sim/eap_sim_server.c          |  2 +-
 src/libimcv/pts/components/ita/ita_comp_ima.c |  6 ++--
 .../pts/components/ita/ita_comp_tboot.c       |  4 +--
 .../pts/components/ita/ita_comp_tgrub.c       |  4 +--
 src/libstrongswan/plugins/ml/ml_kem.c         |  2 +-
 .../plugins/openssl/openssl_pkcs7.c           |  2 +-
 .../plugins/pkcs12/pkcs12_decode.c            |  2 +-
 .../plugins/pkcs7/pkcs7_signed_data.c         |  2 +-
 src/libstrongswan/tests/suites/test_chunk.c   |  4 +++
 src/libstrongswan/utils/chunk.h               | 32 +++++++++++++++----
 src/libtls/tls_peer.c                         |  4 +--
 src/libtls/tls_server.c                       |  2 +-
 src/pki/commands/ocsp.c                       |  4 +--
 14 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/src/libcharon/plugins/eap_aka/eap_aka_server.c b/src/libcharon/plugins/eap_aka/eap_aka_server.c
index 0712ccc20..89e51c733 100644
--- a/src/libcharon/plugins/eap_aka/eap_aka_server.c
+++ b/src/libcharon/plugins/eap_aka/eap_aka_server.c
@@ -426,7 +426,7 @@ static status_t process_challenge(private_eap_aka_server_t *this,
 	enumerator->destroy(enumerator);
 
 	/* compare received RES against stored XRES */
-	if (!chunk_equals_const(res, this->xres))
+	if (!chunk_equals_const(this->xres, res))
 	{
 		DBG1(DBG_IKE, "received RES does not match XRES");
 		return FAILED;
@@ -487,7 +487,7 @@ static status_t process_reauthentication(private_eap_aka_server_t *this,
 		this->crypto->clear_keys(this->crypto);
 		return challenge(this, out);
 	}
-	if (!chunk_equals_const(counter, this->counter))
+	if (!chunk_equals_const(this->counter, counter))
 	{
 		DBG1(DBG_IKE, "received counter does not match");
 		return FAILED;
diff --git a/src/libcharon/plugins/eap_sim/eap_sim_server.c b/src/libcharon/plugins/eap_sim/eap_sim_server.c
index 82004bd2b..cc9feb838 100644
--- a/src/libcharon/plugins/eap_sim/eap_sim_server.c
+++ b/src/libcharon/plugins/eap_sim/eap_sim_server.c
@@ -263,7 +263,7 @@ static status_t process_reauthentication(private_eap_sim_server_t *this,
 		this->crypto->clear_keys(this->crypto);
 		return initiate(this, out);
 	}
-	if (!chunk_equals_const(counter, this->counter))
+	if (!chunk_equals_const(this->counter, counter))
 	{
 		DBG1(DBG_IKE, "received counter does not match");
 		return FAILED;
diff --git a/src/libimcv/pts/components/ita/ita_comp_ima.c b/src/libimcv/pts/components/ita/ita_comp_ima.c
index 1ad4548f4..c5391b545 100644
--- a/src/libimcv/pts/components/ita/ita_comp_ima.c
+++ b/src/libimcv/pts/components/ita/ita_comp_ima.c
@@ -549,7 +549,7 @@ static status_t verify_ima_measuremnt(pts_t *pts, pts_database_t *pts_db,
 			status = FAILED;
 			break;
 		}
-		if (chunk_equals_const(measurement, hash))
+		if (chunk_equals_const(hash, measurement))
 		{
 			status = SUCCESS;
 			break;
@@ -861,7 +861,7 @@ METHOD(pts_component_t, verify, status_t,
 	}
 
 	has_pcr_info = evidence->get_pcr_info(evidence, &pcr_before, &pcr_after);
-	if (has_pcr_info && !chunk_equals_const(pcr_before, pcrs->get(pcrs, pcr)))
+	if (has_pcr_info && !chunk_equals_const(pcrs->get(pcrs, pcr), pcr_before))
 	{
 		DBG1(DBG_PTS, "PCR %2u: pcr_before is not equal to register value", pcr);
 		return FAILED;
@@ -873,7 +873,7 @@ METHOD(pts_component_t, verify, status_t,
 		return FAILED;
 	}
 
-	if (has_pcr_info && !chunk_equals_const(pcr_after, pcr_value))
+	if (has_pcr_info && !chunk_equals_const(pcr_value, pcr_after))
 	{
 		DBG1(DBG_PTS, "PCR %2u: pcr_after is not equal to register value", pcr);
 		return FAILED;
diff --git a/src/libimcv/pts/components/ita/ita_comp_tboot.c b/src/libimcv/pts/components/ita/ita_comp_tboot.c
index 841b8b392..cd9b84a6f 100644
--- a/src/libimcv/pts/components/ita/ita_comp_tboot.c
+++ b/src/libimcv/pts/components/ita/ita_comp_tboot.c
@@ -256,7 +256,7 @@ METHOD(pts_component_t, verify, status_t,
 	}
 
 	has_pcr_info = evidence->get_pcr_info(evidence, &pcr_before, &pcr_after);
-	if (has_pcr_info && !chunk_equals_const(pcr_before, pcrs->get(pcrs, pcr)))
+	if (has_pcr_info && !chunk_equals_const(pcrs->get(pcrs, pcr), pcr_before))
 	{
 		DBG1(DBG_PTS, "PCR %2u: pcr_before is not equal to register value", pcr);
 		return FAILED;
@@ -268,7 +268,7 @@ METHOD(pts_component_t, verify, status_t,
 		return FAILED;
 	}
 
-	if (has_pcr_info && !chunk_equals_const(pcr_after, pcr_value))
+	if (has_pcr_info && !chunk_equals_const(pcr_value, pcr_after))
 	{
 		DBG1(DBG_PTS, "PCR %2u: pcr_after is not equal to register value", pcr);
 		return FAILED;
diff --git a/src/libimcv/pts/components/ita/ita_comp_tgrub.c b/src/libimcv/pts/components/ita/ita_comp_tgrub.c
index c11a49aed..becf938cd 100644
--- a/src/libimcv/pts/components/ita/ita_comp_tgrub.c
+++ b/src/libimcv/pts/components/ita/ita_comp_tgrub.c
@@ -143,7 +143,7 @@ METHOD(pts_component_t, verify, status_t,
 	/* TODO check measurement in database */
 
 	has_pcr_info = evidence->get_pcr_info(evidence, &pcr_before, &pcr_after);
-	if (has_pcr_info && !chunk_equals_const(pcr_before, pcrs->get(pcrs, pcr)))
+	if (has_pcr_info && !chunk_equals_const(pcrs->get(pcrs, pcr), pcr_before))
 	{
 		DBG1(DBG_PTS, "PCR %2u: pcr_before is not equal to pcr value", pcr);
 		return FAILED;
@@ -155,7 +155,7 @@ METHOD(pts_component_t, verify, status_t,
 		return FAILED;
 	}
 
-	if (has_pcr_info && !chunk_equals_const(pcr_after, pcr_value))
+	if (has_pcr_info && !chunk_equals_const(pcr_value, pcr_after))
 	{
 		DBG1(DBG_PTS, "PCR %2u: pcr_after is not equal to register value", pcr);
 		return FAILED;
diff --git a/src/libstrongswan/plugins/ml/ml_kem.c b/src/libstrongswan/plugins/ml/ml_kem.c
index 861ef0876..add4e914d 100644
--- a/src/libstrongswan/plugins/ml/ml_kem.c
+++ b/src/libstrongswan/plugins/ml/ml_kem.c
@@ -845,7 +845,7 @@ static bool decaps_shared_secret(private_key_exchange_t *this, chunk_t ciphertex
 	/* replace the shared secret with K based on whether our own ciphertext
 	 * matches what we received (in constant time) */
 	memcpy_cond(this->shared_secret.ptr, Kr, this->shared_secret.len,
-				chunk_equals_const(ciphertext, c));
+				chunk_equals_const(c, ciphertext));
 
 	success = TRUE;
 
diff --git a/src/libstrongswan/plugins/openssl/openssl_pkcs7.c b/src/libstrongswan/plugins/openssl/openssl_pkcs7.c
index 656813cf6..73611821a 100644
--- a/src/libstrongswan/plugins/openssl/openssl_pkcs7.c
+++ b/src/libstrongswan/plugins/openssl/openssl_pkcs7.c
@@ -318,7 +318,7 @@ static bool verify_digest(CMS_ContentInfo *cms, CMS_SignerInfo *si, int hash_oid
 	}
 	hasher->destroy(hasher);
 
-	if (!chunk_equals_const(digest, hash))
+	if (!chunk_equals_const(hash, digest))
 	{
 		free(hash.ptr);
 		DBG1(DBG_LIB, "invalid messageDigest");
diff --git a/src/libstrongswan/plugins/pkcs12/pkcs12_decode.c b/src/libstrongswan/plugins/pkcs12/pkcs12_decode.c
index c214147ef..a2cfcf9d9 100644
--- a/src/libstrongswan/plugins/pkcs12/pkcs12_decode.c
+++ b/src/libstrongswan/plugins/pkcs12/pkcs12_decode.c
@@ -337,7 +337,7 @@ static bool verify_mac_pw(signer_t *signer, hash_algorithm_t hash, chunk_t salt,
 	if (pkcs12_derive_key(hash, pw, salt, iterations, PKCS12_KEY_MAC, key) &&
 		signer->set_key(signer, key) &&
 		signer->get_signature(signer, data, calculated.ptr) &&
-		chunk_equals_const(mac, calculated))
+		chunk_equals_const(calculated, mac))
 	{
 		success = TRUE;
 	}
diff --git a/src/libstrongswan/plugins/pkcs7/pkcs7_signed_data.c b/src/libstrongswan/plugins/pkcs7/pkcs7_signed_data.c
index f49d52010..7ba9dbd28 100644
--- a/src/libstrongswan/plugins/pkcs7/pkcs7_signed_data.c
+++ b/src/libstrongswan/plugins/pkcs7/pkcs7_signed_data.c
@@ -274,7 +274,7 @@ METHOD(enumerator_t, enumerate, bool,
 		hasher->destroy(hasher);
 		DBG3(DBG_LIB, "hash: %B", &hash);
 
-		valid = chunk_equals_const(chunk, hash);
+		valid = chunk_equals_const(hash, chunk);
 		free(hash.ptr);
 		if (!valid)
 		{
diff --git a/src/libstrongswan/tests/suites/test_chunk.c b/src/libstrongswan/tests/suites/test_chunk.c
index 016e8f340..3b7e4622a 100644
--- a/src/libstrongswan/tests/suites/test_chunk.c
+++ b/src/libstrongswan/tests/suites/test_chunk.c
@@ -76,14 +76,18 @@ START_TEST(test_chunk_equals_const)
 
 	chunk_a = chunk;
 	ck_assert(!chunk_equals_const(chunk_a, chunk_b));
+	ck_assert(!chunk_equals_const(chunk_b, chunk_a));
 	chunk_b = chunk;
 	ck_assert(chunk_equals_const(chunk_a, chunk_b));
+	ck_assert(chunk_equals_const(chunk_b, chunk_a));
 
 	chunk_b = chunk_from_str("asdf");
 	ck_assert(!chunk_equals_const(chunk_a, chunk_b));
+	ck_assert(!chunk_equals_const(chunk_b, chunk_a));
 
 	chunk_b = chunk_from_str("chunk");
 	ck_assert(chunk_equals_const(chunk_a, chunk_b));
+	ck_assert(chunk_equals_const(chunk_b, chunk_a));
 }
 END_TEST
 
diff --git a/src/libstrongswan/utils/chunk.h b/src/libstrongswan/utils/chunk.h
index e0417cf36..dd9e38cab 100644
--- a/src/libstrongswan/utils/chunk.h
+++ b/src/libstrongswan/utils/chunk.h
@@ -362,16 +362,34 @@ static inline bool chunk_equals(chunk_t a, chunk_t b)
 }
 
 /**
- * Compare two chunks for equality, constant time for cryptographic purposes.
+ * Compare a secret chunk against some arbitrary input in constant time for
+ * cryptographic purposes.
  *
- * Note that this function is constant time only for chunks with the same
- * length, i.e. it does not protect against guessing the length of one of the
- * chunks.
+ * If neither chunk is NULL, this function is always constant time and bound by
+ * the secret's length.  If the length is known to be equal, the order of the
+ * arguments doesn't matter.
+ *
+ * @param secret        expected/reference value (loop bound)
+ * @param input         value to verify against secret
+ * @return              TRUE if the chunks are equal
  */
-static inline bool chunk_equals_const(chunk_t a, chunk_t b)
+static inline bool chunk_equals_const(chunk_t secret, chunk_t input)
 {
-	return a.ptr != NULL  && b.ptr != NULL &&
-			a.len == b.len && memeq_const(a.ptr, b.ptr, a.len);
+	u_int diff = 0;
+	size_t i;
+
+	if (!secret.ptr || !input.ptr)
+	{
+		return FALSE;
+	}
+	diff |= constant_time_neq64(secret.len, input.len);
+	for (i = 0; i < secret.len; i++)
+	{
+		/* the ternary here is OK as the input length is known to the attacker
+		 * and the overall time is always bound by the secret's length */
+		diff |= secret.ptr[i] ^ (i < input.len ? input.ptr[i]: 0);
+	}
+	return !diff;
 }
 
 /**
diff --git a/src/libtls/tls_peer.c b/src/libtls/tls_peer.c
index ed2031128..1dff1cc3c 100644
--- a/src/libtls/tls_peer.c
+++ b/src/libtls/tls_peer.c
@@ -232,7 +232,7 @@ static status_t process_server_hello(private_tls_peer_t *this,
 		return NEED_MORE;
 	}
 
-	is_retry_request = chunk_equals_const(random, tls_hello_retry_request_magic);
+	is_retry_request = chunk_equals_const(tls_hello_retry_request_magic, random);
 
 	memcpy(this->server_random, random.ptr, sizeof(this->server_random));
 
@@ -1044,7 +1044,7 @@ static status_t process_finished(private_tls_peer_t *this, bio_reader_t *reader)
 		}
 	}
 
-	if (!chunk_equals_const(received, verify_data))
+	if (!chunk_equals_const(verify_data, received))
 	{
 		DBG1(DBG_TLS, "received server finished invalid");
 		this->alert->add(this->alert, TLS_FATAL, TLS_DECRYPT_ERROR);
diff --git a/src/libtls/tls_server.c b/src/libtls/tls_server.c
index 4581aba6f..2b798020a 100644
--- a/src/libtls/tls_server.c
+++ b/src/libtls/tls_server.c
@@ -992,7 +992,7 @@ static status_t process_finished(private_tls_server_t *this,
 		this->crypto->change_cipher(this->crypto, TRUE);
 	}
 
-	if (!chunk_equals_const(received, verify_data))
+	if (!chunk_equals_const(verify_data, received))
 	{
 		DBG1(DBG_TLS, "received client finished invalid");
 		this->alert->add(this->alert, TLS_FATAL, TLS_DECRYPT_ERROR);
diff --git a/src/pki/commands/ocsp.c b/src/pki/commands/ocsp.c
index 3b010470d..4da0416bc 100644
--- a/src/pki/commands/ocsp.c
+++ b/src/pki/commands/ocsp.c
@@ -106,7 +106,7 @@ static bool find_issuer_cacert(hash_algorithm_t hashAlgorithm,
 			break;
 		}
 
-		issuerKeyHash_ok = chunk_equals_const(issuerKeyHash, caKeyHash);
+		issuerKeyHash_ok = chunk_equals_const(caKeyHash, issuerKeyHash);
 		public->destroy(public);
 		if (!issuerKeyHash_ok)
 		{
@@ -132,7 +132,7 @@ static bool find_issuer_cacert(hash_algorithm_t hashAlgorithm,
 		}
 		hasher->destroy(hasher);
 
-		issuerNameHash_ok = chunk_equals_const(issuerNameHash, caNameHash);
+		issuerNameHash_ok = chunk_equals_const(caNameHash, issuerNameHash);
 		chunk_free(&caNameHash);
 		if (!issuerNameHash_ok)
 		{