Commit d47a4aec authored by Donncha O'Cearbhaill's avatar Donncha O'Cearbhaill
Browse files

Separate lookup function for service rend cache 

Performing lookups in both the client and service side descriptor
caches from the same rend_cache_lookup_entry() function increases the
risk of accidental API misuse.

I'm separating the lookup functions to keep the caches distinct.
parent 61ef356a

src/or/connection_edge.c

+1 −2
Original line number Diff line number Diff line
@@ -1527,8 +1527,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, 
    unsigned int refetch_desc = 0;

    rend_cache_entry_t *entry = NULL;

    const int rend_cache_lookup_result =

      rend_cache_lookup_entry(rend_data->onion_address, -1, &entry,

                              REND_CACHE_TYPE_CLIENT);

      rend_cache_lookup_entry(rend_data->onion_address, -1, &entry);

    if (rend_cache_lookup_result < 0) {

      switch (-rend_cache_lookup_result) {

      case EINVAL:

src/or/control.c

+2 −2
Original line number Diff line number Diff line
@@ -1920,7 +1920,7 @@ getinfo_helper_dir(control_connection_t *control_conn, 
      return -1;

    }



    if (!rend_cache_lookup_entry(question, -1, &e, REND_CACHE_TYPE_CLIENT)) {

    if (!rend_cache_lookup_entry(question, -1, &e)) {

      /* Descriptor found in cache */

      *answer = tor_strdup(e->desc);

    } else {
@@ -1936,7 +1936,7 @@ getinfo_helper_dir(control_connection_t *control_conn, 
      return -1;

    }



    if (!rend_cache_lookup_entry(question, -1, &e, REND_CACHE_TYPE_SERVICE)) {

    if (!rend_cache_lookup_v2_desc_as_service(question, &e)) {

      /* Descriptor found in cache */

      *answer = tor_strdup(e->desc);

    } else {

src/or/rendcache.c

+41 −16
Original line number Diff line number Diff line
@@ -480,27 +480,23 @@ rend_cache_clean_v2_descs_as_dir(time_t now, size_t force_remove) 
  } while (bytes_removed < force_remove);

}



/** Lookup in the client or service cache the given service ID <b>query</b> for

 * <b>version</b>. The <b>service</b> argument determines if the lookup should

 * be from the client cache or the service cache.

/** Lookup in the client cache the given service ID <b>query</b> for

 * <b>version</b>.

 *

 * Return 0 if found and if <b>e</b> is non NULL, set it with the entry

 * found. Else, a negative value is returned and <b>e</b> is untouched.

 * -EINVAL means that <b>query</b> is not a valid service id.

 * -ENOENT means that no entry in the cache was found. */

int

rend_cache_lookup_entry(const char *query, int version, rend_cache_entry_t **e,

                        rend_cache_type_t cache)

rend_cache_lookup_entry(const char *query, int version, rend_cache_entry_t **e)

{

  int ret = 0;

  char key[REND_SERVICE_ID_LEN_BASE32 + 2]; /* <version><query>\0 */

  rend_cache_entry_t *entry = NULL;

  static const int default_version = 2;



  tor_assert(rend_cache_local_service);

  tor_assert(rend_cache);

  tor_assert(query);

  tor_assert(cache);



  if (!rend_valid_service_id(query)) {

    ret = -EINVAL;
@@ -514,21 +510,50 @@ rend_cache_lookup_entry(const char *query, int version, rend_cache_entry_t **e, 
    case 2:

      /* Default is version 2. */

    default:

      if(cache == REND_CACHE_TYPE_SERVICE){

        entry = strmap_get_lc(rend_cache_local_service, query);

      } else if (cache == REND_CACHE_TYPE_CLIENT)  {

      tor_snprintf(key, sizeof(key), "%d%s", default_version, query);

      entry = strmap_get_lc(rend_cache, key);

      }

      break;

  }

  if (!entry) {

    ret = -ENOENT;

    goto end;

  }

  /* Check descriptor is parsed only if lookup is from client cache */

  if(cache == REND_CACHE_TYPE_CLIENT){

  tor_assert(entry->parsed && entry->parsed->intro_nodes);



  if (e) {

    *e = entry;

  }



 end:

  return ret;

}



/*

 * Lookup the v2 service descriptor with the service ID <b>query</b> in the

 * local service descriptor cache. Return 0 if found and if <b>e</b> is

 * non NULL, set it with the entry found. Else, a negative value is returned

 * and <b>e</b> is untouched.

 * -EINVAL means that <b>query</b> is not a valid service id.

 * -ENOENT means that no entry in the cache was found. */

int

rend_cache_lookup_v2_desc_as_service(const char *query, rend_cache_entry_t **e)

{

  int ret = 0;

  rend_cache_entry_t *entry = NULL;



  tor_assert(rend_cache_local_service);

  tor_assert(query);



  if (!rend_valid_service_id(query)) {

    ret = -EINVAL;

    goto end;

  }



  /* Lookup descriptor and return. */

  entry = strmap_get_lc(rend_cache_local_service, query);

  if (!entry) {

    ret = -ENOENT;

    goto end;

  }



  if (e) {

src/or/rendcache.h

+3 −2
Original line number Diff line number Diff line
@@ -60,8 +60,9 @@ void rend_cache_clean_v2_descs_as_dir(time_t now, size_t min_to_remove); 
void rend_cache_purge(void);

void rend_cache_free_all(void);

int rend_cache_lookup_entry(const char *query, int version,

                            rend_cache_entry_t **entry_out,

                            rend_cache_type_t cache);

                            rend_cache_entry_t **entry_out);

int rend_cache_lookup_v2_desc_as_service(const char *query,

                                         rend_cache_entry_t **entry_out);

int rend_cache_lookup_v2_desc_as_dir(const char *query, const char **desc);

/** Return value from rend_cache_store_v2_desc_as_{dir,client}. */

typedef enum {

src/or/rendclient.c

+6 −9
Original line number Diff line number Diff line
@@ -160,7 +160,7 @@ rend_client_send_introduction(origin_circuit_t *introcirc, 
#endif



  r = rend_cache_lookup_entry(introcirc->rend_data->onion_address, -1,

                              &entry, REND_CACHE_TYPE_CLIENT);

                              &entry);

  /* An invalid onion address is not possible else we have a big issue. */

  tor_assert(r != -EINVAL);

  if (r < 0 || !rend_client_any_intro_points_usable(entry)) {
@@ -906,8 +906,7 @@ rend_client_refetch_v2_renddesc(rend_data_t *rend_query) 
    return;

  }

  /* Before fetching, check if we already have a usable descriptor here. */

  if (rend_cache_lookup_entry(rend_query->onion_address, -1, &e,

                              REND_CACHE_TYPE_CLIENT) == 0 &&

  if (rend_cache_lookup_entry(rend_query->onion_address, -1, &e) == 0 &&

      rend_client_any_intro_points_usable(e)) {

    log_info(LD_REND, "We would fetch a v2 rendezvous descriptor, but we "

                      "already have a usable descriptor here. Not fetching.");
@@ -988,8 +987,7 @@ rend_client_report_intro_point_failure(extend_info_t *failed_intro, 
  rend_cache_entry_t *ent;

  connection_t *conn;



  r = rend_cache_lookup_entry(rend_query->onion_address, -1, &ent,

                              REND_CACHE_TYPE_CLIENT);

  r = rend_cache_lookup_entry(rend_query->onion_address, -1, &ent);

  if (r < 0) {

    /* Either invalid onion address or cache entry not found. */

    switch (-r) {
@@ -1215,7 +1213,7 @@ rend_client_desc_trynow(const char *query) 
      continue;

    assert_connection_ok(base_conn, now);

    if (rend_cache_lookup_entry(rend_data->onion_address, -1,

                                &entry, REND_CACHE_TYPE_CLIENT) == 0 &&

                                &entry) == 0 &&

        rend_client_any_intro_points_usable(entry)) {

      /* either this fetch worked, or it failed but there was a

       * valid entry from before which we should reuse */
@@ -1258,7 +1256,7 @@ rend_client_note_connection_attempt_ended(const rend_data_t *rend_data) 
  if (*rend_data->onion_address != '\0') {

    /* Ignore return value; we find an entry, or we don't. */

    (void) rend_cache_lookup_entry(rend_data->onion_address, -1,

                                   &cache_entry, 0);

                                   &cache_entry);

    have_onion = 1;

  }
@@ -1297,8 +1295,7 @@ rend_client_get_random_intro(const rend_data_t *rend_query) 
  extend_info_t *result;

  rend_cache_entry_t *entry;



  ret = rend_cache_lookup_entry(rend_query->onion_address, -1, &entry,

                                REND_CACHE_TYPE_CLIENT);

  ret = rend_cache_lookup_entry(rend_query->onion_address, -1, &entry);

  if (ret < 0 || !rend_client_any_intro_points_usable(entry)) {

    log_warn(LD_REND,

             "Query '%s' didn't have valid rend desc in cache. Failing.",