Continue attack on magic numbers; use new crypto wrappers where possible

svn:r1504

src/common/crypto.c

@@ -560,6 +560,12 @@ int crypto_pk_public_hybrid_encrypt(crypto_pk_env_t *env,
   if (!cipher) return -1;
   if (crypto_cipher_generate_key(cipher)<0)
     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
+   * 'cipher->key' to 0 if we aren't padding.  This means that our
+   * symmetric key is really only 127 bits.
+   */
   if (padding == PK_NO_PADDING)
     cipher->key[0] &= 0x7f;
   if (crypto_cipher_encrypt_init_cipher(cipher)<0)

src/common/util.c

@@ -162,6 +162,15 @@ void hex_encode(const char *from, int fromlen, char *to)
   *to = '\0';
 }
 
+const char *hex_str(const char *from, int fromlen)
+{
+  static char buf[65];
+  if (fromlen>(sizeof(buf)-1)/2)
+    fromlen = (sizeof(buf)-1)/2;
+  hex_encode(from,fromlen,buf);
+  return buf;
+}
+
 /*
  * A simple smartlist interface to make an unordered list of acceptable
  * nodes and then choose a random one.

src/common/util.h

@@ -95,6 +95,7 @@ void set_uint32(char *cp, uint32_t v);
 #endif
 
 void hex_encode(const char *from, int fromlen, char *to);
+const char *hex_str(const char *from, int fromlen);
 
 typedef struct smartlist_t smartlist_t;
 

src/or/circuit.c

@@ -1149,10 +1149,15 @@ static void circuit_failed(circuit_t *circ) {
  */
 static int n_circuit_failures = 0;
 
+/* Don't retry launching a new circuit if we try this many times with no
+ * success. */
+#define MAX_CIRCUIT_FAILURES 5
+
 /* Launch a new circuit and return a pointer to it. Return NULL if you failed. */
 circuit_t *circuit_launch_new(uint8_t purpose, const char *exit_nickname) {
 
-  if(n_circuit_failures > 5) { /* too many failed circs in a row. don't try. */
+  if (n_circuit_failures > MAX_CIRCUIT_FAILURES) {
+    /* too many failed circs in a row. don't try. */
 //    log_fn(LOG_INFO,"%d failures so far, not trying.",n_circuit_failures);
     return NULL;
   }
@@ -1265,7 +1270,7 @@ int circuit_send_next_onion_skin(circuit_t *circ) {
   routerinfo_t *router;
   int r;
   int circ_id_type;
-  char payload[6+ONIONSKIN_CHALLENGE_LEN];
+  char payload[2+4+ONIONSKIN_CHALLENGE_LEN];
 
   assert(circ && circ->cpath);
 
@@ -1388,7 +1393,7 @@ int circuit_extend(cell_t *cell, circuit_t *circ) {
   newcell.command = CELL_CREATE;
   newcell.circ_id = circ->n_circ_id;
 
-  memcpy(newcell.payload, cell->payload+RELAY_HEADER_SIZE+6,
+  memcpy(newcell.payload, cell->payload+RELAY_HEADER_SIZE+2+4,
          ONIONSKIN_CHALLENGE_LEN);
 
   connection_or_write_cell_to_buf(&newcell, circ->n_conn);
@@ -1426,11 +1431,13 @@ int circuit_init_cpath_crypto(crypt_path_t *cpath, char *key_data, int reverse)
 
   log_fn(LOG_DEBUG,"hop init cipher forward 0x%.8x, backward 0x%.8x.",
          (unsigned int)*(uint32_t*)(key_data+40), (unsigned int)*(uint32_t*)(key_data+40+16));
-  if (!(cpath->f_crypto = crypto_create_init_cipher(key_data+40,iv,1))) {
+  if (!(cpath->f_crypto =
+        crypto_create_init_cipher(key_data+(2*DIGEST_LEN),iv,1))) {
     log(LOG_WARN,"forward cipher initialization failed.");
     return -1;
   }
-  if (!(cpath->b_crypto = crypto_create_init_cipher(key_data+40+16,iv,0))) {
+  if (!(cpath->b_crypto =
+     crypto_create_init_cipher(key_data+(2*DIGEST_LEN)+CIPHER_KEY_LEN,iv,0))) {
     log(LOG_WARN,"backward cipher initialization failed.");
     return -1;
   }
@@ -1465,7 +1472,8 @@ int circuit_finish_handshake(circuit_t *circ, char *reply) {
   }
   assert(hop->state == CPATH_STATE_AWAITING_KEYS);
 
-  if(onion_skin_client_handshake(hop->handshake_state, reply, keys, 40+32) < 0) {
+  if(onion_skin_client_handshake(hop->handshake_state, reply, keys,
+                                 DIGEST_LEN*2+CIPHER_KEY_LEN*2) < 0) {
     log_fn(LOG_WARN,"onion_skin_client_handshake failed.");
     return -1;
   }

src/or/dirserv.c

@@ -429,9 +429,7 @@ dirserv_dump_directory_to_string(char *s, int maxlen,
     log_fn(LOG_WARN,"couldn't sign digest");
     return -1;
   }
-  log(LOG_DEBUG,"generated directory digest begins with %02x:%02x:%02x:%02x",
-      ((int)digest[0])&0xff,((int)digest[1])&0xff,
-      ((int)digest[2])&0xff,((int)digest[3])&0xff);
+  log(LOG_DEBUG,"generated directory digest begins with %s",hex_str(digest,4));
 
   if (strlcat(cp, "-----BEGIN SIGNATURE-----\n", maxlen) >= maxlen)
     goto truncated;

src/or/onion.c

@@ -122,10 +122,10 @@ void onion_pending_remove(circuit_t *circ) {
 
 /* given a response payload and keys, initialize, then send a created cell back */
 int onionskin_answer(circuit_t *circ, unsigned char *payload, unsigned char *keys) {
-  unsigned char iv[CIPHER_IV_LEN];
   cell_t cell;
+  crypt_path_t *tmp_cpath;
 
-  memset(iv, 0, CIPHER_IV_LEN);
+  tmp_cpath = tor_malloc_zero(sizeof(tmp_cpath));
 
   memset(&cell, 0, sizeof(cell_t));
   cell.command = CELL_CREATED;
@@ -139,21 +139,16 @@ int onionskin_answer(circuit_t *circ, unsigned char *payload, unsigned char *key
 
   log_fn(LOG_INFO,"init digest forward 0x%.8x, backward 0x%.8x.",
          (unsigned int)*(uint32_t*)(keys), (unsigned int)*(uint32_t*)(keys+20));
-  circ->n_digest = crypto_new_digest_env();
-  crypto_digest_add_bytes(circ->n_digest, keys, DIGEST_LEN);
-  circ->p_digest = crypto_new_digest_env();
-  crypto_digest_add_bytes(circ->p_digest, keys+DIGEST_LEN, DIGEST_LEN);
-
-  log_fn(LOG_DEBUG,"init cipher forward 0x%.8x, backward 0x%.8x.",
-         (unsigned int)*(uint32_t*)(keys+40), (unsigned int)*(uint32_t*)(keys+40+16));
-  if (!(circ->n_crypto = crypto_create_init_cipher(keys+40,iv,0))) {
-    log_fn(LOG_WARN,"Cipher initialization failed (n).");
-    return -1;
-  }
-  if (!(circ->p_crypto = crypto_create_init_cipher(keys+40+16,iv,1))) {
-    log_fn(LOG_WARN,"Cipher initialization failed (p).");
+  if (circuit_init_cpath_crypto(tmp_cpath, keys, 0)<0) {
+    log_fn(LOG_WARN,"Circuit initialization failed");
+    tor_free(tmp_cpath);
     return -1;
   }
+  circ->n_digest = tmp_cpath->f_digest;
+  circ->n_crypto = tmp_cpath->f_crypto;
+  circ->p_digest = tmp_cpath->b_digest;
+  circ->p_crypto = tmp_cpath->b_crypto;
+  tor_free(tmp_cpath);
 
   memcpy(circ->handshake_digest, cell.payload+DH_KEY_LEN, DIGEST_LEN);
 
@@ -553,15 +548,12 @@ onion_skin_create(crypto_pk_env_t *dest_router_key,
                   crypto_dh_env_t **handshake_state_out,
                   char *onion_skin_out) /* Must be ONIONSKIN_CHALLENGE_LEN bytes */
 {
-  char iv[16];
   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, ONIONSKIN_CHALLENGE_LEN);
-  memset(iv, 0, 16);
 
   if (!(dh = crypto_dh_new()))
     goto err;
@@ -570,20 +562,9 @@ onion_skin_create(crypto_pk_env_t *dest_router_key,
   pkbytes = crypto_pk_keysize(dest_router_key);
   assert(dhbytes == 128);
   assert(pkbytes == 128);
-  challenge = (char *)tor_malloc_zero(ONIONSKIN_CHALLENGE_LEN);
+  challenge = (char *)tor_malloc_zero(ONIONSKIN_CHALLENGE_LEN-CIPHER_KEY_LEN);
 
-  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 'challenge' to 0.  This means that our symmetric key is really
-   * only 127 bits.
-   */
-  challenge[0] &= 0x7f;
-
-  if (crypto_dh_get_public(dh, challenge+16, dhbytes))
+  if (crypto_dh_get_public(dh, challenge, dhbytes))
     goto err;
 
 #ifdef DEBUG_ONION_SKINS
@@ -602,29 +583,18 @@ onion_skin_create(crypto_pk_env_t *dest_router_key,
 #endif
 
   /* set meeting point, meeting cookie, etc here. Leave zero for now. */
-
-  cipher = crypto_create_init_cipher(challenge, iv, 1);
-
-  if (!cipher)
-    goto err;
-
-  if (crypto_pk_public_encrypt(dest_router_key, challenge, pkbytes,
-                               onion_skin_out, PK_NO_PADDING)==-1)
-    goto err;
-
-  if (crypto_cipher_encrypt(cipher, challenge+pkbytes, ONIONSKIN_CHALLENGE_LEN-pkbytes,
-                            onion_skin_out+pkbytes))
+  if (crypto_pk_public_hybrid_encrypt(dest_router_key, challenge,
+                                      ONIONSKIN_CHALLENGE_LEN-CIPHER_KEY_LEN,
+                                      onion_skin_out, PK_NO_PADDING)<0)
     goto err;
 
   tor_free(challenge);
-  crypto_free_cipher_env(cipher);
   *handshake_state_out = dh;
 
   return 0;
  err:
   tor_free(challenge);
   if (dh) crypto_dh_free(dh);
-  if (cipher) crypto_free_cipher_env(cipher);
   return -1;
 }
 
@@ -641,41 +611,16 @@ onion_skin_server_handshake(char *onion_skin, /* ONIONSKIN_CHALLENGE_LEN bytes *
                             int key_out_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,
-                                challenge, PK_NO_PADDING) == -1)
+  if (crypto_pk_private_hybrid_decrypt(private_key,
+                                       onion_skin, ONIONSKIN_CHALLENGE_LEN,
+                                       challenge, PK_NO_PADDING)<0)
     goto err;
 
-#ifdef DEBUG_ONION_SKINS
-  printf("Server: client symkey:");
-  PA(buf+0,16);
-  puts("");
-#endif
-
-  cipher = crypto_create_init_cipher(challenge, iv, 0);
-
-  if (crypto_cipher_decrypt(cipher, onion_skin+pkbytes, ONIONSKIN_CHALLENGE_LEN-pkbytes,
-                            challenge+pkbytes))
-    goto err;
-
-#ifdef DEBUG_ONION_SKINS
-  printf("Server: client g^x:");
-  PA(buf+16,3);
-  printf("...");
-  PA(buf+141,3);
-  puts("");
-#endif
-
   dh = crypto_dh_new();
   if (crypto_dh_get_public(dh, handshake_reply_out, DH_KEY_LEN))
     goto err;
@@ -688,17 +633,17 @@ onion_skin_server_handshake(char *onion_skin, /* ONIONSKIN_CHALLENGE_LEN bytes *
   puts("");
 #endif
 
-  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);
+  key_material = tor_malloc(DIGEST_LEN+key_out_len);
+  len = crypto_dh_compute_secret(dh, challenge, DH_KEY_LEN,
+                                 key_material, DIGEST_LEN+key_out_len);
   if (len < 0)
     goto err;
 
   /* send back H(K|0) as proof that we learned K. */
-  memcpy(handshake_reply_out+DH_KEY_LEN, key_material, 20);
+  memcpy(handshake_reply_out+DH_KEY_LEN, key_material, DIGEST_LEN);
 
   /* use the rest of the key material for our shared keys, digests, etc */
-  memcpy(key_out, key_material+20, key_out_len);
+  memcpy(key_out, key_material+DIGEST_LEN, key_out_len);
 
 #ifdef DEBUG_ONION_SKINS
   printf("Server: key material:");

src/or/routerlist.c

@@ -663,9 +663,7 @@ router_get_routerlist_from_directory_impl(const char *str,
     log_fn(LOG_WARN, "Unable to compute digest of directory");
     goto err;
   }
-  log(LOG_DEBUG,"Received directory hashes to %02x:%02x:%02x:%02x",
-      ((int)digest[0])&0xff,((int)digest[1])&0xff,
-      ((int)digest[2])&0xff,((int)digest[3])&0xff);
+  log(LOG_DEBUG,"Received directory hashes to %s",hex_str(digest,4));
 
   if ((end = strstr(str,"\nrouter "))) {
     ++end;
@@ -760,9 +758,8 @@ router_get_routerlist_from_directory_impl(const char *str,
       log_fn(LOG_WARN, "Error reading directory: invalid signature.");
       goto err;
     }
-    log(LOG_DEBUG,"Signed directory hash starts %02x:%02x:%02x:%02x",
-        ((int)signed_digest[0])&0xff,((int)signed_digest[1])&0xff,
-        ((int)signed_digest[2])&0xff,((int)signed_digest[3])&0xff);
+    log(LOG_DEBUG,"Signed directory hash starts %s", hex_str(signed_digest,4));
+
     if (memcmp(digest, signed_digest, 20)) {
       log_fn(LOG_WARN, "Error reading directory: signature does not match.");
       goto err;