dirmngr: Fix CRL DP error fallback to other schemes.

* dirmngr/crlcache.c (crl_cache_reload_crl): Rework the double loop.
Remove the unused issuername_uri stuff.
--

It is quite common that LDAP servers are blocked and thuis the HTTP
access point should be used instead.  This worked well for
certificates where the DP are given in this form:

        crlDP: ldap://x500.bund.de/[...]
               http://x500.bund.de/[...]
               issuer: none

but it failed for this form

        crlDP: ldap://x500.bund.de/[...]
               issuer: none
        crlDP: http://x500.bund.de/[...]
               issuer: none

because the LAST_ERR thing terminated the outer loop.  This pacth
fixes this and also cleans up the code to be more robust.

Note that the common workaround of using --ignore-ldap-dp will now
only be needed if the firewall uses packet dropping instead of proper
ICMP rejects.

1 files changed, 12 insertions, 28 deletions

diff --git a/dirmngr/crlcache.c b/dirmngr/crlcache.c
index fe96b11b2..45e0e6c0c 100644
--- a/dirmngr/crlcache.c
+++ b/dirmngr/crlcache.c
@@ -2662,21 +2662,21 @@ crl_cache_reload_crl (ctrl_t ctrl, ksba_cert_t cert)
   ksba_name_t distpoint = NULL;
   ksba_name_t issuername = NULL;
   char *distpoint_uri = NULL;
-  char *issuername_uri = NULL;
   int any_dist_point = 0;
   int seq;
+  gpg_error_t last_err = 0;
 
   /* Loop over all distribution points, get the CRLs and put them into
      the cache. */
   if (opt.verbose)
     log_info ("checking distribution points\n");
   seq = 0;
-  while ( !(err = ksba_cert_get_crl_dist_point (cert, seq++,
+  while (xfree (distpoint), xfree (issuername),
+         !(err = ksba_cert_get_crl_dist_point (cert, seq++,
                                                 &distpoint,
                                                 &issuername, NULL )))
     {
       int name_seq;
-      gpg_error_t last_err = 0;
 
       if (!distpoint && !issuername)
         {
@@ -2687,13 +2687,11 @@ crl_cache_reload_crl (ctrl_t ctrl, ksba_cert_t cert)
                     suitable CRL. */
         }
 
-      xfree (issuername_uri); issuername_uri = NULL;
-
       /* Get the URIs.  We do this in a loop to iterate over all names
          in the crlDP. */
       for (name_seq=0; ksba_name_enum (distpoint, name_seq); name_seq++)
         {
-          xfree (distpoint_uri); distpoint_uri = NULL;
+          xfree (distpoint_uri);
           distpoint_uri = ksba_name_get_uri (distpoint, name_seq);
           if (!distpoint_uri)
             continue;
@@ -2717,6 +2715,7 @@ crl_cache_reload_crl (ctrl_t ctrl, ksba_cert_t cert)
 
           if (opt.verbose)
             log_info ("fetching CRL from '%s'\n", distpoint_uri);
+          crl_close_reader (reader);
           err = crl_fetch (ctrl, distpoint_uri, &reader);
           if (err)
             {
@@ -2736,28 +2735,16 @@ crl_cache_reload_crl (ctrl_t ctrl, ksba_cert_t cert)
               last_err = err;
               continue; /* with the next name. */
             }
-          last_err = 0;
-          break; /* Ready. */
-        }
-      if (last_err)
-        {
-          err = last_err;
-          goto leave;
+          goto leave; /* Ready - we got the CRL. */
         }
-
-      ksba_name_release (distpoint); distpoint = NULL;
-
-      /* We don't do anything with issuername_uri yet but we keep the
-         code for documentation. */
-      issuername_uri =  ksba_name_get_uri (issuername, 0);
-      ksba_name_release (issuername); issuername = NULL;
-
-      /* Close the reader.  */
-      crl_close_reader (reader);
-      reader = NULL;
     }
   if (gpg_err_code (err) == GPG_ERR_EOF)
     err = 0;
+  if (!err && last_err)
+    {
+      err = last_err;
+      goto leave;
+    }
 
   /* If we did not found any distpoint, try something reasonable. */
   if (!any_dist_point )
@@ -2765,9 +2752,6 @@ crl_cache_reload_crl (ctrl_t ctrl, ksba_cert_t cert)
       if (opt.verbose)
         log_info ("no distribution point - trying issuer name\n");
 
-      crl_close_reader (reader);
-      reader = NULL;
-
       issuer = ksba_cert_get_issuer (cert, 0);
       if (!issuer)
         {
@@ -2778,6 +2762,7 @@ crl_cache_reload_crl (ctrl_t ctrl, ksba_cert_t cert)
 
       if (opt.verbose)
         log_info ("fetching CRL from default location\n");
+      crl_close_reader (reader);
       err = crl_fetch_default (ctrl, issuer, &reader);
       if (err)
           {
@@ -2800,7 +2785,6 @@ crl_cache_reload_crl (ctrl_t ctrl, ksba_cert_t cert)
  leave:
   crl_close_reader (reader);
   xfree (distpoint_uri);
-  xfree (issuername_uri);
   ksba_name_release (distpoint);
   ksba_name_release (issuername);
   ksba_free (issuer);