Add SecureBlob::HexStringToSecureBlob.

Polymorphic usage of SecureBlob is problematic (see crbug.com/728047).
In some cases we're unnecessarily passing plain Blobs into functions
that take SecureBlobs because we don't have code to create a SecureBlob
from a hex string, so we use base::HexStringToBytes to create a Blob
instead.

This CL does attempt to use HexStringToBytes and then zero out the
memory. As long as we still have polymorphic usage of SecureBlobs,
this is not a big deal. Once we decouple SecureBlob from Blob, we can
improve this implementation.

BUG=chromium:728047
TEST=New unit tests.

Change-Id: Iad94cbd8f574dd35730cb2d68b6ae39798b03ef6
Reviewed-on: https://chromium-review.googlesource.com/1114919
Commit-Ready: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

3 files changed, 42 insertions, 0 deletions

diff --git a/brillo/secure_blob.cc b/brillo/secure_blob.cc
index 4294b14..f4b797f 100644
--- a/brillo/secure_blob.cc
+++ b/brillo/secure_blob.cc
@@ -5,6 +5,7 @@
 #include <cstring>  // memcpy
 
 #include <base/stl_util.h>
+#include <base/strings/string_number_conversions.h>
 
 #include "brillo/secure_blob.h"
 
@@ -71,6 +72,20 @@ SecureBlob SecureBlob::Combine(const SecureBlob& blob1,
   return result;
 }
 
+bool SecureBlob::HexStringToSecureBlob(const std::string& input,
+                                       SecureBlob* output) {
+  // TODO(jorgelo,crbug.com/728047): Consider not using an intermediate
+  // std::vector here at all.
+  std::vector<uint8_t> temp;
+  if (!base::HexStringToBytes(input, &temp)) {
+    output->clear();
+    return false;
+  }
+  output->assign(temp.begin(), temp.end());
+  SecureMemset(temp.data(), 0, temp.capacity());
+  return true;
+}
+
 BRILLO_DISABLE_ASAN void* SecureMemset(void* v, int c, size_t n) {
   volatile uint8_t* p = reinterpret_cast<volatile uint8_t*>(v);
   while (n--)
diff --git a/brillo/secure_blob.h b/brillo/secure_blob.h
index d2fe9c7..7b6d03c 100644
--- a/brillo/secure_blob.h
+++ b/brillo/secure_blob.h
@@ -44,6 +44,8 @@ class BRILLO_EXPORT SecureBlob : public Blob {
     return reinterpret_cast<const char*>(data());
   }
   static SecureBlob Combine(const SecureBlob& blob1, const SecureBlob& blob2);
+  static bool HexStringToSecureBlob(const std::string& input,
+                                    SecureBlob* output);
 };
 
 // Secure memset(). This function is guaranteed to fill in the whole buffer
diff --git a/brillo/secure_blob_unittest.cc b/brillo/secure_blob_unittest.cc
index d0de1c4..ff95d0f 100644
--- a/brillo/secure_blob_unittest.cc
+++ b/brillo/secure_blob_unittest.cc
@@ -202,4 +202,29 @@ TEST_F(SecureBlobTest, BlobToStringTest) {
   EXPECT_EQ(test_string.compare(result_string), 0);
 }
 
+TEST_F(SecureBlobTest, HexStringToSecureBlob) {
+  std::string hex_string("112233445566778899aabbccddeeff0f");
+
+  SecureBlob blob;
+  SecureBlob::HexStringToSecureBlob(hex_string, &blob);
+
+  EXPECT_EQ(blob.size(), 16u);
+  EXPECT_EQ(blob[0],  0x11);
+  EXPECT_EQ(blob[1],  0x22);
+  EXPECT_EQ(blob[2],  0x33);
+  EXPECT_EQ(blob[3],  0x44);
+  EXPECT_EQ(blob[4],  0x55);
+  EXPECT_EQ(blob[5],  0x66);
+  EXPECT_EQ(blob[6],  0x77);
+  EXPECT_EQ(blob[7],  0x88);
+  EXPECT_EQ(blob[8],  0x99);
+  EXPECT_EQ(blob[9],  0xaa);
+  EXPECT_EQ(blob[10], 0xbb);
+  EXPECT_EQ(blob[11], 0xcc);
+  EXPECT_EQ(blob[12], 0xdd);
+  EXPECT_EQ(blob[13], 0xee);
+  EXPECT_EQ(blob[14], 0xff);
+  EXPECT_EQ(blob[15], 0x0f);
+}
+
 }  // namespace brillo