From 390666b9fa20e90234518e51b476a88a51204ca9 Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Sat, 12 Mar 2022 21:42:01 -0800 Subject: [PATCH] AK: Add naive implementations of AK::timing_safe_compare For security critical code we need to have some way of performing constant time buffer comparisons. --- AK/Memory.h | 25 +++++++++++++++++++++++++ Tests/AK/TestMemory.cpp | 13 ++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/AK/Memory.h b/AK/Memory.h index a23ca5d219..6cee4fe846 100644 --- a/AK/Memory.h +++ b/AK/Memory.h @@ -1,5 +1,6 @@ /* * Copyright (c) 2020, Andreas Kling + * Copyright (c) 2021-2022, Brian Gianforcaro * * SPDX-License-Identifier: BSD-2-Clause */ @@ -42,6 +43,7 @@ ALWAYS_INLINE void fast_u32_fill(u32* dest, u32 value, size_t count) } namespace AK { + inline void secure_zero(void* ptr, size_t size) { __builtin_memset(ptr, 0, size); @@ -50,6 +52,29 @@ inline void secure_zero(void* ptr, size_t size) asm volatile("" :: : "memory"); } + +// Naive implementation of a constant time buffer comparison function. +// The goal being to not use any conditional branching so calls are +// guarded against potential timing attacks. +// +// See OpenBSD's timingsafe_memcmp for more advanced implementations. +inline bool timing_safe_compare(const void* b1, const void* b2, size_t len) +{ + auto* c1 = static_cast(b1); + auto* c2 = static_cast(b2); + + u8 res = 0; + for (size_t i = 0; i < len; i++) { + res |= c1[i] ^ c2[i]; + } + + // FIXME: !res can potentially inject branches depending + // on which toolchain is being used for compilation. Ideally + // we would use a more advanced algorithm. + return !res; +} + } using AK::secure_zero; +using AK::timing_safe_compare; diff --git a/Tests/AK/TestMemory.cpp b/Tests/AK/TestMemory.cpp index f4dd0a3e52..15ba7fe4c7 100644 --- a/Tests/AK/TestMemory.cpp +++ b/Tests/AK/TestMemory.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, the SerenityOS developers. + * Copyright (c) 2020-2022, the SerenityOS developers. * * SPDX-License-Identifier: BSD-2-Clause */ @@ -7,6 +7,8 @@ #include #include +#include +#include TEST_CASE(bitap) { @@ -66,3 +68,12 @@ TEST_CASE(kmp_two_chunks) EXPECT_EQ(result_2.value_or(9), 4u); EXPECT(!result_3.has_value()); } + +TEST_CASE(timing_safe_compare) +{ + String data_set = "abcdefghijklmnopqrstuvwxyz123456789"; + EXPECT_EQ(true, AK::timing_safe_compare(data_set.characters(), data_set.characters(), data_set.length())); + + String reversed = data_set.reverse(); + EXPECT_EQ(false, AK::timing_safe_compare(data_set.characters(), reversed.characters(), reversed.length())); +}