From c23c18c1543d1ff58f0f78baaa6a8e319c659ecb Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 14 Aug 2014 15:20:53 +0200 Subject: [PATCH] gpg: Screen keyserver responses. * g10/main.h (import_screener_t): New. * g10/import.c (import): Add screener callbacks to param list. (import_one): Ditto. (import_secret_one): Ditto. (import_keys_internal): Ditto. (import_keys_stream): Ditto. * g10/keyserver.c (struct ks_retrieval_screener_arg_s): New. (keyserver_retrieval_screener): New. (keyserver_get): Pass screener to import_keys_es_stream(). -- These changes introduces import functions that apply a constraining filter to imported keys. These filters can verify the fingerprints of the keys returned before importing them into the keyring, ensuring that the keys fetched from the keyserver are in fact those selected by the user beforehand. Signed-off-by: Stefan Tomanek This is an extended and fixed versions of Stefan's patch. In addition to the changes done in gnupg 2.0, namely the commits 5e933008beffbeae7255ece02383606481f9c169 044847a0e2013a2833605c1a9f80cfa6ef353309 088f82c0b5e39687f70e44d3ab719854e808eeb6 the symbol names have been changed to "screener" to void mixing them up with the iobuf filter feature and it has been changed to be used with the dirmngr based keyserver lookup. Signed-off-by: Werner Koch --- g10/import.c | 69 +++++++++++++++++++++++++----------- g10/keyserver.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++--- g10/main.h | 5 ++- 3 files changed, 143 insertions(+), 25 deletions(-) diff --git a/g10/import.c b/g10/import.c index 774a727da..ca35ce1b9 100644 --- a/g10/import.c +++ b/g10/import.c @@ -63,16 +63,19 @@ struct stats_s { static int import (ctrl_t ctrl, IOBUF inp, const char* fname, struct stats_s *stats, - unsigned char **fpr, size_t *fpr_len, unsigned int options); + unsigned char **fpr, size_t *fpr_len, unsigned int options, + import_screener_t screener, void *screener_arg); static int read_block( IOBUF a, PACKET **pending_pkt, KBNODE *ret_root ); static void revocation_present (ctrl_t ctrl, kbnode_t keyblock); static int import_one (ctrl_t ctrl, const char *fname, KBNODE keyblock,struct stats_s *stats, - unsigned char **fpr,size_t *fpr_len, - unsigned int options,int from_sk, int silent); + unsigned char **fpr, size_t *fpr_len, + unsigned int options, int from_sk, int silent, + import_screener_t screener, void *screener_arg); static int import_secret_one (ctrl_t ctrl, const char *fname, KBNODE keyblock, struct stats_s *stats, int batch, - unsigned int options, int for_migration); + unsigned int options, int for_migration, + import_screener_t screener, void *screener_arg); static int import_revoke_cert( const char *fname, KBNODE node, struct stats_s *stats); static int chk_self_sigs( const char *fname, KBNODE keyblock, @@ -169,7 +172,8 @@ import_release_stats_handle (void *p) static int import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames, void *stats_handle, unsigned char **fpr, size_t *fpr_len, - unsigned int options ) + unsigned int options, + import_screener_t screener, void *screener_arg) { int i, rc = 0; struct stats_s *stats = stats_handle; @@ -178,7 +182,8 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames, stats = import_new_stats_handle (); if (inp) { - rc = import (ctrl, inp, "[stream]", stats, fpr, fpr_len, options); + rc = import (ctrl, inp, "[stream]", stats, fpr, fpr_len, options, + screener, screener_arg); } else { if( !fnames && !nnames ) @@ -199,7 +204,8 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames, log_error(_("can't open '%s': %s\n"), fname, strerror(errno) ); else { - rc = import (ctrl, inp2, fname, stats, fpr, fpr_len, options); + rc = import (ctrl, inp2, fname, stats, fpr, fpr_len, options, + screener, screener_arg); iobuf_close(inp2); /* Must invalidate that ugly cache to actually close it. */ iobuf_ioctl (NULL, IOBUF_IOCTL_INVALIDATE_CACHE, @@ -235,15 +241,15 @@ import_keys (ctrl_t ctrl, char **fnames, int nnames, void *stats_handle, unsigned int options ) { import_keys_internal (ctrl, NULL, fnames, nnames, stats_handle, - NULL, NULL, options); + NULL, NULL, options, NULL, NULL); } int import_keys_stream (ctrl_t ctrl, IOBUF inp, void *stats_handle, - unsigned char **fpr, size_t *fpr_len,unsigned int options) + unsigned char **fpr, size_t *fpr_len, unsigned int options) { return import_keys_internal (ctrl, inp, NULL, 0, stats_handle, - fpr, fpr_len, options); + fpr, fpr_len, options, NULL, NULL); } @@ -251,7 +257,8 @@ import_keys_stream (ctrl_t ctrl, IOBUF inp, void *stats_handle, int import_keys_es_stream (ctrl_t ctrl, estream_t fp, void *stats_handle, unsigned char **fpr, size_t *fpr_len, - unsigned int options) + unsigned int options, + import_screener_t screener, void *screener_arg) { int rc; iobuf_t inp; @@ -265,7 +272,8 @@ import_keys_es_stream (ctrl_t ctrl, estream_t fp, void *stats_handle, } rc = import_keys_internal (ctrl, inp, NULL, 0, stats_handle, - fpr, fpr_len, options); + fpr, fpr_len, options, + screener, screener_arg); iobuf_close (inp); return rc; @@ -274,7 +282,8 @@ import_keys_es_stream (ctrl_t ctrl, estream_t fp, void *stats_handle, static int import (ctrl_t ctrl, IOBUF inp, const char* fname,struct stats_s *stats, - unsigned char **fpr,size_t *fpr_len,unsigned int options ) + unsigned char **fpr,size_t *fpr_len, unsigned int options, + import_screener_t screener, void *screener_arg) { PACKET *pending_pkt = NULL; KBNODE keyblock = NULL; /* Need to initialize because gcc can't @@ -296,10 +305,12 @@ import (ctrl_t ctrl, IOBUF inp, const char* fname,struct stats_s *stats, while( !(rc = read_block( inp, &pending_pkt, &keyblock) )) { if( keyblock->pkt->pkttype == PKT_PUBLIC_KEY ) rc = import_one (ctrl, fname, keyblock, - stats, fpr, fpr_len, options, 0, 0); + stats, fpr, fpr_len, options, 0, 0, + screener, screener_arg); else if( keyblock->pkt->pkttype == PKT_SECRET_KEY ) rc = import_secret_one (ctrl, fname, keyblock, stats, - opt.batch, options, 0); + opt.batch, options, 0, + screener, screener_arg); else if( keyblock->pkt->pkttype == PKT_SIGNATURE && keyblock->pkt->pkt.signature->sig_class == 0x20 ) rc = import_revoke_cert( fname, keyblock, stats ); @@ -355,7 +366,8 @@ import_old_secring (ctrl_t ctrl, const char *fname) while (!(err = read_block (inp, &pending_pkt, &keyblock))) { if (keyblock->pkt->pkttype == PKT_SECRET_KEY) - err = import_secret_one (ctrl, fname, keyblock, stats, 1, 0, 1); + err = import_secret_one (ctrl, fname, keyblock, stats, 1, 0, 1, + NULL, NULL); release_kbnode (keyblock); if (err) break; @@ -835,8 +847,9 @@ check_prefs (ctrl_t ctrl, kbnode_t keyblock) static int import_one (ctrl_t ctrl, const char *fname, KBNODE keyblock, struct stats_s *stats, - unsigned char **fpr,size_t *fpr_len,unsigned int options, - int from_sk, int silent) + unsigned char **fpr, size_t *fpr_len, unsigned int options, + int from_sk, int silent, + import_screener_t screener, void *screener_arg) { PKT_public_key *pk; PKT_public_key *pk_orig; @@ -880,6 +893,13 @@ import_one (ctrl_t ctrl, return 0; } + if (screener && screener (keyblock, screener_arg)) + { + log_error (_("key %s: %s\n"), keystr_from_pk (pk), + _("rejected by import screener")); + return 0; + } + if (opt.interactive && !silent) { if(is_status_enabled()) print_import_check (pk, uidnode->pkt->pkt.user_id); @@ -1519,7 +1539,8 @@ sec_to_pub_keyblock (kbnode_t sec_keyblock) static int import_secret_one (ctrl_t ctrl, const char *fname, KBNODE keyblock, struct stats_s *stats, int batch, unsigned int options, - int for_migration) + int for_migration, + import_screener_t screener, void *screener_arg) { PKT_public_key *pk; struct seckey_info *ski; @@ -1540,6 +1561,13 @@ import_secret_one (ctrl_t ctrl, const char *fname, KBNODE keyblock, keyid_from_pk (pk, keyid); uidnode = find_next_kbnode (keyblock, PKT_USER_ID); + if (screener && screener (keyblock, screener_arg)) + { + log_error (_("secret key %s: %s\n"), keystr_from_pk (pk), + _("rejected by import screener")); + return 0; + } + if (opt.verbose && !for_migration) { log_info ("sec %s/%s %s ", @@ -1610,7 +1638,8 @@ import_secret_one (ctrl_t ctrl, const char *fname, KBNODE keyblock, public key block, and below we will output another one for the secret keys. FIXME? */ import_one (ctrl, fname, pub_keyblock, stats, - NULL, NULL, options, 1, for_migration); + NULL, NULL, options, 1, for_migration, + screener, screener_arg); /* Fixme: We should check for an invalid keyblock and cancel the secret key import in this case. */ diff --git a/g10/keyserver.c b/g10/keyserver.c index 4249caf0a..1b2e128ce 100644 --- a/g10/keyserver.c +++ b/g10/keyserver.c @@ -1042,6 +1042,85 @@ keyserver_export (ctrl_t ctrl, strlist_t users) } +/* Structure to convey the arg to keyserver_retrieval_screener. */ +struct ks_retrieval_screener_arg_s +{ + KEYDB_SEARCH_DESC *desc; + int ndesc; +}; + + +/* Check whether a key matches the search description. The function + returns 0 if the key shall be imported. */ +static gpg_error_t +keyserver_retrieval_screener (kbnode_t keyblock, void *opaque) +{ + struct ks_retrieval_screener_arg_s *arg = opaque; + KEYDB_SEARCH_DESC *desc = arg->desc; + int ndesc = arg->ndesc; + kbnode_t node; + PKT_public_key *pk; + int n; + u32 keyid[2]; + byte fpr[MAX_FINGERPRINT_LEN]; + size_t fpr_len = 0; + + /* Secret keys are not expected from a keyserver. We do not + care about secret subkeys because the import code takes care + of skipping them. Not allowing an import of a public key + with a secret subkey would make it too easy to inhibit the + downloading of a public key. Recall that keyservers do only + limited checks. */ + node = find_kbnode (keyblock, PKT_SECRET_KEY); + if (node) + return gpg_error (GPG_ERR_GENERAL); /* Do not import. */ + + if (!ndesc) + return 0; /* Okay if no description given. */ + + /* Loop over all key packets. */ + for (node = keyblock; node; node = node->next) + { + if (node->pkt->pkttype != PKT_PUBLIC_KEY + && node->pkt->pkttype != PKT_PUBLIC_SUBKEY) + continue; + + pk = node->pkt->pkt.public_key; + fingerprint_from_pk (pk, fpr, &fpr_len); + keyid_from_pk (pk, keyid); + + /* Compare requested and returned fingerprints if available. */ + for (n = 0; n < ndesc; n++) + { + if (desc[n].mode == KEYDB_SEARCH_MODE_FPR20) + { + if (fpr_len == 20 && !memcmp (fpr, desc[n].u.fpr, 20)) + return 0; + } + else if (desc[n].mode == KEYDB_SEARCH_MODE_FPR16) + { + if (fpr_len == 16 && !memcmp (fpr, desc[n].u.fpr, 16)) + return 0; + } + else if (desc[n].mode == KEYDB_SEARCH_MODE_LONG_KID) + { + if (keyid[0] == desc[n].u.kid[0] && keyid[1] == desc[n].u.kid[1]) + return 0; + } + else if (desc[n].mode == KEYDB_SEARCH_MODE_SHORT_KID) + { + if (keyid[1] == desc[n].u.kid[1]) + return 0; + } + else /* No keyid or fingerprint - can't check. */ + return 0; /* allow import. */ + } + } + + return gpg_error (GPG_ERR_GENERAL); +} + + int keyserver_import (ctrl_t ctrl, strlist_t users) { @@ -1601,6 +1680,7 @@ keyserver_get (ctrl_t ctrl, KEYDB_SEARCH_DESC *desc, int ndesc, if (!err) { void *stats_handle; + struct ks_retrieval_screener_arg_s screenerarg; stats_handle = import_new_stats_handle(); @@ -1616,10 +1696,14 @@ keyserver_get (ctrl_t ctrl, KEYDB_SEARCH_DESC *desc, int ndesc, never accept or send them but we better protect against rogue keyservers. */ + screenerarg.desc = desc; + screenerarg.ndesc = ndesc; import_keys_es_stream (ctrl, datastream, stats_handle, r_fpr, r_fprlen, (opt.keyserver_options.import_options - | IMPORT_NO_SECKEY)); + | IMPORT_NO_SECKEY), + keyserver_retrieval_screener, &screenerarg); + import_print_stats (stats_handle); import_release_stats_handle (stats_handle); } @@ -1684,7 +1768,7 @@ keyserver_put (ctrl_t ctrl, strlist_t keyspecs, } -/* Loop over all URLs in STRLIST and fetch the key that URL. Note +/* Loop over all URLs in STRLIST and fetch the key at that URL. Note that the fetch operation ignores the configured key servers and instead directly retrieves the keys. */ int @@ -1712,7 +1796,8 @@ keyserver_fetch (ctrl_t ctrl, strlist_t urilist) stats_handle = import_new_stats_handle(); import_keys_es_stream (ctrl, datastream, stats_handle, NULL, NULL, - opt.keyserver_options.import_options); + opt.keyserver_options.import_options, + NULL, NULL); import_print_stats (stats_handle); import_release_stats_handle (stats_handle); @@ -1762,7 +1847,8 @@ keyserver_import_cert (ctrl_t ctrl, err = import_keys_es_stream (ctrl, key, NULL, fpr, fpr_len, (opt.keyserver_options.import_options - | IMPORT_NO_SECKEY)); + | IMPORT_NO_SECKEY), + NULL, NULL); opt.no_armor=armor_status; diff --git a/g10/main.h b/g10/main.h index 8fe03ac43..b2efaae3b 100644 --- a/g10/main.h +++ b/g10/main.h @@ -288,6 +288,8 @@ gcry_mpi_t encode_md_value (PKT_public_key *pk, gcry_md_hd_t md, int hash_algo ); /*-- import.c --*/ +typedef gpg_error_t (*import_screener_t)(kbnode_t keyblock, void *arg); + int parse_import_options(char *str,unsigned int *options,int noisy); void import_keys (ctrl_t ctrl, char **fnames, int nnames, void *stats_hd, unsigned int options); @@ -296,7 +298,8 @@ int import_keys_stream (ctrl_t ctrl, iobuf_t inp, void *stats_hd, size_t *fpr_len, unsigned int options); int import_keys_es_stream (ctrl_t ctrl, estream_t fp, void *stats_handle, unsigned char **fpr, size_t *fpr_len, - unsigned int options); + unsigned int options, + import_screener_t screener, void *screener_arg); gpg_error_t import_old_secring (ctrl_t ctrl, const char *fname); void *import_new_stats_handle (void); void import_release_stats_handle (void *p);