threads / bug / 59009

[BUG] fatal: transport 'file' not allowed during submodule add

Subject: [BUG] fatal: transport 'file' not allowed during submodule add

## tl;dr

12 messages between Dec 27, 2022 and Jan 3, 2023.

replies: 11people: 5as markdown or json

rsbecker@nexbridge.com· Dec 27, 2022, 23:00 UTC · lore

As of 2.39.0, I am now getting fatal: transport 'file' not allowed when performing a submodule add after a clone -l. The simple reproduce of this is:

1. Start with an empty bare repository, src.git.
2. Create an empty non-bare repository and set the upstream remote to the
bare repo.
3. Populate the non-bare repository with:
	touch .gitignore &&
	git add .gitignore &&
	touch file1 &&
	git add file1 &&
	git commit -m initial &&
	git remote add origin ../src.git &&
	git push --set-upstream origin master
4. Create another empty bare repository to be used as the submodule,
subsrc.git.
5. Create another empty non-bare repository and set the upstream remote to
the bare repo for the submodule.
6. Populate the non-bare submodule repository with
	touch .gitignore &&
	git add .gitignore &&
	git commit -m initial &&
	git add .gitignore &&
	touch file2 &&
	git add file2 &&
	git commit -m initial &&
	git remote add origin ../subsrc.git &&
	git push --set-upstream origin master
7. Clone the main repo using -l or without it (makes no difference):
	git clone -l src.git dest
8. Attempt to add the submodule:
	cd dest &&
	git submodule add -- ../subsrc.git subsrc

This results in: Cloning into 'dest'... done. Cloning into '/home/randall/dest/subsrc'... fatal: transport 'file' not allowed fatal: clone of '/home/randall/subsrc.git' into submodule path '/home/randall/dest/subsrc' failed

This happens for any submodule add on the same system. Some online research indicates that there was a security patch to git causing this, but I can't find it. This does not seem correct to me or how this improves security. Help please - this is causing some of my workflows to break.

Thanks, Randall -- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400) NonStop(211288444200000000) -- In real life, I talk too much.

Junio C Hamano· Dec 28, 2022, 03:34 UTC · re: rsbecker@nexbridge.com · lore

Re: [BUG] fatal: transport 'file' not allowed during submodule add

<rsbecker@nexbridge.com> writes:
Show 8 quoted lines
> As of 2.39.0, I am now getting fatal: transport 'file' not allowed when
> performing a submodule add after a clone -l. The simple reproduce of this
> is:
> ...
> This happens for any submodule add on the same system. Some online research
> indicates that there was a security patch to git causing this, but I can't
> find it. This does not seem correct to me or how this improves security.
> Help please - this is causing some of my workflows to break.
Thanks for reporting, Randall.

This suspiciously sounds like what a1d4f67c (transport: make `protocol.file.allow` be "user" by default, 2022-07-29) is doing deliberately. Taylor, does this look like a corner case the 2.30.6 updates forgot to consider?

Thanks.
rsbecker@nexbridge.com· Dec 28, 2022, 14:42 UTC · re: Junio C Hamano · lore

RE: [BUG] fatal: transport 'file' not allowed during submodule add

>-----Original Message-----
>From: Junio C Hamano <jch2355@gmail.com> On Behalf Of Junio C Hamano
On December 27, 2022 10:34 PM, Junio C Hamano wrote:
Show 10 quoted lines
><rsbecker@nexbridge.com> writes:
>
>> As of 2.39.0, I am now getting fatal: transport 'file' not allowed
>> when performing a submodule add after a clone -l. The simple reproduce
>> of this
>> is:
>> ...
>> This happens for any submodule add on the same system. Some online
>> research indicates that there was a security patch to git causing
>> this, but I can't find it. This does not seem correct to me or how this
improves
Show 6 quoted lines
>security.
>> Help please - this is causing some of my workflows to break.
>
>Thanks for reporting, Randall.
>
>This suspiciously sounds like what a1d4f67c (transport: make
`protocol.file.allow`
>be "user" by default, 2022-07-29) is doing deliberately.  Taylor, does this
look like a
>corner case the 2.30.6 updates forgot to consider?

I have tried using 'git config --local protocol.file.allow always' and/or 'git config --local protocol.allow always' to get past this, without success. 

--Randall
Jonathan Nieder· Dec 28, 2022, 22:10 UTC · re: rsbecker@nexbridge.com · lore

Re: [BUG] fatal: transport 'file' not allowed during submodule add

Hi Randall,
rsbecker@nexbridge.com wrote:
> Junio C Hamano wrote:
Show 6 quoted lines
>> This suspiciously sounds like what a1d4f67c (transport: make `protocol.file.allow`
>> be "user" by default, 2022-07-29) is doing deliberately.
>
> I have tried using 'git config --local protocol.file.allow always' and/or
> 'git config --local protocol.allow always' to get past this, without
> success.
Does `git config --global protocol.file.allow always` do the trick?
>>                                                           Taylor, does this look like a
>> corner case the 2.30.6 updates forgot to consider?

I think it's the intended effect (preventing file:// submodules), but I wonder if this hints that we'd want that protection to be more targeted. A file:// submodule (as opposed to a bare path without URL scheme) wouldn't trigger the "git clone --local" behavior that that commit mentions wanting to protect against, so at first glance it would appear to be no more or less dangerous than cloning from a remote repository.

One thing I'd be curious about is whether --local happening automatically is actually worth it nowadays. "git worktree" does a better job of sharing with an existing local repository, since the sharing continues even after the worktree has been created, after any "git gc" operations, and so on. Meanwhile, the distinction between file:// and bare paths is subtle enough that I regularly encounter people not being aware of it (for example when wanting a way to test protocol code locally and not understanding why a bare-path clone doesn't do that). Would it be more in the spirit of secure defaults to require --local when someone wants to request the hardlinking trick of local clone?

Thanks, Jonathan

rsbecker@nexbridge.com· Dec 28, 2022, 22:25 UTC · re: Jonathan Nieder · lore

RE: [BUG] fatal: transport 'file' not allowed during submodule add

On December 28, 2022 5:11 PM, Jonathan Nieder wrote:
Show 7 quoted lines
>Hi Randall,
>
>rsbecker@nexbridge.com wrote:
>> Junio C Hamano wrote:
>
>>> This suspiciously sounds like what a1d4f67c (transport: make
>>> `protocol.file.allow` be "user" by default, 2022-07-29) is doing
deliberately.
Show 6 quoted lines
>>
>> I have tried using 'git config --local protocol.file.allow always'
>> and/or 'git config --local protocol.allow always' to get past this,
>> without success.
>
>Does `git config --global protocol.file.allow always` do the trick?

I tried git config --local protocol.file.allow always after the initial clone. This should work but does not. I also tried git config --global protocol.file.allow always before the initial clone. This also did not work.

>>>                                                           Taylor,
>>> does this look like a corner case the 2.30.6 updates forgot to consider?
>
>I think it's the intended effect (preventing file:// submodules), but I
wonder if this
>hints that we'd want that protection to be more targeted.  A file://
submodule (as
>opposed to a bare path without URL
>scheme) wouldn't trigger the "git clone --local" behavior that that commit
>mentions wanting to protect against, so at first glance it would appear to
be no
>more or less dangerous than cloning from a remote repository.
>
>One thing I'd be curious about is whether --local happening automatically
is
>actually worth it nowadays.  "git worktree" does a better job of sharing
with an
>existing local repository, since the sharing continues even after the
worktree has
>been created, after any "git gc" operations, and so on.  Meanwhile, the
distinction
>between file:// and bare paths is subtle enough that I regularly encounter
people
>not being aware of it (for example when wanting a way to test protocol code
>locally and not understanding why a bare-path clone doesn't do that).
Would it be
>more in the spirit of secure defaults to require --local when someone wants
to
>request the hardlinking trick of local clone?

I think the risk of someone hacking a hardlink is less risky than someone misdirecting a remote site not under a user's direct control.

The tests I did show the same behaviour no matter which combination of the above. --local appears to be implied, at least there is no apparent behavioural difference between specifying the argument and not.

--Randall
Taylor Blau· Dec 30, 2022, 21:08 UTC · re: Jonathan Nieder · lore

Re: [BUG] fatal: transport 'file' not allowed during submodule add

Hi Jonathan,
On Wed, Dec 28, 2022 at 02:10:42PM -0800, Jonathan Nieder wrote:
Show 24 quoted lines
> Hi Randall,
>
> rsbecker@nexbridge.com wrote:
> > Junio C Hamano wrote:
>
> >> This suspiciously sounds like what a1d4f67c (transport: make `protocol.file.allow`
> >> be "user" by default, 2022-07-29) is doing deliberately.
> >
> > I have tried using 'git config --local protocol.file.allow always' and/or
> > 'git config --local protocol.allow always' to get past this, without
> > success.
>
> Does `git config --global protocol.file.allow always` do the trick?
>
> >>                                                           Taylor, does this look like a
> >> corner case the 2.30.6 updates forgot to consider?
>
> I think it's the intended effect (preventing file:// submodules), but
> I wonder if this hints that we'd want that protection to be more
> targeted.  A file:// submodule (as opposed to a bare path without URL
> scheme) wouldn't trigger the "git clone --local" behavior that that
> commit mentions wanting to protect against, so at first glance it
> would appear to be no more or less dangerous than cloning from a
> remote repository.

Changing the default value of 'protocol.file.allow' isn't solely about whether or not we use the `file://` scheme and transport or not. Instead, it's about preventing the user from accidentally cloning local repositories containing sensitive data into the working copy of a malicious repository.

One example might be that I convince you to clone my malicious repository, which has a Dockerfile that uploads everything in the container filesystem to some data harvesting server. Since 'docker run' automatically puts everything in '.' into the volume mount, anything in the working copy of my malicious repository will get exfiltrated.

The worry that I wrote about in a1d4f67c was that if I knew that you stored, say, your SSH private key material in a repository that is at `$HOME/.git` (as is sometimes common practice), then I could add a submodule at /home/jrnieder/.git, and extract any sensitive data therein.

So I think our new default is sensible here if we are concerned with preventing such a case.

Thanks, Taylor

rsbecker@nexbridge.com· Dec 30, 2022, 21:48 UTC · re: Taylor Blau · lore

RE: [BUG] fatal: transport 'file' not allowed during submodule add

On December 30, 2022 4:08 PM, Taylor Blau wrote:
Show 44 quoted lines
>On Wed, Dec 28, 2022 at 02:10:42PM -0800, Jonathan Nieder wrote:
>> Hi Randall,
>>
>> rsbecker@nexbridge.com wrote:
>> > Junio C Hamano wrote:
>>
>> >> This suspiciously sounds like what a1d4f67c (transport: make
>> >> `protocol.file.allow` be "user" by default, 2022-07-29) is doing deliberately.
>> >
>> > I have tried using 'git config --local protocol.file.allow always'
>> > and/or 'git config --local protocol.allow always' to get past this,
>> > without success.
>>
>> Does `git config --global protocol.file.allow always` do the trick?
>>
>> >>                                                           Taylor,
>> >> does this look like a corner case the 2.30.6 updates forgot to consider?
>>
>> I think it's the intended effect (preventing file:// submodules), but
>> I wonder if this hints that we'd want that protection to be more
>> targeted.  A file:// submodule (as opposed to a bare path without URL
>> scheme) wouldn't trigger the "git clone --local" behavior that that
>> commit mentions wanting to protect against, so at first glance it
>> would appear to be no more or less dangerous than cloning from a
>> remote repository.
>
>Changing the default value of 'protocol.file.allow' isn't solely about whether or not
>we use the `file://` scheme and transport or not. Instead, it's about preventing the
>user from accidentally cloning local repositories containing sensitive data into the
>working copy of a malicious repository.
>
>One example might be that I convince you to clone my malicious repository, which
>has a Dockerfile that uploads everything in the container filesystem to some data
>harvesting server. Since 'docker run'
>automatically puts everything in '.' into the volume mount, anything in the working
>copy of my malicious repository will get exfiltrated.
>
>The worry that I wrote about in a1d4f67c was that if I knew that you stored, say,
>your SSH private key material in a repository that is at `$HOME/.git` (as is
>sometimes common practice), then I could add a submodule at
>/home/jrnieder/.git, and extract any sensitive data therein.
>
>So I think our new default is sensible here if we are concerned with preventing
>such a case.
I think the new default is reasonable but this did catch me by surprise as it broke our workflows. I guess I need to look at the release notes in more depth - that's my bad. With the caveat that I do not think this is working as intended, which I am finding, because changing the configuration does not make any behavioural difference on any platform I can test on.
Jeff King· Jan 3, 2023, 08:57 UTC · re: Taylor Blau · lore

Re: [BUG] fatal: transport 'file' not allowed during submodule add

On Fri, Dec 30, 2022 at 04:08:03PM -0500, Taylor Blau wrote:
Show 20 quoted lines
> Changing the default value of 'protocol.file.allow' isn't solely about whether
> or not we use the `file://` scheme and transport or not. Instead, it's
> about preventing the user from accidentally cloning local repositories
> containing sensitive data into the working copy of a malicious
> repository.
> 
> One example might be that I convince you to clone my malicious
> repository, which has a Dockerfile that uploads everything in the
> container filesystem to some data harvesting server. Since 'docker run'
> automatically puts everything in '.' into the volume mount, anything in
> the working copy of my malicious repository will get exfiltrated.
> 
> The worry that I wrote about in a1d4f67c was that if I knew that you
> stored, say, your SSH private key material in a repository that is at
> `$HOME/.git` (as is sometimes common practice), then I could add a
> submodule at /home/jrnieder/.git, and extract any sensitive data
> therein.
> 
> So I think our new default is sensible here if we are concerned with
> preventing such a case.

One of the justifications for disallowing filesystem URLs in submodules is that they're generally a bad idea anyway. If you're cloning over the network, there's nothing to guarantee that the cloner's filesystem looks anything like the original committer's. So it's an accident waiting to happen.

But one case where it's not _completely_ unreasonable is when the superproject clone itself is happening via the local filesystem. That matches Randall's example, though I'm not sure if that's his "real" workflow.

So one thing we could do to soften the change is to allow a submodule to use a filesystem URL if and only if the immediate parent clone also did (and naturally, allow "submodule add" on anything). But:

  - it's not clear to me if that just re-opens the Docker problem you
    were trying to fix (I think not, because it should be forbidding
    file:// remotes for the initial clone, too; but...)
  - as a security rule, it's complicated and confusing, which means it
    is likely to have loopholes or be mis-used. As a user I now have to
    be aware that copying an untrusted .git to my filesystem and cloning
    from it has a different trust level than cloning it over https, with
    respect to submodule protocols.
  - in a distributed system, the notion of "the parent clone" is vague
    anyway. During "git clone --recurse-submodules", sure, you can use
    the top-level URL as your basis. But later if I run "git submodule
    update", what's the "original" protocol? I can guess at it by
    looking at remote.origin.url, but of course it could change, there
    could be multiple remotes, etc.

So I think it's probably not worth doing. People with that workflow should set protocol.file.allow as appropriate.

I also wondered if you could get around this with url.*.insteadOf. That is, it would seem reasonable to use a unique URL in .gitmodules (even if it's not a real functional URL!), and then let local developers override it to point to their filesystem with the insteadOf mechanism. That gives tighter permissions, and provides more options for local redirection.

Unfortunately, it doesn't work here. We check the protocol permissions as we're about to use them (which is good, because we catch all paths that get there). But at that point we have no idea about the rewrite. Looks like there was some discussion in this thread:

  https://lore.kernel.org/git/CAPZ477MCsBsfbqKzp69MT_brwz-0aes6twJofQrhizUBV7ZoeA@mail.gmail.com/

about having a rewrite make the URL "from the user". But ultimately it seems scary. And anyway, it doesn't help this case much anyway (people still need to set up config, so they might as well just set the protocol config).

-Peff
Taylor Blau· Dec 30, 2022, 21:04 UTC · re: rsbecker@nexbridge.com · lore

Re: [BUG] fatal: transport 'file' not allowed during submodule add

On Wed, Dec 28, 2022 at 09:42:39AM -0500, rsbecker@nexbridge.com wrote:
Show 30 quoted lines
>
>
> >-----Original Message-----
> >From: Junio C Hamano <jch2355@gmail.com> On Behalf Of Junio C Hamano
> On December 27, 2022 10:34 PM, Junio C Hamano wrote:
> ><rsbecker@nexbridge.com> writes:
> >
> >> As of 2.39.0, I am now getting fatal: transport 'file' not allowed
> >> when performing a submodule add after a clone -l. The simple reproduce
> >> of this
> >> is:
> >> ...
> >> This happens for any submodule add on the same system. Some online
> >> research indicates that there was a security patch to git causing
> >> this, but I can't find it. This does not seem correct to me or how this
> improves
> >security.
> >> Help please - this is causing some of my workflows to break.
> >
> >Thanks for reporting, Randall.
> >
> >This suspiciously sounds like what a1d4f67c (transport: make
> `protocol.file.allow`
> >be "user" by default, 2022-07-29) is doing deliberately.  Taylor, does this
> look like a
> >corner case the 2.30.6 updates forgot to consider?
>
> I have tried using 'git config --local protocol.file.allow always' and/or
> 'git config --local protocol.allow always' to get past this, without
> success.

I couldn't reproduce the symptom you described. Indeed, the behavior of not allowing local-submodules to be cloned without explicitly opting in via the `protocol.file.allow` configuration is intentional.

The patch Junio mentioned, a1d4f67c12 (transport: make `protocol.file.allow` be "user" by default, 2022-07-29) has some examples of why this behavior was changed in the 2.30.6 update.

If you run either `git config --global protocol.file.allow always`, or replace your last submodule add with:

  $ git -c protocol.file.allow=always submodule add /path/to/subsrc.git
it should work as expected.

Thanks, Taylor

rsbecker@nexbridge.com· Dec 30, 2022, 21:43 UTC · re: Taylor Blau · lore

RE: [BUG] fatal: transport 'file' not allowed during submodule add

On December 30, 2022 4:04 PM, Taylor Blau wrote:
Show 46 quoted lines
>On Wed, Dec 28, 2022 at 09:42:39AM -0500, rsbecker@nexbridge.com wrote:
>> >-----Original Message-----
>> >From: Junio C Hamano <jch2355@gmail.com> On Behalf Of Junio C Hamano
>> On December 27, 2022 10:34 PM, Junio C Hamano wrote:
>> ><rsbecker@nexbridge.com> writes:
>> >
>> >> As of 2.39.0, I am now getting fatal: transport 'file' not allowed
>> >> when performing a submodule add after a clone -l. The simple
>> >> reproduce of this
>> >> is:
>> >> ...
>> >> This happens for any submodule add on the same system. Some online
>> >> research indicates that there was a security patch to git causing
>> >> this, but I can't find it. This does not seem correct to me or how
>> >> this
>> improves
>> >security.
>> >> Help please - this is causing some of my workflows to break.
>> >
>> >Thanks for reporting, Randall.
>> >
>> >This suspiciously sounds like what a1d4f67c (transport: make
>> `protocol.file.allow`
>> >be "user" by default, 2022-07-29) is doing deliberately.  Taylor,
>> >does this
>> look like a
>> >corner case the 2.30.6 updates forgot to consider?
>>
>> I have tried using 'git config --local protocol.file.allow always'
>> and/or 'git config --local protocol.allow always' to get past this,
>> without success.
>
>I couldn't reproduce the symptom you described. Indeed, the behavior of not
>allowing local-submodules to be cloned without explicitly opting in via the
>`protocol.file.allow` configuration is intentional.
>
>The patch Junio mentioned, a1d4f67c12 (transport: make `protocol.file.allow` be
>"user" by default, 2022-07-29) has some examples of why this behavior was
>changed in the 2.30.6 update.
>
>If you run either `git config --global protocol.file.allow always`, or replace your last
>submodule add with:
>
>  $ git -c protocol.file.allow=always submodule add /path/to/subsrc.git
>
>it should work as expected.
I have reproduced this on multiple platforms including NonStop and Cygwin64 on Windows with the same results as earlier. The protocol.file.allowed=always does not appear to even get considered. With some fprintfs in the code, the code in is_transport_allowed falls through to the PROTOCOL_ALLOW_USER_ONLY case and only considers environment variable GIT_PROTOCOL_FROM_USER, which is not passed into the child doing the submodule add. The is_transport_allowed("file",-1) always returns 0 no matter what and 0 is what gets used upwards. There is no difference in the behaviour regardless of the protocol.file.allowed value either in -c, .gitconfig, or on the user environment variable.
rsbecker@nexbridge.com· Dec 30, 2022, 23:16 UTC · re: Taylor Blau · lore

RE: [BUG] fatal: transport 'file' not allowed during submodule add

On December 30, 2022 4:04 PM, Taylor Blau wrote:
Show 45 quoted lines
>On Wed, Dec 28, 2022 at 09:42:39AM -0500, rsbecker@nexbridge.com wrote:
>> >From: Junio C Hamano <jch2355@gmail.com> On Behalf Of Junio C Hamano
>> On December 27, 2022 10:34 PM, Junio C Hamano wrote:
>> ><rsbecker@nexbridge.com> writes:
>> >
>> >> As of 2.39.0, I am now getting fatal: transport 'file' not allowed
>> >> when performing a submodule add after a clone -l. The simple
>> >> reproduce of this
>> >> is:
>> >> ...
>> >> This happens for any submodule add on the same system. Some online
>> >> research indicates that there was a security patch to git causing
>> >> this, but I can't find it. This does not seem correct to me or how
>> >> this
>> improves
>> >security.
>> >> Help please - this is causing some of my workflows to break.
>> >
>> >Thanks for reporting, Randall.
>> >
>> >This suspiciously sounds like what a1d4f67c (transport: make
>> `protocol.file.allow`
>> >be "user" by default, 2022-07-29) is doing deliberately.  Taylor,
>> >does this
>> look like a
>> >corner case the 2.30.6 updates forgot to consider?
>>
>> I have tried using 'git config --local protocol.file.allow always'
>> and/or 'git config --local protocol.allow always' to get past this,
>> without success.
>
>I couldn't reproduce the symptom you described. Indeed, the behavior of not
>allowing local-submodules to be cloned without explicitly opting in via the
>`protocol.file.allow` configuration is intentional.
>
>The patch Junio mentioned, a1d4f67c12 (transport: make `protocol.file.allow` be
>"user" by default, 2022-07-29) has some examples of why this behavior was
>changed in the 2.30.6 update.
>
>If you run either `git config --global protocol.file.allow always`, or replace your last
>submodule add with:
>
>  $ git -c protocol.file.allow=always submodule add /path/to/subsrc.git
>
>it should work as expected.
Ok, operator error. This does work as expected if you run the test slightly different. If the repositories are all cloned, the upstream remotes are set up properly and the submodule add works. In my test case, I used git remote add with a relative path. This seems to be an edge condition that triggers the situation. When avoiding the upstream remote command (implied by clone), the submodule add works if the protocol.file.allow = always, no matter how it is set.
--Randall
rsbecker@nexbridge.com· Dec 30, 2022, 20:15 UTC · re: Junio C Hamano · lore

RE: [BUG] fatal: transport 'file' not allowed during submodule add

On December 27, 2022 10:34 PM, Junio C Hamano wrote:
Show 10 quoted lines
><rsbecker@nexbridge.com> writes:
>
>> As of 2.39.0, I am now getting fatal: transport 'file' not allowed
>> when performing a submodule add after a clone -l. The simple reproduce
>> of this
>> is:
>> ...
>> This happens for any submodule add on the same system. Some online
>> research indicates that there was a security patch to git causing
>> this, but I can't find it. This does not seem correct to me or how this
improves
Show 6 quoted lines
>security.
>> Help please - this is causing some of my workflows to break.
>
>Thanks for reporting, Randall.
>
>This suspiciously sounds like what a1d4f67c (transport: make
`protocol.file.allow`
>be "user" by default, 2022-07-29) is doing deliberately.  Taylor, does this
look like a
>corner case the 2.30.6 updates forgot to consider?

Any updates on this? Neither protocol.file.allow=always nor protocol.allow=always gets past the error condition.

Thanks, Randall

← back to recent threads