Fix the periodic bug that would make handshaking fail.

The culprit: sometimes DH_calculate_key returns less than DH_KEY_LEN bytes;
we needed to check the return value.


svn:r322

2 files changed, 17 insertions, 14 deletions

diff --git a/src/or/onion.c b/src/or/onion.c
index cf4fae531..86430bcdc 100644
--- a/src/or/onion.c
+++ b/src/or/onion.c
@@ -474,6 +474,7 @@ onion_skin_server_handshake(char *onion_skin, /* DH_ONIONSKIN_LEN bytes long */
   crypto_dh_env_t *dh = NULL;
   crypto_cipher_env_t *cipher = NULL;
   int pkbytes;
+  int len;
   
   memset(iv, 0, 16);
   pkbytes = crypto_pk_keysize(private_key);
@@ -515,10 +516,11 @@ onion_skin_server_handshake(char *onion_skin, /* DH_ONIONSKIN_LEN bytes long */
   puts("");
 #endif
 
-  if (crypto_dh_compute_secret(dh, buf+16, DH_KEY_LEN, buf))
+  len = crypto_dh_compute_secret(dh, buf+16, DH_KEY_LEN, buf);
+  if (len < 0)
     goto err;
 
-  memcpy(key_out, buf+DH_KEY_LEN-key_out_len, key_out_len);
+  memcpy(key_out, buf+len-key_out_len, key_out_len);
 
 #ifdef DEBUG_ONION_SKINS
   printf("Server: keys out:");
@@ -550,6 +552,7 @@ onion_skin_client_handshake(crypto_dh_env_t *handshake_state,
                             int key_out_len) 
 {
   char key_material[DH_KEY_LEN];
+  int len;
   assert(crypto_dh_get_bytes(handshake_state) == DH_KEY_LEN);
   
   memset(key_material, 0, DH_KEY_LEN);
@@ -562,11 +565,12 @@ onion_skin_client_handshake(crypto_dh_env_t *handshake_state,
   puts("");
 #endif
 
-  if (crypto_dh_compute_secret(handshake_state, handshake_reply, DH_KEY_LEN,
-                               key_material))
+  len = crypto_dh_compute_secret(handshake_state, handshake_reply, DH_KEY_LEN,
+                                 key_material);
+  if (len < 0)
     return -1;
   
-  memcpy(key_out, key_material+DH_KEY_LEN-key_out_len, key_out_len);
+  memcpy(key_out, key_material+len-key_out_len, key_out_len);
 
 #ifdef DEBUG_ONION_SKINS
   printf("Client: keys out:");
diff --git a/src/or/test.c b/src/or/test.c
index 65fc8a08e..512e13f12 100644
--- a/src/or/test.c
+++ b/src/or/test.c
@@ -165,6 +165,7 @@ test_crypto_dh()
   char p2[CRYPTO_DH_SIZE];
   char s1[CRYPTO_DH_SIZE];
   char s2[CRYPTO_DH_SIZE];
+  int s1len, s2len;
 
   dh1 = crypto_dh_new();
   dh2 = crypto_dh_new();
@@ -180,10 +181,12 @@ test_crypto_dh()
   test_memneq(p1, p2, CRYPTO_DH_SIZE);
   
   memset(s1, 0, CRYPTO_DH_SIZE);
-  memset(s2, 0, CRYPTO_DH_SIZE);
-  test_assert(! crypto_dh_compute_secret(dh1, p2, CRYPTO_DH_SIZE, s1));
-  test_assert(! crypto_dh_compute_secret(dh2, p1, CRYPTO_DH_SIZE, s2));
-  test_memeq(s1, s2, CRYPTO_DH_SIZE);
+  memset(s2, 0xFF, CRYPTO_DH_SIZE);
+  s1len = crypto_dh_compute_secret(dh1, p2, CRYPTO_DH_SIZE, s1);
+  s2len = crypto_dh_compute_secret(dh2, p1, CRYPTO_DH_SIZE, s2);
+  test_assert(s1len > 0);
+  test_eq(s1len, s2len);
+  test_memeq(s1, s2, s1len);
   
   crypto_dh_free(dh1);
   crypto_dh_free(dh2);
@@ -597,22 +600,18 @@ main(int c, char**v) {
 #ifndef DEBUG_ONION_SKINS
   puts("========================== Buffers =========================");
   test_buffers();
-  puts("========================== Crypto ==========================");
+  puts("\n========================== Crypto ==========================");
   test_crypto();
   test_crypto_dh();
   puts("\n========================= Util ============================");
   test_util();
   puts("\n========================= Onion Skins =====================");
 #endif
-#ifdef DEBUG_ONION_SKINS
   crypto_seed_rng();
   while(1) {
-#endif
     test_onion_handshake();
-#ifdef DEBUG_ONION_SKINS
     fflush(NULL);
   }
-#endif
   puts("\n========================= Directory Formats ===============");
   test_dir_format();
   puts("");