Merge remote-tracking branch 'chelseakomlo/20717_hashing_api_bug'

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.

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,19 +1700,21 @@ crypto_cipher_decrypt_with_iv(const char *key,
 
 /** Compute the SHA1 digest of the <b>len</b> bytes on data stored in
  * <b>m</b>.  Write the DIGEST_LEN byte result into <b>digest</b>.
- * 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 <b>len</b> bytes in data stored in <b>m</b>,
  * using the algorithm <b>algorithm</b>.  Write the DIGEST_LEN256-byte result
- * into <b>digest</b>.  Return 0 on success, 1 on failure. */
+ * into <b>digest</b>.  Return 0 on success, -1 on failure. */
 int
 crypto_digest256(char *digest, const char *m, size_t len,
                  digest_algorithm_t algorithm)
@@ -1720,16 +1722,22 @@ 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 <b>len</b> bytes in data stored in <b>m</b>,
  * using the algorithm <b>algorithm</b>.  Write the DIGEST_LEN512-byte result
- * into <b>digest</b>.  Return 0 on success, 1 on failure. */
+ * into <b>digest</b>.  Return 0 on success, -1 on failure. */
 int
 crypto_digest512(char *digest, const char *m, size_t len,
                  digest_algorithm_t algorithm)
@@ -1737,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 <b>ds_out</b> to contain every digest on the
@@ -2628,7 +2642,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)));
   }

src/or/rendservice.c

@@ -3260,7 +3260,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);

src/or/routerparse.c

@@ -4536,12 +4536,12 @@ 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;
     }
   } 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;
     }

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,