Try to ensure the compiler cannot optimise away attempts to erase secrets from memory

5 files changed, 59 insertions, 28 deletions

diff --git a/include/axolotl/crypto.hh b/include/axolotl/crypto.hh
index 09e5b8e..162099f 100644
--- a/include/axolotl/crypto.hh
+++ b/include/axolotl/crypto.hh
@@ -28,6 +28,7 @@ struct Curve25519KeyPair : public Curve25519PublicKey {
     std::uint8_t private_key[32];
 };
 
+
 /** Generate a curve25519 key pair from 32 random bytes. */
 void generate_key(
     std::uint8_t const * random_32_bytes,
@@ -37,6 +38,7 @@ void generate_key(
 
 const std::size_t CURVE25519_SHARED_SECRET_LENGTH = 32;
 
+
 /** Create a shared secret using our private key and their public key.
  * The output buffer must be at least 32 bytes long. */
 void curve25519_shared_secret(
diff --git a/include/axolotl/memory.hh b/include/axolotl/memory.hh
new file mode 100644
index 0000000..7749c54
--- /dev/null
+++ b/include/axolotl/memory.hh
@@ -0,0 +1,17 @@
+#include <cstddef>
+
+namespace axolotl {
+
+/** Clear the memory held in the buffer */
+void unset(
+    volatile void * buffer, std::size_t buffer_length
+);
+
+/** Clear the memory backing an object */
+template<typename T>
+void unset(T & value) {
+    unset(reinterpret_cast<volatile void *>(&value), sizeof(T));
+}
+
+
+} // namespace axolotl
diff --git a/src/axolotl.cpp b/src/axolotl.cpp
index cf49d31..0f0d226 100644
--- a/src/axolotl.cpp
+++ b/src/axolotl.cpp
@@ -14,6 +14,7 @@
  */
 #include "axolotl/axolotl.hh"
 #include "axolotl/message.hh"
+#include "axolotl/memory.hh"
 
 #include <cstring>
 
@@ -26,14 +27,6 @@ std::uint8_t MESSAGE_KEY_SEED[1] = {0x01};
 std::uint8_t CHAIN_KEY_SEED[1] = {0x02};
 std::size_t MAX_MESSAGE_GAP = 2000;
 
-template<typename T>
-void unset(
-    T & value
-) {
-    std::memset(&value, 0, sizeof(T));
-}
-
-
 void create_chain_key(
     axolotl::SharedKey const & root_key,
     axolotl::Curve25519KeyPair const & our_key,
@@ -54,8 +47,8 @@ void create_chain_key(
     std::memcpy(new_root_key, derived_secrets, 32);
     std::memcpy(new_chain_key.key, derived_secrets + 32, 32);
     new_chain_key.index = 0;
-    unset(derived_secrets);
-    unset(secret);
+    axolotl::unset(derived_secrets);
+    axolotl::unset(secret);
 }
 
 
@@ -94,8 +87,8 @@ void create_message_keys(
     std::memcpy(message_key.mac_key, derived_secrets + 32, 32);
     std::memcpy(message_key.iv.iv, derived_secrets + 64, 16);
     message_key.index = chain_key.index;
-    unset(derived_secrets);
-    unset(secret);
+    axolotl::unset(derived_secrets);
+    axolotl::unset(secret);
 }
 
 
@@ -112,7 +105,7 @@ bool verify_mac(
     );
 
     bool result = std::memcmp(mac, reader.mac, MAC_LENGTH) == 0;
-    unset(mac);
+    axolotl::unset(mac);
     return result;
 }
 
@@ -142,7 +135,7 @@ bool verify_mac_for_existing_chain(
     create_message_keys(new_chain, session.kdf_info, message_key);
 
     bool result = verify_mac(message_key, input, reader);
-    unset(new_chain);
+    axolotl::unset(new_chain);
     return result;
 }
 
@@ -178,8 +171,8 @@ bool verify_mac_for_new_chain(
     bool result = verify_mac_for_existing_chain(
         session, new_chain.chain_key, input, reader
     );
-    unset(new_root_key);
-    unset(new_chain);
+    axolotl::unset(new_root_key);
+    axolotl::unset(new_chain);
     return result;
 }
 
@@ -207,7 +200,7 @@ void axolotl::Session::initialise_as_bob(
     std::memcpy(root_key, derived_secrets, 32);
     std::memcpy(receiver_chains[0].chain_key.key, derived_secrets + 32, 32);
     receiver_chains[0].ratchet_key = their_ratchet_key;
-    unset(derived_secrets);
+    axolotl::unset(derived_secrets);
 }
 
 
@@ -226,7 +219,7 @@ void axolotl::Session::initialise_as_alice(
     std::memcpy(root_key, derived_secrets, 32);
     std::memcpy(sender_chain[0].chain_key.key, derived_secrets + 32, 32);
     sender_chain[0].ratchet_key = our_ratchet_key;
-    unset(derived_secrets);
+    axolotl::unset(derived_secrets);
 }
 
 
@@ -303,7 +296,7 @@ std::size_t axolotl::Session::encrypt(
     );
     std::memcpy(writer.mac, mac, MAC_LENGTH);
 
-    unset(keys);
+    axolotl::unset(keys);
     return writer.body_length + MAC_LENGTH;
 }
 
@@ -385,7 +378,7 @@ std::size_t axolotl::Session::decrypt(
 
                     /* Remove the key from the skipped keys now that we've
                      * decoded the message it corresponds to. */
-                    unset(skipped);
+                    axolotl::unset(skipped);
                     skipped_message_keys.erase(&skipped);
                     return result;
                 }
@@ -414,7 +407,7 @@ std::size_t axolotl::Session::decrypt(
             root_key, sender_chain[0].ratchet_key, chain->ratchet_key,
             kdf_info, root_key, chain->chain_key
         );
-        unset(sender_chain[0]);
+        axolotl::unset(sender_chain[0]);
         sender_chain.erase(sender_chain.begin());
     }
 
@@ -433,7 +426,7 @@ std::size_t axolotl::Session::decrypt(
         reader.ciphertext, reader.ciphertext_length,
         plaintext
     );
-    unset(message_key);
+    axolotl::unset(message_key);
 
     advance_chain_key(chain->chain_key, chain->chain_key);
 
diff --git a/src/crypto.cpp b/src/crypto.cpp
index a219f36..57f31cd 100644
--- a/src/crypto.cpp
+++ b/src/crypto.cpp
@@ -13,6 +13,8 @@
  * limitations under the License.
  */
 #include "axolotl/crypto.hh"
+#include "axolotl/memory.hh"
+
 #include <cstring>
 
 extern "C" {
@@ -76,7 +78,7 @@ inline void hmac_sha256_init(
     }
     ::sha256_init(context);
     ::sha256_update(context, i_pad, SHA256_BLOCK_LENGTH);
-    std::memset(i_pad, 0, sizeof(i_pad));
+    axolotl::unset(i_pad);
 }
 
 
@@ -95,7 +97,8 @@ inline void hmac_sha256_final(
     ::sha256_init(&final_context);
     ::sha256_update(&final_context, o_pad, sizeof(o_pad));
     ::sha256_final(&final_context, output);
-    std::memset(o_pad, 0, sizeof(o_pad));
+    axolotl::unset(final_context);
+    axolotl::unset(o_pad);
 }
 
 } // namespace
@@ -154,8 +157,8 @@ void axolotl::aes_encrypt_cbc(
         input_block[i] ^= AES_BLOCK_LENGTH - input_length;
     }
     ::aes_encrypt(input_block, output, key_schedule, 256);
-    std::memset(key_schedule, 0, sizeof(key_schedule));
-    std::memset(input_block, 0, sizeof(AES_BLOCK_LENGTH));
+    axolotl::unset(key_schedule);
+    axolotl::unset(input_block);
 }
 
 
@@ -175,7 +178,7 @@ std::size_t axolotl::aes_decrypt_cbc(
             xor_block<AES_BLOCK_LENGTH>(&output[i], &input[i - AES_BLOCK_LENGTH]);
         }
     }
-    std::memset(key_schedule, 0, sizeof(key_schedule));
+    axolotl::unset(key_schedule);
     std::size_t padding = output[input_length - 1];
     return (padding > input_length) ? std::size_t(-1) : (input_length - padding);
 }
@@ -189,6 +192,7 @@ void axolotl::sha256(
     ::sha256_init(&context);
     ::sha256_update(&context, input, input_length);
     ::sha256_final(&context, output);
+    axolotl::unset(context);
 }
 
 void axolotl::hmac_sha256(
@@ -202,7 +206,8 @@ void axolotl::hmac_sha256(
     hmac_sha256_init(&context, hmac_key);
     ::sha256_update(&context, input, input_length);
     hmac_sha256_final(&context, hmac_key, output);
-    std::memset(hmac_key, 0, sizeof(hmac_key));
+    axolotl::unset(hmac_key);
+    axolotl::unset(context);
 }
 
 
@@ -245,4 +250,7 @@ void axolotl::hkdf_sha256(
         hmac_sha256_final(&context, hmac_key, step_result);
     }
     std::memcpy(output, step_result, bytes_remaining);
+    axolotl::unset(context);
+    axolotl::unset(hmac_key);
+    axolotl::unset(step_result);
 }
diff --git a/src/memory.cpp b/src/memory.cpp
new file mode 100644
index 0000000..14c95dd
--- /dev/null
+++ b/src/memory.cpp
@@ -0,0 +1,11 @@
+#include "axolotl/memory.hh"
+
+void axolotl::unset(
+    volatile void * buffer, std::size_t buffer_length
+) {
+    volatile char * pos = reinterpret_cast<volatile char *>(buffer);
+    volatile char * end = pos + buffer_length;
+    while (pos != end) {
+        *(pos++) = 0;
+    }
+}