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

bin-local Makefile target is missing #999

Closed
ilovezfs opened this issue Aug 29, 2017 · 17 comments
Closed

bin-local Makefile target is missing #999

ilovezfs opened this issue Aug 29, 2017 · 17 comments

Comments

@ilovezfs
Copy link

The bin-local Makefile target that was added in #793 is now missing in v0.19.1. As a result, the Homebrew formula is broken.

CC @sethvargo

@sethvargo
Copy link
Contributor

make dev does a local build now. They can use that.

@ilovezfs
Copy link
Author

@sethvargo As an aside, "they" is me, as I'm the lead maintainer for https://github.com/Homebrew/homebrew-core/.

The dev target does not work, as it causes a sandbox violation:

==> make dev
==> Installing consul-template for darwin/amd64
go install os/user: open /usr/local/Cellar/go/1.9/libexec/pkg/darwin_amd64/os/user.a: operation not permitted
go install net: open /usr/local/Cellar/go/1.9/libexec/pkg/darwin_amd64/net.a: operation not permitted
make: *** [dev] Error 1
/usr/local/Homebrew/Library/Homebrew/build.rb (Formulary::FormulaLoader): loading /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/python.rb
/usr/local/Homebrew/Library/Homebrew/debrew.rb:11:in `raise'
BuildError: Failed executing: make dev
1. raise
2. ignore
3. backtrace
4. irb
5. shell
Choose an action: 

@sethvargo
Copy link
Contributor

What is a sandbox violation?

We provide pre-compiled binaries via releases.hashicorp.com. Now that homebrew cask is integrated with homebrew core, it's probably easier to download and install the binary than try to compile from source each time.

@ilovezfs
Copy link
Author

The sandbox only permits the build to write to locations in the buildpath and in the prefix into which the software is being installed. In this case the build has explicitly set ENV["GOPATH"] = buildpath but the dev target is still trying to open files with write access in the go formula's prefix.

We really don't allow CLI programs in Cask with very few exceptions (e.g. the binary must be signed in order to work with Location Services).

@ilovezfs
Copy link
Author

This is the sandbox profile that is in effect during the consul-template build:

iMac-TMP:~ joe$ cat /tmp/homebrew20170830-44735-q8ewa.sb
(version 1)
(debug deny) ; log all denied operations to /var/log/system.log
(allow file-write* (subpath "/private/tmp"))
(allow file-write* (subpath "/private/var/tmp"))
(allow file-write* (regex #"^/private/var/folders/[^/]+/[^/]+/[C,T]/"))
(allow file-write* (subpath "/private/tmp"))
(allow file-write* (subpath "/Users/joe/Library/Caches/Homebrew"))
(allow file-write* (subpath "/Users/joe/Library/Logs/Homebrew/consul-template"))
(allow file-write* (subpath "/Users/joe/Library/Developer"))
(allow file-write* (subpath "/usr/local/Cellar/consul-template"))
(allow file-write* (subpath "/usr/local/etc"))
(allow file-write* (subpath "/usr/local/var"))
(allow file-write*
    (literal "/dev/ptmx")
    (literal "/dev/dtracehelper")
    (literal "/dev/null")
    (literal "/dev/random")
    (literal "/dev/zero")
    (regex #"^/dev/fd/[0-9]+$")
    (regex #"^/dev/ttys?[0-9]*$")
    )
(deny file-write*) ; deny non-whitelist file write operations
(allow process-exec
    (literal "/bin/ps")
    (with no-sandbox)
    ) ; allow certain processes running without sandbox
(allow default) ; allow everything else

@sethvargo
Copy link
Contributor

That's probably because the Makefile has to parse the version.go file to pull out the version number. I'm not really sure what you're sending over, or how I can help. The Makefile dev target is literally a go install with env isolation and CGO disabling:

dev: 
	@rm -f "${GOPATH}/pkg/${GOOS}_${GOARCH}/${PROJECT}/version.a" # ldflags change and go doesn't detect
	@env \
		CGO_ENABLED="0" \
		go install \
			-ldflags "${LD_FLAGS}" \
			-tags "${GOTAGS}"

Can you tell me what file's it's trying to write to?

I think you're making an assumption that I understand how homebrew works, which is not the case. Sorry.

@ilovezfs
Copy link
Author

ilovezfs commented Aug 30, 2017

OK so it's the CGO_ENABLED=0 that triggers it.

bash-3.2$ export CGO_ENABLED=0
bash-3.2$ go install 
go install os/user: open /usr/local/Cellar/go/1.9/libexec/pkg/darwin_amd64/os/user.a: operation not permitted
go install net: open /usr/local/Cellar/go/1.9/libexec/pkg/darwin_amd64/net.a: operation not permitted
bash-3.2$ unset CGO_ENABLED
bash-3.2$ go install 
bash-3.2$ 

@ilovezfs
Copy link
Author

@sethvargo It looks like this is being caused by golang/go#18981. Maybe a make target that doesn't set CGO_ENABLED=0 would be OK with you?

@ilovezfs
Copy link
Author

diff --git a/Formula/consul-template.rb b/Formula/consul-template.rb
index 83de16a..75e6f6f 100644
--- a/Formula/consul-template.rb
+++ b/Formula/consul-template.rb
@@ -24,8 +24,9 @@ class ConsulTemplate < Formula
     dir.install buildpath.children - [buildpath/".brew_home"]
 
     cd dir do
-      system "make", "bin-local"
-      bin.install "pkg/darwin_#{arch}/consul-template"
+      inreplace "Makefile", "CGO_ENABLED=\"0\" \\", "\\"
+      system "make", "dev"
+      bin.install buildpath/"bin/consul-template"
     end
   end

results in a successful brew build and installation.

@sethvargo
Copy link
Contributor

Compiling with CGO enabled is a security risk

@ilovezfs
Copy link
Author

Compiling with CGO enabled is a security risk

Do you have a reference for this? It seems dubious that the upstream Go defaults are a security risk in and of themselves unless something is very wrong. Moreover, I'm assuming the removal of the bin-local target was not about forcing the use of CGO_ENABLED=0.

@sethvargo
Copy link
Contributor

Do you have a reference for this?

Our security auditors have strongly recommended we do not enable dynamic linking unless we need to link to something (which this tool does not). As one concrete example, go used to link to glibc, which had a vulnerability on Feb 16, 2015.

It seems dubious that the upstream Go defaults are a security risk in and of themselves unless something is very wrong.

They aren't during cross compilation. CGO is specifically disabled during cross-compilation. However, it doesn't make sense to allow dynamic linking for local development when the final build won't be dynamically linked. That actually introduced a bug in a previous release where things worked locally for me, but the final build failed because of dynamic linking.

Moreover, I'm assuming the removal of the bin-local target was not about forcing the use of CGO_ENABLED=0.

In part, it was, from feedback from our auditors. The previous build process also relied on a number of external scripts and bash-isms, which have been refactored out.


We build pre-compiled binaries that are signed with our public GPG key and checksummed for integrity. They are guaranteed to work because we've put them through our own internal CI system.

We are not going to remove the CGO_DISABLE envvar, nor will we add a target that enables compiling without it. It's a bad practice and diverges the local build from the production build. Homebrew should either:

  1. Remove this formula entirely
  2. Move it to cask and download the pre-compiled binary
  3. Write your own install script with go install or go build and do not use the Makefile

If you choose the last option, it will be very challenging for us to support those users, since they will be building from source, using a build chain that we have no control over.

@ilovezfs
Copy link
Author

We will have to go with (3). I wanted to continue to use the Makefile specifically so that you would retain as much control as possible, especially over the ldflags, but it looks like we're just going to have to use the ldflags defaults and hope for the best if upstream is unwilling to provide a make target that doesn't attempt to alter the user's actual Go installation.

@sethvargo
Copy link
Contributor

...if upstream is unwilling to provide a make target that doesn't attempt to alter the user's actual Go installation.

That's not the case. I'm refusing to provide a make target that would produce a different binary than the resulting production build. The fact that disabling CGO causes Go to recompile it's own internal packages and violates Homebrew's sandbox is a separate issue that is already reported in golang/go#18981 as you already pointed out. Once that issue is fixed, theoretically you could use make dev normally.

@ilovezfs
Copy link
Author

Once that issue is fixed, theoretically you could use make dev normally.

Indeed. It just seems like a pretty unfortunate outcome in the interim. The situation in the interim is counterproductive because it leaves you with less control not more :(

@ilovezfs
Copy link
Author

@sethvargo I have done my best to preserve the correct ldflags: https://github.com/Homebrew/homebrew-core/pull/17258/files

@ilovezfs
Copy link
Author

@sethvargo I have now shipped 0.19.1 in Homebrew:
Homebrew/homebrew-core@fff0f01
Homebrew/homebrew-core@5be8ef1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants