Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: Pass the target module to GOPROXY #27133

Closed
marwan-at-work opened this issue Aug 21, 2018 · 7 comments

Comments

Projects
None yet
6 participants
@marwan-at-work
Copy link
Contributor

commented Aug 21, 2018

Summary

The Go Command should send a custom header to the GOPROXY server letting it know what the target module is.

For example if you have the following go.mod file:

module github.com/NYTimes/gziphandler

require github.com/pkg/errors v0.8.0

Then when the users issues a mod-enabled command such as go get, the Go command should send the following header to all of the Download Protocol requests:

x-go-target-module: github.com/NYTimes/gziphandler

This means the GOPROXY server can have access logic based on what you're trying to build. For example, only modules starting with github.com/NYTimes can use the Proxy to build their dependencies.

Please answer these questions before submitting your issue. Thanks!

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

go1.11-rc

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/208581/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/208581/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/n4/35bgdnkd6vlgqrdzbyl0x1ycmhcndx/T/go-build555238269=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

GOPROXY=http://localhost:3000 go build inside a go module.

What did you expect to see?

I expect a header to let the proxy know which module the Go command is actually trying to build.

@thepudds

This comment has been minimized.

Copy link

commented Aug 22, 2018

@gopherbot, please add label modules

@gopherbot gopherbot added the modules label Aug 22, 2018

@balasanjay

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2018

I'm not necessarily against this, but the use-case seems a bit unconvincing to me.

The proxy cannot trust the value from the client, so it provides no appreciable security per se. After all, I could trivially write my own meta-proxy that sets this header to a constant and otherwise forwards on the request to the NYTimes proxy. This seems to limit the value to the server, atleast compared to somehow authenticating the client.

And since the goal appears to be to have a default-enabled set of public proxies, then I don't think we should require disclosing any unnecessary information to the proxy.

@bcmills

This comment has been minimized.

Copy link
Member

commented Oct 30, 2018

This seems a bit redundant with /internal/ path components. How would the two interact?

@marwan-at-work

This comment has been minimized.

Copy link
Contributor Author

commented Nov 15, 2018

@bcmills Not sure how to achieve the same results with /internal/.

For example, if I have a module (say github.com/marwan/secret), and I only want 3 other modules to be able to import it (github.com/bcmills/client, github.com/other/client, and gopkg.in/secret-client)

An attacker can try to ping my GOPROXY server to download github.com/marwan/secret, but they will never be able to get it unless they have a header that has one of those three modules.

I understand the use case is a bit weird, but I thought I'd suggest it to see if there's a valuable use case here.

@bcmills

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

I can't think of a threat model that this would help with: if someone can guess your module path, they can probably guess an importer path too.

(But CC: @FiloSottile for threat-model expertise.)

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

This does not pass muster as a security feature, because there is no reason to believe the client is not lying. If what you want is authentication, you'll need a real authentication protocol.

As a generic feature, I wouldn't ever want this to encourage a proxy to serve different content based on the target module, because the cache is shared and the security systems we are building assume that module version contents never change and are universal.

By the way, the cache breaks your intended use case as well, because once a module is fetched it can be used anywhere. I am not inclined to accept this, but I am not the decision maker.

@bcmills

This comment has been minimized.

Copy link
Member

commented Nov 17, 2018

Thanks for the input, Filippo. We'll decline this one.

@bcmills bcmills closed this Nov 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.