common: Fix multiple VLA issues and add a unit test for encryption/de…

…cryption

common/Makefile.am

@@ -100,7 +100,8 @@ endif
 testrunner_SOURCES = \
 	testrunner.cc \
 	test-minicurl.cc \
-	test-serialize.cc
+	test-serialize.cc \
+	test-crypto.cc
 
 EXTRA_testrunner_DEPENDENCIES = \
 	$(EXT_LIBS) libweakforce.la

common/sodcrypto.cc

@@ -29,11 +29,17 @@
 
 #ifdef HAVE_LIBSODIUM
 
+string newKeyStr()
+{
+  string key;
+  key.resize(crypto_secretbox_KEYBYTES);
+  randombytes_buf(key.data(), crypto_secretbox_KEYBYTES);
+  return key;
+}
+
 string newKey()
 {
-  unsigned char key[crypto_secretbox_KEYBYTES];
-  randombytes_buf(key, sizeof key);
-  return "\""+Base64Encode(string((char*)key, sizeof key))+"\"";
+  return "\""+Base64Encode(newKeyStr())+"\"";
 }
 
 std::string sodEncryptSym(const std::string& msg, const std::string& key, SodiumNonce& nonce)
@@ -48,10 +54,10 @@ std::string sodEncryptSym(const std::string& msg, const std::string& key, Sodium
 
 std::string sodDecryptSym(const std::string& msg, const std::string& key, SodiumNonce& nonce)
 {
+  std::string decrypted;
   // It's fine if there's no data to decrypt
   if ((msg.length() - crypto_secretbox_MACBYTES) > 0) {
-    std::string decrypted;
-    decrypted.resize(msg.length() + crypto_secretbox_MACBYTES);
+    decrypted.resize(msg.length() - crypto_secretbox_MACBYTES);
 
     if (crypto_secretbox_open_easy((unsigned char*)decrypted.data(), (const unsigned char*)msg.c_str(),
          msg.length(), nonce.value, (const unsigned char*)key.c_str()) != 0) {
@@ -69,22 +75,28 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium
 
 #define POLY1305_BLOCK_SIZE 16
 
-string newKey()
+string newKeyStr()
 {
-  unsigned char key[CHACHA20_POLY1305_KEY_SIZE];
-  if (RAND_priv_bytes(key, sizeof key) != 1) {
+  string key;
+  key.resize(CHACHA20_POLY1305_KEY_SIZE);
+  if (RAND_priv_bytes((unsigned char*)key.data(), CHACHA20_POLY1305_KEY_SIZE) != 1) {
     throw std::runtime_error(
         "Could not initialize random number generator for cryptographic functions - this is not recoverable");
   }
-  return "\"" + Base64Encode(string((char*) key, sizeof key)) + "\"";
+  return key;
+}
+
+string newKey()
+{
+  return "\"" + Base64Encode(newKeyStr()) + "\"";
 }
 
 
 std::string sodEncryptSym(const std::string& msg, const std::string& key, SodiumNonce& nonce)
 {
-  unsigned char ciphertext[msg.length() + POLY1305_BLOCK_SIZE];
+  std::string ciphertext;
+  ciphertext.resize(msg.length() + POLY1305_BLOCK_SIZE);
   int len;
-  int ciphertext_len;
   // Each thread gets its own cipher context
   static thread_local auto ctx = std::unique_ptr<EVP_CIPHER_CTX, decltype(&EVP_CIPHER_CTX_free)>(nullptr, EVP_CIPHER_CTX_free);
 
@@ -107,26 +119,24 @@ std::string sodEncryptSym(const std::string& msg, const std::string& key, Sodium
     throw std::runtime_error("sodEncryptSym: EVP_EncryptInit_ex() could not initialize encryption key and IV");
 
   if (1 !=
-      EVP_EncryptUpdate(ctx.get(), ciphertext + POLY1305_BLOCK_SIZE, &len, (unsigned char*) msg.c_str(), msg.length()))
+      EVP_EncryptUpdate(ctx.get(), (unsigned char*)ciphertext.data() + POLY1305_BLOCK_SIZE, &len, (unsigned char*) msg.c_str(), msg.length()))
     throw std::runtime_error("sodEncryptSym: EVP_EncryptUpdate() could not encrypt message");
-  ciphertext_len = len;
 
-  if (1 != EVP_EncryptFinal_ex(ctx.get(), ciphertext + len + POLY1305_BLOCK_SIZE, &len))
+  if (1 != EVP_EncryptFinal_ex(ctx.get(), (unsigned char*)ciphertext.data() + len + POLY1305_BLOCK_SIZE, &len))
     throw std::runtime_error("sodEncryptSym: EVP_EncryptFinal_ex() could finalize message encryption");;
-  ciphertext_len += len;
 
   /* Get the tag */
-  if (1 != EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_GET_TAG, POLY1305_BLOCK_SIZE, ciphertext))
+  if (1 != EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_GET_TAG, POLY1305_BLOCK_SIZE, ciphertext.data()))
     throw std::runtime_error("sodEncryptSym: EVP_CIPHER_CTX_ctrl() could not get tag");
 
   nonce.increment();
-  return string((char*) ciphertext, sizeof(ciphertext));
+  return ciphertext;
 }
 
 std::string sodDecryptSym(const std::string& msg, const std::string& key, SodiumNonce& nonce)
 {
+  std::string plaintext;
   int len;
-  int plaintext_len;
   // Each thread gets its own cipher context
   static thread_local auto ctx = std::unique_ptr<EVP_CIPHER_CTX, decltype(&EVP_CIPHER_CTX_free)>(nullptr, EVP_CIPHER_CTX_free);
 
@@ -139,7 +149,7 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium
   if ((msg.length() - POLY1305_BLOCK_SIZE) > 0) {
 
     string tag = msg.substr(0, POLY1305_BLOCK_SIZE);
-    char plaintext[msg.length() - POLY1305_BLOCK_SIZE];
+    plaintext.resize(msg.length() - POLY1305_BLOCK_SIZE);
 
     if (ctx.get() == nullptr) {
       if (!(ctx = std::unique_ptr<EVP_CIPHER_CTX, decltype(&EVP_CIPHER_CTX_free)>(EVP_CIPHER_CTX_new(), EVP_CIPHER_CTX_free)))
@@ -152,21 +162,20 @@ std::string sodDecryptSym(const std::string& msg, const std::string& key, Sodium
     if (1 != EVP_DecryptInit_ex(ctx.get(), NULL, NULL, (unsigned char*) key.c_str(), nonce.value))
       throw std::runtime_error("sodDecryptSym: EVP_DecryptInit_ex() could not initialize decryption key and IV");
 
-    if (!EVP_DecryptUpdate(ctx.get(), (unsigned char*) plaintext, &len,
+    if (!EVP_DecryptUpdate(ctx.get(), (unsigned char*) plaintext.data(), &len,
                            (unsigned char*) (msg.c_str() + POLY1305_BLOCK_SIZE), msg.length() - POLY1305_BLOCK_SIZE))
       throw std::runtime_error("sodDecryptSym: EVP_DecryptUpdate() could not decrypt message");
-    plaintext_len = len;
 
     /* Set expected tag value. Works in OpenSSL 1.0.1d and later */
     if (!EVP_CIPHER_CTX_ctrl(ctx.get(), EVP_CTRL_AEAD_SET_TAG, POLY1305_BLOCK_SIZE, (void*) tag.c_str()))
       throw std::runtime_error("sodDecryptSym: EVP_CIPHER_CTX_ctrl() AEAD tag could not be validated");
 
-    if (!EVP_DecryptFinal_ex(ctx.get(), (unsigned char*) (plaintext + len), &len))
+    if (!EVP_DecryptFinal_ex(ctx.get(), (unsigned char*) (plaintext.data() + len), &len))
       throw std::runtime_error("sodDecryptSym: EVP_DecryptFinal_ex() failed - plaintext cannot be trusted");
 
     nonce.increment();
 
-    return string(plaintext, plaintext_len);
+    return plaintext;
   }
   else {
     nonce.increment();

common/sodcrypto.hh

@@ -59,9 +59,9 @@ struct SodiumNonce
     *p=ntohl(count);
   }
 
-  string toString() const
+  std::string toString() const
   {
-    return string((const char*)value, sizeof value);
+    return std::string((const char*)value, sizeof value);
   }
 
   unsigned char value[CHACHA20_POLY1305_IV_SIZE];
@@ -91,15 +91,15 @@ struct SodiumNonce
     *p=ntohl(count);
   }
 
-  string toString() const
+  std::string toString() const
   {
-    return string((const char*)value, crypto_secretbox_NONCEBYTES);
+    return std::string((const char*)value, crypto_secretbox_NONCEBYTES);
   }
 
   unsigned char value[crypto_secretbox_NONCEBYTES];
 };
 #endif
-std::string newKeypair();
+std::string newKeyStr();
 std::string sodEncryptSym(const std::string& msg, const std::string& key, SodiumNonce&);
 std::string sodDecryptSym(const std::string& msg, const std::string& key, SodiumNonce&);
 std::string newKey();