In relay_digest_matches(), use stack instead of heap.

We'd been using crypto_digest_dup() and crypto_digest_assign() here,
but they aren't necessary.  Instead we can just use the stack to
store the previous state of the SHA_CTX and avoid a malloc/free pair.

Closes ticket 24914.
This commit is contained in:
Nick Mathewson 2018-01-24 12:33:13 -05:00
parent 7a74b3663f
commit 91c63aae84
5 changed files with 55 additions and 7 deletions

3
changes/bug24914 Normal file
View File

@ -0,0 +1,3 @@
o Minor features (performance):
- Avoid a needless call to malloc() when processing an incoming
relay cell. Closes ticket 24914.

View File

@ -834,6 +834,10 @@ AC_CHECK_MEMBERS([SSL.state], , ,
[#include <openssl/ssl.h> [#include <openssl/ssl.h>
]) ])
AC_CHECK_SIZEOF(SHA_CTX, , [AC_INCLUDES_DEFAULT()
#include <openssl/sha.h>
])
dnl Define the set of checks for KIST scheduler support. dnl Define the set of checks for KIST scheduler support.
AC_DEFUN([CHECK_KIST_SUPPORT],[ AC_DEFUN([CHECK_KIST_SUPPORT],[
dnl KIST needs struct tcp_info and for certain members to exist. dnl KIST needs struct tcp_info and for certain members to exist.

View File

@ -1822,6 +1822,7 @@ crypto_digest_alloc_bytes(digest_algorithm_t alg)
/* Gives the length of crypto_digest_t through the end of the field 'd' */ /* Gives the length of crypto_digest_t through the end of the field 'd' */
#define END_OF_FIELD(f) (offsetof(crypto_digest_t, f) + \ #define END_OF_FIELD(f) (offsetof(crypto_digest_t, f) + \
STRUCT_FIELD_SIZE(crypto_digest_t, f)) STRUCT_FIELD_SIZE(crypto_digest_t, f))
switch (alg) { switch (alg) {
case DIGEST_SHA1: case DIGEST_SHA1:
return END_OF_FIELD(d.sha1); return END_OF_FIELD(d.sha1);
@ -2007,6 +2008,33 @@ crypto_digest_dup(const crypto_digest_t *digest)
return tor_memdup(digest, alloc_bytes); return tor_memdup(digest, alloc_bytes);
} }
/** Temporarily save the state of <b>digest</b> in <b>checkpoint</b>.
* Asserts that <b>digest</b> is a SHA1 digest object.
*/
void
crypto_digest_checkpoint(crypto_digest_checkpoint_t *checkpoint,
const crypto_digest_t *digest)
{
tor_assert(digest->algorithm == DIGEST_SHA1);
/* The optimizer should turn this into a constant... */
const size_t bytes = crypto_digest_alloc_bytes(DIGEST_SHA1);
/* ... and remove this assertion entirely. */
tor_assert(bytes <= sizeof(checkpoint->mem));
memcpy(checkpoint->mem, digest, bytes);
}
/** Restore the state of <b>digest</b> from <b>checkpoint</b>.
* Asserts that <b>digest</b> is a SHA1 digest object. Requires that the
* state was previously stored with crypto_digest_checkpoint() */
void
crypto_digest_restore(crypto_digest_t *digest,
const crypto_digest_checkpoint_t *checkpoint)
{
tor_assert(digest->algorithm == DIGEST_SHA1);
const size_t bytes = crypto_digest_alloc_bytes(DIGEST_SHA1);
memcpy(digest, checkpoint->mem, bytes);
}
/** Replace the state of the digest object <b>into</b> with the state /** Replace the state of the digest object <b>into</b> with the state
* of the digest object <b>from</b>. Requires that 'into' and 'from' * of the digest object <b>from</b>. Requires that 'into' and 'from'
* have the same digest type. * have the same digest type.

View File

@ -98,6 +98,13 @@ typedef struct crypto_digest_t crypto_digest_t;
typedef struct crypto_xof_t crypto_xof_t; typedef struct crypto_xof_t crypto_xof_t;
typedef struct crypto_dh_t crypto_dh_t; typedef struct crypto_dh_t crypto_dh_t;
#define DIGEST_CHECKPOINT_BYTES (SIZEOF_VOID_P + SIZEOF_SHA_CTX)
/** Structure used to temporarily save the a digest object. Only implemented
* for SHA1 digest for now. */
typedef struct crypto_digest_checkpoint_t {
uint8_t mem[DIGEST_CHECKPOINT_BYTES];
} crypto_digest_checkpoint_t;
/* global state */ /* global state */
int crypto_early_init(void) ATTR_WUR; int crypto_early_init(void) ATTR_WUR;
int crypto_global_init(int hardwareAccel, int crypto_global_init(int hardwareAccel,
@ -235,6 +242,10 @@ void crypto_digest_add_bytes(crypto_digest_t *digest, const char *data,
void crypto_digest_get_digest(crypto_digest_t *digest, void crypto_digest_get_digest(crypto_digest_t *digest,
char *out, size_t out_len); char *out, size_t out_len);
crypto_digest_t *crypto_digest_dup(const crypto_digest_t *digest); crypto_digest_t *crypto_digest_dup(const crypto_digest_t *digest);
void crypto_digest_checkpoint(crypto_digest_checkpoint_t *checkpoint,
const crypto_digest_t *digest);
void crypto_digest_restore(crypto_digest_t *digest,
const crypto_digest_checkpoint_t *checkpoint);
void crypto_digest_assign(crypto_digest_t *into, void crypto_digest_assign(crypto_digest_t *into,
const crypto_digest_t *from); const crypto_digest_t *from);
void crypto_hmac_sha256(char *hmac_out, void crypto_hmac_sha256(char *hmac_out,

View File

@ -151,9 +151,9 @@ relay_digest_matches(crypto_digest_t *digest, cell_t *cell)
{ {
uint32_t received_integrity, calculated_integrity; uint32_t received_integrity, calculated_integrity;
relay_header_t rh; relay_header_t rh;
crypto_digest_t *backup_digest=NULL; crypto_digest_checkpoint_t backup_digest;
backup_digest = crypto_digest_dup(digest); crypto_digest_checkpoint(&backup_digest, digest);
relay_header_unpack(&rh, cell->payload); relay_header_unpack(&rh, cell->payload);
memcpy(&received_integrity, rh.integrity, 4); memcpy(&received_integrity, rh.integrity, 4);
@ -167,19 +167,21 @@ relay_digest_matches(crypto_digest_t *digest, cell_t *cell)
crypto_digest_add_bytes(digest, (char*) cell->payload, CELL_PAYLOAD_SIZE); crypto_digest_add_bytes(digest, (char*) cell->payload, CELL_PAYLOAD_SIZE);
crypto_digest_get_digest(digest, (char*) &calculated_integrity, 4); crypto_digest_get_digest(digest, (char*) &calculated_integrity, 4);
int rv = 1;
if (calculated_integrity != received_integrity) { if (calculated_integrity != received_integrity) {
// log_fn(LOG_INFO,"Recognized=0 but bad digest. Not recognizing."); // log_fn(LOG_INFO,"Recognized=0 but bad digest. Not recognizing.");
// (%d vs %d).", received_integrity, calculated_integrity); // (%d vs %d).", received_integrity, calculated_integrity);
/* restore digest to its old form */ /* restore digest to its old form */
crypto_digest_assign(digest, backup_digest); crypto_digest_restore(digest, &backup_digest);
/* restore the relay header */ /* restore the relay header */
memcpy(rh.integrity, &received_integrity, 4); memcpy(rh.integrity, &received_integrity, 4);
relay_header_pack(cell->payload, &rh); relay_header_pack(cell->payload, &rh);
crypto_digest_free(backup_digest); rv = 0;
return 0;
} }
crypto_digest_free(backup_digest);
return 1; memwipe(&backup_digest, 0, sizeof(backup_digest));
return rv;
} }
/** Apply <b>cipher</b> to CELL_PAYLOAD_SIZE bytes of <b>in</b> /** Apply <b>cipher</b> to CELL_PAYLOAD_SIZE bytes of <b>in</b>