ensure the runtime is built in a reproducible way

We went to great lengths to ensure garble builds are reproducible.
This includes how the tool itself works,
as its behavior should be the same given the same inputs.

However, we made one crucial mistake with the runtime package.
It has go:linkname directives pointing at other packages,
and some of those pointed packages aren't its dependencies.

Imagine two scenarios where garble builds the runtime package:

1) We run "garble build runtime". The way we handle linkname directives
   calls listPackage on the target package, to obfuscate the target's
   import path and object name. However, since we only obtained build
   info of runtime and its deps, calls for some linknames such as
   listPackage("sync/atomic") will fail. The linkname directive will
   leave its target untouched.

2) We run "garble build std". Unlike the first scenario, all listPackage
   calls issued by runtime's linkname directives will succeed, so its
   linkname directive targets will be obfuscated.

At best, this can result in inconsistent builds, depending on how the
runtime package was built. At worst, the mismatching object names can
result in errors at link time, if the target packages are actually used.

The modified test reproduces the worst case scenario reliably,
when the fix is reverted:

	> env GOCACHE=${WORK}/gocache-empty
	> garble build -a runtime
	> garble build -o=out_rebuild ./stdimporter
	[stderr]
	# test/main/stdimporter
	JZzQivnl.NtQJu0H3: relocation target JZzQivnl.iioHinYT not defined
	JZzQivnl.NtQJu0H3.func9: relocation target JZzQivnl.yz5z0NaH not defined
	JZzQivnl.(*ypvqhKiQ).String: relocation target JZzQivnl.eVciBQeI not defined
	JZzQivnl.(*ypvqhKiQ).PkgPath: relocation target JZzQivnl.eVciBQeI not defined
	[...]

The fix consists of two steps. First, if we're building the runtime and
listPackage fails on a package, that means we ran into scenario 1 above.
To avoid the inconsistency, we fill ListedPackages with "go list [...] std".
This means we'll always build runtime as described in scenario 2 above.

Second, when building packages other than the runtime,
we only allow listPackage to succeed if we're listing a dependency of
the current package.
This ensures we won't run into similar reproducibility bugs in the future.

Finally, re-enable test-gotip on CI since this was the last test flake.
pull/435/head
Daniel Martí 3 years ago committed by Andrew LeFevre
parent bd0705a536
commit 93b2873c28

@ -32,13 +32,12 @@ jobs:
go test -race ./...
test-gotip:
if: ${{ false }} # this fails on tip: go clean -cache && go test -run Script/goprivate
runs-on: ubuntu-latest
continue-on-error: true # master may not be as stable
steps:
- name: Install Go
env:
GO_COMMIT: 578ada410de8065dbca46bca08a5993d1307f423 # 2021-11-09
GO_COMMIT: b357b05b70d2b8c4988ac2a27f2af176e7a09e1b # 2021-12-21
run: |
cd $HOME
mkdir $HOME/gotip

@ -440,7 +440,13 @@ This command wraps "go %s". Below is its help:
return nil, err
}
if err := setListedPackages(args); err != nil {
binaryBuildID, err := buildidOf(cache.ExecPath)
if err != nil {
return nil, err
}
cache.BinaryContentID = decodeHash(splitContentID(binaryBuildID))
if err := appendListedPackages(args...); err != nil {
return nil, err
}
@ -807,6 +813,12 @@ func (tf *transformer) handleDirectives(comments []*ast.CommentGroup) {
// probably a malformed linkname directive
continue
}
switch newName {
case "main.main", "main..inittask", "runtime..inittask":
// The runtime uses some special symbols with "..".
// We aren't touching those at the moment.
continue
}
// If the package path has multiple dots, split on the
// last one.

@ -10,6 +10,8 @@ import (
"path/filepath"
"strings"
"time"
"golang.org/x/mod/module"
)
// sharedCache is shared as a read-only cache between the many garble toolexec
@ -153,9 +155,9 @@ func (p *listedPackage) obfuscatedImportPath() string {
return newPath
}
// setListedPackages gets information about the current package
// appendListedPackages gets information about the current package
// and all of its dependencies
func setListedPackages(patterns []string) error {
func appendListedPackages(patterns ...string) error {
startTime := time.Now()
args := []string{"list", "-json", "-deps", "-export", "-trimpath"}
args = append(args, cache.ForwardBuildFlags...)
@ -163,7 +165,7 @@ func setListedPackages(patterns []string) error {
cmd := exec.Command("go", args...)
defer func() {
debugf("original build finished in %s via: go %s", debugSince(startTime), strings.Join(args, " "))
debugf("original build info obtained in %s via: go %s", debugSince(startTime), strings.Join(args, " "))
}()
stdout, err := cmd.StdoutPipe()
@ -178,14 +180,10 @@ func setListedPackages(patterns []string) error {
return fmt.Errorf("go list error: %v", err)
}
binaryBuildID, err := buildidOf(cache.ExecPath)
if err != nil {
return err
}
cache.BinaryContentID = decodeHash(splitContentID(binaryBuildID))
dec := json.NewDecoder(stdout)
cache.ListedPackages = make(map[string]*listedPackage)
if cache.ListedPackages == nil {
cache.ListedPackages = make(map[string]*listedPackage)
}
for dec.More() {
var pkg listedPackage
if err := dec.Decode(&pkg); err != nil {
@ -216,7 +214,8 @@ func setListedPackages(patterns []string) error {
}
}
if !anyToObfuscate {
// Don't error if the user ran: GOGARBLE='*' garble build runtime
if !anyToObfuscate && !module.MatchPrefixPatterns(cache.GOGARBLE, "runtime") {
return fmt.Errorf("GOGARBLE=%q does not match any packages to be built", cache.GOGARBLE)
}
@ -225,6 +224,10 @@ func setListedPackages(patterns []string) error {
// listPackage gets the listedPackage information for a certain package
func listPackage(path string) (*listedPackage, error) {
if path == curPkg.ImportPath {
return curPkg, nil
}
// If the path is listed in the top-level ImportMap, use its mapping instead.
// This is a common scenario when dealing with vendored packages in GOROOT.
// The map is flat, so we don't need to recurse.
@ -233,8 +236,33 @@ func listPackage(path string) (*listedPackage, error) {
}
pkg, ok := cache.ListedPackages[path]
// The runtime may list any package in std, even those it doesn't depend on.
// This is due to how it linkname-implements std packages,
// such as sync/atomic or reflect, without importing them in any way.
// If ListedPackages lacks such a package we fill it with "std".
if curPkg.ImportPath == "runtime" {
if ok {
return pkg, nil
}
if err := appendListedPackages("std"); err != nil {
panic(err) // should never happen
}
pkg, ok := cache.ListedPackages[path]
if !ok {
panic(fmt.Sprintf("runtime listed a std package we can't find: %s", path))
}
return pkg, nil
}
// Packages other than runtime can list any package,
// as long as they depend on it directly or indirectly.
if !ok {
return nil, fmt.Errorf("path not found in listed packages: %s", path)
}
return pkg, nil
for _, dep := range curPkg.Deps {
if dep == pkg.ImportPath {
return pkg, nil
}
}
return nil, fmt.Errorf("refusing to list non-dependency package: %s", path)
}

@ -38,10 +38,13 @@ garble build std
# support ImportMap when linking.
garble build -o=out ./stdimporter
# Also check that a full rebuild is reproducible,
# with -a to rebuild all packages.
# Also check that a full rebuild is reproducible, via a new GOCACHE.
# This is slow, but necessary to uncover bugs hidden by the build cache.
garble build -o=out_rebuild -a ./stdimporter
# We also forcibly rebuild runtime on its own, given it used to be non-reproducible
# due to its use of linknames pointing at std packages it doesn't depend upon.
env GOCACHE=${WORK}/gocache-empty
garble build -a runtime
garble build -o=out_rebuild ./stdimporter
bincmp out_rebuild out
-- go.mod --

@ -50,7 +50,7 @@ grep '^\s+\w+ = .*\bappend\(\w+,(\s+\w+\[\d+\][\^\-+]\w+\[\d+\],?)+\)$' debug1/t
grep '^\s+type \w+ func\(byte\) \w+$' debug1/test/main/extra_literals.go
# Finally, sanity check that we can build all of std with -literals.
# Analogous to goprivate.txt.
# Analogous to gogarble.txt.
env GOGARBLE='*'
garble -literals build std

Loading…
Cancel
Save