From: David Foster Date: Fri, 5 Jun 2026 02:02:44 +0000 (-0400) Subject: Add constant-time validation for CRYPTO_memcmp X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=1a824261ef16231ea036b7935613ea4b219e3a42;p=thirdparty%2Fopenssl.git Add constant-time validation for CRYPTO_memcmp Add test/crypto_memcmp_test.c which provides functional coverage for CRYPTO_memcmp under regular builds and constant-time coverage under enable-ct-validation builds. The added constant-time coverage checks: - there are no data dependent branches or memory accesses, on x86_64 and aarch64 architectures The added constant-time coverage does NOT check: - there are no data-dependent variable-time instructions, such as instructions NOT on the x86 Data Operand Independent Timing list or NOT on the ARM Data-Independent Timing list - any architectures beyond x86_64 and aarch64 New CONSTTIME_SECRET annotations live only in the test rather than in the generic C version of CRYPTO_memcmp so that both the C and assembler versions of CRYPTO_memcmp are constant-time covered. CRYPTO_memcmp directly backs CPython's secrets.compare_digest() and hmac.compare_digest(), so a timing leak in it is high impact, yet it had essentially no direct test coverage. References #15076. Co-Authored-By: Claude Opus 4.8 Reviewed-by: Milan Broz Reviewed-by: Bob Beck MergeDate: Thu Jun 11 16:11:58 2026 (Merged from https://github.com/openssl/openssl/pull/31398) --- diff --git a/.github/workflows/ct-validation-daily.yml b/.github/workflows/ct-validation-daily.yml index 5ae384e5a57..18d8911fcb9 100644 --- a/.github/workflows/ct-validation-daily.yml +++ b/.github/workflows/ct-validation-daily.yml @@ -6,7 +6,8 @@ # https://www.openssl.org/source/license.html name: Constant-time validation (daily) -# Verifies that ML-KEM and ML-DSA signing do not branch on secret data. +# Verifies that several algorithms needing constant-time execution do not +# branch on secret data. # # The library is built with enable-ct-validation, which defines # OPENSSL_CONSTANT_TIME_VALIDATION and causes secret regions to be marked @@ -49,11 +50,26 @@ jobs: strategy: fail-fast: false matrix: + # Constant-timeness is a property of the generated machine code, which + # the compiler derives differently per architecture. Therefore we verify + # both the assembly and C implementations on every architecture we can + # run Valgrind on. include: + # Default builds use assembler implementations (when available) - name: linux-x86_64 runs-on: ubuntu-latest + config_extra: "" - name: linux-aarch64 runs-on: ubuntu-24.04-arm + config_extra: "" + + # no-asm builds always use C implementations + - name: linux-x86_64-no-asm + runs-on: ubuntu-latest + config_extra: no-asm + - name: linux-aarch64-no-asm + runs-on: ubuntu-24.04-arm + config_extra: no-asm name: CT validation (${{ matrix.name }}) runs-on: ${{ matrix.runs-on }} @@ -72,18 +88,23 @@ jobs: - name: Configure with CT validation enabled run: | - ./Configure enable-ct-validation + ./Configure enable-ct-validation ${{ matrix.config_extra }} ./configdata.pm --dump - name: Build run: make -j$(nproc) - - name: Run ML-KEM and ML-DSA CT validation under Valgrind + - name: Run CT validation under Valgrind # OSSL_VALGRIND_CT=yes causes OpenSSL::Test::test() to wrap each # test binary with valgrind --track-origins=yes --error-exitcode=1. # util/wrap.pl -> util/shlib_wrap.sh sets LD_LIBRARY_PATH first, so # the shared libraries are found correctly. + # + # Algorithms covered: + # - memcmp: test_crypto_memcmp + # - ML-KEM: test_internal_ml_kem + # - ML-DSA: test_internal_ml_dsa run: | - make TESTS="test_internal_ml_kem test_internal_ml_dsa" \ + make TESTS="test_internal_ml_kem test_internal_ml_dsa test_crypto_memcmp" \ OSSL_VALGRIND_CT=yes \ test diff --git a/crypto/cpuid.c b/crypto/cpuid.c index d659135919d..89d841bfd0d 100644 --- a/crypto/cpuid.c +++ b/crypto/cpuid.c @@ -193,6 +193,10 @@ void OPENSSL_cpuid_setup(void) * not volatile, but compilers do this in practice anyway. * * There are also assembler versions of this function. + * + * This C version and the per-architecture assembler versions are all verified + * to be constant-time under enable-ct-validation for Valgrind-supported + * architectures, currently x86_64 and aarch64. */ #undef CRYPTO_memcmp int CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len) diff --git a/test/build.info b/test/build.info index 6084d0b16b8..7e3bf669c79 100644 --- a/test/build.info +++ b/test/build.info @@ -49,7 +49,7 @@ IF[{- !$disabled{tests} -}] v3nametest v3ext byteorder_test punycode_test evp_byname_test \ crltest danetest bad_dtls_test lhash_test sparse_array_test \ conf_include_test params_api_test params_conversion_test \ - constant_time_test safe_math_test verify_extra_test clienthellotest \ + constant_time_test crypto_memcmp_test safe_math_test verify_extra_test clienthellotest \ packettest asynctest secmemtest srptest memleaktest stack_test \ dtlsv1listentest ct_test threadstest d2i_test \ ssl_test_ctx_test ssl_test x509aux cipherlist_test asynciotest \ @@ -364,6 +364,10 @@ IF[{- !$disabled{tests} -}] INCLUDE[constant_time_test]=../include ../apps/include DEPEND[constant_time_test]=../libcrypto libtestutil.a + SOURCE[crypto_memcmp_test]=crypto_memcmp_test.c + INCLUDE[crypto_memcmp_test]=../include ../apps/include + DEPEND[crypto_memcmp_test]=../libcrypto libtestutil.a + SOURCE[safe_math_test]=safe_math_test.c INCLUDE[safe_math_test]=../include ../apps/include DEPEND[safe_math_test]=../libcrypto libtestutil.a diff --git a/test/crypto_memcmp_test.c b/test/crypto_memcmp_test.c new file mode 100644 index 00000000000..479c54cf76a --- /dev/null +++ b/test/crypto_memcmp_test.c @@ -0,0 +1,102 @@ +/* + * Copyright 2026 The OpenSSL Project Authors. All Rights Reserved. + * + * Licensed under the Apache License 2.0 (the "License"). You may not use + * this file except in compliance with the License. You can obtain a copy + * in the file LICENSE in the source distribution or at + * https://www.openssl.org/source/license.html + */ + +/* + * Functional and constant-time tests for CRYPTO_memcmp(). + * + * CRYPTO_memcmp() must compare its two operands without any control-flow + * branch or memory access that depends on the operand *contents* (only the + * length is public). + * + * When built with enable-ct-validation (OPENSSL_CONSTANT_TIME_VALIDATION), + * CONSTTIME_SECRET marks the operands as "undefined" for Valgrind's memcheck; + * any branch or memory index derived from them then makes Valgrind exit + * non-zero. Because the taint is injected here at the call site, this test + * verifies whichever CRYPTO_memcmp implementation is actually linked -- the + * per-arch assembler version where one exists (x86_64, aarch64, ...), or the + * C fallback in crypto/cpuid.c otherwise. Outside a CT build the macros are + * no-ops and this is an ordinary functional test. + * + * The accumulated comparison result is the function's intended *public* + * output, so it is declassified before being asserted on; the operand buffers + * are declassified too so the (stack) memory does not stay tainted. + */ + +#include + +#include "internal/nelem.h" +#include "internal/constant_time.h" +#include "testutil.h" + +#define MAX_LEN 64 + +/* + * len == 16 exercises the dedicated fast path in several assembler versions + * (e.g. crypto/x86_64cpuid.pl); the other lengths exercise the byte loop and + * the empty-input early return. + */ +static const struct { + /* byte length of buffers to compare; must be <= MAX_LEN */ + size_t len; + /* index at which the second buffer differs, or -1 for equal buffers */ + int diff_pos; +} memcmp_cases[] = { + /* empty: always equal */ + { 0, -1 }, + { 1, -1 }, + { 1, 0 }, + + /* asm fast path (length = 16) */ + { 16, -1 }, + { 16, 0 }, + { 16, 8 }, + { 16, 15 }, + + /* byte loop */ + { 64, -1 }, + { 64, 0 }, + { 64, 31 }, + { 64, 63 }, +}; + +static int test_crypto_memcmp(int idx) +{ + size_t i; + size_t len = memcmp_cases[idx].len; + int diff_pos = memcmp_cases[idx].diff_pos; + /* nonzero result iff buffers differ */ + int expected = diff_pos >= 0; + unsigned char a[MAX_LEN], b[MAX_LEN]; + int result; + + for (i = 0; i < len; i++) + a[i] = b[i] = (unsigned char)(i * 7 + 1); + if (diff_pos >= 0) + b[diff_pos] ^= 0xff; + + CONSTTIME_SECRET(a, len); + CONSTTIME_SECRET(b, len); + + result = CRYPTO_memcmp(a, b, len); + + CONSTTIME_DECLASSIFY(&result, sizeof(result)); + CONSTTIME_DECLASSIFY(a, len); + CONSTTIME_DECLASSIFY(b, len); + + if (!TEST_int_eq(result != 0, expected)) + return 0; + else + return 1; +} + +int setup_tests(void) +{ + ADD_ALL_TESTS(test_crypto_memcmp, OSSL_NELEM(memcmp_cases)); + return 1; +} diff --git a/test/recipes/90-test_crypto_memcmp.t b/test/recipes/90-test_crypto_memcmp.t new file mode 100644 index 00000000000..6ddd279458d --- /dev/null +++ b/test/recipes/90-test_crypto_memcmp.t @@ -0,0 +1,12 @@ +#! /usr/bin/env perl +# Copyright 2026 The OpenSSL Project Authors. All Rights Reserved. +# +# Licensed under the Apache License 2.0 (the "License"). You may not use +# this file except in compliance with the License. You can obtain a copy +# in the file LICENSE in the source distribution or at +# https://www.openssl.org/source/license.html + + +use OpenSSL::Test::Simple; + +simple_test("test_crypto_memcmp", "crypto_memcmp_test");