[lxc-devel] [lxd/master] seccomp: simplify and make more secure
brauner on Github
lxc-bot at linuxcontainers.org
Wed Jul 10 23:02:30 UTC 2019
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190710/9e50caab/attachment.bin>
-------------- next part --------------
From 9f1fb6f392a92fb643593d6a3d871e68ad16d076 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 11 Jul 2019 01:01:53 +0200
Subject: [PATCH] seccomp: simplify and make more secure
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
lxd/main_forkmknod.go | 126 ++++++++++++++++++++----------------
lxd/seccomp.go | 37 ++++++-----
shared/idmap/shift_linux.go | 1 +
3 files changed, 90 insertions(+), 74 deletions(-)
diff --git a/lxd/main_forkmknod.go b/lxd/main_forkmknod.go
index 0c3d08f947..682d2e321f 100644
--- a/lxd/main_forkmknod.go
+++ b/lxd/main_forkmknod.go
@@ -16,6 +16,9 @@ import (
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/capability.h>
+#include <sys/fsuid.h>
+#include <sys/prctl.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/vfs.h>
@@ -87,24 +90,19 @@ static int chowmknod(const char *path, mode_t mode, dev_t dev,
if (ret)
return -1;
- ret = chown(path, uid, gid);
- if (ret)
- (void)unlink(path);
+ if (uid >= 0 && gid >= 0) {
+ ret = chown(path, uid, gid);
+ if (ret)
+ (void)unlink(path);
+ }
return ret;
}
-static int fchowmknodat(int fd, const char *path, mode_t mode, dev_t dev,
- uid_t uid, gid_t gid)
+static int fchowmknodat(int fd, const char *path, mode_t mode, dev_t dev)
{
int ret;
ret = mknodat(fd, path, mode, dev);
- if (ret)
- return -1;
-
- ret = fchownat(fd, path, uid, gid, 0);
- if (ret)
- (void)unlinkat(fd, path, 0);
return ret;
}
@@ -152,10 +150,10 @@ static int fstat_fstatfs(int fd, struct stat *s, struct statfs *sfs)
// <PID> <root-uid> <root-gid> <path> <mode> <dev>
void forkmknod()
{
- __do_close_prot_errno int target_fd = -EBADF;
- __do_free char *target_host_dup = NULL;
+ __do_close_prot_errno int target_fd = -EBADF, host_target_fd;
int ret;
char *cur = NULL, *target = NULL, *target_host = NULL;
+ char cwd[256];
mode_t mode = 0;
dev_t dev = 0;
pid_t pid = 0;
@@ -163,7 +161,7 @@ void forkmknod()
gid_t gid = -1;
struct stat s1, s2;
struct statfs sfs1, sfs2;
- int is_shiftfs = 0;
+ cap_t caps;
// Get the subcommand
cur = advance_arg(false);
@@ -174,7 +172,7 @@ void forkmknod()
// Check that we're root
if (geteuid() != 0) {
- fprintf(stderr, "Error: forkmknod requires root privileges\n");
+ fprintf(stderr, "%d", ENOANO);
_exit(EXIT_FAILURE);
}
@@ -185,72 +183,86 @@ void forkmknod()
target_host = advance_arg(true);
uid = atoi(advance_arg(true));
gid = atoi(advance_arg(true));
- is_shiftfs = atoi(advance_arg(true));
- if (is_shiftfs == 1) {
- uid = get_ns_uid(uid, pid);
- if (uid < 0)
- fprintf(stderr, "No root uid found (%d)\n", uid);
+ snprintf(cwd, sizeof(cwd), "/proc/%d/cwd", pid);
+ target_fd = open(cwd, O_PATH | O_RDONLY | O_CLOEXEC);
+ if (target_fd < 0) {
+ fprintf(stderr, "%d", ENOANO);
+ _exit(EXIT_FAILURE);
+ }
- gid = get_ns_gid(gid, pid);
- if (gid < 0)
- fprintf(stderr, "No root gid found (%d)\n", gid);
+ host_target_fd = open(dirname(target_host), O_PATH | O_RDONLY | O_CLOEXEC);
+ if (host_target_fd < 0) {
+ fprintf(stderr, "%d", ENOANO);
+ _exit(EXIT_FAILURE);
}
- // dirname() can modify its argument
- target_host_dup = strdup(target_host);
- if (!target_host_dup)
+ (void)dosetns(pid, "mnt");
+
+ caps = cap_get_pid(pid);
+ if (!caps) {
+ fprintf(stderr, "%d", ENOANO);
_exit(EXIT_FAILURE);
+ }
- target_fd = open(dirname(target_host_dup), O_PATH | O_RDONLY | O_CLOEXEC);
+ ret = prctl(PR_SET_KEEPCAPS, 1);
+ if (ret) {
+ fprintf(stderr, "%d", ENOANO);
+ _exit(EXIT_FAILURE);
+ }
- ret = chdirchroot(pid);
- if (ret)
- goto eperm;
+ ret = setegid(gid);
+ if (ret) {
+ fprintf(stderr, "%d", ENOANO);
+ _exit(EXIT_FAILURE);
+ }
+
+ setfsgid(gid);
- ret = chowmknod(target, mode, dev, uid, gid);
+ ret = seteuid(uid);
if (ret) {
- if (errno == EEXIST) {
- fprintf(stderr, "%d", errno);
- _exit(EXIT_FAILURE);
- }
- } else {
- _exit(EXIT_SUCCESS);
+ fprintf(stderr, "%d", ENOANO);
+ _exit(EXIT_FAILURE);
}
- if (target_fd < 0)
- goto eperm;
+ setfsuid(uid);
- ret = stat_statfs(dirname(target), &s1, &sfs1);
- if (ret)
- goto eperm;
+ ret = cap_set_proc(caps);
+ if (ret) {
+ fprintf(stderr, "%d", ENOANO);
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = fstat_fstatfs(host_target_fd, &s1, &sfs1);
+ if (ret) {
+ fprintf(stderr, "%d", ENOANO);
+ _exit(EXIT_FAILURE);
+ }
ret = fstat_fstatfs(target_fd, &s2, &sfs2);
- if (ret)
- goto eperm;
+ if (ret) {
+ fprintf(stderr, "%d", ENOANO);
+ _exit(EXIT_FAILURE);
+ }
- if (!same_fsinfo(&s1, &s2, &sfs1, &sfs2))
- goto eperm;
+ if (!same_fsinfo(&s1, &s2, &sfs1, &sfs2)) {
+ fprintf(stderr, "%d", ENOMEDIUM);
+ _exit(EXIT_FAILURE);
+ }
// basename() can modify its argument so accessing target_host is
// invalid from now on.
- ret = fchowmknodat(target_fd, basename(target_host), mode, dev, uid, gid);
+ ret = mknodat(target_fd, target, mode, dev);
if (ret) {
- if (errno == EEXIST) {
- fprintf(stderr, "%d", errno);
- _exit(EXIT_FAILURE);
- }
-
- goto eperm;
+ fprintf(stderr, "%d", errno);
+ _exit(EXIT_FAILURE);
}
_exit(EXIT_SUCCESS);
-
-eperm:
- fprintf(stderr, "%d", EPERM);
- _exit(EXIT_FAILURE);
}
*/
+// #cgo CFLAGS: -std=gnu11 -Wvla
+// #cgo LDFLAGS: -lcap
import "C"
type cmdForkmknod struct {
diff --git a/lxd/seccomp.go b/lxd/seccomp.go
index 91b98603c8..278c12981f 100644
--- a/lxd/seccomp.go
+++ b/lxd/seccomp.go
@@ -786,23 +786,15 @@ func (s *SeccompServer) doMknod(c container, dev types.Device, requestPID int) (
return err, goErrno
}
- diskIdmap, err := c.DiskIdmap()
- if err != nil {
- return err, goErrno
- }
-
- isShiftfs := 0
- if s.d.os.Shiftfs && !c.IsPrivileged() && diskIdmap == nil {
- isShiftfs = 1
- }
-
prefixPath = strings.TrimPrefix(prefixPath, rootPath)
dev["hostpath"] = filepath.Join(c.RootfsPath(), rootPath, prefixPath, dev["path"])
+ logger.Errorf("AAAA: %s", dev["path"])
errnoMsg, err := shared.RunCommand(util.GetExecPath(),
"forkmknod", dev["pid"], dev["path"],
dev["mode_t"], dev["dev_t"], dev["hostpath"],
- fmt.Sprintf("%d", uid), fmt.Sprintf("%d", gid), fmt.Sprintf("%d", isShiftfs))
+ fmt.Sprintf("%d", uid), fmt.Sprintf("%d", gid))
if err != nil {
+ logger.Errorf("AAAA: %s", errnoMsg)
tmp, err2 := strconv.Atoi(errnoMsg)
if err2 == nil {
goErrno = -tmp
@@ -856,13 +848,23 @@ func (s *SeccompServer) Handler(c net.Conn, clientFd int, ucred *ucred,
c, _ := findContainerForPid(int32(msg.monitor_pid), s.d)
if c != nil {
- err, goErrno = s.doMknod(c, dev, int(cPid))
- if err != nil && (goErrno == int(-C.EPERM)) && c != nil {
+ diskIdmap, err := c.DiskIdmap()
+ if err != nil {
+ goErrno = int(-C.EPERM)
+ goto send_response
+ }
+
+ if s.d.os.Shiftfs && !c.IsPrivileged() && diskIdmap == nil {
err = c.InsertSeccompUnixDevice(fmt.Sprintf("forkmknod.unix.%d", int(cPid)), dev, int(cPid))
- if err != nil {
- goErrno = int(-C.EPERM)
- } else {
- goErrno = 0
+ } else {
+ err, goErrno = s.doMknod(c, dev, int(cPid))
+ if err != nil && (goErrno == int(-C.ENOMEDIUM)) && c != nil {
+ err = c.InsertSeccompUnixDevice(fmt.Sprintf("forkmknod.unix.%d", int(cPid)), dev, int(cPid))
+ if err != nil {
+ goErrno = int(-C.EPERM)
+ } else {
+ goErrno = 0
+ }
}
}
}
@@ -872,6 +874,7 @@ func (s *SeccompServer) Handler(c net.Conn, clientFd int, ucred *ucred,
}
}
+send_response:
C.seccomp_notify_mknod_update_response(resp, C.int(goErrno))
msghdr := C.struct_msghdr{}
diff --git a/shared/idmap/shift_linux.go b/shared/idmap/shift_linux.go
index 11a2447955..06c6aa5066 100644
--- a/shared/idmap/shift_linux.go
+++ b/shared/idmap/shift_linux.go
@@ -150,6 +150,7 @@ int shiftowner(char *basepath, char *path, int uid, int gid)
}
*/
// #cgo CFLAGS: -std=gnu11 -Wvla
+// #cgo LDFLAGS: -lcap
import "C"
// ShiftOwner updates uid and gid for a file when entering/exiting a namespace
More information about the lxc-devel
mailing list