libchromeos: Unify interface for SecureBlob to match that of Blob
Streamlined the interface of SecureBlob to match that of Blob, which
is just a vector<uint8_t>. I have kept a couple of custom functions
to inter-operate with std::string.
The change greatly simplifies the usage of SecureBlob class as it
does not require as many type casts and other weird things, and in
addition the functions are pretty much those from std::vector, so
very familiar to people
BUG=None
TEST=`FEATURES=test emerge-link libchromeos chaps cryptohome privetd webserver update_engine`
Change-Id: Ifb82a2ba7fe3e8f8bdef4898d929372ee4a75476
Reviewed-on: https://chromium-review.googlesource.com/262479
Trybot-Ready: Alex Vakulenko <avakulenko@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Utkarsh Sanghi <usanghi@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/chromeos/cryptohome.cc b/chromeos/cryptohome.cc
index f7f4f93..5aa53d8 100644
--- a/chromeos/cryptohome.cc
+++ b/chromeos/cryptohome.cc
@@ -45,7 +45,7 @@
}
std::vector<char> buf;
buf.resize(file_size);
- unsigned int data_read = base::ReadFile(salt_path, &buf.front(), file_size);
+ unsigned int data_read = base::ReadFile(salt_path, buf.data(), file_size);
if (data_read != file_size) {
PLOG(ERROR) << "Could not read entire file: " << data_read
<< " != " << file_size;
@@ -54,7 +54,7 @@
if (!salt)
salt = new std::string();
- salt->assign(&buf.front(), file_size);
+ salt->assign(buf.data(), file_size);
return true;
}
diff --git a/chromeos/secure_blob.cc b/chromeos/secure_blob.cc
index 1ef950b..b7e215f 100644
--- a/chromeos/secure_blob.cc
+++ b/chromeos/secure_blob.cc
@@ -10,64 +10,34 @@
namespace chromeos {
-SecureBlob::SecureBlob() : chromeos::Blob() {
-}
-
-SecureBlob::SecureBlob(const_iterator begin, const_iterator end)
- : chromeos::Blob(begin, end) {
-}
-
-SecureBlob::SecureBlob(size_t size) : chromeos::Blob(size) {
-}
-
-SecureBlob::SecureBlob(const std::string& from)
- : chromeos::Blob(from.length()) {
- memcpy(data(), from.c_str(), from.length());
-}
-
-SecureBlob::SecureBlob(const uint8_t* from, size_t from_length)
- : chromeos::Blob(from_length) {
- memcpy(data(), from, from_length);
-}
-
-SecureBlob::SecureBlob(const char* from, size_t from_length)
- : chromeos::Blob(from_length) {
- memcpy(data(), from, from_length);
-}
+SecureBlob::SecureBlob(const std::string& data)
+ : SecureBlob(data.begin(), data.end()) {}
SecureBlob::~SecureBlob() {
- chromeos::SecureMemset(&this->front(), 0, this->capacity());
+ clear();
}
-void SecureBlob::resize(size_type sz) {
- if (sz < size()) {
- chromeos::SecureMemset(&this->at(sz), 0, size() - sz);
+void SecureBlob::resize(size_type count) {
+ if (count < size()) {
+ SecureMemset(data() + count, 0, capacity() - count);
}
- chromeos::Blob::resize(sz);
+ Blob::resize(count);
}
-void SecureBlob::resize(size_type sz, const value_type& x) {
- if (sz < size()) {
- chromeos::SecureMemset(&this->at(sz), 0, size() - sz);
+void SecureBlob::resize(size_type count, const value_type& value) {
+ if (count < size()) {
+ SecureMemset(data() + count, 0, capacity() - count);
}
- chromeos::Blob::resize(sz, x);
+ Blob::resize(count, value);
}
-void SecureBlob::clear_contents() {
- chromeos::SecureMemset(this->data(), 0, size());
-}
-
-void* SecureBlob::data() {
- return chromeos::Blob::data();
-}
-
-const void* SecureBlob::const_data() const {
- return chromeos::Blob::data();
+void SecureBlob::clear() {
+ SecureMemset(data(), 0, capacity());
+ Blob::clear();
}
std::string SecureBlob::to_string() const {
- auto data_ptr = reinterpret_cast<const char*>(const_data());
- return std::string(data_ptr, data_ptr + size());
+ return std::string(data(), data() + size());
}
SecureBlob SecureBlob::Combine(const SecureBlob& blob1,
diff --git a/chromeos/secure_blob.h b/chromeos/secure_blob.h
index 8bdc9da..e92658e 100644
--- a/chromeos/secure_blob.h
+++ b/chromeos/secure_blob.h
@@ -16,29 +16,26 @@
// SecureBlob erases the contents on destruction. It does not guarantee erasure
// on resize, assign, etc.
-class CHROMEOS_EXPORT SecureBlob : public chromeos::Blob {
+class CHROMEOS_EXPORT SecureBlob : public Blob {
public:
- SecureBlob();
- SecureBlob(const_iterator begin, const_iterator end);
- explicit SecureBlob(size_t size);
- explicit SecureBlob(const std::string& str);
- SecureBlob(const uint8_t* from, size_t from_length);
- SecureBlob(const char* from, size_t from_length);
- virtual ~SecureBlob();
+ SecureBlob() = default;
+ using Blob::vector; // Inherit standard constructors from vector.
+ explicit SecureBlob(const std::string& data);
+ ~SecureBlob();
- void resize(size_type sz);
- void resize(size_type sz, const value_type& x);
-
- void clear_contents();
-
- void* data();
- const void* const_data() const;
+ void resize(size_type count);
+ void resize(size_type count, const value_type& value);
+ void clear();
std::string to_string() const;
+ char* char_data() { return reinterpret_cast<char*>(data()); }
+ const char* char_data() const {
+ return reinterpret_cast<const char*>(data());
+ }
static SecureBlob Combine(const SecureBlob& blob1, const SecureBlob& blob2);
};
-// Secure memset(). This function is guaranteed to fill int the whole buffer
+// Secure memset(). This function is guaranteed to fill in the whole buffer
// and is not subject to compiler optimization as allowed by Sub-clause 5.1.2.3
// of C Standard [ISO/IEC 9899:2011] which states:
// In the abstract machine, all expressions are evaluated as specified by the
diff --git a/chromeos/secure_blob_unittest.cc b/chromeos/secure_blob_unittest.cc
index 409613f..e85e2af 100644
--- a/chromeos/secure_blob_unittest.cc
+++ b/chromeos/secure_blob_unittest.cc
@@ -52,11 +52,9 @@
TEST_F(SecureBlobTest, AllocationCopyTest) {
// Check that allocating a SecureBlob with an iterator works
unsigned char from_data[32];
- for (unsigned int i = 0; i < sizeof(from_data); i++) {
- from_data[i] = i;
- }
+ std::iota(std::begin(from_data), std::end(from_data), 0);
- SecureBlob blob(from_data, sizeof(from_data));
+ SecureBlob blob(std::begin(from_data), std::end(from_data));
EXPECT_EQ(sizeof(from_data), blob.size());
@@ -82,10 +80,10 @@
// Check that resizing a SecureBlob wipes the excess memory. The test assumes
// that resize() down by one will not re-allocate the memory, so the last byte
// will still be part of the SecureBlob's allocation
- unsigned int length = 1024;
+ size_t length = 1024;
SecureBlob blob(length);
void* original_data = blob.data();
- for (unsigned int i = 0; i < length; i++) {
+ for (size_t i = 0; i < length; i++) {
blob[i] = i;
}
@@ -93,7 +91,7 @@
EXPECT_EQ(original_data, blob.data());
EXPECT_EQ(length - 1, blob.size());
- EXPECT_EQ(0, static_cast<unsigned char*>(blob.data())[length - 1]);
+ EXPECT_EQ(0, blob.data()[length - 1]);
}
TEST_F(SecureBlobTest, CombineTest) {
@@ -113,7 +111,7 @@
TEST_F(SecureBlobTest, BlobToStringTest) {
std::string test_string("Test String");
- SecureBlob blob = SecureBlob(test_string);
+ SecureBlob blob = SecureBlob(test_string.begin(), test_string.end());
EXPECT_EQ(blob.size(), test_string.length());
std::string result_string = blob.to_string();
EXPECT_EQ(test_string.compare(result_string), 0);