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

From: Jeff King <peff@peff.net>
Date: Jan 3, 2023, 08:57 UTC
Message-ID: <Y7Pt7R0VX3kGI5Dc@coredump.intra.peff.net>
In-Reply-To: <Y69TMzIf/bdsZe6/@nand.local>

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