prop250: Use RSA identity digest instead of fingerprint

The prop250 code used the RSA identity key fingerprint to index commit in a
digestmap instead of using the digest.

To behavior change except the fact that we are actually using digestmap
correctly.

Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit is contained in:
David Goulet 2016-05-09 18:58:19 -04:00 committed by David Goulet
parent c0cec2ffd3
commit 056b6186ad
4 changed files with 78 additions and 75 deletions

View File

@ -120,20 +120,19 @@ srv_dup(const sr_srv_t *orig)
return dup;
}
/* Allocate a new commit object and initializing it with <b>identity</b>
/* Allocate a new commit object and initializing it with <b>rsa_identity</b>
* that MUST be provided. The digest algorithm is set to the default one
* that is supported. The rest is uninitialized. This never returns NULL. */
static sr_commit_t *
commit_new(const char *rsa_identity_fpr)
commit_new(const char *rsa_identity)
{
sr_commit_t *commit;
tor_assert(rsa_identity_fpr);
tor_assert(rsa_identity);
commit = tor_malloc_zero(sizeof(*commit));
commit->alg = SR_DIGEST_ALG;
strlcpy(commit->rsa_identity_fpr, rsa_identity_fpr,
sizeof(commit->rsa_identity_fpr));
memcpy(commit->rsa_identity, rsa_identity, sizeof(commit->rsa_identity));
return commit;
}
@ -143,7 +142,7 @@ commit_log(const sr_commit_t *commit)
{
tor_assert(commit);
log_debug(LD_DIR, "SR: Commit from %s", commit->rsa_identity_fpr);
log_debug(LD_DIR, "SR: Commit from %s", sr_commit_get_rsa_fpr(commit));
log_debug(LD_DIR, "SR: Commit: [TS: %" PRIu64 "] [Encoded: %s]",
commit->commit_ts, commit->encoded_commit);
log_debug(LD_DIR, "SR: Reveal: [TS: %" PRIu64 "] [Encoded: %s]",
@ -160,7 +159,7 @@ verify_commit_and_reveal(const sr_commit_t *commit)
tor_assert(commit);
log_debug(LD_DIR, "SR: Validating commit from authority %s",
commit->rsa_identity_fpr);
sr_commit_get_rsa_fpr(commit));
/* Check that the timestamps match. */
if (commit->commit_ts != commit->reveal_ts) {
@ -194,7 +193,7 @@ verify_commit_and_reveal(const sr_commit_t *commit)
sizeof(received_hashed_reveal))) {
log_warn(LD_BUG, "SR: Received reveal value from authority %s "
"does't match the commit value.",
commit->rsa_identity_fpr);
sr_commit_get_rsa_fpr(commit));
goto invalid;
}
}
@ -242,14 +241,14 @@ commit_decode(const char *encoded, sr_commit_t *commit)
encoded, strlen(encoded));
if (decoded_len < 0) {
log_warn(LD_BUG, "SR: Commit from authority %s can't be decoded.",
commit->rsa_identity_fpr);
sr_commit_get_rsa_fpr(commit));
goto error;
}
if (decoded_len != SR_COMMIT_LEN) {
log_warn(LD_BUG, "SR: Commit from authority %s decoded length doesn't "
"match the expected length (%d vs %d).",
commit->rsa_identity_fpr, decoded_len, SR_COMMIT_LEN);
sr_commit_get_rsa_fpr(commit), decoded_len, SR_COMMIT_LEN);
goto error;
}
@ -295,14 +294,14 @@ reveal_decode(const char *encoded, sr_commit_t *commit)
encoded, strlen(encoded));
if (decoded_len < 0) {
log_warn(LD_BUG, "SR: Reveal from authority %s can't be decoded.",
commit->rsa_identity_fpr);
sr_commit_get_rsa_fpr(commit));
goto error;
}
if (decoded_len != SR_REVEAL_LEN) {
log_warn(LD_BUG, "SR: Reveal from authority %s decoded length is "
"doesn't match the expected length (%d vs %d)",
commit->rsa_identity_fpr, decoded_len, SR_REVEAL_LEN);
sr_commit_get_rsa_fpr(commit), decoded_len, SR_REVEAL_LEN);
goto error;
}
@ -396,7 +395,7 @@ get_srv_element_from_commit(const sr_commit_t *commit)
return NULL;
}
tor_asprintf(&element, "%s%s", commit->rsa_identity_fpr,
tor_asprintf(&element, "%s%s", sr_commit_get_rsa_fpr(commit),
commit->encoded_reveal);
return element;
}
@ -465,7 +464,7 @@ get_vote_line_from_commit(const sr_commit_t *commit, sr_phase_t phase)
tor_asprintf(&vote_line, "%s %s %s %s\n",
commit_ns_str,
crypto_digest_algorithm_get_name(commit->alg),
commit->rsa_identity_fpr,
sr_commit_get_rsa_fpr(commit),
commit->encoded_commit);
break;
case SR_PHASE_REVEAL:
@ -479,7 +478,7 @@ get_vote_line_from_commit(const sr_commit_t *commit, sr_phase_t phase)
tor_asprintf(&vote_line, "%s %s %s %s %s\n",
commit_ns_str,
crypto_digest_algorithm_get_name(commit->alg),
commit->rsa_identity_fpr,
sr_commit_get_rsa_fpr(commit),
commit->encoded_commit, reveal_str);
break;
}
@ -567,7 +566,8 @@ commit_is_authoritative(const sr_commit_t *commit,
tor_assert(commit);
tor_assert(voter_key);
return !strcmp(commit->rsa_identity_fpr, voter_key);
return !memcmp(commit->rsa_identity, voter_key,
sizeof(commit->rsa_identity));
}
/* Decide if the newly received <b>commit</b> should be kept depending on
@ -586,7 +586,8 @@ should_keep_commit(const sr_commit_t *commit, const char *voter_key,
tor_assert(voter_key);
log_debug(LD_DIR, "SR: Inspecting commit from %s (voter: %s)?",
commit->rsa_identity_fpr, voter_key);
sr_commit_get_rsa_fpr(commit),
hex_str(voter_key, DIGEST_LEN));
/* For a commit to be considered, it needs to be authoritative (it should
* be the voter's own commit). */
@ -597,7 +598,7 @@ should_keep_commit(const sr_commit_t *commit, const char *voter_key,
/* Check if the authority that voted for <b>commit</b> has already posted
* a commit before. */
saved_commit = sr_state_get_commit(commit->rsa_identity_fpr);
saved_commit = sr_state_get_commit(commit->rsa_identity);
switch (phase) {
case SR_PHASE_COMMIT:
@ -611,7 +612,8 @@ should_keep_commit(const sr_commit_t *commit, const char *voter_key,
if (commit_has_reveal_value(commit)) {
log_warn(LD_DIR, "SR: Commit from authority %s has a reveal value "
"during COMMIT phase. (voter: %s)",
commit->rsa_identity_fpr, voter_key);
sr_commit_get_rsa_fpr(commit),
hex_str(voter_key, DIGEST_LEN));
goto ignore;
}
break;
@ -635,7 +637,8 @@ should_keep_commit(const sr_commit_t *commit, const char *voter_key,
if (!commitments_are_the_same(commit, saved_commit)) {
log_warn(LD_DIR, "SR: Commit from authority %s is different from "
"previous commit in our state (voter: %s)",
commit->rsa_identity_fpr, voter_key);
sr_commit_get_rsa_fpr(commit),
hex_str(voter_key, DIGEST_LEN));
goto ignore;
}
@ -652,7 +655,8 @@ should_keep_commit(const sr_commit_t *commit, const char *voter_key,
if (verify_commit_and_reveal(commit) < 0) {
log_warn(LD_BUG, "SR: Commit from authority %s has an invalid "
"reveal value. (voter: %s)",
commit->rsa_identity_fpr, voter_key);
sr_commit_get_rsa_fpr(commit),
hex_str(voter_key, DIGEST_LEN));
goto ignore;
}
break;
@ -677,7 +681,7 @@ save_commit_during_reveal_phase(const sr_commit_t *commit)
tor_assert(commit);
/* Get the commit from our state. */
saved_commit = sr_state_get_commit(commit->rsa_identity_fpr);
saved_commit = sr_state_get_commit(commit->rsa_identity);
tor_assert(saved_commit);
/* Safety net. They can not be different commitments at this point. */
int same_commits = commitments_are_the_same(commit, saved_commit);
@ -868,18 +872,17 @@ sr_commit_t *
sr_generate_our_commit(time_t timestamp, const authority_cert_t *my_rsa_cert)
{
sr_commit_t *commit = NULL;
char fingerprint[FINGERPRINT_LEN+1];
char digest[DIGEST_LEN];
tor_assert(my_rsa_cert);
/* Get our RSA identity fingerprint */
if (crypto_pk_get_fingerprint(my_rsa_cert->identity_key,
fingerprint, 0) < 0) {
if (crypto_pk_get_digest(my_rsa_cert->identity_key, digest) < 0) {
goto error;
}
/* New commit with our identity key. */
commit = commit_new(fingerprint);
commit = commit_new(digest);
/* Generate the reveal random value */
crypto_strongest_rand(commit->random_number,
@ -1036,7 +1039,7 @@ sr_parse_srv(const smartlist_t *args)
sr_commit_t *
sr_parse_commit(const smartlist_t *args)
{
char *value;
char *value, digest[DIGEST_LEN];
digest_algorithm_t alg;
const char *rsa_identity_fpr;
sr_commit_t *commit = NULL;
@ -1054,7 +1057,8 @@ sr_parse_commit(const smartlist_t *args)
goto error;
}
/* Second argument is the RSA fingerprint of the auth */
/* Second argument is the RSA fingerprint of the auth and turn it into a
* digest value. */
rsa_identity_fpr = smartlist_get(args, 1);
if (base16_decode(digest, DIGEST_LEN, rsa_identity_fpr,
HEX_DIGEST_LEN) < 0) {
@ -1073,7 +1077,7 @@ sr_parse_commit(const smartlist_t *args)
}
/* Allocate commit since we have a valid identity now. */
commit = commit_new(rsa_identity_fpr);
commit = commit_new(digest);
/* Third argument is the commitment value base64-encoded. */
value = smartlist_get(args, 2);
@ -1103,7 +1107,7 @@ sr_parse_commit(const smartlist_t *args)
void
sr_handle_received_commits(smartlist_t *commits, crypto_pk_t *voter_key)
{
char rsa_identity_fpr[FINGERPRINT_LEN + 1];
char rsa_identity[DIGEST_LEN];
tor_assert(voter_key);
@ -1113,7 +1117,7 @@ sr_handle_received_commits(smartlist_t *commits, crypto_pk_t *voter_key)
}
/* Get the RSA identity fingerprint of this voter */
if (crypto_pk_get_fingerprint(voter_key, rsa_identity_fpr, 0) < 0) {
if (crypto_pk_get_digest(voter_key, rsa_identity) < 0) {
return;
}
@ -1121,7 +1125,7 @@ sr_handle_received_commits(smartlist_t *commits, crypto_pk_t *voter_key)
/* We won't need the commit in this list anymore, kept or not. */
SMARTLIST_DEL_CURRENT(commits, commit);
/* Check if this commit is valid and should be stored in our state. */
if (!should_keep_commit(commit, rsa_identity_fpr,
if (!should_keep_commit(commit, rsa_identity,
sr_state_get_phase())) {
sr_commit_free(commit);
continue;

View File

@ -71,8 +71,8 @@ typedef struct sr_commit_t {
/* Commit owner info */
/* The RSA identity fingerprint of the authority. */
char rsa_identity_fpr[FINGERPRINT_LEN + 1];
/* The RSA identity key of the authority. */
char rsa_identity[DIGEST_LEN];
/* Commitment information */
@ -112,11 +112,16 @@ void sr_commit_free(sr_commit_t *commit);
void sr_srv_encode(char *dst, const sr_srv_t *srv);
/* Private methods (only used by shared_random_state.c): */
static inline
const char *sr_commit_get_rsa_fpr(const sr_commit_t *commit)
{
return hex_str((const char *) commit->rsa_identity,
sizeof(commit->rsa_identity));
}
void sr_compute_srv(void);
sr_commit_t *sr_generate_our_commit(time_t timestamp,
const authority_cert_t *my_rsa_cert);
#ifdef SHARED_RANDOM_PRIVATE
/* Encode */

View File

@ -218,14 +218,14 @@ commit_add_to_state(sr_commit_t *commit, sr_state_t *state)
tor_assert(commit);
tor_assert(state);
saved_commit = digestmap_set(state->commits, commit->rsa_identity_fpr,
saved_commit = digestmap_set(state->commits, commit->rsa_identity,
commit);
if (saved_commit != NULL) {
/* This means we already have that commit in our state so adding twice
* the same commit is either a code flow error, a corrupted disk state
* or some new unknown issue. */
log_warn(LD_DIR, "SR: Commit from %s exists in our state while "
"adding it: '%s'", commit->rsa_identity_fpr,
"adding it: '%s'", sr_commit_get_rsa_fpr(commit),
commit->encoded_commit);
sr_commit_free(saved_commit);
}
@ -562,7 +562,7 @@ disk_state_put_commit_line(const sr_commit_t *commit, config_line_t *line)
}
tor_asprintf(&line->value, "%s %s %s%s",
crypto_digest_algorithm_get_name(commit->alg),
commit->rsa_identity_fpr,
sr_commit_get_rsa_fpr(commit),
commit->encoded_commit,
reveal_str != NULL ? reveal_str : "");
if (reveal_str != NULL) {
@ -1124,17 +1124,17 @@ sr_state_update(time_t valid_after)
}
}
/* Return commit object from the given authority digest <b>identity</b>.
/* Return commit object from the given authority digest <b>rsa_identity</b>.
* Return NULL if not found. */
sr_commit_t *
sr_state_get_commit(const char *rsa_fpr)
sr_state_get_commit(const char *rsa_identity)
{
sr_commit_t *commit;
tor_assert(rsa_fpr);
tor_assert(rsa_identity);
state_query(SR_STATE_ACTION_GET, SR_STATE_OBJ_COMMIT,
(void *) rsa_fpr, (void *) &commit);
(void *) rsa_identity, (void *) &commit);
return commit;
}
@ -1150,7 +1150,7 @@ sr_state_add_commit(sr_commit_t *commit)
(void *) commit, NULL);
log_debug(LD_DIR, "SR: Commit from %s has been added to our state.",
commit->rsa_identity_fpr);
sr_commit_get_rsa_fpr(commit));
}
/* Remove all commits from our state. */
@ -1178,7 +1178,7 @@ sr_state_copy_reveal_info(sr_commit_t *saved_commit, const sr_commit_t *commit)
state_query(SR_STATE_ACTION_SAVE, 0, NULL, NULL);
log_debug(LD_DIR, "SR: Reveal value learned %s (for commit %s) from %s",
saved_commit->encoded_reveal, saved_commit->encoded_commit,
saved_commit->rsa_identity_fpr);
sr_commit_get_rsa_fpr(saved_commit));
}
/* Set the fresh SRV flag from our state. This doesn't need to trigger a

View File

@ -331,7 +331,7 @@ test_sr_commit(void *arg)
sr_commit_t *parsed_commit;
smartlist_add(args,
tor_strdup(crypto_digest_algorithm_get_name(our_commit->alg)));
smartlist_add(args, our_commit->rsa_identity_fpr);
smartlist_add(args, tor_strdup(sr_commit_get_rsa_fpr(our_commit)));
smartlist_add(args, our_commit->encoded_commit);
smartlist_add(args, our_commit->encoded_reveal);
parsed_commit = sr_parse_commit(args);
@ -480,7 +480,7 @@ test_vote(void *arg)
tt_assert(our_commit);
sr_state_add_commit(our_commit);
/* Make sure it's there. */
saved_commit = sr_state_get_commit(our_commit->rsa_identity_fpr);
saved_commit = sr_state_get_commit(our_commit->rsa_identity);
tt_assert(saved_commit);
}
@ -508,8 +508,10 @@ test_vote(void *arg)
tt_str_op(smartlist_get(tokens, 0), OP_EQ, "shared-rand-commit");
tt_str_op(smartlist_get(tokens, 1), OP_EQ,
crypto_digest_algorithm_get_name(DIGEST_SHA3_256));
tt_str_op(smartlist_get(tokens, 2), OP_EQ,
our_commit->rsa_identity_fpr);
char digest[DIGEST_LEN];
base16_decode(digest, sizeof(digest), smartlist_get(tokens, 2),
HEX_DIGEST_LEN);
tt_mem_op(digest, ==, our_commit->rsa_identity, sizeof(digest));
tt_str_op(smartlist_get(tokens, 3), OP_EQ, our_commit->encoded_commit);
tt_str_op(smartlist_get(tokens, 4), OP_EQ, our_commit->encoded_reveal);
@ -664,9 +666,7 @@ test_sr_setup_commits(void)
tt_assert(commit_a);
/* Do some surgery on the commit */
strlcpy(commit_a->rsa_identity_fpr,
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
sizeof(commit_a->rsa_identity_fpr));
memset(commit_a->rsa_identity, 'A', sizeof(commit_a->rsa_identity));
strlcpy(commit_a->encoded_reveal,
"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
sizeof(commit_a->encoded_reveal));
@ -680,9 +680,7 @@ test_sr_setup_commits(void)
tt_assert(commit_b);
/* Do some surgery on the commit */
strlcpy(commit_b->rsa_identity_fpr,
"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
sizeof(commit_b->rsa_identity_fpr));
memset(commit_b->rsa_identity, 'B', sizeof(commit_b->rsa_identity));
strlcpy(commit_b->encoded_reveal,
"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB",
sizeof(commit_b->encoded_reveal));
@ -696,9 +694,7 @@ test_sr_setup_commits(void)
tt_assert(commit_c);
/* Do some surgery on the commit */
strlcpy(commit_c->rsa_identity_fpr,
"ccccccccccccccccccccccccccccccccccccccccccccccccc",
sizeof(commit_c->rsa_identity_fpr));
memset(commit_c->rsa_identity, 'C', sizeof(commit_c->rsa_identity));
strlcpy(commit_c->encoded_reveal,
"CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC",
sizeof(commit_c->encoded_reveal));
@ -712,9 +708,7 @@ test_sr_setup_commits(void)
tt_assert(commit_d);
/* Do some surgery on the commit */
strlcpy(commit_d->rsa_identity_fpr,
"ddddddddddddddddddddddddddddddddddddddddddddddddd",
sizeof(commit_d->rsa_identity_fpr));
memset(commit_d->rsa_identity, 'D', sizeof(commit_d->rsa_identity));
strlcpy(commit_d->encoded_reveal,
"DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD",
sizeof(commit_d->encoded_reveal));
@ -946,17 +940,17 @@ test_utils(void *arg)
/* Testing commit_is_authoritative(). */
{
crypto_pk_t *k = crypto_pk_new();
char fp[FINGERPRINT_LEN + 1];
char digest[DIGEST_LEN];
sr_commit_t commit;
tt_assert(!crypto_pk_generate_key(k));
tt_int_op(0, ==, crypto_pk_get_fingerprint(k, fp, 0));
memcpy(fp, commit.rsa_identity_fpr, sizeof(fp));
tt_int_op(commit_is_authoritative(&commit, fp), ==, 1);
tt_int_op(0, ==, crypto_pk_get_digest(k, digest));
memcpy(commit.rsa_identity, digest, sizeof(commit.rsa_identity));
tt_int_op(commit_is_authoritative(&commit, digest), ==, 1);
/* Change the pubkey. */
memset(commit.rsa_identity_fpr, 0, sizeof(commit.rsa_identity_fpr));
tt_int_op(commit_is_authoritative(&commit, fp), ==, 0);
memset(commit.rsa_identity, 0, sizeof(commit.rsa_identity));
tt_int_op(commit_is_authoritative(&commit, digest), ==, 0);
}
/* Testing get_phase_str(). */
@ -1105,17 +1099,17 @@ test_keep_commit(void *arg)
tt_int_op(should_keep_commit(commit, fp, SR_PHASE_COMMIT), ==, 0);
/* This should NOT be kept because it has a reveal value in it. */
tt_assert(commit_has_reveal_value(commit));
tt_int_op(should_keep_commit(commit, commit->rsa_identity_fpr,
tt_int_op(should_keep_commit(commit, commit->rsa_identity,
SR_PHASE_COMMIT), ==, 0);
/* Add it to the state which should return to not keep it. */
sr_state_add_commit(commit);
tt_int_op(should_keep_commit(commit, commit->rsa_identity_fpr,
tt_int_op(should_keep_commit(commit, commit->rsa_identity,
SR_PHASE_COMMIT), ==, 0);
/* Remove it from state so we can continue our testing. */
digestmap_remove(state->commits, commit->rsa_identity_fpr);
digestmap_remove(state->commits, commit->rsa_identity);
/* Let's remove our reveal value which should make it OK to keep it. */
memset(commit->encoded_reveal, 0, sizeof(commit->encoded_reveal));
tt_int_op(should_keep_commit(commit, commit->rsa_identity_fpr,
tt_int_op(should_keep_commit(commit, commit->rsa_identity,
SR_PHASE_COMMIT), ==, 1);
/* Let's reset our commit and go into REVEAL phase. */
@ -1130,14 +1124,14 @@ test_keep_commit(void *arg)
/* We should never keep a commit from a non authoritative authority. */
tt_int_op(should_keep_commit(commit, fp, SR_PHASE_REVEAL), ==, 0);
/* We shouldn't accept a commit that is not in our state. */
tt_int_op(should_keep_commit(commit, commit->rsa_identity_fpr,
tt_int_op(should_keep_commit(commit, commit->rsa_identity,
SR_PHASE_REVEAL), ==, 0);
/* Important to add the commit _without_ the reveal here. */
sr_state_add_commit(dup_commit);
tt_int_op(digestmap_size(state->commits), ==, 1);
/* Our commit should be valid that is authoritative, contains a reveal, be
* in the state and commitment and reveal values match. */
tt_int_op(should_keep_commit(commit, commit->rsa_identity_fpr,
tt_int_op(should_keep_commit(commit, commit->rsa_identity,
SR_PHASE_REVEAL), ==, 1);
/* The commit shouldn't be kept if it's not verified that is no matchin
* hashed reveal. */
@ -1147,17 +1141,17 @@ test_keep_commit(void *arg)
memcpy(place_holder.hashed_reveal, commit->hashed_reveal,
sizeof(place_holder.hashed_reveal));
memset(commit->hashed_reveal, 0, sizeof(commit->hashed_reveal));
tt_int_op(should_keep_commit(commit, commit->rsa_identity_fpr,
tt_int_op(should_keep_commit(commit, commit->rsa_identity,
SR_PHASE_REVEAL), ==, 0);
memcpy(commit->hashed_reveal, place_holder.hashed_reveal,
sizeof(commit->hashed_reveal));
}
/* We shouldn't keep a commit that has no reveal. */
tt_int_op(should_keep_commit(dup_commit, dup_commit->rsa_identity_fpr,
tt_int_op(should_keep_commit(dup_commit, dup_commit->rsa_identity,
SR_PHASE_REVEAL), ==, 0);
/* We must not keep a commit that is not the same from the commit phase. */
memset(commit->encoded_commit, 0, sizeof(commit->encoded_commit));
tt_int_op(should_keep_commit(commit, commit->rsa_identity_fpr,
tt_int_op(should_keep_commit(commit, commit->rsa_identity,
SR_PHASE_REVEAL), ==, 0);
done: