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] 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);