Skip to content

Conversation

@bk2204
Copy link
Member

@bk2204 bk2204 commented May 28, 2024

Git recently learned to announce capabilities for credential helpers and this functionality will be in Git 2.46. One of the capabilities is the ability for credential helpers to specify arbitrary HTTP credential schemes and the credentials for these schemes. This allows users with a suitable credential helper to implement support for, say, Bearer authentication where the credentials come from a credential helper.

With this capability, Git LFS will advertise the capability[]=authtype line, and a suitable version of Git will pass this along to the helper, which can then return something like authtype=Bearer and credential=my-bearer-token to provide a Authorization: Bearer my-bearer-token header. It can also return other schemes as well, and the credential field can include any necessary parameters that the scheme may require.

In order to implement this in Git LFS, we first need to adjust our test credential helper and the server so that they can handle this new authentication type and return appropriate credentials. Then, we add support in the main codebase and verify that it works as expected.

Note that Git versions which do not support this capability will exit unsuccessfully for git credential capability (causing our tests to be skipped) and will do nothing with the capability[]=authtype declaration, which means that it will not be passed to the credential helper. At that point, the credential helper can either return the previous username/password pair, or decide to return nothing at all.

There is an additional capability to allow multi-stage authentication, such as NTLM or Kerberos, in the credential helper. These both require two round trips for authentication to succeed, which needs additional support from the helper and Git or Git LFS. Supporting NTLM in Git LFS via a suitable credential helper was in fact one of the goals of the upstream series, in addition to discouraging the use of the config file for storing authentication credentials (such as used to be required for Bearer auth). However, the additional capability required for that is not implemented here, and can be implemented in an additional series in due course.

@bk2204 bk2204 marked this pull request as ready for review June 7, 2024 16:10
@bk2204 bk2204 requested a review from a team as a code owner June 7, 2024 16:10
Copy link
Member

@chrisd8088 chrisd8088 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nifty, thank you!


func credsFromFilename(file string) (string, string, error) {
func credsFromFilename(file string) (string, string, string, error) {
userPass, err := os.ReadFile(file)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor note that userPass is no longer entirely accurate as a variable name here, since it's expected to hold authorization type, username, and credential fields instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll fix that.

grep "Tracking \"\*.dat\"" track.log

contents="b"
contents_oid=$(calc_oid "$contents")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this contents_oid variable is used, so perhaps it could be removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'll drop that.


GIT_TERMINAL_PROMPT=0 GIT_TRACE=1 GIT_TRANSFER_TRACE=1 GIT_CURL_VERBOSE=1 git push origin new-branch 2>&1 | tee push.log
grep "Uploading LFS objects: 100% (1/1), 1 B" push.log
echo "approvals:"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor thought: these echo statements could possibly just be inline comments, as I don't think anything output to stdout is usually visible when running the tests.

bk2204 added 3 commits June 10, 2024 13:27
In newer versions of Git, the credential helper protocol has learned to
support capabilities and credentials that aren't based around a username
and password.  This allows the use of Bearer credentials, and it allows
credential helpers to implement complex functionality like Digest and
NTLM (both of which require insecure password storage) so we don't have
to.

We'd like to support this in our testsuite, so extend our credential
helper to accept these new capabilities and use them to determine a
response.  In conformance with the credential helper protocol, we only
advertise support for capabilities which we ourselves support.

In our files in `CREDSDIR`, instead of looking for two parts separated
by colons which are the username and password, accept three parts, which
are the authentication type, username, and credential.  If the
authentication type is empty, then the credential is the password;
otherwise, the username is empty, and the credential is sent in the
credential field.

Right now, we don't make use of this new functionality, but we do update
existing credential files to make them compatible.  Use of the new
functionality will come in a future commit.
Implement some endpoints with Bearer authentication so we can test this
elsewhere in the testsuite.  If the path starts with `/auth-bearer`,
send a `WWW-Authenticate` header with `Bearer` instead of an
`LFS-Authenticate` header with `Basic`.  This lets us test both types of
header as well as the new Bearer authentication.
Now that our test credential helper and test server are capable of
supporting the new authtype and credential fields, let's pass the
appropriate capability to the helper.  We use the fields if they exist,
and pass them back to the `approve` call to the helper on success, along
with the capability.

If these fields don't both exist, we fall back to the username and
password fields.  This allows us to do thing such as query different
accounts with the username if one is specified in the URL, but get the
credentials back in a format that contains a non-Basic response.

Since we now support more than just Basic and Negotiate, let's make sure
we iterate through all of the supported authentication schemes, not just
those two.  This allows the credential helper to support multiple
schemes.
@bk2204 bk2204 force-pushed the credential-authtype branch from bbba2c4 to e2cd22c Compare June 10, 2024 13:27
@bk2204 bk2204 merged commit d065d1f into git-lfs:main Jun 10, 2024
@bk2204 bk2204 deleted the credential-authtype branch June 10, 2024 15:02
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 8, 2024
In commit 08998d6 of PR git-lfs#5779 we
updated our Git credential helper test program to reflect the new
support in Git v2.46.0 for authentication schemes other than those
using usernames and passwords, specifically by accepting input
lines with the "capability[]" key and replying with similar lines
as appropriate.

In particular, if the client sends one or more "capability[]" lines,
we check their values against the list of capabilities our helper
program supports, which at the moment is just the "authtype" and
"state" capabilities.  If any matches are found, we include those
in "capability[]" lines in our reply, excluding any capabilities
the client announced but which our program does not support.

At the moment, we do this by first overwriting the "capability[]"
entry in the map of key/value pairs we created from the client's input,
and then copying that entry into the "return" map of key/value pairs
we'll send back to the client.  Finally, we make sure to output
the values from the "capability[]" entry in that map before sending
the values of all the remaining entries.  (Note that each entry
in these maps may have multiple values.  If a key is multi-valued,
we send each value on a separate line in the output.)

We can simplify our handling of these "capability[]" lines somewhat
by skipping the step where we overwrite the entry in the map of
input lines, and also by skipping the copy of the values we intend
to return into the "result" map.  Instead, we can just return the
filtered set of capabilities we support from our discoverCapabilities()
function, and then output the values from that set first before
sending the values from our "return" map.

This does mean that we only pass the filtered set of capabilities
to the Serialize() method of our "credential" structure, instead of
the full set received from the client.  But this change has no
functional impact because that method only needs to check for
the capability types we support ("authtype" and "state"), so those
will always be in the filtered set of capabilities, assuming the
client sent them to us.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 15, 2024
In commit 08998d6 of PR git-lfs#5779 we
updated our Git credential helper test program to reflect the new
support in Git v2.46.0 for authentication schemes other than those
using usernames and passwords, specifically by accepting input
lines with the "capability[]" key and replying with similar lines
as appropriate.

In particular, if the client sends one or more "capability[]" lines,
we check their values against the list of capabilities our helper
program supports, which at the moment is just the "authtype" and
"state" capabilities.  If any matches are found, we include those
in "capability[]" lines in our reply, excluding any capabilities
the client announced but which our program does not support.

At the moment, we do this by first overwriting the "capability[]"
entry in the map of key/value pairs we created from the client's input,
and then copying that entry into the "return" map of key/value pairs
we'll send back to the client.  Finally, we make sure to output
the values from the "capability[]" entry in that map before sending
the values of all the remaining entries.  (Note that each entry
in these maps may have multiple values.  If a key is multi-valued,
we send each value on a separate line in the output.)

We can simplify our handling of these "capability[]" lines somewhat
by skipping the step where we overwrite the entry in the map of
input lines, and also by skipping the copy of the values we intend
to return into the "result" map.  Instead, we can just return the
filtered set of capabilities we support from our discoverCapabilities()
function, and then output the values from that set first before
sending the values from our "return" map.

This does mean that we only pass the filtered set of capabilities
to the Serialize() method of our "credential" structure, instead of
the full set received from the client.  But this change has no
functional impact because that method only needs to check for
the capability types we support ("authtype" and "state"), so those
will always be in the filtered set of capabilities, assuming the
client sent them to us.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
In commit 08998d6 of PR git-lfs#5779 we
updated our Git credential helper test program to reflect the new
support in Git v2.46.0 for authentication schemes other than those
using usernames and passwords, specifically by accepting input
lines with the "capability[]" key and replying with similar lines
as appropriate.

In particular, if the client sends one or more "capability[]" lines,
we check their values against the list of capabilities our helper
program supports, which at the moment is just the "authtype" and
"state" capabilities.  If any matches are found, we include those
in "capability[]" lines in our reply, excluding any capabilities
the client announced but which our program does not support.

At the moment, we do this by first overwriting the "capability[]"
entry in the map of key/value pairs we created from the client's input,
and then copying that entry into the "return" map of key/value pairs
we'll send back to the client.  Finally, we make sure to output
the values from the "capability[]" entry in that map before sending
the values of all the remaining entries.  (Note that each entry
in these maps may have multiple values.  If a key is multi-valued,
we send each value on a separate line in the output.)

We can simplify our handling of these "capability[]" lines somewhat
by skipping the step where we overwrite the entry in the map of
input lines, and also by skipping the copy of the values we intend
to return into the "result" map.  Instead, we can just return the
filtered set of capabilities we support from our discoverCapabilities()
function, and then output the values from that set first before
sending the values from our "return" map.

This does mean that we only pass the filtered set of capabilities
to the Serialize() method of our "credential" structure, instead of
the full set received from the client.  But this change has no
functional impact because that method only needs to check for
the capability types we support ("authtype" and "state"), so those
will always be in the filtered set of capabilities, assuming the
client sent them to us.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
In commit 08998d6 of PR git-lfs#5779 we
updated our Git credential helper test program to reflect the new
support in Git v2.46.0 for authentication schemes other than those
using usernames and passwords, specifically by accepting input
lines with the "capability[]" key and replying with similar lines
as appropriate.

In particular, if the client sends one or more "capability[]" lines,
we check their values against the list of capabilities our helper
program supports, which at the moment is just the "authtype" and
"state" capabilities.  If any matches are found, we include those
in "capability[]" lines in our reply, excluding any capabilities
the client announced but which our program does not support.

At the moment, we do this by first overwriting the "capability[]"
entry in the map of key/value pairs we created from the client's input,
and then copying that entry into the "return" map of key/value pairs
we will send back to the client.  Finally, we make sure to output
the values from the "capability[]" entry in that map before sending
the values of all the remaining entries.  (Note that each entry
in these maps may have multiple values.  If a key is multi-valued,
we send each value on a separate line in the output.)

We can simplify our handling of these "capability[]" lines somewhat
by skipping the step where we overwrite the entry in the map of
input lines, and also by not making a copy of the values we intend
to return into the "result" map.  Instead, we can just return the
filtered set of capabilities we support from our discoverCapabilities()
function, and then output the values from that set first before
sending the values from our "return" map.

This does mean that we only pass the filtered set of capabilities
to the Serialize() method of our "credential" structure, instead of
the full set received from the client.  But this change has no
functional impact because that method only needs to check for
the capability types we support ("authtype" and "state"), so those
will always be in the filtered set of capabilities, assuming the
client sent them to us.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants