RFC patch, 2 partsAllow adding .git files and directories

> To: Lukas Straub <lukasstraub2@web.de>
> Cc: git <git@vger.kernel.org>; Elijah Newren <newren@gmail.com>;
> Brandon Williams <bwilliams.eng@gmail.com>; Johannes Schindelin
> <Johannes.Schindelin@gmx.de>; Jeff King <peff@peff.net>
> Subject: Re: [RFC PATCH 0/2] Allow adding .git files and directories
> 
> Lukas Straub <lukasstraub2@web.de> writes:
> 
> > These patches allow this and work well in a quick test. Of course some
> > tests fail because with this the handling of nested git repos changed.
> 
> In other words, this breaks the workflow existing users rely on, right?  I

> To: Randall S. Becker <rsbecker@nexbridge.com>
> Cc: 'Lukas Straub' <lukasstraub2@web.de>; 'git' <git@vger.kernel.org>;
> 'Elijah Newren' <newren@gmail.com>; 'Brandon Williams'
> <bwilliams.eng@gmail.com>; 'Johannes Schindelin'
> <Johannes.Schindelin@gmx.de>; 'Jeff King' <peff@peff.net>
> Subject: Re: [RFC PATCH 0/2] Allow adding .git files and directories
> 
> "Randall S. Becker" <rsbecker@nexbridge.com> writes:
> 
> > Just putting my CSIO hat on here. We would need a system-wide setting
> > to prohibit users from using this capability.
> 
> Or just discard this patch, which is a lot simpler.  I don't see any need for this
> one.

> "Randall S. Becker" <rsbecker@nexbridge.com> writes:
> 
> > Just putting my CSIO hat on here. We would need a system-wide setting to
> > prohibit users from using this capability.
> 
> Or just discard this patch, which is a lot simpler.  I don't see any
> need for this one.

> It _could_ still be useful in a more isolated environment (e.g., your
> company server that is serving only internal repos to employees). But I
> have misgivings about a feature that lets people intentionally create
> repositories whose history cannot ever interact with other users who
> haven't set a special config flag. It's one thing to say "to take
> advantage of this feature, we must all agree to have version X, or set
> flag Y". But it's another to bake that restriction into the repository
> history for all time.

> Jeff King <peff@peff.net> writes:
> 
> > It _could_ still be useful in a more isolated environment (e.g., your
> > company server that is serving only internal repos to employees). But I
> > have misgivings about a feature that lets people intentionally create
> > repositories whose history cannot ever interact with other users who
> > haven't set a special config flag. It's one thing to say "to take
> > advantage of this feature, we must all agree to have version X, or set
> > flag Y". But it's another to bake that restriction into the repository
> > history for all time.
> 
> If people want a pre-prepared repository propagated to CI
> environment and keep trakc of the state of such repository over
> time, for example, they can use (versioned) tarballs.  Such a
> tarball won't automatically get extracted after "git pull" (which
> is a feature), but those who want such a pre-prepared repository
> for CI can make the extraction step as a part of their CI build
> procedure.

> On Wed, Aug 19, 2020 at 01:32:28PM -0700, Junio C Hamano wrote:
> 
> > Jeff King <peff@peff.net> writes:
> >
> > > It _could_ still be useful in a more isolated environment (e.g.,
> > > your company server that is serving only internal repos to
> > > employees). But I have misgivings about a feature that lets people
> > > intentionally create repositories whose history cannot ever interact
> > > with other users who haven't set a special config flag. It's one
> > > thing to say "to take advantage of this feature, we must all agree
> > > to have version X, or set flag Y". But it's another to bake that
> > > restriction into the repository history for all time.
> >
> > If people want a pre-prepared repository propagated to CI environment
> > and keep trakc of the state of such repository over time, for example,
> > they can use (versioned) tarballs.  Such a tarball won't automatically
> > get extracted after "git pull" (which is a feature), but those who
> > want such a pre-prepared repository for CI can make the extraction
> > step as a part of their CI build procedure.
> 
> Yeah, I almost went into more detail there. There are lots of solutions that
> make accessing an embedded sub-repository only one command away for
> the person who clones. :)  Some others are:
> 
>   - just call it "foo.git", and "mv foo.git .git" solves it (you'd
>     probably want to "git checkout -f" after that, but even if it were
>     embedded it seems silly to hold the data in two separate formats
>     anyway
> 
>   - just hold a bare repository ("foo.git") and then clone it

> On Wed, Aug 19, 2020 at 01:32:28PM -0700, Junio C Hamano wrote:
>
> > Jeff King <peff@peff.net> writes:
> >
> > > It _could_ still be useful in a more isolated environment (e.g., your
> > > company server that is serving only internal repos to employees). But I
> > > have misgivings about a feature that lets people intentionally create
> > > repositories whose history cannot ever interact with other users who
> > > haven't set a special config flag. It's one thing to say "to take
> > > advantage of this feature, we must all agree to have version X, or set
> > > flag Y". But it's another to bake that restriction into the repository
> > > history for all time.
> >
> > If people want a pre-prepared repository propagated to CI
> > environment and keep trakc of the state of such repository over
> > time, for example, they can use (versioned) tarballs.  Such a
> > tarball won't automatically get extracted after "git pull" (which
> > is a feature), but those who want such a pre-prepared repository
> > for CI can make the extraction step as a part of their CI build
> > procedure.
>
> Yeah, I almost went into more detail there. There are lots of solutions
> that make accessing an embedded sub-repository only one command away for
> the person who clones. :)  Some others are:
>
>   - just call it "foo.git", and "mv foo.git .git" solves it (you'd
>     probably want to "git checkout -f" after that, but even if it were
>     embedded it seems silly to hold the data in two separate formats
>     anyway
>
>   - just hold a bare repository ("foo.git") and then clone it
>
> etc. I think this is really a solution in search of a problem.

> On Wed, Aug 19, 2020 at 01:32:28PM -0700, Junio C Hamano wrote:
> 
> > Jeff King <peff@peff.net> writes:
> >   
> > > It _could_ still be useful in a more isolated environment (e.g., your
> > > company server that is serving only internal repos to employees). But I
> > > have misgivings about a feature that lets people intentionally create
> > > repositories whose history cannot ever interact with other users who
> > > haven't set a special config flag. It's one thing to say "to take
> > > advantage of this feature, we must all agree to have version X, or set
> > > flag Y". But it's another to bake that restriction into the repository
> > > history for all time.  
> > 
> > If people want a pre-prepared repository propagated to CI
> > environment and keep trakc of the state of such repository over
> > time, for example, they can use (versioned) tarballs.  Such a
> > tarball won't automatically get extracted after "git pull" (which
> > is a feature), but those who want such a pre-prepared repository
> > for CI can make the extraction step as a part of their CI build
> > procedure.  
> 
> Yeah, I almost went into more detail there. There are lots of solutions
> that make accessing an embedded sub-repository only one command away for
> the person who clones. :)  Some others are:
> 
>   - just call it "foo.git", and "mv foo.git .git" solves it (you'd
>     probably want to "git checkout -f" after that, but even if it were
>     embedded it seems silly to hold the data in two separate formats
>     anyway
> 
>   - just hold a bare repository ("foo.git") and then clone it
> 
> etc. I think this is really a solution in search of a problem.
> 
> -Peff

> Yes, there are many workarounds and they work well in the CI usecase. However,
> for the arbitrary files usecase there is no good workaround. I currently use
> a script which iterates over the tree and renames .git -> dotgit before running
> any git command and back again afterwards, but it is slow and brittle. I toyed
> with the idea of writing a FUSE filesystem to do the renaming, but it is
> needlessly complex and hurts performance.
> 
> Really, this problem should be solved in git itself.

> On Thu, Aug 20, 2020 at 01:34:45PM +0200, Lukas Straub wrote:
> 
> > Yes, there are many workarounds and they work well in the CI usecase. However,
> > for the arbitrary files usecase there is no good workaround. I currently use
> > a script which iterates over the tree and renames .git -> dotgit before running
> > any git command and back again afterwards, but it is slow and brittle. I toyed
> > with the idea of writing a FUSE filesystem to do the renaming, but it is
> > needlessly complex and hurts performance.
> > 
> > Really, this problem should be solved in git itself.  
> 
> It is unclear to me why need to hold many sub-repositories within the
> parent one, nor what Git operations you expect to perform over them. And
> what disadvantages your script solution has.
> 
> Perhaps you can give a more concrete use-case (but before you spend a
> lot of time doing so, I'll warn you that I find it pretty unlikely that
> it will cross the bar necessary to counter the downsides we've discussed
> so far).
> 
> -Peff

> On Thu, 20 Aug 2020 09:01:25 -0400
> Jeff King <peff@peff.net> wrote:
> 
> > On Thu, Aug 20, 2020 at 01:34:45PM +0200, Lukas Straub wrote:
> >
> > > Yes, there are many workarounds and they work well in the CI
> > > usecase. However, for the arbitrary files usecase there is no good
> > > workaround. I currently use a script which iterates over the tree
> > > and renames .git -> dotgit before running any git command and back
> > > again afterwards, but it is slow and brittle. I toyed with the idea
> > > of writing a FUSE filesystem to do the renaming, but it is needlessly
> complex and hurts performance.
> > >
> > > Really, this problem should be solved in git itself.
> >
> > It is unclear to me why need to hold many sub-repositories within the
> > parent one, nor what Git operations you expect to perform over them.
> > And what disadvantages your script solution has.
> >
> > Perhaps you can give a more concrete use-case (but before you spend a
> > lot of time doing so, I'll warn you that I find it pretty unlikely
> > that it will cross the bar necessary to counter the downsides we've
> > discussed so far).
> >
> > -Peff
> 
> I store all my files in several git(-annex) repositories. By "files" I

> sync everything (including WIPs) across my machines. I tried alternative
> workflows and they didn't work out.
> 
> In both cases, the script introduces a delay of ~10 seconds to every git
> command I run in the outer repository and I have to remember to use the
> script on these repos.
> Moreover, I can't use the git-annex assistant, which watches the repo and
> automatically commits and syncs file changes.
> In my opinion, I want to be able to set the allow-dotgit config and it'll

> On 2020-08-21 at 12:39:41, Lukas Straub wrote:
> > The downsides we discussed don't apply in this usecase. These are mostly
> > personal files, so I wont upload them to any hosting site (not even private
> > ones). There is no security impact as I only sync with trusted devices.  
> 
> I realize this works for you, but in general Git's security model does
> not permit untrusted configuration files or hooks.  Configuration can
> have numerous different commands that Git may execute and it is not, in
> general, safe to share across users.  This is why Git does not provide a
> way to sync whole repositories, only the objects within them.
> 
> Adding the ability to transport configuration through a repository is a
> security problem because it allows an attacker to potentially execute
> arbitrary code on the user's machine, and I can tell you that many, many
> people do clone untrusted repositories.  Just because you are aware of
> the risks, are comfortable with them, and are the only user in this
> scenario does not mean that this feature is a prudent one to add to Git.
> It violates our own security model, and as such, isn't a feature we're
> going to want to add.

> I want to be clear that it is not that we don't see your use case as
> valuable or important, only that we can't see a way to implement it
> securely as proposed.  Warning users unfortunately isn't sufficient
> because users tend not to read documentation.
> 
> Multiple core contributors representing various aspects of the Git
> community have weighed in, and it looks like the answer is unanimous.
> 
> Sorry for the bad news.

> On Fri, 21 Aug 2020 22:52:37 +0000
> "brian m. carlson" <sandals@crustytoothpaste.net> wrote:
> 
> > On 2020-08-21 at 12:39:41, Lukas Straub wrote:
> > > The downsides we discussed don't apply in this usecase. These are mostly
> > > personal files, so I wont upload them to any hosting site (not even private
> > > ones). There is no security impact as I only sync with trusted devices.  
> > 
> > I realize this works for you, but in general Git's security model does
> > not permit untrusted configuration files or hooks.  Configuration can
> > have numerous different commands that Git may execute and it is not, in
> > general, safe to share across users.  This is why Git does not provide a
> > way to sync whole repositories, only the objects within them.
> > 
> > Adding the ability to transport configuration through a repository is a
> > security problem because it allows an attacker to potentially execute
> > arbitrary code on the user's machine, and I can tell you that many, many
> > people do clone untrusted repositories.  Just because you are aware of
> > the risks, are comfortable with them, and are the only user in this
> > scenario does not mean that this feature is a prudent one to add to Git.
> > It violates our own security model, and as such, isn't a feature we're
> > going to want to add.
> 
> I don't understand. If the attacker gets the user to set git config options,
> then all hope is lost anyways, no?

> On 2020-08-22 at 14:21:52, Lukas Straub wrote:
> > On Fri, 21 Aug 2020 22:52:37 +0000
> > "brian m. carlson" <sandals@crustytoothpaste.net> wrote:
> >   
> > > On 2020-08-21 at 12:39:41, Lukas Straub wrote:  
> > > > The downsides we discussed don't apply in this usecase. These are mostly
> > > > personal files, so I wont upload them to any hosting site (not even private
> > > > ones). There is no security impact as I only sync with trusted devices.    
> > > 
> > > I realize this works for you, but in general Git's security model does
> > > not permit untrusted configuration files or hooks.  Configuration can
> > > have numerous different commands that Git may execute and it is not, in
> > > general, safe to share across users.  This is why Git does not provide a
> > > way to sync whole repositories, only the objects within them.
> > > 
> > > Adding the ability to transport configuration through a repository is a
> > > security problem because it allows an attacker to potentially execute
> > > arbitrary code on the user's machine, and I can tell you that many, many
> > > people do clone untrusted repositories.  Just because you are aware of
> > > the risks, are comfortable with them, and are the only user in this
> > > scenario does not mean that this feature is a prudent one to add to Git.
> > > It violates our own security model, and as such, isn't a feature we're
> > > going to want to add.  
> > 
> > I don't understand. If the attacker gets the user to set git config options,
> > then all hope is lost anyways, no?  
> 
> When you can embed repositories in other repositories like you're
> proposing, those embedded repositories can have configuration files in
> them (e.g., .git/config), which leads to the security problem.

> On Sat, 22 Aug 2020 18:53:07 +0000
> "brian m. carlson" <sandals@crustytoothpaste.net> wrote:
>
> > On 2020-08-22 at 14:21:52, Lukas Straub wrote:
> > > On Fri, 21 Aug 2020 22:52:37 +0000
> > > "brian m. carlson" <sandals@crustytoothpaste.net> wrote:
> > >
> > > > On 2020-08-21 at 12:39:41, Lukas Straub wrote:
> > > > > The downsides we discussed don't apply in this usecase. These are mostly
> > > > > personal files, so I wont upload them to any hosting site (not even private
> > > > > ones). There is no security impact as I only sync with trusted devices.
> > > >
> > > > I realize this works for you, but in general Git's security model does
> > > > not permit untrusted configuration files or hooks.  Configuration can
> > > > have numerous different commands that Git may execute and it is not, in
> > > > general, safe to share across users.  This is why Git does not provide a
> > > > way to sync whole repositories, only the objects within them.
> > > >
> > > > Adding the ability to transport configuration through a repository is a
> > > > security problem because it allows an attacker to potentially execute
> > > > arbitrary code on the user's machine, and I can tell you that many, many
> > > > people do clone untrusted repositories.  Just because you are aware of
> > > > the risks, are comfortable with them, and are the only user in this
> > > > scenario does not mean that this feature is a prudent one to add to Git.
> > > > It violates our own security model, and as such, isn't a feature we're
> > > > going to want to add.
> > >
> > > I don't understand. If the attacker gets the user to set git config options,
> > > then all hope is lost anyways, no?
> >
> > When you can embed repositories in other repositories like you're
> > proposing, those embedded repositories can have configuration files in
> > them (e.g., .git/config), which leads to the security problem.
>
> Yes, I understand that, but the user has to actively allow this via the
> allowDotGit config option, which I'll implement in the next patch version.
> So the attacker has to get the user to set the option. If the user does this,
> the attacker could get the user to set any other option (like core.gitProxy)
> anyway and gain remote execution regardless of this patch.

> On Wed, Aug 19, 2020 at 12:09:11PM -0700, Junio C Hamano wrote:
> 
> > "Randall S. Becker" <rsbecker@nexbridge.com> writes:
> >   
> > > Just putting my CSIO hat on here. We would need a system-wide setting to
> > > prohibit users from using this capability.  
> > 
> > Or just discard this patch, which is a lot simpler.  I don't see any
> > need for this one.  
> 
> Yes. Configurability is a lot more complicated than you might think.
> Because it's not just system-wide, but _ecosystem_ wide.
> 
> Right now git-fsck complains about ".git" appearing in a tree, and that
> check blocks people from pushing such trees to any hosting sites that
> enable transfer.fsckObjects (which includes hosters like GitHub). So
> you'd not only need to allow the behavior to be loosened for all of the
> people using the feature, but you'd need to convince server-side hosters
> to loosen their config. And because part of the purpose is to protect
> downstream clients from attacks, I doubt that public hosters like GitHub
> would do so.

> It _could_ still be useful in a more isolated environment (e.g., your
> company server that is serving only internal repos to employees). But I
> have misgivings about a feature that lets people intentionally create
> repositories whose history cannot ever interact with other users who
> haven't set a special config flag. It's one thing to say "to take
> advantage of this feature, we must all agree to have version X, or set
> flag Y". But it's another to bake that restriction into the repository
> history for all time.

> > Right now git-fsck complains about ".git" appearing in a tree, and that
> > check blocks people from pushing such trees to any hosting sites that
> > enable transfer.fsckObjects (which includes hosters like GitHub). So
> > you'd not only need to allow the behavior to be loosened for all of the
> > people using the feature, but you'd need to convince server-side hosters
> > to loosen their config. And because part of the purpose is to protect
> > downstream clients from attacks, I doubt that public hosters like GitHub
> > would do so.
> 
> I guess they can add a checkbox to their (secured) web-ui to configure
> this.

> On August 19, 2020 2:04 PM, Junio C Hamano
> > To: Lukas Straub <lukasstraub2@web.de>
> > Cc: git <git@vger.kernel.org>; Elijah Newren <newren@gmail.com>;
> > Brandon Williams <bwilliams.eng@gmail.com>; Johannes Schindelin
> > <Johannes.Schindelin@gmx.de>; Jeff King <peff@peff.net>
> > Subject: Re: [RFC PATCH 0/2] Allow adding .git files and directories
> > 
> > Lukas Straub <lukasstraub2@web.de> writes:
> >   
> > > These patches allow this and work well in a quick test. Of course some
> > > tests fail because with this the handling of nested git repos changed.  
> > 
> > In other words, this breaks the workflow existing users rely on, right?  I  
> do
> > not know if such a behaviour ever needs to exist even as an opt-in  
> feature,
> > but it definitely feels wrong to make the behaviour these patches  
> introduce
> > the default.  
> 
> I am concerned about broader implications. I might be stating the obvious,
> but a key security vulnerability that would open up here is to put contents
> of files like .git/config into a repository. This capability would allow
> scripts to be introduced without the explicit knowledge of the user. While
> I'm sure some of the heavy clean/smudge users might appreciate it, this can
> represent a vector for the introduction of hostile code into an environment.
> While this enhancement seems like a good idea on the surface, if it goes
> forward, it should not be the default and should not be under the control of
> the upstream repository. You would need loads of warnings about potential
> script hazards at the very least presented to the user, beyond what is
> already documented in git. This change would not interoperate with JGit -
> not that that is a huge concern here, but heavy Jenkins and other pipeline
> users could be significantly impacted.
> 
> Just putting my CSIO hat on here. We would need a system-wide setting to
> prohibit users from using this capability.

> Sincerely,
> Randall
> 
> -- Brief whoami:
>  NonStop developer since approximately 211288444200000000
>  UNIX developer since approximately 421664400
> -- In my real life, I talk too much.
> 
> 
> 

> Lukas Straub <lukasstraub2@web.de> writes:
> 
> > These patches allow this and work well in a quick test. Of course some tests
> > fail because with this the handling of nested git repos changed.  
> 
> In other words, this breaks the workflow existing users rely on,
> right?  I do not know if such a behaviour ever needs to exist even
> as an opt-in feature, but it definitely feels wrong to make the
> behaviour these patches introduce the default.

> To: Junio C Hamano <gitster@pobox.com>
> Cc: git <git@vger.kernel.org>; Elijah Newren <newren@gmail.com>;
> Brandon Williams <bwilliams.eng@gmail.com>; Johannes Schindelin
> <Johannes.Schindelin@gmx.de>; Jeff King <peff@peff.net>
> Subject: Re: [RFC PATCH 0/2] Allow adding .git files and directories
> 
> On Wed, 19 Aug 2020 11:03:30 -0700
> Junio C Hamano <gitster@pobox.com> wrote:
> 
> > Lukas Straub <lukasstraub2@web.de> writes:
> >
> > > These patches allow this and work well in a quick test. Of course
> > > some tests fail because with this the handling of nested git repos
> changed.
> >
> > In other words, this breaks the workflow existing users rely on,
> > right?  I do not know if such a behaviour ever needs to exist even as
> > an opt-in feature, but it definitely feels wrong to make the behaviour
> > these patches introduce the default.
> 
> Well, the current behavior is that nested repos (that are not submodules)

> On August 19, 2020 2:48 PM, Lukas Straub wrote:
> > To: Junio C Hamano <gitster@pobox.com>
> > Cc: git <git@vger.kernel.org>; Elijah Newren <newren@gmail.com>;
> > Brandon Williams <bwilliams.eng@gmail.com>; Johannes Schindelin
> > <Johannes.Schindelin@gmx.de>; Jeff King <peff@peff.net>
> > Subject: Re: [RFC PATCH 0/2] Allow adding .git files and directories
> > 
> > On Wed, 19 Aug 2020 11:03:30 -0700
> > Junio C Hamano <gitster@pobox.com> wrote:
> >   
> > > Lukas Straub <lukasstraub2@web.de> writes:
> > >  
> > > > These patches allow this and work well in a quick test. Of course
> > > > some tests fail because with this the handling of nested git repos  
> > changed.  
> > >
> > > In other words, this breaks the workflow existing users rely on,
> > > right?  I do not know if such a behaviour ever needs to exist even as
> > > an opt-in feature, but it definitely feels wrong to make the behaviour
> > > these patches introduce the default.  
> > 
> > Well, the current behavior is that nested repos (that are not submodules)  
> are
> > completely ignored and none of the files within can be added. So the old
> > behavior can be restored with .gitignore. The same goes for files/dirs  
> named
> > .git.
> > 
> > Of course I don't know what the current policy for behavioral changes in  
> git
> > is, but I see that there have been such changes in the past.  
> 
> I honestly am concerned about a repeat of things like
> https://nvd.nist.gov/vuln/detail/CVE-2019-19604 (the submodule update
> problem). This change in behaviour is of serious concern from a risk
> standpoint. To be blunt, I don't think users on my platform will move to a
> version of git that supports this by default.

> Sincerely,
> Randall
>
> -- Brief whoami:
>  NonStop developer since approximately 211288444200000000
>  UNIX developer since approximately 421664400
> -- In my real life, I talk too much.
> 
> 
>