Go's syscall package FreeBSD is missing S_IRWXG and S_IRWXO

[janl]

This patch adds a _freebsd.go file that builds instead of the regular
one only on Freebsd (to the best of my cross-platform Go abilities).

It imports the C constants S_IRWXG and S_IRWXO and uses them instead
of syscall.S_IRWXG and syscall.S_IRWXO.

Closes #2078

The relevant diff is:

--- pkg/secrets/check_right.go	2018-07-29 16:56:36.000000000 +0200
+++ pkg/secrets/check_right_freebsd.go	2018-07-29 16:56:36.000000000 +0200
@@ -3,10 +3,17 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/).
 // Copyright 2018 Datadog, Inc.
 
-// +build !windows,!freebsd
+// +build freebsd
 
 package secrets
 
+// temporary fix until go gets S_IRWXG and S_IRWXO
+
+/*
+#include <sys/stat.h>
+*/
+import "C"
+
 import (
 	"fmt"
 	"os/user"
@@ -20,7 +27,7 @@
 	}
 
 	// checking that group and others don't have any rights
-	if stat.Mode&(syscall.S_IRWXG|syscall.S_IRWXO) != 0 {
+	if stat.Mode&(C.S_IRWXG|C.S_IRWXO) != 0 {
 		return fmt.Errorf("invalid executable '%s', 'groups' or 'others' have rights on it", path)
 	}

[janl]

this line might be double the trouble, but doesn’t hurt either

[janl]

ping cc @masci <3

[olivielpeau]

Hi @janl, thanks for opening this PR! (and for the detailed explanation of the root cause)

The one downside I see to your PR is that it adds a dependency on cgo in a "core" part of the Agent, which will prevent the core of the Agent from being cross-compiled using the regular go toolchain. We want to keep the ability to do so in order to easily ship minimal Agents for all the platforms that go supports at some point in the future (we call this minimal Agent the "puppy" Agent, we don't have docs for it right now, but there's some info on #346 if you're interested).

Anyway, do you plan to use the features provided in the secrets package (documented here: https://github.com/DataDog/datadog-agent/blob/master/docs/agent/secrets.md) in your build of the Agent on FreeBSD? It's a beta feature that we only support on Linux for now.

If you don't plan to, I'd suggest removing the secrets package entirely from the FreeBSD build (by modifying the build tag on the files of the secrets package to !windows,!freebsd), and use a "placeholder" file for freebsd similar to what's done for Windows.

Let me know what you think!

[janl]

@olivielpeau thanks for the detailed and thoughtful reply. I didn’t realise secrets was something non-essential to be enabled at build time. That’s definitely good enough for me for now.

That said, an alternative patch could be a bit more “risky”. These constants are ancient Unix 25+year old constants and likely to have the same bitmask values everywhere. Given that this patch introduces a FreeBSD-only compilation path, you could be open to accept using the raw bitmask values in the two places needed. The FreeBSD-only complication path could be removed once you require a go version that this is fixed in.

If you’re interested, I can amend the patch.

[olivielpeau]

@janl Using the raw bitmask values sounds good to me, if you could amend your patch this way that'd be great, thanks 👍. Could you also leave a FIXME comment as a reminder to update it once your patch on golang is shipped in go stable?

[irabinovitch]

@janl Thanks again for your contributions here. Were you still interested in working through the changes olivielpeau suggested?

[irabinovitch]

@janl Thanks again for your contributions here. It sounds like you might have gotten busy with other projects. Should we close this PR and revisit at a future date?

[arapulido]

Thanks again @janl for your contributions here. Any chance you will be able to work on the changes suggested by @olivielpeau or shall we close for now and revisit later?