transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[M Hickford]

Assuming config key transfer.credentialsInUrl is set to "warn", Git warns about "plaintext credentials" if the user includes a password in the remote URL. https://git-scm.com/docs/git-config#Documentation/git-config.txt-transfercredentialsInUrl . This is implemented in remote.c

    $ git clone https://tim:hunter2@example.com/example.git
    warning: URL 'https://tim:<redacted>@example.com/example.git' uses plaintext credentials

It would be neat to warn similarly if the user includes a personal access token in the *user* field of the remote URL:

    git clone https://<pat>@github.com/...

This is a popular practice according to StackOverflow https://stackoverflow.com/a/70320541/284795 (800k views).

GitHub personal access tokens are easily recognised by their prefixes "ghp_" and "github_pat_" https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/about-authentication-to-github#githubs-token-formats 

#leftoverbits

Re: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[Junio C Hamano]

M Hickford <mirth.hickford@gmail.com> writes:

> It would be neat to warn similarly if the user includes a personal access token in the *user* field of the remote URL:
>
>     git clone https://<pat>@github.com/...
>
> This is a popular practice according to StackOverflow
> https://stackoverflow.com/a/70320541/284795 (800k views).
>
> GitHub personal access tokens are easily recognised by their
> prefixes "ghp_" and "github_pat_"

Curious.  I do not think we have *any* code to special case such a
"token", so to Git itself, https://<pat>@github.com/... should look
as it (assuming <pat> does not have any colon in it) is trying to
access the site with <pat> as the username.

How do we _know_ that this request with <pat> do not need a
password?  I ask because I'd prefer not to see us hardcoding any
hosting-site specific heuristics in the code, and these users
apparently are doing fine without any such hardcoding.  If we can
reuse the mechanism that is letting them do so when deciding if we
should warn, it would be great.

Are users expected to configure their credential helpers know that
it is a <pat> (perhaps with authtype=Bearer)?

Sorry for not giving any answer and piling more questions on top.

Re: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 3338 bytes --]

On 2025-01-10 at 21:32:44, Junio C Hamano wrote:
> M Hickford <mirth.hickford@gmail.com> writes:
> 
> > It would be neat to warn similarly if the user includes a personal access token in the *user* field of the remote URL:
> >
> >     git clone https://<pat>@github.com/...
> >
> > This is a popular practice according to StackOverflow
> > https://stackoverflow.com/a/70320541/284795 (800k views).
> >
> > GitHub personal access tokens are easily recognised by their
> > prefixes "ghp_" and "github_pat_"

There are also other prefixes as well.

> Curious.  I do not think we have *any* code to special case such a
> "token", so to Git itself, https://<pat>@github.com/... should look
> as it (assuming <pat> does not have any colon in it) is trying to
> access the site with <pat> as the username.

That is accurate.  GitHub has accepted a token in the username for some
time (at least since 2019, and probably longer) for reasons which are
unknown to me.  From a security perspective, I would recommend users not
do so, since many tools log the username and not the password[0], but
it's almost certainly too late to change it now.  I can mention it, but
even if there was a decision to drop that, it would require a long
deprecation period.

> How do we _know_ that this request with <pat> do not need a
> password?  I ask because I'd prefer not to see us hardcoding any
> hosting-site specific heuristics in the code, and these users
> apparently are doing fine without any such hardcoding.  If we can
> reuse the mechanism that is letting them do so when deciding if we
> should warn, it would be great.

I don't think in general we can know whether a credential is just a
plain username or a token without trying to guess based on the content.
For instance, before `http.emptyAuth`, it was common if one was using
Kerberos to put one's username in the URL because that triggered libcurl
to do authentication, whereas it would not if no credentials were
specified.  I still have that configured, and I bet a lot of others do
as well.

It's also common for people with both work and personal accounts on a
site to put the username in the URL so that the correct credentials are
looked up in the credential helper.  And all of that is fine and secure
since there are no actual secrets in the username in those cases.

So there are lots of legitimate reasons to place only a username there,
and we'd only be able to know if it's actually a token by hard-coding
patterns.  I would recommend that we not do that, since I can't
guarantee that the list of patterns won't expand in the future (it
already has before), and there are still some older hex-only patterns
which may be in use and which are much less obvious.

> Are users expected to configure their credential helpers know that
> it is a <pat> (perhaps with authtype=Bearer)?

No, this is Basic auth.  It's just that GitHub will look at either the
username or password field for the token.

[0] The two systems at GitHub which handle these credentials (the Git
service and the Git auth service) are designed to recognize tokens in
the username and obscure them instead of logging them.  Pretty much no
other tools on the Internet do that, though.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> I don't think in general we can know whether a credential is just a
> plain username or a token without trying to guess based on the content.
> For instance, before `http.emptyAuth`, it was common if one was using
> Kerberos to put one's username in the URL because that triggered libcurl
> to do authentication, whereas it would not if no credentials were
> specified.  I still have that configured, and I bet a lot of others do
> as well.
>
> It's also common for people with both work and personal accounts on a
> site to put the username in the URL so that the correct credentials are
> looked up in the credential helper.  And all of that is fine and secure
> since there are no actual secrets in the username in those cases.
>
> So there are lots of legitimate reasons to place only a username there,
> and we'd only be able to know if it's actually a token by hard-coding
> patterns.  I would recommend that we not do that, since I can't
> guarantee that the list of patterns won't expand in the future (it
> already has before), and there are still some older hex-only patterns
> which may be in use and which are much less obvious.

Yes, I do strongly object us to keep a hardcoded list that can go
stale (or be stale from the beginning).

What I was wondering is that because we are in control of the Git
end of the credential subsystem (even if the user may be using a
third-party credential helper), we

 - can notice that the URL has embedded single thing (which could be
   username, but which could be a token);

 - can also notice that we asked the credential-helper, or
   keyboard-interactive, and obtained a password (or not).

When the former is true and the latter is false, it is an indication
that for that site with the username-or-token, there wasn't anything
necessary to authenticate and the access was authorized.  Which is
what the original poster wants us to warn against.

And if we can do so, we do not need any list of patterns, right?

Re: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]

On 2025-01-10 at 22:51:01, Junio C Hamano wrote:
> Yes, I do strongly object us to keep a hardcoded list that can go
> stale (or be stale from the beginning).
> 
> What I was wondering is that because we are in control of the Git
> end of the credential subsystem (even if the user may be using a
> third-party credential helper), we
> 
>  - can notice that the URL has embedded single thing (which could be
>    username, but which could be a token);
> 
>  - can also notice that we asked the credential-helper, or
>    keyboard-interactive, and obtained a password (or not).
> 
> When the former is true and the latter is false, it is an indication
> that for that site with the username-or-token, there wasn't anything
> necessary to authenticate and the access was authorized.  Which is
> what the original poster wants us to warn against.

No, I don't think that will work in the general case.  Here's why.  If I
do `git push https://bmc@git.crustytoothpaste.net/git/bmc/xyzzy.git`,
that uses Kerberos (Negotiate).  There's a username there to make
libcurl enable auth, but it's never used and a credential helper is
never invoked, so case 1 is true and case 2 is false.

Now, we _could_ do that only for Basic auth, which would catch the
GitHub case.  However, it's _also_ possible to use TLS client
certificate auth (and I think Bitbucket does support that) and use the
username only for choosing the account (because, say, your work account
uses a client certificate and your personal account uses something
else).  There might be Basic auth sent (say, if you'd set
`http.proactiveAuth`), but the server would ignore it since you were
already authenticated via the TLS cert.  That would also make case 1 be
true and case 2 be false.

Perhaps that latter case is not worth worrying about, but it is a
possibility and I'm sure some people will hit it.  Maybe with a config
option for the advice that's okay, though.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

>> When the former is true and the latter is false, it is an indication
>> that for that site with the username-or-token, there wasn't anything
>> necessary to authenticate and the access was authorized.  Which is
>> what the original poster wants us to warn against.
>
> No, I don't think that will work in the general case.  Here's why.  If I
> do `git push https://bmc@git.crustytoothpaste.net/git/bmc/xyzzy.git`,
> that uses Kerberos (Negotiate).  There's a username there to make
> libcurl enable auth, but it's never used and a credential helper is
> never invoked, so case 1 is true and case 2 is false.
>
> Now, we _could_ do that only for Basic auth, which would catch the
> GitHub case.  However, it's _also_ possible to use TLS client
> certificate auth (and I think Bitbucket does support that) and use the
> username only for choosing the account (because, say, your work account
> uses a client certificate and your personal account uses something
> else).  There might be Basic auth sent (say, if you'd set
> `http.proactiveAuth`), but the server would ignore it since you were
> already authenticated via the TLS cert.  That would also make case 1 be
> true and case 2 be false.
>
> Perhaps that latter case is not worth worrying about, but it is a
> possibility and I'm sure some people will hit it.  Maybe with a config
> option for the advice that's okay, though.

Thanks, so in short, if we really want to do this "warning", it has
to look at the "username" string and "guess" that it is something
the user does not want to have as a value to remote.*.url field X-<.

So "should warn" in the title is not really #leftoverbits (i.e. we
clearly know it is a good idea, we just didn't have bandwidth to do
it, anybody is welcome to fill the void) at this point.

Thanks.

RE: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[rsbecker]

On January 10, 2025 7:45 PM, Junio C Hamano wrote:
>"brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
>>> When the former is true and the latter is false, it is an indication
>>> that for that site with the username-or-token, there wasn't anything
>>> necessary to authenticate and the access was authorized.  Which is
>>> what the original poster wants us to warn against.
>>
>> No, I don't think that will work in the general case.  Here's why.  If
>> I do `git push
>> https://bmc@git.crustytoothpaste.net/git/bmc/xyzzy.git`,
>> that uses Kerberos (Negotiate).  There's a username there to make
>> libcurl enable auth, but it's never used and a credential helper is
>> never invoked, so case 1 is true and case 2 is false.
>>
>> Now, we _could_ do that only for Basic auth, which would catch the
>> GitHub case.  However, it's _also_ possible to use TLS client
>> certificate auth (and I think Bitbucket does support that) and use the
>> username only for choosing the account (because, say, your work
>> account uses a client certificate and your personal account uses
>> something else).  There might be Basic auth sent (say, if you'd set
>> `http.proactiveAuth`), but the server would ignore it since you were
>> already authenticated via the TLS cert.  That would also make case 1
>> be true and case 2 be false.

If the use case is like mine, where someone wants to use something similar
to a PAT for creating a Pull Request, then GitHub, BitBucket, GitLab, and
AzureGit all behave in a very similar way. The PAT structure is different,
but
there is conceptual equivalence.

--Randall

>>
>> Perhaps that latter case is not worth worrying about, but it is a
>> possibility and I'm sure some people will hit it.  Maybe with a config
>> option for the advice that's okay, though.
>
>Thanks, so in short, if we really want to do this "warning", it has to look
at the
>"username" string and "guess" that it is something the user does not want
to have
>as a value to remote.*.url field X-<.
>
>So "should warn" in the title is not really #leftoverbits (i.e. we clearly
know it is a
>good idea, we just didn't have bandwidth to do it, anybody is welcome to
fill the
>void) at this point.
>
>Thanks.

Re: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[M Hickford]

On 2025-01-10 22:06, brian m. carlson wrote:
> No, this is Basic auth.  It's just that GitHub will look at either the
> username or password field for the token.

Thanks Brian and Junio for the discussion. I understand your aversion to 
introducing GitHub-specific PAT detection logic.

The better solution would be for GitHub to stop accepting PAT in the 
username field. Hopefully that will happen one day. It's undocumented,
so its usage ought to diminish.

To speed that along, I've edited the offending StackOverflow answer 
https://stackoverflow.com/a/70320541/284795

RE: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[rsbecker]

On January 10, 2025 4:33 PM, Junio C Hamano wrote:
>M Hickford <mirth.hickford@gmail.com> writes:
>
>> It would be neat to warn similarly if the user includes a personal access token in
>the *user* field of the remote URL:
>>
>>     git clone https://<pat>@github.com/...
>>
>> This is a popular practice according to StackOverflow
>> https://stackoverflow.com/a/70320541/284795 (800k views).
>>
>> GitHub personal access tokens are easily recognised by their prefixes
>> "ghp_" and "github_pat_"
>
>Curious.  I do not think we have *any* code to special case such a "token", so to Git
>itself, https://<pat>@github.com/... should look as it (assuming <pat> does not
>have any colon in it) is trying to access the site with <pat> as the username.
>
>How do we _know_ that this request with <pat> do not need a password?  I ask
>because I'd prefer not to see us hardcoding any hosting-site specific heuristics in the
>code, and these users apparently are doing fine without any such hardcoding.  If we
>can reuse the mechanism that is letting them do so when deciding if we should
>warn, it would be great.
>
>Are users expected to configure their credential helpers know that it is a <pat>
>(perhaps with authtype=Bearer)?

This is why I implemented my own credential layer - sorry, cannot share,
but can describe if asked (maybe at a Git Merge? 😉 ). Git should not need to
know about GitHub specific access methods for HTTPS, where PAT is required
for git push. There is no password for PAT use, which makes it highly insecure
if persisted on disk - I do not know why GitHub went down that path, I guess
they did not want any potential CVEs associated with SSH-like passphrases.
PAT became a real annoyance for my community, so I had to do something quick
and adding my own credential manager seemed expedient (and works pretty
well - but if someone wants to do this, they must be very careful managing
access to the cached PAT's memory use). The primary use case in my situation
is not actually git push, but the creation of Pull Requests on GitHub from
scripts or a CLI since NonStop has no GUI adapter to access their website.

>
>Sorry for not giving any answer and piling more questions on top.

RE: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[Randall Becker]

My email rsbecker@nexbridge.com is being blocked by gmail, so I cannot get to Derek.
Anyone know why? Have I been a bad boy somehow? Resending via another account.

On January 10, 2025 4:33 PM, Junio C Hamano wrote:
>M Hickford <mirth.hickford@gmail.com> writes:
>
>> It would be neat to warn similarly if the user includes a personal access token in
>the *user* field of the remote URL:
>>
>>     git clone https://<pat>@github.com/...
>>
>> This is a popular practice according to StackOverflow
>> https://stackoverflow.com/a/70320541/284795 (800k views).
>>
>> GitHub personal access tokens are easily recognised by their prefixes
>> "ghp_" and "github_pat_"
>
>Curious.  I do not think we have *any* code to special case such a "token", so to Git
>itself, https://<pat>@github.com/... should look as it (assuming <pat> does not
>have any colon in it) is trying to access the site with <pat> as the username.
>
>How do we _know_ that this request with <pat> do not need a password?  I ask
>because I'd prefer not to see us hardcoding any hosting-site specific heuristics in the
>code, and these users apparently are doing fine without any such hardcoding.  If we
>can reuse the mechanism that is letting them do so when deciding if we should
>warn, it would be great.
>
>Are users expected to configure their credential helpers know that it is a <pat>
>(perhaps with authtype=Bearer)?

This is why I implemented my own credential layer - sorry, cannot share,
but can describe if asked (maybe at a Git Merge? 😉 ). Git should not need to
know about GitHub specific access methods for HTTPS, where PAT is required
for git push. There is no password for PAT use, which makes it highly insecure
if persisted on disk - I do not know why GitHub went down that path, I guess
they did not want any potential CVEs associated with SSH-like passphrases.
PAT became a real annoyance for my community, so I had to do something quick
and adding my own credential manager seemed expedient (and works pretty
well - but if someone wants to do this, they must be very careful managing
access to the cached PAT's memory use). The primary use case in my situation
is not actually git push, but the creation of Pull Requests on GitHub from
scripts or a CLI since NonStop has no GUI adapter to access their website.

>
>Sorry for not giving any answer and piling more questions on top.

Re: transfer.credentialsInUrl should warn about personal access tokens in user field #leftoverbits

[Junio C Hamano]

Randall Becker <randall.becker@nexbridge.ca> writes:

> My email rsbecker@nexbridge.com is being blocked by gmail, so I cannot get to Derek.
> Anyone know why? Have I been a bad boy somehow? Resending via another account.

It is not you, but him.

commit 6e5457d8c7332c279e1482c442922f954af5f8f2
Author: Derrick Stolee <stolee@gmail.com>
Date:   Thu Oct 12 17:30:33 2023 +0000

    mailmap: change primary address for Derrick Stolee
    
    The previous primary address is no longer valid.
    
    Signed-off-by: Derrick Stolee <stolee@gmail.com>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/.mailmap b/.mailmap
index dc31d70b8c..82129be449 100644
--- a/.mailmap
+++ b/.mailmap
@@ -59,9 +59,9 @@ David Reiss <dreiss@facebook.com> <dreiss@dreiss-vmware.(none)>
 David S. Miller <davem@davemloft.net>
 David Turner <novalis@novalis.org> <dturner@twopensource.com>
 David Turner <novalis@novalis.org> <dturner@twosigma.com>
-Derrick Stolee <derrickstolee@github.com> <stolee@gmail.com>
-Derrick Stolee <derrickstolee@github.com> Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
-Derrick Stolee <derrickstolee@github.com> <dstolee@microsoft.com>
+Derrick Stolee <stolee@gmail.com> <derrickstolee@github.com>
+Derrick Stolee <stolee@gmail.com> Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
+Derrick Stolee <stolee@gmail.com> <dstolee@microsoft.com>
 Deskin Miller <deskinm@umich.edu>
 Đoàn Trần Công Danh <congdanhqx@gmail.com> Doan Tran Cong Danh
 Dirk Süsserott <newsletter@dirk.my1.cc>