From 5abbd7ba2d56741d79583840410507b54b219477 Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Sat, 20 Jul 2024 08:13:59 -0400 Subject: [PATCH] Refactor internal usage of PK encryption padding Revamp EME class with std::span and CT::Option --- src/fuzzer/oaep.cpp | 61 +++++-------------- src/fuzzer/pkcs1.cpp | 15 ++--- src/lib/pk_pad/eme.cpp | 19 +----- src/lib/pk_pad/eme.h | 68 +++++++-------------- src/lib/pk_pad/eme_oaep/oaep.cpp | 82 +++++++++++++------------ src/lib/pk_pad/eme_oaep/oaep.h | 21 +++---- src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp | 53 ++++++++-------- src/lib/pk_pad/eme_pkcs1/eme_pkcs.h | 11 ++-- src/lib/pk_pad/eme_raw/eme_raw.cpp | 31 +++++++--- src/lib/pk_pad/eme_raw/eme_raw.h | 11 ++-- src/lib/pk_pad/mgf1/mgf1.h | 5 ++ src/lib/pubkey/pk_ops.cpp | 13 +++- src/lib/utils/ct_utils.cpp | 87 ++++++++++++++++----------- src/lib/utils/ct_utils.h | 38 +++++++----- src/lib/utils/int_utils.h | 8 +++ src/tests/test_ct_utils.cpp | 22 +++---- src/tests/test_pk_pad.cpp | 25 ++++---- 17 files changed, 279 insertions(+), 291 deletions(-) diff --git a/src/fuzzer/oaep.cpp b/src/fuzzer/oaep.cpp index 51bfd1d80d2..9a5ee7d8856 100644 --- a/src/fuzzer/oaep.cpp +++ b/src/fuzzer/oaep.cpp @@ -11,77 +11,46 @@ namespace { -Botan::secure_vector ref_oaep_unpad(uint8_t& valid_mask, - const uint8_t in[], - size_t len, - const Botan::secure_vector& Phash) { +Botan::CT::Option ref_oaep_unpad(const uint8_t in[], size_t len, const std::vector& Phash) { const size_t hlen = Phash.size(); if(len < 2 * hlen + 1) { - return Botan::secure_vector(); + return Botan::CT::Option(); } for(size_t i = hlen; i != 2 * hlen; ++i) { if(in[i] != Phash[i - hlen]) { - return Botan::secure_vector(); + return Botan::CT::Option(); } } for(size_t i = 2 * hlen; i != len; ++i) { if(in[i] != 0x00 && in[i] != 0x01) { - return Botan::secure_vector(); + return Botan::CT::Option(); } if(in[i] == 0x01) { - valid_mask = 0xFF; - return Botan::secure_vector(in + i + 1, in + len); + return Botan::CT::Option(i + 1); } } - return Botan::secure_vector(); -} - -inline bool all_zeros(const Botan::secure_vector& v) { - for(size_t i = 0; i != v.size(); ++i) { - if(v[i] != 0) { - return false; - } - } - return true; + return Botan::CT::Option(); } } // namespace void fuzz(const uint8_t in[], size_t len) { - static const Botan::secure_vector Phash = {1, 2, 3, 4}; - - uint8_t lib_valid_mask = 0; - const Botan::secure_vector lib_output = Botan::oaep_find_delim(lib_valid_mask, in, len, Phash); - FUZZER_ASSERT_TRUE(lib_valid_mask == 0 || lib_valid_mask == 0xFF); + static const std::vector Phash = {1, 2, 3, 4}; - uint8_t ref_valid_mask = 0; - const Botan::secure_vector ref_output = ref_oaep_unpad(ref_valid_mask, in, len, Phash); - FUZZER_ASSERT_TRUE(ref_valid_mask == 0 || ref_valid_mask == 0xFF); + auto lib_idx = Botan::oaep_find_delim(std::span{in, len}, std::span{Phash}); - if(ref_valid_mask == 0xFF && lib_valid_mask == 0x00) { - FUZZER_WRITE_AND_CRASH("Ref accepted but library rejected, output " << Botan::hex_encode(ref_output) << "\n"); - } else if(ref_valid_mask == 0x00 && lib_valid_mask == 0xFF) { - FUZZER_WRITE_AND_CRASH("Lib accepted but ref rejected, output = " << Botan::hex_encode(lib_output) << "\n"); - } + auto ref_idx = ref_oaep_unpad(in, len, Phash); - if(ref_valid_mask == 0x00) { - FUZZER_ASSERT_TRUE(all_zeros(ref_output)); - } - - if(lib_valid_mask == 0x00) { - FUZZER_ASSERT_TRUE(all_zeros(lib_output)); - } - - if(ref_valid_mask && lib_valid_mask) { - if(ref_output != lib_output) { - FUZZER_WRITE_AND_CRASH("Ref and lib both accepted but produced different output:" - << " ref = " << Botan::hex_encode(ref_output) - << " lib = " << Botan::hex_encode(lib_output)); - } + if(lib_idx.has_value().as_bool() && ref_idx.has_value().as_bool()) { + FUZZER_ASSERT_EQUAL(lib_idx.value(), ref_idx.value()); + } else if(lib_idx.has_value().as_bool() && !ref_idx.has_value().as_bool()) { + FUZZER_WRITE_AND_CRASH("Ref accepted but lib rejected\n"); + } else if(!lib_idx.has_value().as_bool() && ref_idx.has_value().as_bool()) { + FUZZER_WRITE_AND_CRASH("Lib accepted but ref rejected\n"); } } diff --git a/src/fuzzer/pkcs1.cpp b/src/fuzzer/pkcs1.cpp index 7227c76fb0c..9730cc45048 100644 --- a/src/fuzzer/pkcs1.cpp +++ b/src/fuzzer/pkcs1.cpp @@ -37,21 +37,16 @@ std::vector simple_pkcs1_unpad(const uint8_t in[], size_t len) { void fuzz(const uint8_t in[], size_t len) { static Botan::EME_PKCS1v15 pkcs1; - Botan::secure_vector lib_result; + std::vector lib_result; std::vector ref_result; bool lib_rejected = false, ref_rejected = false; try { - uint8_t valid_mask = 0; - Botan::secure_vector decoded = (static_cast(&pkcs1))->unpad(valid_mask, in, len); + lib_result.resize(len); + auto written = (static_cast(&pkcs1))->unpad(lib_result, std::span{in, len}); + lib_rejected = !written.has_value().as_bool(); - if(valid_mask == 0) { - lib_rejected = true; - } else if(valid_mask == 0xFF) { - lib_rejected = false; - } else { - FUZZER_WRITE_AND_CRASH("Invalid valid_mask from unpad"); - } + lib_result.resize(written.value_or(0)); } catch(Botan::Decoding_Error&) { lib_rejected = true; } diff --git a/src/lib/pk_pad/eme.cpp b/src/lib/pk_pad/eme.cpp index 52a725eac1f..02166e501c4 100644 --- a/src/lib/pk_pad/eme.cpp +++ b/src/lib/pk_pad/eme.cpp @@ -64,23 +64,6 @@ std::unique_ptr EME::create(std::string_view algo_spec) { throw Algorithm_Not_Found(algo_spec); } -/* -* Encode a message -*/ -secure_vector EME::encode(const uint8_t msg[], - size_t msg_len, - size_t key_bits, - RandomNumberGenerator& rng) const { - return pad(msg, msg_len, key_bits, rng); -} - -/* -* Encode a message -*/ -secure_vector EME::encode(const secure_vector& msg, - size_t key_bits, - RandomNumberGenerator& rng) const { - return pad(msg.data(), msg.size(), key_bits, rng); -} +EME::~EME() = default; } // namespace Botan diff --git a/src/lib/pk_pad/eme.h b/src/lib/pk_pad/eme.h index 8d285cb79bb..2644d7d78d2 100644 --- a/src/lib/pk_pad/eme.h +++ b/src/lib/pk_pad/eme.h @@ -1,15 +1,17 @@ /* -* EME Classes -* (C) 1999-2007 Jack Lloyd +* (C) 1999-2007,2024 Jack Lloyd * * Botan is released under the Simplified BSD License (see license.txt) */ -#ifndef BOTAN_PUBKEY_EME_ENCRYPTION_PAD_H_ -#define BOTAN_PUBKEY_EME_ENCRYPTION_PAD_H_ +#ifndef BOTAN_PUBKEY_EME_H_ +#define BOTAN_PUBKEY_EME_H_ -#include -#include +#include +#include +#include +#include +#include namespace Botan { @@ -18,9 +20,9 @@ class RandomNumberGenerator; /** * Encoding Method for Encryption */ -class EME { +class BOTAN_TEST_API EME { public: - virtual ~EME() = default; + virtual ~EME(); /** * Factory method for EME (message-encoding methods for encryption) objects @@ -38,50 +40,26 @@ class EME { /** * Encode an input - * @param in the plaintext - * @param in_length length of plaintext in bytes + * @param output buffer that is written to + * @param input the plaintext * @param key_length length of the key in bits * @param rng a random number generator - * @return encoded plaintext + * @return number of bytes written to output */ - secure_vector encode(const uint8_t in[], - size_t in_length, - size_t key_length, - RandomNumberGenerator& rng) const; - - /** - * Encode an input - * @param in the plaintext - * @param key_length length of the key in bits - * @param rng a random number generator - * @return encoded plaintext - */ - secure_vector encode(const secure_vector& in, - size_t key_length, - RandomNumberGenerator& rng) const; + virtual size_t pad(std::span output, + std::span input, + size_t key_length, + RandomNumberGenerator& rng) const = 0; /** * Decode an input - * @param valid_mask written to specifies if output is valid - * @param in the encoded plaintext - * @param in_len length of encoded plaintext in bytes - * @return bytes of out[] written to along with - * validity mask (0xFF if valid, else 0x00) - */ - virtual secure_vector unpad(uint8_t& valid_mask, const uint8_t in[], size_t in_len) const = 0; - - /** - * Encode an input - * @param in the plaintext - * @param in_length length of plaintext in bytes - * @param key_length length of the key in bits - * @param rng a random number generator - * @return encoded plaintext + * @param output buffer where output is placed + * @param input the encoded plaintext + * @return number of bytes written to output if valid, + * or an empty option if invalid. If an empty option is + * returned the contents of output are undefined */ - virtual secure_vector pad(const uint8_t in[], - size_t in_length, - size_t key_length, - RandomNumberGenerator& rng) const = 0; + virtual CT::Option unpad(std::span output, std::span input) const = 0; }; } // namespace Botan diff --git a/src/lib/pk_pad/eme_oaep/oaep.cpp b/src/lib/pk_pad/eme_oaep/oaep.cpp index 5719c28aae0..6ad39ae8f46 100644 --- a/src/lib/pk_pad/eme_oaep/oaep.cpp +++ b/src/lib/pk_pad/eme_oaep/oaep.cpp @@ -1,6 +1,6 @@ /* * OAEP -* (C) 1999-2010,2015,2018 Jack Lloyd +* (C) 1999-2010,2015,2018,2024 Jack Lloyd * * Botan is released under the Simplified BSD License (see license.txt) */ @@ -18,38 +18,45 @@ namespace Botan { /* * OAEP Pad Operation */ -secure_vector OAEP::pad(const uint8_t in[], - size_t in_length, - size_t key_length, - RandomNumberGenerator& rng) const { +size_t OAEP::pad(std::span output, + std::span input, + size_t key_length, + RandomNumberGenerator& rng) const { key_length /= 8; - if(in_length > maximum_input_size(key_length * 8)) { + if(input.size() > maximum_input_size(key_length * 8)) { throw Invalid_Argument("OAEP: Input is too large"); } - secure_vector out(key_length); - BufferStuffer stuffer(out); + const size_t output_size = key_length; + + output = output.first(output_size); // remainder ignored + + BufferStuffer stuffer(output); // We always use a seed len equal to the underlying hash rng.randomize(stuffer.next(m_Phash.size())); stuffer.append(m_Phash); - stuffer.append(0x00, stuffer.remaining_capacity() - (1 + in_length)); + stuffer.append(0x00, stuffer.remaining_capacity() - (1 + input.size())); stuffer.append(0x01); - stuffer.append({in, in_length}); + stuffer.append(input); BOTAN_ASSERT_NOMSG(stuffer.full()); - mgf1_mask(*m_mgf1_hash, out.data(), m_Phash.size(), &out[m_Phash.size()], out.size() - m_Phash.size()); + const size_t hlen = m_Phash.size(); + + mgf1_mask(*m_mgf1_hash, output.first(hlen), output.subspan(hlen)); - mgf1_mask(*m_mgf1_hash, &out[m_Phash.size()], out.size() - m_Phash.size(), out.data(), m_Phash.size()); + mgf1_mask(*m_mgf1_hash, output.subspan(hlen), output.first(hlen)); - return out; + return key_length; } /* * OAEP Unpad Operation */ -secure_vector OAEP::unpad(uint8_t& valid_mask, const uint8_t in[], size_t in_length) const { +CT::Option OAEP::unpad(std::span output, std::span input) const { + BOTAN_ASSERT_NOMSG(output.size() >= input.size()); + /* Must be careful about error messages here; if an attacker can distinguish them, it is easy to use the differences as an oracle to @@ -70,39 +77,39 @@ secure_vector OAEP::unpad(uint8_t& valid_mask, const uint8_t in[], size Therefore, the first byte should always be zero. */ - const auto leading_0 = CT::Mask::is_zero(in[0]); + if(input.empty()) { + return CT::Option(); + } + + auto scope = CT::scoped_poison(input); + + const auto has_leading_0 = CT::Mask::is_zero(input[0]).as_choice(); - secure_vector input(in + 1, in + in_length); + secure_vector decoded(input.begin() + 1, input.end()); + auto buf = std::span{decoded}; const size_t hlen = m_Phash.size(); - mgf1_mask(*m_mgf1_hash, &input[hlen], input.size() - hlen, input.data(), hlen); + mgf1_mask(*m_mgf1_hash, buf.subspan(hlen), buf.first(hlen)); - mgf1_mask(*m_mgf1_hash, input.data(), hlen, &input[hlen], input.size() - hlen); + mgf1_mask(*m_mgf1_hash, buf.first(hlen), buf.subspan(hlen)); - auto unpadded = oaep_find_delim(valid_mask, input.data(), input.size(), m_Phash); - valid_mask &= leading_0.unpoisoned_value(); - return unpadded; -} + auto delim = oaep_find_delim(buf, m_Phash); -secure_vector oaep_find_delim(uint8_t& valid_mask, - const uint8_t input[], - size_t input_len, - const secure_vector& Phash) { - const size_t hlen = Phash.size(); + return CT::copy_output(delim.has_value() && has_leading_0, output, buf, delim.value_or(0)); +} +CT::Option oaep_find_delim(std::span input, std::span phash) { // Too short to be valid, reject immediately - if(input_len < 1 + 2 * hlen) { - return secure_vector(); + if(input.size() < 1 + 2 * phash.size()) { + return CT::Option(); } - CT::poison(input, input_len); - - size_t delim_idx = 2 * hlen; + size_t delim_idx = 2 * phash.size(); CT::Mask waiting_for_delim = CT::Mask::set(); CT::Mask bad_input_m = CT::Mask::cleared(); - for(size_t i = delim_idx; i < input_len; ++i) { + for(size_t i = delim_idx; i < input.size(); ++i) { const auto zero_m = CT::Mask::is_zero(input[i]); const auto one_m = CT::Mask::is_equal(input[i], 1); @@ -119,16 +126,13 @@ secure_vector oaep_find_delim(uint8_t& valid_mask, bad_input_m |= waiting_for_delim; // If the P hash is wrong, then it's not valid - bad_input_m |= CT::is_not_equal(&input[hlen], Phash.data(), hlen); + bad_input_m |= CT::is_not_equal(&input[phash.size()], phash.data(), phash.size()); delim_idx += 1; - valid_mask = (~bad_input_m).unpoisoned_value(); - auto output = CT::copy_output(bad_input_m, input, input_len, delim_idx); - - CT::unpoison(input, input_len); + const auto accept = !(bad_input_m.as_choice()); - return output; + return CT::Option(delim_idx, accept); } /* diff --git a/src/lib/pk_pad/eme_oaep/oaep.h b/src/lib/pk_pad/eme_oaep/oaep.h index 5200a905de8..a118bddad34 100644 --- a/src/lib/pk_pad/eme_oaep/oaep.h +++ b/src/lib/pk_pad/eme_oaep/oaep.h @@ -1,6 +1,6 @@ /* * OAEP -* (C) 1999-2007,2018 Jack Lloyd +* (C) 1999-2007,2018,2024 Jack Lloyd * * Botan is released under the Simplified BSD License (see license.txt) */ @@ -8,9 +8,11 @@ #ifndef BOTAN_OAEP_H_ #define BOTAN_OAEP_H_ -#include #include +#include +#include + namespace Botan { /** @@ -35,21 +37,18 @@ class OAEP final : public EME { OAEP(std::unique_ptr hash, std::unique_ptr mgf1_hash, std::string_view P = ""); private: - secure_vector pad(const uint8_t in[], - size_t in_length, - size_t key_length, - RandomNumberGenerator& rng) const override; + size_t pad(std::span output, + std::span input, + size_t key_length, + RandomNumberGenerator& rng) const override; - secure_vector unpad(uint8_t& valid_mask, const uint8_t in[], size_t in_len) const override; + CT::Option unpad(std::span output, std::span input) const override; secure_vector m_Phash; std::unique_ptr m_mgf1_hash; }; -secure_vector BOTAN_TEST_API oaep_find_delim(uint8_t& valid_mask, - const uint8_t input[], - size_t input_len, - const secure_vector& Phash); +BOTAN_FUZZER_API CT::Option oaep_find_delim(std::span input, std::span phash); } // namespace Botan diff --git a/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp b/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp index 4fc9d33bab0..4ac8ca05b6e 100644 --- a/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp +++ b/src/lib/pk_pad/eme_pkcs1/eme_pkcs.cpp @@ -1,6 +1,6 @@ /* * PKCS #1 v1.5 Type 2 (encryption) padding -* (C) 1999-2007,2015,2016 Jack Lloyd +* (C) 1999-2007,2015,2016,2024 Jack Lloyd * * Botan is released under the Simplified BSD License (see license.txt) */ @@ -11,6 +11,7 @@ #include #include #include +#include #include namespace Botan { @@ -18,58 +19,61 @@ namespace Botan { /* * PKCS1 Pad Operation */ -secure_vector EME_PKCS1v15::pad(const uint8_t in[], - size_t inlen, - size_t key_length, - RandomNumberGenerator& rng) const { +size_t EME_PKCS1v15::pad(std::span output, + std::span input, + size_t key_length, + RandomNumberGenerator& rng) const { key_length /= 8; - if(inlen > maximum_input_size(key_length * 8)) { + if(input.size() > maximum_input_size(key_length * 8)) { throw Invalid_Argument("PKCS1: Input is too large"); } - secure_vector out(key_length); - BufferStuffer stuffer(out); + BufferStuffer stuffer(output); - const size_t padding_bytes = key_length - inlen - 2; + const size_t padding_bytes = [&]() { + auto d = checked_sub(key_length, input.size() + 2); + BOTAN_ASSERT_NOMSG(d.has_value()); + return *d; + }(); stuffer.append(0x02); for(size_t i = 0; i != padding_bytes; ++i) { stuffer.append(rng.next_nonzero_byte()); } stuffer.append(0x00); - stuffer.append({in, inlen}); - BOTAN_ASSERT_NOMSG(stuffer.full()); + stuffer.append(input); - return out; + return output.size() - stuffer.remaining_capacity(); } /* * PKCS1 Unpad Operation */ -secure_vector EME_PKCS1v15::unpad(uint8_t& valid_mask, const uint8_t in[], size_t inlen) const { +CT::Option EME_PKCS1v15::unpad(std::span output, std::span input) const { + BOTAN_ASSERT_NOMSG(output.size() >= input.size()); + /* * RSA decryption pads the ciphertext up to the modulus size, so this only * occurs with very (!) small keys, or when fuzzing. * * 11 bytes == 00,02 + 8 bytes mandatory padding + 00 */ - if(inlen < 11) { - valid_mask = false; - return secure_vector(); + if(input.size() < 11) { + return CT::Option(); } - CT::poison(in, inlen); + auto scope = CT::scoped_poison(input); CT::Mask bad_input_m = CT::Mask::cleared(); CT::Mask seen_zero_m = CT::Mask::cleared(); size_t delim_idx = 2; // initial 0002 - bad_input_m |= ~CT::Mask::is_equal(in[0], 0); - bad_input_m |= ~CT::Mask::is_equal(in[1], 2); + bad_input_m |= ~CT::Mask::is_equal(input[0], 0); + bad_input_m |= ~CT::Mask::is_equal(input[1], 2); - for(size_t i = 2; i < inlen; ++i) { - const auto is_zero_m = CT::Mask::is_zero(in[i]); + for(size_t i = 2; i < input.size(); ++i) { + const auto is_zero_m = CT::Mask::is_zero(input[i]); delim_idx += seen_zero_m.if_not_set_return(1); seen_zero_m |= is_zero_m; } @@ -83,12 +87,9 @@ secure_vector EME_PKCS1v15::unpad(uint8_t& valid_mask, const uint8_t in */ bad_input_m |= CT::Mask(CT::Mask::is_lt(delim_idx, 11)); - valid_mask = (~bad_input_m).unpoisoned_value(); - auto output = CT::copy_output(bad_input_m, in, inlen, delim_idx); - - CT::unpoison(in, inlen); + const CT::Choice accept = !(bad_input_m.as_choice()); - return output; + return CT::copy_output(accept, output, input, delim_idx); } /* diff --git a/src/lib/pk_pad/eme_pkcs1/eme_pkcs.h b/src/lib/pk_pad/eme_pkcs1/eme_pkcs.h index c21041cf626..bb3a3447b03 100644 --- a/src/lib/pk_pad/eme_pkcs1/eme_pkcs.h +++ b/src/lib/pk_pad/eme_pkcs1/eme_pkcs.h @@ -15,13 +15,16 @@ namespace Botan { /** * EME from PKCS #1 v1.5 */ -class BOTAN_TEST_API EME_PKCS1v15 final : public EME { - public: +class BOTAN_FUZZER_API EME_PKCS1v15 final : public EME { + private: size_t maximum_input_size(size_t) const override; - secure_vector pad(const uint8_t[], size_t, size_t, RandomNumberGenerator&) const override; + size_t pad(std::span output, + std::span input, + size_t key_length, + RandomNumberGenerator& rng) const override; - secure_vector unpad(uint8_t& valid_mask, const uint8_t in[], size_t in_len) const override; + CT::Option unpad(std::span output, std::span input) const override; }; } // namespace Botan diff --git a/src/lib/pk_pad/eme_raw/eme_raw.cpp b/src/lib/pk_pad/eme_raw/eme_raw.cpp index 4e6fd19f86e..f707cb3355c 100644 --- a/src/lib/pk_pad/eme_raw/eme_raw.cpp +++ b/src/lib/pk_pad/eme_raw/eme_raw.cpp @@ -1,29 +1,40 @@ /* -* (C) 2015,2016 Jack Lloyd +* (C) 2015,2016,2024 Jack Lloyd * * Botan is released under the Simplified BSD License (see license.txt) */ #include -#include +#include #include namespace Botan { -secure_vector EME_Raw::pad(const uint8_t in[], - size_t in_length, - size_t /*key_length*/, - RandomNumberGenerator& /*rng*/) const { - return secure_vector(in, in + in_length); +size_t EME_Raw::pad(std::span output, + std::span input, + size_t key_length, + RandomNumberGenerator& rng) const { + BOTAN_UNUSED(rng); + BOTAN_ASSERT_NOMSG(input.size() < maximum_input_size(8 * key_length)); + BOTAN_ASSERT_NOMSG(output.size() >= input.size()); + copy_mem(std::span{output.data(), input.size()}, input); + return input.size(); } -secure_vector EME_Raw::unpad(uint8_t& valid_mask, const uint8_t in[], size_t in_length) const { - valid_mask = 0xFF; - return CT::strip_leading_zeros(in, in_length); +CT::Option EME_Raw::unpad(std::span output, std::span input) const { + BOTAN_ASSERT_NOMSG(output.size() >= input.size()); + + if(input.empty()) { + return CT::Option(0); + } + + const size_t leading_zeros = CT::count_leading_zero_bytes(input); + return CT::copy_output(CT::Choice::yes(), output, input, leading_zeros); } size_t EME_Raw::maximum_input_size(size_t keybits) const { return keybits / 8; } + } // namespace Botan diff --git a/src/lib/pk_pad/eme_raw/eme_raw.h b/src/lib/pk_pad/eme_raw/eme_raw.h index 728f308d2f1..3064cf5d0ee 100644 --- a/src/lib/pk_pad/eme_raw/eme_raw.h +++ b/src/lib/pk_pad/eme_raw/eme_raw.h @@ -13,14 +13,17 @@ namespace Botan { class EME_Raw final : public EME { public: - size_t maximum_input_size(size_t i) const override; - EME_Raw() = default; private: - secure_vector pad(const uint8_t[], size_t, size_t, RandomNumberGenerator&) const override; + size_t maximum_input_size(size_t i) const override; + + size_t pad(std::span output, + std::span input, + size_t key_length, + RandomNumberGenerator& rng) const override; - secure_vector unpad(uint8_t& valid_mask, const uint8_t in[], size_t in_len) const override; + CT::Option unpad(std::span output, std::span input) const override; }; } // namespace Botan diff --git a/src/lib/pk_pad/mgf1/mgf1.h b/src/lib/pk_pad/mgf1/mgf1.h index d8c86e8e7e2..e12265915dc 100644 --- a/src/lib/pk_pad/mgf1/mgf1.h +++ b/src/lib/pk_pad/mgf1/mgf1.h @@ -9,6 +9,7 @@ #define BOTAN_MGF1_H_ #include +#include namespace Botan { @@ -24,6 +25,10 @@ class HashFunction; */ void mgf1_mask(HashFunction& hash, const uint8_t in[], size_t in_len, uint8_t out[], size_t out_len); +inline void mgf1_mask(HashFunction& hash, std::span input, std::span output) { + mgf1_mask(hash, input.data(), input.size(), output.data(), output.size()); +} + } // namespace Botan #endif diff --git a/src/lib/pubkey/pk_ops.cpp b/src/lib/pubkey/pk_ops.cpp index f81a8cf2543..e18bf32ae30 100644 --- a/src/lib/pubkey/pk_ops.cpp +++ b/src/lib/pubkey/pk_ops.cpp @@ -35,8 +35,9 @@ secure_vector PK_Ops::Encryption_with_EME::encrypt(const uint8_t msg[], size_t msg_len, RandomNumberGenerator& rng) { const size_t max_raw = max_ptext_input_bits(); - const auto encoded = m_eme->encode(msg, msg_len, max_raw, rng); - return raw_encrypt(encoded.data(), encoded.size(), rng); + secure_vector eme_output((max_raw + 7) / 8); + size_t written = m_eme->pad(eme_output, std::span{msg, msg_len}, max_raw, rng); + return raw_encrypt(eme_output.data(), written, rng); } PK_Ops::Decryption_with_EME::Decryption_with_EME(std::string_view eme) : m_eme(EME::create(eme)) {} @@ -45,7 +46,13 @@ secure_vector PK_Ops::Decryption_with_EME::decrypt(uint8_t& valid_mask, const uint8_t ciphertext[], size_t ciphertext_len) { const secure_vector raw = raw_decrypt(ciphertext, ciphertext_len); - return m_eme->unpad(valid_mask, raw.data(), raw.size()); + + secure_vector ctext(raw.size()); + auto len = m_eme->unpad(ctext, raw); + + valid_mask = CT::Mask::from_choice(len.has_value()).if_set_return(0xFF); + ctext.resize(len.value_or(0)); + return ctext; } PK_Ops::Key_Agreement_with_KDF::Key_Agreement_with_KDF(std::string_view kdf) { diff --git a/src/lib/utils/ct_utils.cpp b/src/lib/utils/ct_utils.cpp index a0d60a5377c..e843cbdfd91 100644 --- a/src/lib/utils/ct_utils.cpp +++ b/src/lib/utils/ct_utils.cpp @@ -6,44 +6,50 @@ #include -namespace Botan::CT { +#include -secure_vector copy_output(CT::Mask bad_input_u8, - const uint8_t input[], - size_t input_length, +namespace Botan { + +CT::Option CT::copy_output(CT::Choice accept, + std::span output, + std::span input, size_t offset) { + // This leaks information about the input length, but this happens + // unavoidably since we are unable to ready any bytes besides those + // in input[0..n] + BOTAN_ARG_CHECK(output.size() >= input.size(), "Invalid span lengths"); + /* * We do not poison the input here because if we did we would have * to unpoison it at exit. We assume instead that callers have * already poisoned the input and will unpoison it at their own * time. */ - CT::poison(&offset, sizeof(size_t)); - - secure_vector output(input_length); + CT::poison(offset); - auto bad_input = CT::Mask::expand(bad_input_u8); + /** + * Zeroize the entire output buffer to get started + */ + clear_mem(output); /* - * If the offset is greater than input_length then the arguments are - * invalid. Ideally we would through an exception but that leaks + * If the offset is greater than input length, then the arguments are + * invalid. Ideally we would throw an exception, but that leaks * information about the offset. Instead treat it as if the input * was invalid. */ - bad_input |= CT::Mask::is_gt(offset, input_length); + accept = accept && CT::Mask::is_lte(offset, input.size()).as_choice(); /* - * If the input is invalid, then set offset == input_length as a result - * at the end we will set output_bytes == 0 causing the final result to - * be an empty vector. + * If the input is invalid, then set offset == input_length */ - offset = bad_input.select(input_length, offset); + offset = CT::Mask::from_choice(accept).select(offset, input.size()); /* - Move the desired output bytes to the front using a slow (O^n) - but constant time loop that does not leak the value of the offset + * Move the desired output bytes to the front using a slow (O^n) + * but constant time loop that does not leak the value of the offset */ - for(size_t i = 0; i != input_length; ++i) { + for(size_t i = 0; i != input.size(); ++i) { /* * If bad_input was set then we modified offset to equal the input_length. * In that case, this_loop will be greater than input_length, and so is_eq @@ -52,7 +58,7 @@ secure_vector copy_output(CT::Mask bad_input_u8, * * This is ignoring the possibility of integer overflow of offset + i. But * for this to happen the input would have to consume nearly the entire - * address space, and we just allocated an output buffer of equal size. + * address space. */ const size_t this_loop = offset + i; @@ -60,38 +66,47 @@ secure_vector copy_output(CT::Mask bad_input_u8, start index from i rather than 0 since we know j must be >= i + offset to have any effect, and starting from i does not reveal information */ - for(size_t j = i; j != input_length; ++j) { + for(size_t j = i; j != input.size(); ++j) { const uint8_t b = input[j]; const auto is_eq = CT::Mask::is_equal(j, this_loop); output[i] |= is_eq.if_set_return(b); } } - const size_t output_bytes = input_length - offset; + // This will always be zero if the input was invalid + const size_t output_bytes = input.size() - offset; - CT::unpoison(output.data(), output.size()); + CT::unpoison(output); CT::unpoison(output_bytes); - /* - This is potentially not const time, depending on how std::vector is - implemented. But since we are always reducing length, it should - just amount to setting the member var holding the length. - */ - output.resize(output_bytes); - return output; + return CT::Option(output_bytes, accept); } -secure_vector strip_leading_zeros(const uint8_t in[], size_t length) { +size_t CT::count_leading_zero_bytes(std::span input) { size_t leading_zeros = 0; - auto only_zeros = Mask::set(); - - for(size_t i = 0; i != length; ++i) { - only_zeros &= CT::Mask::is_zero(in[i]); + for(size_t i = 0; i != input.size(); ++i) { + only_zeros &= CT::Mask::is_zero(input[i]); leading_zeros += only_zeros.if_set_return(1); } + return leading_zeros; +} + +secure_vector CT::strip_leading_zeros(std::span input) { + const size_t leading_zeros = CT::count_leading_zero_bytes(input); + + secure_vector output(input.size()); - return copy_output(CT::Mask::cleared(), in, length, leading_zeros); + const auto written = CT::copy_output(CT::Choice::yes(), output, input, leading_zeros); + + /* + This is potentially not const time, depending on how std::vector is + implemented. But since we are always reducing length, it should + just amount to setting the member var holding the length. + */ + output.resize(written.value_or(0)); + + return output; } -} // namespace Botan::CT +} // namespace Botan diff --git a/src/lib/utils/ct_utils.h b/src/lib/utils/ct_utils.h index 32e06cb606f..b803abf5960 100644 --- a/src/lib/utils/ct_utils.h +++ b/src/lib/utils/ct_utils.h @@ -635,6 +635,9 @@ class Option { } } + /// Return a new CT::Choice that is true if also is set as well + constexpr CT::Option operator&&(CT::Choice also) { return CT::Option(m_value, m_has_value && also); } + private: Choice m_has_value; T m_value; @@ -731,28 +734,33 @@ constexpr inline CT::Mask is_not_equal(const T x[], const T y[], size_t len) } /** -* If bad_input is unset, return input[offset:input_length] copied to new -* buffer. If bad_input is set, return an empty vector. In all cases, the capacity -* of the vector is equal to input_length +* Constant time conditional copy out with offset +* +* If accept is set and offset <= input_length, sets output[0..] to +* input[offset:input_length] and returns input_length - offset. The +* remaining bytes of output are zeroized. +* +* Otherwise, output is zeroized, and returns an empty Ct::Option * -* This function attempts to avoid leaking the following: -* - if bad_input was set or not +* The input and output spans may not overlap, and output must be at +* least as large as input. +* +* This function attempts to avoid leaking the following to side channels +* - if accept was set or not * - the value of offset -* - the values in input[] +* - the value of input * -* This function leaks the value of input_length +* This function leaks the length of the input */ BOTAN_TEST_API -secure_vector copy_output(CT::Mask bad_input, - const uint8_t input[], - size_t input_length, - size_t offset); +CT::Option copy_output(CT::Choice accept, + std::span output, + std::span input, + size_t offset); -secure_vector strip_leading_zeros(const uint8_t in[], size_t length); +size_t count_leading_zero_bytes(std::span input); -inline secure_vector strip_leading_zeros(const secure_vector& in) { - return strip_leading_zeros(in.data(), in.size()); -} +secure_vector strip_leading_zeros(std::span input); } // namespace Botan::CT diff --git a/src/lib/utils/int_utils.h b/src/lib/utils/int_utils.h index 2dc827b68cb..c4265746817 100644 --- a/src/lib/utils/int_utils.h +++ b/src/lib/utils/int_utils.h @@ -24,6 +24,14 @@ constexpr inline std::optional checked_add(T a, T b) { return r; } +template +constexpr inline std::optional checked_sub(T a, T b) { + if(b > a) { + return {}; + } + return a - b; +} + template requires all_same_v constexpr inline std::optional checked_add(T a, T b, Ts... rest) { diff --git a/src/tests/test_ct_utils.cpp b/src/tests/test_ct_utils.cpp index f6943adea7f..01df21b32a5 100644 --- a/src/tests/test_ct_utils.cpp +++ b/src/tests/test_ct_utils.cpp @@ -7,7 +7,6 @@ #include "tests.h" #include #include -#include namespace Botan_Tests { @@ -43,26 +42,27 @@ class CT_Mask_Tests final : public Test { result.test_eq_sz("CT::is_less32", Botan::CT::Mask::is_lt(5, 0xFFFFFFFF).value(), 0xFFFFFFFF); for(auto bad_input : {0, 1}) { - for(size_t input_length : {0, 1, 2, 32}) { - for(size_t offset = 0; offset != input_length + 1; ++offset) { - const auto mask = Botan::CT::Mask::expand(static_cast(bad_input)); + for(size_t input_length = 0; input_length != 64; ++input_length) { + for(size_t offset = 0; offset != input_length + 5; ++offset) { + const auto accept = !Botan::CT::Choice::from_int(static_cast(bad_input)); std::vector input(input_length); this->rng().randomize(input.data(), input.size()); - auto output = Botan::CT::copy_output(mask, input.data(), input.size(), offset); + std::vector output(input_length); - result.test_eq_sz("CT::copy_output capacity", output.capacity(), input.size()); + auto written = Botan::CT::copy_output(accept, output, input, offset); if(bad_input) { - result.confirm("If bad input, no output", output.empty()); + result.confirm("If bad input, no output", !written.has_value().as_bool()); } else { - if(offset >= input_length) { - result.confirm("If offset is too large, output is empty", output.empty()); + if(offset > input_length) { + result.confirm("If offset is too large, no output", !written.has_value().as_bool()); } else { - result.test_eq_sz("CT::copy_output length", output.size(), input.size() - offset); + const size_t bytes = written.value(); + result.test_eq_sz("CT::copy_output length", bytes, input.size() - offset); - for(size_t i = 0; i != output.size(); ++i) { + for(size_t i = 0; i != bytes; ++i) { result.test_eq_sz("CT::copy_output offset", output[i], input[i + offset]); } } diff --git a/src/tests/test_pk_pad.cpp b/src/tests/test_pk_pad.cpp index 673523e0469..e639fcd39e6 100644 --- a/src/tests/test_pk_pad.cpp +++ b/src/tests/test_pk_pad.cpp @@ -7,18 +7,14 @@ #include "tests.h" #if defined(BOTAN_HAS_PK_PADDING) + #include #include + #include #endif -#if defined(BOTAN_HAS_EME_PKCS1) - #include -#endif - -#include - namespace Botan_Tests { -#if defined(BOTAN_HAS_EME_PKCS1) +#if defined(BOTAN_HAS_PK_PADDING) class EME_PKCS1v15_Decoding_Tests final : public Text_Based_Test { public: @@ -29,7 +25,10 @@ class EME_PKCS1v15_Decoding_Tests final : public Text_Based_Test { Test::Result result("PKCSv15 Decoding"); - Botan::EME_PKCS1v15 pkcs; + auto pkcs = Botan::EME::create("PKCS1v15"); + if(!pkcs) { + return result; + } const std::vector ciphertext = vars.get_req_bin("RawCiphertext"); const std::vector plaintext = vars.get_opt_bin("Plaintext"); @@ -38,13 +37,13 @@ class EME_PKCS1v15_Decoding_Tests final : public Text_Based_Test { result.test_eq("Plaintext value should be empty for invalid EME inputs", plaintext.size(), 0); } - uint8_t valid_mask = 0; - Botan::secure_vector decoded = pkcs.unpad(valid_mask, ciphertext.data(), ciphertext.size()); + std::vector decoded(ciphertext.size()); + auto len = pkcs->unpad(decoded, ciphertext); - result.confirm("EME valid_mask has expected value", valid_mask == 0x00 || valid_mask == 0xFF); - result.test_eq("EME decoding valid/invalid matches", valid_mask == 0xFF, is_valid); + result.test_eq("EME decoding valid/invalid matches", len.has_value().as_bool(), is_valid); - if(valid_mask == 0xFF) { + if(len.has_value().as_bool()) { + decoded.resize(len.value_or(0)); result.test_eq("EME decoded plaintext correct", decoded, plaintext); } else { bool all_zeros = true;