From 705d8e9cf0d109005b3441766270c0e584f7847d Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 25 Apr 2018 09:43:18 +0200 Subject: 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. Signed-off-by: Werner Koch --- dirmngr/crlfetch.c | 160 ++++++++++++++++------------------------------------- 1 file changed, 47 insertions(+), 113 deletions(-) (limited to 'dirmngr/crlfetch.c') 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; } -- cgit From 1de4462974113ac18cf98f903e97cd1127fa842f Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 25 Apr 2018 12:37:34 +0200 Subject: dirmngr: Allow redirection from https to http for CRLs * dirmngr/ks-engine.h (KS_HTTP_FETCH_NOCACHE): New flag. (KS_HTTP_FETCH_TRUST_CFG): Ditto. (KS_HTTP_FETCH_NO_CRL): Ditto. (KS_HTTP_FETCH_ALLOW_DOWNGRADE): Ditto. * dirmngr/ks-engine-http.c (ks_http_fetch): Replace args send_no_cache and extra_http_trust_flags by a new flags arg. Allow redirectiong from https to http it KS_HTTP_FETCH_ALLOW_DOWNGRADE is set. * dirmngr/loadswdb.c (fetch_file): Call with KS_HTTP_FETCH_NOCACHE. * dirmngr/ks-action.c (ks_action_get): Ditto. (ks_action_fetch): Ditto. * dirmngr/crlfetch.c (crl_fetch): Call with the appropriate flags. -- Signed-off-by: Werner Koch --- dirmngr/crlfetch.c | 13 ++++++++----- dirmngr/ks-action.c | 5 +++-- dirmngr/ks-engine-http.c | 31 ++++++++++++++++++++----------- dirmngr/ks-engine.h | 10 ++++++++-- dirmngr/loadswdb.c | 2 +- 5 files changed, 40 insertions(+), 21 deletions(-) (limited to 'dirmngr/crlfetch.c') diff --git a/dirmngr/crlfetch.c b/dirmngr/crlfetch.c index 0d27aa0f1..57ac51b93 100644 --- a/dirmngr/crlfetch.c +++ b/dirmngr/crlfetch.c @@ -175,11 +175,14 @@ crl_fetch (ctrl_t ctrl, const char *url, ksba_reader_t *reader) else { /* 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), + * "/etc/gnupg/trusted-certs/". We also do not consult the + * CRL for the TLS connection - that may lead to a loop. + * Due to cacert.org redirecting their https URL to http we + * also allow such a downgrade. */ + err = ks_http_fetch (ctrl, url, + (KS_HTTP_FETCH_TRUST_CFG + | KS_HTTP_FETCH_NO_CRL + | KS_HTTP_FETCH_ALLOW_DOWNGRADE ), &httpfp); } diff --git a/dirmngr/ks-action.c b/dirmngr/ks-action.c index eb15e40dd..c1ecafb58 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, 1, 0, + err = ks_http_fetch (ctrl, uri->parsed_uri->original, + KS_HTTP_FETCH_NOCACHE, &infp); else BUG (); @@ -315,7 +316,7 @@ ks_action_fetch (ctrl_t ctrl, const char *url, estream_t outfp) if (parsed_uri->is_http) { - err = ks_http_fetch (ctrl, url, 1, 0, &infp); + err = ks_http_fetch (ctrl, url, KS_HTTP_FETCH_NOCACHE, &infp); if (!err) { err = copy_stream (infp, outfp); diff --git a/dirmngr/ks-engine-http.c b/dirmngr/ks-engine-http.c index a03580373..946c92769 100644 --- a/dirmngr/ks-engine-http.c +++ b/dirmngr/ks-engine-http.c @@ -67,11 +67,12 @@ ks_http_help (ctrl_t ctrl, parsed_uri_t uri) * data via https or http. */ 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_http_fetch (ctrl_t ctrl, const char *url, unsigned int flags, + estream_t *r_fp) { gpg_error_t err; http_session_t session = NULL; + unsigned int session_flags; http_t http = NULL; int redirects_left = MAX_REDIRECTS; estream_t fp = NULL; @@ -85,14 +86,16 @@ ks_http_fetch (ctrl_t ctrl, const char *url, int send_no_cache, is_onion = uri->onion; is_https = uri->use_tls; - once_more: /* 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 - | extra_http_trust_flags), + * fetch command. */ + session_flags = HTTP_FLAG_TRUST_SYS; + if ((flags & KS_HTTP_FETCH_NO_CRL) || ctrl->http_no_crl) + session_flags |= HTTP_FLAG_NO_CRL; + if ((flags & KS_HTTP_FETCH_TRUST_CFG)) + session_flags |= HTTP_FLAG_TRUST_CFG; + + once_more: + err = http_session_new (&session, NULL, session_flags, gnupg_http_tls_verify_cb, ctrl); if (err) goto leave; @@ -120,7 +123,7 @@ ks_http_fetch (ctrl_t ctrl, const char *url, int send_no_cache, /* 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. */ - if (send_no_cache) + if ((flags & KS_HTTP_FETCH_NOCACHE)) es_fputs ("Pragma: no-cache\r\n" "Cache-Control: no-cache\r\n", fp); http_start_data (http); @@ -172,7 +175,13 @@ ks_http_fetch (ctrl_t ctrl, const char *url, int send_no_cache, if (err) goto leave; - if ((is_onion && ! uri->onion) || (is_https && ! uri->use_tls)) + if (is_onion && !uri->onion) + { + err = gpg_error (GPG_ERR_FORBIDDEN); + goto leave; + } + if (!(flags & KS_HTTP_FETCH_ALLOW_DOWNGRADE) + && is_https && !uri->use_tls) { err = gpg_error (GPG_ERR_FORBIDDEN); goto leave; diff --git a/dirmngr/ks-engine.h b/dirmngr/ks-engine.h index ce51141bd..d28c6ab71 100644 --- a/dirmngr/ks-engine.h +++ b/dirmngr/ks-engine.h @@ -41,9 +41,15 @@ gpg_error_t ks_hkp_put (ctrl_t ctrl, parsed_uri_t uri, const void *data, size_t datalen); /*-- ks-engine-http.c --*/ + +/* Flags for the ks_http_fetch. */ +#define KS_HTTP_FETCH_NOCACHE 1 /* Request no caching. */ +#define KS_HTTP_FETCH_TRUST_CFG 2 /* Requests HTTP_FLAG_TRUST_CFG. */ +#define KS_HTTP_FETCH_NO_CRL 4 /* Requests HTTP_FLAG_NO_CRL. */ +#define KS_HTTP_FETCH_ALLOW_DOWNGRADE 8 /* Allow redirect https -> http. */ + 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, int send_no_cache, - unsigned int extra_http_trust_flags, +gpg_error_t ks_http_fetch (ctrl_t ctrl, const char *url, unsigned int flags, estream_t *r_fp); diff --git a/dirmngr/loadswdb.c b/dirmngr/loadswdb.c index dfa027386..fb883722a 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, 1, 0, &httpfp))) + if ((err = ks_http_fetch (ctrl, url, KS_HTTP_FETCH_NOCACHE, &httpfp))) goto leave; /* We now read the data from the web server into a memory buffer. -- cgit