Harden key handling and crypto backend edge cases

include/sframe/map.h

@@ -83,28 +83,35 @@ class map : private vector<std::optional<std::pair<K, V>>, N>
 
 namespace SFRAME_NAMESPACE {
 
-// NOTE: NOT RECOMMENDED FOR USE OUTSIDE THIS LIBRARY
-//
-// We have used public inheritance from std::map<T> to simplify the interface
-// here.  This works fine for the use cases we have within this library.  If you
-// choose to use this map type outside this library, you MUST NOT store it as a
-// std::map<T> pointer or reference.  This will cause memory leaks, because the
-// destructor ~std::map<T> is not virtual.
 template<typename K, typename V, size_t N>
-class map : public std::map<K, V>
+class map : private std::map<K, V>
 {
 private:
   using parent = std::map<K, V>;
 
 public:
+  template<class... Args>
+  void emplace(Args&&... args)
+  {
+    parent::emplace(std::forward<Args>(args)...);
+  }
+
+  auto find(const K& key) { return parent::find(key); }
+  auto find(const K& key) const { return parent::find(key); }
+
   bool contains(const K& key) const { return this->count(key) > 0; }
 
+  const V& at(const K& key) const { return parent::at(key); }
+  V& at(const K& key) { return parent::at(key); }
+
+  void erase(const K& key) { parent::erase(key); }
+
   template<typename F>
   void erase_if_key(F&& f)
   {
-    for (auto iter = this->begin(); iter != this->end();) {
+    for (auto iter = parent::begin(); iter != parent::end();) {
       if (f(iter->first)) {
-        iter = this->erase(iter);
+        iter = parent::erase(iter);
       } else {
         ++iter;
       }

include/sframe/result.h

@@ -45,6 +45,8 @@ class SFrameError
 
 private:
   SFrameErrorType type_;
+  // Message storage is borrowed; callers must pass a string with static or
+  // otherwise stable lifetime.
   const char* message_ = nullptr;
 };
 

include/sframe/vector.h

@@ -29,12 +29,14 @@ class vector
 
   constexpr vector(std::initializer_list<uint8_t> content)
   {
+    std::fill(_data.begin(), _data.end(), T());
     resize(content.size());
     std::copy(content.begin(), content.end(), _data.begin());
   }
 
   constexpr vector(gsl::span<const T> content)
   {
+    std::fill(_data.begin(), _data.end(), T());
     resize(content.size());
     std::copy(content.begin(), content.end(), _data.begin());
   }
@@ -44,6 +46,7 @@ class vector
   template<size_t M>
   constexpr vector(const vector<T, M>& content)
   {
+    std::fill(_data.begin(), _data.end(), T());
     resize(content.size());
     std::copy(content.begin(), content.end(), _data.begin());
   }
@@ -95,15 +98,8 @@ class vector
 
 namespace SFRAME_NAMESPACE {
 
-// NOTE: NOT RECOMMENDED FOR USE OUTSIDE THIS LIBRARY
-//
-// We have used public inheritance from std::vector<T> to simplify the interface
-// here.  This works fine for the use cases we have within this library.  If you
-// choose to use this vector type outside this library, you MUST NOT store it as
-// a std::vector<T> pointer or reference.  This will cause memory leaks, because
-// the destructor ~std::vector<T> is not virtual.
 template<typename T, size_t N>
-class vector : public std::vector<T>
+class vector : private std::vector<T>
 {
 private:
   using parent = std::vector<T>;
@@ -126,16 +122,39 @@ class vector : public std::vector<T>
 
   template<size_t M>
   constexpr vector(const vector<T, M>& content)
-    : parent(content)
+    : parent(content.begin(), content.end())
   {
   }
 
+  T* data() { return parent::data(); }
+  const T* data() const { return parent::data(); }
+
+  auto begin() const { return parent::begin(); }
+  auto begin() { return parent::begin(); }
+
+  auto end() const { return parent::end(); }
+  auto end() { return parent::end(); }
+
+  auto size() const { return parent::size(); }
+  auto capacity() const { return parent::capacity(); }
+  void resize(size_t size) { parent::resize(size); }
+
+  auto& operator[](size_t i) { return parent::operator[](i); }
+  const auto& operator[](size_t i) const { return parent::operator[](i); }
+
   void append(gsl::span<const T> content)
   {
     const auto start = this->size();
     this->resize(start + content.size());
     std::copy(content.begin(), content.end(), this->begin() + start);
   }
+
+  operator gsl::span<const T>() const
+  {
+    return gsl::span(parent::data(), parent::size());
+  }
+
+  operator gsl::span<T>() { return gsl::span(parent::data(), parent::size()); }
 };
 
 } // namespace SFRAME_NAMESPACE

src/crypto.cpp

@@ -1,6 +1,10 @@
 #include "crypto.h"
 #include "header.h"
 
+#include <openssl/err.h>
+
+#include <climits>
+
 namespace SFRAME_NAMESPACE {
 
 Result<size_t>
@@ -94,4 +98,38 @@ cipher_overhead(CipherSuite suite)
   }
 }
 
+Result<int>
+checked_int(size_t size)
+{
+  if (size > INT_MAX) {
+    return SFrameError(SFrameErrorType::invalid_parameter_error,
+                       "Input too large for OpenSSL");
+  }
+
+  return static_cast<int>(size);
+}
+
+Result<void>
+validate_ctr_size(size_t size)
+{
+  static constexpr uint64_t max_ctr_size = uint64_t(1) << 36;
+  if (uint64_t(size) > max_ctr_size) {
+    return SFrameError(SFrameErrorType::invalid_parameter_error,
+                       "CTR input too large");
+  }
+
+  auto size_int = checked_int(size);
+  if (size_int.is_err()) {
+    return size_int.error();
+  }
+
+  return Result<void>::ok();
+}
+
+void
+clear_openssl_errors()
+{
+  ERR_clear_error();
+}
+
 } // namespace SFRAME_NAMESPACE

src/crypto.h

@@ -15,6 +15,12 @@ Result<size_t>
 cipher_nonce_size(CipherSuite suite);
 Result<size_t>
 cipher_overhead(CipherSuite suite);
+Result<int>
+checked_int(size_t size);
+Result<void>
+validate_ctr_size(size_t size);
+void
+clear_openssl_errors();
 
 ///
 /// HKDF

src/crypto_boringssl.cpp

@@ -69,6 +69,7 @@ openssl_cipher(CipherSuite suite)
 Result<owned_bytes<max_hkdf_expand_size>>
 hkdf_extract(CipherSuite suite, input_bytes salt, input_bytes ikm)
 {
+  clear_openssl_errors();
   SFRAME_VALUE_OR_RETURN(md, openssl_digest_type(suite));
   auto out = owned_bytes<max_hkdf_expand_size>(EVP_MD_size(md));
   auto out_len = size_t(out.size());
@@ -88,6 +89,7 @@ hkdf_extract(CipherSuite suite, input_bytes salt, input_bytes ikm)
 Result<owned_bytes<max_hkdf_extract_size>>
 hkdf_expand(CipherSuite suite, input_bytes prk, input_bytes info, size_t size)
 {
+  clear_openssl_errors();
   SFRAME_VALUE_OR_RETURN(md, openssl_digest_type(suite));
   auto out = owned_bytes<max_hkdf_expand_size>(size);
   if (1 != HKDF_expand(out.data(),
@@ -115,14 +117,15 @@ compute_tag(CipherSuite suite,
             input_bytes ct,
             size_t tag_size)
 {
+  clear_openssl_errors();
   using scoped_hmac_ctx = std::unique_ptr<HMAC_CTX, decltype(&HMAC_CTX_free)>;
 
   auto ctx = scoped_hmac_ctx(HMAC_CTX_new(), HMAC_CTX_free);
   SFRAME_VALUE_OR_RETURN(md, openssl_digest_type(suite));
 
   // Guard against sending nullptr to HMAC_Init_ex
   const auto* key_data = auth_key.data();
-  auto key_size = static_cast<int>(auth_key.size());
+  SFRAME_VALUE_OR_RETURN(key_size, checked_int(auth_key.size()));
   const auto non_null_zero_length_key = uint8_t(0);
   if (key_data == nullptr) {
     key_data = &non_null_zero_length_key;
@@ -173,6 +176,7 @@ ctr_crypt(CipherSuite suite,
           output_bytes out,
           input_bytes in)
 {
+  clear_openssl_errors();
   if (out.size() != in.size()) {
     return SFrameError(SFrameErrorType::buffer_too_small_error,
                        "CTR size mismatch");
@@ -194,12 +198,14 @@ ctr_crypt(CipherSuite suite,
   }
 
   int outlen = 0;
-  auto in_size_int = static_cast<int>(in.size());
+  SFRAME_VALUE_OR_RETURN(in_size_int, checked_int(in.size()));
   if (1 != EVP_EncryptUpdate(
              ctx.get(), out.data(), &outlen, in.data(), in_size_int)) {
     return SFrameErrorType::crypto_error;
   }
 
+  // CTR is a streaming mode, so finalization does not emit more bytes and a
+  // null output pointer is fine here.
   if (1 != EVP_EncryptFinal(ctx.get(), nullptr, &outlen)) {
     return SFrameErrorType::crypto_error;
   }
@@ -216,6 +222,7 @@ seal_ctr(CipherSuite suite,
          input_bytes pt)
 {
   SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite));
+  SFRAME_VOID_OR_RETURN(validate_ctr_size(pt.size()));
   if (ct.size() < pt.size() + tag_size) {
     return SFrameError(SFrameErrorType::buffer_too_small_error,
                        "Ciphertext buffer too small");
@@ -247,6 +254,7 @@ seal_aead(CipherSuite suite,
           input_bytes aad,
           input_bytes pt)
 {
+  clear_openssl_errors();
   SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite));
   if (ct.size() < pt.size() + tag_size) {
     return SFrameError(SFrameErrorType::buffer_too_small_error,
@@ -264,15 +272,15 @@ seal_aead(CipherSuite suite,
   }
 
   int outlen = 0;
-  auto aad_size_int = static_cast<int>(aad.size());
+  SFRAME_VALUE_OR_RETURN(aad_size_int, checked_int(aad.size()));
   if (aad.size() > 0) {
     if (1 != EVP_EncryptUpdate(
                ctx.get(), nullptr, &outlen, aad.data(), aad_size_int)) {
       return SFrameErrorType::crypto_error;
     }
   }
 
-  auto pt_size_int = static_cast<int>(pt.size());
+  SFRAME_VALUE_OR_RETURN(pt_size_int, checked_int(pt.size()));
   if (1 != EVP_EncryptUpdate(
              ctx.get(), ct.data(), &outlen, pt.data(), pt_size_int)) {
     return SFrameErrorType::crypto_error;
@@ -286,7 +294,7 @@ seal_aead(CipherSuite suite,
 
   auto tag = ct.subspan(pt.size(), tag_size);
   auto tag_ptr = const_cast<void*>(static_cast<const void*>(tag.data()));
-  auto tag_size_downcast = static_cast<int>(tag.size());
+  SFRAME_VALUE_OR_RETURN(tag_size_downcast, checked_int(tag.size()));
   if (1 != EVP_CIPHER_CTX_ctrl(
              ctx.get(), EVP_CTRL_GCM_GET_TAG, tag_size_downcast, tag_ptr)) {
     return SFrameErrorType::crypto_error;
@@ -334,6 +342,7 @@ open_ctr(CipherSuite suite,
   }
 
   auto inner_ct_size = ct.size() - tag_size;
+  SFRAME_VOID_OR_RETURN(validate_ctr_size(inner_ct_size));
   auto inner_ct = ct.subspan(0, inner_ct_size);
   auto tag = ct.subspan(inner_ct_size, tag_size);
 
@@ -365,6 +374,7 @@ open_aead(CipherSuite suite,
           input_bytes aad,
           input_bytes ct)
 {
+  clear_openssl_errors();
   SFRAME_VALUE_OR_RETURN(tag_size, cipher_overhead(suite));
   if (ct.size() < tag_size) {
     return SFrameError(SFrameErrorType::buffer_too_small_error,
@@ -389,22 +399,22 @@ open_aead(CipherSuite suite,
 
   auto tag = ct.subspan(inner_ct_size, tag_size);
   auto tag_ptr = const_cast<void*>(static_cast<const void*>(tag.data()));
-  auto tag_size_downcast = static_cast<int>(tag.size());
+  SFRAME_VALUE_OR_RETURN(tag_size_downcast, checked_int(tag.size()));
   if (1 != EVP_CIPHER_CTX_ctrl(
              ctx.get(), EVP_CTRL_GCM_SET_TAG, tag_size_downcast, tag_ptr)) {
     return SFrameErrorType::crypto_error;
   }
 
   int out_size;
-  auto aad_size_int = static_cast<int>(aad.size());
+  SFRAME_VALUE_OR_RETURN(aad_size_int, checked_int(aad.size()));
   if (aad.size() > 0) {
     if (1 != EVP_DecryptUpdate(
                ctx.get(), nullptr, &out_size, aad.data(), aad_size_int)) {
       return SFrameErrorType::crypto_error;
     }
   }
 
-  auto inner_ct_size_int = static_cast<int>(inner_ct_size);
+  SFRAME_VALUE_OR_RETURN(inner_ct_size_int, checked_int(inner_ct_size));
   if (1 != EVP_DecryptUpdate(
              ctx.get(), pt.data(), &out_size, ct.data(), inner_ct_size_int)) {
     return SFrameErrorType::crypto_error;