[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[or-cvs] r11099: Resolve a pile of XXXXs in and around voting code (in tor/trunk: . src/or)
Author: nickm
Date: 2007-08-13 22:23:57 -0400 (Mon, 13 Aug 2007)
New Revision: 11099
Modified:
tor/trunk/
tor/trunk/src/or/config.c
tor/trunk/src/or/dirvote.c
tor/trunk/src/or/or.h
tor/trunk/src/or/routerparse.c
Log:
r14003@kushana: nickm | 2007-08-13 22:23:49 -0400
Resolve a pile of XXXXs in and around voting code
Property changes on: tor/trunk
___________________________________________________________________
svk:merge ticket from /tor/trunk [r14003] on c95137ef-5f19-0410-b913-86e773d04f59
Modified: tor/trunk/src/or/config.c
===================================================================
--- tor/trunk/src/or/config.c 2007-08-14 00:07:29 UTC (rev 11098)
+++ tor/trunk/src/or/config.c 2007-08-14 02:23:57 UTC (rev 11099)
@@ -271,8 +271,12 @@
VAR("V1AuthoritativeDirectory",BOOL, V1AuthoritativeDir, "0"),
VAR("V2AuthoritativeDirectory",BOOL, V2AuthoritativeDir, "0"),
VAR("V3AuthoritativeDirectory",BOOL, V3AuthoritativeDir, "0"),
- /* XXXX020 check this for sanity. */
+ /* XXXX020 check these for sanity. */
VAR("V3AuthVotingInterval",INTERVAL, V3AuthVotingInterval, "1 hour"),
+ VAR("V3AuthVoteDelay", INTERVAL, V3AuthVoteDelay, "5 minutes"),
+ VAR("V3AuthDistDelay", INTERVAL, V3AuthDistDelay, "5 minutes"),
+ VAR("V3AuthNIntervalsValid", UINT, V3AuthNIntervalsValid, "3"),
+
VAR("VersioningAuthoritativeDirectory",BOOL,VersioningAuthoritativeDir, "0"),
VAR("VirtualAddrNetwork", STRING, VirtualAddrNetwork, "127.192.0.0/10"),
VAR("__AllDirActionsPrivate",BOOL, AllDirActionsPrivate, "0"),
Modified: tor/trunk/src/or/dirvote.c
===================================================================
--- tor/trunk/src/or/dirvote.c 2007-08-14 00:07:29 UTC (rev 11098)
+++ tor/trunk/src/or/dirvote.c 2007-08-14 02:23:57 UTC (rev 11099)
@@ -293,7 +293,10 @@
* authority <b>identity_key</b>, our private authority <b>signing_key</b>,
* and the number of <b>total_authorities</b> that we believe exist in our
* voting quorum, generate the text of a new v3 consensus vote, and return the
- * value in a newly allocated string. */
+ * value in a newly allocated string.
+ *
+ * Note: this function DOES NOT check whether the votes are from
+ * recognized authorities. (dirvote_add_vote does that.) */
char *
networkstatus_compute_consensus(smartlist_t *votes,
int total_authorities,
@@ -313,8 +316,6 @@
log_warn(LD_DIR, "Can't compute a consensus from no votes.");
return NULL;
}
- /* XXXX020 somebody needs to check vote authority. It could be this
- * function, it could be somebody else. */
flags = smartlist_create();
/* Compute medians of time-related things, and figure out how many
@@ -455,10 +456,10 @@
/* Add the actual router entries. */
{
- /* document these XXXX020 */
- int *index;
- int *size;
- int *flag_counts;
+ int *index; /* index[j] is the current index into votes[j]. */
+ int *size; /* size[j] is the number of routerstatuses in votes[j]. */
+ int *flag_counts; /* The number of voters that list flag[j] for the
+ * currently considered router. */
int i;
smartlist_t *matching_descs = smartlist_create();
smartlist_t *chosen_flags = smartlist_create();
@@ -470,7 +471,7 @@
* about flags[f]. */
int **flag_map; /* flag_map[j][b] is an index f such that flag_map[f]
* is the same flag as votes[j]->known_flags[b]. */
- int *named_flag;
+ int *named_flag; /* Index of the flag "Named" for votes[j] */
index = tor_malloc_zero(sizeof(int)*smartlist_len(votes));
size = tor_malloc_zero(sizeof(int)*smartlist_len(votes));
@@ -510,6 +511,7 @@
int i;
char buf[256];
+ /* Of the next-to-be-considered digest in each voter, which is first? */
SMARTLIST_FOREACH(votes, networkstatus_vote_t *, v, {
if (index[v_sl_idx] < size[v_sl_idx]) {
rs = smartlist_get(v->routerstatus_list, index[v_sl_idx]);
@@ -549,8 +551,11 @@
++flag_counts[flag_map[v_sl_idx][i]];
}
if (rs->flags & (U64_LITERAL(1) << named_flag[v_sl_idx])) {
- if (chosen_name && strcmp(chosen_name, rs->status.nickname))
- naming_conflict = 1; /* XXXX020 warn? */
+ if (chosen_name && strcmp(chosen_name, rs->status.nickname)) {
+ log_notice(LD_DIR, "Conflict on naming for router: %s vs %s",
+ chosen_name, rs->status.nickname);
+ naming_conflict = 1;
+ }
chosen_name = rs->status.nickname;
}
@@ -610,12 +615,10 @@
smartlist_join_strings(chosen_flags, " ", 0, NULL));
/* Now the version line. */
if (chosen_version) {
- /* XXXX020 fails on very long version string */
- tor_snprintf(buf, sizeof(buf), "\nv %s\n", chosen_version);
- smartlist_add(chunks, tor_strdup(buf));
- } else {
- smartlist_add(chunks, tor_strdup("\n"));
+ smartlist_add(chunks, tor_strdup("\nv "));
+ smartlist_add(chunks, tor_strdup(chosen_version));
}
+ smartlist_add(chunks, tor_strdup("\n"));
/* And the loop is over and we move on to the next router */
}
@@ -653,8 +656,11 @@
tor_snprintf(buf, sizeof(buf), "%s %s\n", fingerprint,
signing_key_fingerprint);
/* And the signature. */
- /* XXXX020 check return */
- router_append_dirobj_signature(buf, sizeof(buf), digest, signing_key);
+ if (router_append_dirobj_signature(buf, sizeof(buf), digest,
+ signing_key)) {
+ log_warn(LD_BUG, "Couldn't sign consensus networkstatus.");
+ return NULL; /* This leaks, but it should never happen. */
+ }
smartlist_add(chunks, tor_strdup(buf));
}
@@ -680,7 +686,7 @@
return result;
}
-/** Check whether the pending_signature on <b>voter</b> is correctly signed by
+/** Check whether the signature on <b>voter</b> is correctly signed by
* the signing key of <b>cert</b>. Return -1 if <b>cert</b> doesn't match the
* signing key; otherwise set the good_signature or bad_signature flag on
* <b>voter</b>, and return 0. */
@@ -693,11 +699,10 @@
char d[DIGEST_LEN];
char *signed_digest;
size_t signed_digest_len;
- /*XXXX020 check return*/
- crypto_pk_get_digest(cert->signing_key, d);
- if (memcmp(voter->signing_key_digest, d, DIGEST_LEN)) {
+ if (crypto_pk_get_digest(cert->signing_key, d)<0)
return -1;
- }
+ if (memcmp(voter->signing_key_digest, d, DIGEST_LEN))
+ return -1;
signed_digest_len = crypto_pk_keysize(cert->signing_key);
signed_digest = tor_malloc(signed_digest_len);
if (crypto_pk_public_checksig(cert->signing_key,
@@ -705,14 +710,11 @@
voter->signature,
voter->signature_len) != DIGEST_LEN ||
memcmp(signed_digest, consensus->networkstatus_digest, DIGEST_LEN)) {
- log_warn(LD_DIR, "Got a bad signature."); /*XXXX020 say more*/
+ log_warn(LD_DIR, "Got a bad signature on a networkstatus vote");
voter->bad_signature = 1;
} else {
voter->good_signature = 1;
}
- /* XXXX020 did anything rely on this?
- * also, rename so it's no longer called "pending". */
- // tor_free(voter->signature);
return 0;
}
@@ -725,7 +727,7 @@
int n_bad = 0;
int n_unknown = 0;
int n_no_signature = 0;
- int n_required = 1; /* XXXX020 This is completely wrong. */
+ int n_required = get_n_authorities(V3_AUTHORITY)/2 + 1;
tor_assert(! consensus->is_vote);
@@ -741,7 +743,7 @@
continue;
}
if (networkstatus_check_voter_signature(consensus, voter, cert) < 0) {
- ++n_missing_key; /* XXXX020 what, really? */
+ ++n_missing_key;
continue;
}
}
@@ -754,7 +756,7 @@
});
log_notice(LD_DIR,
- "%d unknown, %d missing key, %d good, %d bad, %d no signature,"
+ "%d unknown, %d missing key, %d good, %d bad, %d no signature, "
"%d required", n_unknown, n_missing_key, n_good, n_bad,
n_no_signature, n_required);
@@ -1008,10 +1010,9 @@
tor_assert(timing_out);
timing_out->vote_interval = options->V3AuthVotingInterval;
- /* XXXX020 make these configurable. */
- timing_out->n_intervals_valid = 3;
- timing_out->vote_delay = 300;
- timing_out->dist_delay = 300;
+ timing_out->n_intervals_valid = options->V3AuthNIntervalsValid;
+ timing_out->vote_delay = options->V3AuthVoteDelay;
+ timing_out->dist_delay = options->V3AuthDistDelay;
}
/** DOCDOC */
@@ -1056,6 +1057,7 @@
void
dirvote_recalculate_timing(time_t now)
{
+ /*XXXX020 call this when inputs may have changed. */
int interval, vote_delay, dist_delay;
time_t start;
networkstatus_vote_t *consensus = networkstatus_get_latest_consensus();
@@ -1266,7 +1268,6 @@
*msg_out = "Error adding vote";
if (!*status_out)
*status_out = 400;
- /*XXXX020 free other fields */
return NULL;
}
@@ -1286,7 +1287,10 @@
n_voters = get_n_authorities(V3_AUTHORITY);
n_votes = smartlist_len(pending_vote_list);
- /* XXXX020 see if there are enough to go ahead. */
+ if (n_votes <= n_voters/2) {
+ log_warn(LD_DIR, "We don't have enough votes to generate a consensus.");
+ goto err;
+ }
if (!(my_cert = get_my_v3_authority_cert())) {
log_warn(LD_DIR, "Can't generate consensus without a certificate.");
@@ -1326,15 +1330,18 @@
pending_consensus = consensus;
if (pending_consensus_signature_list) {
+ int n_sigs = 0;
/* we may have gotten signatures for this consensus before we built
* it ourself. Add them now. */
SMARTLIST_FOREACH(pending_consensus_signature_list, char *, sig,
{
const char *msg = NULL;
- dirvote_add_signatures_to_pending_consensus(sig, &msg);
- /* XXXX020 log result. */
+ n_sigs += dirvote_add_signatures_to_pending_consensus(sig, &msg);
tor_free(sig);
});
+ if (n_sigs)
+ log_notice(LD_DIR, "Added %d pending signatures while building "
+ "consensus.", n_sigs);
smartlist_clear(pending_consensus_signature_list);
}
Modified: tor/trunk/src/or/or.h
===================================================================
--- tor/trunk/src/or/or.h 2007-08-14 00:07:29 UTC (rev 11098)
+++ tor/trunk/src/or/or.h 2007-08-14 02:23:57 UTC (rev 11099)
@@ -2067,8 +2067,11 @@
* if we are a cache). For authorities, this is always true. */
int DownloadExtraInfo;
- /** DOCDOC */
+ /** The length of time that we think a consensus should be */
int V3AuthVotingInterval;
+ int V3AuthVoteDelay;
+ int V3AuthDistDelay;
+ int V3AuthNIntervalsValid;
} or_options_t;
@@ -3530,7 +3533,7 @@
routerinfo_t *router_parse_entry_from_string(const char *s, const char *end,
int cache_copy);
extrainfo_t *extrainfo_parse_entry_from_string(const char *s, const char *end,
- int cache_copy, digestmap_t *routermap);
+ int cache_copy, struct digest_ri_map_t *routermap);
addr_policy_t *router_parse_addr_policy_from_string(const char *s,
int assume_action);
version_status_t tor_version_is_obsolete(const char *myversion,
Modified: tor/trunk/src/or/routerparse.c
===================================================================
--- tor/trunk/src/or/routerparse.c 2007-08-14 00:07:29 UTC (rev 11098)
+++ tor/trunk/src/or/routerparse.c 2007-08-14 02:23:57 UTC (rev 11099)
@@ -914,10 +914,9 @@
if (have_extrainfo && want_extrainfo) {
routerlist_t *rl = router_get_routerlist();
- /* XXXX020 fix this cast to digestmap_t* */
extrainfo = extrainfo_parse_entry_from_string(*s, end,
saved_location != SAVED_IN_CACHE,
- (digestmap_t*)rl->identity_map);
+ rl->identity_map);
if (extrainfo) {
signed_desc = &extrainfo->cache_info;
elt = extrainfo;
@@ -1197,7 +1196,7 @@
*/
extrainfo_t *
extrainfo_parse_entry_from_string(const char *s, const char *end,
- int cache_copy, digestmap_t *routermap)
+ int cache_copy, struct digest_ri_map_t *routermap)
{
extrainfo_t *extrainfo = NULL;
char digest[128];
@@ -1265,7 +1264,7 @@
}
if (routermap &&
- (router = digestmap_get(routermap,
+ (router = digestmap_get((digestmap_t*)routermap,
extrainfo->cache_info.identity_digest))) {
key = router->identity_pkey;
}
@@ -2517,30 +2516,28 @@
o_syn = table[i].os;
*s = eat_whitespace_eos_no_nl(next, eol);
next = find_whitespace_eos(*s, eol);
- if (1 || *s < eol) { /* make sure there are arguments to store */
- /* XXXX020 actually, we go ahead whether there are arguments or not,
- * so that tok->args is always set if we want arguments. */
- if (table[i].concat_args) {
- /* The keyword takes the line as a single argument */
- tok->args = tor_malloc(sizeof(char*));
- tok->args[0] = tor_strndup(*s,eol-*s); /* Grab everything on line */
- tok->n_args = 1;
- } else {
- /* This keyword takes multiple arguments. */
- j = 0;
- allocated = 16;
- tok->args = tor_malloc(sizeof(char*)*allocated);
- while (*s < eol) { /* While not at eol, store the next token */
- if (j == allocated) {
- allocated *= 2;
- tok->args = tor_realloc(tok->args,sizeof(char*)*allocated);
- }
- tok->args[j++] = tor_strndup(*s, next-*s);
- *s = eat_whitespace_eos_no_nl(next, eol); /* eat intra-line ws */
- next = find_whitespace_eos(*s, eol); /* find end of token at *s */
+ /* We go ahead whether there are arguments or not, so that tok->args is
+ * always set if we want arguments. */
+ if (table[i].concat_args) {
+ /* The keyword takes the line as a single argument */
+ tok->args = tor_malloc(sizeof(char*));
+ tok->args[0] = tor_strndup(*s,eol-*s); /* Grab everything on line */
+ tok->n_args = 1;
+ } else {
+ /* This keyword takes multiple arguments. */
+ j = 0;
+ allocated = 16;
+ tok->args = tor_malloc(sizeof(char*)*allocated);
+ while (*s < eol) { /* While not at eol, store the next token */
+ if (j == allocated) {
+ allocated *= 2;
+ tok->args = tor_realloc(tok->args,sizeof(char*)*allocated);
}
- tok->n_args = j;
+ tok->args[j++] = tor_strndup(*s, next-*s);
+ *s = eat_whitespace_eos_no_nl(next, eol); /* eat intra-line ws */
+ next = find_whitespace_eos(*s, eol); /* find end of token at *s */
}
+ tok->n_args = j;
}
if (tok->n_args < table[i].min_args) {
tor_snprintf(ebuf, sizeof(ebuf), "Too few arguments to %s", kwd);