Download raw body.
[got-portable] landlock support, second try
Stefan Sperling <stsp@stsp.name> writes:
> On Fri, Jan 28, 2022 at 11:19:38PM +0100, Omar Polo wrote:
>> add landlock support on linux
>
>> +static int
>> +open_landlock(void)
>> +{
>> + struct landlock_ruleset_attr rattr = {
>> + .handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE |
>> + LANDLOCK_ACCESS_FS_WRITE_FILE |
>> + LANDLOCK_ACCESS_FS_READ_FILE |
>> + LANDLOCK_ACCESS_FS_READ_DIR |
>> + LANDLOCK_ACCESS_FS_REMOVE_DIR |
>> + LANDLOCK_ACCESS_FS_REMOVE_FILE |
>> + LANDLOCK_ACCESS_FS_MAKE_CHAR |
>> + LANDLOCK_ACCESS_FS_MAKE_DIR |
>> + LANDLOCK_ACCESS_FS_MAKE_REG |
>> + LANDLOCK_ACCESS_FS_MAKE_SOCK |
>> + LANDLOCK_ACCESS_FS_MAKE_FIFO |
>> + LANDLOCK_ACCESS_FS_MAKE_BLOCK |
>> + LANDLOCK_ACCESS_FS_MAKE_SYM,
>
> Pardon my ignorance, I don't know anything about landlock yet.
my fault, I should have added more explanations
> Is the above a list which restricts operations that the process can
> perform, or something else?
>
> Or is it a list of operations which landlock should act upon?
It's the latter. Any action not explicitly listed there is implicitly
denied, otherwise the decision is made consulting the list of rules
defined with landlock_add_rule.
(note that in this version of the diff we don't ever call
landlock_add_rule so everything is denied.)
There's a twist thought, handled_access_fs can't be zero! If it is,
landlock_create_ruleset fails with ENOMSG! That's why I kept the full
list of actions there.
But I do agree that it doesn't read good, so here's another attempt,
which also adds an explanatory comment.
I've also condensed everything into a single function. Before it would
make sense to have the initialization and locking in different
functions, since I was also emulating unveil(2), and I kept that
separation to ease the re-introduction of my unveil emulation in the
future eventually, but the code is simple enough that it doesn't really
pays off so I've simplified it further.
> If there is a possibility to here to disable features we do not need,
> please make good use of it.
>
> AFAIK the application code only uses regular files, symlinks, and directories.
> And the only file mode bit we about is the x bit.
> I don't think we would ever need to create character or block devices.
> Perhaps the imsg framework requires fifos or named sockets, but do
> we need both?
here's a revised diff. it's equivalent in practice to the previous one,
but hopefully less scary :)
P.S.: now that I think of it, there's still a thing that can be
improved. I went with compat/landlock.c because it was an easy way to
add the support, but should we move that file elsewhere?
-----------------------------------------------
commit 63534df85fc8df3ec5bcbe1bbf2c78943a1432e1 (linux)
from: Omar Polo <op@openbsd.org>
date: Sun Feb 6 10:57:38 2022 UTC
add landlock support on linux
landlock is a new set of linux APIs that is conceptually similar to
unveil(2): the idea is to restrict what a process can do on a
specified part of the filesystem. There are some differences in the
behaviour: the major one being that the landlock ruleset is inherited
across execve(2).
This "just" restricts the libexec helpers by completely revoking ANY
filesystem access; after all they are the biggest attack surface. got
send/fetch/clone *may* end up spawning ssh(1), so at the moment is not
possible to landlock the main process.
diff 4f88c9e434c3e21dfc79ed9851383863592e05b2 a57c227c001eac09b32abe7f2ca97ffdfa05db74
blob - 958a3d67dfe92ddf1caf9fd8e3586f887a36001b
blob + 0b90a45a21dee8de83282118e67277aef5bd3370
--- compat/Makefile.am
+++ compat/Makefile.am
@@ -31,6 +31,10 @@ else
libopenbsd_compat_a_SOURCES += uuid.c
endif
+if HAVE_LINUX_LANDLOCK
+libopenbsd_compat_a_SOURCES += landlock.c
+endif
+
EXTRA_DIST = \
$(top_srcdir)/include/got_compat.h \
imsg.h \
blob - /dev/null
blob + fb7acfcd3d98b9e6ab434a2d1c816cc5503c0d49 (mode 644)
--- /dev/null
+++ compat/landlock.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2021 Omar Polo <op@openbsd.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <linux/landlock.h>
+#include <linux/prctl.h>
+
+#include <sys/prctl.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <libgen.h>
+#include <limits.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "got_compat.h"
+
+/*
+ * What's the deal with landlock? While distro with linux >= 5.13
+ * have the struct declarations, libc wrappers are missing. The
+ * sample landlock code provided by the authors includes these "shims"
+ * in their example for the landlock API until libc provides them.
+ */
+
+#ifndef landlock_create_ruleset
+static inline int
+landlock_create_ruleset(const struct landlock_ruleset_attr *attr, size_t size,
+ __u32 flags)
+{
+ return syscall(__NR_landlock_create_ruleset, attr, size, flags);
+}
+#endif
+
+#ifndef landlock_add_rule
+static inline int
+landlock_add_rule(int ruleset_fd, enum landlock_rule_type type,
+ const void *attr, __u32 flags)
+{
+ return syscall(__NR_landlock_add_rule, ruleset_fd, type, attr, flags);
+}
+#endif
+
+#ifndef landlock_restrict_self
+static inline int
+landlock_restrict_self(int ruleset_fd, __u32 flags)
+{
+ return syscall(__NR_landlock_restrict_self, ruleset_fd, flags);
+}
+#endif
+
+/*
+ * Revoke any fs access.
+ */
+int
+landlock_no_fs(void)
+{
+ struct landlock_ruleset_attr rattr = {
+ /*
+ * handled_access_fs can't be zero! Even if we don't
+ * add any path at all with landlock_add_rule, and thus
+ * rejecting *any* filesystem access, we still have to
+ * list some "possible actions" here.
+ */
+ .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
+ };
+ int fd, saved_errno;
+
+ if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1)
+ return -1;
+
+ fd = landlock_create_ruleset(&rattr, sizeof(rattr), 0);
+ if (fd == -1)
+ return -1;
+
+ if (landlock_restrict_self(fd, 0)) {
+ saved_errno = errno;
+ close(fd);
+ errno = saved_errno;
+ return -1;
+ }
+
+ close(fd);
+ return 0;
+}
blob - 43a5990bf98d65ccc952753e2357ac3c76966c71
blob + f285b19bd9d1127e57c16e5cb7b19ecd0f36b028
--- configure.ac
+++ configure.ac
@@ -42,6 +42,7 @@ AC_CHECK_HEADERS([ \
fcntl.h \
langinfo.h \
limits.h \
+ linux/landlock.h \
locale.h \
netdb.h \
netinet/in.h \
@@ -163,6 +164,16 @@ AC_SUBST(PLATFORM)
AM_CONDITIONAL([HOST_FREEBSD], [test "$PLATFORM" = "freebsd"])
AM_CONDITIONAL([HOST_LINUX], [test "$PLATFORM" = "linux"])
+# Landlock detection.
+AC_MSG_CHECKING([for landlock])
+AM_CONDITIONAL([HAVE_LINUX_LANDLOCK],
+ [test "x$ac_cv_header_linux_landlock_h" = "xyes"])
+if test "x$ac_cv_header_linux_landlock_h" = "xyes"; then
+ AC_MSG_RESULT(yes)
+else
+ AC_MSG_RESULT(no)
+fi
+
# Clang sanitizers wrap reallocarray even if it isn't available on the target
# system. When compiled it always returns NULL and crashes the program. To
# detect this we need a more complicated test.
blob - c07416a808a6799841c0c9978d004659680beda2
blob + a944748ff0e57eca39bbaf958e4279cdfd6d50cc
--- include/got_compat.h
+++ include/got_compat.h
@@ -43,6 +43,10 @@
#define unveil(s, p) (0)
#endif
+#ifdef HAVE_LINUX_LANDLOCK_H
+int landlock_no_fs(void);
+#endif
+
#ifndef INFTIM
#define INFTIM -1
#endif
blob - 65c075409a0f6850f37b585385b6429b80a143ed
blob + 5d49a76cce24797690c81eb2df2097c70f190a48
--- libexec/got-fetch-pack/got-fetch-pack.c
+++ libexec/got-fetch-pack/got-fetch-pack.c
@@ -809,7 +809,15 @@ main(int argc, char **argv)
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
err = got_privsep_recv_imsg(&imsg, &ibuf, 0);
if (err) {
if (err->code == GOT_ERR_PRIVSEP_PIPE)
blob - b8572cfe5b35acf5d04456dc811ef5e1cf3b918d
blob + ed5379d6b64f8dd9f5fdb614e41bfeb866f4ed25
--- libexec/got-index-pack/got-index-pack.c
+++ libexec/got-index-pack/got-index-pack.c
@@ -1008,7 +1008,15 @@ main(int argc, char **argv)
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
err = got_privsep_recv_imsg(&imsg, &ibuf, 0);
if (err)
goto done;
blob - 24a9911b4e2319200c86e0be0d525a3980763d80
blob + f9bebfa57082a3fc3cb4d6139049a7ac13175453
--- libexec/got-read-blob/got-read-blob.c
+++ libexec/got-read-blob/got-read-blob.c
@@ -65,7 +65,15 @@ main(int argc, char *argv[])
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
for (;;) {
struct imsg imsg, imsg_outfd;
blob - f324bf809bdb1dbc68030ee2676b88f4ec11dc4e
blob + c7fd8d83bdb9a2ebb61e97f6cea0522cb877ce06
--- libexec/got-read-commit/got-read-commit.c
+++ libexec/got-read-commit/got-read-commit.c
@@ -119,7 +119,15 @@ main(int argc, char *argv[])
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
for (;;) {
struct imsg imsg;
blob - 9820c8bd9e58d215598df363fdb6e25fbd81323d
blob + dd8c66482918064be78f91b05ed28afbb23b2471
--- libexec/got-read-gitconfig/got-read-gitconfig.c
+++ libexec/got-read-gitconfig/got-read-gitconfig.c
@@ -341,7 +341,15 @@ main(int argc, char *argv[])
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
for (;;) {
struct imsg imsg;
blob - 69f6999b8df42a9f230c521287de6e5aadebd128
blob + b859ce76aec0ea4584ad8d3042fcca3cc959ac48
--- libexec/got-read-gotconfig/got-read-gotconfig.c
+++ libexec/got-read-gotconfig/got-read-gotconfig.c
@@ -498,7 +498,15 @@ main(int argc, char *argv[])
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
if (argc > 1)
filename = argv[1];
blob - 3d9bc64ad66ca532a2c58b8d50e3d4ab51abb4db
blob + a0cb6ac36a0989faf5364b55bd6c435fc155300a
--- libexec/got-read-object/got-read-object.c
+++ libexec/got-read-object/got-read-object.c
@@ -140,7 +140,15 @@ main(int argc, char *argv[])
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
for (;;) {
if (sigint_received) {
blob - 760a54783d11db6cc8fed461bc956854e6b5f2ed
blob + 00bb9ba4c8bdc10dcff315fc457391c7e0a1d090
--- libexec/got-read-pack/got-read-pack.c
+++ libexec/got-read-pack/got-read-pack.c
@@ -1031,7 +1031,15 @@ main(int argc, char *argv[])
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
err = receive_packidx(&packidx, &ibuf);
if (err) {
blob - 870e8ca6c8d4439248bc960e5d6797861de4de07
blob + 9de8f7818ae3436fdc90ca108a1ae0f0bc82544f
--- libexec/got-read-tag/got-read-tag.c
+++ libexec/got-read-tag/got-read-tag.c
@@ -114,7 +114,15 @@ main(int argc, char *argv[])
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
for (;;) {
struct imsg imsg;
blob - 35323190d763a2a21c96e0b0f6c3d19f6687f007
blob + d69822de57ceb1ae9f3ccbbeb2fa55115d3d6eea
--- libexec/got-read-tree/got-read-tree.c
+++ libexec/got-read-tree/got-read-tree.c
@@ -113,7 +113,15 @@ main(int argc, char *argv[])
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
for (;;) {
struct imsg imsg;
blob - 5a7564d43fbe801786356a9a6036bb9372e66c68
blob + ad4a3b76d7ea4c0d32c25016615561b8f26a992e
--- libexec/got-send-pack/got-send-pack.c
+++ libexec/got-send-pack/got-send-pack.c
@@ -599,7 +599,15 @@ main(int argc, char **argv)
got_privsep_send_error(&ibuf, err);
return 1;
}
+#ifdef HAVE_LINUX_LANDLOCK_H
+ /* revoke fs access */
+ if (landlock_no_fs() == -1) {
+ err = got_error_from_errno("landlock_no_fs");
+ got_privsep_send_error(&ibuf, err);
+ return 1;
+ }
#endif
+#endif
if ((err = got_privsep_recv_imsg(&imsg, &ibuf, 0)) != 0) {
if (err->code == GOT_ERR_PRIVSEP_PIPE)
err = NULL;
[got-portable] landlock support, second try