git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Ilya Tretyakov <it@it3xl.ru>, Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH v2 2/3] credential: optionally allow partial URLs in credential_from_url_gently()
Date: Thu, 23 Apr 2020 23:43:16 +0000	[thread overview]
Message-ID: <fc772d21b7472afc6ad862ff30ec4be91f0e8d80.1587685397.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.615.v2.git.1587685397.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Prior to the fixes for CVE-2020-11008, we were _very_ lenient in what we
required from a URL in order to parse it into a `struct credential`.
That led to serious vulnerabilities.

There was one call site, though, that really needed that leniency: when
parsing config settings a la `credential.dev.azure.com.useHTTPPath`.
Settings like this might be desired when users want to use, say, a given
user name on a given host, regardless of the protocol to be used.

In preparation for fixing that bug, let's add a parameter called
`allow_partial_url` to the `credential_from_url_gently()` function and
convert the existing callers to set that parameter to 0, i.e. do not
change the existing behavior, just add the option to be more lenient.

Please note that this patch does more than just reinstating a way to
imitate the behavior before those CVE-2020-11008 fixes: Before that, we
would simply ignore URLs without a protocol. In other words,
misleadingly, the following setting would be applied to _all_ URLs:

	[credential "example.com"]
		username = that-me

The obvious intention is to match the host name only. With this patch,
we allow precisely that: when parsing the URL with non-zero
`allow_partial_url`, we do not simply return success if there was no
protocol, but we simply leave the protocol unset and continue parsing
the URL.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 credential.c | 14 ++++++++------
 credential.h | 26 +++++++++++++++++++++++++-
 fsck.c       |  2 +-
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/credential.c b/credential.c
index 64a841eddca..52965a5122c 100644
--- a/credential.c
+++ b/credential.c
@@ -344,7 +344,7 @@ static int check_url_component(const char *url, int quiet,
 }
 
 int credential_from_url_gently(struct credential *c, const char *url,
-			       int quiet)
+			       int allow_partial_url, int quiet)
 {
 	const char *at, *colon, *cp, *slash, *host, *proto_end;
 
@@ -357,12 +357,12 @@ int credential_from_url_gently(struct credential *c, const char *url,
 	 *   (3) proto://<user>:<pass>@<host>/...
 	 */
 	proto_end = strstr(url, "://");
-	if (!proto_end || proto_end == url) {
+	if (!allow_partial_url && (!proto_end || proto_end == url)) {
 		if (!quiet)
 			warning(_("url has no scheme: %s"), url);
 		return -1;
 	}
-	cp = proto_end + 3;
+	cp = proto_end ? proto_end + 3 : url;
 	at = strchr(cp, '@');
 	colon = strchr(cp, ':');
 	slash = strchrnul(cp, '/');
@@ -382,8 +382,10 @@ int credential_from_url_gently(struct credential *c, const char *url,
 		host = at + 1;
 	}
 
-	c->protocol = xmemdupz(url, proto_end - url);
-	c->host = url_decode_mem(host, slash - host);
+	if (proto_end && proto_end - url > 0)
+		c->protocol = xmemdupz(url, proto_end - url);
+	if (!allow_partial_url || slash - host > 0)
+		c->host = url_decode_mem(host, slash - host);
 	/* Trim leading and trailing slashes from path */
 	while (*slash == '/')
 		slash++;
@@ -407,6 +409,6 @@ int credential_from_url_gently(struct credential *c, const char *url,
 
 void credential_from_url(struct credential *c, const char *url)
 {
-	if (credential_from_url_gently(c, url, 0) < 0)
+	if (credential_from_url_gently(c, url, 0, 0) < 0)
 		die(_("credential url cannot be parsed: %s"), url);
 }
diff --git a/credential.h b/credential.h
index 5a86502d95c..5abc865b95d 100644
--- a/credential.h
+++ b/credential.h
@@ -41,9 +41,33 @@ void credential_write(const struct credential *, FILE *);
  * an error but leave the broken state in the credential object for further
  * examination.  The non-gentle form will issue a warning to stderr and return
  * an empty credential.
+ *
+ * If allow_partial_url is non-zero, partial URLs are allowed, i.e. it can, but
+ * does not have to, contain
+ *
+ * - a protocol (or scheme) of the form "<protocol>://"
+ *
+ * - a host name (the part after the protocol and before the first slash after
+ *   that, if any)
+ *
+ * - a user name and potentially a password (as "<user>[:<password>]@" part of
+ *   the host name)
+ *
+ * - a path (the part after the host name, if any, starting with the slash)
+ *
+ * Missing parts will be left unset in `struct credential`. Thus, `https://`
+ * will have only the `protocol` set, `example.com` only the host name, and
+ * `/git` only the path.
+ *
+ * Note that an empty host name in an otherwise fully-qualified URL will be
+ * treated as unset when allow_partial_url is non-zero (and only then,
+ * otherwise it is treated as the empty string).
+ *
+ * The credential_from_url() function does not allow partial URLs.
  */
 void credential_from_url(struct credential *, const char *url);
-int credential_from_url_gently(struct credential *, const char *url, int quiet);
+int credential_from_url_gently(struct credential *, const char *url,
+			       int allow_partial_url, int quiet);
 
 int credential_match(const struct credential *have,
 		     const struct credential *want);
diff --git a/fsck.c b/fsck.c
index 31b5be05f54..cec97b63901 100644
--- a/fsck.c
+++ b/fsck.c
@@ -1076,7 +1076,7 @@ static int check_submodule_url(const char *url)
 	else if (url_to_curl_url(url, &curl_url)) {
 		struct credential c = CREDENTIAL_INIT;
 		int ret = 0;
-		if (credential_from_url_gently(&c, curl_url, 1) ||
+		if (credential_from_url_gently(&c, curl_url, 0, 1) ||
 		    !*c.host)
 			ret = -1;
 		credential_clear(&c);
-- 
gitgitgadget


  parent reply	other threads:[~2020-04-23 23:43 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-22 20:51 [PATCH 0/3] credential: handle partial URLs in config settings again Johannes Schindelin via GitGitGadget
2020-04-22 20:51 ` [PATCH 1/3] credential: fix grammar Johannes Schindelin via GitGitGadget
2020-04-22 23:29   ` Jonathan Nieder
2020-04-22 20:51 ` [PATCH 2/3] credential: teach `credential_from_url()` a non-strict mode Johannes Schindelin via GitGitGadget
2020-04-22 22:29   ` Junio C Hamano
2020-04-22 22:50     ` Johannes Schindelin
2020-04-22 23:02       ` Junio C Hamano
2020-04-22 23:38   ` Jonathan Nieder
2020-04-23  0:02     ` Carlo Arenas
2020-04-23 13:28       ` Johannes Schindelin
2020-04-23 21:22         ` Carlo Marcelo Arenas Belón
2020-04-23 22:03           ` Johannes Schindelin
2020-04-23 22:11             ` Junio C Hamano
2020-04-23 22:16               ` Junio C Hamano
2020-04-23 23:22                 ` Johannes Schindelin
2020-04-23 22:50     ` Johannes Schindelin
2020-04-22 20:51 ` [PATCH 3/3] credential: handle `credential.<partial-URL>.<key>` again Johannes Schindelin via GitGitGadget
2020-04-22 23:57   ` Jonathan Nieder
2020-04-23 23:19     ` Johannes Schindelin
2020-04-22 22:13 ` [PATCH 0/3] credential: handle partial URLs in config settings again Jeff King
2020-04-22 22:26   ` Johannes Schindelin
2020-04-22 22:47     ` Jonathan Nieder
2020-04-23 22:11       ` Johannes Schindelin
2020-04-23 23:43 ` [PATCH v2 " Johannes Schindelin via GitGitGadget
2020-04-23 23:43   ` [PATCH v2 1/3] credential: fix grammar Johannes Schindelin via GitGitGadget
2020-04-23 23:43   ` Johannes Schindelin via GitGitGadget [this message]
2020-04-23 23:43   ` [PATCH v2 3/3] credential: handle `credential.<partial-URL>.<key>` again Johannes Schindelin via GitGitGadget
2020-04-24  0:05     ` Carlo Marcelo Arenas Belón
2020-04-24  0:16       ` Johannes Schindelin
2020-04-24  0:39         ` Carlo Marcelo Arenas Belón
2020-04-24 11:34           ` Johannes Schindelin
2020-04-24  0:34       ` Junio C Hamano
2020-04-24  0:40         ` Junio C Hamano
2020-04-24 11:36           ` Johannes Schindelin
2020-04-24  0:49   ` [PATCH v2 0/3] credential: handle partial URLs in config settings again Carlo Marcelo Arenas Belón
2020-04-24 11:49   ` [PATCH v3 " Johannes Schindelin via GitGitGadget
2020-04-24 11:49     ` [PATCH v3 1/3] credential: fix grammar Johannes Schindelin via GitGitGadget
2020-04-24 11:49     ` [PATCH v3 2/3] credential: optionally allow partial URLs in credential_from_url_gently() Johannes Schindelin via GitGitGadget
2020-04-24 11:49     ` [PATCH v3 3/3] credential: handle `credential.<partial-URL>.<key>` again Johannes Schindelin via GitGitGadget
2020-04-24 15:23       ` Carlo Marcelo Arenas Belón
2020-04-25 10:43         ` Johannes Schindelin
2020-04-24 22:20       ` Junio C Hamano
2020-04-24 22:29         ` Johannes Schindelin
2020-04-28 23:13           ` Junio C Hamano
2020-04-29 14:58             ` Johannes Schindelin
2020-04-29 15:36               ` Junio C Hamano
2020-05-01 19:58                 ` Johannes Schindelin
2020-05-01 20:01                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fc772d21b7472afc6ad862ff30ec4be91f0e8d80.1587685397.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=it@it3xl.ru \
    --cc=johannes.schindelin@gmx.de \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).