commit 488cdee5e7e9b23cf7bfee78e47e070489c6ca20
Author: Nick Mathewson <nickm(a)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