From 52b05a08c7803561f93a42a1e22891fa417acc51 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 12 Jan 2021 09:25:55 +0100 Subject: [PATCH] LibCrypto: Reduce use of ByteBuffer in AES code Use Bytes/ReadonlyBytes more where possible. --- Libraries/LibCrypto/Cipher/AES.cpp | 28 +++++++++++++-------------- Libraries/LibCrypto/Cipher/AES.h | 14 ++++++-------- Libraries/LibCrypto/Cipher/Cipher.h | 14 ++++++-------- Libraries/LibCrypto/Cipher/Mode/CBC.h | 6 +++--- Libraries/LibCrypto/Cipher/Mode/CTR.h | 2 +- Libraries/LibCrypto/Cipher/Mode/GCM.h | 2 +- 6 files changed, 31 insertions(+), 35 deletions(-) diff --git a/Libraries/LibCrypto/Cipher/AES.cpp b/Libraries/LibCrypto/Cipher/AES.cpp index 66dbac25525..1ce0e122405 100644 --- a/Libraries/LibCrypto/Cipher/AES.cpp +++ b/Libraries/LibCrypto/Cipher/AES.cpp @@ -222,10 +222,10 @@ void AESCipher::encrypt_block(const AESCipherBlock& in, AESCipherBlock& out) const auto& dec_key = key(); const auto* round_keys = dec_key.round_keys(); - s0 = get_key(in.data().offset_pointer(0)) ^ round_keys[0]; - s1 = get_key(in.data().offset_pointer(4)) ^ round_keys[1]; - s2 = get_key(in.data().offset_pointer(8)) ^ round_keys[2]; - s3 = get_key(in.data().offset_pointer(12)) ^ round_keys[3]; + s0 = get_key(in.bytes().offset_pointer(0)) ^ round_keys[0]; + s1 = get_key(in.bytes().offset_pointer(4)) ^ round_keys[1]; + s2 = get_key(in.bytes().offset_pointer(8)) ^ round_keys[2]; + s3 = get_key(in.bytes().offset_pointer(12)) ^ round_keys[3]; r = dec_key.rounds() >> 1; @@ -315,10 +315,10 @@ void AESCipher::decrypt_block(const AESCipherBlock& in, AESCipherBlock& out) const auto& dec_key = key(); const auto* round_keys = dec_key.round_keys(); - s0 = get_key(in.data().offset_pointer(0)) ^ round_keys[0]; - s1 = get_key(in.data().offset_pointer(4)) ^ round_keys[1]; - s2 = get_key(in.data().offset_pointer(8)) ^ round_keys[2]; - s3 = get_key(in.data().offset_pointer(12)) ^ round_keys[3]; + s0 = get_key(in.bytes().offset_pointer(0)) ^ round_keys[0]; + s1 = get_key(in.bytes().offset_pointer(4)) ^ round_keys[1]; + s2 = get_key(in.bytes().offset_pointer(8)) ^ round_keys[2]; + s3 = get_key(in.bytes().offset_pointer(12)) ^ round_keys[3]; r = dec_key.rounds() >> 1; @@ -401,21 +401,21 @@ void AESCipherBlock::overwrite(ReadonlyBytes bytes) auto data = bytes.data(); auto length = bytes.size(); - ASSERT(length <= m_data.size()); - m_data.overwrite(0, data, length); - if (length < m_data.size()) { + ASSERT(length <= this->data_size()); + this->bytes().overwrite(0, data, length); + if (length < this->data_size()) { switch (padding_mode()) { case PaddingMode::Null: // fill with zeros - __builtin_memset(m_data.data() + length, 0, m_data.size() - length); + __builtin_memset(m_data + length, 0, this->data_size() - length); break; case PaddingMode::CMS: // fill with the length of the padding bytes - __builtin_memset(m_data.data() + length, m_data.size() - length, m_data.size() - length); + __builtin_memset(m_data + length, this->data_size() - length, this->data_size() - length); break; case PaddingMode::RFC5246: // fill with the length of the padding bytes minus one - __builtin_memset(m_data.data() + length, m_data.size() - length - 1, m_data.size() - length); + __builtin_memset(m_data + length, this->data_size() - length - 1, this->data_size() - length); break; default: // FIXME: We should handle the rest of the common padding modes diff --git a/Libraries/LibCrypto/Cipher/AES.h b/Libraries/LibCrypto/Cipher/AES.h index c1a0e53cc51..7ce273d66d2 100644 --- a/Libraries/LibCrypto/Cipher/AES.h +++ b/Libraries/LibCrypto/Cipher/AES.h @@ -42,7 +42,6 @@ public: explicit AESCipherBlock(PaddingMode mode = PaddingMode::CMS) : CipherBlock(mode) { - m_data = ByteBuffer::create_zeroed(BlockSizeInBits / 8); } AESCipherBlock(const u8* data, size_t length, PaddingMode mode = PaddingMode::CMS) : AESCipherBlock(mode) @@ -52,12 +51,10 @@ public: static size_t block_size() { return BlockSizeInBits / 8; }; - virtual ByteBuffer get() const override { return m_data; }; - virtual const ByteBuffer& data() const override { return m_data; }; - ReadonlyBytes bytes() const { return m_data; } + virtual ReadonlyBytes bytes() const override { return ReadonlyBytes { m_data, sizeof(m_data) }; } + virtual Bytes bytes() override { return Bytes { m_data, sizeof(m_data) }; } virtual void overwrite(ReadonlyBytes) override; - virtual void overwrite(const ByteBuffer& buffer) override { overwrite(buffer.bytes()); } virtual void overwrite(const u8* data, size_t size) override { overwrite({ data, size }); } virtual void apply_initialization_vector(const u8* ivec) override @@ -69,12 +66,13 @@ public: String to_string() const; private: - virtual ByteBuffer& data() override { return m_data; }; - ByteBuffer m_data; + size_t data_size() const { return sizeof(m_data); } + + u8 m_data[BlockSizeInBits / 8] {}; }; struct AESCipherKey : public CipherKey { - virtual ByteBuffer data() const override { return ByteBuffer::copy(m_rd_keys, sizeof(m_rd_keys)); }; + virtual ReadonlyBytes bytes() const override { return ReadonlyBytes { m_rd_keys, sizeof(m_rd_keys) }; }; virtual void expand_encrypt_key(ReadonlyBytes user_key, size_t bits) override; virtual void expand_decrypt_key(ReadonlyBytes user_key, size_t bits) override; static bool is_valid_key_size(size_t bits) { return bits == 128 || bits == 192 || bits == 256; }; diff --git a/Libraries/LibCrypto/Cipher/Cipher.h b/Libraries/LibCrypto/Cipher/Cipher.h index ce000ad28a6..3b16c61cd95 100644 --- a/Libraries/LibCrypto/Cipher/Cipher.h +++ b/Libraries/LibCrypto/Cipher/Cipher.h @@ -26,8 +26,8 @@ #pragma once -#include #include +#include #include namespace Crypto { @@ -61,11 +61,9 @@ public: static size_t block_size() { ASSERT_NOT_REACHED(); } - virtual ByteBuffer get() const = 0; - virtual const ByteBuffer& data() const = 0; + virtual ReadonlyBytes bytes() const = 0; virtual void overwrite(ReadonlyBytes) = 0; - virtual void overwrite(const ByteBuffer& buffer) { overwrite(buffer.bytes()); } virtual void overwrite(const u8* data, size_t size) { overwrite({ data, size }); } virtual void apply_initialization_vector(const u8* ivec) = 0; @@ -76,8 +74,8 @@ public: template void put(size_t offset, T value) { - ASSERT(offset + sizeof(T) <= data().size()); - auto* ptr = data().data() + offset; + ASSERT(offset + sizeof(T) <= bytes().size()); + auto* ptr = bytes().offset_pointer(offset); auto index { 0 }; ASSERT(sizeof(T) <= 4); @@ -95,12 +93,12 @@ public: } private: - virtual ByteBuffer& data() = 0; + virtual Bytes bytes() = 0; PaddingMode m_padding_mode; }; struct CipherKey { - virtual ByteBuffer data() const = 0; + virtual ReadonlyBytes bytes() const = 0; static bool is_valid_key_size(size_t) { return false; }; virtual ~CipherKey() { } diff --git a/Libraries/LibCrypto/Cipher/Mode/CBC.h b/Libraries/LibCrypto/Cipher/Mode/CBC.h index fc4aa27aacb..68358a05ca1 100644 --- a/Libraries/LibCrypto/Cipher/Mode/CBC.h +++ b/Libraries/LibCrypto/Cipher/Mode/CBC.h @@ -78,7 +78,7 @@ public: m_cipher_block.apply_initialization_vector(iv); cipher.encrypt_block(m_cipher_block, m_cipher_block); ASSERT(offset + block_size <= out.size()); - __builtin_memcpy(out.offset(offset), m_cipher_block.get().data(), block_size); + __builtin_memcpy(out.offset(offset), m_cipher_block.bytes().data(), block_size); iv = out.offset(offset); length -= block_size; offset += block_size; @@ -89,7 +89,7 @@ public: m_cipher_block.apply_initialization_vector(iv); cipher.encrypt_block(m_cipher_block, m_cipher_block); ASSERT(offset + block_size <= out.size()); - __builtin_memcpy(out.offset(offset), m_cipher_block.get().data(), block_size); + __builtin_memcpy(out.offset(offset), m_cipher_block.bytes().data(), block_size); iv = out.offset(offset); } @@ -122,7 +122,7 @@ public: m_cipher_block.overwrite(slice, block_size); cipher.decrypt_block(m_cipher_block, m_cipher_block); m_cipher_block.apply_initialization_vector(iv); - auto decrypted = m_cipher_block.get(); + auto decrypted = m_cipher_block.bytes(); ASSERT(offset + decrypted.size() <= out.size()); __builtin_memcpy(out.offset(offset), decrypted.data(), decrypted.size()); iv = slice; diff --git a/Libraries/LibCrypto/Cipher/Mode/CTR.h b/Libraries/LibCrypto/Cipher/Mode/CTR.h index 928208e7fcb..26895a8fca1 100644 --- a/Libraries/LibCrypto/Cipher/Mode/CTR.h +++ b/Libraries/LibCrypto/Cipher/Mode/CTR.h @@ -193,7 +193,7 @@ protected: auto write_size = min(block_size, length); ASSERT(offset + write_size <= out.size()); - __builtin_memcpy(out.offset(offset), m_cipher_block.get().data(), write_size); + __builtin_memcpy(out.offset(offset), m_cipher_block.bytes().data(), write_size); increment(iv); length -= write_size; diff --git a/Libraries/LibCrypto/Cipher/Mode/GCM.h b/Libraries/LibCrypto/Cipher/Mode/GCM.h index 6f3afec2e8a..4d034f5bac4 100644 --- a/Libraries/LibCrypto/Cipher/Mode/GCM.h +++ b/Libraries/LibCrypto/Cipher/Mode/GCM.h @@ -105,7 +105,7 @@ public: auto auth_tag = m_ghash->process(aad, out); block0.apply_initialization_vector(auth_tag.data); - block0.get().bytes().copy_to(tag); + block0.bytes().copy_to(tag); } VerificationConsistency decrypt(ReadonlyBytes in, Bytes out, ReadonlyBytes iv_in, ReadonlyBytes aad, ReadonlyBytes tag)