[tor-commits] [tor/master] Improve documentation all around the codebase.
nickm at torproject.org
nickm at torproject.org
Wed Aug 9 00:36:38 UTC 2017
commit 5ca9b830eacf46acde56469e5f19f977e5f02668
Author: George Kadianakis <desnacked at riseup.net>
Date: Thu Aug 3 16:02:51 2017 +0300
Improve documentation all around the codebase.
---
src/or/hs_cell.h | 17 +++++++++++------
src/or/hs_circuit.c | 18 +++++++++++-------
src/or/hs_common.c | 9 +++++----
src/or/hs_ident.h | 4 +++-
src/or/hs_service.c | 47 +++++++++++++++++++++++++++++++---------------
src/or/hs_service.h | 10 +++++-----
src/or/or.h | 4 ++--
src/test/test_hs_common.c | 3 +++
src/test/test_hs_service.c | 12 ++++++++++++
9 files changed, 84 insertions(+), 40 deletions(-)
diff --git a/src/or/hs_cell.h b/src/or/hs_cell.h
index b7c3d1127..f32f7a421 100644
--- a/src/or/hs_cell.h
+++ b/src/or/hs_cell.h
@@ -20,22 +20,27 @@ typedef enum {
/* This data structure contains data that we need to parse an INTRODUCE2 cell
* which is used by the INTRODUCE2 cell parsing function. On a successful
* parsing, the onion_pk and rendezvous_cookie will be populated with the
- * computed key material from the cell data. */
+ * computed key material from the cell data. This structure is only used during
+ * INTRO2 parsing and discarded after that. */
typedef struct hs_cell_introduce2_data_t {
- /*** Immutable Section. ***/
+ /*** Immutable Section: Set on structure init. ***/
- /* Introduction point authentication public key. */
+ /* Introduction point authentication public key. Pointer owned by the
+ introduction point object through which we received the INTRO2 cell. */
const ed25519_public_key_t *auth_pk;
- /* Introduction point encryption keypair for the ntor handshake. */
+ /* Introduction point encryption keypair for the ntor handshake. Pointer
+ owned by the introduction point object through which we received the
+ INTRO2 cell*/
const curve25519_keypair_t *enc_kp;
- /* Subcredentials of the service. */
+ /* Subcredentials of the service. Pointer owned by the descriptor that owns
+ the introduction point through which we received the INTRO2 cell. */
const uint8_t *subcredential;
/* Payload of the received encoded cell. */
const uint8_t *payload;
/* Size of the payload of the received encoded cell. */
size_t payload_len;
- /*** Muttable Section. ***/
+ /*** Mutable Section: Set upon parsing INTRODUCE2 cell. ***/
/* Onion public key computed using the INTRODUCE2 encrypted section. */
curve25519_public_key_t onion_pk;
diff --git a/src/or/hs_circuit.c b/src/or/hs_circuit.c
index 3d67f24cb..2a753f659 100644
--- a/src/or/hs_circuit.c
+++ b/src/or/hs_circuit.c
@@ -233,7 +233,7 @@ count_opened_desc_intro_point_circuits(const hs_service_t *service,
return count;
}
-/* From a given service, rendezvous cookie and handshake infor, create a
+/* From a given service, rendezvous cookie and handshake info, create a
* rendezvous point circuit identifier. This can't fail. */
static hs_ident_circuit_t *
create_rp_circuit_identifier(const hs_service_t *service,
@@ -351,7 +351,7 @@ send_establish_intro(const hs_service_t *service,
* if direct_conn, IPv6 is prefered if we have one available.
* if firewall does not allow the chosen address, error.
*
- * Return NULL if we can fulfill the conditions. */
+ * Return NULL if we can't fulfill the conditions. */
static extend_info_t *
get_rp_extend_info(const smartlist_t *link_specifiers,
const curve25519_public_key_t *onion_key, int direct_conn)
@@ -778,6 +778,8 @@ hs_circ_service_intro_has_opened(hs_service_t *service,
tor_assert(desc);
tor_assert(circ);
+ /* Cound opened circuits that have sent ESTABLISH_INTRO cells or are already
+ * established introduction circuits */
num_intro_circ = count_opened_desc_intro_point_circuits(service, desc);
num_needed_circ = service->config.num_intro_points;
if (num_intro_circ > num_needed_circ) {
@@ -879,8 +881,9 @@ hs_circ_service_rp_has_opened(const hs_service_t *service,
memwipe(payload, 0, sizeof(payload));
}
-/* Handle an INTRO_ESTABLISHED cell payload of length payload_len arriving on
- * the given introduction circuit circ. The service is only used for logging
+/* Circ has been expecting an INTRO_ESTABLISHED cell that just arrived. Handle
+ * the INTRO_ESTABLISHED cell payload of length payload_len arriving on the
+ * given introduction circuit circ. The service is only used for logging
* purposes. Return 0 on success else a negative value. */
int
hs_circ_handle_intro_established(const hs_service_t *service,
@@ -919,9 +922,10 @@ hs_circ_handle_intro_established(const hs_service_t *service,
return ret;
}
-/* Handle an INTRODUCE2 unparsed payload of payload_len for the given circuit
- * and service. This cell is associated with the intro point object ip and the
- * subcredential. Return 0 on success else a negative value. */
+/* We just received an INTRODUCE2 cell on the established introduction circuit
+ * circ. Handle the INTRODUCE2 payload of size payload_len for the given
+ * circuit and service. This cell is associated with the intro point object ip
+ * and the subcredential. Return 0 on success else a negative value. */
int
hs_circ_handle_introduce2(const hs_service_t *service,
const origin_circuit_t *circ,
diff --git a/src/or/hs_common.c b/src/or/hs_common.c
index d68c446fd..570c1132c 100644
--- a/src/or/hs_common.c
+++ b/src/or/hs_common.c
@@ -669,7 +669,8 @@ hs_parse_address_impl(const char *address, ed25519_public_key_t *key_out,
}
/* Using the given identity public key and a blinded public key, compute the
- * subcredential and put it in subcred_out. This can't fail. */
+ * subcredential and put it in subcred_out (must be of size DIGEST256_LEN).
+ * This can't fail. */
void
hs_get_subcredential(const ed25519_public_key_t *identity_pk,
const ed25519_public_key_t *blinded_pk,
@@ -707,9 +708,9 @@ hs_get_subcredential(const ed25519_public_key_t *identity_pk,
memwipe(credential, 0, sizeof(credential));
}
-/* From the given list of hidden service ports, find the matching one from the
- * given edge connection conn and set the connection address from the service
- * port object. Return 0 on success or -1 if none. */
+/* From the given list of hidden service ports, find the ones that much the
+ * given edge connection conn, pick one at random and use it to set the
+ * connection address. Return 0 on success or -1 if none. */
int
hs_set_conn_addr_port(const smartlist_t *ports, edge_connection_t *conn)
{
diff --git a/src/or/hs_ident.h b/src/or/hs_ident.h
index a3ebd07da..e259fde54 100644
--- a/src/or/hs_ident.h
+++ b/src/or/hs_ident.h
@@ -53,7 +53,9 @@ typedef struct hs_ident_circuit_t {
hs_ident_circuit_type_t circuit_type;
/* (All circuit) Introduction point authentication key. It's also needed on
- * the rendezvous circuit for the ntor handshake. */
+ * the rendezvous circuit for the ntor handshake. It's used as the unique key
+ * of the introduction point so it should not be shared between multiple
+ * intro points. */
ed25519_public_key_t intro_auth_pk;
/* (Only client rendezvous circuit) Introduction point encryption public
diff --git a/src/or/hs_service.c b/src/or/hs_service.c
index a6f548d31..b46fd6329 100644
--- a/src/or/hs_service.c
+++ b/src/or/hs_service.c
@@ -206,8 +206,9 @@ service_clear_config(hs_service_config_t *config)
memset(config, 0, sizeof(*config));
}
-/* Return the number of minimum INTRODUCE2 cell defined by a consensus
- * parameter or the default value. */
+/* Return the lower bound of maximum INTRODUCE2 cells per circuit before we
+ * rotate intro point (defined by a consensus parameter or the default
+ * value). */
static int32_t
get_intro_point_min_introduce2(void)
{
@@ -218,8 +219,9 @@ get_intro_point_min_introduce2(void)
0, INT32_MAX);
}
-/* Return the number of maximum INTRODUCE2 cell defined by a consensus
- * parameter or the default value. */
+/* Return the upper bound of maximum INTRODUCE2 cells per circuit before we
+ * rotate intro point (defined by a consensus parameter or the default
+ * value). */
static int32_t
get_intro_point_max_introduce2(void)
{
@@ -230,8 +232,8 @@ get_intro_point_max_introduce2(void)
0, INT32_MAX);
}
-/* Return the minimum lifetime of an introduction point defined by a consensus
- * parameter or the default value. */
+/* Return the minimum lifetime in seconds of an introduction point defined by a
+ * consensus parameter or the default value. */
static int32_t
get_intro_point_min_lifetime(void)
{
@@ -247,8 +249,8 @@ get_intro_point_min_lifetime(void)
0, INT32_MAX);
}
-/* Return the maximum lifetime of an introduction point defined by a consensus
- * parameter or the default value. */
+/* Return the maximum lifetime in seconds of an introduction point defined by a
+ * consensus parameter or the default value. */
static int32_t
get_intro_point_max_lifetime(void)
{
@@ -466,7 +468,16 @@ service_intro_point_find(const hs_service_t *service,
tor_assert(service);
tor_assert(auth_key);
- /* Trying all descriptors. */
+ /* Trying all descriptors to find the right intro point.
+ *
+ * Even if we use the same node as intro point in both descriptors, the node
+ * will have a different intro auth key for each descriptor since we generate
+ * a new one everytime we pick an intro point.
+ *
+ * After #22893 gets implemented, intro points will be moved to be
+ * per-service instead of per-descriptor so this function will need to
+ * change.
+ */
FOR_EACH_DESCRIPTOR_BEGIN(service, desc) {
if ((ip = digest256map_get(desc->intro_points.map,
auth_key->pubkey)) != NULL) {
@@ -1647,6 +1658,10 @@ rotate_service_descriptors(hs_service_t *service)
STATIC void
rotate_all_descriptors(time_t now)
{
+ /* XXX We rotate all our service descriptors at once. In the future it might
+ * be wise, to rotate service descriptors independently to hide that all
+ * those descriptors are on the same tor instance */
+
FOR_EACH_SERVICE_BEGIN(service) {
/* We are _not_ in the overlap period so skip rotation. */
if (!hs_overlap_mode_is_active(NULL, now)) {
@@ -2379,7 +2394,7 @@ service_rendezvous_circ_has_opened(origin_circuit_t *circ)
tor_assert(circ);
tor_assert(circ->cpath);
- /* Getting here means this is a v3 intro circuit. */
+ /* Getting here means this is a v3 rendezvous circuit. */
tor_assert(circ->hs_ident);
tor_assert(TO_CIRCUIT(circ)->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND);
@@ -2411,8 +2426,9 @@ service_rendezvous_circ_has_opened(origin_circuit_t *circ)
return;
}
-/* Handle an INTRO_ESTABLISHED cell arriving on the given introduction
- * circuit. Return 0 on success else a negative value. */
+/* We've been expecting an INTRO_ESTABLISHED cell on this circuit and it just
+ * arrived. Handle the INTRO_ESTABLISHED cell arriving on the given
+ * introduction circuit. Return 0 on success else a negative value. */
static int
service_handle_intro_established(origin_circuit_t *circ,
const uint8_t *payload,
@@ -2446,7 +2462,8 @@ service_handle_intro_established(origin_circuit_t *circ,
}
/* Try to parse the payload into a cell making sure we do actually have a
- * valid cell. On success, the ip object is updated. */
+ * valid cell. On success, the ip object and circuit purpose is updated to
+ * reflect the fact that the introduction circuit is established. */
if (hs_circ_handle_intro_established(service, ip, circ, payload,
payload_len) < 0) {
goto err;
@@ -2467,8 +2484,8 @@ service_handle_intro_established(origin_circuit_t *circ,
return -1;
}
-/* Handle an INTRODUCE2 cell arriving on the given introduction circuit.
- * Return 0 on success else a negative value. */
+/* We just received an INTRODUCE2 cell on the established introduction circuit
+ * circ. Handle the cell and return 0 on success else a negative value. */
static int
service_handle_introduce2(origin_circuit_t *circ, const uint8_t *payload,
size_t payload_len)
diff --git a/src/or/hs_service.h b/src/or/hs_service.h
index cf2e1fa6f..1ab0c1a4f 100644
--- a/src/or/hs_service.h
+++ b/src/or/hs_service.h
@@ -123,11 +123,11 @@ typedef struct hs_service_descriptor_t {
* couldn't pick any nodes. */
unsigned int missing_intro_points : 1;
- /* List of hidden service directories node_t object to which we couldn't
- * upload this descriptor because we didn't have its router descriptor at
- * the time. If this list is non-empty, only the relays in this list are
- * re-tried to upload this descriptor when our directory information have
- * been updated. */
+ /* List of identity digests for hidden service directories to which we
+ * couldn't upload this descriptor because we didn't have its router
+ * descriptor at the time. If this list is non-empty, only the relays in this
+ * list are re-tried to upload this descriptor when our directory information
+ * have been updated. */
smartlist_t *hsdir_missing_info;
} hs_service_descriptor_t;
diff --git a/src/or/or.h b/src/or/or.h
index 14dd4e0d6..1e498aecf 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -421,9 +421,9 @@ typedef enum {
#define DIR_PURPOSE_FETCH_RENDDESC_V2 18
/** A connection to a directory server: download a microdescriptor. */
#define DIR_PURPOSE_FETCH_MICRODESC 19
-/** A connetion to a hidden service directory: upload a descriptor. */
+/** A connection to a hidden service directory: upload a v3 descriptor. */
#define DIR_PURPOSE_UPLOAD_HSDESC 20
-/** A connetion to a hidden service directory: fetch a descriptor. */
+/** A connection to a hidden service directory: fetch a v3 descriptor. */
#define DIR_PURPOSE_FETCH_HSDESC 21
#define DIR_PURPOSE_MAX_ 21
diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c
index a5c499112..60577a2a5 100644
--- a/src/test/test_hs_common.c
+++ b/src/test/test_hs_common.c
@@ -16,6 +16,7 @@
#include "hs_common.h"
#include "config.h"
+/** Test the validation of HS v3 addresses */
static void
test_validate_address(void *arg)
{
@@ -69,6 +70,7 @@ test_validate_address(void *arg)
;
}
+/** Test building HS v3 onion addresses */
static void
test_build_address(void *arg)
{
@@ -133,6 +135,7 @@ test_time_period(void *arg)
;
}
+/** Test that we can correctly find the start time of the next time period */
static void
test_start_time_of_next_time_period(void *arg)
{
diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c
index 4ee2cdac8..10eeaa432 100644
--- a/src/test/test_hs_service.c
+++ b/src/test/test_hs_service.c
@@ -397,6 +397,7 @@ test_access_service(void *arg)
hs_free_all();
}
+/** Test that we can create intro point objects, index them and find them */
static void
test_service_intro_point(void *arg)
{
@@ -609,6 +610,7 @@ test_helper_functions(void *arg)
UNMOCK(node_get_by_id);
}
+/** Test that we do the right operations when an intro circuit opens */
static void
test_intro_circuit_opened(void *arg)
{
@@ -666,6 +668,8 @@ test_intro_circuit_opened(void *arg)
UNMOCK(relay_send_command_from_edge_);
}
+/** Test the operations we do on a circuit after we learn that we successfuly
+ * established an intro point on it */
static void
test_intro_established(void *arg)
{
@@ -735,6 +739,8 @@ test_intro_established(void *arg)
UNMOCK(circuit_mark_for_close_);
}
+/** Check the operations we do on a rendezvous circuit after we learn it's
+ * open */
static void
test_rdv_circuit_opened(void *arg)
{
@@ -776,6 +782,7 @@ test_rdv_circuit_opened(void *arg)
UNMOCK(relay_send_command_from_edge_);
}
+/** Test sending and receiving introduce2 cells */
static void
test_introduce2(void *arg)
{
@@ -852,6 +859,8 @@ test_introduce2(void *arg)
UNMOCK(circuit_mark_for_close_);
}
+/** Test basic hidden service housekeeping operations (maintaining intro
+ * points, etc) */
static void
test_service_event(void *arg)
{
@@ -933,6 +942,7 @@ test_service_event(void *arg)
UNMOCK(circuit_mark_for_close_);
}
+/** Test that we rotate descriptors correctly in overlap period */
static void
test_rotate_descriptors(void *arg)
{
@@ -1013,6 +1023,8 @@ test_rotate_descriptors(void *arg)
UNMOCK(networkstatus_get_live_consensus);
}
+/** Test building descriptors: picking intro points, setting up their link
+ * specifiers, etc. */
static void
test_build_update_descriptors(void *arg)
{
More information about the tor-commits
mailing list