gpg: Print better diagnostics for keyserver operations.
authorWerner Koch <wk@gnupg.org>
Wed, 18 Feb 2015 11:14:22 +0000 (12:14 +0100)
committerWerner Koch <wk@gnupg.org>
Wed, 18 Feb 2015 11:14:22 +0000 (12:14 +0100)
* 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@gnupg.org>
g10/armor.c
g10/filter.h
g10/import.c
g10/keyserver.c
g10/main.h

index 2065335..6c0013d 100644 (file)
@@ -381,6 +381,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
@@ -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 ) {
index 923cfda..6bcb037 100644 (file)
@@ -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 <keyid> FAILED <err>" 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 */
index 8e509dd..e1f43b2 100644 (file)
@@ -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;
 }
 
index af00401..dc49e1b 100644 (file)
@@ -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;
 
index 7cd6756..4ee1b73 100644 (file)
@@ -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);