From b410a3cb7683fc7c2a253e23130c44df42a6203c Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Thu, 1 Sep 2016 23:31:18 +0200 Subject: [PATCH] g10: Don't consider cross-signed keys to be in conflict. * g10/tofu.c (cross_sigs): New function. (ask_about_binding): If apparently conflicting keys are cross signed, then don't mark them as conflicting. -- Signed-off-by: Neal H. Walfield If two keys are cross signed, then the same person (probably) controlled them both. In this case, don't raise a TOFU conflict. This usually occurs when someone transitions to a new key. When that person rotates to a third key, she will typically only cross sign it with the second key. As such, we check this transitively to avoid declaring a conflict between the 1st and 3rd key. --- g10/tofu.c | 162 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 158 insertions(+), 4 deletions(-) diff --git a/g10/tofu.c b/g10/tofu.c index 214782d2a..2ac60658b 100644 --- a/g10/tofu.c +++ b/g10/tofu.c @@ -1204,6 +1204,80 @@ format_conflict_msg_part1 (int policy, const char *conflict, } +/* Return 1 if A signed B and B signed A. */ +int +cross_sigs (kbnode_t a, kbnode_t b) +{ + int i; + + PKT_public_key *a_pk = a->pkt->pkt.public_key; + PKT_public_key *b_pk = b->pkt->pkt.public_key; + + char a_keyid[33]; + char b_keyid[33]; + + if (DBG_TRUST) + { + format_keyid (pk_main_keyid (a_pk), + KF_DEFAULT, a_keyid, sizeof (a_keyid)); + format_keyid (pk_main_keyid (b_pk), + KF_DEFAULT, b_keyid, sizeof (b_keyid)); + } + + for (i = 0; i < 2; i ++) + { + /* See if SIGNER signed SIGNEE. */ + + kbnode_t signer = i == 0 ? a : b; + kbnode_t signee = i == 0 ? b : a; + + PKT_public_key *signer_pk = signer->pkt->pkt.public_key; + u32 *signer_kid = pk_main_keyid (signer_pk); + kbnode_t n; + + /* Iterate over SIGNEE's keyblock and see if there is a valid + signature from SIGNER. */ + for (n = signee; n; n = n->next) + { + PKT_signature *sig; + + if (n->pkt->pkttype != PKT_SIGNATURE) + continue; + + sig = n->pkt->pkt.signature; + + if (! (sig->sig_class == 0x10 + || sig->sig_class == 0x11 + || sig->sig_class == 0x12 + || sig->sig_class == 0x13)) + /* Not a signature over a user id. */ + continue; + + /* SIG is on SIGNEE's keyblock. If SIG was generated by the + signer, then it's a match. */ + if (keyid_cmp (sig->keyid, signer_kid) == 0) + /* Match! */ + break; + } + if (! n) + /* We didn't find a signature from signer over signee. */ + { + if (DBG_TRUST) + log_info ("No cross sig between %s and %s\n", + a_keyid, b_keyid); + return 0; + } + } + + /* A signed B and B signed A. */ + if (DBG_TRUST) + log_info ("Cross sig between %s and %s\n", + a_keyid, b_keyid); + + return 1; +} + + /* Ask the user about the binding. There are three ways we could end * up here: * @@ -1237,7 +1311,7 @@ ask_about_binding (ctrl_t ctrl, strlist_t other_user_ids = NULL; struct signature_stats *stats = NULL; struct signature_stats *stats_iter = NULL; - char *prompt; + char *prompt = NULL; char *choices; dbs = ctrl->tofu.dbs; @@ -1361,9 +1435,17 @@ ask_about_binding (ctrl_t ctrl, } else { + int stats_count = 0; + kbnode_t *kb_all; KEYDB_HANDLE hd; + int i; char *key = NULL; + /* Get the keyblock for each key. */ + for (stats_iter = stats; stats_iter; stats_iter = stats_iter->next) + stats_count ++; + kb_all = xcalloc (sizeof (kb_all[0]), stats_count); + if (! stats || strcmp (stats->fingerprint, fingerprint)) { /* If we have already added this key to the DB, then it will @@ -1375,7 +1457,9 @@ ask_about_binding (ctrl_t ctrl, /* Figure out which user ids are revoked or expired. */ hd = keydb_new (); - for (stats_iter = stats; stats_iter; stats_iter = stats_iter->next) + for (stats_iter = stats, i = 0; + stats_iter; + stats_iter = stats_iter->next, i ++) { KEYDB_SEARCH_DESC desc; kbnode_t kb; @@ -1420,6 +1504,9 @@ ask_about_binding (ctrl_t ctrl, merge_keys_and_selfsig (kb); log_assert (kb->pkt->pkttype == PKT_PUBLIC_KEY); + + kb_all[i] = kb; + pk = kb->pkt->pkt.public_key; if (pk->has_expired) @@ -1451,7 +1538,6 @@ ask_about_binding (ctrl_t ctrl, xfree (email2); } - release_kbnode (kb); if (! found_user_id) log_info (_("TOFU db may be corrupted: user id (%s)" @@ -1460,7 +1546,74 @@ ask_about_binding (ctrl_t ctrl, } keydb_release (hd); - es_fprintf (fp, _("Statistics for keys with the email address \"%s\":\n"), + { + int j; + struct signature_stats **stats_prevp; + struct signature_stats *stats_iter_next; + int die[stats_count]; + + memset (die, 0, sizeof (die)); + + for (i = 0; i < stats_count; i ++) + { + /* i or a key that has cross sigs with i (possible + indirectly)? */ + if (! (i == 0 || die[i])) + continue; + + for (j = i + 1; j < stats_count; j ++) + if (cross_sigs (kb_all[i], kb_all[j])) + die[j] = 1; + } + + /* Free the dead stat structures. */ + for (stats_iter = stats, stats_prevp = &stats, i = 0; + stats_iter; + stats_iter = stats_iter_next, i ++) + { + stats_iter_next = stats_iter->next; + + release_kbnode (kb_all[i]); + + if (die[i]) + { + *stats_prevp = stats_iter_next; + stats_iter->next = NULL; + signature_stats_free (stats_iter); + + bindings_with_this_email_count --; + } + else + { + stats_prevp = &stats_iter->next; + } + } + } + + log_assert (stats); + log_assert (bindings_with_this_email_count >= 1); + + if ((*policy == TOFU_POLICY_NONE && bindings_with_this_email_count == 1) + || (*policy == TOFU_POLICY_ASK && conflict)) + if (bindings_with_this_email_count == 1) + { + /* All "conflicts" were not really conflicts. */ + log_assert (! stats->next); + + if (DBG_TRUST) + log_debug ("%s: all apparent TOFU conflicts are legitimate " + "(cross sigs), setting policy to auto.\n", + stats_iter->fingerprint); + + *policy = TOFU_POLICY_AUTO; + record_binding (dbs, fingerprint, email, user_id, *policy, 0); + *trust_level = tofu_policy_to_trust_level (*policy); + + goto out; + } + + es_fprintf (fp, _("Statistics for potentially conflicting keys" + " with the email address \"%s\":\n"), email); for (stats_iter = stats; stats_iter; stats_iter = stats_iter->next) { @@ -1644,6 +1797,7 @@ ask_about_binding (ctrl_t ctrl, } xfree (response); } + out: tofu_resume_batch_transaction (ctrl); xfree (prompt);