Makefile: Fix rebuilding Go binaries in place to add GNU build ID

Back when we added support for GNU build IDs to our binaries we started
building our Go binaries twice: the first time we do it so that we can
derive a deterministic GNU build ID from the Go build ID, and the second
time to embed that derived GNU build ID into the final binary. This has
two problems:

    1. We build the binary twice, and even though Go caches most of the
       build process this still significantly slows down incremental
       builds of our binaries.

    2. We're rebuilding the binary in-place by overwriting the binary
       with no GNU build ID with the one that contains the GNU build ID.
       While I'm not a 100% sure, this seems to leads to issues from
       time to time where the resulting Go binary may be invalid when
       the build got cancelled at the wrong point in time. This then
       broke subsequent rebuilds of the binary.

Ideally, we wouldn't have to care about generating a deterministic GNU
build ID at all. But unfortunately, the only part of Go's build infra
that supports them is `go build`, so we have no easy way to avoid the
rebuild.

Instead, we can use a very ugly workaround though: when building the
binary, we embed a fixed GNU build ID with a known string and put this
binary into an intermediate location. We now derive the GNU build ID
from that intermediate binary, but instead of rebuilding it we simply
replace the known GNU build ID with the derived GNU build ID. Like this
we don't have to rebuild the binary but still get the same end result as
before.

This is implemented via a new naive Go tool that does this replacement
for us. Note that we cannot use e.g. sed(1) for this, and we don't want
to start depending on new tools like xxd(1). The tool is simple enough
though and allows us to have some additional safeguards to verify that
we are unlikely to wreak havoc on the binary.

2 files changed, 149 insertions, 13 deletions

diff --git a/Makefile b/Makefile
index c6afc4f27..75a8dd935 100644
--- a/Makefile
+++ b/Makefile
@@ -71,6 +71,11 @@ GO_LDFLAGS        := -X ${GITALY_PACKAGE}/internal/version.version=${GITALY_VERS
 SERVER_BUILD_TAGS := tracer_static,tracer_static_jaeger,tracer_static_stackdriver,continuous_profiler_stackdriver
 GIT2GO_BUILD_TAGS := static,system_libgit2
 
+# Temporary GNU build ID used as a placeholder value so that we can replace it
+# with our own one after binaries have been built. This is the ASCII encoding
+# of the string "TEMP_GITALY_BUILD_ID".
+TEMPORARY_BUILD_ID := 54454D505F474954414C595F4255494C445F4944
+
 ifeq (${FIPS_MODE}, 1)
     SERVER_BUILD_TAGS := ${SERVER_BUILD_TAGS},fips
     GIT2GO_BUILD_TAGS := ${GIT2GO_BUILD_TAGS},fips
@@ -278,22 +283,28 @@ help:
 ## Build Go binaries and install required Ruby Gems.
 build: ${SOURCE_DIR}/.ruby-bundle ${GITALY_EXECUTABLES}
 
-${BUILD_DIR}/bin/gitaly:            GO_BUILD_TAGS = ${SERVER_BUILD_TAGS}
-${BUILD_DIR}/bin/praefect:          GO_BUILD_TAGS = ${SERVER_BUILD_TAGS}
-${BUILD_DIR}/bin/gitaly-git2go-v14: GO_BUILD_TAGS = ${GIT2GO_BUILD_TAGS}
-${BUILD_DIR}/bin/gitaly-git2go-v14: libgit2
-
-${BUILD_DIR}/bin/%: .FORCE
-	${Q}go build -o "$@" -ldflags '${GO_LDFLAGS}' -tags "${GO_BUILD_TAGS}" $(addprefix ${SOURCE_DIR}/cmd/,$(@F))
-	@ # To compute a unique and deterministic value for GNU build-id, we build the Go binary a second time.
-	@ # From the first build, we extract its unique and deterministic Go build-id, and use that to derive
-	@ # comparably unique and deterministic GNU build-id to inject into the final binary.
+${BUILD_DIR}/bin/%: ${BUILD_DIR}/intermediate/% | ${BUILD_DIR}/bin
+	@ # To compute a unique and deterministic value for GNU build-id, we use an
+	@ # intermediate binary which has a fixed build ID of "TEMP_GITALY_BUILD_ID",
+	@ # which we replace with a deterministic build ID derived from the Go build ID.
 	@ # If we cannot extract a Go build-id, we punt and fallback to using a random 32-byte hex string.
 	@ # This fallback is unique but non-deterministic, making it sufficient to avoid generating the
 	@ # GNU build-id from the empty string and causing guaranteed collisions.
-	${Q}GO_BUILD_ID=$$( go tool buildid "$@" || openssl rand -hex 32 ) && \
-	GNU_BUILD_ID=$$( echo $$GO_BUILD_ID | sha1sum | cut -d' ' -f1 ) && \
-	go build -o "$@" -ldflags '${GO_LDFLAGS} -B 0x$$GNU_BUILD_ID' -tags "${GO_BUILD_TAGS}" $(addprefix ${SOURCE_DIR}/cmd/,$(@F))
+	${Q}GO_BUILD_ID=$$(go tool buildid "$<" || openssl rand -hex 32) && \
+	GNU_BUILD_ID=$$(echo $$GO_BUILD_ID | sha1sum | cut -d' ' -f1) && \
+	go run "${SOURCE_DIR}"/tools/replace-buildid \
+		-input "$<" -input-build-id "${TEMPORARY_BUILD_ID}" \
+		-output "$@" -output-build-id "$$GNU_BUILD_ID"
+
+${BUILD_DIR}/intermediate/gitaly:            GO_BUILD_TAGS = ${SERVER_BUILD_TAGS}
+${BUILD_DIR}/intermediate/praefect:          GO_BUILD_TAGS = ${SERVER_BUILD_TAGS}
+${BUILD_DIR}/intermediate/gitaly-git2go-v14: GO_BUILD_TAGS = ${GIT2GO_BUILD_TAGS}
+${BUILD_DIR}/intermediate/gitaly-git2go-v14: libgit2
+${BUILD_DIR}/intermediate/%: .FORCE
+	@ # We're building intermediate binaries first which contain a fixed build ID
+	@ # of "TEMP_GITALY_BUILD_ID". In the final binary we replace this build ID with
+	@ # the computed build ID for this binary.
+	${Q}go build -o "$@" -ldflags '-B 0x${TEMPORARY_BUILD_ID} ${GO_LDFLAGS}' -tags "${GO_BUILD_TAGS}" $(addprefix ${SOURCE_DIR}/cmd/,$(@F))
 
 .PHONY: install
 ## Install Gitaly binaries. The target directory can be modified by setting PREFIX and DESTDIR.
diff --git a/tools/replace-buildid/main.go b/tools/replace-buildid/main.go
new file mode 100644
index 000000000..0eab44149
--- /dev/null
+++ b/tools/replace-buildid/main.go
@@ -0,0 +1,125 @@
+// The `replace-buildid` tool is used to replace a build ID in an ELF binary with a new build ID.
+// Note that this tool is extremely naive: it simply takes the old input ID as string, verifies
+// that this ID is contained in the binary exactly once, and then replaces it. It has no knowledge
+// about ELF binaries whatsoever.
+//
+// This tool is mainly used to replace our static GNU build ID we set in our Makefile with a
+// derived build ID without having to build binaries twice.
+
+package main
+
+import (
+	"bytes"
+	"encoding/hex"
+	"flag"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+)
+
+func main() {
+	var inputPath, outputPath, inputBuildID, outputBuildID string
+
+	flag.StringVar(&inputPath, "input", "", "path to the binary whose GNU build ID should be replaced")
+	flag.StringVar(&outputPath, "output", "", "path whether the resulting binary should be placed")
+	flag.StringVar(&inputBuildID, "input-build-id", "", "static build ID to replace")
+	flag.StringVar(&outputBuildID, "output-build-id", "", "new build ID to replace old value with")
+	flag.Parse()
+
+	if err := replaceBuildID(inputPath, outputPath, inputBuildID, outputBuildID); err != nil {
+		fmt.Fprintf(os.Stderr, "%s\n", err)
+		os.Exit(1)
+	}
+}
+
+func replaceBuildID(inputPath, outputPath, inputBuildID, outputBuildID string) error {
+	if inputPath == "" {
+		return fmt.Errorf("missing input path")
+	}
+	if outputPath == "" {
+		return fmt.Errorf("missing output path")
+	}
+	if inputBuildID == "" {
+		return fmt.Errorf("missing output path")
+	}
+	if outputBuildID == "" {
+		return fmt.Errorf("missing output path")
+	}
+	if flag.NArg() > 0 {
+		return fmt.Errorf("extra arguments")
+	}
+
+	inputBuildIDDecoded, err := hex.DecodeString(inputBuildID)
+	if err != nil {
+		return fmt.Errorf("decoding input build ID: %w", err)
+	}
+
+	outputBuildIDDecoded, err := hex.DecodeString(outputBuildID)
+	if err != nil {
+		return fmt.Errorf("decoding output build ID: %w", err)
+	}
+
+	if len(inputBuildIDDecoded) != len(outputBuildIDDecoded) {
+		return fmt.Errorf("input and output build IDs do not have the same length")
+	}
+
+	data, err := readAndReplace(inputPath, inputBuildIDDecoded, outputBuildIDDecoded)
+	if err != nil {
+		return fmt.Errorf("could not replace build ID: %w", err)
+	}
+
+	if err := writeBinary(outputPath, data); err != nil {
+		return fmt.Errorf("writing binary: %w", err)
+	}
+
+	return nil
+}
+
+func readAndReplace(binaryPath string, inputBuildID, outputBuildID []byte) ([]byte, error) {
+	inputFile, err := os.Open(binaryPath)
+	if err != nil {
+		return nil, fmt.Errorf("opening input file: %w", err)
+	}
+	defer inputFile.Close()
+
+	data, err := io.ReadAll(inputFile)
+	if err != nil {
+		return nil, fmt.Errorf("reading input file: %w", err)
+	}
+
+	if occurrences := bytes.Count(data, inputBuildID); occurrences != 1 {
+		return nil, fmt.Errorf("exactly one match for old build ID expected, got %d", occurrences)
+	}
+
+	return bytes.ReplaceAll(data, inputBuildID, outputBuildID), nil
+}
+
+func writeBinary(binaryPath string, contents []byte) error {
+	f, err := os.CreateTemp(filepath.Dir(binaryPath), filepath.Base(binaryPath))
+	if err != nil {
+		return fmt.Errorf("could not create binary: %w", err)
+	}
+	defer func() {
+		_ = os.RemoveAll(f.Name())
+		f.Close()
+	}()
+
+	if err := f.Chmod(0o755); err != nil {
+		return fmt.Errorf("could not change permissions: %w", err)
+	}
+
+	if _, err := io.Copy(f, bytes.NewReader(contents)); err != nil {
+		return fmt.Errorf("could not write binary: %w", err)
+	}
+
+	if err := f.Close(); err != nil {
+		return fmt.Errorf("could not close binary: %w", err)
+	}
+
+	if err := os.Rename(f.Name(), binaryPath); err != nil {
+		return fmt.Errorf("could not move binary into place: %w", err)
+	}
+
+	return nil
+}