From 276d07a88a3b595aff9e28c7f1862563d2751b55 Mon Sep 17 00:00:00 2001 From: "Chelsea H. Komlo" Date: Thu, 17 Nov 2016 22:45:24 -0500 Subject: [PATCH 1/4] crypto_digest returns expected error value of -1 --- src/common/crypto.c | 10 ++++++---- src/or/rendservice.c | 2 +- src/or/routerparse.c | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/common/crypto.c b/src/common/crypto.c index fff516cc8e..f59b6745ba 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -1506,7 +1506,7 @@ crypto_pk_get_hashed_fingerprint(crypto_pk_t *pk, char *fp_out) if (crypto_pk_get_digest(pk, digest)) { return -1; } - if (crypto_digest(hashed_digest, digest, DIGEST_LEN)) { + if (crypto_digest(hashed_digest, digest, DIGEST_LEN) < 0) { return -1; } base16_encode(fp_out, FINGERPRINT_LEN + 1, hashed_digest, DIGEST_LEN); @@ -1700,14 +1700,16 @@ crypto_cipher_decrypt_with_iv(const char *key, /** Compute the SHA1 digest of the len bytes on data stored in * m. Write the DIGEST_LEN byte result into digest. - * Return 0 on success, 1 on failure. + * Return 0 on success, -1 on failure. */ int crypto_digest(char *digest, const char *m, size_t len) { tor_assert(m); tor_assert(digest); - return (SHA1((const unsigned char*)m,len,(unsigned char*)digest) == NULL); + if(SHA1((const unsigned char*)m,len,(unsigned char*)digest) == NULL) + return -1; + return 0; } /** Compute a 256-bit digest of len bytes in data stored in m, @@ -2628,7 +2630,7 @@ crypto_expand_key_material_TAP(const uint8_t *key_in, size_t key_in_len, for (cp = key_out, i=0; cp < key_out+key_out_len; ++i, cp += DIGEST_LEN) { tmp[key_in_len] = i; - if (crypto_digest((char*)digest, (const char *)tmp, key_in_len+1)) + if (crypto_digest((char*)digest, (const char *)tmp, key_in_len+1) < 0) goto exit; memcpy(cp, digest, MIN(DIGEST_LEN, key_out_len-(cp-key_out))); } diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 8ffd0bc319..4d25251f08 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -3084,7 +3084,7 @@ rend_service_intro_has_opened(origin_circuit_t *circuit) len += 2; memcpy(auth, circuit->cpath->prev->rend_circ_nonce, DIGEST_LEN); memcpy(auth+DIGEST_LEN, "INTRODUCE", 9); - if (crypto_digest(buf+len, auth, DIGEST_LEN+9)) + if (crypto_digest(buf+len, auth, DIGEST_LEN+9) < 0) goto err; len += 20; note_crypto_pk_op(REND_SERVER); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 2cfd3fc58a..8f8d2b8cd9 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -4536,7 +4536,7 @@ router_get_hash_impl(const char *s, size_t s_len, char *digest, return -1; if (alg == DIGEST_SHA1) { - if (crypto_digest(digest, start, end-start)) { + if (crypto_digest(digest, start, end-start) < 0) { log_warn(LD_BUG,"couldn't compute digest"); return -1; } From 9d9110f65db8af5ea4ddf93b01a099eb53e9b59f Mon Sep 17 00:00:00 2001 From: "Chelsea H. Komlo" Date: Thu, 17 Nov 2016 22:58:36 -0500 Subject: [PATCH 2/4] crypto_digest256 returns expected error value of -1 --- src/common/crypto.c | 14 ++++++++++---- src/or/routerparse.c | 2 +- src/or/shared_random.c | 6 +++--- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/common/crypto.c b/src/common/crypto.c index f59b6745ba..c075423d58 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -1714,7 +1714,7 @@ crypto_digest(char *digest, const char *m, size_t len) /** Compute a 256-bit digest of len bytes in data stored in m, * using the algorithm algorithm. Write the DIGEST_LEN256-byte result - * into digest. Return 0 on success, 1 on failure. */ + * into digest. Return 0 on success, -1 on failure. */ int crypto_digest256(char *digest, const char *m, size_t len, digest_algorithm_t algorithm) @@ -1722,11 +1722,17 @@ crypto_digest256(char *digest, const char *m, size_t len, tor_assert(m); tor_assert(digest); tor_assert(algorithm == DIGEST_SHA256 || algorithm == DIGEST_SHA3_256); + + int ret = 0; if (algorithm == DIGEST_SHA256) - return (SHA256((const uint8_t*)m,len,(uint8_t*)digest) == NULL); + ret = (SHA256((const uint8_t*)m,len,(uint8_t*)digest) != NULL); else - return (sha3_256((uint8_t *)digest, DIGEST256_LEN,(const uint8_t *)m, len) - == -1); + ret = (sha3_256((uint8_t *)digest, DIGEST256_LEN,(const uint8_t *)m, len) + > -1); + + if (!ret) + return -1; + return 0; } /** Compute a 512-bit digest of len bytes in data stored in m, diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 8f8d2b8cd9..f3246c954e 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -4541,7 +4541,7 @@ router_get_hash_impl(const char *s, size_t s_len, char *digest, return -1; } } else { - if (crypto_digest256(digest, start, end-start, alg)) { + if (crypto_digest256(digest, start, end-start, alg) < 0) { log_warn(LD_BUG,"couldn't compute digest"); return -1; } diff --git a/src/or/shared_random.c b/src/or/shared_random.c index 5f6b03f1ba..0eb93382ca 100644 --- a/src/or/shared_random.c +++ b/src/or/shared_random.c @@ -192,7 +192,7 @@ verify_commit_and_reveal(const sr_commit_t *commit) /* Use the invariant length since the encoded reveal variable has an * extra byte for the NUL terminated byte. */ if (crypto_digest256(received_hashed_reveal, commit->encoded_reveal, - SR_REVEAL_BASE64_LEN, commit->alg)) { + SR_REVEAL_BASE64_LEN, commit->alg) < 0) { /* Unable to digest the reveal blob, this is unlikely. */ goto invalid; } @@ -932,7 +932,7 @@ sr_generate_our_commit(time_t timestamp, const authority_cert_t *my_rsa_cert) /* The invariant length is used here since the encoded reveal variable * has an extra byte added for the NULL terminated byte. */ if (crypto_digest256(commit->hashed_reveal, commit->encoded_reveal, - SR_REVEAL_BASE64_LEN, commit->alg)) { + SR_REVEAL_BASE64_LEN, commit->alg) < 0) { goto error; } @@ -1012,7 +1012,7 @@ sr_compute_srv(void) SMARTLIST_FOREACH(chunks, char *, s, tor_free(s)); smartlist_free(chunks); if (crypto_digest256(hashed_reveals, reveals, strlen(reveals), - SR_DIGEST_ALG)) { + SR_DIGEST_ALG) < 0) { goto end; } current_srv = generate_srv(hashed_reveals, reveal_num, From e01b09d5cecac33fa8633a18982560e34a67ee88 Mon Sep 17 00:00:00 2001 From: "Chelsea H. Komlo" Date: Thu, 17 Nov 2016 23:02:39 -0500 Subject: [PATCH 3/4] crypto_digest512 returns expected error value of -1 --- src/common/crypto.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/common/crypto.c b/src/common/crypto.c index c075423d58..2571829b74 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -1737,7 +1737,7 @@ crypto_digest256(char *digest, const char *m, size_t len, /** Compute a 512-bit digest of len bytes in data stored in m, * using the algorithm algorithm. Write the DIGEST_LEN512-byte result - * into digest. Return 0 on success, 1 on failure. */ + * into digest. Return 0 on success, -1 on failure. */ int crypto_digest512(char *digest, const char *m, size_t len, digest_algorithm_t algorithm) @@ -1745,12 +1745,18 @@ crypto_digest512(char *digest, const char *m, size_t len, tor_assert(m); tor_assert(digest); tor_assert(algorithm == DIGEST_SHA512 || algorithm == DIGEST_SHA3_512); + + int ret = 0; if (algorithm == DIGEST_SHA512) - return (SHA512((const unsigned char*)m,len,(unsigned char*)digest) - == NULL); + ret = (SHA512((const unsigned char*)m,len,(unsigned char*)digest) + != NULL); else - return (sha3_512((uint8_t*)digest, DIGEST512_LEN, (const uint8_t*)m, len) - == -1); + ret = (sha3_512((uint8_t*)digest, DIGEST512_LEN, (const uint8_t*)m, len) + > -1); + + if (!ret) + return -1; + return 0; } /** Set the common_digests_t in ds_out to contain every digest on the From 1ca777474b858acfa97137c155488a78006d494b Mon Sep 17 00:00:00 2001 From: "Chelsea H. Komlo" Date: Sun, 20 Nov 2016 20:00:24 -0500 Subject: [PATCH 4/4] adds changes file --- changes/ticket20717 | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changes/ticket20717 diff --git a/changes/ticket20717 b/changes/ticket20717 new file mode 100644 index 0000000000..c896f8ad98 --- /dev/null +++ b/changes/ticket20717 @@ -0,0 +1,4 @@ + o Code simplification and refactoring: + - Refactors the hashing API to return negative values for errors as is done + as a standard throughout the codebase. + - Refactors calling functions to expect negative values for errors.