[PATCH 10/13] gpg: Print better diagnostics for keyserver operations.
Daniel Kahn Gillmor
dkg at fifthhorseman.net
Sun Feb 22 05:10:34 CET 2015
From: Werner Koch <wk at gnupg.org>
* 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 <wk at gnupg.org>
[dkg: rebased to STABLE-BRANCH-1-4]
Signed-off-by: Daniel Kahn Gillmor <dkg at fifthhorseman.net>
---
g10/armor.c | 37 +++++++++++++++++++++++++++++++++
g10/filter.h | 2 ++
g10/import.c | 41 +++++++++++++++++++++++++------------
g10/keyserver.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++--------
g10/main.h | 4 +++-
5 files changed, 125 insertions(+), 22 deletions(-)
diff --git a/g10/armor.c b/g10/armor.c
index c50525c..4332d70 100644
--- a/g10/armor.c
+++ b/g10/armor.c
@@ -385,6 +385,32 @@ is_armor_header( byte *line, unsigned len )
}
+/* Helper to parse a "KEY <keyid> FAILED <code>" 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
@@ -505,6 +531,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 54a4152..f746695 100644
--- a/g10/filter.h
+++ b/g10/filter.h
@@ -40,6 +40,8 @@ typedef struct {
/* these fileds 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 <keyid> FAILED <err>" line. */
/* the following fields must be initialized to zero */
int inp_checked; /* set if the input has been checked */
diff --git a/g10/import.c b/g10/import.c
index c9df368..7050039 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,34 +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 (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, 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 = new_armor_context ();
+ 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) )) {
@@ -297,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 810713c..d1ddc99 100644
--- a/g10/keyserver.c
+++ b/g10/keyserver.c
@@ -1040,6 +1040,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 = "keyserver 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 = "key not found"; break;
+ case KEYSERVER_KEY_EXISTS: s = "key exists"; break;
+ case KEYSERVER_KEY_INCOMPLETE:s = "key incomplete (EOF)"; break;
+ case KEYSERVER_UNREACHABLE: s = "keyserver unreachable"; break;
+ case KEYSERVER_TIMEOUT: s = "keyserver timeout"; break;
+ default: s = "?"; break;
+ }
+ return s;
+}
+
+
static int
keyserver_spawn(enum ks_action action,STRLIST list,KEYDB_SEARCH_DESC *desc,
int count,int *prog,unsigned char **fpr,size_t *fpr_len,
@@ -1539,8 +1563,11 @@ keyserver_spawn(enum ks_action action,STRLIST list,KEYDB_SEARCH_DESC *desc,
plen--;
ptr[plen]='\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)
{
@@ -1561,6 +1588,14 @@ keyserver_spawn(enum ks_action action,STRLIST 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)
@@ -1577,6 +1612,7 @@ keyserver_spawn(enum ks_action action,STRLIST list,KEYDB_SEARCH_DESC *desc,
{
void *stats_handle;
struct ks_retrieval_filter_arg_s filterarg;
+ int gpgkeys_err;
stats_handle=import_new_stats_handle();
@@ -1591,14 +1627,21 @@ keyserver_spawn(enum ks_action action,STRLIST 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;
}
@@ -1619,7 +1662,6 @@ keyserver_spawn(enum ks_action action,STRLIST list,KEYDB_SEARCH_DESC *desc,
xfree(line);
xfree(searchstr);
-
*prog=exec_finish(spawn);
return ret;
@@ -1644,9 +1686,11 @@ keyserver_work(enum ks_action action,STRLIST 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)
@@ -1675,6 +1719,9 @@ keyserver_work(enum ks_action action,STRLIST list,KEYDB_SEARCH_DESC *desc,
log_error(_("keyserver timed out\n"));
break;
+ case KEYSERVER_UNREACHABLE:
+ return G10ERR_UNKNOWN_HOST;
+
case KEYSERVER_INTERNAL_ERROR:
default:
log_error(_("keyserver internal error\n"));
@@ -2127,7 +2174,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 05a4059..05b60bb 100644
--- a/g10/main.h
+++ b/g10/main.h
@@ -67,6 +67,7 @@ void print_digest_algo_note( int algo );
/*-- 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);
@@ -216,7 +217,8 @@ void import_keys( char **fnames, int nnames,
void *stats_hd, unsigned int options );
int import_keys_stream (IOBUF 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);
--
2.1.4
More information about the Gnupg-devel
mailing list