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: GOPROXY credentials exposed in case of errors #30610

Closed
leitzler opened this issue Mar 5, 2019 · 7 comments
Closed

cmd/go: GOPROXY credentials exposed in case of errors #30610

leitzler opened this issue Mar 5, 2019 · 7 comments

Comments

@leitzler
Copy link
Contributor

leitzler commented Mar 5, 2019

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

$ go version
go version go1.12 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
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/leitzler/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/leitzler/go"
GOPROXY="https://user:pass@myproxy"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build942859588=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I'm running a CI environment that uses GOPROXY to fetch dependencies, the proxy (athens) has basic auth enabled since it has access to private repos that all users shouldn't be able to access.

The basic auth credentials are passed as a part of the GOPROXY, as https://user:pass@myproxy.

If go get fails to fetch a dependency (read: if someone just injects a faulty dependency), go get will include username and password from the proxy in the logs readable by the user.

What did you expect to see?

The URL without credentials, or credentials masked (for all go commands that uses GOPROXY)

What did you see instead?

Plain text username & password.

@bcmills
Copy link
Contributor

bcmills commented Mar 6, 2019

@bcmills
Copy link
Contributor

bcmills commented Mar 6, 2019

Don't pass credentials in the GOPROXY environment variable. Please, just don't do that.

Proper HTTPS basic auth support is coming in #26232. At that point you won't need to encode it in the URL, and logging the URL will not leak anything. So I'm closing this issue as a duplicate of #26232, and in the interim please find some other way to inject your credentials (maybe another layer of GOPROXY?).

@bcmills bcmills closed this as completed Mar 6, 2019
@leitzler
Copy link
Contributor Author

leitzler commented Mar 6, 2019

Passing credentials via GOPROXY is indeed a bad workaround for not being able to do "Proper HTTPS basic auth". I'm aware that this is being worked on, but I assume (and correct me if I'm wrong), that #26232 won't land before 1.13 and that is quite far away.

As seen in gomods/athens#1046 this is an issue that others already have noticed. The fact that Athens supports basic auth will increase the risk that others go down this path since proper auth isn't in place yet.

Even when proper auth is in place, will there be anything that prevents the user from still setting credentials in their GOPROXY?

(CC @FiloSottile who might have something to add since he asked me to open this issue when I asked about it)

@bcmills bcmills reopened this Mar 6, 2019
@bcmills
Copy link
Contributor

bcmills commented Mar 6, 2019

Ah, we can probably at least detect and reject credentials in the URL.

@marwan-at-work
Copy link
Contributor

marwan-at-work commented Mar 6, 2019

FWIW Once Go Modules supports proper authentication such as sending headers to GOPROXY we will make Athens accept that mechanism and probably remove the basic auth option.

That said, I also agree that Go should detect and reject credentials in the URL or at least detect and obfuscate them in logs.

@gopherbot
Copy link

Change https://golang.org/cl/166179 mentions this issue: cmd/go/internal/modfetch: reject credentials in proxy URL

@gopherbot
Copy link

Change https://golang.org/cl/170879 mentions this issue: cmd/go/internal/web{,2}: consolidate web packages

@bcmills bcmills self-assigned this Apr 12, 2019
@golang golang locked and limited conversation to collaborators Apr 22, 2020
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants