From 0c3d7645dfad9968d0128fb35a304881121ec61b Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 18 Feb 2015 12:14:22 +0100 Subject: [PATCH] gpg: Print better diagnostics for keyserver operations. * g10/armor.c (parse_key_failed_line): New. (check_input): Watch out for gpgkeys_ error lines. * g10/filter.h (armor_filter_context_t): Add field key_failed_code. * g10/import.c (import): Add arg r_gpgkeys_err. (import_keys_internal): Ditto. (import_keys_stream): Ditto. * g10/keyserver.c (keyserver_errstr): New. (keyserver_spawn): Detect "KEY " lines while sending. Get gpgkeys_err while receiving keys. (keyserver_work): Add kludge for better error messages. -- GnuPG-bug-id: 1832 Note that these changes can be backported to 1.4 but they don't make sense for 2.1 due to the removal of the keyserver helpers. The error reporting could be improved even more but given that this is an old GnuPG branch it is not justified to put too much effort into it. Signed-off-by: Werner Koch --- g10/armor.c | 37 +++++++++++++++++++++++++++++ g10/filter.h | 4 +++- g10/import.c | 36 ++++++++++++++++++---------- g10/keyserver.c | 63 ++++++++++++++++++++++++++++++++++++++++++------- g10/main.h | 4 +++- 5 files changed, 122 insertions(+), 22 deletions(-) diff --git a/g10/armor.c b/g10/armor.c index 20653356d..6c0013de9 100644 --- a/g10/armor.c +++ b/g10/armor.c @@ -381,6 +381,32 @@ is_armor_header( byte *line, unsigned len ) } +/* Helper to parse a "KEY FAILED " line and return the + error code. LINEPTR points right behind "KEY ". */ +int +parse_key_failed_line (const void *lineptr, unsigned int len) +{ + const byte *line = lineptr; + int code = 0; + + for (; len && !spacep (line); len--, line++) + ; + for (; len && spacep (line); len--, line++) + ; + if (len > 7 && !memcmp (line, "FAILED ", 7)) + { + line += 7; + len -= 7; + for (; len && digitp (line); len--, line++) + { + code *= 10; + code += atoi_1 (line); + } + } + + return code; +} + /**************** * Parse a header lines @@ -501,6 +527,17 @@ check_input( armor_filter_context_t *afx, IOBUF a ) /* find the armor header */ while(len) { i = is_armor_header( line, len ); + if (i == -1 && afx->only_keyblocks + && !afx->key_failed_code + && len > 4 && !memcmp (line, "KEY ", 4)) + { + /* This is probably input from a keyserver helper and we + have not yet seen an error line. */ + afx->key_failed_code = parse_key_failed_line (line+4, len-4); + log_debug ("armor-keys-failed (%.*s) ->%d\n", + (int)len, line, + afx->key_failed_code); + } if( i >= 0 && !(afx->only_keyblocks && i != 1 && i != 5 && i != 6 )) { hdr_line = i; if( hdr_line == BEGIN_SIGNED_MSG_IDX ) { diff --git a/g10/filter.h b/g10/filter.h index 923cfdadf..6bcb0372f 100644 --- a/g10/filter.h +++ b/g10/filter.h @@ -39,6 +39,8 @@ typedef struct { /* these fields must be initialized to zero */ int no_openpgp_data; /* output flag: "No valid OpenPGP data found" */ + int key_failed_code; /* Error code from the first gpgkkeys_* + "KEY FAILED " line. */ /* the following fields must be initialized to zero */ int inp_checked; /* set if the input has been checked */ @@ -121,7 +123,7 @@ typedef struct { unsigned long last; /* last amount reported */ unsigned long offset; /* current amount */ unsigned long total; /* total amount */ - int refcount; + int refcount; } progress_filter_context_t; /* encrypt_filter_context_t defined in main.h */ diff --git a/g10/import.c b/g10/import.c index 8e509ddf8..e1f43b230 100644 --- a/g10/import.c +++ b/g10/import.c @@ -60,7 +60,8 @@ struct stats_s { static int import( IOBUF inp, const char* fname,struct stats_s *stats, unsigned char **fpr,size_t *fpr_len,unsigned int options, - import_filter_t filter, void *filter_arg ); + import_filter_t filter, void *filter_arg, + int *r_gpgkeys_err); static int read_block( IOBUF a, PACKET **pending_pkt, KBNODE *ret_root ); static void revocation_present(KBNODE keyblock); static int import_one(const char *fname, KBNODE keyblock,struct stats_s *stats, @@ -177,7 +178,8 @@ static int import_keys_internal( IOBUF inp, char **fnames, int nnames, void *stats_handle, unsigned char **fpr, size_t *fpr_len, unsigned int options, - import_filter_t filter, void *filter_arg) + import_filter_t filter, void *filter_arg, + int *r_gpgkeys_err) { int i, rc = 0; struct stats_s *stats = stats_handle; @@ -187,7 +189,7 @@ import_keys_internal( IOBUF inp, char **fnames, int nnames, if (inp) { rc = import (inp, "[stream]", stats, fpr, fpr_len, options, - filter, filter_arg); + filter, filter_arg, r_gpgkeys_err); } else { int once = (!fnames && !nnames); @@ -208,7 +210,7 @@ import_keys_internal( IOBUF inp, char **fnames, int nnames, else { rc = import (inp2, fname, stats, fpr, fpr_len, options, - NULL, NULL); + NULL, NULL, r_gpgkeys_err); iobuf_close(inp2); /* Must invalidate that ugly cache to actually close it. */ iobuf_ioctl (NULL, 2, 0, (char*)fname); @@ -240,37 +242,42 @@ import_keys( char **fnames, int nnames, void *stats_handle, unsigned int options ) { import_keys_internal (NULL, fnames, nnames, stats_handle, NULL, NULL, - options, NULL, NULL); + options, NULL, NULL, NULL); } + +/* Import keys from an open stream. */ int import_keys_stream( IOBUF inp, void *stats_handle, unsigned char **fpr, size_t *fpr_len,unsigned int options, - import_filter_t filter, void *filter_arg) + import_filter_t filter, void *filter_arg, + int *r_gpgkeys_err) { return import_keys_internal (inp, NULL, 0, stats_handle, fpr, fpr_len, - options, filter, filter_arg); + options, filter, filter_arg, r_gpgkeys_err); } +/* Note: If R_GPGKEYS_ERR is not NULL an error code from the keyserver + helpers will be stored there. */ static int import (IOBUF inp, const char* fname,struct stats_s *stats, unsigned char **fpr, size_t *fpr_len, unsigned int options, - import_filter_t filter, void *filter_arg) + import_filter_t filter, void *filter_arg, int *r_gpgkeys_err) { PACKET *pending_pkt = NULL; KBNODE keyblock = NULL; int rc = 0; + int need_armor = (!opt.no_armor || r_gpgkeys_err); + armor_filter_context_t *afx = NULL; getkey_disable_caches(); - if( !opt.no_armor ) { /* armored reading is not disabled */ - armor_filter_context_t *afx; - + if (!opt.no_armor || r_gpgkeys_err) { + /* armored reading is not disabled or enforced. */ afx = new_armor_context (); afx->only_keyblocks = 1; push_armor_filter (afx, inp); - release_armor_context (afx); } while( !(rc = read_block( inp, &pending_pkt, &keyblock) )) { @@ -300,6 +307,11 @@ import (IOBUF inp, const char* fname,struct stats_s *stats, else if( rc && rc != G10ERR_INV_KEYRING ) log_error( _("error reading `%s': %s\n"), fname, g10_errstr(rc)); + if (afx && r_gpgkeys_err) + *r_gpgkeys_err = afx->key_failed_code; + + release_armor_context (afx); + return rc; } diff --git a/g10/keyserver.c b/g10/keyserver.c index af00401f9..dc49e1b14 100644 --- a/g10/keyserver.c +++ b/g10/keyserver.c @@ -1062,6 +1062,30 @@ keyserver_retrieval_filter (kbnode_t keyblock, void *opaque) } +static const char * +keyserver_errstr (int code) +{ + const char *s; + + switch (code) + { + case KEYSERVER_OK: s = "success"; break; + case KEYSERVER_INTERNAL_ERROR:s = "keyserver helper internal error"; break; + case KEYSERVER_NOT_SUPPORTED: s =gpg_strerror (GPG_ERR_NOT_SUPPORTED);break; + case KEYSERVER_VERSION_ERROR: s = "keyserver helper version mismatch";break; + case KEYSERVER_GENERAL_ERROR: s = "keyserver helper general error"; break; + case KEYSERVER_NO_MEMORY: s = "keyserver helper is out of core"; break; + case KEYSERVER_KEY_NOT_FOUND: s =gpg_strerror (GPG_ERR_NOT_FOUND); break; + case KEYSERVER_KEY_EXISTS: s = "key exists"; break; + case KEYSERVER_KEY_INCOMPLETE:s = "key incomplete (EOF)"; break; + case KEYSERVER_UNREACHABLE: s =gpg_strerror (GPG_ERR_UNKNOWN_HOST);break; + case KEYSERVER_TIMEOUT: s =gpg_strerror (GPG_ERR_TIMEOUT); break; + default: s = "?"; break; + } + return s; +} + + static int keyserver_spawn (enum ks_action action, strlist_t list, KEYDB_SEARCH_DESC *desc, int count, int *prog, unsigned char **fpr, size_t *fpr_len, @@ -1534,8 +1558,11 @@ keyserver_spawn (enum ks_action action, strlist_t list, KEYDB_SEARCH_DESC *desc, plen--; plen[ptr]='\0'; - if(*ptr=='\0') - break; + /* Stop at the first empty line but not if we are sending keys. + In the latter case we won't continue reading later and thus + we need to watch out for errors right in this loop. */ + if(*ptr=='\0' && action != KS_SEND) + break; if(ascii_strncasecmp(ptr,"VERSION ",8)==0) { @@ -1556,6 +1583,14 @@ keyserver_spawn (enum ks_action action, strlist_t list, KEYDB_SEARCH_DESC *desc, } else if(ascii_strncasecmp(ptr,"OPTION OUTOFBAND",16)==0) outofband=1; /* Currently the only OPTION */ + else if (action == KS_SEND + && ascii_strncasecmp(ptr,"KEY ",4)==0) + { + ret = parse_key_failed_line (ptr+4, strlen (ptr+4)); + break; /* We stop at the first KEY line so that we won't + run into an EOF which would return an unspecified + error message (due to iobuf_read_line). */ + } } if(!gotversion) @@ -1572,6 +1607,7 @@ keyserver_spawn (enum ks_action action, strlist_t list, KEYDB_SEARCH_DESC *desc, { void *stats_handle; struct ks_retrieval_filter_arg_s filterarg; + int gpgkeys_err; stats_handle=import_new_stats_handle(); @@ -1586,14 +1622,21 @@ keyserver_spawn (enum ks_action action, strlist_t list, KEYDB_SEARCH_DESC *desc, but we better protect against rogue keyservers. */ filterarg.desc = desc; filterarg.ndesc = count; + gpgkeys_err = 0; import_keys_stream (spawn->fromchild, stats_handle, fpr, fpr_len, (opt.keyserver_options.import_options | IMPORT_NO_SECKEY), - keyserver_retrieval_filter, &filterarg); + keyserver_retrieval_filter, &filterarg, + &gpgkeys_err); import_print_stats(stats_handle); import_release_stats_handle(stats_handle); - + if (gpgkeys_err) + { + log_error (_("keyserver communications error: %s\n"), + keyserver_errstr (gpgkeys_err)); + ret = gpgkeys_err; + } break; } @@ -1614,7 +1657,6 @@ keyserver_spawn (enum ks_action action, strlist_t list, KEYDB_SEARCH_DESC *desc, xfree(line); xfree(searchstr); - *prog=exec_finish(spawn); return ret; @@ -1641,9 +1683,11 @@ keyserver_work (enum ks_action action, strlist_t list, KEYDB_SEARCH_DESC *desc, return G10ERR_KEYSERVER; #else - /* Spawn a handler */ - + /* Spawn a handler. The use of RC and RET is a mess. We use a + kludge to return a suitable error message. */ rc=keyserver_spawn(action,list,desc,count,&ret,fpr,fpr_len,keyserver); + if (ret == KEYSERVER_INTERNAL_ERROR && rc) + ret = rc; if(ret) { switch(ret) @@ -1672,6 +1716,9 @@ keyserver_work (enum ks_action action, strlist_t list, KEYDB_SEARCH_DESC *desc, log_error(_("keyserver timed out\n")); break; + case KEYSERVER_UNREACHABLE: + return gpg_error (GPG_ERR_UNKNOWN_HOST); + case KEYSERVER_INTERNAL_ERROR: default: log_error(_("keyserver internal error\n")); @@ -2136,7 +2183,7 @@ keyserver_import_cert(const char *name,unsigned char **fpr,size_t *fpr_len) rc=import_keys_stream (key, NULL, fpr, fpr_len, (opt.keyserver_options.import_options - | IMPORT_NO_SECKEY), NULL, NULL); + | IMPORT_NO_SECKEY), NULL, NULL, NULL); opt.no_armor=armor_status; diff --git a/g10/main.h b/g10/main.h index 7cd6756f1..4ee1b735f 100644 --- a/g10/main.h +++ b/g10/main.h @@ -66,6 +66,7 @@ extern int g10_errors_seen; /*-- armor.c --*/ char *make_radix64_string( const byte *data, size_t len ); +int parse_key_failed_line (const void *lineptr, unsigned int len); /*-- misc.c --*/ void trap_unaligned(void); @@ -271,7 +272,8 @@ void import_keys( char **fnames, int nnames, void *stats_hd, unsigned int options ); int import_keys_stream (iobuf_t inp, void *stats_hd, unsigned char **fpr, size_t *fpr_len, unsigned int options, - import_filter_t filter, void *filter_arg); + import_filter_t filter, void *filter_arg, + int *r_gpgkeys_err); void *import_new_stats_handle (void); void import_release_stats_handle (void *p); void import_print_stats (void *hd);