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: allow users to control VCS usage #41730

Closed
rsc opened this issue Oct 1, 2020 · 19 comments
Closed

cmd/go: allow users to control VCS usage #41730

rsc opened this issue Oct 1, 2020 · 19 comments

Comments

@rsc
Copy link
Contributor

rsc commented Oct 1, 2020

The go command runs commands like git and hg to download modules. In the past, we have had problems with security bugs in version control systems becoming security bugs in “go get”.

The original modules draft design removed use of these commands entirely, saying:

We want to move away from invoking version control tools such as bzr, fossil, git, hg, and svn to download source code. These fragment the ecosystem: packages developed using Bazaar or Fossil, for example, are effectively unavailable to users who cannot or choose not to install these tools. The version control tools have also been a source of exciting security problems. It would be good to move them outside the security perimeter.

The removal of these commands was not possible in the end: being able to fetch directly from Git repos is too important, especially for closed source. But the security exposure has not gone away. We remain vulnerable to problems in VCS systems, especially the less scrutinized ones.

I propose to add a new GOVCS setting that controls which version control systems are allowed to be used for downloads.

In its simplest form, GOVCS is a pipe-separated list of allowed version control systems:

GOVCS=bzr|fossil|git|hg|svn  # any known VCS system

The set of all known version control systems can be abbreviated “all’:

GOVCS=all  # any known VCS system

The set of no VCS systems can be written “off”:

GOVCS=off  # no VCS usage allowed at all

A glob pattern (already used for GOPRIVATE, GONOPROXY, and so on) followed by a colon can restrict the effect to a particular (module or import) path prefix:

GOVCS=github.com:git  # on github.com, only use git

The special pattern “private” means any pattern from GOPRIVATE.
The special pattern “public” means anything not matched by GOPRIVATE.

A comma-separated list of restricted can be given. The first match in the list applies:

GOVCS=github.com:git,bitbucket.com:git|hg

Finally, there is a default setting in the go command itself. An explicit setting of GOVCS takes priority but does not remove the defaults; the effect is as if the explicit GOVCS setting were inserted ahead of the defaults in the comma-separated list.

(Note that this gives considerably more control over exactly when a VCS can be used than the current behavior of “if it’s in the PATH, it can be used.”)

I propose the default GOVCS setting allow all VCS for private modules but only Git and Mercurial for public modules.

That is, the default setting is equivalent to:

GOVCS=private:all,public:git|hg

The rationale behind allowing only Git and Mercurial is that these two systems have had the most attention to issues of being run as clients of untrusted servers. In contrast, Bazaar, Fossil, and Subversion have primarily been used in trusted, authenticated environments and are not as well scrutinized as attack surfaces.

It is certainly the case that even Git and Mercurial are unfortunately not free of bugs either, but those projects have had more scrutiny and have established a record of fixing them quickly.

The rationale behind allowing private:all is that private servers are trusted. It is beyond the go command’s threat model to guard against scenarios in which a company’s private source code host has already been compromised.

Users who want to allow all VCS even for public source code servers would use

GOVCS=private:all,public:all

I propose that the public Go module mirror proxy.golang.org continue to allow all VCS.

It is already the case that the Go module mirror handles fetching Bazaar, Fossil, and Subversion repos for the vast majority of Go users: those users do not even need to install the corresponding binaries, and it is likely that most do not.

The module mirror also invokes the go command and its subprocesses in a security sandbox for added security, so it is more equipped to deal with potential compromises of the version control tools. It would simply run with GOVCS=public:all to continue allowing all the VCS tools to be used. We would encourage alternate module mirrors to do the same.

The combination of disallowing less-popular version control systems by default but continuing to be able to fetch them through module mirrors balances improving baseline security on developer systems machines while not fragmenting the ecosystem.

This default does put the module mirror on the critical path for downloads of public Bazaar/Fossil/Subversion downloads, but it is the same system that serves the checksum database, which is already on the critical path for all downloads. And the setting that opts out of the checksum database and proxy for all modules—GOPRIVATE=*—would automatically opt out of the VCS restrictions as well.

@rsc rsc added the Proposal label Oct 1, 2020
@rsc rsc added this to the Proposal milestone Oct 1, 2020
@rsc rsc added this to Incoming in Proposals (old) Oct 1, 2020
@FiloSottile
Copy link
Contributor

Strong support. This would significantly reduce the attack surface of "go get".

I think the proposal could make the relative priority of comma and pipe more clear, but it looks like there is only one reasonable interpretation.

Once this is implemented, I think we should consider vulnerabilities in any VCS other than Git and Mercurial as out of scope for golang.org/security.

/cc @golang/security

@beoran
Copy link

beoran commented Oct 3, 2020

This is a step in the right direction, however I agree with the original "We want to move away from invoking version control tools".

In other programming languages, like say, Ruby, the equivalent of go modules is distributed as special purpose archives, like .gem files for Ruby. Go should allow something similar with a go module pack command that packs your module foo v1.2.3 to such an archive, maybe named foo@1.2.3.nut, which go get then can download over https, based on the go.mod file.

@bcmills
Copy link
Contributor

bcmills commented Oct 5, 2020

@beoran, we already support a protocol that does not need to invoke any version control tools. The go-import metadata can indicate mod as the VCS, which causes the go command to use the GOPROXY protocol over native HTTPS.

I presume that under this proposal, GOVCS=off would still allow use of the mod protocol, since that does not require any tools other than the go command itself.

@rsc rsc moved this from Incoming to Active in Proposals (old) Oct 7, 2020
@rsc
Copy link
Contributor Author

rsc commented Oct 14, 2020

It seems like everyone is in favor of this. Do I have that right? Are there any objections?

@beoran
Copy link

beoran commented Oct 14, 2020

@bcmills That is nice to know, but not very practical. Ruby gems are far easier to use than a raw goprogy. But I support this proposal as a step in the right direction.

@rsc
Copy link
Contributor Author

rsc commented Oct 21, 2020

Based on the discussion above, this seems like a likely accept.

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Oct 21, 2020
@rsc
Copy link
Contributor Author

rsc commented Oct 28, 2020

No change in consensus, so accepted.

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Oct 28, 2020
@rsc rsc changed the title proposal: cmd/go: allow users to control VCS usage cmd/go: allow users to control VCS usage Oct 28, 2020
@rsc rsc modified the milestones: Proposal, Backlog Oct 28, 2020
@rsc
Copy link
Contributor Author

rsc commented Oct 29, 2020

As I implement this and look at parsing error cases, it seems like a mistake to allow items in the list that do not have a pattern: prefix. If we did that, then

GOVCS=git,svn

mysteriously means "git only", because all paths match the "git" rule, leaving nothing to go on to the "svn" rule. Similarly,

GOVCS=myserver.com:git,svn

means "myserver.com can only use git; everything else can only use svn", which is not what it looks like at all.

The most ergonomic answer here seems to be to require every item in the comma-separated list to have the form pattern:vcslist, so that you can write

GOVCS=myserver.com:git,*:svn

if that's what you mean, but

GOVCS=myserver.com:git,svn

will be flagged as invalid instead of silently meaning the same thing.

@gopherbot
Copy link

Change https://golang.org/cl/266420 mentions this issue: cmd/go: add GOVCS setting to control version control usage

@tmthrgd
Copy link
Contributor

tmthrgd commented Oct 30, 2020

How will this interact with go-import meta tags? From what I can discern it would apply to the import/module path and not the repository that go-import points to, is that correct?

For example go.tmthrgd.dev (my vanity domain) serves go-import meta tags like: <meta name="go-import" content="go.tmthrgd.dev/example git https://github.com/tmthrgd/example">, if the go tool was fetching go.tmthrgd.dev/example which GOVCS pattern would match? One specifying github.com or go.tmthrgd.dev or either of them? (I know this example is somewhat academic as git is supported by default, but imagine it was using svn).

Intuitively I would expect github.com to match (as that's what the git tool is invoked) against and not the vanity domain, but I suspect it's the other way around. I think the interaction with go-import might be worth documenting somewhere.

@jayconrod
Copy link
Contributor

@tmthrgd GOVCS patterns will only be used to match module paths or import paths. So in your example, that would be go.tmthrgd.dev/example. You could migrate the repository to another host without users needing to change Go settings (though .netrc and .gitconfig might still need to be adjusted).

@dmitshur dmitshur modified the milestones: Backlog, Go1.16 Nov 5, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Nov 5, 2020

Reopening because CL 266420 was reverted in CL 267799; it needs to be re-sent.

@dmitshur dmitshur reopened this Nov 5, 2020
@gopherbot
Copy link

Change https://golang.org/cl/267888 mentions this issue: cmd/go: add GOVCS setting to control version control usage

@heschi
Copy link
Contributor

heschi commented Nov 9, 2020

GOVCS wasn't added to go help environment AFAICT?

@jayconrod
Copy link
Contributor

Looks like it was not. go help private explains it, but go help environment should point to that.

We should add a release note, too.

@gopherbot
Copy link

Change https://golang.org/cl/290992 mentions this issue: content/static/doc: document GOVCS

@beoran
Copy link

beoran commented Feb 11, 2021

@dmitshur
This isssue needs to be reopened for the docs if I am not mistaken?

@gopherbot
Copy link

Change https://golang.org/cl/291230 mentions this issue: cmd/go: multiple small 'go help' fixes

@jayconrod
Copy link
Contributor

@beoran Thanks for pinging this issue. go help environment includes GOVCS since CL 282615. That should have referenced this issue, but there's no need to reopen.

gopherbot pushed a commit that referenced this issue Feb 11, 2021
* Link to privacy policies for proxy.golang.org and sum.golang.org in
  'go help modules'. It's important that both policies are linked from
  the go command's documentation.
* Fix wording and typo in 'go help vcs' following comments in CL 290992,
  which adds reference documentation for GOVCS.
* Fix whitespace on GOVCS in 'go help environment'.

For #41730

Change-Id: I86abceacd4962b748361244026f219157c9285e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/291230
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
gopherbot pushed a commit to golang/website that referenced this issue Feb 16, 2021
For golang/go#41730

Change-Id: Ie5437ad57be79e42a2a7cd2e89264dbba5285941
Reviewed-on: https://go-review.googlesource.com/c/website/+/290992
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@golang golang locked and limited conversation to collaborators Feb 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

9 participants