commit 488cdee5e7e9b23cf7bfee78e47e070489c6ca20 Author: Nick Mathewson nickm@torproject.org Date: Sun Dec 20 15:15:11 2015 -0500
When allocating a crypto_digest_t, allocate no more bytes than needed
Previously we would allocate as many bytes as we'd need for a keccak--even when we were only calculating SHA1.
Closes ticket 17796. --- changes/feature17796 | 6 +++++ src/common/crypto.c | 72 ++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 58 insertions(+), 20 deletions(-)
diff --git a/changes/feature17796 b/changes/feature17796 new file mode 100644 index 0000000..d96daed --- /dev/null +++ b/changes/feature17796 @@ -0,0 +1,6 @@ + o Minor features (crypto): + - When allocating a digest state object, allocate no more space than we + actually need. Previously, we were allocating as much space as the + state for the largest algorithm would need. This change saves up to + 672 bytes per circuit. Closes ticket 17796. + diff --git a/src/common/crypto.c b/src/common/crypto.c index a11ca79..265d065 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -1736,23 +1736,57 @@ crypto_digest_algorithm_get_length(digest_algorithm_t alg)
/** Intermediate information about the digest of a stream of data. */ struct crypto_digest_t { + digest_algorithm_t algorithm; /**< Which algorithm is in use? */ + /** State for the digest we're using. Only one member of the + * union is usable, depending on the value of <b>algorithm</b>. Note also + * that space for other members might not even be allocated! + */ union { SHA_CTX sha1; /**< state for SHA1 */ SHA256_CTX sha2; /**< state for SHA256 */ SHA512_CTX sha512; /**< state for SHA512 */ keccak_state sha3; /**< state for SHA3-[256,512] */ - } d; /**< State for the digest we're using. Only one member of the - * union is usable, depending on the value of <b>algorithm</b>. */ - digest_algorithm_bitfield_t algorithm : 8; /**< Which algorithm is in use? */ + } d; };
+/** + * Return the number of bytes we need to malloc in order to get a + * crypto_digest_t for <b>alg</b>, or the number of bytes we need to wipe + * when we free one. + */ +static size_t +crypto_digest_alloc_bytes(digest_algorithm_t alg) +{ + /* Helper: returns the number of bytes in the 'f' field of 'st' */ +#define STRUCT_FIELD_SIZE(st, f) (sizeof( ((st*)0)->f )) + /* Gives the length of crypto_digest_t through the end of the field 'd' */ +#define END_OF_FIELD(f) (STRUCT_OFFSET(crypto_digest_t, f) + \ + STRUCT_FIELD_SIZE(crypto_digest_t, f)) + switch (alg) { + case DIGEST_SHA1: + return END_OF_FIELD(d.sha1); + case DIGEST_SHA256: + return END_OF_FIELD(d.sha2); + case DIGEST_SHA512: + return END_OF_FIELD(d.sha512); + case DIGEST_SHA3_256: + case DIGEST_SHA3_512: + return END_OF_FIELD(d.sha3); + default: + tor_assert(0); + return 0; + } +#undef END_OF_FIELD +#undef STRUCT_FIELD_SIZE +} + /** Allocate and return a new digest object to compute SHA1 digests. */ crypto_digest_t * crypto_digest_new(void) { crypto_digest_t *r; - r = tor_malloc(sizeof(crypto_digest_t)); + r = tor_malloc(crypto_digest_alloc_bytes(DIGEST_SHA1)); SHA1_Init(&r->d.sha1); r->algorithm = DIGEST_SHA1; return r; @@ -1765,7 +1799,7 @@ crypto_digest256_new(digest_algorithm_t algorithm) { crypto_digest_t *r; tor_assert(algorithm == DIGEST_SHA256 || algorithm == DIGEST_SHA3_256); - r = tor_malloc(sizeof(crypto_digest_t)); + r = tor_malloc(crypto_digest_alloc_bytes(algorithm)); if (algorithm == DIGEST_SHA256) SHA256_Init(&r->d.sha2); else @@ -1781,7 +1815,7 @@ crypto_digest512_new(digest_algorithm_t algorithm) { crypto_digest_t *r; tor_assert(algorithm == DIGEST_SHA512 || algorithm == DIGEST_SHA3_512); - r = tor_malloc(sizeof(crypto_digest_t)); + r = tor_malloc(crypto_digest_alloc_bytes(algorithm)); if (algorithm == DIGEST_SHA512) SHA512_Init(&r->d.sha512); else @@ -1797,7 +1831,8 @@ crypto_digest_free(crypto_digest_t *digest) { if (!digest) return; - memwipe(digest, 0, sizeof(crypto_digest_t)); + size_t bytes = crypto_digest_alloc_bytes(digest->algorithm); + memwipe(digest, 0, bytes); tor_free(digest); }
@@ -1856,8 +1891,9 @@ crypto_digest_get_digest(crypto_digest_t *digest, return; }
+ const size_t alloc_bytes = crypto_digest_alloc_bytes(digest->algorithm); /* memcpy into a temporary ctx, since SHA*_Final clears the context */ - memcpy(&tmpenv, digest, sizeof(crypto_digest_t)); + memcpy(&tmpenv, digest, alloc_bytes); switch (digest->algorithm) { case DIGEST_SHA1: SHA1_Final(r, &tmpenv.d.sha1); @@ -1873,12 +1909,7 @@ crypto_digest_get_digest(crypto_digest_t *digest, log_warn(LD_BUG, "Handling unexpected algorithm %d", digest->algorithm); tor_assert(0); /* This is fatal, because it should never happen. */ default: - log_warn(LD_BUG, "Called with unknown algorithm %d", digest->algorithm); - /* If fragile_assert is not enabled, then we should at least not - * leak anything. */ - memwipe(r, 0xff, sizeof(r)); - memwipe(&tmpenv, 0, sizeof(crypto_digest_t)); - tor_fragile_assert(); + tor_assert(0); /* Unreachable. */ break; } memcpy(out, r, out_len); @@ -1891,15 +1922,14 @@ crypto_digest_get_digest(crypto_digest_t *digest, crypto_digest_t * crypto_digest_dup(const crypto_digest_t *digest) { - crypto_digest_t *r; tor_assert(digest); - r = tor_malloc(sizeof(crypto_digest_t)); - memcpy(r,digest,sizeof(crypto_digest_t)); - return r; + const size_t alloc_bytes = crypto_digest_alloc_bytes(digest->algorithm); + return tor_memdup(digest, alloc_bytes); }
/** Replace the state of the digest object <b>into</b> with the state - * of the digest object <b>from</b>. + * of the digest object <b>from</b>. Requires that 'into' and 'from' + * have the same digest type. */ void crypto_digest_assign(crypto_digest_t *into, @@ -1907,7 +1937,9 @@ crypto_digest_assign(crypto_digest_t *into, { tor_assert(into); tor_assert(from); - memcpy(into,from,sizeof(crypto_digest_t)); + tor_assert(into->algorithm == from->algorithm); + const size_t alloc_bytes = crypto_digest_alloc_bytes(from->algorithm); + memcpy(into,from,alloc_bytes); }
/** Given a list of strings in <b>lst</b>, set the <b>len_out</b>-byte digest