cmd/go: options missing from cgo whitelists

[ianlancetaylor]

The recent security patches for cmd/go (#23672) added a whitelist of options permitted for use with cgo. Several different people have reported packages that fail to build by default because they use options that are not on the whitelists. This issue is intended to collect a list of all the missing options, so that we can add them in a single CL.

[ianlancetaylor]

23737 reports that flags generated by pkg-config -libs are being checked against the compiler whitelist, and CGO_CFLAGS_ALLOW, when they should instead by checked against the linker whitelist and CGO_LDFLAGS_ALLOW. There is a CL to fix this: https://golang.org/cl/92755.

[ianlancetaylor]

The linker whitelist should accept .a files as it already accepts .o, .so, etc., files. There is a CL to fix this: https://golang.org/cl/92855. This is #23739.

[ianlancetaylor]

Comments on #23672 list these compiler options:

-fno-exceptions
-fno-rtti
-fpermissive

and these linker options:

-Wl,-framework
-Wl,--no-as-needed

[ianlancetaylor]

23742 suggests adding the compiler option -fmodules. The clang compiler supports a number of -fmodules options, but it's not clear that they are all safe. In particular -fmodules-cache-path and -fmodules-user-build-path would seem to permit specifying a path that clang will use to read modules, which could perhaps change the compilation mode in various ways.

[ianlancetaylor]

23743 suggests adding the linker option -Wl,--no-as-needed. There is a CL for this: https://golang.org/cl/92795.

[ianlancetaylor]

23744 suggests adding these compiler options:

-finput-charset=UTF-8
--std=c99
-pedantic-errors

[ianlancetaylor]

There are many compiler and linker options that may be used with either a single dash or a double dash. We should be similarly lax in the whitelists.

[andlabs]

For orthogonality: I forget if the option to add a directory to -framework's search path is already covered or not. I also forget what option that is. (The typical use case that I can think of is /Library/Frameworks, which is where Apple puts app-specific frameworks, and is not searched by default.)

[andlabs]

Also is -as-needed safe to use with cgo in the first place? This blog post (which is the first result I can find for "gcc as-needed") says that it's a positional argument, but I'm not sure if cgo guarantees anything about where your flags go on the resultant command lines.

[ianlancetaylor]

@andlabs It's fine to write

#cgo LDFLAGS: -Wl,--as-needed -loptlib -WL,--no-as-needed

In any case, the topic for this issue should be whether options are safe to use from go get.

[rgburke]

When using:

#cgo !windows pkg-config: --static ${SRCDIR}/vendor/libgit2/build/libgit2.pc

compilation fails with the following message:

invalid pkg-config package name: --static

Looking over the code (for go 1.9.4) it appears there aren't any enviroment variables that can be used to whiteslist pkg-config arguments.

[ianlancetaylor]

@rgburke pkg-config output goes through the same FLAGS_ALLOW variables as other outputs.

However, it appears that pkg-config -libs checks CGO_CFLAGS_ALLOW when it ought to check CGO_LDFLAGS_ALLOW.

[mirtchovski]

We link a large number of C libraries statically in a closed-source project. Up until now we have been doing the following:

#cgo LDFLAGS:/path/to/one/liblibrary1.a
#cgo LDFLAGS:/path/to/two/liblibrary2.a
etc.

This is of course disallowed now. A workaround:

#cgo LDFLAGS:-L/path/to/one -llibrary1
#cgo LDFLAGS:-L/path/to/two -llibrary2

However some of these directories contain dynamic libraries too, which confuses the linker. Another option is to add '/' to the list of allowed "names" in https://go-review.googlesource.com/c/go/+/92855 In particular change on line 91 is:

re(`[a-zA-Z0-9_\/].*\.(o|obj|dll|dylib|so|a)`), // direct linker inputs: x.o or libfoo.so (but not -foo.o or @foo.o)

the latter option fixes our issue but I can't speak to its impact on security.

[andlabs]

@mirtchovski there's a patch for that (the issue is that .a was not whitelisted, but other object file formats were)

[mirtchovski]

.a is now whitelisted (after that patch) so 'libsomething.a' will work, but '/path/to/libsomething.a' will not work.

[jeddenlea]

@ianlancetaylor @rgburke I actually ran into the very same problem with --static which lead me down the hole to #23737. --static is rejected because it doesn't appear to be a valid package name before attempting to execute pkg-config, here https://github.com/golang/go/blob/104445e3140f4468839db49a25cb0182f7923174/src/cmd/go/internal/work/exec.go#L939-L940.

Our quick fix internally was to point PKG_CONFIG to a script which simply executed pkg-config --static "$@".

[rgburke]

@jeddenlea - Thank you very much! I was trying to find a workaround but didn't think of that.

[bobrik]

We hit this with -msse and -msse4.2.

[minux]

I hit this issue with "${SRCDIR}/file.o" in cgo LDFLAGS.

I'd like to argue that we should allow plain filenames that is linker input files in LDFLAGS (at least .a, .o and *.so).

Unlike .a and .so which in theory could be handled with "-L${SRCDIR} -lname", adding extra object files to linker command cannot be fixed that way.

A workaround is to set CGO_LDFLAGS_ALLOW, but that's very cumbersome. Another alternative is to rename my file.o into file.syso, however, for my specific case, I can't use this option because I only include that file.o when a specific build tag is included (the build tag applies to file that contains the #cgo LDFLAGS preamble) and there is no way to set arbitrary build tag in syso filenames.

If we ignore previous deployed Go versions, we might be able to include a new "#cgo LDLIBS" that is designed specifically for adding more files to linker command line. Then we can have a stricter rule for LDLIBS (filenames only, and no dash allowed in prefix.)

[peterebden]

We've hit this with --std=c99

[FlorianUekermann]

-std=c++11

I guess -std= should be white listed?

[shuLhan]

Probably on whitelist: -fopenmp

Error when building bimg package using Go v1.10rc2,

23:24 ~/go-test
master ms 130 % go get -v github.com/h2non/bimg
github.com/h2non/bimg (download)
github.com/h2non/bimg
go build github.com/h2non/bimg: invalid flag in pkg-config --cflags: -fopenmp

[steeve]

I'm unable to whitelist -isystem as the argument next to it (which is a path) will be denied

[aronatkins]

We also needed to use this workaround: https://github.com/golang/go/issues/23749#issuecomment-364239496

We previously had:

#cgo linux LDFLAGS: ${SRCDIR}/../../../../path/to/thing/libthing_static.a

and have shifted to:

#cgo linux LDFLAGS: -L${SRCDIR}/../../../../path/to/thing -lthing_static

The .../path/to/thing is outside ${SRCDIR}.

Adding this comment so this issue includes an example with a libthing.a reference that needs SRCDIR expansion to resolve.

[glycerine]

on OSX, with the newly minted go1.9.4 with its cgo flag restrictions, I was telling the linker specifically what .a archive to link against: (here https://github.com/gijit/gi/blob/master/vendor/github.com/glycerine/golua/lua/lua.go#L10 )

#cgo LDFLAGS: ${SRCDIR}/../../../LuaJIT/LuaJIT/src/libluajit.a -lm -ldl

produces on attempt to build:

make build                                                                                      
go build -ldflags "-X main.LastGitCommitHash=30259813c10c0f6b63768b4f35358828e2e29f0b -X main.BuildTimeStamp=2018-02-09T22:49:48+0700 -X main.GitBranch=master -X main.NearestGitTag=v0.9.6  -X main.GoVersion=go_version_go1.9.4_darwin/amd64" -o gi                                         
go build github.com/gijit/gi/vendor/github.com/glycerine/golua/lua: invalid flag in #cgo LDFLAGS: /Users/jaten/go/src/github.com/gijit/gi/vendor/github.com/glycerine/golua/lua/../../../LuaJIT/LuaJIT/src/libluajit.a                                                                        
make[2]: *** [build] Error 1                                                                    
make[1]: *** [install] Error 2                                                                  
make: *** [install] Error 2                                                                     
jaten@jatens-MacBook-Pro ~/go/src/github.com/gijit/gi (master) $ 

I tried the -L -l workaround,

#cgo LDFLAGS: -L${SRCDIR}/../../../LuaJIT/LuaJIT/src -lluajit -lm -ldl

but then the linker thinks it can use a different library by the same name in a different location, and I get a runtime rather than linktime error.

at runtime...                                                  
dyld: lazy symbol binding failed: Symbol not found: _luajit_ctypeid                             
  Referenced from: /var/folders/6s/zdc0hvvx7kqcglg5yqm3kl4r0000gn/T/go-build615587282/github.com/gijit/gi/pkg/compiler/_test/compiler.test                                                     
  Expected in: /usr/local/lib/libluajit-5.1.2.dylib                                             

dyld: Symbol not found: _luajit_ctypeid                                                         
  Referenced from: /var/folders/6s/zdc0hvvx7kqcglg5yqm3kl4r0000gn/T/go-build615587282/github.com/gijit/gi/pkg/compiler/_test/compiler.test                                                     
  Expected in: /usr/local/lib/libluajit-5.1.2.dylib                                             

SIGTRAP: trace trap                                                                             
PC=0x7fff66ff4075 m=0 sigcode=1                                                                 
signal arrived during cgo execution                                                             

/usr/local/lib/libluajit-5.1.2.dylib is not the library required, though that was dynamically linked; rather it must be the one specified in ${SRCDIR}/../../../LuaJIT/LuaJIT/src/libluajit.a.

So still searching for a workaround.

Update: looks like adding this to my makefiles does the trick.

export CGO_LDFLAGS_ALLOW="${GOPATH}/src/github.com/gijit/gi/vendor/github.com/glycerine\
/golua/lua/../../../LuaJIT/LuaJIT/src/libluajit.a";

Update: Thankfully, Ian pointed out that the shorter regex version will do:

export CGO_LDFLAGS_ALLOW=".*\.a"; 

[rsc]

What is -Wl,-framework for? If that's the Apple framework, it has an argument, so you'd probably want -Wl,-framework,foo. But if it's the Apple framework then -framework (no -Wl,) works just as well.

[calmh]

With 1.10rc2, golang.org/x/net/internal/socket doesn't build any more for Solaris.

$ GOOS=solaris go build golang.org/x/net/ipv4
# golang.org/x/net/internal/socket
ext/src/golang.org/x/net/internal/socket/sys_solaris.go:24:3: //go:cgo_import_dynamic libc___xnet_getsockopt __xnet_getsockopt "libsocket.so" only allowed in cgo-generated code
ext/src/golang.org/x/net/internal/socket/sys_solaris.go:25:3: //go:cgo_import_dynamic libc_setsockopt setsockopt "libsocket.so" only allowed in cgo-generated code
ext/src/golang.org/x/net/internal/socket/sys_solaris.go:26:3: //go:cgo_import_dynamic libc___xnet_recvmsg __xnet_recvmsg "libsocket.so" only allowed in cgo-generated code
ext/src/golang.org/x/net/internal/socket/sys_solaris.go:27:3: //go:cgo_import_dynamic libc___xnet_sendmsg __xnet_sendmsg "libsocket.so" only allowed in cgo-generated code

(There's no cgo happening here as this is a cross build, but I guess that doesn't matter.)

[piotrnar]

I can't link to a static lib file by specifying a full path to it:

#cgo LDFLAGS: /usr/local/lib/libsecp256k1.a

I see no workaround for it :(

[ianlancetaylor]

@piotrnar CGO_LDFLAGS_ALLOW='.*\.a$'

[piotrnar]

@ianlancetaylor, thank you!

that will do for me, but telling all the other people who use my project to do CGO_LDFLAGS_ALLOW='.*\.a$' for go 1.9.4 seems a bit dodgy.

hopefully there will be a better (out of the box) solution in the future.

[ianlancetaylor]

@piotrnar Yes, the point of this issue is to collect all the changes we need to make to the whitelist. Certainly we will be whitelisting .a files.

[aviaoh]

can you please add also -fstack-protector?

thanks :)

[mkevac]

Whitelist -static-libstdc++ please.

[crewjam]

The package github.com/flynn/hid fails to build with 1.9.4 due to passing -fconstant-cfstrings via LDFLAGS on darwin.

[YijinLiu]

Please add these linker flags to the white list -Wl,-Bstatic -Wl,-Bdynamic -Wl,--start-group -Wl,--end-group

[nightlyone]

Honestly: At this point a blacklist of bad options seems more useful than such an extensive whitelist to me.

[ianlancetaylor]

Going forward, a blacklist would mean that if some new potentially-insecure compiler option were introduced, all Go releases would become vulnerable to that option. To the extent that we can protect against this kind of attack at all, I think it has to be a whitelist.

[glycerine]

some new potentially-insecure compiler option were introduced

I still think blacklist is preferable. Because it cuts both ways. If any new compiler option can't be utilized until it is whitelisted, that would seem to require a new Go release every time any new C compiler adds a flag...

[ianlancetaylor]

@glycerine That's why we provide the environment variables as an escape hatch. You can always use the environment variables until the whitelist is updated.

[glycerine]

The problem is that env variables don't work for projects that are installed purely through 'go get'.

Another approach: allow the top level, named go project to set environment variables.

Then the primary 'go build' project being installed can whitelist the flags it needs, e.g. using the ALLOW regex, while still preventing dependent projects from doing aribitrary things.

[ianlancetaylor]

@glycerine I'm not sure I understand how that would work. But I suggest that you open a separate issue for that suggestion, rather than discussing it on this issue, which is intended to collect options that need to be whitelisted.

[andybons]

For now we've decided on implementing a whitelist. That may change in the future but this issue is not the place to discuss it (feel free to file a new one). We are trying to collect options that should be whitelisted here and nothing more.

Thank you.

[DenkBrettl]

invalid flag in #cgo CFLAGS: -pipe

[lloeki]

go build github.com/zchee/docker-machine-driver-xhyve/vendor/github.com/zchee/libhyperkit: invalid flag in #cgo CFLAGS: -fno-common

[qeedquan]

Hi, please add these flags to the whitelist, -Wl,--enable-new-dtags

[erfanio]

Can't build therecipe/qt

go build github.com/therecipe/qt/core: invalid flag in #cgo CFLAGS: -pipe

[wheelerlaw]

Adding .*\.a to the allowed flags would be great. See https://github.com/golang/go/issues/23807

[mattn]

--mms-bitfields also seems be required.

[calmh]

Isn't it reasonable to assume that every compiler (and linker, etc) option exists for a reason, and the whitelist should cover all of them except the ones specifically deemed dangerous? Obviously there's hundreds of them and the maintenance of the white list isn't going to be fun, but if that's the path that's been decided on...

[lloeki]

Isn't it reasonable to assume that every compiler (and linker, etc) option exists for a reason, and the whitelist should cover all of them except the ones specifically deemed dangerous? Obviously there's hundreds of them and the maintenance of the white list isn't going to be fun, but if that's the path that's been decided on...

This has probably been discussed internally but I found the thing surprising for a patch release: I would have expected the offending flags to have been blacklisted first to plug the compiler plugin hole, and a whitelist system enabled for 1.10, whose list could have been built up during RC. The extra env vars are not quite practical to integrate in some build systems, and I observed this leads to people just reverting to 1.9.3 and thus being completely unprotected, which I believe is entirely counterproductive.

At which point does a whitelist full of wildcards and regexes becomes a blacklist in disguise?