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: using --trimpath in go build removes -ldflags from go version -m output #63432

Open
wallyqs opened this issue Oct 7, 2023 · 12 comments · May be fixed by #67072
Open

cmd/go: using --trimpath in go build removes -ldflags from go version -m output #63432

wallyqs opened this issue Oct 7, 2023 · 12 comments · May be fixed by #67072
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@wallyqs
Copy link

wallyqs commented Oct 7, 2023

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

$ go version 1.21.2

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/example/Library/Caches/go-build'
GOENV='/Users/example/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/example/repos/dev/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/example/repos/dev'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.21.2/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.21.2/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.2'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/example/repos/dev/src/github.com/nats-io/nats-server/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/kr/7z1nxlm160ng389wq51vpktw0000gn/T/go-build4075377866=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

git clone https://github.com/nats-io/nats-server
go build -ldflags="-X main.version='2.10.2'"  --trimpath .
go version -m ./nats-server | grep ldflags

What did you expect to see?

go version -m ./nats-server | grep ldflags
	build	-ldflags="-X main.version='2.10.2'"
echo $?
0

What did you see instead?

go version -m ./nats-server | grep ldflags
echo $?
1
@cuonglm
Copy link
Member

cuonglm commented Oct 7, 2023

See: #52372

@wallyqs
Copy link
Author

wallyqs commented Oct 7, 2023

Thanks ok I see now there are a couple of TODOs so this is a known issue:

// TODO: since we control cmd/link, in theory we can parse ldflags to

Maybe just whitelisting -X main.version which is what sbom tooling like syft uses to detect a go build version might be sufficient?

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Oct 9, 2023
@dmitshur dmitshur added this to the Backlog milestone Oct 9, 2023
@dmitshur
Copy link
Contributor

dmitshur commented Oct 9, 2023

CC @bcmills, @matloob.

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 12, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 12, 2023
@bcmills
Copy link
Contributor

bcmills commented Oct 12, 2023

@wallyqs if you'd like to make progress on that TODO, you're welcome to send a fix! 🙃
(https://go.dev/doc/contribute)

I don't think we should add a special case for -X main.version, though. This is a general problem and ought to have a general solution.

@gopherbot
Copy link

Change https://go.dev/cl/536795 mentions this issue: cmd/go: add -ldflags to build metadata when CGO is disabled

@dolmen
Copy link
Contributor

dolmen commented Nov 10, 2023

What about removing -ldflags only if it contains a path separator (/, \)?

@dolmen
Copy link
Contributor

dolmen commented Nov 10, 2023

What about having special info in buildinfo to mention that -ldflags was used for build but is omitted there?

@bcmills
Copy link
Contributor

bcmills commented Nov 10, 2023

@dolmen, I think the right solution here is probably to parse the ldflags and replace absolute references in flags with with environment variables when possible (such as $GOROOT or $GOPATH or $GOMODCACHE).

On the other hand, if we can't identify a specific variable that refers to the path passed in -ldflags then I'm not sure what we should do about it. 🤔

@dolmen
Copy link
Contributor

dolmen commented Nov 13, 2023

Is go build protected against shell injection via -ldflags?

I'm thinking about the case where a tool that rebuilds binaries would take the raw ldflags value from a binary and pass it to go build...

@bcmills
Copy link
Contributor

bcmills commented Nov 14, 2023

go build disallows many flags in #cgo directives (search for CGO_CFLAGS_ALLOW in https://pkg.go.dev/cmd/cgo). I don't know whether the -ldflags flag itself has those protections.

oatovar added a commit to oatovar/trivy that referenced this issue Apr 26, 2024
* Parse the build info for -ldflags. If included, the flags will be
  further parsed, and searched for -X sub-flags. These sub-flags are
  commonly used to set the binary version during build time. A common
  pattern is to run `go build -ldflags='-X main.version=<semver>'` so
  that the main package's version variable is replaced with the latest
  tag. It's not guaranteed that this flag will propogate into the
  binary. See golang/go#63432 for more info.

* Flag parsing reuses the pflags library that's used by the main Trivy
  binary. This keeps the implementation concise, and a bit more robust
  than creating a custom flag parser for -X flag commonly passed to
  -ldflags.

* Add simple binary to test for validity of ldflags parsing. The flag

* The documentation has been updated to reflect the improved accuracy of
  the Go binary parser.
xnox added a commit to xnox/go that referenced this issue Apr 26, 2024
Add a new boolean option -trimldflags. Only meaningful when -trimpath
is true. Defaults to true for backwards compatibility. Otheriwise when
set to false reports ldflags in buildinfo, in spite of -trimpath
setting. Also when ldflags are trimmed from the output, leave a
reproducible marker that it happened.

The difference between `-trimpath` and `-trimpath -trimldflags=false`
shown below for a compilation with ldflags set:

```diff
$ diff -u default.txt trimldflags\=false.txt
--- default.txt	2024-04-26 18:51:15.106892203 +0100
+++ trimldflags=false.txt	2024-04-26 18:50:19.187008488 +0100
@@ -137,7 +137,7 @@
 	dep	sigs.k8s.io/yaml	v1.4.0	h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
 	build	-buildmode=exe
 	build	-compiler=gc
-	build	-trimldflags=true
+	build	-ldflags="-X main.Version=234"
 	build	-trimpath=true
 	build	DefaultGODEBUG=asynctimerchan=1,gotypesalias=0
 	build	CGO_ENABLED=1
```

Fixes: golang#63432
xnox added a commit to xnox/go that referenced this issue Apr 26, 2024
Add a new boolean option -trimldflags. Only meaningful when -trimpath
is true. Defaults to true for backwards compatibility. Otheriwise when
set to false reports ldflags in buildinfo, in spite of -trimpath
setting. Also when ldflags are trimmed from the output, leave a
reproducible marker that it happened.

The difference between `-trimpath` and `-trimpath -trimldflags=false`
shown below for a compilation with ldflags set:

```diff
$ diff -u default.txt trimldflags\=false.txt
--- default.txt	2024-04-26 18:51:15.106892203 +0100
+++ trimldflags=false.txt	2024-04-26 18:50:19.187008488 +0100
@@ -137,7 +137,7 @@
 	dep	sigs.k8s.io/yaml	v1.4.0	h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
 	build	-buildmode=exe
 	build	-compiler=gc
-	build	-trimldflags=true
+	build	-ldflags="-X main.Version=234"
 	build	-trimpath=true
 	build	DefaultGODEBUG=asynctimerchan=1,gotypesalias=0
 	build	CGO_ENABLED=1
```

Fixes: golang#63432
@gopherbot
Copy link

Change https://go.dev/cl/582055 mentions this issue: cmd/go: report trimpath erasing ldflags, and allow override

xnox added a commit to xnox/go that referenced this issue Apr 26, 2024
Add a new boolean option -trimldflags. Only meaningful when -trimpath
is true. Defaults to true for backwards compatibility. Otheriwise when
set to false reports ldflags in buildinfo, in spite of -trimpath
setting. Also when ldflags are trimmed from the output, leave a
reproducible marker that it happened.

Building with '-trimpath -ldflags="-X main.Version=234"' will now emit:
	build	-trimldflags=true

Adding -trimldflags=false to the above will emit ldflags:
	build	-ldflags="-X main.Version=234"

Fixes: golang#63432
oatovar added a commit to oatovar/trivy that referenced this issue Apr 28, 2024
* Parse the build info for -ldflags. If included, the flags will be
  further parsed, and searched for -X sub-flags. These sub-flags are
  commonly used to set the binary version during build time. A common
  pattern is to run `go build -ldflags='-X main.version=<semver>'` so
  that the main package's version variable is replaced with the latest
  tag. It's not guaranteed that this flag will propogate into the
  binary. See golang/go#63432 for more info.

* Flag parsing reuses the pflags library that's used by the main Trivy
  binary. This keeps the implementation concise, and a bit more robust
  than creating a custom flag parser for -X flag commonly passed to
  -ldflags.

* Add simple binary to test for validity of ldflags parsing. The flag

* The documentation has been updated to reflect the improved accuracy of
  the Go binary parser.
@xnox
Copy link

xnox commented Apr 30, 2024

Hi, I am proposing to add a flag to allow override, and emit ldflags despite trimpath active.

Another option is to a heuristic to allow ldflags..... as long as it doesn't have any slashes.

Separately, other commands allow substituting prefixes, so maybe an option to regexp substitute things in ldflags would work too?

@matloob requested to discuss this further, so happy to discuss more.

Note, that my proposal to override ldflags should not result in any detrimental behavour w.r.t. reproducible builds by default.

xnox added a commit to xnox/go that referenced this issue Apr 30, 2024
Add a new boolean option -trimldflags. Only meaningful when -trimpath
is true. Defaults to true for backwards compatibility. Otheriwise when
set to false reports ldflags in buildinfo, in spite of -trimpath
setting. Also when ldflags are trimmed from the output, leave a
reproducible marker that it happened.

Building with '-trimpath -ldflags="-X main.Version=234"' will now emit:
	build	-trimldflags=true

Adding -trimldflags=false to the above will emit ldflags:
	build	-ldflags="-X main.Version=234"

Fixes: golang#63432

Change-Id: I47d633ef0e6a90136799bef9701b7ff22c92f095
GitHub-Pull-Request: golang#67072
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants