From f3a63dd1f856842e7e390000f38513bdf0259dd5 Mon Sep 17 00:00:00 2001 From: Rene Meusel Date: Mon, 25 Mar 2024 17:14:23 +0100 Subject: [PATCH 1/4] Introduce BufferTransformer --- src/lib/utils/buffer_transformer.h | 61 ++++++++++++++++++++++++++++++ src/lib/utils/info.txt | 1 + src/lib/utils/stl_util.h | 1 + 3 files changed, 63 insertions(+) create mode 100644 src/lib/utils/buffer_transformer.h diff --git a/src/lib/utils/buffer_transformer.h b/src/lib/utils/buffer_transformer.h new file mode 100644 index 00000000000..cb918a12021 --- /dev/null +++ b/src/lib/utils/buffer_transformer.h @@ -0,0 +1,61 @@ +/* + * Buffer Transformer + * (C) 2024 Jack Lloyd + * 2024 René Meusel - Rohde & Schwarz Cybersecurity + * + * Botan is released under the Simplified BSD License (see license.txt) + */ + +#ifndef BOTAN_BUFFER_TRANSFORMER_H_ +#define BOTAN_BUFFER_TRANSFORMER_H_ + +#include + +namespace Botan { + +/** + * @brief Helper class combining the BufferStuffer and BufferSlicer + * + * Use this class to transform data from one buffer to another in a streaming + * fashion. The input and output buffers must be of the same size. + */ +class BufferTransformer { + public: + template + using ispan = std::span; + + template + using ospan = std::span; + + public: + BufferTransformer(ispan<> in, ospan<> out) : m_in(in), m_out(out) { + BOTAN_ARG_CHECK(in.size() == out.size(), "Input and output buffers must be the same size"); + } + + std::pair, ospan<>> next(size_t count) { return {m_in.take(count), m_out.next(count)}; } + + template + std::pair, ospan> next() { + return {m_in.take(), m_out.next()}; + } + + void skip(const size_t count) { next(count); } + + size_t remaining() const { + BOTAN_DEBUG_ASSERT(m_in.remaining() == m_out.remaining_capacity()); + return m_in.remaining(); + } + + bool done() const { + BOTAN_DEBUG_ASSERT(m_in.empty() == m_out.full()); + return m_in.empty(); + } + + private: + BufferSlicer m_in; + BufferStuffer m_out; +}; + +} // namespace Botan + +#endif diff --git a/src/lib/utils/info.txt b/src/lib/utils/info.txt index 4c4fcc733f5..a0981d2bfb1 100644 --- a/src/lib/utils/info.txt +++ b/src/lib/utils/info.txt @@ -28,6 +28,7 @@ version.h alignment_buffer.h bit_ops.h bswap.h +buffer_transformer.h calendar.h charset.h codec_base.h diff --git a/src/lib/utils/stl_util.h b/src/lib/utils/stl_util.h index c22899a5be3..50ed8ca114a 100644 --- a/src/lib/utils/stl_util.h +++ b/src/lib/utils/stl_util.h @@ -19,6 +19,7 @@ #include #include +#include #include #include #include From df205287c4e817c2a12dfa62b62db6d22b70f571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Meusel?= Date: Wed, 26 Jun 2024 15:24:31 +0200 Subject: [PATCH 2/4] generalize BufferTransformer for block-wise processing --- src/lib/utils/buffer_transformer.h | 133 ++++++++++++-- src/tests/test_utils_buffer.cpp | 268 ++++++++++++++++++++++++++++- 2 files changed, 386 insertions(+), 15 deletions(-) diff --git a/src/lib/utils/buffer_transformer.h b/src/lib/utils/buffer_transformer.h index cb918a12021..c8a4f514064 100644 --- a/src/lib/utils/buffer_transformer.h +++ b/src/lib/utils/buffer_transformer.h @@ -9,37 +9,131 @@ #ifndef BOTAN_BUFFER_TRANSFORMER_H_ #define BOTAN_BUFFER_TRANSFORMER_H_ +#include #include namespace Botan { +namespace detail { + +template +using ispan = std::span; + +template +using ospan = std::span; + +template +concept block_processing_callback = (std::invocable, ospan> && ...); + +template +consteval bool is_strictly_monotonic_decreasing() { + return true; +} + +template +consteval bool is_strictly_monotonic_decreasing() { + return (biggest > bigger) && is_strictly_monotonic_decreasing(); +} + /** - * @brief Helper class combining the BufferStuffer and BufferSlicer + * Ensures that the sequence of block_sizes is strictly becoming smaller + * A sequence of length 1 or the empty sequence satisfy this by definition. + */ +template +concept strictly_monotonic_decreasing = is_strictly_monotonic_decreasing(); + +} // namespace detail + +/** + * @brief Process data from an input buffer into an output buffer * * Use this class to transform data from one buffer to another in a streaming * fashion. The input and output buffers must be of the same size. */ class BufferTransformer { public: - template - using ispan = std::span; - - template - using ospan = std::span; - - public: - BufferTransformer(ispan<> in, ospan<> out) : m_in(in), m_out(out) { - BOTAN_ARG_CHECK(in.size() == out.size(), "Input and output buffers must be the same size"); + template + requires(std::same_as, uint8_t> && + std::same_as, uint8_t>) + BufferTransformer(InT&& in, OutT&& out) : + m_in(detail::ispan<>(in)), m_out(detail::ospan<>(out)), m_block_processing(false) { + ranges::assert_equal_byte_lengths(in, out); } - std::pair, ospan<>> next(size_t count) { return {m_in.take(count), m_out.next(count)}; } + /** + * @returns a pair of input and output spans of the given byte @p count + */ + std::pair, detail::ospan<>> next(size_t count) { + BOTAN_STATE_CHECK(!m_block_processing); + return {m_in.take(count), m_out.next(count)}; + } + /** + * @returns a pair of input and output spans with static extent + */ template - std::pair, ospan> next() { + std::pair, detail::ospan> next() { + BOTAN_STATE_CHECK(!m_block_processing); return {m_in.take(), m_out.next()}; } - void skip(const size_t count) { next(count); } + /** + * Skips the next @p count bytes in both input and output buffers + */ + void skip(const size_t count) { + BOTAN_STATE_CHECK(!m_block_processing); + next(count); + } + + /** + * @brief Block-wise processing of the input into the output buffer + * + * Specify the desired block size(s) in strictly decreasing order as + * template parameter(s). This will process the input buffer in the + * largest possible block size and fall back to smaller ones as necessary. + * + * The provided callback @p fn is expected to be a callable object that + * accepts input/output spans of all given block sizes. Typically this is + * done with the `overloaded{}` helper or a generic lambda. E.g. + * + * @code + * transformer.process_blocks_of<64, 32, 16>(overloaded{ + * [](std::span, std::span) { ... }, + * [](std::span, std::span) { ... }, + * [](std::span, std::span) { ... }}); + * + * transformer.process_blocks_of<64, 32, 16>([](auto in, auto out) { + * if constexpr(in.size() == 64) { ... } + * else if constexpr(in.size() == 32) { ... } + * else if constexpr(in.size() == 16) { ... } + * }); + * @endcode + * + * @note This will always transform the entire buffer. Consuming data via + * any other method is prohibited once block processing has started. + */ + template + requires(sizeof...(block_sizes) > 0) && (detail::strictly_monotonic_decreasing) + void process_blocks_of(detail::block_processing_callback auto&& fn) { + BOTAN_STATE_CHECK(!m_block_processing); + m_block_processing = true; + + constexpr size_t smallest_block_size = std::min({block_sizes...}); + // If there is a 1-byte block size, we can trivially process any byte + // range, else we validate that the range will be fully processible by + // the given sequence of block sizes. + if constexpr(smallest_block_size > 1) { + size_t bytes_to_process = remaining(); + ([&](size_t block_size) { bytes_to_process = bytes_to_process % block_size; }(block_sizes), ...); + BOTAN_ARG_CHECK(bytes_to_process == 0, "Input size cannot be block-processed"); + } + + // Process the range in blocks of the given sizes, falling back to + // the smaller blocks as needed. + (opportunistically_process_blocks_of(fn), ...); + + BOTAN_DEBUG_ASSERT(done()); + } size_t remaining() const { BOTAN_DEBUG_ASSERT(m_in.remaining() == m_out.remaining_capacity()); @@ -51,9 +145,22 @@ class BufferTransformer { return m_in.empty(); } + private: + /** + * @brief Process as many blocks of the given size as possible + */ + template + void opportunistically_process_blocks_of(detail::block_processing_callback auto& fn) { + BOTAN_DEBUG_ASSERT(m_block_processing); + while(remaining() >= block_size) { + fn(m_in.take(), m_out.next()); + } + } + private: BufferSlicer m_in; BufferStuffer m_out; + bool m_block_processing = false; }; } // namespace Botan diff --git a/src/tests/test_utils_buffer.cpp b/src/tests/test_utils_buffer.cpp index df7c4fbea58..630d3f62778 100644 --- a/src/tests/test_utils_buffer.cpp +++ b/src/tests/test_utils_buffer.cpp @@ -10,6 +10,8 @@ #include #include #include +#include +#include #include #include @@ -664,8 +666,270 @@ std::vector test_concat() { }; } -BOTAN_REGISTER_TEST_FN( - "utils", "buffer_utilities", test_buffer_slicer, test_buffer_stuffer, test_alignment_buffer, test_concat); +std::vector test_buffer_transformer() { + return { + Botan_Tests::CHECK("BufferTransformer basics with arrays", + [](Test::Result& result) { + const std::array input = {1, 2, 3, 4, 5, 6, 7, 8}; + std::array output = {}; + + Botan::BufferTransformer bt(input, output); + auto n1 = bt.next(2); + result.require("got requested in bytes (1)", n1.first.size() == 2); + result.require("got requested out bytes (1)", n1.second.size() == 2); + result.confirm("dynamic extent in", decltype(n1.first)::extent == std::dynamic_extent); + result.confirm("dynamic extent out", decltype(n1.second)::extent == std::dynamic_extent); + Botan::copy_mem(n1.second, n1.first); + + auto n2 = bt.next<5>(); + result.require("got requested in bytes (2)", n2.first.size() == 5); + result.require("got requested out bytes (2)", n2.second.size() == 5); + result.confirm("static extent in", decltype(n2.first)::extent == 5); + result.confirm("static extent out", decltype(n2.second)::extent == 5); + Botan::copy_mem(n2.second, n2.first); + + result.require("not done yet", !bt.done()); + result.test_eq("has one more bytes", bt.remaining(), 1); + + bt.skip(1); + result.require("done now", bt.done()); + result.test_eq("has no more bytes", bt.remaining(), 0); + + result.test_is_eq("output is as expected", output, {1, 2, 3, 4, 5, 6, 7, 0 /* skipped */}); + + result.test_no_throw("empty request is okay (dynamic)", [&] { bt.next(0); }); + result.test_no_throw("empty request is okay (static)", [&] { bt.next<0>(); }); + result.test_no_throw("empty skipping request is okay", [&] { bt.skip(0); }); + result.test_throws("data request will fail (1)", [&] { bt.next(1); }); + result.test_throws("data request will fail (2)", [&] { bt.next<1>(); }); + result.test_throws("data skipping request will fail", [&] { bt.skip(1); }); + }), + + Botan_Tests::CHECK("BufferTransformer basics with vectors", + [](Test::Result& result) { + const std::vector input = {1, 2, 3, 4, 5, 6, 7, 8}; + std::vector output(input.size()); + + Botan::BufferTransformer bt(input, output); + auto n1 = bt.next(4); + result.require("got requested in bytes (1)", n1.first.size() == 4); + result.require("got requested out bytes (1)", n1.second.size() == 4); + result.confirm("dynamic extent in", decltype(n1.first)::extent == std::dynamic_extent); + result.confirm("dynamic extent out", decltype(n1.second)::extent == std::dynamic_extent); + Botan::copy_mem(n1.second, n1.first); + + bt.skip(2); + result.require("not done yet", !bt.done()); + result.test_eq("has two more bytes", bt.remaining(), 2); + + auto n2 = bt.next<2>(); + result.require("got requested in bytes (2)", n2.first.size() == 2); + result.require("got requested out bytes (2)", n2.second.size() == 2); + result.confirm("static extent in", decltype(n2.first)::extent == 2); + result.confirm("static extent out", decltype(n2.second)::extent == 2); + Botan::copy_mem(n2.second, n2.first); + + result.require("done now", bt.done()); + result.test_eq("has no more bytes", bt.remaining(), 0); + + result.test_is_eq("output is as expected", output, {1, 2, 3, 4, 0, 0, 7, 8}); + + result.test_no_throw("empty request is okay (dynamic)", [&] { bt.next(0); }); + result.test_no_throw("empty request is okay (static)", [&] { bt.next<0>(); }); + result.test_no_throw("empty skipping request is okay", [&] { bt.skip(0); }); + result.test_throws("data request will fail (1)", [&] { bt.next(1); }); + result.test_throws("data request will fail (2)", [&] { bt.next<1>(); }); + result.test_throws("data skipping request will fail", [&] { bt.skip(1); }); + }), + + Botan_Tests::CHECK("BufferTransformer::process_blocks_of() with a single block", + [](Test::Result& result) { + const auto in = + Botan::hex_decode("000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F"); + std::vector out(in.size()); + + Botan::BufferTransformer bt(in, out); + bt.process_blocks_of<8>( + [](std::span i, std::span o) { Botan::copy_mem(o, i); }); + + result.require("done now", bt.done()); + result.test_is_eq("output is as expected", out, in); + }), + + Botan_Tests::CHECK("concept strictly_monotonic_decreasing", + [](Test::Result& result) { + result.confirm("empty is strictly monotonic decreasing by definition", + Botan::detail::strictly_monotonic_decreasing<>); + result.confirm("single element is strictly monotonic decreasing by definition", + Botan::detail::strictly_monotonic_decreasing<1>); + result.confirm("strictly monotonic decreasing", + Botan::detail::strictly_monotonic_decreasing<3, 2, 1>); + result.confirm("not strictly monotonic decreasing because 1 < 2", + !Botan::detail::strictly_monotonic_decreasing<3, 1, 2>); + result.confirm("not strictly monotonic decreasing because 2 == 2", + !Botan::detail::strictly_monotonic_decreasing<3, 2, 2>); + }), + + Botan_Tests::CHECK( + "concept block_processing_callback", + [](Test::Result& result) { + auto takes_8 = [](std::span, std::span) {}; + auto takes_dynamic = [](std::span, std::span) {}; + auto takes_8_or_16 = Botan::overloaded{ + [](std::span, std::span) {}, + [](std::span, std::span) {}, + }; + auto takes_anything = [](auto, auto) {}; + result.confirm("need just 8", Botan::detail::block_processing_callback); + result.confirm("need just 8 but could also take 16", + Botan::detail::block_processing_callback); + result.confirm("need both 8 and 16 but can do only 8", + !Botan::detail::block_processing_callback); + result.confirm("need both 8 and 16, can do dynamic", + Botan::detail::block_processing_callback); + result.confirm("need both 8 and 16, can do anything", + Botan::detail::block_processing_callback); + result.confirm("need 4, can only do 8 or 16", + !Botan::detail::block_processing_callback); + result.confirm("need 4, can only do 8", !Botan::detail::block_processing_callback); + }), + + Botan_Tests::CHECK( + "BufferTransformer::process_blocks_of() failure modes with a single block", + [](Test::Result& result) { + const auto in = Botan::hex_decode("000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F"); + std::vector out(in.size()); + + Botan::BufferTransformer bt1(in, out); + result.test_throws("data must be divisible by block size", + [&] { bt1.process_blocks_of<3>([](auto, auto) {}); }); + + Botan::BufferTransformer bt2(in, out); + result.test_throws("dynamically requesting data while block processing", [&] { + bt2.process_blocks_of<8>([&](auto, auto) { bt2.next(8); }); + }); + + Botan::BufferTransformer bt3(in, out); + result.test_throws("statically requesting data while block processing", [&] { + bt3.process_blocks_of<8>([&](auto, auto) { bt3.next<8>(); }); + }); + + Botan::BufferTransformer bt4(in, out); + result.test_throws("skipping data while block processing", [&] { + bt4.process_blocks_of<8>([&](auto, auto) { bt4.skip(8); }); + }); + + Botan::BufferTransformer bt5(in, out); + result.test_throws("nested block processing", [&] { + bt5.process_blocks_of<8>([&](auto, auto) { bt5.process_blocks_of<8>([](auto, auto) {}); }); + }); + }), + + Botan_Tests::CHECK("BufferTransformer::process_blocks_of() with two block sizes", + [](Test::Result& result) { + const auto in = + Botan::hex_decode("000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F"); + std::vector out(in.size()); + + Botan::BufferTransformer bt(in, out); + size_t block5 = 0; + size_t block2 = 0; + + bt.process_blocks_of<5, 2>( + Botan::overloaded{[&](std::span ins, std::span outs) { + ++block5; + Botan::copy_mem(outs, ins); + }, + [&](std::span ins, std::span outs) { + ++block2; + Botan::copy_mem(outs, ins); + }}); + + result.require("done now", bt.done()); + result.test_is_eq("output is as expected", out, in); + + result.test_eq("block size 5 processed", block5, 6); + result.test_eq("block size 2 processed", block2, 1); + }), + + Botan_Tests::CHECK("BufferTransformer::process_blocks_of() with four block sizes", + [](Test::Result& result) { + const auto in = Botan::hex_decode("000102030405060708090A0B0C0D0E0FFFFEFDFFFCFBFA"); + std::vector out(in.size()); + + // Will be processed in blocks of 8, 4, 2, 1 bytes + // 2x8 bytes, 1x4 bytes, 1x2 bytes, 1x1 bytes + result.require("sanity check input length", in.size() == 2 * 8 + 1 * 4 + 1 * 2 + 1 * 1); + + Botan::BufferTransformer bt(in, out); + + size_t processed_8_bytes = 0; + size_t processed_4_bytes = 0; + size_t processed_2_bytes = 0; + size_t processed_1_bytes = 0; + + bt.process_blocks_of<8, 4, 2, 1>([&](auto i, auto o) { + const auto integer = Botan::load_be(i); + Botan::copy_mem(o, Botan::store_be(i)); + + // This is more involved than it has to be to exercise the + // ability to use constexpr if in the lambda with the + // std::span::size() methods. + if constexpr(i.size() == 8) { + result.confirm("uint64_t", std::same_as); + ++processed_8_bytes; + } else if constexpr(i.size() == 4) { + result.confirm("uint32_t", std::same_as); + ++processed_4_bytes; + } else if constexpr(i.size() == 2) { + result.confirm("uint16_t", std::same_as); + ++processed_2_bytes; + } else if constexpr(i.size() == 1) { + result.confirm("uint8_t", std::same_as); + ++processed_1_bytes; + } else { + result.test_failure("unexpected block size"); + } + }); + + result.require("done now", bt.done()); + result.test_is_eq("output is as expected", out, in); + + result.test_eq("2x8 bytes processed", processed_8_bytes, 2); + result.test_eq("1x4 bytes processed", processed_4_bytes, 1); + result.test_eq("1x2 bytes processed", processed_2_bytes, 1); + result.test_eq("1x1 bytes processed", processed_1_bytes, 1); + }), + + Botan_Tests::CHECK( + "BufferTransformer::process_blocks_of() failure modes with multiple block sizes", + [](Test::Result& result) { + std::array in1 = {}; + std::array out1 = {}; + + Botan::BufferTransformer bt1(in1, out1); + result.test_throws("data must be processible by the sequence block size", [&] { + bt1.process_blocks_of<5, 3>([&](auto, auto) { result.test_failure("should not be called"); }); + }); + + std::vector in2; + std::vector out2; + + Botan::BufferTransformer bt2(in2, out2); + result.test_no_throw("empty input should always work", [&] { + bt2.process_blocks_of<50, 5, 2>([&](auto, auto) { result.test_failure("should not be called"); }); + }); + }), + }; +} + +BOTAN_REGISTER_TEST_FN("utils", + "buffer_utilities", + test_buffer_slicer, + test_buffer_stuffer, + test_alignment_buffer, + test_concat, + test_buffer_transformer); } // namespace From 8247124e277b350819080ec242a911fed7f68276 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Meusel?= Date: Wed, 26 Jun 2024 15:40:46 +0200 Subject: [PATCH 3/4] use BufferTransformer in blowfish --- src/lib/block/blowfish/blowfish.cpp | 47 ++++++++++++++--------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/src/lib/block/blowfish/blowfish.cpp b/src/lib/block/blowfish/blowfish.cpp index 611df1e1e4d..f688519044b 100644 --- a/src/lib/block/blowfish/blowfish.cpp +++ b/src/lib/block/blowfish/blowfish.cpp @@ -7,6 +7,7 @@ #include +#include #include namespace Botan { @@ -153,10 +154,15 @@ inline uint32_t BFF(uint32_t X, const secure_vector& S) { /* * Blowfish Encryption */ -void Blowfish::encrypt_n(const uint8_t in[], uint8_t out[], size_t blocks) const { +void Blowfish::encrypt_n(const uint8_t inb[], uint8_t outb[], size_t blocks) const { assert_key_material_set(); + std::span ins(inb, blocks * BLOCK_SIZE); + std::span outs(outb, blocks * BLOCK_SIZE); - while(blocks >= 4) { + constexpr size_t b4 = 4 * BLOCK_SIZE; + constexpr size_t b1 = 1 * BLOCK_SIZE; + + auto encrypt4 = [this](std::span in, std::span out) { uint32_t L0, R0, L1, R1, L2, R2, L3, R3; load_be(in, L0, R0, L1, R1, L2, R2, L3, R3); @@ -190,13 +196,9 @@ void Blowfish::encrypt_n(const uint8_t in[], uint8_t out[], size_t blocks) const R3 ^= m_P[17]; store_be(out, R0, L0, R1, L1, R2, L2, R3, L3); + }; - in += 4 * BLOCK_SIZE; - out += 4 * BLOCK_SIZE; - blocks -= 4; - } - - while(blocks) { + auto encrypt1 = [this](std::span in, std::span out) { uint32_t L, R; load_be(in, L, R); @@ -212,20 +214,23 @@ void Blowfish::encrypt_n(const uint8_t in[], uint8_t out[], size_t blocks) const R ^= m_P[17]; store_be(out, R, L); + }; - in += BLOCK_SIZE; - out += BLOCK_SIZE; - blocks--; - } + BufferTransformer(ins, outs).process_blocks_of(overloaded{encrypt4, encrypt1}); } /* * Blowfish Decryption */ -void Blowfish::decrypt_n(const uint8_t in[], uint8_t out[], size_t blocks) const { +void Blowfish::decrypt_n(const uint8_t inb[], uint8_t outb[], size_t blocks) const { assert_key_material_set(); + std::span ins(inb, blocks * BLOCK_SIZE); + std::span outs(outb, blocks * BLOCK_SIZE); - while(blocks >= 4) { + constexpr size_t b4 = 4 * BLOCK_SIZE; + constexpr size_t b1 = 1 * BLOCK_SIZE; + + auto decrypt4 = [this](std::span in, std::span out) { uint32_t L0, R0, L1, R1, L2, R2, L3, R3; load_be(in, L0, R0, L1, R1, L2, R2, L3, R3); @@ -260,13 +265,9 @@ void Blowfish::decrypt_n(const uint8_t in[], uint8_t out[], size_t blocks) const R3 ^= m_P[0]; store_be(out, R0, L0, R1, L1, R2, L2, R3, L3); + }; - in += 4 * BLOCK_SIZE; - out += 4 * BLOCK_SIZE; - blocks -= 4; - } - - while(blocks) { + auto decrypt1 = [this](std::span in, std::span out) { uint32_t L, R; load_be(in, L, R); @@ -282,11 +283,9 @@ void Blowfish::decrypt_n(const uint8_t in[], uint8_t out[], size_t blocks) const R ^= m_P[0]; store_be(out, R, L); + }; - in += BLOCK_SIZE; - out += BLOCK_SIZE; - blocks--; - } + BufferTransformer(ins, outs).process_blocks_of(overloaded{decrypt4, decrypt1}); } bool Blowfish::has_keying_material() const { From c88455d251c9010258d1314718db3a38595ed30b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Meusel?= Date: Wed, 26 Jun 2024 16:17:12 +0200 Subject: [PATCH 4/4] Chore: refactor blowfish using std::span<> where sensible --- src/lib/block/blowfish/blowfish.cpp | 80 ++++++++++----------- src/lib/block/blowfish/blowfish.h | 22 +++--- src/lib/passhash/bcrypt/bcrypt.cpp | 3 +- src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.cpp | 3 +- src/tests/test_blowfish.cpp | 2 +- 5 files changed, 56 insertions(+), 54 deletions(-) diff --git a/src/lib/block/blowfish/blowfish.cpp b/src/lib/block/blowfish/blowfish.cpp index f688519044b..2349cc87252 100644 --- a/src/lib/block/blowfish/blowfish.cpp +++ b/src/lib/block/blowfish/blowfish.cpp @@ -1,6 +1,7 @@ /* * Blowfish * (C) 1999-2011,2018 Jack Lloyd +* (C) 2024 René Meusel - Rohde & Schwarz Cybersecurity * * Botan is released under the Simplified BSD License (see license.txt) */ @@ -16,12 +17,12 @@ namespace { // clang-format off -const uint32_t P_INIT[18] = { +constexpr std::array P_INIT = { 0x243F6A88, 0x85A308D3, 0x13198A2E, 0x03707344, 0xA4093822, 0x299F31D0, 0x082EFA98, 0xEC4E6C89, 0x452821E6, 0x38D01377, 0xBE5466CF, 0x34E90C6C, 0xC0AC29B7, 0xC97C50DD, 0x3F84D5B5, 0xB5470917, 0x9216D5D9, 0x8979FB1B }; -const uint32_t S_INIT[1024] = { +constexpr std::array S_INIT = { 0xD1310BA6, 0x98DFB5AC, 0x2FFD72DB, 0xD01ADFB7, 0xB8E1AFED, 0x6A267E96, 0xBA7C9045, 0xF12C7F99, 0x24A19947, 0xB3916CF7, 0x0801F2E2, 0x858EFC16, 0x636920D8, 0x71574E69, 0xA458FEA3, 0xF4933D7E, 0x0D95748F, 0x728EB658, 0x718BCD58, 0x82154AEE, 0x7B54A41D, 0xC25A59B5, 0x9C30D539, 0x2AF26013, 0xC5D1B023, 0x286085F0, 0xCA417918, @@ -216,7 +217,7 @@ void Blowfish::encrypt_n(const uint8_t inb[], uint8_t outb[], size_t blocks) con store_be(out, R, L); }; - BufferTransformer(ins, outs).process_blocks_of(overloaded{encrypt4, encrypt1}); + BufferTransformer(ins, outs).process_blocks_of(overloaded{std::move(encrypt4), std::move(encrypt1)}); } /* @@ -285,7 +286,7 @@ void Blowfish::decrypt_n(const uint8_t inb[], uint8_t outb[], size_t blocks) con store_be(out, R, L); }; - BufferTransformer(ins, outs).process_blocks_of(overloaded{decrypt4, decrypt1}); + BufferTransformer(ins, outs).process_blocks_of(overloaded{std::move(decrypt4), std::move(decrypt1)}); } bool Blowfish::has_keying_material() const { @@ -295,59 +296,53 @@ bool Blowfish::has_keying_material() const { /* * Blowfish Key Schedule */ -void Blowfish::key_schedule(std::span key) { - m_P.resize(18); - copy_mem(m_P.data(), P_INIT, 18); - - m_S.resize(1024); - copy_mem(m_S.data(), S_INIT, 1024); - - key_expansion(key.data(), key.size(), nullptr, 0); +void Blowfish::key_schedule(std::span key, std::span salt) { + m_P.assign(P_INIT.begin(), P_INIT.end()); + m_S.assign(S_INIT.begin(), S_INIT.end()); + key_expansion(key, salt); } -void Blowfish::key_expansion(const uint8_t key[], size_t length, const uint8_t salt[], size_t salt_length) { - BOTAN_ASSERT_NOMSG(salt_length % 4 == 0); +void Blowfish::key_expansion(std::span key, std::span salt) { + BOTAN_ASSERT_NOMSG(salt.size() % 4 == 0); + const size_t length = key.size(); for(size_t i = 0, j = 0; i != 18; ++i, j += 4) { m_P[i] ^= make_uint32(key[(j) % length], key[(j + 1) % length], key[(j + 2) % length], key[(j + 3) % length]); } - const size_t P_salt_offset = (salt_length > 0) ? 18 % (salt_length / 4) : 0; + const size_t P_salt_offset = (!salt.empty()) ? 18 % (salt.size() / 4) : 0; uint32_t L = 0, R = 0; - generate_sbox(m_P, L, R, salt, salt_length, 0); - generate_sbox(m_S, L, R, salt, salt_length, P_salt_offset); + generate_sbox(m_P, L, R, salt, 0); + generate_sbox(m_S, L, R, salt, P_salt_offset); } /* * Modified key schedule used for bcrypt password hashing */ -void Blowfish::salted_set_key( - const uint8_t key[], size_t length, const uint8_t salt[], size_t salt_length, size_t workfactor, bool salt_first) { - BOTAN_ARG_CHECK(salt_length > 0 && salt_length % 4 == 0, "Invalid salt length for Blowfish salted key schedule"); +void Blowfish::salted_set_key(std::span key, + std::span salt, + size_t workfactor, + bool salt_first) { + BOTAN_ARG_CHECK(!salt.empty() && salt.size() % 4 == 0, "Invalid salt length for Blowfish salted key schedule"); - if(length > 72) { + if(key.size() > 72) { // Truncate longer passwords to the 72 char bcrypt limit - length = 72; + key = key.first(72); } - m_P.resize(18); - copy_mem(m_P.data(), P_INIT, 18); - - m_S.resize(1024); - copy_mem(m_S.data(), S_INIT, 1024); - key_expansion(key, length, salt, salt_length); + key_schedule(key, salt); if(workfactor > 0) { const size_t rounds = static_cast(1) << workfactor; for(size_t r = 0; r != rounds; ++r) { if(salt_first) { - key_expansion(salt, salt_length, nullptr, 0); - key_expansion(key, length, nullptr, 0); + key_expansion(salt, {}); + key_expansion(key, {}); } else { - key_expansion(key, length, nullptr, 0); - key_expansion(salt, salt_length, nullptr, 0); + key_expansion(key, {}); + key_expansion(salt, {}); } } } @@ -356,16 +351,19 @@ void Blowfish::salted_set_key( /* * Generate one of the Sboxes */ -void Blowfish::generate_sbox(secure_vector& box, - uint32_t& L, - uint32_t& R, - const uint8_t salt[], - size_t salt_length, - size_t salt_off) const { +void Blowfish::generate_sbox( + std::span box, uint32_t& L, uint32_t& R, std::span salt, size_t salt_off) const { + auto salt_words = [salt, salt_off](size_t off) -> std::pair { + const auto offset = (off + salt_off) * 4; + return {load_be(salt.subspan((offset + 0) % salt.size()).first<4>()), + load_be(salt.subspan((offset + 4) % salt.size()).first<4>())}; + }; + for(size_t i = 0; i != box.size(); i += 2) { - if(salt_length > 0) { - L ^= load_be(salt, (i + salt_off) % (salt_length / 4)); - R ^= load_be(salt, (i + salt_off + 1) % (salt_length / 4)); + if(!salt.empty()) { + auto [S_L, S_R] = salt_words(i); + L ^= S_L; + R ^= S_R; } for(size_t r = 0; r != 16; r += 2) { diff --git a/src/lib/block/blowfish/blowfish.h b/src/lib/block/blowfish/blowfish.h index 982927b757d..250f3549a4f 100644 --- a/src/lib/block/blowfish/blowfish.h +++ b/src/lib/block/blowfish/blowfish.h @@ -23,11 +23,19 @@ class BOTAN_TEST_API Blowfish final : public Block_Cipher_Fixed_Params<8, 1, 56> /** * Modified EKSBlowfish key schedule, used for bcrypt password hashing */ + BOTAN_DEPRECATED("use std::span<> overload") void salted_set_key(const uint8_t key[], size_t key_length, const uint8_t salt[], size_t salt_length, size_t workfactor, + bool salt_first = false) { + salted_set_key({key, key_length}, {salt, salt_length}, workfactor, salt_first); + } + + void salted_set_key(std::span key, + std::span salt, + size_t workfactor, bool salt_first = false); void clear() override; @@ -39,16 +47,14 @@ class BOTAN_TEST_API Blowfish final : public Block_Cipher_Fixed_Params<8, 1, 56> bool has_keying_material() const override; private: - void key_schedule(std::span key) override; + void key_schedule(std::span key) override { key_schedule(key, {}); } + + void key_schedule(std::span key, std::span salt); - void key_expansion(const uint8_t key[], size_t key_length, const uint8_t salt[], size_t salt_length); + void key_expansion(std::span key, std::span salt); - void generate_sbox(secure_vector& box, - uint32_t& L, - uint32_t& R, - const uint8_t salt[], - size_t salt_length, - size_t salt_off) const; + void generate_sbox( + std::span box, uint32_t& L, uint32_t& R, std::span salt, size_t salt_off) const; secure_vector m_S, m_P; }; diff --git a/src/lib/passhash/bcrypt/bcrypt.cpp b/src/lib/passhash/bcrypt/bcrypt.cpp index 478e79f999a..53dccb129bf 100644 --- a/src/lib/passhash/bcrypt/bcrypt.cpp +++ b/src/lib/passhash/bcrypt/bcrypt.cpp @@ -116,8 +116,7 @@ std::string make_bcrypt(std::string_view pass, const std::vector& salt, copy_mem(pass_with_trailing_null.data(), cast_char_ptr_to_uint8(pass.data()), pass.length()); // Include the trailing NULL byte, so we need c_str() not data() - blowfish.salted_set_key( - pass_with_trailing_null.data(), pass_with_trailing_null.size(), salt.data(), salt.size(), work_factor); + blowfish.salted_set_key(pass_with_trailing_null, salt, work_factor); std::vector ctext(BCRYPT_MAGIC, BCRYPT_MAGIC + 8 * 3); diff --git a/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.cpp b/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.cpp index 0b58d908610..7df974daadd 100644 --- a/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.cpp +++ b/src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.cpp @@ -93,8 +93,7 @@ void bcrypt_round(Blowfish& blowfish, const size_t BCRYPT_PBKDF_WORKFACTOR = 6; const size_t BCRYPT_PBKDF_ROUNDS = 64; - blowfish.salted_set_key( - pass_hash.data(), pass_hash.size(), salt_hash.data(), salt_hash.size(), BCRYPT_PBKDF_WORKFACTOR, true); + blowfish.salted_set_key(pass_hash, salt_hash, BCRYPT_PBKDF_WORKFACTOR, true); copy_mem(tmp.data(), BCRYPT_PBKDF_MAGIC, BCRYPT_PBKDF_OUTPUT); for(size_t i = 0; i != BCRYPT_PBKDF_ROUNDS; ++i) { diff --git a/src/tests/test_blowfish.cpp b/src/tests/test_blowfish.cpp index cf43673913b..c720f67b7c2 100644 --- a/src/tests/test_blowfish.cpp +++ b/src/tests/test_blowfish.cpp @@ -25,7 +25,7 @@ class Blowfish_Salted_Tests final : public Text_Based_Test { Botan::Blowfish blowfish; - blowfish.salted_set_key(key.data(), key.size(), salt.data(), salt.size(), 0); + blowfish.salted_set_key(key, salt, 0); std::vector block(8); blowfish.encrypt(block);