From 44cdb9d73f1a0b7d2c8483a119b9c4d6caabc1ec Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Fri, 19 Feb 2016 15:30:03 +0100 Subject: [PATCH] gpg: Split check_key_signature2. * g10/sig-check.c (hash_uid_node): Rename from this... (hash_uid_packet): ... to this. Take a PKT_user_id instead of a KBNODE. (check_key_signature2): Split the basic signature checking functionality into... (check_signature_over_key_or_uid): ... this new function. -- Signed-off-by: Neal H. Walfield --- g10/main.h | 13 ++ g10/sig-check.c | 365 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 286 insertions(+), 92 deletions(-) diff --git a/g10/main.h b/g10/main.h index 863afa9e0..f7c47e91f 100644 --- a/g10/main.h +++ b/g10/main.h @@ -263,6 +263,19 @@ int check_key_signature2( KBNODE root, KBNODE node, PKT_public_key *check_pk, PKT_public_key *ret_pk, int *is_selfsig, u32 *r_expiredate, int *r_expired ); +/* Returns whether SIGNER generated the signature SIG over the packet + PACKET, which is a key, subkey or uid, and comes from the key block + KB. If SIGNER is NULL, it is looked up based on the information in + SIG. If not NULL, sets *IS_SELFSIG to indicate whether the + signature is a self-signature and *RET_PK to a copy of the signer's + key. */ +gpg_error_t check_signature_over_key_or_uid (PKT_public_key *signer, + PKT_signature *sig, + KBNODE kb, PACKET *packet, + int *is_selfsig, + PKT_public_key *ret_pk); + + /*-- delkey.c --*/ gpg_error_t delete_keys (strlist_t names, int secret, int allow_both); diff --git a/g10/sig-check.c b/g10/sig-check.c index 262afed37..4530a64c3 100644 --- a/g10/sig-check.c +++ b/g10/sig-check.c @@ -1,7 +1,7 @@ /* sig-check.c - Check a signature * Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, * 2004, 2006 Free Software Foundation, Inc. - * Copyright (C) 2015 g10 Code GmbH + * Copyright (C) 2015, 2016 g10 Code GmbH * * This file is part of GnuPG. * @@ -481,11 +481,8 @@ check_signature_end_simple (PKT_public_key *pk, PKT_signature *sig, /* Add a uid node to a hash context. See section 5.2.4, paragraph 4 of RFC 4880. */ static void -hash_uid_node( KBNODE unode, gcry_md_hd_t md, PKT_signature *sig ) +hash_uid_packet (PKT_user_id *uid, gcry_md_hd_t md, PKT_signature *sig ) { - PKT_user_id *uid = unode->pkt->pkt.user_id; - - assert( unode->pkt->pkttype == PKT_USER_ID ); if( uid->attrib_data ) { if( sig->version >=4 ) { byte buf[5]; @@ -691,6 +688,232 @@ check_key_signature (KBNODE root, KBNODE node, int *is_selfsig) } +/* Returns whether SIGNER generated the signature SIG over the packet + PACKET, which is a key, subkey or uid, and comes from the key block + KB. (KB is PACKET's corresponding keyblock; we don't assume that + SIG has been added to the keyblock.) + + If SIGNER is set, then checks whether SIGNER generated the + signature. Otherwise, uses SIG->KEYID to find the alleged signer. + This parameter can be used to effectively override the alleged + signer that is stored in SIG. + + KB may be NULL if SIGNER is set. + + Unlike check_key_signature, this function ignores any cached + results! That is, it does not consider SIG->FLAGS.CHECKED and + SIG->FLAGS.VALID nor does it set them. + + This doesn't check the signature's semantic mean. Concretely, it + doesn't check whether a non-self signed revocation signature was + created by a designated revoker. In fact, it doesn't return an + error for a binding generated by a completely different key! + + Returns 0 if the signature is valid. Returns GPG_ERR_SIG_CLASS if + this signature can't be over PACKET. Returns GPG_ERR_NOT_FOUND if + the key that generated the signature (according to SIG) could not + be found. Returns GPG_ERR_BAD_SIGNATURE if the signature is bad. + Other errors codes may be returned if something else goes wrong. + + IF IS_SELFSIG is not NULL, sets *IS_SELFSIG to 1 if this is a + self-signature (by the key's primary key) or 0 if not. + + If RET_PK is not NULL, returns a copy of the public key that + generated the signature (i.e., the signer) on success. This must + be released by the caller using release_public_key_parts (). */ +gpg_error_t +check_signature_over_key_or_uid (PKT_public_key *signer, + PKT_signature *sig, KBNODE kb, PACKET *packet, + int *is_selfsig, PKT_public_key *ret_pk) +{ + int rc; + PKT_public_key *pripk = kb->pkt->pkt.public_key; + gcry_md_hd_t md; + int signer_alloced = 0; + + rc = openpgp_pk_test_algo (sig->pubkey_algo); + if (rc) + return rc; + rc = openpgp_md_test_algo (sig->digest_algo); + if (rc) + return rc; + + /* A signature's class indicates the type of packet that it + signs. */ + if (/* Primary key binding (made by a subkey). */ + sig->sig_class == 0x19 + /* Direct key signature. */ + || sig->sig_class == 0x1f + /* Primary key revocation. */ + || sig->sig_class == 0x20) + { + if (packet->pkttype != PKT_PUBLIC_KEY) + /* Key revocations can only be over primary keys. */ + return gpg_error (GPG_ERR_SIG_CLASS); + } + else if (/* Subkey binding. */ + sig->sig_class == 0x18 + /* Subkey revocation. */ + || sig->sig_class == 0x28) + { + if (packet->pkttype != PKT_PUBLIC_SUBKEY) + return gpg_error (GPG_ERR_SIG_CLASS); + } + else if (/* Certification. */ + sig->sig_class == 0x10 + || sig->sig_class == 0x11 + || sig->sig_class == 0x12 + || sig->sig_class == 0x13 + /* Certification revocation. */ + || sig->sig_class == 0x30) + { + if (packet->pkttype != PKT_USER_ID) + return gpg_error (GPG_ERR_SIG_CLASS); + } + else + return gpg_error (GPG_ERR_SIG_CLASS); + + /* PACKET is the right type for SIG. */ + + if (signer) + { + if (is_selfsig) + { + if (signer->keyid[0] == pripk->keyid[0] + && signer->keyid[1] == pripk->keyid[1]) + *is_selfsig = 1; + else + *is_selfsig = 0; + } + } + else + { + /* Get the signer. If possible, avoid a look up. */ + if (sig->keyid[0] == pripk->keyid[0] + && sig->keyid[1] == pripk->keyid[1]) + /* Issued by the primary key. */ + { + signer = pripk; + if (is_selfsig) + *is_selfsig = 1; + } + else + /* See if one of the subkeys was the signer (although this is + extremely unlikely). */ + { + kbnode_t ctx = NULL; + kbnode_t n; + + while ((n = walk_kbnode (kb, &ctx, PKT_PUBLIC_SUBKEY))) + { + PKT_public_key *subk = n->pkt->pkt.public_key; + if (sig->keyid[0] == subk->keyid[0] + && sig->keyid[1] == subk->keyid[1]) + /* Issued by a subkey. */ + { + signer = subk; + break; + } + } + + if (! signer) + /* Signer by some other key. */ + { + if (is_selfsig) + *is_selfsig = 0; + if (ret_pk) + { + signer = ret_pk; + memset (signer, 0, sizeof (*signer)); + signer_alloced = 1; + } + else + { + signer = xmalloc_clear (sizeof (*signer)); + signer_alloced = 2; + } + + rc = get_pubkey (signer, sig->keyid); + if (rc) + { + xfree (signer); + signer = NULL; + signer_alloced = 0; + goto out; + } + } + } + } + + /* We checked above that we supported this algo, so an error here is + a bug. */ + if (gcry_md_open (&md, sig->digest_algo, 0)) + BUG (); + + /* Hash the relevant data. */ + + if (/* Direct key signature. */ + sig->sig_class == 0x1f + /* Primary key revocation. */ + || sig->sig_class == 0x20) + { + assert (packet->pkttype == PKT_PUBLIC_KEY); + hash_public_key (md, packet->pkt.public_key); + rc = check_signature_end_simple (signer, sig, md); + } + else if (/* Primary key binding (made by a subkey). */ + sig->sig_class == 0x19) + { + assert (packet->pkttype == PKT_PUBLIC_KEY); + hash_public_key (md, packet->pkt.public_key); + hash_public_key (md, signer); + rc = check_signature_end_simple (signer, sig, md); + } + else if (/* Subkey binding. */ + sig->sig_class == 0x18 + /* Subkey revocation. */ + || sig->sig_class == 0x28) + { + assert (packet->pkttype == PKT_PUBLIC_SUBKEY); + hash_public_key (md, pripk); + hash_public_key (md, packet->pkt.public_key); + rc = check_signature_end_simple (signer, sig, md); + } + else if (/* Certification. */ + sig->sig_class == 0x10 + || sig->sig_class == 0x11 + || sig->sig_class == 0x12 + || sig->sig_class == 0x13 + /* Certification revocation. */ + || sig->sig_class == 0x30) + { + assert (packet->pkttype == PKT_USER_ID); + hash_public_key (md, pripk); + hash_uid_packet (packet->pkt.user_id, md, sig); + rc = check_signature_end_simple (signer, sig, md); + } + else + /* We should never get here. (The first if above should have + already caught this error.) */ + BUG (); + + gcry_md_close (md); + + out: + if (! rc && ret_pk && (signer_alloced == -1 || ret_pk != signer)) + copy_public_key (ret_pk, signer); + if (signer_alloced == 1) + /* We looked up SIGNER; it is not a pointer into KB. */ + { + release_public_key_parts (signer); + if (signer_alloced == 2) + /* We also allocated the memory. */ + xfree (signer); + } + + return rc; +} + /* Check that a signature over a key (e.g., a key revocation, key * binding, user id certification, etc.) is valid. If the function * detects a self-signature, it uses the public key from the specified @@ -730,7 +953,6 @@ check_key_signature2 (kbnode_t root, kbnode_t node, PKT_public_key *check_pk, PKT_public_key *ret_pk, int *is_selfsig, u32 *r_expiredate, int *r_expired ) { - gcry_md_hd_t md; PKT_public_key *pk; PKT_signature *sig; int algo; @@ -791,114 +1013,69 @@ check_key_signature2 (kbnode_t root, kbnode_t node, PKT_public_key *check_pk, rc = check_revocation_keys (pk, sig); else { - if (gcry_md_open (&md, algo, 0)) - BUG (); - hash_public_key (md, pk); - rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk); - cache_sig_result (sig, rc); - gcry_md_close (md); + rc = check_signature_metadata_validity (pk, sig, + r_expired, NULL); + if (! rc) + rc = check_signature_over_key_or_uid (pk, sig, root, root->pkt, + is_selfsig, ret_pk); } } - else if (sig->sig_class == 0x28) /* subkey revocation */ + else if (sig->sig_class == 0x28 /* subkey revocation */ + || sig->sig_class == 0x18) /* key binding */ { kbnode_t snode = find_prev_kbnode (root, node, PKT_PUBLIC_SUBKEY); if (snode) { - if (gcry_md_open (&md, algo, 0)) - BUG (); - hash_public_key (md, pk); - hash_public_key (md, snode->pkt->pkt.public_key); - rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk); - cache_sig_result (sig, rc); - gcry_md_close (md); + rc = check_signature_metadata_validity (pk, sig, + r_expired, NULL); + if (! rc) + /* 0x28 must be a self-sig, but 0x18 needn't be. */ + rc = check_signature_over_key_or_uid (sig->sig_class == 0x18 + ? NULL : pk, + sig, root, snode->pkt, + is_selfsig, ret_pk); } else { if (opt.verbose) - log_info (_("key %s: no subkey for subkey" - " revocation signature\n"), keystr_from_pk(pk)); + { + if (sig->sig_class == 0x28) + log_info (_("key %s: no subkey for subkey" + " revocation signature\n"), keystr_from_pk(pk)); + else if (sig->sig_class == 0x18) + log_info(_("key %s: no subkey for subkey" + " binding signature\n"), keystr_from_pk(pk)); + } rc = GPG_ERR_SIG_CLASS; } } - else if (sig->sig_class == 0x18) /* key binding */ - { - kbnode_t snode = find_prev_kbnode (root, node, PKT_PUBLIC_SUBKEY); - - if (snode) - { - if (is_selfsig) - { - /* Does this make sense? It should always be a - selfsig. Yes: We can't be sure about this and we - need to be able to indicate that it is a selfsig. - FIXME: The question is whether we should reject - such a signature if it is not a selfsig. */ - u32 keyid[2]; - - keyid_from_pk (pk, keyid); - if (keyid[0] == sig->keyid[0] && keyid[1] == sig->keyid[1]) - *is_selfsig = 1; - } - if (gcry_md_open (&md, algo, 0)) - BUG (); - hash_public_key (md, pk); - hash_public_key (md, snode->pkt->pkt.public_key); - rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk); - cache_sig_result ( sig, rc ); - gcry_md_close (md); - } - else - { - if (opt.verbose) - log_info(_("key %s: no subkey for subkey" - " binding signature\n"), keystr_from_pk(pk)); - rc = GPG_ERR_SIG_CLASS; - } - } else if (sig->sig_class == 0x1f) /* direct key signature */ { - if (gcry_md_open (&md, algo, 0 )) - BUG (); - hash_public_key( md, pk ); - rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk); - cache_sig_result (sig, rc); - gcry_md_close (md); + rc = check_signature_metadata_validity (pk, sig, + r_expired, NULL); + if (! rc) + rc = check_signature_over_key_or_uid (pk, sig, root, root->pkt, + is_selfsig, ret_pk); } - else /* all other classes */ + else if (/* Certification. */ + sig->sig_class == 0x10 + || sig->sig_class == 0x11 + || sig->sig_class == 0x12 + || sig->sig_class == 0x13 + /* Certification revocation. */ + || sig->sig_class == 0x30) { kbnode_t unode = find_prev_kbnode (root, node, PKT_USER_ID); if (unode) { - u32 keyid[2]; - - keyid_from_pk (pk, keyid); - if (gcry_md_open (&md, algo, 0)) - BUG (); - hash_public_key (md, pk); - hash_uid_node (unode, md, sig); - if (keyid[0] == sig->keyid[0] && keyid[1] == sig->keyid[1]) - { /* The primary key is the signing key. */ - - if (is_selfsig) - *is_selfsig = 1; - rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk); - } - else if (check_pk) - { /* The caller specified a key. Try that. */ - - rc = check_signature_end (check_pk, sig, md, - r_expired, NULL, ret_pk); - } - else - { /* Look up the key. */ - rc = check_signature2 (sig, md, r_expiredate, r_expired, - NULL, ret_pk); - } - - cache_sig_result (sig, rc); - gcry_md_close (md); + rc = check_signature_metadata_validity (pk, sig, r_expired, NULL); + if (! rc) + /* If this is a self-sig, ignore check_pk. */ + rc = check_signature_over_key_or_uid + (keyid_cmp (pk_keyid (pk), sig->keyid) == 0 ? pk : check_pk, + sig, root, unode->pkt, NULL, ret_pk); } else { @@ -908,6 +1085,10 @@ check_key_signature2 (kbnode_t root, kbnode_t node, PKT_public_key *check_pk, rc = GPG_ERR_SIG_CLASS; } } + else + BUG (); + + cache_sig_result (sig, rc); return rc; }