proposal: cmd/go: detect proxy redirect loops

[marwan-at-work]

Precursor

This issue is a continuation of #31458

While in the original issue, I was mostly focusing on how the functionality is meant to be used, this issue is about what the Go command can do to help go proxies avoid that vulnerability.

Problem statement

Go treats certain hostnames such as github.com, bitbucket.org and others as "well known" and therefore Go bypasses the ?go-get=1 logic. This means, Go immediately knows the VCS Destination of an import path without having to ask the hostname for it through a meta tag.

The logic for this functionality lives here

When it comes to a private module, this is important because a "GET github.com/user/private?go-get=1" will return a 404 even if you send it Basic Authentication credentials through a ".netrc" file because this is a Web request that requires cookie authentication and not an API request.

This is the only way go proxies have been able to support private module fetching from such code hosting sites. So it's important to know that if Go changed its behavior on these well-known hostnames, then all go proxies that deal with private modules would break.

However, there's a problem: say you have a proxy running on proxy.com and say a malicious user runs a vanity import server at loop.net. Say the malicious user makes it so that loop.net/pkg?go-get=1 returns a meta tag with a mod value that points to proxy.com -- If proxy.com runs a go mod download loop.net/[email protected], it will trigger an infinite loop.

Therefore, the only way for a GOPROXY to avoid that vulnerability is by doing either of the following:

Possible solutions

1. Write de-duplication logic that single-flights similar requests which would stop the infinite loop from happening. This echos what @hyangah suggested in the original issue. However, this works well when you have a single instance of a proxy but when you horizontally scale your proxy, this becomes a more difficult feature to implement. You would need to not only implement distributed lock, you would also need to propagate the error from one replica to the remaining replicas waiting on the lock.

2. Telling go mod download to never fetch from the proxy that triggered it by passing it the proxy.com hostname. For example: GONOPROXYHOSTS=proxy.com go mod download loop.net/[email protected] would trigger an error.

3. Have the GOPROXY call GET loop.net/pkg?go-get=1 itself to parse a possible meta tag with that vulnerability and error out before calling go mod download.
   Point 3 above would work well for public modules but it would not work for private modules as I mentioned above that go-get=1 would not work on well known sites that have private modules such as github.com, bitbucket.org etc
   Therefore, the GOPROXY would have to check the import path and if it is a well-known hostname, then it wouldn't need to call "?go-get=1" at all because we know that Go itself wouldn't do it and therefore there is no vulnerability.

Personally, I believe option 2 is the easiest one from a proxy perspective though I can't speak for the Go implementation.

Thanks!

CC: @bcmills @jayconrod

[tmthrgd]

3 is vulnerable to a TOCTOU (time-of-check to time-of-use) attack because a malicious server can simply return different sets of go-import tags for the first and second requests and this escape the check.

2 and 3 are both vulnerable to loops if they involve two or more proxies. Consider a situation where proxy-a connects to evil which serves a go-import mod tag for proxy-b, and when proxy-b then connects to evil it now serves a different go-import mod tag that points back to proxy-a, which loops.
So: proxy-a -> proxy-b -> proxy-a > proxy-b -> ...

In that case telling the go tool not to connect to itself doesn’t help. It’s not at all hard to serve different tags based of off the requesting IP address.

With both 2 and 3 you also have to consider how the comparison is done, given that hostnames are case-insensitive a naive implementation could be fooled by proxy example.com seeing a tag of EXAMPLE.COM. Also, internationalised domain names or plain IP addresses could cause problems as could www. vs no-www. (if they both served the same content).

A similar problem is faced by content delivery networks, Cloudflare’s solution has been a CDN-Loop HTTP header. I’m not sure how workable such a solution would be with go mod download, but it could be viable. It might need full buy in to be practice though as a single non-implementing proxy might allow loops through itself.

Of the three solutions you’ve proposed only 1 would actually prevent loops, though it is obviously much harder to implement in certain configurations.

go-import mod tags really feel like a loaded gun that’s very tricky to handle safely and enabled by default without override.

[rsc]

I retitled this to be about adding some way to detect proxy redirect loops, but I admit I don't quite understand the connection to private repos. Did I miss another part of the proposal? Should it be two separate proposals? Thanks.

[rsc]

ping @marwan-at-work - do I understand this proposal right? Thanks.

[marwan-at-work]

Thank you for the follow up @rsc

do I understand this proposal right?

Yes!

I don't quite understand the connection to private repos. Did I miss another part of the proposal?

It can definitely be omitted from this proposal. It was just something I ran into as a product of this proposal but I will create a separate issue and it can totally be ignored in the context of this proposal.

The main concern of this proposal is to see if there's something the Go command can help to detect those proxy loops. And if not, if there's any documentation/guidance in how proxy implementors should protect against it.

Thanks!

[rsc]

/cc @bcmills @jayconrod @matloob

[rsc]

Looking at the choices, it sounds like maybe the proxies should add and look for Via headers to detect the loops. That seems to be the established HTTP way to do that. We'd need to tell the go command what Via headers we want it to add during a particular fetch.

Thoughts?

[marwan-at-work]

That sounds good to me 👍🏼

We'd need to tell the go command what Via headers we want it to add during a particular fetch.

From a proxy's perspective these are the two commands that would need to support accepting Via headers:

go mod download and go list

Thanks

[oiooj]

So all Go proxy services should be RFC 7230 compliant. And we only allow appending the Via headers in go cmd. Maybe we can refer to the implementation of GOINSECURE, it looks simpler.

[rsc]

Looping in @katiehockman and @heschik for thoughts about whether proxy.golang.org can/should support Via headers for detecting proxy loops.

[rsc]

Ping @katiehockman, @heschik. The question is whether we can and should make proxy.golang.org set and check Via headers to detect request loops.

[katiehockman]

I'm trying to understand the main issue here. Is it that we are concerned about proxy.golang.org (or other proxies) being vulnerable to a redirect loop when fetching via go mod download, or concern about proxies having a potential redirect loop when speaking to each other, or something else?

If the former, I'm not terribly concerned about that since proxy.golang.org runs all fetches in a sandbox. If the latter, then we can work with other proxies to avoid this.

I'm not arguing that we can't do this, since I haven't thought too deeply into the technical solution, but I'm not understanding the motivation for why proxy.golang.org should.

[marwan-at-work]

The suggestion in this issue in particular is to have the Go command be able to forward via headers so that proxy implementations (such as proxy.golang.org) can use them to detect redirect loops.

Of course, this is not the only way for a proxy to detect redirect loops as a proxy can implement some sort of a distributed lock or run fetches in a sandbox as you mentioned, which I'm curious to know more about in case it's easier to implement than distributed-locks.

Therefore, if we don't go with the particular suggestion of via headers, I'd love it if Go provided some official documentation and/or code that helps proxy implementors not expose themselves to this vulnerability.

Thanks!

[hyangah]

proxy.golang.org already uses the approach similar to the option 1 along with the sandbox & various resource usage limits internally, so I am not too concerned about this specific attack scenario, but if Via header is available, proxy.golang.org can utilize it to break the part of the loop early and save resources. In particular, useful when the attacker's server returns arbitrary values for go-import requests every time.

As @katiehockman said, if it is to help other proxies and the go commands provide a way to set the Via header, proxy.golang.org should do it by configuring the go commands to do the right thing.

Is this Via header sufficient to avoid the original issue? @tmthrgd mentioned a scenario that involves multiple malicious proxies in play in a single loop - I don't know much about the go command internal, so I am not sure if it's possible. If it's possible, I want to understand what will prevent those malicious proxies from mutating the Via header.

ps I am interested in go command's support of the Via header for some other reasons though.

[rsc]

I wrote a go mod proxy subcommand a while back (never landed) and the very first thing it did was hit a proxy loop that wedged itself. Accidental misconfiguration is easy. It would be nice to diagnose well.

I do agree that no amount of Via header is going to guard against malicious behavior.

[rsc]

@marwan-at-work How important is it to you to defend against "malicious" users? It seems like very little we can do here will help with that case. But misconfiguration might be worth solving. You mentioned malicious users in the original comment and I'm curious how much that matters to you versus misconfiguration. Thanks.

[marwan-at-work]

@rsc misconfiguration is definitely another reason I'd love to guard against. I assume here you mean if a user accidentally misconfigured their meta tag to point back into a proxy a request came from. Otherwise, I'd be curious if there are other misconfigurations I'm not aware of.

As for "malicious" activity, while it is true that very little can be done to a well thought-out malicious attack, I think the Download Protocol (with the combination of the mod meta-tag feature) is by default prone to redirect loops. At the very least, I think Go should document this fact. Even better would be is to provide guidance on how to avoid such attack/misconfiguration.

[rsc]

I'm not really sure what to do here. It's unclear whether the Via header solves enough of a problem to be worth the churn. Does anyone feel strongly that we should add this in some form?

[rsc]

Given the lack of responses it seems like we should probably not do this.

[rsc]

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

[marwan-at-work]

@rsc sounds good to me. Appreciate your input 👍🏼

Now that the proposal is declined, Is it still worth documenting that proxy implementors should be aware of proxy redirect loops?

[bcmills]

@marwan-at-work, a note somewhere in https://golang.org/ref/mod#module-proxy might be appropriate. Want to send a CL for discussion?

(The source for that doc is in https://github.com/golang/website/blob/master/content/static/doc/mod.md.)

[rsc]

No change in consensus, so declined.
— rsc for the proposal review group