Omar Polo <op@omarpolo.com> writes:

> Omar Polo <op@omarpolo.com> writes:
>
>> Bryan Steele <brynet@gmail.com> writes:
>> [...]
>>>> > I just tried on a devuan ceres (unstable) vm and they ship
>>>> > linux/landlock.h, but the actual kernel has landlock disabled!  I've
>>>> > added a check for ENOSYS and ENOTSUP to handle this situation where
>>>> > landlock was found at compile time but it's not present at runtime.
>>>> 
>>>> I'd agrue that in the ENOTSUP case it shouldn't fail silently and
>>>> should instead return an error, that way distributions attempting
>>>> to ship packages have a chance of catching it, rather than shipping
>>>> out binaries that don't actually provide any sort of protection.
>
> Please discard everything, I misunderstood.  I thought you were talking
> about dropping the check completely, not the ENOTSUP case alone.
>
> I sort of agree then, leaving ENOSYS for older kernels but removing the
> ENOTSUP to make sure that distros build the package correctly.
>
> I'll do some testing on different distribution, but I agree on the idea,
> thanks!

unfortunately, landlock being a "stackable security module" means that
users can enable and/or disable it on demand independently from the
distro.  (also don't forget that the linux people loves containers so
much, they may end up running stuff on a different kernel from the
distro they're pulling the packages from, don't know if we should take
this into account... bah)

In that devuan vm I added

	landlock=1 \
	lsm=landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,\
	selinux,smack,tomoyo"

to the linux cmdline and got landlock working.  (not sure if landlock=1
is needed)

(this because the devuan kernel was compiled without landlock in
CONFIG_LSM but with CONFIG_SECURITY_LANDLOCK=y)

So I'm not sure what to do.  I don't like this whole idea of "optional"
security mechanisms that are so easily tunable at boot time, but that's
what the linux devs came up with.  I'd like for got to be a better
example thought.

Logging in landlock_no_fs is too much noisy IMHO, as it implies that
every libexec helpers prints something.  Another option would be to do
something like the following early in got main() which produces:

	op@devuan:~/w/got-portable$ got st -S?
	got: sandboxing disabled: Operation not supported
	M  compat/landlock.c
	M  got/got.c
	M  include/got_compat.h

Thomas: do you think it's acceptable?  It adds a bit of platform
dependant code outside of compat/, but it's only in one and hopefully we
don't have to touch it for a long time.  (it's also in a section of the
code which doesn't change often, so possibly won't break with future
updates.)


diff 97799ccd4b67a81f97039305d4fdd66588da9962 /home/op/w/got-portable
blob - 47a5209dbfe20f149a142f16faaca86ca659120c
file + compat/landlock.c
--- compat/landlock.c
+++ compat/landlock.c
@@ -64,6 +64,25 @@ landlock_restrict_self(int ruleset_fd, __u32 flags)
 #endif
 
 /*
+ * Check if landlock is enabled.
+ */
+int
+landlock_enabled(void)
+{
+	struct landlock_ruleset_attr rattr = {
+		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
+	};
+	int fd;
+
+	fd = landlock_create_ruleset(&rattr, sizeof(rattr), 0);
+	if (fd == -1)
+		return 0;
+
+	close(fd);
+	return 1;
+}
+
+/*
  * Revoke any fs access.
  */
 int
blob - 2026976547ad6f6b1827376170f282135b76ad9d
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -210,6 +210,9 @@ main(int argc, char *argv[])
 
 	setlocale(LC_CTYPE, "");
 
+	if (!landlock_enabled())
+		warn("sandboxing disabled");
+
 	while ((ch = getopt_long(argc, argv, "+hV", longopts, NULL)) != -1) {
 		switch (ch) {
 		case 'h':
blob - 43e2e48d8e287757aa51bd8b89fa4d136904b88d
file + include/got_compat.h
--- include/got_compat.h
+++ include/got_compat.h
@@ -45,8 +45,10 @@
 
 #ifndef HAVE_LINUX_LANDLOCK_H
 #define landlock_no_fs() (0)
+#define landlock_enabled() (1)
 #else
 int	landlock_no_fs(void);
+int	landlock_enabled(void);
 #endif
 
 #ifndef INFTIM