Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP1:Update
gpg2.9849
gnupg-CVE-2018-1000858.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File gnupg-CVE-2018-1000858.patch of Package gpg2.9849
From 4a4bb874f63741026bd26264c43bb32b1099f060 Mon Sep 17 00:00:00 2001 From: Werner Koch <wk@gnupg.org> Date: Thu, 22 Nov 2018 22:27:56 +0100 Subject: [PATCH] dirmngr: Avoid possible CSRF attacks via http redirects. * dirmngr/http.h (parsed_uri_s): Add fields off_host and off_path. (http_redir_info_t): New. * dirmngr/http.c (do_parse_uri): Set new fields. (same_host_p): New. (http_prepare_redirect): New. * dirmngr/t-http-basic.c: New test. * dirmngr/ks-engine-hkp.c (send_request): Use http_prepare_redirect instead of the open code. * dirmngr/ks-engine-http.c (ks_http_fetch): Ditto. -- With this change a http query will not follow a redirect unless the Location header gives the same host. If the host is different only the host and port is taken from the Location header and the original path and query parts are kept. Signed-off-by: Werner Koch <wk@gnupg.org> (cherry picked from commit fa1b1eaa4241ff3f0634c8bdf8591cbc7c464144) --- dirmngr/Makefile.am | 11 ++- dirmngr/http.c | 171 +++++++++++++++++++++++++++++++++++++++- dirmngr/http.h | 21 +++++ dirmngr/ks-engine-hkp.c | 56 ++++--------- dirmngr/ks-engine-http.c | 68 +++++----------- dirmngr/t-http-basic.c | 199 +++++++++++++++++++++++++++++++++++++++++++++++ dirmngr/t-http.c | 4 +- 7 files changed, 435 insertions(+), 95 deletions(-) create mode 100644 dirmngr/t-http-basic.c diff --git a/dirmngr/Makefile.am b/dirmngr/Makefile.am index 43f59bd..081f723 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 6846107..d37faa8 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 4cfb4c8..a0458f8 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 7c2836c..3645789 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"); - - 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); + err = http_prepare_redirect (&redirinfo, http_get_status_code (http), + http_get_header (http, "Location"), + &request_buffer); + if (err) + 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 946c927..1abb350 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"); - - 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")); + xfree (request_buffer); + err = http_prepare_redirect (&redirinfo, http_get_status_code (http), + http_get_header (http, "Location"), + &request_buffer); + if (err) + goto leave; + + 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 0000000..edf82ef --- /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 <https://gnu.org/licenses/>. + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +#include <config.h> +#include <stdlib.h> + +#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 440633d..3cf08ad 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); -- 2.8.0.rc3
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
