dirmngr: Implement CRL fetching via https.
authorWerner Koch <wk@gnupg.org>
Wed, 25 Apr 2018 07:43:18 +0000 (09:43 +0200)
committerWerner Koch <wk@gnupg.org>
Wed, 25 Apr 2018 10:38:04 +0000 (12:38 +0200)
* dirmngr/http.h (HTTP_FLAG_TRUST_CFG): New flag.
* dirmngr/http.c (http_register_cfg_ca): New.
(http_session_new) [HTTP_USE_GNUTLS]: Implement new trust flag.
* dirmngr/certcache.c (load_certs_from_dir): Call new function.
(cert_cache_deinit): Ditto.
* dirmngr/http-ntbtls.c (gnupg_http_tls_verify_cb): Ditto.
* dirmngr/ks-engine-http.c (ks_http_fetch): Add new args
'send_no_cache' and 'extra_http_trust_flags'.  Change all callers to
provide the default value.
* dirmngr/crlfetch.c (crl_fetch): Rewrite to make use of
ks_http_fetch.
--

The old code simply did not use https for downloading of CRLS.
Instead it rewrote https to http under the assumption that the CRL
service was also available without encryption.  Note that a CRL is
self-standing and thus it does not need to have extra authenticity as
provided by TLS.  These days we should not use any unencrypted content
and thus this patch.

Be aware that cacert.org give a https CRL DP but that currently
redirects to to http!  This is a downgrade attack which we detect and
don't allow.  The outcome is that it is right now not possible to use
CAcert certificates.

Signed-off-by: Werner Koch <wk@gnupg.org>
dirmngr/certcache.c
dirmngr/crlfetch.c
dirmngr/http-ntbtls.c
dirmngr/http.c
dirmngr/http.h
dirmngr/ks-action.c
dirmngr/ks-engine-http.c
dirmngr/ks-engine.h
dirmngr/loadswdb.c
dirmngr/server.c

index 56629fd..adb005e 100644 (file)
@@ -423,6 +423,9 @@ load_certs_from_dir (const char *dirname, unsigned int trustclass)
         log_info (_("certificate '%s' already cached\n"), fname);
       else if (!err)
         {
+          if ((trustclass & CERTTRUST_CLASS_CONFIG))
+            http_register_cfg_ca (fname);
+
           if (trustclass)
             log_info (_("trusted certificate '%s' loaded\n"), fname);
           else
@@ -763,6 +766,8 @@ cert_cache_deinit (int full)
         }
     }
 
+  http_register_cfg_ca (NULL);
+
   total_nonperm_certificates = 0;
   any_cert_of_class = 0;
   initialization_done = 0;
index 0892421..0d27aa0 100644 (file)
@@ -28,6 +28,7 @@
 #include "dirmngr.h"
 #include "misc.h"
 #include "http.h"
+#include "ks-engine.h"  /* For ks_http_fetch.  */
 
 #if USE_LDAP
 # include "ldap-wrapper.h"
@@ -154,41 +155,17 @@ crl_fetch (ctrl_t ctrl, const char *url, ksba_reader_t *reader)
 {
   gpg_error_t err;
   parsed_uri_t uri;
-  char *free_this = NULL;
-  int redirects_left = 2; /* We allow for 2 redirect levels.  */
+  estream_t httpfp = NULL;
 
   *reader = NULL;
 
   if (!url)
     return gpg_error (GPG_ERR_INV_ARG);
 
- once_more:
   err = http_parse_uri (&uri, url, 0);
   http_release_parsed_uri (uri);
-  if (err && !strncmp (url, "https:", 6))
-    {
-      /* FIXME: We now support https.
-       * Our HTTP code does not support TLS, thus we can't use this
-       * scheme and it is frankly not useful for CRL retrieval anyway.
-       * We resort to using http, assuming that the server also
-       * provides plain http access.  */
-      free_this = xtrymalloc (strlen (url) + 1);
-      if (free_this)
-        {
-          strcpy (stpcpy (free_this,"http:"), url+6);
-          err = http_parse_uri (&uri, free_this, 0);
-          http_release_parsed_uri (uri);
-          if (!err)
-            {
-              log_info (_("using \"http\" instead of \"https\"\n"));
-              url = free_this;
-            }
-        }
-    }
   if (!err) /* Yes, our HTTP code groks that. */
     {
-      http_t hd;
-
       if (opt.disable_http)
         {
           log_error (_("CRL access not possible due to disabled %s\n"),
@@ -196,97 +173,54 @@ crl_fetch (ctrl_t ctrl, const char *url, ksba_reader_t *reader)
           err = gpg_error (GPG_ERR_NOT_SUPPORTED);
         }
       else
-        err = http_open_document (&hd, url, NULL,
-                                  ((opt.honor_http_proxy? HTTP_FLAG_TRY_PROXY:0)
-                                   |(DBG_LOOKUP? HTTP_FLAG_LOG_RESP:0)
-                                   |(dirmngr_use_tor()? HTTP_FLAG_FORCE_TOR:0)
-                                   |(opt.disable_ipv4? HTTP_FLAG_IGNORE_IPv4:0)
-                                   |(opt.disable_ipv6? HTTP_FLAG_IGNORE_IPv6:0)
-                                   ),
-                                  ctrl->http_proxy, NULL, NULL, NULL);
-
-      switch ( err? 99999 : http_get_status_code (hd) )
         {
-        case 200:
-          {
-            estream_t fp = http_get_read_ptr (hd);
-            struct reader_cb_context_s *cb_ctx;
-
-            cb_ctx = xtrycalloc (1, sizeof *cb_ctx);
-            if (!cb_ctx)
-              err = gpg_error_from_syserror ();
-            if (!err)
-              err = ksba_reader_new (reader);
-            if (!err)
-              {
-                cb_ctx->fp = fp;
-                err = ksba_reader_set_cb (*reader, &my_es_read, cb_ctx);
-              }
-            if (err)
-              {
-                log_error (_("error initializing reader object: %s\n"),
-                           gpg_strerror (err));
-                ksba_reader_release (*reader);
-                *reader = NULL;
-                http_close (hd, 0);
-              }
-            else
-              {
-                /* The ksba reader misses a user pointer thus we need
-                   to come up with our own way of associating a file
-                   pointer (or well the callback context) with the
-                   reader.  It is only required when closing the
-                   reader thus there is no performance issue doing it
-                   this way.  FIXME: We now have a close notification
-                   which might be used here. */
-                register_file_reader (*reader, cb_ctx);
-                http_close (hd, 1);
-              }
-          }
-          break;
+          /* Note that we also allow root certificates loaded from
+           * "/etc/gnupg/trusted-certs/".  We also do not consult
+           * the CRL for the TLS connection - that may lwad to a
+           * loop.  */
+          err = ks_http_fetch (ctrl, url, 0,
+                               (HTTP_FLAG_TRUST_CFG | HTTP_FLAG_NO_CRL),
+                               &httpfp);
+        }
 
-        case 301: /* Redirection (perm.). */
-        case 302: /* Redirection (temp.). */
-          {
-            const char *s = http_get_header (hd, "Location");
-
-            log_info (_("URL '%s' redirected to '%s' (%u)\n"),
-                      url, s?s:"[none]", http_get_status_code (hd));
-            if (s && *s && redirects_left-- )
-              {
-                xfree (free_this); url = NULL;
-                free_this = xtrystrdup (s);
-                if (!free_this)
-                  err = gpg_error_from_errno (errno);
-                else
-                  {
-                    url = free_this;
-                    http_close (hd, 0);
-                    /* Note, that our implementation of redirection
-                       actually handles a redirect to LDAP.  */
-                    goto once_more;
-                  }
-              }
-            else
-              err = gpg_error (GPG_ERR_NO_DATA);
-            log_error (_("too many redirections\n")); /* Or no "Location". */
-            http_close (hd, 0);
-          }
-          break;
-
-        case 99999: /* Made up status code for error reporting.  */
-          log_error (_("error retrieving '%s': %s\n"),
-                     url, gpg_strerror (err));
-          break;
-
-        default:
-          log_error (_("error retrieving '%s': http status %u\n"),
-                     url, http_get_status_code (hd));
-          err = gpg_error (GPG_ERR_NO_DATA);
-          http_close (hd, 0);
+      if (err)
+        log_error (_("error retrieving '%s': %s\n"), url, gpg_strerror (err));
+      else
+        {
+          struct reader_cb_context_s *cb_ctx;
+
+          cb_ctx = xtrycalloc (1, sizeof *cb_ctx);
+          if (!cb_ctx)
+            err = gpg_error_from_syserror ();
+          else if (!(err = ksba_reader_new (reader)))
+            {
+              cb_ctx->fp = httpfp;
+              err = ksba_reader_set_cb (*reader, &my_es_read, cb_ctx);
+              if (!err)
+                {
+                  /* The ksba reader misses a user pointer thus we
+                   * need to come up with our own way of associating a
+                   * file pointer (or well the callback context) with
+                   * the reader.  It is only required when closing the
+                   * reader thus there is no performance issue doing
+                   * it this way.  FIXME: We now have a close
+                   * notification which might be used here. */
+                  register_file_reader (*reader, cb_ctx);
+                  httpfp = NULL;
+                }
+            }
+
+          if (err)
+            {
+              log_error (_("error initializing reader object: %s\n"),
+                         gpg_strerror (err));
+              ksba_reader_release (*reader);
+              *reader = NULL;
+              xfree (cb_ctx);
+            }
         }
     }
-  else /* Let the LDAP code try other schemes. */
+  else /* Let the LDAP code parse other schemes.  */
     {
       if (opt.disable_ldap)
         {
@@ -310,7 +244,7 @@ crl_fetch (ctrl_t ctrl, const char *url, ksba_reader_t *reader)
         }
     }
 
-  xfree (free_this);
+  es_fclose (httpfp);
   return err;
 }
 
index ea66a4d..ed4cdd4 100644 (file)
@@ -87,13 +87,15 @@ gnupg_http_tls_verify_cb (void *opaque,
     }
   else /* Use the certificates as requested from the HTTP module.  */
     {
+      if ((http_flags & HTTP_FLAG_TRUST_CFG))
+        validate_flags |= VALIDATE_FLAG_TRUST_CONFIG;
       if ((http_flags & HTTP_FLAG_TRUST_DEF))
         validate_flags |= VALIDATE_FLAG_TRUST_HKP;
       if ((http_flags & HTTP_FLAG_TRUST_SYS))
         validate_flags |= VALIDATE_FLAG_TRUST_SYSTEM;
 
       /* If HKP trust is requested and there are no HKP certificates
-       * configured, also try thye standard system certificates.  */
+       * configured, also try the standard system certificates.  */
       if ((validate_flags & VALIDATE_FLAG_TRUST_HKP)
           && !cert_cache_any_in_class (CERTTRUST_CLASS_HKP))
         validate_flags |= VALIDATE_FLAG_TRUST_SYSTEM;
index 8e778df..4624d5f 100644 (file)
@@ -318,6 +318,9 @@ static gpg_error_t (*tls_callback) (http_t, http_session_t, int);
 /* The list of files with trusted CA certificates.  */
 static strlist_t tls_ca_certlist;
 
+/* The list of files with extra trusted CA certificates.  */
+static strlist_t cfg_ca_certlist;
+
 /* The global callback for net activity.  */
 static void (*netactivity_cb)(void);
 
@@ -596,6 +599,35 @@ http_register_tls_ca (const char *fname)
 }
 
 
+/* Register a CA certificate for future use.  The certificate is
+ * expected to be in FNAME.  PEM format is assume if FNAME has a
+ * suffix of ".pem".  If FNAME is NULL the list of CA files is
+ * removed.  This is a variant of http_register_tls_ca which puts the
+ * certificate into a separate list enabled using HTTP_FLAG_TRUST_CFG.  */
+void
+http_register_cfg_ca (const char *fname)
+{
+  strlist_t sl;
+
+  if (!fname)
+    {
+      free_strlist (cfg_ca_certlist);
+      cfg_ca_certlist = NULL;
+    }
+  else
+    {
+      /* Warn if we can't access right now, but register it anyway in
+         case it becomes accessible later */
+      if (access (fname, F_OK))
+        log_info (_("can't access '%s': %s\n"), fname,
+                  gpg_strerror (gpg_error_from_syserror()));
+      sl = add_to_strlist (&cfg_ca_certlist, fname);
+      if (*sl->d && !strcmp (sl->d + strlen (sl->d) - 4, ".pem"))
+        sl->flags = 1;
+    }
+}
+
+
 /* Register a callback which is called every time the HTTP mode has
  * made a successful connection to some server.  */
 void
@@ -680,6 +712,7 @@ http_session_release (http_session_t sess)
  * Valid values for FLAGS are:
  *   HTTP_FLAG_TRUST_DEF - Use the CAs set with http_register_tls_ca
  *   HTTP_FLAG_TRUST_SYS - Also use the CAs defined by the system
+ *   HTTP_FLAG_TRUST_CFG - Also use CAs set with http_register_cfg_ca
  *   HTTP_FLAG_NO_CRL    - Do not consult CRLs for https.
  */
 gpg_error_t
@@ -793,6 +826,21 @@ http_session_new (http_session_t *r_session,
 #endif /* gnutls >= 3.0.20 */
       }
 
+    /* Add other configured certificates to the session.  */
+    if ((flags & HTTP_FLAG_TRUST_CFG))
+      {
+        for (sl = cfg_ca_certlist; sl; sl = sl->next)
+          {
+            rc = gnutls_certificate_set_x509_trust_file
+              (sess->certcred, sl->d,
+               (sl->flags & 1)? GNUTLS_X509_FMT_PEM : GNUTLS_X509_FMT_DER);
+            if (rc < 0)
+              log_info ("setting extra CA from file '%s' failed: %s\n",
+                        sl->d, gnutls_strerror (rc));
+          }
+      }
+
+
     rc = gnutls_init (&sess->tls_session, GNUTLS_CLIENT);
     if (rc < 0)
       {
index 9fa462c..4cfb4c8 100644 (file)
@@ -88,8 +88,9 @@ enum
     HTTP_FLAG_IGNORE_IPv4 = 64,  /* Do not use IPv4.  */
     HTTP_FLAG_IGNORE_IPv6 = 128, /* Do not use IPv6.  */
     HTTP_FLAG_TRUST_DEF   = 256, /* Use the CAs configured for HKP.  */
-    HTTP_FLAG_TRUST_SYS   = 512, /* Also use the system defined CAs.  */
-    HTTP_FLAG_NO_CRL     = 1024  /* Do not consult CRLs for https.  */
+    HTTP_FLAG_TRUST_SYS   = 512, /* Also use the system defined CAs. */
+    HTTP_FLAG_TRUST_CFG  = 1024, /* Also use configured CAs.         */
+    HTTP_FLAG_NO_CRL     = 2048  /* Do not consult CRLs for https.   */
   };
 
 
@@ -110,6 +111,7 @@ void http_set_verbose (int verbose, int debug);
 
 void http_register_tls_callback (gpg_error_t (*cb)(http_t,http_session_t,int));
 void http_register_tls_ca (const char *fname);
+void http_register_cfg_ca (const char *fname);
 void http_register_netactivity_cb (void (*cb)(void));
 
 
index 38cd02f..eb15e40 100644 (file)
@@ -257,7 +257,8 @@ ks_action_get (ctrl_t ctrl, uri_item_t keyservers,
               if (is_hkp_s)
                 err = ks_hkp_get (ctrl, uri->parsed_uri, sl->d, &infp);
               else if (is_http_s)
-                err = ks_http_fetch (ctrl, uri->parsed_uri->original, &infp);
+                err = ks_http_fetch (ctrl, uri->parsed_uri->original, 1, 0,
+                                     &infp);
               else
                 BUG ();
 
@@ -314,7 +315,7 @@ ks_action_fetch (ctrl_t ctrl, const char *url, estream_t outfp)
 
   if (parsed_uri->is_http)
     {
-      err = ks_http_fetch (ctrl, url, &infp);
+      err = ks_http_fetch (ctrl, url, 1, 0, &infp);
       if (!err)
         {
           err = copy_stream (infp, outfp);
index 6492dda..a035803 100644 (file)
@@ -62,9 +62,13 @@ ks_http_help (ctrl_t ctrl, parsed_uri_t uri)
 
 
 /* Get the key from URL which is expected to specify a http style
-   scheme.  On success R_FP has an open stream to read the data.  */
+ * scheme.  On success R_FP has an open stream to read the data.
+ * Despite its name this function is also used to retrieve arbitrary
+ * data via https or http.
+ */
 gpg_error_t
-ks_http_fetch (ctrl_t ctrl, const char *url, estream_t *r_fp)
+ks_http_fetch (ctrl_t ctrl, const char *url, int send_no_cache,
+               unsigned int extra_http_trust_flags, estream_t *r_fp)
 {
   gpg_error_t err;
   http_session_t session = NULL;
@@ -82,11 +86,13 @@ ks_http_fetch (ctrl_t ctrl, const char *url, estream_t *r_fp)
   is_https = uri->use_tls;
 
  once_more:
-  /* Note that we only use the system provided certificates with the
-   * fetch command.  */
+  /* By default we only use the system provided certificates with this
+   * fetch command.  However, EXTRA_HTTP_FLAGS can be used to add more
+   * flags. */
   err = http_session_new (&session, NULL,
                           ((ctrl->http_no_crl? HTTP_FLAG_NO_CRL : 0)
-                           | HTTP_FLAG_TRUST_SYS),
+                           | HTTP_FLAG_TRUST_SYS
+                           | extra_http_trust_flags),
                           gnupg_http_tls_verify_cb, ctrl);
   if (err)
     goto leave;
@@ -100,6 +106,7 @@ ks_http_fetch (ctrl_t ctrl, const char *url, estream_t *r_fp)
                    /* httphost */ NULL,
                    /* fixme: AUTH */ NULL,
                    ((opt.honor_http_proxy? HTTP_FLAG_TRY_PROXY:0)
+                    | (DBG_LOOKUP? HTTP_FLAG_LOG_RESP:0)
                     | (dirmngr_use_tor ()? HTTP_FLAG_FORCE_TOR:0)
                     | (opt.disable_ipv4? HTTP_FLAG_IGNORE_IPv4 : 0)
                     | (opt.disable_ipv6? HTTP_FLAG_IGNORE_IPv6 : 0)),
@@ -111,10 +118,11 @@ ks_http_fetch (ctrl_t ctrl, const char *url, estream_t *r_fp)
     {
       fp = http_get_write_ptr (http);
       /* Avoid caches to get the most recent copy of the key.  We set
-         both the Pragma and Cache-Control versions of the header, so
-         we're good with both HTTP 1.0 and 1.1.  */
-      es_fputs ("Pragma: no-cache\r\n"
-                "Cache-Control: no-cache\r\n", fp);
+       * both the Pragma and Cache-Control versions of the header, so
+       * we're good with both HTTP 1.0 and 1.1.  */
+      if (send_no_cache)
+        es_fputs ("Pragma: no-cache\r\n"
+                  "Cache-Control: no-cache\r\n", fp);
       http_start_data (http);
       if (es_ferror (fp))
         err = gpg_error_from_syserror ();
index b5b4dd0..ce51141 100644 (file)
@@ -42,7 +42,9 @@ gpg_error_t ks_hkp_put (ctrl_t ctrl, parsed_uri_t uri,
 
 /*-- ks-engine-http.c --*/
 gpg_error_t ks_http_help (ctrl_t ctrl, parsed_uri_t uri);
-gpg_error_t ks_http_fetch (ctrl_t ctrl, const char *url, estream_t *r_fp);
+gpg_error_t ks_http_fetch (ctrl_t ctrl, const char *url, int send_no_cache,
+                           unsigned int extra_http_trust_flags,
+                           estream_t *r_fp);
 
 
 /*-- ks-engine-finger.c --*/
index bc00466..dfa0273 100644 (file)
@@ -126,7 +126,7 @@ fetch_file (ctrl_t ctrl, const char *url, estream_t *r_fp)
   size_t nread, nwritten;
   char buffer[1024];
 
-  if ((err = ks_http_fetch (ctrl, url, &httpfp)))
+  if ((err = ks_http_fetch (ctrl, url, 1, 0, &httpfp)))
     goto leave;
 
   /* We now read the data from the web server into a memory buffer.
index 48244d4..8a0b940 100644 (file)
@@ -1877,7 +1877,7 @@ static const char hlp_validate[] =
   "  INQUIRE CERTLIST\n"
   "\n"
   "Here the first certificate is the target certificate, the remaining\n"
-  "certificates are suggested intermediary certificates.  All certifciates\n"
+  "certificates are suggested intermediary certificates.  All certificates\n"
   "need to be PEM encoded.\n"
   "\n"
   "The option --systrust changes the behaviour to include the system\n"
@@ -1928,7 +1928,7 @@ cmd_validate (assuan_context_t ctx, char *line)
             err = gpg_error (GPG_ERR_MISSING_CERT);
           if (!err)
             {
-              /* Extraxt the first certificate from the list.  */
+              /* Extract the first certificate from the list.  */
               cert = certlist->cert;
               ksba_cert_ref (cert);
             }