Skip to content

Commit

Permalink
Remove crypto::Encryptor::Counter
Browse files Browse the repository at this point in the history 
This helper class is all but unused. Probably a remnant of another
crypto backend. Also remove a redundant EVP_CIPHER_CTX scoper since
BoringSSL already provides one.

Bug: none
Change-Id: Ie4594313fbe4ecc0b0352a92c361ca3e77665eea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2245889
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779056}
  • Loading branch information
@davidben
davidben authored and Commit Bot committed Jun 16, 2020
1 parent a3c30f6 commit 47feaaf
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 125 deletions.
73 changes: 11 additions & 62 deletions crypto/encryptor.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,59 +28,8 @@ const EVP_CIPHER* GetCipherForKey(const SymmetricKey* key) {
}
}

// On destruction this class will cleanup the ctx, and also clear the OpenSSL
// ERR stack as a convenience.
class ScopedCipherCTX {
public:
ScopedCipherCTX() {
EVP_CIPHER_CTX_init(&ctx_);
}
~ScopedCipherCTX() {
EVP_CIPHER_CTX_cleanup(&ctx_);
ClearOpenSSLERRStack(FROM_HERE);
}
EVP_CIPHER_CTX* get() { return &ctx_; }

private:
EVP_CIPHER_CTX ctx_;
};

} // namespace

/////////////////////////////////////////////////////////////////////////////
// Encyptor::Counter Implementation.
Encryptor::Counter::Counter(base::span<const uint8_t> counter) {
CHECK(sizeof(counter_) == counter.size());

memcpy(&counter_, counter.data(), sizeof(counter_));
}

Encryptor::Counter::~Counter() = default;

bool Encryptor::Counter::Increment() {
uint64_t low_num = base::NetToHost64(counter_.components64[1]);
uint64_t new_low_num = low_num + 1;
counter_.components64[1] = base::HostToNet64(new_low_num);

// If overflow occured then increment the most significant component.
if (new_low_num < low_num) {
counter_.components64[0] =
base::HostToNet64(base::NetToHost64(counter_.components64[0]) + 1);
}

// TODO(hclam): Return false if counter value overflows.
return true;
}

void Encryptor::Counter::Write(void* buf) {
uint8_t* buf_ptr = reinterpret_cast<uint8_t*>(buf);
memcpy(buf_ptr, &counter_, sizeof(counter_));
}

size_t Encryptor::Counter::GetLengthInBytes() const {
return sizeof(counter_);
}

/////////////////////////////////////////////////////////////////////////////
// Encryptor Implementation.

Expand All @@ -101,6 +50,9 @@ bool Encryptor::Init(const SymmetricKey* key,
EnsureOpenSSLInit();
if (mode == CBC && iv.size() != AES_BLOCK_SIZE)
return false;
// CTR mode passes the starting counter separately, via SetCounter().
if (mode == CTR && !iv.empty())
return false;

if (GetCipherForKey(key) == nullptr)
return false;
Expand Down Expand Up @@ -143,7 +95,7 @@ bool Encryptor::SetCounter(base::span<const uint8_t> counter) {
if (counter.size() != 16u)
return false;

counter_ = std::make_unique<Counter>(counter);
iv_.assign(counter.begin(), counter.end());
return true;
}

Expand Down Expand Up @@ -203,7 +155,8 @@ base::Optional<size_t> Encryptor::Crypt(bool do_encrypt,
DCHECK_EQ(EVP_CIPHER_iv_length(cipher), iv_.size());
DCHECK_EQ(EVP_CIPHER_key_length(cipher), key.size());

ScopedCipherCTX ctx;
OpenSSLErrStackTracer err_tracer(FROM_HERE);
bssl::ScopedEVP_CIPHER_CTX ctx;
if (!EVP_CipherInit_ex(ctx.get(), cipher, nullptr,
reinterpret_cast<const uint8_t*>(key.data()),
iv_.data(), do_encrypt)) {
Expand Down Expand Up @@ -231,7 +184,7 @@ base::Optional<size_t> Encryptor::Crypt(bool do_encrypt,
base::Optional<size_t> Encryptor::CryptCTR(bool do_encrypt,
base::span<const uint8_t> input,
base::span<uint8_t> output) {
if (!counter_.get()) {
if (iv_.size() != AES_BLOCK_SIZE) {
LOG(ERROR) << "Counter value not set in CTR mode.";
return base::nullopt;
}
Expand All @@ -242,19 +195,15 @@ base::Optional<size_t> Encryptor::CryptCTR(bool do_encrypt,
return base::nullopt;
}

uint8_t ivec[AES_BLOCK_SIZE] = { 0 };
uint8_t ecount_buf[AES_BLOCK_SIZE] = { 0 };
unsigned int block_offset = 0;

counter_->Write(ivec);

// |output| must have room for |input|.
CHECK_GE(output.size(), input.size());
AES_ctr128_encrypt(input.data(), output.data(), input.size(), &aes_key, ivec,
ecount_buf, &block_offset);

// AES_ctr128_encrypt() updates |ivec|. Update the |counter_| here.
SetCounter(ivec);
// Note AES_ctr128_encrypt() will update |iv_|. However, this method discards
// |ecount_buf| and |block_offset|, so this is not quite a streaming API.
AES_ctr128_encrypt(input.data(), output.data(), input.size(), &aes_key,
iv_.data(), ecount_buf, &block_offset);
return input.size();
}

Expand Down
28 changes: 3 additions & 25 deletions crypto/encryptor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,30 +32,6 @@ class CRYPTO_EXPORT Encryptor {
CTR,
};

// This class implements a 128-bits counter to be used in AES-CTR encryption.
// Only 128-bits counter is supported in this class.
class CRYPTO_EXPORT Counter {
public:
explicit Counter(base::span<const uint8_t> counter);
~Counter();

// Increment the counter value.
bool Increment();

// Write the content of the counter to |buf|. |buf| should have enough
// space for |GetLengthInBytes()|.
void Write(void* buf);

// Return the length of this counter.
size_t GetLengthInBytes() const;

private:
union {
uint32_t components32[4];
uint64_t components64[2];
} counter_;
};

Encryptor();
~Encryptor();

Expand Down Expand Up @@ -98,7 +74,6 @@ class CRYPTO_EXPORT Encryptor {
private:
const SymmetricKey* key_;
Mode mode_;
std::unique_ptr<Counter> counter_;

bool CryptString(bool do_encrypt,
base::StringPiece input,
Expand All @@ -116,6 +91,9 @@ class CRYPTO_EXPORT Encryptor {
base::Optional<size_t> CryptCTR(bool do_encrypt,
base::span<const uint8_t> input,
base::span<uint8_t> output);

// In CBC mode, the IV passed to Init(). In CTR mode, the counter value passed
// to SetCounter().
std::vector<uint8_t> iv_;
};

Expand Down
38 changes: 0 additions & 38 deletions crypto/encryptor_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,44 +336,6 @@ TEST(EncryptorTest, EncryptDecryptCTR) {
EXPECT_EQ(plaintext, decrypted);
}

TEST(EncryptorTest, CTRCounter) {
const int kCounterSize = 16;
const unsigned char kTest1[] =
{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
unsigned char buf[16];

// Increment 10 times.
crypto::Encryptor::Counter counter1(kTest1);
for (int i = 0; i < 10; ++i)
counter1.Increment();
counter1.Write(buf);
EXPECT_EQ(0, memcmp(buf, kTest1, 15));
EXPECT_EQ(10, buf[15]);

// Check corner cases.
const unsigned char kTest2[] = {
0, 0, 0, 0, 0, 0, 0, 0,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
};
const unsigned char kExpect2[] =
{0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0};
crypto::Encryptor::Counter counter2(kTest2);
counter2.Increment();
counter2.Write(buf);
EXPECT_EQ(0, memcmp(buf, kExpect2, kCounterSize));

const unsigned char kTest3[] = {
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
};
const unsigned char kExpect3[] =
{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
crypto::Encryptor::Counter counter3(kTest3);
counter3.Increment();
counter3.Write(buf);
EXPECT_EQ(0, memcmp(buf, kExpect3, kCounterSize));
}

// TODO(wtc): add more known-answer tests. Test vectors are available from
// http://www.ietf.org/rfc/rfc3602
// http://csrc.nist.gov/publications/nistpubs/800-38a/sp800-38a.pdf
Expand Down

0 comments on commit 47feaaf

Please sign in to comment.