golang / go

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

cmd/go: add GOINSECURE for insecure dependencies #32966

Closed witchard closed 5 years ago

witchard commented 5 years ago

As discussed in #32104, #31741 and #27332; there is sometimes a need for users to fetch dependencies in an insecure manner (e.g. where dependencies are on servers with certificates that are not trusted by the system, or where the server is not https at all). The current go get command supports a -insecure flag for this use-case; however this is not supported by the new go mod commands. The -insecure flag is probably overkill in most cases and could lead to users fetching dependencies insecurely by accident.

I propose the addition of two new environment variables that would be used by all commands fetching dependencies. The first of these would provide the go tools with additional CA certificates to trust (in situations where the user is unable to modify the system trust, or where they only want to trust a certificate for the duration of the go command). The second would list servers where insecure fetching is allowed. For example, these could be:

If this were implemented, then I would also propose the removal of the -insecure flag from go get.

I would be willing to work on this issue if it were accepted, but I don't really know where to start!

bcmills commented 5 years ago

What's the use-case for GOTRUST? (Under what conditions would you want to build — and likely run — arbitrary source code signed by a CA that you don't trust enough to add to your system roots?)

bcmills commented 5 years ago

CC @FiloSottile @jayconrod @rsc

witchard commented 5 years ago

The only situation I can think of is if a user does not have permission to modify their system trust. Pythons pip tool has similar options (https://pip.pypa.io/en/stable/reference/pip/#general-options), trusted-host is similar to GO_INSECURE and cert similar to GO_TRUST.

ernestoaparicio commented 5 years ago

Second this. Ran into an issue using Athens go proxy because it uses go mod download and that doesn't support --insecure.

FiloSottile commented 5 years ago

I am in favor of GOINSECURE.

GOTRUST is complicated. On Windows, for example, we don't get to mix system and custom roots, because system verification is performed by calling the system API. We probably can get there by attempting two verifications in VerifyPeerCertificate.

I'd like to hear more support for the use case before introducing complexity for it. (But OTOH I don't want users reverting to GOINSECURE when unnecessary.)

witchard commented 5 years ago

Ah interesting, I hadn’t appreciated that complexity. I wonder if GOTRUST could be of some form like host1:certfile1,host2:certfile2,... so you could then choose a custom cert for specific hosts; and then it would only use that cert for access to that host and not touch system ones? The env variable looks a bit more complex though.

I agree that forcing a users to use GOINSECURE to access a resource that they just haven’t got a mechanism to trust a cert for is a bit odd.

xrfang commented 5 years ago

A usecase for GOINSECURE is that the package is hosted on an Intranet. Is it possible to add a switch to allow inscure fetch for ALL intranet hosted packages?

witchard commented 5 years ago

@xrfang - If all the intranet module sources share a common domain, e.g foo.company.org, bar.company.org then you’d just set it to *.company.org.

xrfang commented 5 years ago

@witchard my suggestion is not based on domain matching, but by IP address, if any domain is resolved to internal IPs, or a specific CIDR range, then it is considered internal.

I personally think it is not hard to implement, and might be useful.

witchard commented 5 years ago

@xrfang oh I see, I guess that could work - so you’d put something like 192.168.1.0/24 in GOINSECURE?

I wonder if anything similar has been considered for GONOSUMDB and GONOPROXY? I feel like the go team would probably want it to be consistent.

FiloSottile commented 5 years ago

if any domain is resolved to internal IPs, or a specific CIDR range, then it is considered internal

That would not be secure. DNS resolution is ordinarily unsecured, so with such a configuration option enabled all an attacker would have to do to downgrade security for a package would be to fake its IP address. It's also unclear to me what use cases that addresses that can't be addressed with a GOINSECURE list.

witchard commented 5 years ago

Based on the comments so far then, would it make sense to proceed with just GOINSECURE?

GOINSECURE would be defined as: a comma separated list of hostnames (with globs) where insecure fetches (either over https due to untrusted authority, or over http) are allowed. Example: GOINSECURE=foo.com,*.bar.com.

GOTRUST and anything based on ip subnets out of scope for now.

xrfang commented 5 years ago

@witchard this means, to achieve what I want, I just set GOTRUST=192.168.0.0/24, and leave GOINSECURE empty. Then any package repo that resolve to that ip range will be considered trusted, and allow an http fetch?

@FiloSottile yes it is insecure, but I don't think it will be even more insecure to specify ip range than to specify package names. Both relies on DNS anyway.

witchard commented 5 years ago

@xrfang what @FiloSottile is saying is that a malicious actor on your network could modify DNS to point github.com at a local IP and then (if you had GOINSECURE for local IPs) they could get control of your packages from github.com. This wouldn’t be good and so having a blanket setting for local IPs is probably not a good idea. I.e. it is less secure to allow IP ranges in GOINSECURE.

GOINSECURE is essentially saying “I know packages from this domain are not coming to me over a trusted channel”. Therefore a malicious actor on your network could give you malicious packages for that domain, but for that domain only - and other packages from other domains are still protected.

As such the only way you’d achieve what you want would only be to put all the domains for your local packages in GOINSECURE I’m afraid.

xrfang commented 5 years ago

@witchard OK thanks for pointing out. I didn't notice this possibility. However, I still consider that allow GOINSECURE to specify ip ranges is useful, provided that the implication is well documented. Its up to the core team to decide.

rsc commented 5 years ago

It seems like the discussion here has converged on (1) adding GOINSECURE and (2) not adding GOTRUST.

Is that accurate? Thanks.

witchard commented 5 years ago

Yes that’s my understanding. What needs to happen next? I’m willing to put some time into developing the feature but might need some pointers towards the right areas of the code to look at.

wuxc commented 5 years ago

what about add a git+ssh vcs path pattern in cmd/go/internal/get/vcs.go? A dynamic vcs path parser and an environment variable like 'GOPRIVATE_VCS=myprivate.site/;another.me/'


// vcsPaths defines the meaning of import paths referring to
// commonly-used VCS hosting sites (github.com/user/dir)
// and import paths referring to a fully-qualified importPath
// containing a VCS type (foo.com/repo.git/dir)
var vcsPaths = []*vcsPath{
    // Github
    {
        prefix: "github.com/",
        regexp: lazyregexp.New(`^(?P<root>github\.com/[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+)(/[\p{L}0-9_.\-]+)*$`),
        vcs:    "git",
        repo:   "https://{root}",
        check:  noVCSSuffix,
    },

    // wuxc gitlab
    {
        prefix: "myprivate.site/",
        regexp: lazyregexp.New(`^(?P<root>myprivate\.site/[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-]+)(/[\p{L}0-9_.\-]+)*$`),
        vcs:    "git",
        repo:   "ssh://{root}",
        check:  noVCSSuffix,
    },
bcmills commented 5 years ago

@wuxc, we want to move toward fewer hard-coded paths in the go binary, not more. A .git suffix on the module path should already be sufficient to at least attempt a fetch via git+ssh.

witchard commented 5 years ago

Using the .git suffix does actually help in my situation, and removes most of the need for GOINSECURE for my use case. This in conjunction with git url rewriting (https://www.jvt.me/posts/2019/03/20/git-rewrite-url-https-ssh/) enables me to import something like go.company.org/foo/bar.git which actually clones from ssh://long.internal.boring.git.url/foo/bar.git. The one annoying thing about this is that the git is part of the module path which (a) looks ugly, and (b) becomes part of the name for main binaries installed with go install. I wonder if something like GONOPROXY could be extended so that you could specify the vcs type, in this example: GONOPROXY=git:go.company.org.

bcmills commented 5 years ago

@witchard, GONOPROXY operates on package paths, but the reason we need the .git suffix is in the step that resolves a package path to a URL. So there would be a bit of an impedance mismatch there: GONOPROXY would have to make an assumption about the package-to-URL mapping that it does not require today.

witchard commented 5 years ago

Yes, fair enough. This is all just making me wonder that if there were ways to express custom module-path to repo mappings then perhaps the need for insecure fetch disappears somewhat. The current mechanism for custom module base to repo mappings (html meta tags at a trusted https custom domain) is challenging to meet in corporate environments with tightly controlled networks. GOINSECURE allows us to relax some of that by removing the requirement for trusted https, but some way to specify explicit mappings might allow us to not need to have GOINSECURE at all.

Would it make sense for me to open a separate issue on this subject, or is it something that we wouldn’t want to consider?

bcmills commented 5 years ago

Would it make sense for me to open a separate issue on this subject, or is it something that we wouldn’t want to consider?

You can open a separate issue if you like, but just to set expectations I doubt that we will go that route. (We already have one mechanism for setting up custom import paths, so the bar for adding another should be very high — we don't add redundancy lightly.)

witchard commented 5 years ago

Ok, I’ll leave it for now :-). What is the next step for progressing with this proposal?

ianlancetaylor commented 5 years ago

It looks like people are in favor of adding GOINSECURE with a list of wildcarded host names, so this is a likely accept. Leaving open for one week for final comments.

-- for @golang/proposal-review

bradfitz commented 5 years ago

No new comments in the past week, so accepting.

gopherbot commented 5 years ago

Change https://golang.org/cl/205238 mentions this issue: cmd/go/internal/modfetch: add GOINSECURE

witchard commented 5 years ago

Just a thought after an initial implementation on this... I think it makes sense that setting GOINSECURE should imply GONOPROXY. As insecure fetch implies you are accessing the resource directly. Does this make sense?

dayadev commented 4 years ago

@witchard may be an outdated question but what version of go has this GOINSECURE flag?

I am on go version go1.13.4 darwin/amd64 and I get error when I try to set the flag

go env -w: unknown go command variable GOINSECURE
FiloSottile commented 4 years ago

@dayadev it's going to be in Go 1.14, so you can use golang.org/dl/gotip to try it (but be warned that the dev tree is not stable).

dayadev commented 4 years ago

@FiloSottile Thank you!

arvenil commented 4 years ago

I've run into issue with x509: certificate signed by unknown authority when running go mod download in Dockerfile.

I've tried:

RUN apk update && apk add --no-cache git ca-certificates && update-ca-certificates

And then I tried GOINSECURE=googleapis.com

 > [build_base 6/8] RUN GOINSECURE=googleapis.com go mod download:                                                    
#10 33.58 github.com/klauspost/compress@v1.10.10: Get "https://storage.googleapis.com/proxy-golang-org-prod/d9521529eeba67d2-github.com:klauspost:compress-v1.10.10.zip?Expires=1602676985&GoogleAccessId=gcs-urlsigner-prod%40golang-modproxy.iam.gserviceaccount.com&Signature=X9KcpzaewN7L0trqQIC0Ak2VM5M5xobvKoaVdUF3zWzVuklJtU1%2BmKIdMrppMBuZTyZoCf9MpeISitDbAC%2BR1dTYSJCzIqRFUlXNLV%2FrYnq8OWIQi0G%2FxQlrfyMC3PmHamLR6EX644FG%2BmOgyswsBVnH6qJu67vvQR8cyAak1rIAptE4%2FqKLoxnn8KgBL%2F5OqXSrsvzNzTvD6kgGqWmj5J%2BGjWQXrQ5TMFNeyqIvFE3zR1YrsxcK0mjcan3xAol0ti4hNFGbjrOikm4UbN16gKhaiPcXL9X0zeLbj4O0KstYM0JQBqitZHR9x2MExGvp%2Bfcfi7qVFNd9BotKD9qd6Q%3D%3D": x509: certificate signed by unknown authority
#10 33.58 github.com/mozilla/tls-observatory@v0.0.0-20200317151703-4fa42e1c2dee: Get "https://storage.googleapis.com/proxy-golang-org-prod/403d0c0e9c9c4380-github.com:mozilla:tls-observatory-v0.0.0-20200317151703-4fa42e1c2dee.zip?Expires=1602682118&GoogleAccessId=gcs-urlsigner-prod%40golang-modproxy.iam.gserviceaccount.com&Signature=etjtv5w1mLH9204QSi1hfhAdJylLeFgp5ZDQKFs7KzKCyQdhIMFi7dhTwTZTCtNA9n3cwx%2FrN5wspIrpP%2BCR66hQes9DRRvFH2wz%2BLk9OnquSWPZIOXcFaEgo4yQtm0elDwiR1HGIfX4vF1o042V4a%2BrYWJOigScqKHVQRPZYOHiHr4stmCKmSyXpX3ASgbpYNfvwYBfxZMYDlga8%2BAJcfWYT5Yyu8D5Kcy3IbXNAn1pnFub6xH8MH%2BfzlgDfABc%2F14koxNV2msXIrIaG%2Bjm%2Byi4gDGS9uwh4f1mO7Xn0zclFqN9XGaTZNhs8OB5ujqNOzzILqoxGqDuMHbgkv3yIw%3D%3D": x509: certificate signed by unknown authority
#10 33.58 google.golang.org/api@v0.13.0: Get "https://storage.googleapis.com/proxy-golang-org-prod/dc7230740beda144-google.golang.org:api-v0.13.0.zip?Expires=1602683220&GoogleAccessId=gcs-urlsigner-prod%40golang-modproxy.iam.gserviceaccount.com&Signature=W0nKtvtCQW5I861BsHnvIoil%2FxJM4eTmBBprfdijOiVriV4VKNBiCQ5kpPcDjHnuAeiJYsTu60sCEByj6TQOEa0VgHDlcY9YlVS4OOHLCec3ny2HxQOEB0w1iutnaTkSyxpN5r%2FEYvExfax0mA%2F42fYTYn6fhbHE52Fq1hcjmyXyt4WJzM74QoEQFNeIgsq2x75Yr2Fi2TbSI3aD%2FnUWvf9rozGfI9tL92%2BjGG0QsynKHKzf6lZrOY8InkPy%2BNLpOxZf6%2BV3nvSt6jW8bmsuUpve0Wqjfw3d7E1CMmcjA8kf3fb7yOlCv%2BtKT4KoMJSYxQPZCeLegIT6Bd%2BbqC%2FJdg%3D%3D": x509: certificate signed by unknown authority
------
failed to solve with frontend dockerfile.v0: failed to build LLB: executor failed running [/bin/sh -c GOINSECURE=googleapis.com go mod download]: runc did not terminate sucessfully

But this doesn't seem to work. Any ideas?

bcmills commented 4 years ago

@arvenil, this issue is about implementing GOINSECURE. Questions about its usage are off-topic.

There are many other methods to get help if you're still looking for answers, such as:

Thanks.

arvenil commented 4 years ago

What I'm trying to say is that I would expect GOINSECURE to work in that case but it doesn't and it's not obvious for me why not.

FiloSottile commented 4 years ago

What I'm trying to say is that I would expect GOINSECURE to work in that case but it doesn't and it's not obvious for me why not.

@arvenil GOINSECURE applies to module import paths, not to the URL they are fetched from. That is intentional, because its intended use is for when a certain subset of modules are fetched over secure connections (like a VPN) without HTTPS.

Anyway, your issue should not require GOINSECURE, and instead you should find a way to make HTTPS verification work as expected. The resources @bcmills shared can help you with that.