[RFC] Add way to make Git credentials accessible from clean/smudge filter

[RFC] Add way to make Git credentials accessible from clean/smudge filter

[Lars Schneider]

Hi,

we just implemented the first "real-world" user of the new clean/smudge 
"filter protocol" interface (see "convert: add filter.<driver>.process option"
edcc858 for details) and the results are fantastic. Filtering 12,000 files in
my artificial test repo is more than 60x faster (depending on the platform). 
On Windows that means the clean/smudge operations runs in 57 seconds instead 
of 55 minutes [1]!

I have a number of ideas to improve the protocol even further and I am seeking
early feedback on the following - possibly most controversial - idea:

Some filters might want to perform additional network interactions and these
filters would like to use the Git credentials to perform these actions. If
such a filter is configured with "offerCredentials = true" then the filter 
could request the current Git credentials via the "filter-protocol".

A configuration could look like this:
------------------------
[filter "myfilter"]
    process = my-filter-process
    required = true
    offerCredentials = true
------------------------

The default would, of course, be "offerCredentials = false".

I haven't looked at an implemenation approach at all. I wonder if this could
be OK from a conceptional point of view or if there are obvious security 
problems that I am missing.

Thanks,
Lars


[1] https://github.com/github/git-lfs/pull/1617#issuecomment-259411850

Re: [RFC] Add way to make Git credentials accessible from clean/smudge filter

[Matthieu Moy]

Lars Schneider <larsxschneider@gmail.com> writes:

> I haven't looked at an implemenation approach at all. I wonder if this could
> be OK from a conceptional point of view or if there are obvious security 
> problems that I am missing.

Did you consider just running "git credential" from the filter? It may
not be the perfect solution, but it should work. I already used it to
get credential from a remote-helper (git-remote-mediawiki). When
prompting credentials interactively, it grabs the terminal directly, so
it work even if stdin/stdout are used for the protocol.

Asking the main git process to get the credentials probably has added
value like the ability to prompt once and use the same for several
filter processes.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC] Add way to make Git credentials accessible from clean/smudge filter

[Jeff King]

On Thu, Nov 10, 2016 at 01:10:17PM +0100, Matthieu Moy wrote:

> Lars Schneider <larsxschneider@gmail.com> writes:
> 
> > I haven't looked at an implemenation approach at all. I wonder if this could
> > be OK from a conceptional point of view or if there are obvious security 
> > problems that I am missing.
> 
> Did you consider just running "git credential" from the filter? It may
> not be the perfect solution, but it should work. I already used it to
> get credential from a remote-helper (git-remote-mediawiki). When
> prompting credentials interactively, it grabs the terminal directly, so
> it work even if stdin/stdout are used for the protocol.

Yeah, that is the solution I was going to suggest. The credentials are
totally orthogonal to the filters, and I would rather not shove them
into the protocol. It's an extra process, but with the new multi-use
smudge filter, it's one per git invocation, not one per file.

-Peff

Re: [RFC] Add way to make Git credentials accessible from clean/smudge filter

[Lars Schneider]

On 10 Nov 2016, at 17:08, Jeff King <peff@peff.net> wrote:

> On Thu, Nov 10, 2016 at 01:10:17PM +0100, Matthieu Moy wrote:
> 
>> Lars Schneider <larsxschneider@gmail.com> writes:
>> 
>>> I haven't looked at an implemenation approach at all. I wonder if this could
>>> be OK from a conceptional point of view or if there are obvious security 
>>> problems that I am missing.
>> 
>> Did you consider just running "git credential" from the filter? It may
>> not be the perfect solution, but it should work. I already used it to
>> get credential from a remote-helper (git-remote-mediawiki). When
>> prompting credentials interactively, it grabs the terminal directly, so
>> it work even if stdin/stdout are used for the protocol.
> 
> Yeah, that is the solution I was going to suggest. The credentials are
> totally orthogonal to the filters, and I would rather not shove them
> into the protocol. It's an extra process, but with the new multi-use
> smudge filter, it's one per git invocation, not one per file.

The trouble with "git credential" is that it works only if the credential 
helper is setup correctly. Although I assume that most people have setup this, 
I have also worked with a number of people who prefer to enter their passwords 
every time Git makes a network connection.

- Lars

Re: [RFC] Add way to make Git credentials accessible from clean/smudge filter

[Jeff King]

On Fri, Nov 11, 2016 at 10:28:56AM +0100, Lars Schneider wrote:

> > Yeah, that is the solution I was going to suggest. The credentials are
> > totally orthogonal to the filters, and I would rather not shove them
> > into the protocol. It's an extra process, but with the new multi-use
> > smudge filter, it's one per git invocation, not one per file.
> 
> The trouble with "git credential" is that it works only if the credential 
> helper is setup correctly. Although I assume that most people have setup this, 
> I have also worked with a number of people who prefer to enter their passwords 
> every time Git makes a network connection.

Are you sure about that? If I do:

  echo url=https://example.com/repo.git |
  git credential fill

I get prompted for a username and password.

-Peff

Re: [RFC] Add way to make Git credentials accessible from clean/smudge filter

[Lars Schneider]

On 11 Nov 2016, at 10:31, Jeff King <peff@peff.net> wrote:

> On Fri, Nov 11, 2016 at 10:28:56AM +0100, Lars Schneider wrote:
> 
>>> Yeah, that is the solution I was going to suggest. The credentials are
>>> totally orthogonal to the filters, and I would rather not shove them
>>> into the protocol. It's an extra process, but with the new multi-use
>>> smudge filter, it's one per git invocation, not one per file.
>> 
>> The trouble with "git credential" is that it works only if the credential 
>> helper is setup correctly. Although I assume that most people have setup this, 
>> I have also worked with a number of people who prefer to enter their passwords 
>> every time Git makes a network connection.
> 
> Are you sure about that? If I do:
> 
>  echo url=https://example.com/repo.git |
>  git credential fill
> 
> I get prompted for a username and password.

Hm.. either I don't understand you or I expressed myself unclear. 

Let's say a user runs:

$ git clone https://myrepo.git

If no credential helper is setup, then Git asks the user for credentials.
Afterwards Git starts downloading stuff. At some point Git will run my
smudge filter on some files and in my case the smudge filter needs the
Git credentials. AFAIK, the smudge filter has no way to get the credentials 
from Git at this point - not even by invoking "git credential". 
Is this correct?

- Lars

Re: [RFC] Add way to make Git credentials accessible from clean/smudge filter

[Dennis Kaarsemaker]

On Fri, 2016-11-11 at 10:40 +0100, Lars Schneider wrote:
> On 11 Nov 2016, at 10:31, Jeff King <peff@peff.net> wrote:
> 
> > On Fri, Nov 11, 2016 at 10:28:56AM +0100, Lars Schneider wrote:
> > 
> > > > Yeah, that is the solution I was going to suggest. The credentials are
> > > > totally orthogonal to the filters, and I would rather not shove them
> > > > into the protocol. It's an extra process, but with the new multi-use
> > > > smudge filter, it's one per git invocation, not one per file.
> > > 
> > > 
> > > The trouble with "git credential" is that it works only if the credential 
> > > helper is setup correctly. Although I assume that most people have setup this, 
> > > I have also worked with a number of people who prefer to enter their passwords 
> > > every time Git makes a network connection.
> > 
> > 
> > Are you sure about that? If I do:
> > 
> >  echo url=https://example.com/repo.git |
> >  git credential fill
> > 
> > I get prompted for a username and password.
> 
> 
> Hm.. either I don't understand you or I expressed myself unclear. 
> 
> Let's say a user runs:
> 
> $ git clone https://myrepo.git
> 
> If no credential helper is setup, then Git asks the user for credentials.
> Afterwards Git starts downloading stuff. At some point Git will run my
> smudge filter on some files and in my case the smudge filter needs the
> Git credentials. AFAIK, the smudge filter has no way to get the credentials 
> from Git at this point - not even by invoking "git credential". 
> Is this correct?

I think that's correct, but the same argument goes both ways: unless I
use a credential helper, or explicitely give a filter application my
credentials, I don't want a helper to be able to get to those
credentials. I'd consider that a security bug.

D.

Re: [RFC] Add way to make Git credentials accessible from clean/smudge filter

[Jeff King]

On Fri, Nov 11, 2016 at 09:02:52PM +0100, Dennis Kaarsemaker wrote:

> > > Are you sure about that? If I do:
> > > 
> > >  echo url=https://example.com/repo.git |
> > >  git credential fill
> > > 
> > > I get prompted for a username and password.
> > 
> > 
> > Hm.. either I don't understand you or I expressed myself unclear. 
> > 
> > Let's say a user runs:
> > 
> > $ git clone https://myrepo.git
> > 
> > If no credential helper is setup, then Git asks the user for credentials.
> > Afterwards Git starts downloading stuff. At some point Git will run my
> > smudge filter on some files and in my case the smudge filter needs the
> > Git credentials. AFAIK, the smudge filter has no way to get the credentials 
> > from Git at this point - not even by invoking "git credential". 
> > Is this correct?
> 
> I think that's correct, but the same argument goes both ways: unless I
> use a credential helper, or explicitely give a filter application my
> credentials, I don't want a helper to be able to get to those
> credentials. I'd consider that a security bug.

Yeah, agreed. They are logically two separate operations, so I think it
is a feature that they do not implicitly share credentials.

I think the only place where we implicitly share credentials is when
serving an HTTP fetch or push requires multiple HTTP requests. And there
it seems pretty sane to do so.

-Peff

Re: [RFC] Add way to make Git credentials accessible from clean/smudge filter

[Lars Schneider]

> On 11 Nov 2016, at 21:27, Jeff King <peff@peff.net> wrote:
> 
> On Fri, Nov 11, 2016 at 09:02:52PM +0100, Dennis Kaarsemaker wrote:
> 
>>>> Are you sure about that? If I do:
>>>> 
>>>> echo url=https://example.com/repo.git |
>>>> git credential fill
>>>> 
>>>> I get prompted for a username and password.
>>> 
>>> 
>>> Hm.. either I don't understand you or I expressed myself unclear. 
>>> 
>>> Let's say a user runs:
>>> 
>>> $ git clone https://myrepo.git
>>> 
>>> If no credential helper is setup, then Git asks the user for credentials.
>>> Afterwards Git starts downloading stuff. At some point Git will run my
>>> smudge filter on some files and in my case the smudge filter needs the
>>> Git credentials. AFAIK, the smudge filter has no way to get the credentials 
>>> from Git at this point - not even by invoking "git credential". 
>>> Is this correct?
>> 
>> I think that's correct, but the same argument goes both ways: unless I
>> use a credential helper, or explicitely give a filter application my
>> credentials, I don't want a helper to be able to get to those
>> credentials. I'd consider that a security bug.
> 
> Yeah, agreed. They are logically two separate operations, so I think it
> is a feature that they do not implicitly share credentials.
> 
> I think the only place where we implicitly share credentials is when
> serving an HTTP fetch or push requires multiple HTTP requests. And there
> it seems pretty sane to do so.

Agreed. Thanks for your thoughts on this!

- Lars