add H(K|1) to the onionskin reply

verify it at the client end
abstract the onionskin handshake lengths

breaks backward compatibility (again)


svn:r941

6 files changed, 132 insertions, 74 deletions

diff --git a/src/or/circuit.c b/src/or/circuit.c
index 0d908bf20e..fa5883ec1e 100644
--- a/src/or/circuit.c
+++ b/src/or/circuit.c
@@ -114,6 +114,10 @@ static void circuit_free_cpath_node(crypt_path_t *victim) {
     crypto_free_cipher_env(victim->f_crypto);
   if(victim->b_crypto)
     crypto_free_cipher_env(victim->b_crypto);
+  if(victim->f_digest)
+    crypto_free_digest_env(victim->f_digest);
+  if(victim->b_digest)
+    crypto_free_digest_env(victim->b_digest);
   if(victim->handshake_state)
     crypto_dh_free(victim->handshake_state);
   free(victim);
@@ -319,6 +323,12 @@ int circuit_deliver_relay_cell(cell_t *cell, circuit_t *circ,
 
   if(recognized) {
     if(cell_direction == CELL_DIRECTION_OUT) {
+#if 0
+       if(relay_update_digest(circ->n_digest, cell) < 0) {
+        log_fn(LOG_WARN,"outgoing cell failed integrity check. Closing circ.");
+        return -1;
+      }
+#endif
       ++stats_n_relay_cells_delivered;
       log_fn(LOG_DEBUG,"Sending to exit.");
       if (connection_edge_process_relay_cell(cell, circ, conn, EDGE_EXIT, NULL) < 0) {
@@ -327,6 +337,12 @@ int circuit_deliver_relay_cell(cell_t *cell, circuit_t *circ,
       }
     }
     if(cell_direction == CELL_DIRECTION_IN) {
+#if 0
+      if(relay_update_digest(layer_hint->p_digest, cell) < 0) {
+        log_fn(LOG_WARN,"outgoing cell failed integrity check. Closing circ.");
+        return -1;
+      }
+#endif
       ++stats_n_relay_cells_delivered;
       log_fn(LOG_DEBUG,"Sending to AP.");
       if (connection_edge_process_relay_cell(cell, circ, conn, EDGE_AP, layer_hint) < 0) {
@@ -365,7 +381,8 @@ int relay_crypt(circuit_t *circ, char *in, int inlen, char cell_direction,
   assert(inlen < 256);
 
   if(cell_direction == CELL_DIRECTION_IN) { 
-    if(circ->cpath) { /* we're at the beginning of the circuit. We'll want to do layered crypts. */
+    if(circ->cpath) { /* we're at the beginning of the circuit.
+                         We'll want to do layered crypts. */
       thishop = circ->cpath;
       if(thishop->state != CPATH_STATE_OPEN) {
         log_fn(LOG_WARN,"Relay cell before first created cell?");
@@ -845,7 +862,7 @@ int circuit_send_next_onion_skin(circuit_t *circ) {
     memset(&cell, 0, sizeof(cell_t));
     cell.command = CELL_CREATE;
     cell.circ_id = circ->n_circ_id;
-    cell.length = DH_ONIONSKIN_LEN;
+    cell.length = ONIONSKIN_CHALLENGE_LEN;
 
     if(onion_skin_create(circ->n_conn->onion_pkey, &(circ->cpath->handshake_state), cell.payload) < 0) {
       log_fn(LOG_WARN,"onion_skin_create (first hop) failed.");
@@ -883,7 +900,7 @@ int circuit_send_next_onion_skin(circuit_t *circ) {
     SET_CELL_RELAY_COMMAND(cell, RELAY_COMMAND_EXTEND);
     SET_CELL_STREAM_ID(cell, ZERO_STREAM);
 
-    cell.length = RELAY_HEADER_SIZE + 6 + DH_ONIONSKIN_LEN;
+    cell.length = RELAY_HEADER_SIZE + 6 + ONIONSKIN_CHALLENGE_LEN;
     *(uint32_t*)(cell.payload+RELAY_HEADER_SIZE) = htonl(hop->addr);
     *(uint16_t*)(cell.payload+RELAY_HEADER_SIZE+4) = htons(hop->port);
     if(onion_skin_create(router->onion_pkey, &(hop->handshake_state), cell.payload+RELAY_HEADER_SIZE+6) < 0) {
@@ -954,9 +971,10 @@ int circuit_extend(cell_t *cell, circuit_t *circ) {
   memset(&newcell, 0, sizeof(cell_t));
   newcell.command = CELL_CREATE;
   newcell.circ_id = circ->n_circ_id;
-  newcell.length = DH_ONIONSKIN_LEN;
+  newcell.length = ONIONSKIN_CHALLENGE_LEN;
 
-  memcpy(newcell.payload, cell->payload+RELAY_HEADER_SIZE+6, DH_ONIONSKIN_LEN);
+  memcpy(newcell.payload, cell->payload+RELAY_HEADER_SIZE+6,
+         ONIONSKIN_CHALLENGE_LEN);
 
   connection_or_write_cell_to_buf(&newcell, circ->n_conn);
   return 0;
diff --git a/src/or/command.c b/src/or/command.c
index fa83d23c83..ed0a8d04d0 100644
--- a/src/or/command.c
+++ b/src/or/command.c
@@ -103,7 +103,7 @@ static void command_process_create_cell(cell_t *cell, connection_t *conn) {
 
   circ = circuit_new(cell->circ_id, conn);
   circ->state = CIRCUIT_STATE_ONIONSKIN_PENDING;
-  if(cell->length != DH_ONIONSKIN_LEN) {
+  if(cell->length != ONIONSKIN_CHALLENGE_LEN) {
     log_fn(LOG_WARN,"Bad cell length %d. Dropping.", cell->length);
     circuit_close(circ);
     return;
@@ -135,7 +135,7 @@ static void command_process_created_cell(cell_t *cell, connection_t *conn) {
     circuit_close(circ);
     return;
   }
-  assert(cell->length == DH_KEY_LEN);
+  assert(cell->length == ONIONSKIN_REPLY_LEN);
 
   if(circ->cpath) { /* we're the OP. Handshake this. */
     log_fn(LOG_DEBUG,"at OP. Finishing handshake.");
@@ -153,7 +153,7 @@ static void command_process_created_cell(cell_t *cell, connection_t *conn) {
   } else { /* pack it into an extended relay cell, and send it. */
     log_fn(LOG_INFO,"Converting created cell to extended relay cell, sending.");
     connection_edge_send_command(NULL, circ, RELAY_COMMAND_EXTENDED,
-                                 cell->payload, DH_KEY_LEN, NULL);
+                                 cell->payload, cell->length, NULL);
   }
 }
 
diff --git a/src/or/cpuworker.c b/src/or/cpuworker.c
index c964115f90..fee8eebb47 100644
--- a/src/or/cpuworker.c
+++ b/src/or/cpuworker.c
@@ -9,8 +9,8 @@ extern or_options_t options; /* command-line and config-file options */
 #define MIN_CPUWORKERS 1
 
 #define TAG_LEN 8
-#define LEN_ONION_QUESTION (1+TAG_LEN+DH_ONIONSKIN_LEN)
-#define LEN_ONION_RESPONSE (1+TAG_LEN+DH_KEY_LEN+32)
+#define LEN_ONION_QUESTION (1+TAG_LEN+ONIONSKIN_CHALLENGE_LEN)
+#define LEN_ONION_RESPONSE (1+TAG_LEN+ONIONSKIN_REPLY_LEN+32)
 
 int num_cpuworkers=0;
 int num_cpuworkers_busy=0;
@@ -95,7 +95,7 @@ int connection_cpu_process_inbuf(connection_t *conn) {
       circuit_close(circ);
       goto done_processing;
     }
-    if(onionskin_answer(circ, buf+1+TAG_LEN, buf+1+TAG_LEN+DH_KEY_LEN) < 0) {
+    if(onionskin_answer(circ, buf+1+TAG_LEN, buf+1+TAG_LEN+ONIONSKIN_REPLY_LEN) < 0) {
       log_fn(LOG_WARN,"onionskin_answer failed. Closing.");
       circuit_close(circ);
       goto done_processing;
@@ -113,14 +113,14 @@ done_processing:
 }
 
 int cpuworker_main(void *data) {
-  unsigned char question[DH_ONIONSKIN_LEN];
+  unsigned char question[ONIONSKIN_CHALLENGE_LEN];
   unsigned char question_type;
   int *fdarray = data;
   int fd;
 
   /* variables for onion processing */
   unsigned char keys[32];
-  unsigned char reply_to_proxy[DH_KEY_LEN];
+  unsigned char reply_to_proxy[ONIONSKIN_REPLY_LEN];
   unsigned char buf[LEN_ONION_RESPONSE];
   char tag[TAG_LEN];
 
@@ -140,7 +140,7 @@ int cpuworker_main(void *data) {
       spawn_exit();
     }
 
-    if(read_all(fd, question, DH_ONIONSKIN_LEN) != DH_ONIONSKIN_LEN) {
+    if(read_all(fd, question, ONIONSKIN_CHALLENGE_LEN) != ONIONSKIN_CHALLENGE_LEN) {
       log_fn(LOG_ERR,"read question failed. Exiting.");
       spawn_exit();
     }
@@ -156,8 +156,8 @@ int cpuworker_main(void *data) {
         log_fn(LOG_INFO,"onion_skin_server_handshake succeeded.");
         buf[0] = 1; /* 1 means success */
         memcpy(buf+1,tag,TAG_LEN);
-        memcpy(buf+1+TAG_LEN,reply_to_proxy,DH_KEY_LEN);
-        memcpy(buf+1+TAG_LEN+DH_KEY_LEN,keys,32);
+        memcpy(buf+1+TAG_LEN,reply_to_proxy,ONIONSKIN_REPLY_LEN);
+        memcpy(buf+1+TAG_LEN+ONIONSKIN_REPLY_LEN,keys,32);
       }
       if(write_all(fd, buf, LEN_ONION_RESPONSE) != LEN_ONION_RESPONSE) {
         log_fn(LOG_ERR,"writing response buf failed. Exiting.");
@@ -272,7 +272,7 @@ int assign_to_cpuworker(connection_t *cpuworker, unsigned char question_type,
 
     connection_write_to_buf(&question_type, 1, cpuworker);
     connection_write_to_buf(tag, sizeof(tag), cpuworker);
-    connection_write_to_buf(circ->onionskin, DH_ONIONSKIN_LEN, cpuworker);
+    connection_write_to_buf(circ->onionskin, ONIONSKIN_CHALLENGE_LEN, cpuworker);
   }
   return 0;
 }
diff --git a/src/or/onion.c b/src/or/onion.c
index 5578fb7935..0923741d92 100644
--- a/src/or/onion.c
+++ b/src/or/onion.c
@@ -124,13 +124,13 @@ int onionskin_answer(circuit_t *circ, unsigned char *payload, unsigned char *key
   memset(&cell, 0, sizeof(cell_t));
   cell.command = CELL_CREATED;
   cell.circ_id = circ->p_circ_id;
-  cell.length = DH_KEY_LEN;
+  cell.length = ONIONSKIN_REPLY_LEN;
 
   circ->state = CIRCUIT_STATE_OPEN;
 
   log_fn(LOG_DEBUG,"Entering.");
 
-  memcpy(cell.payload, payload, DH_KEY_LEN);
+  memcpy(cell.payload, payload, ONIONSKIN_REPLY_LEN);
 
   log_fn(LOG_DEBUG,"init cipher forward %d, backward %d.", *(int*)keys, *(int*)(keys+16));
 
@@ -548,52 +548,56 @@ int onion_extend_cpath(crypt_path_t **head_ptr, cpath_build_state_t *state, rout
 
 /*----------------------------------------------------------------------*/
 
-/* Given a router's public key, generates a 144-byte encrypted DH pubkey,
- * and stores it into onion_skin out.  Stores the DH private key into 
- * handshake_state_out for later completion of the handshake.
+/* Given a router's 128 byte public key,
+   stores the following in onion_skin_out:
+[16 bytes] Symmetric key for encrypting blob past RSA
+[112 bytes] g^x part 1 (inside the RSA)
+[16 bytes] g^x part 2 (symmetrically encrypted)
+[ 6 bytes] Meeting point (IP/port)
+[ 8 bytes] Meeting cookie
+[16 bytes] End-to-end authentication [optional]
+
+ * Stores the DH private key into handshake_state_out for later completion
+ * of the handshake.
  *
- * The encrypted pubkey is formed as follows:
- *    16 bytes of symmetric key
- *   128 bytes of g^x for DH.
- * The first 128 bytes are RSA-encrypted with the server's public key,
- * and the last 16 are encrypted with the symmetric key.
+ * The meeting point/cookies and auth are zeroed out for now.
  */
 int
 onion_skin_create(crypto_pk_env_t *dest_router_key,
                   crypto_dh_env_t **handshake_state_out,
-                  char *onion_skin_out) /* Must be DH_ONIONSKIN_LEN bytes long */
+                  char *onion_skin_out) /* Must be ONIONSKIN_CHALLENGE_LEN bytes */
 {
   char iv[16];
-  char *pubkey = NULL;
+  char *challenge = NULL;
   crypto_dh_env_t *dh = NULL;
   crypto_cipher_env_t *cipher = NULL;
   int dhbytes, pkbytes;
 
   *handshake_state_out = NULL;
-  memset(onion_skin_out, 0, DH_ONIONSKIN_LEN);
+  memset(onion_skin_out, 0, ONIONSKIN_CHALLENGE_LEN);
   memset(iv, 0, 16);
 
   if (!(dh = crypto_dh_new()))
     goto err;
-  
+
   dhbytes = crypto_dh_get_bytes(dh);
   pkbytes = crypto_pk_keysize(dest_router_key);
-  assert(dhbytes+16 == DH_ONIONSKIN_LEN);
-  pubkey = (char *)tor_malloc(dhbytes+16);
+  assert(dhbytes == 128);
+  assert(pkbytes == 128);
+  challenge = (char *)tor_malloc_zero(ONIONSKIN_CHALLENGE_LEN);
 
-  if (crypto_rand(16, pubkey))
+  if (crypto_rand(16, challenge))
     goto err;
 
   /* You can't just run around RSA-encrypting any bitstream: if it's
    * greater than the RSA key, then OpenSSL will happily encrypt,
    * and later decrypt to the wrong value.  So we set the first bit
-   * of 'pubkey' to 0.  This means that our symmetric key is really only
-   * 127 bits long, but since it shouldn't be necessary to encrypt
-   * DH public keys values in the first place, we should be fine.
+   * of 'challenge' to 0.  This means that our symmetric key is really
+   * only 127 bits.
    */
-  pubkey[0] &= 0x7f; 
+  challenge[0] &= 0x7f; 
 
-  if (crypto_dh_get_public(dh, pubkey+16, dhbytes))
+  if (crypto_dh_get_public(dh, challenge+16, dhbytes))
     goto err;
 
 #ifdef DEBUG_ONION_SKINS
@@ -601,65 +605,69 @@ onion_skin_create(crypto_pk_env_t *dest_router_key,
   { int _i; for (_i = 0; _i<n; ++_i) printf("%02x ",((int)(a)[_i])&0xFF); }
 
   printf("Client: client g^x:");
-  PA(pubkey+16,3);
+  PA(challenge+16,3);
   printf("...");
-  PA(pubkey+141,3);
+  PA(challenge+141,3);
   puts("");
 
   printf("Client: client symkey:");
-  PA(pubkey+0,16);
+  PA(challenge+0,16);
   puts("");
 #endif
 
-  cipher = crypto_create_init_cipher(ONION_CIPHER, pubkey, iv, 1);
+  /* set meeting point, meeting cookie, etc here. Leave zero for now. */
+
+  cipher = crypto_create_init_cipher(ONION_CIPHER, challenge, iv, 1);
 
   if (!cipher)
     goto err;
 
-  if (crypto_pk_public_encrypt(dest_router_key, pubkey, pkbytes,
+  if (crypto_pk_public_encrypt(dest_router_key, challenge, pkbytes,
                                onion_skin_out, RSA_NO_PADDING)==-1)
     goto err;
 
-  if (crypto_cipher_encrypt(cipher, pubkey+pkbytes, dhbytes+16-pkbytes,
+  if (crypto_cipher_encrypt(cipher, challenge+pkbytes, ONIONSKIN_CHALLENGE_LEN-pkbytes,
                             onion_skin_out+pkbytes))
     goto err;
 
-  free(pubkey);
+  tor_free(challenge);
   crypto_free_cipher_env(cipher);
   *handshake_state_out = dh;
 
   return 0;
  err:
-  tor_free(pubkey);
+  tor_free(challenge);
   if (dh) crypto_dh_free(dh);
   if (cipher) crypto_free_cipher_env(cipher);
   return -1;
 }
 
 /* Given an encrypted DH public key as generated by onion_skin_create,
- * and the private key for this onion router, generate the 128-byte DH
- * reply, and key_out_len bytes of key material, stored in key_out.
+ * and the private key for this onion router, generate the reply (128-byte
+ * DH plus the first 20 bytes of shared key material), and store the
+ * next key_out_len bytes of key material in key_out.
  */
 int
-onion_skin_server_handshake(char *onion_skin, /* DH_ONIONSKIN_LEN bytes long */
+onion_skin_server_handshake(char *onion_skin, /* ONIONSKIN_CHALLENGE_LEN bytes */
                             crypto_pk_env_t *private_key,
-                            char *handshake_reply_out, /* DH_KEY_LEN bytes long */
+                            char *handshake_reply_out, /* ONIONSKIN_REPLY_LEN bytes */
                             char *key_out,
                             int key_out_len)
 {
-  char buf[DH_ONIONSKIN_LEN];
+  char challenge[ONIONSKIN_CHALLENGE_LEN];
   char iv[16];
   crypto_dh_env_t *dh = NULL;
   crypto_cipher_env_t *cipher = NULL;
   int pkbytes;
   int len;
+  char *key_material=NULL;
 
   memset(iv, 0, 16);
   pkbytes = crypto_pk_keysize(private_key);
 
   if (crypto_pk_private_decrypt(private_key,
                                 onion_skin, pkbytes,
-                                buf, RSA_NO_PADDING) == -1)
+                                challenge, RSA_NO_PADDING) == -1)
     goto err;
 
 #ifdef DEBUG_ONION_SKINS
@@ -668,10 +676,10 @@ onion_skin_server_handshake(char *onion_skin, /* DH_ONIONSKIN_LEN bytes long */
   puts("");
 #endif
 
-  cipher = crypto_create_init_cipher(ONION_CIPHER, buf, iv, 0);
+  cipher = crypto_create_init_cipher(ONION_CIPHER, challenge, iv, 0);
 
-  if (crypto_cipher_decrypt(cipher, onion_skin+pkbytes, DH_ONIONSKIN_LEN-pkbytes,
-                            buf+pkbytes))
+  if (crypto_cipher_decrypt(cipher, onion_skin+pkbytes, ONIONSKIN_CHALLENGE_LEN-pkbytes,
+                            challenge+pkbytes))
     goto err;
 
 #ifdef DEBUG_ONION_SKINS
@@ -681,7 +689,7 @@ onion_skin_server_handshake(char *onion_skin, /* DH_ONIONSKIN_LEN bytes long */
   PA(buf+141,3);
   puts("");
 #endif
-  
+
   dh = crypto_dh_new();
   if (crypto_dh_get_public(dh, handshake_reply_out, DH_KEY_LEN))
     goto err;
@@ -694,10 +702,18 @@ onion_skin_server_handshake(char *onion_skin, /* DH_ONIONSKIN_LEN bytes long */
   puts("");
 #endif
 
-  len = crypto_dh_compute_secret(dh, buf+16, DH_KEY_LEN, key_out, key_out_len);
+  key_material = tor_malloc(20+key_out_len);
+  len = crypto_dh_compute_secret(dh, challenge+16, DH_KEY_LEN,
+                                 key_material, 20+key_out_len);
   if (len < 0)
     goto err;
 
+  /* send back H(K) as proof that we learned K. */
+  memcpy(handshake_reply_out+DH_KEY_LEN, key_material, 20);
+
+  /* use the rest of the key material for our shared keys, digests, etc */
+  memcpy(key_out, key_material+20, key_out_len);
+
 #ifdef DEBUG_ONION_SKINS
   printf("Server: key material:");
   PA(buf, DH_KEY_LEN);
@@ -707,10 +723,12 @@ onion_skin_server_handshake(char *onion_skin, /* DH_ONIONSKIN_LEN bytes long */
   puts("");
 #endif
 
+  tor_free(key_material);
   crypto_free_cipher_env(cipher);
   crypto_dh_free(dh);
   return 0;
  err:
+  tor_free(key_material);
   if (cipher) crypto_free_cipher_env(cipher);
   if (dh) crypto_dh_free(dh);
 
@@ -718,19 +736,22 @@ onion_skin_server_handshake(char *onion_skin, /* DH_ONIONSKIN_LEN bytes long */
 }
 
 /* Finish the client side of the DH handshake.
- * Given the 128 byte DH reply as generated by onion_skin_server_handshake
- * and the handshake state generated by onion_skin_create, generate
- * key_out_len bytes of shared key material and store them in key_out.
+ * Given the 128 byte DH reply + 20 byte hash as generated by
+ * onion_skin_server_handshake and the handshake state generated by
+ * onion_skin_create, verify H(K) with the first 20 bytes of shared
+ * key material, then generate key_out_len more bytes of shared key
+ * material and store them in key_out.
  *
  * After the invocation, call crypto_dh_free on handshake_state.
  */
 int
 onion_skin_client_handshake(crypto_dh_env_t *handshake_state,
-                            char *handshake_reply,/* Must be DH_KEY_LEN bytes long*/
+                            char *handshake_reply, /* Must be ONIONSKIN_REPLY_LEN bytes */
                             char *key_out,
-                            int key_out_len) 
+                            int key_out_len)
 {
   int len;
+  char *key_material=NULL;
   assert(crypto_dh_get_bytes(handshake_state) == DH_KEY_LEN);
 
 #ifdef DEBUG_ONION_SKINS
@@ -741,17 +762,29 @@ onion_skin_client_handshake(crypto_dh_env_t *handshake_state,
   puts("");
 #endif
 
+  key_material = tor_malloc(20+key_out_len);
   len = crypto_dh_compute_secret(handshake_state, handshake_reply, DH_KEY_LEN,
-                                 key_out, key_out_len);
+                                 key_material, 20+key_out_len);
   if (len < 0)
     return -1;
 
+  if(memcmp(key_material, handshake_reply+DH_KEY_LEN, 20)) {
+    /* H(K) does *not* match. Something fishy. */
+    tor_free(key_material);
+    log_fn(LOG_WARN,"Digest DOES NOT MATCH on onion handshake. Bug or attack.");
+    return -1;
+  }
+
+  /* use the rest of the key material for our shared keys, digests, etc */
+  memcpy(key_out, key_material+20, key_out_len);
+
 #ifdef DEBUG_ONION_SKINS
   printf("Client: keys out:");
   PA(key_out, key_out_len);
   puts("");
 #endif
 
+  tor_free(key_material);
   return 0;
 }
 
diff --git a/src/or/or.h b/src/or/or.h
index 753cbf3e4c..7cab9f17be 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -374,6 +374,9 @@ struct crypt_path_t {
   crypto_cipher_env_t *f_crypto;
   crypto_cipher_env_t *b_crypto;
 
+  crypto_digest_env_t *f_digest; /* for integrity checking */
+  crypto_digest_env_t *b_digest;
+
   crypto_dh_env_t *handshake_state;
 
   uint32_t addr;
@@ -391,7 +394,8 @@ struct crypt_path_t {
 };
 
 #define DH_KEY_LEN CRYPTO_DH_SIZE
-#define DH_ONIONSKIN_LEN DH_KEY_LEN+16
+#define ONIONSKIN_CHALLENGE_LEN (16+DH_KEY_LEN+6+8+16)
+#define ONIONSKIN_REPLY_LEN (DH_KEY_LEN+20)
 
 typedef struct crypt_path_t crypt_path_t;
 
@@ -417,10 +421,13 @@ struct circuit_t {
   crypto_cipher_env_t *p_crypto; /* used only for intermediate hops */
   crypto_cipher_env_t *n_crypto;
 
+  crypto_digest_env_t *p_digest; /* for integrity checking, */
+  crypto_digest_env_t *n_digest; /* intermediate hops only */
+
   cpath_build_state_t *build_state;
   crypt_path_t *cpath;
 
-  char onionskin[DH_ONIONSKIN_LEN]; /* for storage while onionskin pending */
+  char onionskin[ONIONSKIN_CHALLENGE_LEN]; /* for storage while onionskin pending */
   time_t timestamp_created;
   time_t timestamp_dirty; /* when the circuit was first used, or 0 if clean */
 
@@ -709,16 +716,16 @@ int onion_extend_cpath(crypt_path_t **head_ptr, cpath_build_state_t *state,
 
 int onion_skin_create(crypto_pk_env_t *router_key,
                       crypto_dh_env_t **handshake_state_out,
-                      char *onion_skin_out); /* Must be DH_ONIONSKIN_LEN bytes long */
+                      char *onion_skin_out);
 
-int onion_skin_server_handshake(char *onion_skin, /* DH_ONIONSKIN_LEN bytes long */
+int onion_skin_server_handshake(char *onion_skin,
                                 crypto_pk_env_t *private_key,
-                                char *handshake_reply_out, /* DH_KEY_LEN bytes long */
+                                char *handshake_reply_out,
                                 char *key_out,
                                 int key_out_len);
 
 int onion_skin_client_handshake(crypto_dh_env_t *handshake_state,
-                             char *handshake_reply,/* Must be DH_KEY_LEN bytes long*/
+                             char *handshake_reply,
                              char *key_out,
                              int key_out_len);
 
diff --git a/src/or/test.c b/src/or/test.c
index a248141287..511cda861d 100644
--- a/src/or/test.c
+++ b/src/or/test.c
@@ -485,11 +485,11 @@ void
 test_onion_handshake() {
   /* client-side */
   crypto_dh_env_t *c_dh = NULL;
-  char c_buf[DH_ONIONSKIN_LEN];
+  char c_buf[ONIONSKIN_CHALLENGE_LEN];
   char c_keys[40];
 
   /* server-side */
-  char s_buf[DH_KEY_LEN];
+  char s_buf[ONIONSKIN_REPLY_LEN];
   char s_keys[40];
   
   /* shared */
@@ -499,11 +499,11 @@ test_onion_handshake() {
   test_assert(! crypto_pk_generate_key(pk));
 
   /* client handshake 1. */
-  memset(c_buf, 0, DH_ONIONSKIN_LEN);
+  memset(c_buf, 0, ONIONSKIN_CHALLENGE_LEN);
   test_assert(! onion_skin_create(pk, &c_dh, c_buf));
 
   /* server handshake */
-  memset(s_buf, 0, DH_KEY_LEN);
+  memset(s_buf, 0, ONIONSKIN_REPLY_LEN);
   memset(s_keys, 0, 40);
   test_assert(! onion_skin_server_handshake(c_buf, pk, s_buf, s_keys, 40));