Skip to content

Commit

Permalink
urlmatch: create fetch.credentialsInUrl config
Browse files Browse the repository at this point in the history
Users sometimes provide a "username:password" combination in their
plaintext URLs. Since Git stores these URLs in plaintext in the
.git/config file, this is a very insecure way of storing these
credentials. Credential managers are a more secure way of storing this
information.

System administrators might want to prevent this kind of use by users on
their machines.

Create a new "fetch.credentialsInUrl" config option and teach Git to
warn or die when seeing a URL with this kind of information. The warning
anonymizes the sensitive information of the URL to be clear about the
issue.

This change currently defaults the behavior to "ignore" which does
nothing with these URLs. We can consider changing this behavior to
"warn" by default if we wish. At that time, we may want to add some
advice about setting fetch.credentialsInUrl=ignore for users who still
want to follow this pattern (and not receive the warning).

As an attempt to ensure the parsing logic did not catch any
unintentional cases, I modified this change locally to to use the "die"
option by default. Running the test suite succeeds except for the
explicit username:password URLs used in t5550-http-fetch-dumb.s and
t5541-http-push-smart.sh. This means that all other tested URLs did not
trigger this logic.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
  • Loading branch information
derrickstolee committed May 23, 2022
1 parent f9b9594 commit 5145dc2
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 0 deletions.
13 changes: 13 additions & 0 deletions Documentation/config/fetch.txt
Expand Up @@ -96,3 +96,16 @@ fetch.writeCommitGraph::
merge and the write may take longer. Having an updated commit-graph
file helps performance of many Git commands, including `git merge-base`,
`git push -f`, and `git log --graph`. Defaults to false.

fetch.credentialsInUrl::
A URL can contain plaintext credentials in the form
`protocol://<user>:<password>@domain/path`. Using such URLs is not
recommended as it exposes the password in multiple ways. The
`fetch.credentialsInUrl` option provides instruction for how Git
should react to seeing such a URL, with these values:
+
* `ignore` (default): Git will proceed with its activity without warning.
* `warn`: Git will write a warning message to `stderr` when parsing a URL
with a plaintext credential.
* `die`: Git will write a failure message to `stderr` when parsing a URL
with a plaintext credential.
7 changes: 7 additions & 0 deletions t/t5601-clone.sh
Expand Up @@ -71,6 +71,13 @@ test_expect_success 'clone respects GIT_WORK_TREE' '
'

test_expect_success 'clone warns or fails when using username:password' '
test_must_fail git -c fetch.credentialsInUrl=warn clone https://username:password@localhost attempt1 2>err &&
grep "warning: URL '\''https://username:\*\*\*\*\*\*\*\*@localhost/'\'' uses plaintext credentials" err &&
test_must_fail git -c fetch.credentialsInUrl=die clone https://username:password@localhost attempt2 2>err &&
grep "fatal: URL '\''https://username:\*\*\*\*\*\*\*\*@localhost/'\'' uses plaintext credentials" err
'

test_expect_success 'clone from hooks' '
test_create_repo r0 &&
Expand Down
43 changes: 43 additions & 0 deletions urlmatch.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "urlmatch.h"
#include "config.h"

#define URL_ALPHA "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
#define URL_DIGIT "0123456789"
Expand Down Expand Up @@ -106,6 +107,46 @@ static int match_host(const struct url_info *url_info,
return (!url_len && !pat_len);
}

static void detected_credentials_in_url(const char *url, size_t scheme_len)
{
char *value = NULL;
const char *at_ptr;
const char *colon_ptr;
struct strbuf anonymized = STRBUF_INIT;

/* "ignore" is the default behavior. */
if (git_config_get_string("fetch.credentialsinurl", &value) ||
!strcasecmp("ignore", value))
goto cleanup;

at_ptr = strchr(url, '@');
colon_ptr = strchr(url + scheme_len + 3, ':');

if (!colon_ptr)
BUG("failed to find colon in url '%s' with scheme_len %"PRIuMAX,
url, (uintmax_t) scheme_len);

/* Include everything including the colon. */
colon_ptr++;
strbuf_add(&anonymized, url, colon_ptr - url);

while (colon_ptr < at_ptr) {
strbuf_addch(&anonymized, '*');
colon_ptr++;
}

strbuf_addstr(&anonymized, at_ptr);

if (!strcasecmp("warn", value))
warning(_("URL '%s' uses plaintext credentials"), anonymized.buf);
if (!strcasecmp("die", value))
die(_("URL '%s' uses plaintext credentials"), anonymized.buf);

cleanup:
free(value);
strbuf_release(&anonymized);
}

static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
{
/*
Expand Down Expand Up @@ -144,6 +185,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
*/

size_t url_len = strlen(url);
const char *orig_url = url;
struct strbuf norm;
size_t spanned;
size_t scheme_len, user_off=0, user_len=0, passwd_off=0, passwd_len=0;
Expand Down Expand Up @@ -191,6 +233,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
}
colon_ptr = strchr(norm.buf + scheme_len + 3, ':');
if (colon_ptr) {
detected_credentials_in_url(orig_url, scheme_len);
passwd_off = (colon_ptr + 1) - norm.buf;
passwd_len = norm.len - passwd_off;
user_len = (passwd_off - 1) - (scheme_len + 3);
Expand Down

0 comments on commit 5145dc2

Please sign in to comment.