Skip to content

Commit

Permalink
Chore: refactor blowfish using std::span<> where sensible
Browse files Browse the repository at this point in the history
  • Loading branch information
@reneme
reneme committed Jun 27, 2024
1 parent 9fe94fa commit 4feb67d
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 52 deletions.
80 changes: 41 additions & 39 deletions src/lib/block/blowfish/blowfish.cpp
View file
Original file line number Diff line number Diff line change
@@ -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)
*/
Expand All @@ -16,12 +17,12 @@ namespace {

// clang-format off

const uint32_t P_INIT[18] = {
constexpr std::array<uint32_t, 18> 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<uint32_t, 1024> 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,
Expand Down Expand Up @@ -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<b4, b1>(overloaded{encrypt4, encrypt1});
BufferTransformer(ins, outs).process_blocks_of<b4, b1>(overloaded{std::move(encrypt4), std::move(encrypt1)});
}

/*
Expand Down Expand Up @@ -285,7 +286,7 @@ void Blowfish::decrypt_n(const uint8_t in[], uint8_t out[], size_t blocks) const
store_be(out, R, L);
};

BufferTransformer(ins, outs).process_blocks_of<b4, b1>(overloaded{decrypt4, decrypt1});
BufferTransformer(ins, outs).process_blocks_of<b4, b1>(overloaded{std::move(decrypt4), std::move(decrypt1)});
}

bool Blowfish::has_keying_material() const {
Expand All @@ -295,59 +296,57 @@ bool Blowfish::has_keying_material() const {
/*
* Blowfish Key Schedule
*/
void Blowfish::key_schedule(std::span<const uint8_t> key) {
m_P.resize(18);
copy_mem(m_P.data(), P_INIT, 18);
void Blowfish::key_schedule(std::span<const uint8_t> key, std::span<const uint8_t> salt) {
m_P.resize(P_INIT.size());
copy_mem(m_P, P_INIT);

m_S.resize(1024);
copy_mem(m_S.data(), S_INIT, 1024);
m_S.resize(S_INIT.size());
copy_mem(m_S, S_INIT);

key_expansion(key.data(), key.size(), nullptr, 0);
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<const uint8_t> key, std::span<const uint8_t> 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<const uint8_t> key,
std::span<const uint8_t> salt,
size_t workfactor,
bool salt_first) {
BOTAN_ARG_CHECK(salt.size() > 0 && salt.size() % 4 == 0, "Invalid salt length for Blowfish salted key schedule");

Check warning on line 331 in src/lib/block/blowfish/blowfish.cpp

View workflow job for this annotation

GitHub Actions / Clang Tidy 

the 'empty' method should be used to check for emptiness instead of 'size'

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<size_t>(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, {});
}
}
}
Expand All @@ -356,16 +355,19 @@ void Blowfish::salted_set_key(
/*
* Generate one of the Sboxes
*/
void Blowfish::generate_sbox(secure_vector<uint32_t>& 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<uint32_t> box, uint32_t& L, uint32_t& R, std::span<const uint8_t> salt, size_t salt_off) const {
auto salt_words = [salt, salt_off](size_t off) -> std::pair<uint32_t, uint32_t> {
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<uint32_t>(salt, (i + salt_off) % (salt_length / 4));
R ^= load_be<uint32_t>(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) {
Expand Down
22 changes: 14 additions & 8 deletions src/lib/block/blowfish/blowfish.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<const uint8_t> key,
std::span<const uint8_t> salt,
size_t workfactor,
bool salt_first = false);

void clear() override;
Expand All @@ -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<const uint8_t> key) override;
void key_schedule(std::span<const uint8_t> key) override { key_schedule(key, {}); }

void key_schedule(std::span<const uint8_t> key, std::span<const uint8_t> 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<const uint8_t> key, std::span<const uint8_t> salt);

void generate_sbox(secure_vector<uint32_t>& 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<uint32_t> box, uint32_t& L, uint32_t& R, std::span<const uint8_t> salt, size_t salt_off) const;

secure_vector<uint32_t> m_S, m_P;
};
Expand Down
3 changes: 1 addition & 2 deletions src/lib/passhash/bcrypt/bcrypt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,7 @@ std::string make_bcrypt(std::string_view pass, const std::vector<uint8_t>& 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<uint8_t> ctext(BCRYPT_MAGIC, BCRYPT_MAGIC + 8 * 3);

Expand Down
3 changes: 1 addition & 2 deletions src/lib/pbkdf/bcrypt_pbkdf/bcrypt_pbkdf.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/tests/test_blowfish.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t> block(8);
blowfish.encrypt(block);
Expand Down

0 comments on commit 4feb67d

Please sign in to comment.