diff --git a/dirmngr/Makefile.am b/dirmngr/Makefile.am index 43f59bd45..081f7239f 100644 --- a/dirmngr/Makefile.am +++ b/dirmngr/Makefile.am @@ -120,7 +120,7 @@ t_common_ldadd = $(libcommon) $(LIBASSUAN_LIBS) $(LIBGCRYPT_LIBS) \ $(NTBTLS_LIBS) $(LIBGNUTLS_LIBS) \ $(DNSLIBS) $(LIBINTL) $(LIBICONV) -module_tests = +module_tests = t-http-basic if USE_LDAP module_tests += t-ldap-parse-uri @@ -151,6 +151,15 @@ t_http_CFLAGS = -DWITHOUT_NPTH=1 $(USE_C99_CFLAGS) \ t_http_LDADD = $(t_common_ldadd) \ $(NTBTLS_LIBS) $(KSBA_LIBS) $(LIBGNUTLS_LIBS) $(DNSLIBS) +t_http_basic_SOURCES = $(t_common_src) t-http-basic.c http.c \ + dns-stuff.c http-common.c +t_http_basic_CFLAGS = -DWITHOUT_NPTH=1 $(USE_C99_CFLAGS) \ + $(LIBGCRYPT_CFLAGS) $(NTBTLS_CFLAGS) $(LIBGNUTLS_CFLAGS) \ + $(LIBASSUAN_CFLAGS) $(GPG_ERROR_CFLAGS) $(KSBA_CFLAGS) +t_http_basic_LDADD = $(t_common_ldadd) \ + $(NTBTLS_LIBS) $(KSBA_LIBS) $(LIBGNUTLS_LIBS) $(DNSLIBS) + + t_ldap_parse_uri_SOURCES = \ t-ldap-parse-uri.c ldap-parse-uri.c ldap-parse-uri.h \ http.c http-common.c dns-stuff.c \ diff --git a/dirmngr/http.c b/dirmngr/http.c index 6846107d0..d37faa87f 100644 --- a/dirmngr/http.c +++ b/dirmngr/http.c @@ -1347,6 +1347,8 @@ do_parse_uri (parsed_uri_t uri, int only_local_part, uri->v6lit = 0; uri->onion = 0; uri->explicit_port = 0; + uri->off_host = 0; + uri->off_path = 0; /* A quick validity check. */ if (strspn (p, VALID_URI_CHARS) != n) @@ -1390,7 +1392,19 @@ do_parse_uri (parsed_uri_t uri, int only_local_part, { p += 2; if ((p2 = strchr (p, '/'))) - *p2++ = 0; + { + if (p2 - uri->buffer > 10000) + return GPG_ERR_BAD_URI; + uri->off_path = p2 - uri->buffer; + *p2++ = 0; + } + else + { + n = (p - uri->buffer) + strlen (p); + if (n > 10000) + return GPG_ERR_BAD_URI; + uri->off_path = n; + } /* Check for username/password encoding */ if ((p3 = strchr (p, '@'))) @@ -1409,11 +1423,19 @@ do_parse_uri (parsed_uri_t uri, int only_local_part, *p3++ = '\0'; /* worst case, uri->host should have length 0, points to \0 */ uri->host = p + 1; + if (p - uri->buffer > 10000) + return GPG_ERR_BAD_URI; + uri->off_host = (p + 1) - uri->buffer; uri->v6lit = 1; p = p3; } else - uri->host = p; + { + uri->host = p; + if (p - uri->buffer > 10000) + return GPG_ERR_BAD_URI; + uri->off_host = p - uri->buffer; + } if ((p3 = strchr (p, ':'))) { @@ -3490,3 +3512,148 @@ uri_query_lookup (parsed_uri_t uri, const char *key) return NULL; } + + +/* Return true if both URI point to the same host. */ +static int +same_host_p (parsed_uri_t a, parsed_uri_t b) +{ + return a->host && b->host && !ascii_strcasecmp (a->host, b->host); +} + + +/* Prepare a new URL for a HTTP redirect. INFO has flags controlling + * the operaion, STATUS_CODE is used for diagnostics, LOCATION is the + * value of the "Location" header, and R_URL reveives the new URL on + * success or NULL or error. Note that INFO->ORIG_URL is + * required. */ +gpg_error_t +http_prepare_redirect (http_redir_info_t *info, unsigned int status_code, + const char *location, char **r_url) +{ + gpg_error_t err; + parsed_uri_t locuri; + parsed_uri_t origuri; + char *newurl; + char *p; + + *r_url = NULL; + + if (!info || !info->orig_url) + return gpg_error (GPG_ERR_INV_ARG); + + if (!info->silent) + log_info (_("URL '%s' redirected to '%s' (%u)\n"), + info->orig_url, location? location:"[none]", status_code); + + if (!info->redirects_left) + { + if (!info->silent) + log_error (_("too many redirections\n")); + return gpg_error (GPG_ERR_NO_DATA); + } + info->redirects_left--; + + if (!location || !*location) + return gpg_error (GPG_ERR_NO_DATA); + + err = http_parse_uri (&locuri, location, 0); + if (err) + return err; + + /* Make sure that an onion address only redirects to another + * onion address, or that a https address only redirects to a + * https address. */ + if (info->orig_onion && !locuri->onion) + { + http_release_parsed_uri (locuri); + return gpg_error (GPG_ERR_FORBIDDEN); + } + if (!info->allow_downgrade && info->orig_https && !locuri->use_tls) + { + http_release_parsed_uri (locuri); + return gpg_error (GPG_ERR_FORBIDDEN); + } + + if (info->trust_location) + { + /* We trust the Location - return it verbatim. */ + http_release_parsed_uri (locuri); + newurl = xtrystrdup (location); + if (!newurl) + { + err = gpg_error_from_syserror (); + http_release_parsed_uri (locuri); + return err; + } + } + else if ((err = http_parse_uri (&origuri, info->orig_url, 0))) + { + http_release_parsed_uri (locuri); + return err; + } + else if (same_host_p (origuri, locuri)) + { + /* The host is the same and thus we can take the location + * verbatim. */ + http_release_parsed_uri (origuri); + http_release_parsed_uri (locuri); + newurl = xtrystrdup (location); + if (!newurl) + { + err = gpg_error_from_syserror (); + http_release_parsed_uri (locuri); + return err; + } + } + else + { + /* We take only the host and port from the URL given in the + * Location. This limits the effects of redirection attacks by + * rogue hosts returning an URL to servers in the client's own + * network. We don't even include the userinfo because they + * should be considered similar to the path and query parts. + */ + if (!(locuri->off_path - locuri->off_host)) + { + http_release_parsed_uri (origuri); + http_release_parsed_uri (locuri); + return gpg_error (GPG_ERR_BAD_URI); + } + if (!(origuri->off_path - origuri->off_host)) + { + http_release_parsed_uri (origuri); + http_release_parsed_uri (locuri); + return gpg_error (GPG_ERR_BAD_URI); + } + + newurl = xtrymalloc (strlen (origuri->original) + + (locuri->off_path - locuri->off_host) + 1); + if (!newurl) + { + err = gpg_error_from_syserror (); + http_release_parsed_uri (origuri); + http_release_parsed_uri (locuri); + return err; + } + /* Build new URL from + * uriguri: scheme userinfo ---- ---- path rest + * locuri: ------ -------- host port ---- ---- + */ + p = newurl; + memcpy (p, origuri->original, origuri->off_host); + p += origuri->off_host; + memcpy (p, locuri->original + locuri->off_host, + (locuri->off_path - locuri->off_host)); + p += locuri->off_path - locuri->off_host; + strcpy (p, origuri->original + origuri->off_path); + + http_release_parsed_uri (origuri); + http_release_parsed_uri (locuri); + if (!info->silent) + log_info (_("redirection changed to '%s'\n"), newurl); + } + + *r_url = newurl; + return 0; +} diff --git a/dirmngr/http.h b/dirmngr/http.h index 4cfb4c890..a0458f87c 100644 --- a/dirmngr/http.h +++ b/dirmngr/http.h @@ -58,6 +58,8 @@ struct parsed_uri_s char *auth; /* username/password for basic auth. */ char *host; /* Host (converted to lowercase). */ unsigned short port; /* Port (always set if the host is set). */ + unsigned short off_host; /* Offset to the HOST respective PATH parts */ + unsigned short off_path; /* in the original URI buffer. */ char *path; /* Path. */ uri_tuple_t params; /* ";xxxxx" */ uri_tuple_t query; /* "?xxx=yyy" */ @@ -100,6 +102,21 @@ typedef struct http_session_s *http_session_t; struct http_context_s; typedef struct http_context_s *http_t; +/* An object used to track redirection infos. */ +struct http_redir_info_s +{ + unsigned int redirects_left; /* Number of still possible redirects. */ + const char *orig_url; /* The original requested URL. */ + unsigned int orig_onion:1; /* Original request was an onion address. */ + unsigned int orig_https:1; /* Original request was a http address. */ + unsigned int silent:1; /* No diagnostics. */ + unsigned int allow_downgrade:1;/* Allow a downgrade from https to http. */ + unsigned int trust_location:1; /* Trust the received Location header. */ +}; +typedef struct http_redir_info_s http_redir_info_t; + + + /* A TLS verify callback function. */ typedef gpg_error_t (*http_verify_cb_t) (void *opaque, http_t http, @@ -176,5 +193,9 @@ gpg_error_t http_verify_server_credentials (http_session_t sess); char *http_escape_string (const char *string, const char *specials); char *http_escape_data (const void *data, size_t datalen, const char *specials); +gpg_error_t http_prepare_redirect (http_redir_info_t *info, + unsigned int status_code, + const char *location, char **r_url); + #endif /*GNUPG_COMMON_HTTP_H*/ diff --git a/dirmngr/ks-engine-hkp.c b/dirmngr/ks-engine-hkp.c index 7c2836c8b..3645789bd 100644 --- a/dirmngr/ks-engine-hkp.c +++ b/dirmngr/ks-engine-hkp.c @@ -1159,18 +1159,21 @@ send_request (ctrl_t ctrl, const char *request, const char *hostportstr, gpg_error_t err; http_session_t session = NULL; http_t http = NULL; - int redirects_left = MAX_REDIRECTS; + http_redir_info_t redirinfo = { MAX_REDIRECTS }; estream_t fp = NULL; char *request_buffer = NULL; parsed_uri_t uri = NULL; - int is_onion; *r_fp = NULL; err = http_parse_uri (&uri, request, 0); if (err) goto leave; - is_onion = uri->onion; + redirinfo.orig_url = request; + redirinfo.orig_onion = uri->onion; + redirinfo.allow_downgrade = 1; + /* FIXME: I am not sure whey we allow a downgrade for hkp requests. + * Needs at least an explanation here.. */ err = http_session_new (&session, httphost, ((ctrl->http_no_crl? HTTP_FLAG_NO_CRL : 0) @@ -1251,45 +1254,18 @@ send_request (ctrl_t ctrl, const char *request, const char *hostportstr, case 302: case 307: { - const char *s = http_get_header (http, "Location"); + xfree (request_buffer); + err = http_prepare_redirect (&redirinfo, http_get_status_code (http), + http_get_header (http, "Location"), + &request_buffer); + if (err) + goto leave; - log_info (_("URL '%s' redirected to '%s' (%u)\n"), - request, s?s:"[none]", http_get_status_code (http)); - if (s && *s && redirects_left-- ) - { - if (is_onion) - { - /* Make sure that an onion address only redirects to - * another onion address. */ - http_release_parsed_uri (uri); - uri = NULL; - err = http_parse_uri (&uri, s, 0); - if (err) - goto leave; - - if (! uri->onion) - { - err = gpg_error (GPG_ERR_FORBIDDEN); - goto leave; - } - } - - xfree (request_buffer); - request_buffer = xtrystrdup (s); - if (request_buffer) - { - request = request_buffer; - http_close (http, 0); - http = NULL; - goto once_more; - } - err = gpg_error_from_syserror (); - } - else - err = gpg_error (GPG_ERR_NO_DATA); - log_error (_("too many redirections\n")); + request = request_buffer; + http_close (http, 0); + http = NULL; } - goto leave; + goto once_more; case 501: err = gpg_error (GPG_ERR_NOT_IMPLEMENTED); diff --git a/dirmngr/ks-engine-http.c b/dirmngr/ks-engine-http.c index 946c92769..1abb3501d 100644 --- a/dirmngr/ks-engine-http.c +++ b/dirmngr/ks-engine-http.c @@ -74,17 +74,18 @@ ks_http_fetch (ctrl_t ctrl, const char *url, unsigned int flags, http_session_t session = NULL; unsigned int session_flags; http_t http = NULL; - int redirects_left = MAX_REDIRECTS; + http_redir_info_t redirinfo = { MAX_REDIRECTS }; estream_t fp = NULL; char *request_buffer = NULL; parsed_uri_t uri = NULL; - int is_onion, is_https; err = http_parse_uri (&uri, url, 0); if (err) goto leave; - is_onion = uri->onion; - is_https = uri->use_tls; + redirinfo.orig_url = url; + redirinfo.orig_onion = uri->onion; + redirinfo.orig_https = uri->use_tls; + redirinfo.allow_downgrade = !!(flags & KS_HTTP_FETCH_ALLOW_DOWNGRADE); /* By default we only use the system provided certificates with this * fetch command. */ @@ -158,53 +159,20 @@ ks_http_fetch (ctrl_t ctrl, const char *url, unsigned int flags, case 302: case 307: { - const char *s = http_get_header (http, "Location"); + xfree (request_buffer); + err = http_prepare_redirect (&redirinfo, http_get_status_code (http), + http_get_header (http, "Location"), + &request_buffer); + if (err) + goto leave; - log_info (_("URL '%s' redirected to '%s' (%u)\n"), - url, s?s:"[none]", http_get_status_code (http)); - if (s && *s && redirects_left-- ) - { - if (is_onion || is_https) - { - /* Make sure that an onion address only redirects to - * another onion address, or that a https address - * only redirects to a https address. */ - http_release_parsed_uri (uri); - uri = NULL; - err = http_parse_uri (&uri, s, 0); - if (err) - goto leave; - - 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; - } - } - - xfree (request_buffer); - request_buffer = xtrystrdup (s); - if (request_buffer) - { - url = request_buffer; - http_close (http, 0); - http = NULL; - http_session_release (session); - goto once_more; - } - err = gpg_error_from_syserror (); - } - else - err = gpg_error (GPG_ERR_NO_DATA); - log_error (_("too many redirections\n")); + url = request_buffer; + http_close (http, 0); + http = NULL; + http_session_release (session); + session = NULL; } - goto leave; + goto once_more; default: log_error (_("error accessing '%s': http status %u\n"), diff --git a/dirmngr/t-http-basic.c b/dirmngr/t-http-basic.c new file mode 100644 index 000000000..edf82efb9 --- /dev/null +++ b/dirmngr/t-http-basic.c @@ -0,0 +1,199 @@ +/* t-http-basic.c - Basic regression tests for http.c + * Copyright (C) 2018 g10 Code GmbH + * + * This file is part of GnuPG. + * + * GnuPG is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * GnuPG is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see . + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +#include +#include + +#include "../common/util.h" +#include "t-support.h" +#include "http.h" + +#define PGM "t-http-basic" + + +static void +test_http_prepare_redirect (void) +{ + static struct { + const char *url; + const char *location; + const char *expect_url; + gpg_error_t expect_err; + } tests[] = { + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + NULL, + "", + GPG_ERR_NO_DATA + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "", + "", + GPG_ERR_NO_DATA + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "foo//bla", + "", + GPG_ERR_BAD_URI + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + 0 + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + 0 + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://foo.gnupg.org:8080/.not-so-well-known/openpgpkey/hu/12345678", + "http://foo.gnupg.org:8080/.well-known/openpgpkey/hu/12345678", + 0 + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http:///.no-so-well-known/openpgpkey/hu/12345678", + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + GPG_ERR_BAD_URI + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://gnupg.org:8080/.not-so-well-known/openpgpkey/hu/12345678", + "http://gnupg.org:8080/.not-so-well-known/openpgpkey/hu/12345678", + 0 + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://gnupg.org:8/.not-so-well-known/openpgpkey/hu/12345678", + "http://gnupg.org:8/.not-so-well-known/openpgpkey/hu/12345678", + 0 + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://gnupg.org:/.no-so-well-known/openpgpkey/hu/12345678", + "http://gnupg.org:/.no-so-well-known/openpgpkey/hu/12345678", + 0 + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://gnupg.org/", + "http://gnupg.org/", + 0 + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://gnupg.net", + "http://gnupg.net/.well-known/openpgpkey/hu/12345678", + 0 + }, + { + "http://gnupg.org", + "http://gnupg.org", + "http://gnupg.org", + 0 + }, + { + "http://gnupg.org", + "http://foo.gnupg.org", + "http://foo.gnupg.org", + 0 + }, + { + "http://gnupg.org/", + "http://foo.gnupg.org", + "http://foo.gnupg.org/", + 0 + }, + { + "http://gnupg.org", + "http://foo.gnupg.org/", + "http://foo.gnupg.org", + 0 + }, + { + "http://gnupg.org/.well-known/openpgpkey/hu/12345678", + "http://gnupg.org/something-else", + "http://gnupg.org/something-else", + 0 + }, + }; + int tidx; + http_redir_info_t ri; + gpg_error_t err; + char *newurl; + + err = http_prepare_redirect (NULL, 301, tests[0].location, &newurl); + if (gpg_err_code (err) != GPG_ERR_INV_ARG) + fail (0); + memset (&ri, 0, sizeof ri); + err = http_prepare_redirect (&ri, 301, tests[0].location, &newurl); + if (gpg_err_code (err) != GPG_ERR_INV_ARG) + fail (0); + memset (&ri, 0, sizeof ri); + ri.silent = 1; + ri.orig_url = "http://example.org"; + err = http_prepare_redirect (&ri, 301, tests[0].location, &newurl); + if (gpg_err_code (err) != GPG_ERR_NO_DATA) + fail (0); + + for (tidx = 0; tidx < DIM (tests); tidx++) + { + memset (&ri, 0, sizeof ri); + ri.silent = 1; + ri.redirects_left = 1; + ri.orig_url = tests[tidx].url; + + err = http_prepare_redirect (&ri, 301, tests[tidx].location, &newurl); + if (err && newurl) + fail (tidx); + if (err && gpg_err_code (err) != tests[tidx].expect_err) + fail (tidx); + if (err) + continue; + if (!newurl) + fail (tidx); + if (strcmp (tests[tidx].expect_url, newurl)) + { + fprintf (stderr, "want: '%s'\n", tests[tidx].expect_url); + fprintf (stderr, "got : '%s'\n", newurl); + fail (tidx); + } + + xfree (newurl); + } +} + + +int +main (int argc, char **argv) +{ + (void)argc; + (void)argv; + + test_http_prepare_redirect (); + + return 0; +} diff --git a/dirmngr/t-http.c b/dirmngr/t-http.c index 440633db4..3cf08ada2 100644 --- a/dirmngr/t-http.c +++ b/dirmngr/t-http.c @@ -394,9 +394,9 @@ main (int argc, char **argv) else { printf ("Auth : %s\n", uri->auth? uri->auth:"[none]"); - printf ("Host : %s\n", uri->host); + printf ("Host : %s (off=%hu)\n", uri->host, uri->off_host); printf ("Port : %u\n", uri->port); - printf ("Path : %s\n", uri->path); + printf ("Path : %s (off=%hu)\n", uri->path, uri->off_path); for (r = uri->params; r; r = r->next) { printf ("Params: %s", r->name);