[tor-commits] [stegotorus/master] Switch back to OpenSSL as cryptography library.

commit 069607870199763ab539fc7d458646a6c3ee5f4b Author: Zack Weinberg zackw@panix.com Date: Tue Mar 27 21:02:20 2012 -0700

Switch back to OpenSSL as cryptography library. --- README.Linda | 4 +- configure.ac | 9 +- src/audit-globals.sh | 2 + src/crypt.cc | 598 +++++++++++++++++++++++++++----------------- src/crypt.h | 1 + src/rng.cc | 260 +++++++++---------- src/rng.h | 2 +- src/test/unittest.cc | 6 +- src/test/unittest_crypt.cc | 6 +- src/util.h | 1 + 10 files changed, 515 insertions(+), 374 deletions(-)

diff --git a/README.Linda b/README.Linda index 83e0dad..3cb4d2e 100644 --- a/README.Linda +++ b/README.Linda @@ -22,9 +22,9 @@ automake (GNU automake) 1.11.1

$> sudo port install pkgconfig

- d) libcrypto++, libevent-2, + d) openssl >= 1.0.1, libevent-2

-$> sudo port install libcryptopp +$> sudo port install openssl $> sudo port install libevent

e) tor diff --git a/configure.ac b/configure.ac index d491479..9002078 100644 --- a/configure.ac +++ b/configure.ac @@ -43,16 +43,17 @@ AM_CONDITIONAL([INTEGRATION_TESTS], [test "$PYOS" = "posix"])

### Libraries ###

-# libcrypto++ 5.6.0 is the earliest version with GCM support. -PKG_CHECK_MODULES([libcryptopp], [libcrypto++ >= 5.6.0]) +# Presently no need for libssl, only libcrypto. +# We require version 1.0.1 for GCM support. +PKG_CHECK_MODULES([libcrypto], [libcrypto >= 1.0.1]) # libevent 2.0 radically changed the API PKG_CHECK_MODULES([libevent], [libevent >= 2.0]) # there's no good reason not to require the latest zlib, which is # from 2009 PKG_CHECK_MODULES([libz], [zlib >= 1.2.3.4])

-LIBS="$libevent_LIBS $libcryptopp_LIBS $libz_LIBS" -lib_CPPFLAGS="$libevent_CFLAGS $libcryptopp_CFLAGS $libz_CFLAGS" +LIBS="$libevent_LIBS $libcrypto_LIBS $libz_LIBS" +lib_CPPFLAGS="$libevent_CFLAGS $libcrypto_CFLAGS $libz_CFLAGS" AC_SUBST(lib_CPPFLAGS)

# ntohl and a bunch of related functions require a special library on Windows. diff --git a/src/audit-globals.sh b/src/audit-globals.sh index e1de07d..ec08b21 100644 --- a/src/audit-globals.sh +++ b/src/audit-globals.sh @@ -40,6 +40,8 @@ sed ' /^util log_dest$/d /^util log_min_sev$/d /^util the_evdns_base$/d + /^crypt log_crypto()::initialized$/d + /^crypt init_crypto()::initialized$/d

# These are grandfathered; they need to be removed. /^steg/embed embed_init$/d diff --git a/src/crypt.cc b/src/crypt.cc index f0f9c4f..d964708 100644 --- a/src/crypt.cc +++ b/src/crypt.cc @@ -5,61 +5,137 @@ #include "util.h" #include "crypt.h"

-// temporary, I hope, kludge -#ifdef __APPLE_CC__ -#define CRYPTOPP_DISABLE_X86ASM -#endif - -#include <cryptopp/aes.h> -#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 -namespace CryptoPP { -class Integer { -public: - Integer& operator++(); - Integer& operator+(Integer const&); - void Encode(...); -}; +#include <openssl/engine.h> +#include <openssl/err.h> +#include <openssl/evp.h> +#include <openssl/hmac.h> + +static void +init_crypto() +{ + static bool initialized = false; + if (!initialized) { + initialized = true; + ENGINE_load_builtin_engines(); + ENGINE_register_all_complete(); + atexit(ENGINE_cleanup); + + // we don't need to call OpenSSL_add_all_algorithms or EVP_cleanup, + // since we never look up ciphers by textual name. + } } -#endif

-#include <cryptopp/pwdbased.h> +static void +log_crypto() +{ + static bool initialized = false; + if (!initialized) { + initialized = true; + ERR_load_crypto_strings(); + atexit(ERR_free_strings); + } + unsigned long err; + while ((err = ERR_get_error()) != 0) + log_warn("%s: %s: %s", + ERR_lib_error_string(err), + ERR_func_error_string(err), + ERR_reason_error_string(err)); +}

-// 32 bytes -> no extra memory allocation even for big keys, hopefully -typedef CryptoPP::SecBlockWithHint<byte, 32> KeyBlock; +static void ATTR_NORETURN +log_crypto_abort(const char *msg) +{ + log_crypto(); + log_abort("libcrypto error in %s", msg); +}

-/* 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. */ +static int +log_crypto_warn(const char *msg) +{ + log_crypto(); + log_warn("libcrypto error in %s", msg); + return -1; +}

-#define CATCH_ALL_EXCEPTIONS(rv) \ - catch (std::exception& e) { \ - log_warn("%s: %s", __func__, e.what()); \ - } catch (...) { \ - log_warn("%s: exception of abnormal type", __func__); \ - } \ - return rv /* deliberate absence of semicolon */ +static const EVP_CIPHER * +aes_ecb_by_size(size_t keylen) +{ + switch (keylen) { + case 16: return EVP_aes_128_ecb(); + case 24: return EVP_aes_192_ecb(); + case 32: return EVP_aes_256_ecb(); + default: + log_abort("AES only supports 16, 24, or 32-byte keys"); + } +}

-// Crypto++ doesn't let us set a key without also setting an IV, -// even though we will always override the IV later. -static const uint8_t dummy_iv[16] = {}; +static const EVP_CIPHER * +aes_gcm_by_size(size_t keylen) +{ + switch (keylen) { + case 16: return EVP_aes_128_gcm(); + case 24: return EVP_aes_192_gcm(); + case 32: return EVP_aes_256_gcm(); + default: + log_abort("AES only supports 16, 24, or 32-byte keys"); + } +}

namespace { + + // loosely based on crypto++'s SecByteBlock + class MemBlock { + public: + explicit MemBlock(size_t l) : data(new uint8_t[l]), len(l) + { memset(data, 0, l); } + + MemBlock(const uint8_t *d, size_t l) : data(new uint8_t[l]), len(l) + { if (d) memcpy(data, d, l); else memset(data, 0, l); } + + ~MemBlock() + { memset(data, 0, len); delete [] data; } + + operator const void*() const + { return data; } + operator void*() + { return data; } + + operator const uint8_t*() const + { return data; } + operator uint8_t*() + { return data; } + + const uint8_t *begin() const + { return data; } + uint8_t *begin() + { return data; } + + const uint8_t *end() const + { return data+len; } + uint8_t *end() + { return data+len; } + + size_t size() const { return len; } + + private: + MemBlock(MemBlock const&) DELETE_METHOD; + + uint8_t *data; + size_t len; + }; + struct ecb_encryptor_impl : ecb_encryptor { - CryptoPP::ECB_ModeCryptoPP::AES::Encryption ctx; + EVP_CIPHER_CTX ctx; + ecb_encryptor_impl() { EVP_CIPHER_CTX_init(&ctx); } virtual ~ecb_encryptor_impl(); virtual void encrypt(uint8_t *out, const uint8_t *in); };

struct ecb_decryptor_impl : ecb_decryptor { - CryptoPP::ECB_ModeCryptoPP::AES::Decryption ctx; + EVP_CIPHER_CTX ctx; + ecb_decryptor_impl() { EVP_CIPHER_CTX_init(&ctx); } virtual ~ecb_decryptor_impl(); virtual void decrypt(uint8_t *out, const uint8_t *in); }; @@ -68,82 +144,98 @@ namespace { ecb_encryptor * ecb_encryptor::create(const uint8_t *key, size_t keylen) { - try { - ecb_encryptor_impl *enc = new ecb_encryptor_impl; - enc->ctx.SetKey(key, keylen); - return enc; - } - CATCH_ALL_EXCEPTIONS(0); + init_crypto(); + + ecb_encryptor_impl *enc = new ecb_encryptor_impl; + if (!EVP_EncryptInit_ex(&enc->ctx, aes_ecb_by_size(keylen), 0, key, 0)) + log_crypto_abort("ecb_encryptor::create"); + if (!EVP_CIPHER_CTX_set_padding(&enc->ctx, 0)) + log_crypto_abort("ecb_encryptor::disable padding"); + + return enc; }

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); + init_crypto(); + + MemBlock key(keylen); + size_t got = gen->generate(key, keylen); + log_assert(got == keylen); + + ecb_encryptor_impl *enc = new ecb_encryptor_impl; + if (!EVP_EncryptInit_ex(&enc->ctx, aes_ecb_by_size(keylen), 0, key, 0)) + log_crypto_abort("ecb_encryptor::create"); + if (!EVP_CIPHER_CTX_set_padding(&enc->ctx, 0)) + log_crypto_abort("ecb_encryptor::disable padding"); + + return enc; }

ecb_decryptor * ecb_decryptor::create(const uint8_t *key, size_t keylen) { - try { - ecb_decryptor_impl *dec = new ecb_decryptor_impl; - dec->ctx.SetKey(key, keylen); - return dec; - } - CATCH_ALL_EXCEPTIONS(0); + init_crypto(); + + ecb_decryptor_impl *dec = new ecb_decryptor_impl; + if (!EVP_DecryptInit_ex(&dec->ctx, aes_ecb_by_size(keylen), 0, key, 0)) + log_crypto_abort("ecb_decryptor::create"); + if (!EVP_CIPHER_CTX_set_padding(&dec->ctx, 0)) + log_crypto_abort("ecb_decryptor::disable padding"); + + return dec; }

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); + init_crypto(); + + MemBlock key(keylen); + size_t got = gen->generate(key, keylen); + log_assert(got == keylen); + + ecb_decryptor_impl *dec = new ecb_decryptor_impl; + if (!EVP_DecryptInit_ex(&dec->ctx, aes_ecb_by_size(keylen), 0, key, 0)) + log_crypto_abort("ecb_decryptor::create"); + if (!EVP_CIPHER_CTX_set_padding(&dec->ctx, 0)) + log_crypto_abort("ecb_decryptor::disable padding"); + + return dec; }

ecb_encryptor::~ecb_encryptor() {} -ecb_encryptor_impl::~ecb_encryptor_impl() {} +ecb_encryptor_impl::~ecb_encryptor_impl() +{ EVP_CIPHER_CTX_cleanup(&ctx); } + ecb_decryptor::~ecb_decryptor() {} -ecb_decryptor_impl::~ecb_decryptor_impl() {} +ecb_decryptor_impl::~ecb_decryptor_impl() +{ EVP_CIPHER_CTX_cleanup(&ctx); }

void ecb_encryptor_impl::encrypt(uint8_t *out, const uint8_t *in) { - try { - this->ctx.ProcessData(out, in, AES_BLOCK_LEN); - } - CATCH_ALL_EXCEPTIONS(); + int olen; + if (!EVP_EncryptUpdate(&ctx, out, &olen, in, AES_BLOCK_LEN) || + size_t(olen) != AES_BLOCK_LEN) + log_crypto_abort("ecb_encryptor::encrypt"); }

void ecb_decryptor_impl::decrypt(uint8_t *out, const uint8_t *in) { - try { - this->ctx.ProcessData(out, in, AES_BLOCK_LEN); - } - CATCH_ALL_EXCEPTIONS(); + int olen; + if (!EVP_DecryptUpdate(&ctx, out, &olen, in, AES_BLOCK_LEN) || + size_t(olen) != AES_BLOCK_LEN) + log_crypto_abort("ecb_decryptor::decrypt"); }

namespace { struct gcm_encryptor_impl : gcm_encryptor { - CryptoPP::GCMCryptoPP::AES::Encryption ctx; + EVP_CIPHER_CTX ctx; + gcm_encryptor_impl() { EVP_CIPHER_CTX_init(&ctx); } virtual ~gcm_encryptor_impl(); virtual void encrypt(uint8_t *out, const uint8_t *in, size_t inlen, const uint8_t *nonce, size_t nlen); @@ -151,158 +243,202 @@ namespace {

struct gcm_decryptor_impl : gcm_decryptor { - CryptoPP::GCMCryptoPP::AES::Decryption ctx; + EVP_CIPHER_CTX ctx; + gcm_decryptor_impl() { EVP_CIPHER_CTX_init(&ctx); } virtual ~gcm_decryptor_impl(); virtual int decrypt(uint8_t *out, const uint8_t *in, size_t inlen, const uint8_t *nonce, size_t nlen); }; }

+// It *appears* (from inspecting the guts of libcrypto, *not* from the +// documentation) that - at least for AES-GCM - it is legitimate to +// call EVP_EncryptInit once with a key but no IV, and then once per +// block with an IV but no key. If this doesn't turn out to work, +// there are also some (completely undocumented, feh) EVP_CTRL_* ops +// that might help, but in the worst case we may have to save the key +// at creation time, and delay the EVP_EncryptInit call to when we +// know both key and IV, i.e. at block-encryption time. This would be +// unfortunate since it would entail doing AES key expansion once per +// block instead of once per key. + +// It also *appears* (again, from inspecting the guts of libcrypto) +// that we do not have to worry about EVP_EncryptFinal trying to do +// PKCS padding when GCM is in use. If this is wrong we will have to +// find some way to make it not happen, which might entail ditching +// EVP. Feh, I say, feh. + gcm_encryptor * gcm_encryptor::create(const uint8_t *key, size_t keylen) { - try { - 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); + init_crypto(); + + gcm_encryptor_impl *enc = new gcm_encryptor_impl; + if (!EVP_EncryptInit_ex(&enc->ctx, aes_gcm_by_size(keylen), 0, key, 0)) + log_crypto_abort("gcm_encryptor::create"); + + return enc; }

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); + init_crypto(); + + MemBlock key(keylen); + size_t got = gen->generate(key, keylen); + log_assert(got == keylen); + + gcm_encryptor_impl *enc = new gcm_encryptor_impl; + if (!EVP_EncryptInit_ex(&enc->ctx, aes_gcm_by_size(keylen), 0, key, 0)) + log_crypto_abort("gcm_encryptor::create"); + + return enc; }

gcm_decryptor * gcm_decryptor::create(const uint8_t *key, size_t keylen) { - try { - 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); + init_crypto(); + + gcm_decryptor_impl *dec = new gcm_decryptor_impl; + if (!EVP_DecryptInit_ex(&dec->ctx, aes_gcm_by_size(keylen), 0, key, 0)) + log_crypto_abort("gcm_decryptor::create"); + + return dec; }

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); + init_crypto(); + + MemBlock key(keylen); + size_t got = gen->generate(key, keylen); + log_assert(got == keylen); + + gcm_decryptor_impl *dec = new gcm_decryptor_impl; + if (!EVP_DecryptInit_ex(&dec->ctx, aes_gcm_by_size(keylen), 0, key, 0)) + log_crypto_abort("gcm_decryptor::create"); + + return dec; }

gcm_encryptor::~gcm_encryptor() {} -gcm_encryptor_impl::~gcm_encryptor_impl() {} +gcm_encryptor_impl::~gcm_encryptor_impl() +{ EVP_CIPHER_CTX_cleanup(&ctx); } gcm_decryptor::~gcm_decryptor() {} -gcm_decryptor_impl::~gcm_decryptor_impl() {} +gcm_decryptor_impl::~gcm_decryptor_impl() +{ EVP_CIPHER_CTX_cleanup(&ctx); }

void gcm_encryptor_impl::encrypt(uint8_t *out, const uint8_t *in, size_t inlen, const uint8_t *nonce, size_t nlen) { - try { - this->ctx.EncryptAndAuthenticate(out, out + inlen, 16, - nonce, nlen, 0, 0, in, inlen); - } - CATCH_ALL_EXCEPTIONS(); + log_assert(inlen <= size_t(INT_MAX)); + + if (nlen != size_t(EVP_CIPHER_CTX_iv_length(&ctx))) + if (!EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_SET_IVLEN, nlen, 0)) + log_crypto_abort("gcm_encryptor::reset nonce length"); + + if (!EVP_EncryptInit_ex(&ctx, 0, 0, 0, nonce)) + log_crypto_abort("gcm_encryptor::set nonce"); + + int olen; + if (!EVP_EncryptUpdate(&ctx, 0, &olen, (const uint8_t *)"", 0) || olen != 0) + log_crypto_abort("gcm_encryptor::set null AAD"); + + if (!EVP_EncryptUpdate(&ctx, out, &olen, in, inlen) || size_t(olen) != inlen) + log_crypto_abort("gcm_encryptor::encrypt"); + + if (!EVP_EncryptFinal_ex(&ctx, out + inlen, &olen) || olen != 0) + log_crypto_abort("gcm_encryptor::finalize"); + + if (!EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_GET_TAG, 16, out + inlen)) + log_crypto_abort("gcm_encryptor::write tag"); }

int gcm_decryptor_impl::decrypt(uint8_t *out, const uint8_t *in, size_t inlen, const uint8_t *nonce, size_t nlen) { - try { - return this->ctx.DecryptAndVerify(out, - in + inlen - 16, 16, - nonce, nlen, 0, 0, in, inlen - 16) - ? 0 : -1; // caller will log decryption failure - } - CATCH_ALL_EXCEPTIONS(-1); -} + log_assert(inlen <= size_t(INT_MAX)); + + if (nlen != size_t(EVP_CIPHER_CTX_iv_length(&ctx))) + if (!EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_SET_IVLEN, nlen, 0)) + log_crypto_abort("gcm_decryptor::reset nonce length"); + + if (!EVP_DecryptInit_ex(&ctx, 0, 0, 0, nonce)) + return log_crypto_warn("gcm_decryptor::set nonce");

-typedef CryptoPP::HMACCryptoPP::SHA256 HMAC_SHA256; -typedef CryptoPP::PKCS5_PBKDF2_HMACCryptoPP::SHA256 PBKDF2_SHA256; -const size_t SHA256_LEN = CryptoPP::SHA256::DIGESTSIZE; + if (!EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_SET_TAG, 16, + (void *)(in + inlen - 16))) + return log_crypto_warn("gcm_decryptor::set tag"); + + int olen; + if (!EVP_DecryptUpdate(&ctx, 0, &olen, (const uint8_t *)"", 0) || olen != 0) + return log_crypto_warn("gcm_decryptor::set null AAD"); + + inlen -= 16; + if (!EVP_DecryptUpdate(&ctx, out, &olen, in, inlen) || size_t(olen) != inlen) + return log_crypto_warn("gcm_encryptor::decrypt"); + + if (!EVP_DecryptFinal_ex(&ctx, out + inlen, &olen) || olen != 0) + return log_crypto_warn("gcm_decryptor::check tag"); + + return 0; +}

namespace { struct key_generator_impl : key_generator { - HMAC_SHA256 expander; - CryptoPP::SecByteBlock prevT; - CryptoPP::SecByteBlock info; + HMAC_CTX expander; + MemBlock prevT; + MemBlock info; + uint8_t counter; uint8_t leftover; - bool dead : 1; + bool dead;

virtual ~key_generator_impl(); virtual size_t generate(uint8_t *buf, size_t len);

- key_generator_impl(const uint8_t *prk, - const uint8_t *info, size_t ilen) - : expander(prk, SHA256_LEN), - prevT(0), + key_generator_impl(const uint8_t *prk, const uint8_t *info, size_t ilen) + : prevT(SHA256_LEN), info(info, ilen), counter(1), leftover(0), dead(false) - {} + { + HMAC_CTX_init(&expander); + if (!HMAC_Init_ex(&expander, prk, SHA256_LEN, EVP_sha256(), 0)) + log_crypto_abort("key_generator_impl::construction"); + } }; }

+static const uint8_t nosalt[SHA256_LEN] = {}; + key_generator * key_generator::from_random_secret(const uint8_t *key, size_t klen, const uint8_t *salt, size_t slen, const uint8_t *ctxt, size_t clen) { - try { - HMAC_SHA256 extractor; - static const uint8_t nosalt[SHA256_LEN] = {}; - uint8_t prk[SHA256_LEN]; - - if (slen == 0) { - salt = nosalt; - slen = SHA256_LEN; - } + log_assert(klen <= INT_MAX && slen < INT_MAX && clen < INT_MAX);

- extractor.SetKey(salt, slen); - extractor.CalculateDigest(prk, key, klen); + MemBlock prk(SHA256_LEN);

- key_generator *r = new key_generator_impl(prk, ctxt, clen); - memset(prk, 0, SHA256_LEN); - return r; + if (slen == 0) { + salt = nosalt; + slen = SHA256_LEN; } - CATCH_ALL_EXCEPTIONS(0); + + init_crypto(); + + if (HMAC(EVP_sha256(), salt, slen, key, klen, prk, 0) == 0) + log_crypto_abort("key_generator::from_random_secret"); + + return new key_generator_impl(prk, ctxt, clen); }

key_generator * @@ -310,30 +446,30 @@ key_generator::from_passphrase(const uint8_t *phra, size_t plen, const uint8_t *salt, size_t slen, const uint8_t *ctxt, size_t clen) { - try { - PBKDF2_SHA256 extractor; - static const uint8_t nosalt[SHA256_LEN] = {}; - uint8_t prk[SHA256_LEN]; - - if (slen == 0) { - salt = nosalt; - slen = SHA256_LEN; - } + // The PBKDF2-HMAC<hash> construction, ignoring the iteration + // process, is very similar to the HKDF-Extract<hash> construction; + // the biggest difference is that you key the HMAC with the + // passphrase rather than the salt. I *think* it is appropriate + // to just feed its output directly to the HKDF-Expand phase; an + // alternative would be to run PBKDF2 on the passphrase without a + // salt, then put the result through HKDF-Extract with the salt. + + log_assert(plen <= INT_MAX && slen < INT_MAX); + + MemBlock prk(SHA256_LEN);

- // The PBKDF2-HMAC<hash> construction, ignoring the iteration - // process, is very similar to the HKDF-Extract<hash> construction; - // the biggest difference is that you key the HMAC with the - // passphrase rather than the salt. I *think* it is appropriate - // to just feed its output directly to the HKDF-Expand phase; an - // alternative would be to run PBKDF2 on the passphrase without a - // salt, then put the result through HKDF-Extract with the salt. - extractor.DeriveKey(prk, SHA256_LEN, 0, phra, plen, salt, slen, 1000); - - key_generator *r = new key_generator_impl(prk, ctxt, clen); - memset(prk, 0, SHA256_LEN); - return r; + if (slen == 0) { + salt = nosalt; + slen = SHA256_LEN; } - CATCH_ALL_EXCEPTIONS(0); + + init_crypto(); + + if (!PKCS5_PBKDF2_HMAC((const char *)phra, plen, salt, slen, + 10000, EVP_sha256(), SHA256_LEN, prk)) + log_crypto_abort("key_generator::from_passphrase"); + + return new key_generator_impl(prk, ctxt, clen); }

size_t @@ -343,45 +479,53 @@ key_generator_impl::generate(uint8_t *buf, size_t len) memset(buf, 0, len); return 0; } - try { - size_t n = 0; - if (leftover >= len) { - memcpy(buf, prevT.end() - leftover, len); - leftover -= len; - return len; - } else if (leftover) { - memcpy(buf, prevT.end() - leftover, leftover); - n += leftover; - leftover = 0; + + size_t n = 0; + if (leftover >= len) { + memcpy(buf, prevT.end() - leftover, len); + leftover -= len; + return len; + } else if (leftover) { + memcpy(buf, prevT.end() - leftover, leftover); + n += leftover; + leftover = 0; + } + while (n < len) { + // compute the next block + if (!HMAC_Update(&expander, info, info.size())) + log_crypto_abort("generate::apply info"); + if (!HMAC_Update(&expander, &counter, 1)) + log_crypto_abort("generate::apply counter"); + if (!HMAC_Final(&expander, prevT, 0)) + log_crypto_abort("generate::extract"); + + if (n + SHA256_LEN < len) { + memcpy(buf + n, prevT, SHA256_LEN); + n += SHA256_LEN; + } else { + leftover = SHA256_LEN - (len - n); + memcpy(buf + n, prevT, len - n); + n = len; } - while (n < len) { - // generate the next block - expander.Update(prevT, prevT.size()); - expander.Update(info, info.size()); - expander.Update(&counter, 1); - counter++; - prevT.New(SHA256_LEN); - expander.Final(prevT); - - if (n + SHA256_LEN < len) { - memcpy(buf + n, prevT, SHA256_LEN); - n += SHA256_LEN; - } else { - leftover = SHA256_LEN - (len - n); - memcpy(buf + n, prevT, len - n); - n = len; - } - if (counter == 0) { - if (n < len) - memset(buf + n, 0, len - n); - dead = true; - break; - } + + // prepare to compute the next+1 block + counter++; + if (counter == 0) { + if (n < len) + memset(buf + n, 0, len - n); + dead = true; + break; } - return n; + + if (!HMAC_Init_ex(&expander, 0, 0, 0, 0)) + log_crypto_abort("generate::reset hmac"); + if (!HMAC_Update(&expander, prevT, prevT.size())) + log_crypto_abort("generate::feedback"); } - CATCH_ALL_EXCEPTIONS((memset(buf, 0, len), 0)); + + return n; }

key_generator::~key_generator() {} -key_generator_impl::~key_generator_impl() {} +key_generator_impl::~key_generator_impl() +{ HMAC_CTX_cleanup(&expander); } diff --git a/src/crypt.h b/src/crypt.h index 4136a35..b3bbcfa 100644 --- a/src/crypt.h +++ b/src/crypt.h @@ -7,6 +7,7 @@

const size_t AES_BLOCK_LEN = 16; const size_t GCM_TAG_LEN = 16; +const size_t SHA256_LEN = 32;

struct key_generator;

diff --git a/src/rng.cc b/src/rng.cc index 9b7ff57..86a55c1 100644 --- a/src/rng.cc +++ b/src/rng.cc @@ -5,112 +5,105 @@ #include "util.h" #include "rng.h"

-#include <limits> #include <cmath> -#include <cryptopp/osrng.h> +#include <algorithm>

-/* 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. */ +#include <openssl/rand.h>

-#define CATCH_ALL_EXCEPTIONS(rv) \ - catch (std::exception& e) { \ - log_warn("%s: %s", __func__, e.what()); \ - } catch (...) { \ - log_warn("%s: exception of abnormal type", __func__); \ - } \ - return rv /* deliberate absence of semicolon */ - -static CryptoPP::AutoSeededRandomPool *rng; - -static void -rng_teardown() -{ - delete rng; - rng = 0; -} - -static void -rng_init() -{ - if (!rng) { - rng = new CryptoPP::AutoSeededRandomPool; - if (!rng) - throw std::bad_alloc(); - atexit(rng_teardown); - } -} +/* OpenSSL's rng is global, automatically seeds itself, and does not + appear to need to be torn down explicitly. */

/** - Fills 'buf' with 'buflen' random bytes and returns 0 on success. - Returns -1 on failure. -*/ -int + * Fills 'buf' with 'buflen' random bytes. Cannot fail. + */ +void rng_bytes(uint8_t *buf, size_t buflen) { - try { - rng_init(); - rng->GenerateBlock(buf, buflen); - return 0; - } - CATCH_ALL_EXCEPTIONS(-1); + log_assert(buflen < INT_MAX); + int rv = RAND_bytes(buf, (int)buflen); + log_assert(rv); }

- -/** Return a pseudorandom integer, chosen uniformly from the values - * between 0 and <b>max</b>-1 inclusive. <b>max</b> must be between 1 and - * INT_MAX+1, inclusive. */ +/** + * Return a pseudorandom integer, chosen uniformly from the values + * in the range [0, max). 'max' must be in the range [1, INT_MAX). + */ int rng_int(unsigned int max) { - log_assert(max <= ((unsigned int)INT_MAX)+1); - log_assert(max > 0); /* don't div by 0 */ - - try { - rng_init(); - return rng->GenerateWord32(0, max-1); + log_assert(max > 0 && max <= ((unsigned int)INT_MAX)+1); + + /* rng_bytes will only give us a whole number of bytes, so to get a + uniformly random number in [0, max) we need to rejection-sample. + To minimize the number of rejections, we do the following: Find + the least k ("nbits") such that 2**k >= max, and the least b + ("nbytes") such that b*CHAR_BIT >= k. Generate b random bytes, + and rearrange them into an integer. Mask off all but the least k + bits. Accept the result if it is less than max. This way, the + probability of accepting each candidate result is always greater + than 0.5. */ + + unsigned int nbytes, nbits, mask, rv; + unsigned char buf[sizeof(int)]; + +#if __GNUC__ >= 4 + nbits = CHAR_BIT*sizeof(int) - __builtin_clz(max); +#else +#error "Need fallback for __builtin_clz" +#endif + nbytes = (nbits / CHAR_BIT) + 1; + mask = (1U << nbits) - 1; + + for (;;) { + rng_bytes(buf, nbytes); + + rv = 0; + for (unsigned int i = 0; i < nbytes; i++) + rv = (rv << CHAR_BIT) | buf[i]; + + rv &= mask; + + if (rv < max) + return rv; } - CATCH_ALL_EXCEPTIONS(-1); }

-/** Return a pseudorandom integer, chosen uniformly from the values +/** + * Return a pseudorandom integer, chosen uniformly from the values * between 'min' and 'max-1', inclusive. 'max' must be between - * 'min+1' and 'INT_MAX+1', inclusive. */ + * 'min+1' and 'INT_MAX+1', inclusive. + */ int rng_range(unsigned int min, unsigned int max) { log_assert(max <= ((unsigned int)INT_MAX)+1); log_assert(max > min);

- try { - rng_init(); - return rng->GenerateWord32(min, max-1); - } - CATCH_ALL_EXCEPTIONS(-1); + return min + rng_int(max - min); }

-/** Internal use only (can be externalized if someone has a good use - * for it): generate a random double-precision floating-point number - * in the range (0.0, 1.0] (note that this is _not_ the usual convention, - * but it saves a call to nextafter() in the sole current user). - * - * For what we use this for, it is important that we can, at least - * potentially, generate _every_ representable real number in the - * desired interval, with genuine uniformity. The usual tactic of - * generating a random integer and dividing does not do this, because - * the rational numbers produced by random()/MAX are evenly spaced on - * the real line, but floating point numbers close to zero are *not*. +/** + * Internal use only (can be externalized if someone has a good use + * for it): generate a random double-precision floating-point number + * in the range (0.0, 1.0] (note that this is _not_ the usual convention, + * but it saves a call to nextafter() in the sole current user). * - * For the same reason, the trick for avoiding division suggested - * e.g. by "Common Lisp, the Language", generating a random number in - * [1.0, 2.0) by overwriting the mantissa of a 1.0 and then - * subtracting 1.0, does not help -- you can do the first step - * precisely because the representable binary floating point numbers - * between 1.0 and 2.0 *are* evenly spaced on the real line. + * For what we use this for, it is important that we can, at least + * potentially, generate _every_ representable real number in the + * desired interval, with genuine uniformity. The usual tactic of + * generating a random integer and dividing does not do this, because + * the rational numbers produced by random()/MAX are evenly spaced on + * the real line, but floating point numbers close to zero are *not*. * - * The more complicated, but correct, algorithm here was developed by - * Allen B. Downey: http://allendowney.com/research/rand/ + * For the same reason, the trick for avoiding division suggested + * e.g. by "Common Lisp, the Language", generating a random number in + * [1.0, 2.0) by overwriting the mantissa of a 1.0 and then + * subtracting 1.0, does not help -- you can do the first step + * precisely because the representable binary floating point numbers + * between 1.0 and 2.0 *are* evenly spaced on the real line. * + * The more complicated, but correct, algorithm here was developed by + * Allen B. Downey: http://allendowney.com/research/rand/ */ static double rng_double() @@ -122,7 +115,7 @@ rng_double() bool get() { if (n == 0) { - bits = rng->GenerateByte(); + rng_bytes((uint8_t *)&bits, 1); n = CHAR_BIT; } bool rv = bits & 1; @@ -135,66 +128,64 @@ rng_double() unsigned int n; };

+ static_assert(sizeof(double) == sizeof(uint64_t), + "this code works only with 64-bit, IEEE double"); union ieee754_double { double d; uint64_t i; };

- try { - rng_init(); - - /* Because of how the Crypto++ RNG works, it is convenient to - generate the mantissa first, contra Downey, and use the - leftover bits to seed the bit-generator that we use for the - exponent; this does not change the algorithm fundamentally, - because only the final adjustment step depends on both. */ - - uint64_t mantissa = rng->GenerateWord32(); - uint32_t b = rng->GenerateWord32(); - - mantissa |= uint64_t(b & 0x000FFFFF) << 32; - - /* This is the core of Downey's algorithm: 50% of the time we - should generate the highest exponent of a number in (0,1) (note - that _neither_ endpoint is included right now). 25% of the - time, we should generate the second highest exponent, 12.5% of - the time, we should generate the third highest, and so on. In - other words, we should start with the highest exponent, flip a - coin, and keep subtracting 1 until either we hit zero or the - coin comes up heads. - - If anyone knows how to do this in _constant_ time, instead of - variable time bounded by a constant, please tell me. - */ - - rngbit bits((b & 0xFFF00000) >> 20, 12); - uint32_t exponent = 0x3FE; /* 1111111110 = 2^{-1} */ - do { - if (bits.get()) break; - } while (--exponent); - - /* Finally a slight adjustment: if the mantissa is zero, then - half the time we should increment the exponent by one. - Do this unconditionally if the exponent is also zero - (so we never generate 0.0). */ - if (mantissa == 0 && (exponent == 0 || bits.get())) - exponent++; - - /* Assemble and return the number. */ - union ieee754_double n; - n.i = (uint64_t(exponent) << 52) | mantissa; - return n.d; - } - CATCH_ALL_EXCEPTIONS(std::numeric_limits<double>::quiet_NaN()); + /* It is convenient to generate the mantissa first, contra Downey, + and use the leftover bits to seed the bit-generator that we use + for the exponent; this does not change the algorithm + fundamentally, because only the final adjustment step depends + on both. */ + + uint64_t mantissa = 0; + rng_bytes((uint8_t *)&mantissa, sizeof(mantissa)); + + rngbit bits(uint32_t(mantissa >> 52), 12); + mantissa &= UINT64_C(0x000FFFFFFFFFFFFF); + + /* This is the core of Downey's algorithm: 50% of the time we + should generate the highest exponent of a number in (0,1) (note + that _neither_ endpoint is included right now). 25% of the + time, we should generate the second highest exponent, 12.5% of + the time, we should generate the third highest, and so on. In + other words, we should start with the highest exponent, flip a + coin, and keep subtracting 1 until either we hit zero or the + coin comes up heads. + + If anyone knows how to do this in _constant_ time, instead of + variable time bounded by a constant, please tell me. + */ + + uint32_t exponent = 0x3FE; /* 1111111110 = 2^{-1} */ + do { + if (bits.get()) break; + } while (--exponent); + + /* Finally a slight adjustment: if the mantissa is zero, then + half the time we should increment the exponent by one. + Do this unconditionally if the exponent is also zero + (so we never generate 0.0). */ + if (mantissa == 0 && (exponent == 0 || bits.get())) + exponent++; + + /* Assemble and return the number. */ + union ieee754_double n; + n.i = (uint64_t(exponent) << 52) | mantissa; + return n.d; }

-/** Return a random integer in the range [0, hi), - * from a truncated geometric distribution whose expected value - * (prior to truncation) is 'xv'. - * (The rate parameter 'lambda' that's usually used to characterize - * the geometric/exponential distribution is equal to 1/xv.) - * 'hi' must be no more than INT_MAX+1, as for 'rng_range'. - * 'xv' must be greater than 0 and less than 'hi'. +/** + * Return a random integer in the range [0, hi), + * from a truncated geometric distribution whose expected value + * (prior to truncation) is 'xv'. + * (The rate parameter 'lambda' that's usually used to characterize + * the geometric/exponential distribution is equal to 1/xv.) + * 'hi' must be no more than INT_MAX+1, as for 'rng_range'. + * 'xv' must be greater than 0 and less than 'hi'. */ int rng_range_geom(unsigned int hi, unsigned int xv) @@ -202,7 +193,6 @@ rng_range_geom(unsigned int hi, unsigned int xv) using std::exp; using std::log; using std::floor; - using std::isnan; using std::min; using std::max;

@@ -210,8 +200,6 @@ rng_range_geom(unsigned int hi, unsigned int xv) log_assert(0 < xv && xv < hi);

double U = rng_double(); - if (isnan(U)) - return -1;

/* The exponential distribution with expected value xe = 1/log(1 + 1/xv) diff --git a/src/rng.h b/src/rng.h index 02b0947..5cc0a5a 100644 --- a/src/rng.h +++ b/src/rng.h @@ -6,7 +6,7 @@ #define RNG_H

/** Set b to contain n random bytes. */ -int rng_bytes(uint8_t *b, size_t n); +void rng_bytes(uint8_t *b, size_t n);

/** Return a random integer in the range [0, max). * 'max' must be between 1 and INT_MAX+1, inclusive. diff --git a/src/test/unittest.cc b/src/test/unittest.cc index 31d237a..d09c2a6 100644 --- a/src/test/unittest.cc +++ b/src/test/unittest.cc @@ -100,8 +100,12 @@ main(int argc, const char **argv) char *logminsev;

logminsev = getenv("TT_LOG"); - if (logminsev) + if (logminsev) { + log_set_method(LOG_METHOD_STDERR, 0); log_set_min_severity(logminsev); + } else { + log_set_method(LOG_METHOD_NULL, 0); + }

/* Ugly method to fix a Windows problem: http://archives.seul.org/libevent/users/Oct-2010/msg00049.html */ diff --git a/src/test/unittest_crypt.cc b/src/test/unittest_crypt.cc index 2e50623..0053646 100644 --- a/src/test/unittest_crypt.cc +++ b/src/test/unittest_crypt.cc @@ -968,10 +968,10 @@ test_crypt_rng(void *) An entropy test wouldn't really help either. I guess I'll just copy Tor's unit test methodology here :3 */

- uint8_t data1[100],data2[100]; + uint8_t data1[100], data2[100];

- tt_int_op(0, ==, rng_bytes(data1, 100)); - tt_int_op(0, ==, rng_bytes(data2, 100)); + rng_bytes(data1, 100); + rng_bytes(data2, 100);

tt_mem_op(data1, !=, data2, 100);

diff --git a/src/util.h b/src/util.h index 392d15a..32750c1 100644 --- a/src/util.h +++ b/src/util.h @@ -8,6 +8,7 @@ #include "config.h"

#define __STDC_LIMIT_MACROS +#define __STDC_CONSTANT_MACROS #include <limits.h> #include <stdarg.h> /* va_list */ #include <stddef.h> /* size_t, ptrdiff_t, offsetof, NULL */