cmd/go: add more options to security whitelist #23937

olt · 2018-02-20T09:17:54Z

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

Go 1.10

What operating system and processor architecture are you using (`go env`)?

MacOS 10.13.2

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/olt/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/olt/dev/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.10/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.10/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/np/2fhk15fn5qq6fm4j9s3vt_6r0000gn/T/go-build391399925=/tmp/go-build -gno-record-gcc-switches -fno-common"

Debian Jessie 3.16.0-4-amd64

GOARCH="amd64"
GOBIN=""
GOCHAR="6"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/olt/go"
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"

What did you do?

Build code cgo code which includes the evaluated output of

// #cgo CXXFLAGS: $(mapnik-config --cxxflags --includes --dep-includes | tr '\n' ' ')

The output of mapnik-config --cxxflags is as follows (for two different versions):

-g -O2 -fstack-protector-strong -Wformat -Werror=format-security -g0 -ansi -Wall -pthread -O2 -fno-strict-aliasing -finline-functions -Wno-inline -Wno-parentheses -Wno-char-subscripts

and

-std=c++11 -stdlib=libc++ -DBOOST_EXCEPTION_DISABLE -fvisibility=hidden -fvisibility-inlines-hidden -Wall -ftemplate-depth-300 -Wsign-compare -Wshadow -O3

What did you see instead?

For the first version:

go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -ansi
go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -finline-functions

It works with the following env: CGO_CXXFLAGS_ALLOW='-ansi|-finline-functions'

For the second version:

go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -stdlib=libc++
go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -fvisibility=hidden
go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -fvisibility-inlines-hidden
go build: xx/xx/xx: invalid flag in #cgo CXXFLAGS: -ftemplate-depth-300

CGO_CXXFLAGS_ALLOW='-stdlib=.*|-fvisibility=hidden|-fvisibility-inlines-hidden|-ftemplate-depth[=-]\d+

-stdlib is already included in master, but the other options are missing. Note that -ftemplate-depth-300 uses - instead of =.

The text was updated successfully, but these errors were encountered:

ianlancetaylor · 2018-02-20T23:25:08Z

More options to add to the whitelists:

From #23904:

LDFLAGS: -Wl,-rpath=$ORIGIN/lib

From #23909:

LDFLAGS: -mwindows

From #23923:

CXXFLAGS: -ffat-lto-objects
CXXFLAGS: -fuse-linker-plugin

(Note that -fuse-linker-plugin is safe as long as -B is not used, and -B is never safe anyhow.)

From #23749:

LDFLAGS: -mmacosx-version-min=10.8

(We currently accept -mmacos-x for the compiler, but not the linker.)

ianlancetaylor · 2018-02-20T23:26:10Z

From #23749:

LDFLAGS: -O3

ianlancetaylor · 2018-02-20T23:27:36Z

From #23938:

CFLAGS: -w

andlabs · 2018-02-21T12:31:32Z

(You meant to say 23904 instead of 23094 there; in case that bug accidentally gets marked as fixed by this fix.)

ianlancetaylor · 2018-02-21T15:23:44Z

Thanks; I updated the comment.

ianlancetaylor · 2018-02-21T15:46:38Z

Reported on #23749:

LDFLAGS: -shared

andlabs · 2018-02-21T16:05:10Z

(Oops, missed it: likewise for 23090. Not sure what the right issue number there is.)

Seconding -mmacosx in the linker here; was just about to report that myself. I will point out that the macOS linker will complain if the values of this flag conflict for any object file; what does Go itself do?

AlexRouSg · 2018-02-21T19:23:47Z

-mwindows is from #23909

ianlancetaylor · 2018-02-22T01:48:24Z

Corrected issue reference in comment above. Thanks.

Go itself does not even know about the -maccosx option, and as such will not complain about it.

andlabs · 2018-02-22T05:52:47Z

No; I meant to ask what does Go set in the Mach-O headers for the minimum version. I can make that a separate discussion, though.

ianlancetaylor · 2018-02-22T06:09:11Z

@andlabs Currently when using internal linking the program requests OS X version 10.7.0. When using external linking it's set by the system linker. Let's take any followup on this to golang-nuts.

ianlancetaylor · 2018-02-23T04:13:14Z

From #23749:

LDFLAGS: -Wl,-static

mattn · 2018-02-23T07:20:28Z

go build github.com/golang-ui/nuklear/nk: invalid flag in #cgo LDFLAGS: -Wl,--allow-multiple-definition

andlabs · 2018-02-25T07:20:44Z

I am starting to see a number of people on my own project assume that the consequences of the security fix are bugs in my project specifically — people are updating Go without realizing the security patch exists in in the first place. I'll be editing up my README, but I have to wonder what other projects also have this problem — and worse, what missing options have gone unnoticed by us as a result.

To further address the lack of information-spreading, I wonder if the error message from cgo should point to the security fix announcement as well.

Furthermore, at least two people are resorting to setting the _ALLOW env vars to .* as a workaround. Should we be disallowing this entirely, since it unintentionally circumvents the security fix? I'm probably going to find out who originated the idea; hopefully it's on github and thus can be fine-tuned...

ianlancetaylor · 2018-02-25T22:50:10Z

@andlabs In 1.10 the error message points to https://golang.org/s/invalidflag, and in 1.9.5 it will as well.

ianlancetaylor · 2018-02-25T22:51:16Z

From #24113:

CFLAGS: -arch

ianlancetaylor · 2018-02-25T23:32:01Z

From #23749:

CFLAGS: -m32
LDFLAGS: -O3
LDFLAGS: -static-libgcc
LDFLAGS: -static-libstdc++

ianlancetaylor · 2018-02-26T19:05:03Z

From #23749:

LDFLAGS: -Wl,-z,relro

AlexRouSg · 2018-02-26T20:08:50Z

LDFLAGS: -Wl,-z,relro
Is from #24124 and I see a bunch more flags in the sample code.

LDFLAGS: -Wl,-undefined 
LDFLAGS: -Wl,dynamic_lookup 
LDFLAGS: -Wl,-unresolved-symbols=ignore-all 
LDFLAGS: -Wl,-z,noexecstack 
LDFLAGS: -Wl,-dn 
LDFLAGS: -Wl,-dy

Might be a good idea to ask the user to use CGO_CFLAGS_ALLOW and friends to find out if there are more flags not in the sample code.

ianlancetaylor · 2018-03-06T15:12:05Z

From #23749:

LDFLAGS: -Wl,--subsystem,windows
CFLAGS: -mwindows

ianlancetaylor · 2018-03-06T15:12:34Z

From #23749:

-isysroot (ios)
-mios-simulator-version-min= (ios)
-miphoneos-version-min= (ios)

-target (for android)
-sysroot (for android)

@eliasnaur Are these compiler options or linker options?

eliasnaur · 2018-03-06T18:19:40Z

-target and --sysroot are put in CFLAGS, CPPFLAGS and LDFLAGS by gomobile on android. Note that I spelled --sysroot with one dash in #23749. Sorry.

-mios-simulator-version-min=, -miphoneos-version-min=, and -isysroot are passed to CFLAGS and LDFLAGS by gomobile on ios.

andlabs · 2018-03-06T18:22:05Z

(You should probably put those iOS CFLAGS into CXXFLAGS as well, lest someone using Objective-C++ be blocked.)

eliasnaur · 2018-03-07T21:32:57Z

I found another one: "-arch ". It's added to all of CFLAG, CPPFLAG, and LDFLAG and isn't the same as -march.

mattn · 2018-03-10T13:37:17Z

-Wa,-mbig-obj to link big objects.

AlexRouSg · 2018-03-11T10:16:43Z

From #23749 (comment)
LDFLAGS: -Wl,-undefined,dynamic_lookup

coolgeek6667 · 2018-03-23T12:00:55Z

A couple more CFLAGS that I ran into in my current project:
-fsigned-char
-funsigned-char

JakeDOD · 2018-03-23T16:41:08Z

LDFLAGS: -Wl,-sectcreate,segname,sectname,filename to add sections to the binary.

From a security standpoint, on Darwin this is also an important flag for adding the __RESTRICT,__restrict segment to prevent DYLD environment variable injection.

andlabs · 2018-03-23T16:52:28Z

@JakeDOD what is this __RESTRICT,__restrict injection thing, out of curiosity?

JakeDOD · 2018-03-23T18:45:09Z

@andlabs More generally, it allows a macOS binary to opt-in as a restricted binary and tell DYLD to prune dangerous environment variables like DYLD_INSERT_LIBRARIES, override the use of @rpaths, and limit Framework and dylib loading to the SIP protected locations /System/Library/Frameworks and /usr/lib respectively. See dyld.cpp source

AlexRouSg · 2018-03-27T22:46:41Z

In case they're missed:

From #23749 (comment)

CFLAGS: -fmessage-length=152
CFLAGS: -fdiagnostics-show-note-include-stack
CFLAGS: -fmacro-backtrace-limit=0

From #23749 (comment)
(Assuming CFLAGS): -stdlib=libstdc++

gopherbot · 2018-03-28T00:07:40Z

Change https://golang.org/cl/102818 mentions this issue: cmd/go: add more C compiler/linker options to whitelist

ianlancetaylor · 2018-03-28T05:59:53Z

Reopening for 1.10.1 and 1.9.5 cherry picks.

andybons · 2018-03-28T16:02:50Z

CL 102818 OK for Go 1.10.1

gopherbot · 2018-03-28T16:11:43Z

Change https://golang.org/cl/103015 mentions this issue: [release-branch.go1.10] cmd/go: add more C compiler/linker options to whitelist

gopherbot · 2018-03-28T18:30:11Z

Change https://golang.org/cl/103156 mentions this issue: [release-branch.go1.9] cmd/go: add more C compiler/linker options to whitelist

…whitelist Fixes #23937 Change-Id: Ie63d91355d1a724d0012d99d457d939deeeb8d3e Reviewed-on: https://go-review.googlesource.com/102818 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org> Reviewed-on: https://go-review.googlesource.com/103156 Run-TryBot: Andrew Bonventre <andybons@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

… whitelist Fixes #23937 Change-Id: Ie63d91355d1a724d0012d99d457d939deeeb8d3e Reviewed-on: https://go-review.googlesource.com/102818 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org> Reviewed-on: https://go-review.googlesource.com/103015 Run-TryBot: Andrew Bonventre <andybons@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>

ianlancetaylor added the release-blocker label Feb 20, 2018

ianlancetaylor added this to the Go1.10.1 milestone Feb 20, 2018

ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 20, 2018

This was referenced Feb 20, 2018

cmd/cgo: -Wl , -rpath in LDFLAGS throws invalid flag in #cgo LDFLAGS: -Wl,-rpath=$ORIGIN/lib #23904

Closed

cmd/go: add missing C LDFLAG #23909

Closed

cmd/go: add CXXFLAGS options to whitelist #23923

Closed

ianlancetaylor mentioned this issue Feb 20, 2018

cmd/cgo: invalid flag in #cgo CFLAGS: -w #23938

Closed

andlabs mentioned this issue Feb 23, 2018

Water elemental resistance missing in Foe Editor PhantasarProductions/63fires#65

Closed

ianlancetaylor mentioned this issue Feb 25, 2018

invalid flag in #cgo CFLAGS: -arch #24113

Closed

dontpanic92 mentioned this issue Mar 12, 2018

Invalid flag in #cgo dontpanic92/wxGo#44

Closed

andlabs mentioned this issue Mar 24, 2018

harden the build andlabs/libui#6

Open

andybons mentioned this issue Mar 27, 2018

all: Go 1.10.1 release status #24563

Closed

gopherbot closed this as completed in 7e34ac1 Mar 28, 2018

ianlancetaylor reopened this Mar 28, 2018

andybons added CherryPickApproved Used during the release process for point releases and removed NeedsFix The path to resolution is known, but the work has not been done. labels Mar 28, 2018

andybons closed this as completed Mar 29, 2018

golang locked and limited conversation to collaborators Mar 29, 2019

gopherbot added the FrozenDueToAge label Mar 29, 2019

cmd/go: add more options to security whitelist #23937

cmd/go: add more options to security whitelist #23937

Comments

olt commented Feb 20, 2018

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

What operating system and processor architecture are you using (go env)?

What did you do?

What did you see instead?

ianlancetaylor commented Feb 20, 2018 • edited

ianlancetaylor commented Feb 20, 2018

ianlancetaylor commented Feb 20, 2018

andlabs commented Feb 21, 2018

ianlancetaylor commented Feb 21, 2018

ianlancetaylor commented Feb 21, 2018

andlabs commented Feb 21, 2018

AlexRouSg commented Feb 21, 2018

ianlancetaylor commented Feb 22, 2018

andlabs commented Feb 22, 2018

ianlancetaylor commented Feb 22, 2018

ianlancetaylor commented Feb 23, 2018

mattn commented Feb 23, 2018

andlabs commented Feb 25, 2018 • edited

ianlancetaylor commented Feb 25, 2018

ianlancetaylor commented Feb 25, 2018

ianlancetaylor commented Feb 25, 2018

ianlancetaylor commented Feb 26, 2018

AlexRouSg commented Feb 26, 2018 • edited

ianlancetaylor commented Mar 6, 2018

ianlancetaylor commented Mar 6, 2018

eliasnaur commented Mar 6, 2018

andlabs commented Mar 6, 2018

eliasnaur commented Mar 7, 2018

mattn commented Mar 10, 2018

AlexRouSg commented Mar 11, 2018 • edited

coolgeek6667 commented Mar 23, 2018

JakeDOD commented Mar 23, 2018

andlabs commented Mar 23, 2018

JakeDOD commented Mar 23, 2018

AlexRouSg commented Mar 27, 2018

gopherbot commented Mar 28, 2018

ianlancetaylor commented Mar 28, 2018

andybons commented Mar 28, 2018

gopherbot commented Mar 28, 2018

gopherbot commented Mar 28, 2018

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

What operating system and processor architecture are you using (`go env`)?

ianlancetaylor commented Feb 20, 2018 •

edited

andlabs commented Feb 25, 2018 •

edited

AlexRouSg commented Feb 26, 2018 •

edited

AlexRouSg commented Mar 11, 2018 •

edited