Opened 8 months ago

Closed 4 weeks ago

Last modified 4 weeks ago

#32027 closed defect (fixed)

Bump version of Go to 1.13+

Reported by: cohosh Owned by: boklm
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: snowflake, tbb-rbm, ReleaseTrainMigration, GeorgKoppen202004, TorBrowserTeam202004R, tbb-9.5a12
Cc: dcf, cohosh, phw, JeremyRand, tbb-team Actual Points:
Parent ID: #31688 Points: 2
Reviewer: sysrqb Sponsor: Sponsor58

Description

We're going to need it eventually for newer versions of pion/webrtc, and there's a nice feature in to log package that allows us to pass the log output writer to libraries.

Child Tickets

Change History (31)

comment:1 Changed 8 months ago by cohosh

Note: as mentioned in https://trac.torproject.org/projects/tor/ticket/28942#comment:66 we should be careful about how modules are handled in 1.13

Perhaps we want to tackle #28325 first.

comment:2 Changed 8 months ago by dcf

Another thing to watch out for in Go 1.13. By default, even commands like go build will phone home to proxy.golang.org and sum.golang.org. See:

The phone-home behavior is annoying, but probably mostly harmless in the rbm context. To disable the proxy.golang.org reporting, you can set GOPROXY=direct -- but even better for us may be GOPROXY=off, which is supposed to "disallow downloading modules from any source," which is what we want during the offline portion of the build.

To disable the sum.golang.org reporting, you can set GOSUMDB=off.
https://golang.org/cmd/go/#hdr-Module_authentication_failures

If GOSUMDB is set to "off", or if "go get" is invoked with the -insecure flag, the checksum database is not consulted, and all unrecognized modules are accepted, at the cost of giving up the security guarantee of verified repeatable downloads for all modules.

I personally had problems this week with checksum mismatches using go1.13.1 -- it turns out they changed how checksums are calculated with respect to symlinks, or something, and invalidated previous checksums. I tried clearing my cache and everything, and could not get https://github.com/lucas-clemente/quic-go to build using go1.13.1 because of checksum mismatches. So if you get "checksum mismatch" errors, it's something related to that.

comment:3 Changed 7 months ago by sysrqb

It looks like we need for this for supporting obfs4proxy on Android Q (and likely any golang project, for that matter), so that is very exciting.

The fix for https://github.com/golang/go/issues/29674 landed in 1.13 which is the cause of:

WARN: Managed proxy at '/data/app/org.torproject.torbrowser-xxxxxxxx==/lib/arm64/libObfs4proxy.so' reported: error: "/data/app/org.torproject.torbrowser-xxxxxxxx==/lib/arm64/libObfs4proxy.so": executable's TLS segment is underaligned: alignment is 8, needs to be at least 64 for ARM64 Bionic

Multiple other project ran into this problem, too (just for reference):
https://github.com/shadowsocks/v2ray-plugin-android/issues/6
https://github.com/termux/termux-packages/issues/3619
https://github.com/Catfriend1/syncthing-android/issues/370

comment:4 Changed 7 months ago by sysrqb

Actual Points: snowflake
Keywords: snowflake added

comment:5 Changed 7 months ago by gk

Cc: JeremyRand added
Parent ID: #31688

#31689 is a duplicate.

comment:6 Changed 6 months ago by sysrqb

Keywords: tbb-rbm TorBrowserTeam202001 added

comment:7 Changed 6 months ago by pili

Points: 2

comment:8 Changed 4 months ago by sysrqb

Keywords: TorBrowserTeam202002 added; TorBrowserTeam202001 removed

This can come after we solve supporting module.

comment:9 Changed 3 months ago by sysrqb

Keywords: ReleaseTrainMigration added

Release Train Migration.

comment:10 Changed 3 months ago by dcf

The GOMODULE111=off setting still works in go1.13 to disable the modules mode and work in the rbm setup. I had to do that in a branch recently where an upgraded quic-go required an upgraded go1.13.

It definitely does not work by default if you don't set GOMODULE111=off. You get a lot of errors like:

go: github.com/kr/pty@v1.1.1: Get https://proxy.golang.org/github.com/kr/pty/@v/v1.1.1.mod: \
dial tcp: lookup proxy.golang.org on 173.255.243.5:53: dial udp 173.255.243.5:53: \
connect: network is unreachable

comment:11 Changed 3 months ago by pili

Keywords: TorBrowserTeam202003 added; TorBrowserTeam202002 removed

We are no longer in February, moving tickets

comment:12 Changed 2 months ago by gk

I guess this ticket is relevant as well as there will be no security fixes for 1.12 anymore now that 1.14 is out.

comment:13 Changed 8 weeks ago by pili

Keywords: TorBrowserTeam202004 added; TorBrowserTeam202003 removed

We are no longer in March

comment:14 Changed 7 weeks ago by pili

Owner: changed from tbb-team to boklm
Reviewer: sysrqb
Sponsor: Sponsor58
Status: newassigned

comment:15 Changed 7 weeks ago by boklm

Cc: tbb-team added

comment:16 in reply to:  10 Changed 5 weeks ago by gk

Keywords: GeorgKoppen202004R TorBrowserTeam202004R added; TorBrowserTeam202004 removed
Status: assignedneeds_review

Replying to dcf:

The GOMODULE111=off setting still works in go1.13 to disable the modules mode and work in the rbm setup. I had to do that in a branch recently where an upgraded quic-go required an upgraded go1.13.

It definitely does not work by default if you don't set GOMODULE111=off. You get a lot of errors like:

go: github.com/kr/pty@v1.1.1: Get https://proxy.golang.org/github.com/kr/pty/@v/v1.1.1.mod: \
dial tcp: lookup proxy.golang.org on 173.255.243.5:53: dial udp 173.255.243.5:53: \
connect: network is unreachable

That's a good idea, thanks. It makes we nervous that we are still using an old and unsupported Go version, so I picked it up: bug_32027_v2 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_32027_v2&id=e50ba24bb7445986df3c2ebac1fb02b75569b86d)

I verified that both obfs4 and snwoflake are still compiling on all supported platforms. (meek compiles at least for Linux x86_64 and Android armv7) Furthermore I tested bundles on Linux and an armv7 device and nothing exploded. Finally, I compared artifacts and it seems Go 1.13.10 is not introducing any reproducibility issues. Thus, I think it's ready for review and testing in the next alpha (I did not jump to 1.14 immediately as we are late in the alpha process for 9.5, so we might want to be a bit careful at this point).

Last edited 5 weeks ago by gk (previous) (diff)

comment:17 Changed 5 weeks ago by gk

Keywords: GeorgKoppen202004 added; GeorgKoppen202004R removed

comment:18 Changed 5 weeks ago by boklm

Should we remove the workaround we have in projects/obfs4/build and projects/snowflake/build (removing the files go.mod and go.sum) if setting GOMODULE111=off makes that not needed?

comment:19 in reply to:  18 ; Changed 5 weeks ago by gk

Keywords: TorBrowserTeam202004 added; TorBrowserTeam202004R removed
Status: needs_reviewneeds_revision

Replying to boklm:

Should we remove the workaround we have in projects/obfs4/build and projects/snowflake/build (removing the files go.mod and go.sum) if setting GOMODULE111=off makes that not needed?

We can do that, good idea. Let me try this tomorrow and push a better branch if that works.

comment:20 in reply to:  19 Changed 5 weeks ago by gk

Keywords: TorBrowserTeam202004R added; TorBrowserTeam202004 removed
Status: needs_revisionneeds_review

Replying to gk:

Replying to boklm:

Should we remove the workaround we have in projects/obfs4/build and projects/snowflake/build (removing the files go.mod and go.sum) if setting GOMODULE111=off makes that not needed?

We can do that, good idea. Let me try this tomorrow and push a better branch if that works.

Okay, some further digging. Yes, we can remove our previous workaround for the missing support for Go modules. But only if we set GO111MODULE=off in the obfs4 and snowflake build scripts instead (too). I am not sure what's been causing this but for some reason a go.sum file in one of the PT *deps* was no issue with 1.12.x but is with 1.13.x now (which is what dcf was hitting).

bug_32027_v3 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_32027_v3&id=81dd92051328e23ae7b02354fb84450905c62b52) contains the fixups.

comment:21 Changed 5 weeks ago by cypherpunks

But only if we set GO111MODULE=off in the obfs4 and snowflake build scripts instead (too).

Move it to https://gitweb.torproject.org/user/gk/tor-browser-build.git/tree/projects/go/config?h=bug_32027_v3&id=81dd92051328e23ae7b02354fb84450905c62b52#n20 instead?

comment:22 in reply to:  21 Changed 5 weeks ago by gk

Replying to cypherpunks:

But only if we set GO111MODULE=off in the obfs4 and snowflake build scripts instead (too).

Move it to https://gitweb.torproject.org/user/gk/tor-browser-build.git/tree/projects/go/config?h=bug_32027_v3&id=81dd92051328e23ae7b02354fb84450905c62b52#n20 instead?

I am not sure. Not everything that uses the setup part does need that env variable set. So, I am inclined to just apply it where it is needed. But that can probably be said for the build_go_lib part, too. I guess I am fine making that change if we think that's good. So far we kept the workaround for the lack of go module support in the respective projects which is why I added the env variable there as well.

comment:24 in reply to:  23 Changed 5 weeks ago by gk

Keywords: TorBrowserTeam202004 added; TorBrowserTeam202004R removed
Status: needs_reviewneeds_revision

Replying to cypherpunks:

Some comment about adding https://gitweb.torproject.org/user/gk/tor-browser-build.git/tree/projects/obfs4/build?h=bug_32027_v3&id=81dd92051328e23ae7b02354fb84450905c62b52#n10?

Yeah, I can do that. The build breaks without it for some reason.

comment:26 in reply to:  25 Changed 5 weeks ago by gk

Keywords: TorBrowserTeam202004R added; TorBrowserTeam202004 removed
Status: needs_revisionneeds_review

comment:27 Changed 5 weeks ago by dcf

bug_32037_v4 looks good to me.

comment:28 Changed 4 weeks ago by sysrqb

Resolution: fixed
Status: needs_reviewclosed

Patch looks good to me. Merged to master with commit 30bdc472d4d2b222b9435c4a1c728ab74ff96365.

comment:29 Changed 4 weeks ago by sysrqb

And thanks!

comment:30 Changed 4 weeks ago by sysrqb

Keywords: tbb-9.5a12 added

comment:31 Changed 4 weeks ago by sysrqb

And for the documentation, the error in building obfs4 for Android without setting CGO_ENABLED is:

# _/var/tmp/build/obfs4-2d8f3c8bbfd7/obfs4proxy
/var/tmp/dist/go/pkg/tool/linux_amd64/link: running /var/tmp/dist/android-toolchain/android-ndk/arm/bin/clang failed: exit status 1
/tmp/go-link-925939917/go.o:go.go:_cgo_init: error: undefined reference to 'x_cgo_init'
/tmp/go-link-925939917/go.o:go.go:_cgo_notify_runtime_init_done: error: undefined reference to 'x_cgo_notify_runtime_init_done'
/tmp/go-link-925939917/go.o:go.go:_cgo_thread_start: error: undefined reference to 'x_cgo_thread_start'
/tmp/go-link-925939917/go.o:go.go:runtime._cgo_setenv: error: undefined reference to 'x_cgo_setenv'
/tmp/go-link-925939917/go.o:go.go:runtime._cgo_unsetenv: error: undefined reference to 'x_cgo_unsetenv'
/tmp/go-link-925939917/go.o:go.go:runtime.cgo_yield: error: undefined reference to '_cgo_yield'
clang60: error: linker command failed with exit code 1 (use -v to see invocation)
Note: See TracTickets for help on using tickets.