[tor-commits] [stegotorus/master] Crypto tweaks post-new chopper.

zwol at torproject.org zwol at torproject.org
Fri Jul 20 23:17:07 UTC 2012


commit 3ae30293e91c5b77dd29306045468682d4e3656e
Author: Zack Weinberg <zackw at panix.com>
Date:   Sat Mar 24 21:16:16 2012 -0700

    Crypto tweaks post-new chopper.
    
    * It is now possible to create an encryption or decryption object directly
      from a key generator.
    * The dangerous, no longer required decrypt_unchecked() method has been
      removed.
---
 src/crypt.cc               |   86 +++++++++++++++++++++++++++++++++++--------
 src/crypt.h                |   30 +++++++++++----
 src/protocol/chop.cc       |   33 ++++-------------
 src/test/unittest_crypt.cc |    5 ---
 4 files changed, 100 insertions(+), 54 deletions(-)

diff --git a/src/crypt.cc b/src/crypt.cc
index e81e8d4..f0f9c4f 100644
--- a/src/crypt.cc
+++ b/src/crypt.cc
@@ -14,6 +14,7 @@
 #include <cryptopp/hmac.h>
 #include <cryptopp/gcm.h>
 #include <cryptopp/sha.h>
+#include <cryptopp/secblock.h>
 
 // work around a bug in crypto++ 5.6.0's pwdbased.h
 #if CRYPTOPP_VERSION < 561
@@ -29,6 +30,9 @@ public:
 
 #include <cryptopp/pwdbased.h>
 
+// 32 bytes -> no extra memory allocation even for big keys, hopefully
+typedef CryptoPP::SecBlockWithHint<byte, 32> KeyBlock;
+
 /* Note: this file wraps a C++ library into a C-style program and must
    insulate that program from C++ semantics it is not prepared to handle;
    most importantly, all exceptions must be converted to error codes. */
@@ -72,6 +76,21 @@ ecb_encryptor::create(const uint8_t *key, size_t keylen)
   CATCH_ALL_EXCEPTIONS(0);
 }
 
+ecb_encryptor *
+ecb_encryptor::create(key_generator *gen, size_t keylen)
+{
+  try {
+    KeyBlock key(keylen);
+    size_t got = gen->generate(key, keylen);
+    log_assert(got == keylen);
+
+    ecb_encryptor_impl *enc = new ecb_encryptor_impl;
+    enc->ctx.SetKey(key, keylen);
+    return enc;
+  }
+  CATCH_ALL_EXCEPTIONS(0);
+}
+
 ecb_decryptor *
 ecb_decryptor::create(const uint8_t *key, size_t keylen)
 {
@@ -83,6 +102,21 @@ ecb_decryptor::create(const uint8_t *key, size_t keylen)
   CATCH_ALL_EXCEPTIONS(0);
 }
 
+ecb_decryptor *
+ecb_decryptor::create(key_generator *gen, size_t keylen)
+{
+  try {
+    KeyBlock key(keylen);
+    size_t got = gen->generate(key, keylen);
+    log_assert(got == keylen);
+
+    ecb_decryptor_impl *dec = new ecb_decryptor_impl;
+    dec->ctx.SetKey(key, keylen);
+    return dec;
+  }
+  CATCH_ALL_EXCEPTIONS(0);
+}
+
 ecb_encryptor::~ecb_encryptor() {}
 ecb_encryptor_impl::~ecb_encryptor_impl() {}
 ecb_decryptor::~ecb_decryptor() {}
@@ -121,9 +155,6 @@ namespace {
     virtual ~gcm_decryptor_impl();
     virtual int decrypt(uint8_t *out, const uint8_t *in, size_t inlen,
                         const uint8_t *nonce, size_t nlen);
-    virtual void decrypt_unchecked(uint8_t *out,
-                                   const uint8_t *in, size_t inlen,
-                                   const uint8_t *nonce, size_t nlen);
   };
 }
 
@@ -141,6 +172,24 @@ gcm_encryptor::create(const uint8_t *key, size_t keylen)
   CATCH_ALL_EXCEPTIONS(0);
 }
 
+gcm_encryptor *
+gcm_encryptor::create(key_generator *gen, size_t keylen)
+{
+  try {
+    KeyBlock key(keylen);
+    size_t got = gen->generate(key, keylen);
+    log_assert(got == keylen);
+
+    gcm_encryptor_impl *enc = new gcm_encryptor_impl;
+    // sadly, these are not checkable at compile time
+    log_assert(enc->ctx.DigestSize() == GCM_TAG_LEN);
+    log_assert(!enc->ctx.NeedsPrespecifiedDataLengths());
+    enc->ctx.SetKeyWithIV(key, keylen, dummy_iv, sizeof dummy_iv);
+    return enc;
+  }
+  CATCH_ALL_EXCEPTIONS(0);
+}
+
 gcm_decryptor *
 gcm_decryptor::create(const uint8_t *key, size_t keylen)
 {
@@ -155,6 +204,24 @@ gcm_decryptor::create(const uint8_t *key, size_t keylen)
   CATCH_ALL_EXCEPTIONS(0);
 }
 
+gcm_decryptor *
+gcm_decryptor::create(key_generator *gen, size_t keylen)
+{
+  try {
+    KeyBlock key(keylen);
+    size_t got = gen->generate(key, keylen);
+    log_assert(got == keylen);
+
+    gcm_decryptor_impl *dec = new gcm_decryptor_impl;
+    // sadly, these are not checkable at compile time
+    log_assert(dec->ctx.DigestSize() == GCM_TAG_LEN);
+    log_assert(!dec->ctx.NeedsPrespecifiedDataLengths());
+    dec->ctx.SetKeyWithIV(key, keylen, dummy_iv, sizeof dummy_iv);
+    return dec;
+  }
+  CATCH_ALL_EXCEPTIONS(0);
+}
+
 gcm_encryptor::~gcm_encryptor() {}
 gcm_encryptor_impl::~gcm_encryptor_impl() {}
 gcm_decryptor::~gcm_decryptor() {}
@@ -184,19 +251,6 @@ gcm_decryptor_impl::decrypt(uint8_t *out, const uint8_t *in, size_t inlen,
   CATCH_ALL_EXCEPTIONS(-1);
 }
 
-void
-gcm_decryptor_impl::decrypt_unchecked(uint8_t *out,
-                                      const uint8_t *in, size_t inlen,
-                                      const uint8_t *nonce, size_t nlen)
-{
-  try {
-    // there is no convenience function for this
-    this->ctx.Resynchronize(nonce, nlen);
-    this->ctx.ProcessData(out, in, inlen);
-  }
-  CATCH_ALL_EXCEPTIONS();
-}
-
 typedef CryptoPP::HMAC<CryptoPP::SHA256> HMAC_SHA256;
 typedef CryptoPP::PKCS5_PBKDF2_HMAC<CryptoPP::SHA256> PBKDF2_SHA256;
 const size_t SHA256_LEN = CryptoPP::SHA256::DIGESTSIZE;
diff --git a/src/crypt.h b/src/crypt.h
index c0084e9..4136a35 100644
--- a/src/crypt.h
+++ b/src/crypt.h
@@ -8,6 +8,8 @@
 const size_t AES_BLOCK_LEN = 16;
 const size_t GCM_TAG_LEN   = 16;
 
+struct key_generator;
+
 struct ecb_encryptor
 {
   ecb_encryptor() {}
@@ -17,6 +19,11 @@ struct ecb_encryptor
       as the symmetric key.  'keylen' must be 16, 24, or 32 bytes. */
   static ecb_encryptor *create(const uint8_t *key, size_t keylen);
 
+  /** Return a new AES/ECB encryption state, generating a key of
+      length 'keylen' from the key generator 'gen'.  'keylen' must be
+      16, 24, or 32 bytes. */
+  static ecb_encryptor *create(key_generator *gen, size_t keylen);
+
   /** Encrypt exactly AES_BLOCK_LEN bytes of data in the buffer 'in' and
       write the result to 'out'.  */
   virtual void encrypt(uint8_t *out, const uint8_t *in) = 0;
@@ -35,6 +42,11 @@ struct ecb_decryptor
       as the symmetric key.  'keylen' must be 16, 24, or 32 bytes. */
   static ecb_decryptor *create(const uint8_t *key, size_t keylen);
 
+  /** Return a new AES/ECB decryption state, generating a key of
+      length 'keylen' from the key generator 'gen'.  'keylen' must be
+      16, 24, or 32 bytes. */
+  static ecb_decryptor *create(key_generator *gen, size_t keylen);
+
   /** Decrypt exactly AES_BLOCK_LEN bytes of data in the buffer 'in' and
       write the result to 'out'.  */
   virtual void decrypt(uint8_t *out, const uint8_t *in) = 0;
@@ -54,6 +66,11 @@ struct gcm_encryptor
       as the symmetric key.  'keylen' must be 16, 24, or 32 bytes. */
   static gcm_encryptor *create(const uint8_t *key, size_t keylen);
 
+  /** Return a new AES/GCM encryption state, generating a key of
+      length 'keylen' from the key generator 'gen'.  'keylen' must be
+      16, 24, or 32 bytes. */
+  static gcm_encryptor *create(key_generator *gen, size_t keylen);
+
   /** Encrypt 'inlen' bytes of data in the buffer 'in', writing the
       result plus an authentication tag to the buffer 'out', whose
       length must be at least 'inlen'+16 bytes.  Use 'nonce'
@@ -76,6 +93,11 @@ struct gcm_decryptor
       as the symmetric key.  'keylen' must be 16, 24, or 32 bytes. */
   static gcm_decryptor *create(const uint8_t *key, size_t keylen);
 
+  /** Return a new AES/GCM decryption state, generating a key of
+      length 'keylen' from the key generator 'gen'.  'keylen' must be
+      16, 24, or 32 bytes. */
+  static gcm_decryptor *create(key_generator *gen, size_t keylen);
+
   /** Decrypt 'inlen' bytes of data in the buffer 'in'; the last 16
       bytes of this buffer are assumed to be the authentication tag.
       Write the result to the buffer 'out', whose length must be at
@@ -85,14 +107,6 @@ struct gcm_decryptor
   virtual int decrypt(uint8_t *out, const uint8_t *in, size_t inlen,
                       const uint8_t *nonce, size_t nlen) = 0;
 
-  /** Decrypt 'inlen' bytes of data in the buffer 'in' WITHOUT
-      CHECKING THE AUTHENTICATION TAG.  Arguments same as decrypt().
-      This should be used only to decode just enough of an incoming
-      block to know how long it's going to be and therefore where the
-      tag begins. */
-  virtual void decrypt_unchecked(uint8_t *out, const uint8_t *in, size_t inlen,
-                                 const uint8_t *nonce, size_t nlen) = 0;
-
 private:
   gcm_decryptor(const gcm_decryptor&) DELETE_METHOD;
   gcm_decryptor& operator=(const gcm_decryptor&) DELETE_METHOD;
diff --git a/src/protocol/chop.cc b/src/protocol/chop.cc
index e81214a..44e79a3 100644
--- a/src/protocol/chop.cc
+++ b/src/protocol/chop.cc
@@ -512,32 +512,17 @@ chop_config_t::circuit_create(size_t)
     key_generator::from_passphrase((const uint8_t *)passphrase,
                                    sizeof(passphrase) - 1,
                                    0, 0, 0, 0);
-  uint8_t kbuf[16];
 
   if (mode == LSN_SIMPLE_SERVER) {
-    log_assert(kgen->generate(kbuf, 16) == 16);
-    ckt->send_crypt = gcm_encryptor::create(kbuf, 16);
-
-    log_assert(kgen->generate(kbuf, 16) == 16);
-    ckt->send_hdr_crypt = ecb_encryptor::create(kbuf, 16);
-
-    log_assert(kgen->generate(kbuf, 16) == 16);
-    ckt->recv_crypt = gcm_decryptor::create(kbuf, 16);
-
-    log_assert(kgen->generate(kbuf, 16) == 16);
-    ckt->recv_hdr_crypt = ecb_decryptor::create(kbuf, 16);
+    ckt->send_crypt     = gcm_encryptor::create(kgen, 16);
+    ckt->send_hdr_crypt = ecb_encryptor::create(kgen, 16);
+    ckt->recv_crypt     = gcm_decryptor::create(kgen, 16);
+    ckt->recv_hdr_crypt = ecb_decryptor::create(kgen, 16);
   } else {
-    log_assert(kgen->generate(kbuf, 16) == 16);
-    ckt->recv_crypt = gcm_decryptor::create(kbuf, 16);
-
-    log_assert(kgen->generate(kbuf, 16) == 16);
-    ckt->recv_hdr_crypt = ecb_decryptor::create(kbuf, 16);
-
-    log_assert(kgen->generate(kbuf, 16) == 16);
-    ckt->send_crypt = gcm_encryptor::create(kbuf, 16);
-
-    log_assert(kgen->generate(kbuf, 16) == 16);
-    ckt->send_hdr_crypt = ecb_encryptor::create(kbuf, 16);
+    ckt->recv_crypt     = gcm_decryptor::create(kgen, 16);
+    ckt->recv_hdr_crypt = ecb_decryptor::create(kgen, 16);
+    ckt->send_crypt     = gcm_encryptor::create(kgen, 16);
+    ckt->send_hdr_crypt = ecb_encryptor::create(kgen, 16);
 
     std::pair<chop_circuit_table::iterator, bool> out;
     do {
@@ -551,7 +536,6 @@ chop_config_t::circuit_create(size_t)
     out.first->second = ckt;
   }
 
-  memset(kbuf, 0, 16);
   delete kgen;
   return ckt;
 }
@@ -1027,7 +1011,6 @@ chop_circuit_t::check_for_eof()
       conn_send_eof(conn);
     }
   }
-  
 
   // If we're the client we have to keep trying to talk as long as we
   // haven't both sent and received a FIN, or we might deadlock.
diff --git a/src/test/unittest_crypt.cc b/src/test/unittest_crypt.cc
index b903c11..2e50623 100644
--- a/src/test/unittest_crypt.cc
+++ b/src/test/unittest_crypt.cc
@@ -762,11 +762,6 @@ test_crypt_aesgcm_good_dec(void *)
     tt_int_op(rv, ==, 0);
     tt_mem_op(obuf, ==, testvecs[i].pt, testvecs[i].len);
 
-    c->decrypt_unchecked(obuf,
-                         (const uint8_t *)testvecs[i].ct, testvecs[i].len,
-                         (const uint8_t *)testvecs[i].iv, 16);
-    tt_mem_op(obuf, ==, testvecs[i].pt, testvecs[i].len);
-
     delete c;
   }
   c = 0;





More information about the tor-commits mailing list