File gnupg-CRL-fetching-via-https.patch of Package gpg2

Overview Repositories Revisions Requests Users Attributes Meta

File gnupg-CRL-fetching-via-https.patch of Package gpg2 (Revision 37b8c42e3edbd32245bae8060b89d24b)

Currently displaying revision 37b8c42e3edbd32245bae8060b89d24b , Show latest

From 705d8e9cf0d109005b3441766270c0e584f7847d Mon Sep 17 00:00:00 2001
From: Werner Koch <wk@gnupg.org>
Date: Wed, 25 Apr 2018 09:43:18 +0200
Subject: [PATCH] dirmngr: Implement CRL fetching via https.

* 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.

diff --git a/dirmngr/certcache.c b/dirmngr/certcache.c
index 56629fdda..adb005ec8 100644
--- a/dirmngr/certcache.c
+++ b/dirmngr/certcache.c
@@ -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;
diff --git a/dirmngr/crlfetch.c b/dirmngr/crlfetch.c
index 0892421e9..0d27aa0f1 100644
--- a/dirmngr/crlfetch.c
+++ b/dirmngr/crlfetch.c
@@ -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;
 }
 
diff --git a/dirmngr/http-ntbtls.c b/dirmngr/http-ntbtls.c
index ea66a4d73..ed4cdd496 100644
--- a/dirmngr/http-ntbtls.c
+++ b/dirmngr/http-ntbtls.c
@@ -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;
diff --git a/dirmngr/http.c b/dirmngr/http.c
index 8e778dfa2..4624d5fe6 100644
--- a/dirmngr/http.c
+++ b/dirmngr/http.c
@@ -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)
       {
diff --git a/dirmngr/http.h b/dirmngr/http.h
index 9fa462c05..4cfb4c890 100644
--- a/dirmngr/http.h
+++ b/dirmngr/http.h
@@ -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));
 
 
diff --git a/dirmngr/ks-action.c b/dirmngr/ks-action.c
index 38cd02feb..eb15e40dd 100644
--- a/dirmngr/ks-action.c
+++ b/dirmngr/ks-action.c
@@ -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);
diff --git a/dirmngr/ks-engine-http.c b/dirmngr/ks-engine-http.c
index 6492dda8a..a03580373 100644
--- a/dirmngr/ks-engine-http.c
+++ b/dirmngr/ks-engine-http.c
@@ -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 ();
diff --git a/dirmngr/ks-engine.h b/dirmngr/ks-engine.h
index b5b4dd08b..ce51141bd 100644
--- a/dirmngr/ks-engine.h
+++ b/dirmngr/ks-engine.h
@@ -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 --*/
diff --git a/dirmngr/loadswdb.c b/dirmngr/loadswdb.c
index bc004665d..dfa027386 100644
--- a/dirmngr/loadswdb.c
+++ b/dirmngr/loadswdb.c
@@ -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.
diff --git a/dirmngr/server.c b/dirmngr/server.c
index 48244d4af..8a0b940ce 100644
--- a/dirmngr/server.c
+++ b/dirmngr/server.c
@@ -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);
             }

Places

File gnupg-CRL-fetching-via-https.patch of Package gpg2 (Revision 37b8c42e3edbd32245bae8060b89d24b)

Places