From 95d0f3e5eebd85dcf226dca14891a1215bfe93ae Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Thu, 13 Oct 2016 20:54:06 +0200 Subject: [PATCH] g10: Be more careful when checking if a binding is signed by a UTK. * g10/tofu.c (signed_by_utk): When checking if a key is signed by an ultimately trusted key, only consider the signatures on the specified user id. * tests/openpgp/tofu.scm: Add test for the above. -- Signed-off-by: Neal H. Walfield --- g10/tofu.c | 27 +++++++++++++- tests/openpgp/tofu.scm | 84 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/g10/tofu.c b/g10/tofu.c index dcee6e70f..d437c5a72 100644 --- a/g10/tofu.c +++ b/g10/tofu.c @@ -1308,14 +1308,37 @@ cross_sigs (const char *email, kbnode_t a, kbnode_t b) /* Return whether the key was signed by an ultimately trusted key. */ static int -signed_by_utk (kbnode_t a) +signed_by_utk (const char *email, kbnode_t a) { kbnode_t n; + int saw_email = 0; for (n = a; n; n = n->next) { PKT_signature *sig; + if (n->pkt->pkttype == PKT_USER_ID) + { + if (saw_email) + /* We're done: we've processed all signatures on the + user id. */ + break; + else + { + /* See if this is the matching user id. */ + PKT_user_id *user_id = n->pkt->pkt.user_id; + char *email2 = email_from_user_id (user_id->name); + + if (strcmp (email, email2) == 0) + saw_email = 1; + + xfree (email2); + } + } + + if (! saw_email) + continue; + if (n->pkt->pkttype != PKT_SIGNATURE) continue; @@ -2221,7 +2244,7 @@ get_trust (ctrl_t ctrl, PKT_public_key *pk, } else { - is_signed_by_utk = signed_by_utk (kb); + is_signed_by_utk = signed_by_utk (email, kb); release_kbnode (kb); } } diff --git a/tests/openpgp/tofu.scm b/tests/openpgp/tofu.scm index 96f7abe7a..2939250e8 100755 --- a/tests/openpgp/tofu.scm +++ b/tests/openpgp/tofu.scm @@ -198,7 +198,7 @@ (verify-messages) (display "<\n") -;; Since their is no conflict, the policy should be auto. +;; Since there is no conflict, the policy should be auto. (checkpolicy KEYA "auto") (checkpolicy KEYB "auto") @@ -232,3 +232,85 @@ (checkpolicy KEYA "auto") (checkpolicy KEYB "auto") + +;; Remove the keys. +(call-check `(,@GPG --delete-key ,KEYA)) +(call-check `(,@GPG --delete-key ,KEYB)) + + +;; Check that we detect the following attack: +;; +;; Alice has an ultimately trusted key and she signs Bob's key. Then +;; Bob adds a new user id, "Alice". TOFU should now detect a +;; conflict, because Alice only signed Bob's "Bob" user id. + +(display "Checking UTK sigs...\n") +(define GPG `(,(tool 'gpg) --no-permission-warning + --faked-system-time=1476304861)) + +;; Carefully remove the TOFU db. +(catch '() (unlink (string-append GNUPGHOME "/tofu.db"))) + +(define DIR "tofu/cross-sigs") +;; The test keys. +(define KEYA "1938C3A0E4674B6C217AC0B987DB2814EC38277E") +(define KEYB "DC463A16E42F03240D76E8BA8B48C6BD871C2247") + +(define (verify-messages) + (for-each + (lambda (key) + (for-each + (lambda (i) + (let ((fn (in-srcdir DIR (string-append key "-" i ".txt")))) + (call-check `(,@GPG --trust-model=tofu --verify ,fn)))) + (list "1" "2"))) + (list KEYA KEYB))) + +;; Import the public keys. +(display " > Two keys. ") +(call-check `(,@GPG --import ,(in-srcdir DIR (string-append KEYA "-1.gpg")))) +(call-check `(,@GPG --import ,(in-srcdir DIR (string-append KEYB "-1.gpg")))) +(display "<\n") + +;; Import the cross sigs. +(display " > Adding cross signatures. ") +(call-check `(,@GPG --import ,(in-srcdir DIR (string-append KEYA "-2.gpg")))) +(call-check `(,@GPG --import ,(in-srcdir DIR (string-append KEYB "-2.gpg")))) +(display "<\n") + +;; Make KEYA ultimately trusted. +(display (string-append " > Marking " KEYA " as ultimately trusted. ")) +(pipe:do + (pipe:echo (string-append KEYA ":6:\n")) + (pipe:gpg `(--import-ownertrust))) +(display "<\n") + +;; An ultimately trusted key's policy is good. +(checkpolicy KEYA "good") +;; A key signed by a UTK for which there is no policy gets the default +;; policy of good. +(checkpolicy KEYB "good") + +;; Import the conflicting user id. +(display " > Adding conflicting user id. ") +(call-check `(,@GPG --import ,(in-srcdir DIR (string-append KEYB "-3.gpg")))) +(call-check `(,@GPG --trust-model=tofu + --verify ,(in-srcdir DIR (string-append KEYB "-1.txt")))) +(verify-messages) +(display "<\n") + +(checkpolicy KEYA "good") +(checkpolicy KEYB "ask") + +;; Import Alice's signature on the conflicting user id. +(display " > Adding cross signature on user id. ") +(call-check `(,@GPG --import ,(in-srcdir DIR (string-append KEYB "-4.gpg")))) +(verify-messages) +(display "<\n") + +(checkpolicy KEYA "good") +(checkpolicy KEYB "good") + +;; Remove the keys. +(call-check `(,@GPG --delete-key ,KEYA)) +(call-check `(,@GPG --delete-key ,KEYB))