security-and-privacy/gnupg
security-and-privacy/gnupg
1
0
Fork 0
mirror of git://git.gnupg.org/gnupg.git synced 2024-12-22 10:19:57 +01:00
Code Activity

dirmngr: Avoid possible CSRF attacks via http redirects.

Browse Source 
* 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>
This commit is contained in:
Werner Koch 2018-11-22 22:27:56 +01:00
parent e5c3a6999a
commit fa1b1eaa42
No known key found for this signature in database
GPG Key ID: E3FDFF218E45B72B
7 changed files with 434 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
dirmngr/Makefile.am
View File

@ -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 \

171
dirmngr/http.c
View File

@ -1350,6 +1350,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)
@ -1393,7 +1395,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, '@')))
@ -1412,11 +1426,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, ':')))
{
@ -3496,3 +3518,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;
}

21
dirmngr/http.h
View File

@ -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*/

56
dirmngr/ks-engine-hkp.c
View File

@ -1201,18 +1201,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)
@ -1293,45 +1296,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);

66
dirmngr/ks-engine-http.c
View File

@ -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"),

199
dirmngr/t-http-basic.c Normal file
View File

@ -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;
}

4
dirmngr/t-http.c
View File

@ -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);