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) {