From: Roberto S. <rob...@po...> - 2012-02-15 13:26:51
Attachments:
smime.p7s
|
The mount of the securityfs filesystem is now performed in the main systemd executable as it is used by IMA to provide the interface for loading custom policies. The unit file 'units/sys-kernel-security.mount' has been removed because it is not longer necessary. Signed-off-by: Roberto Sassu <rob...@po...> Acked-by: Gianluca Ramunno <ra...@po...> --- Makefile.am | 3 --- src/mount-setup.c | 6 ++++-- src/mount-setup.h | 2 ++ units/sys-kernel-security.mount | 17 ----------------- 4 files changed, 6 insertions(+), 22 deletions(-) delete mode 100644 units/sys-kernel-security.mount diff --git a/Makefile.am b/Makefile.am index 983ea16..d3d0ed8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -291,7 +291,6 @@ dist_systemunit_DATA = \ units/dev-mqueue.mount \ units/sys-kernel-config.mount \ units/sys-kernel-debug.mount \ - units/sys-kernel-security.mount \ units/sys-fs-fuse-connections.mount \ units/var-run.mount \ units/media.mount \ @@ -2374,7 +2373,6 @@ systemd-install-data-hook: dev-mqueue.mount \ sys-kernel-config.mount \ sys-kernel-debug.mount \ - sys-kernel-security.mount \ sys-fs-fuse-connections.mount \ systemd-modules-load.service \ systemd-tmpfiles-setup.service \ @@ -2384,7 +2382,6 @@ systemd-install-data-hook: $(LN_S) ../dev-mqueue.mount dev-mqueue.mount && \ $(LN_S) ../sys-kernel-config.mount sys-kernel-config.mount && \ $(LN_S) ../sys-kernel-debug.mount sys-kernel-debug.mount && \ - $(LN_S) ../sys-kernel-security.mount sys-kernel-security.mount && \ $(LN_S) ../sys-fs-fuse-connections.mount sys-fs-fuse-connections.mount && \ $(LN_S) ../systemd-modules-load.service systemd-modules-load.service && \ $(LN_S) ../systemd-tmpfiles-setup.service systemd-tmpfiles-setup.service && \ diff --git a/src/mount-setup.c b/src/mount-setup.c index 7c14ea8..62bf743 100644 --- a/src/mount-setup.c +++ b/src/mount-setup.c @@ -51,13 +51,15 @@ typedef struct MountPoint { } MountPoint; /* The first three entries we might need before SELinux is up. The - * other ones we can delay until SELinux is loaded. */ -#define N_EARLY_MOUNT 3 + * fourth (securityfs) is needed by IMA to load a custom policy. The + * other ones we can delay until SELinux and IMA are loaded. */ +#define N_EARLY_MOUNT 4 static const MountPoint mount_table[] = { { "proc", "/proc", "proc", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, { "sysfs", "/sys", "sysfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, { "devtmpfs", "/dev", "devtmpfs", "mode=755", MS_NOSUID, true }, + { "securityfs", SECURITYFS_MNTPOINT, "securityfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, false }, { "tmpfs", "/dev/shm", "tmpfs", "mode=1777", MS_NOSUID|MS_NODEV, true }, { "devpts", "/dev/pts", "devpts", "mode=620,gid=" STRINGIFY(TTY_GID), MS_NOSUID|MS_NOEXEC, false }, { "tmpfs", "/run", "tmpfs", "mode=755", MS_NOSUID|MS_NODEV, true }, diff --git a/src/mount-setup.h b/src/mount-setup.h index c1a27ba..df6f518 100644 --- a/src/mount-setup.h +++ b/src/mount-setup.h @@ -24,6 +24,8 @@ #include <stdbool.h> +#define SECURITYFS_MNTPOINT "/sys/kernel/security" + int mount_setup_early(void); int mount_setup(bool loaded_policy); diff --git a/units/sys-kernel-security.mount b/units/sys-kernel-security.mount deleted file mode 100644 index 80cd761..0000000 --- a/units/sys-kernel-security.mount +++ /dev/null @@ -1,17 +0,0 @@ -# This file is part of systemd. -# -# systemd is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. - -[Unit] -Description=Security File System -DefaultDependencies=no -ConditionPathExists=/sys/kernel/security -Before=sysinit.target - -[Mount] -What=securityfs -Where=/sys/kernel/security -Type=securityfs -- 1.7.7.6 |
From: Roberto S. <rob...@po...> - 2012-02-22 14:56:24
Attachments:
smime.p7s
|
The mount of the securityfs filesystem is now performed in the main systemd executable as it is used by IMA to provide the interface for loading custom policies. The unit file 'units/sys-kernel-security.mount' has been removed because it is not longer necessary. Signed-off-by: Roberto Sassu <rob...@po...> Acked-by: Gianluca Ramunno <ra...@po...> --- Makefile.am | 3 --- src/mount-setup.c | 6 ++++-- units/sys-kernel-security.mount | 17 ----------------- 3 files changed, 4 insertions(+), 22 deletions(-) delete mode 100644 units/sys-kernel-security.mount diff --git a/Makefile.am b/Makefile.am index ab5000b..5a50e15 100644 --- a/Makefile.am +++ b/Makefile.am @@ -291,7 +291,6 @@ dist_systemunit_DATA = \ units/dev-mqueue.mount \ units/sys-kernel-config.mount \ units/sys-kernel-debug.mount \ - units/sys-kernel-security.mount \ units/sys-fs-fuse-connections.mount \ units/var-run.mount \ units/media.mount \ @@ -2342,7 +2341,6 @@ systemd-install-data-hook: dev-mqueue.mount \ sys-kernel-config.mount \ sys-kernel-debug.mount \ - sys-kernel-security.mount \ sys-fs-fuse-connections.mount \ systemd-modules-load.service \ systemd-tmpfiles-setup.service \ @@ -2352,7 +2350,6 @@ systemd-install-data-hook: $(LN_S) ../dev-mqueue.mount dev-mqueue.mount && \ $(LN_S) ../sys-kernel-config.mount sys-kernel-config.mount && \ $(LN_S) ../sys-kernel-debug.mount sys-kernel-debug.mount && \ - $(LN_S) ../sys-kernel-security.mount sys-kernel-security.mount && \ $(LN_S) ../sys-fs-fuse-connections.mount sys-fs-fuse-connections.mount && \ $(LN_S) ../systemd-modules-load.service systemd-modules-load.service && \ $(LN_S) ../systemd-tmpfiles-setup.service systemd-tmpfiles-setup.service && \ diff --git a/src/mount-setup.c b/src/mount-setup.c index 7c14ea8..75d5cae 100644 --- a/src/mount-setup.c +++ b/src/mount-setup.c @@ -51,13 +51,15 @@ typedef struct MountPoint { } MountPoint; /* The first three entries we might need before SELinux is up. The - * other ones we can delay until SELinux is loaded. */ -#define N_EARLY_MOUNT 3 + * fourth (securityfs) is needed by IMA to load a custom policy. The + * other ones we can delay until SELinux and IMA are loaded. */ +#define N_EARLY_MOUNT 4 static const MountPoint mount_table[] = { { "proc", "/proc", "proc", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, { "sysfs", "/sys", "sysfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, { "devtmpfs", "/dev", "devtmpfs", "mode=755", MS_NOSUID, true }, + { "securityfs", "/sys/kernel/security", "securityfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, { "tmpfs", "/dev/shm", "tmpfs", "mode=1777", MS_NOSUID|MS_NODEV, true }, { "devpts", "/dev/pts", "devpts", "mode=620,gid=" STRINGIFY(TTY_GID), MS_NOSUID|MS_NOEXEC, false }, { "tmpfs", "/run", "tmpfs", "mode=755", MS_NOSUID|MS_NODEV, true }, diff --git a/units/sys-kernel-security.mount b/units/sys-kernel-security.mount deleted file mode 100644 index 80cd761..0000000 --- a/units/sys-kernel-security.mount +++ /dev/null @@ -1,17 +0,0 @@ -# This file is part of systemd. -# -# systemd is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. - -[Unit] -Description=Security File System -DefaultDependencies=no -ConditionPathExists=/sys/kernel/security -Before=sysinit.target - -[Mount] -What=securityfs -Where=/sys/kernel/security -Type=securityfs -- 1.7.7.6 |
From: Roberto S. <rob...@po...> - 2012-02-22 14:56:43
Attachments:
smime.p7s
|
The new function ima_setup() loads an IMA custom policy from a file in the default location '/etc/ima/ima-policy', if present, and writes it to the path 'ima/policy' in the security filesystem. This function is executed at early stage in order to avoid that some file operations are not measured by IMA and it is placed after the initialization of SELinux because IMA needs the latter (or other security modules) to understand LSM-specific rules. This feature is disabled by default and can be enabled by providing the option '--enable-ima' to the configure script. Signed-off-by: Roberto Sassu <rob...@po...> Acked-by: Gianluca Ramunno <ra...@po...> --- Makefile.am | 1 + configure.ac | 14 ++++++ src/build.h | 8 +++- src/ima-setup.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/ima-setup.h | 29 +++++++++++++ src/main.c | 6 ++- 6 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 src/ima-setup.c create mode 100644 src/ima-setup.h diff --git a/Makefile.am b/Makefile.am index 5a50e15..6e6d79e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -515,6 +515,7 @@ libsystemd_core_la_SOURCES = \ src/mount-setup.c \ src/hostname-setup.c \ src/selinux-setup.c \ + src/ima-setup.c \ src/loopback-setup.c \ src/kmod-setup.c \ src/locale-setup.c \ diff --git a/configure.ac b/configure.ac index 62e8cdf..93d3984 100644 --- a/configure.ac +++ b/configure.ac @@ -127,6 +127,19 @@ PKG_CHECK_MODULES(UDEV, [ libudev >= 172 ]) PKG_CHECK_MODULES(DBUS, [ dbus-1 >= 1.3.2 ]) PKG_CHECK_MODULES(KMOD, [ libkmod >= 5 ]) +have_ima=no +AC_ARG_ENABLE([ima], AS_HELP_STRING([--disable-ima],[Disable optional IMA support]), + [case "${enableval}" in + yes) have_ima=yes ;; + no) have_ima=no ;; + *) AC_MSG_ERROR(bad value ${enableval} for --disable-ima) ;; + esac], + [have_ima=no]) + +if test "x${have_ima}" != xno ; then + AC_DEFINE(HAVE_IMA, 1, [Define if IMA is available]) +fi + have_selinux=no AC_ARG_ENABLE(selinux, AS_HELP_STRING([--disable-selinux], [Disable optional SELINUX support])) if test "x$enable_selinux" != "xno"; then @@ -628,6 +641,7 @@ AC_MSG_RESULT([ tcpwrap: ${have_tcpwrap} PAM: ${have_pam} AUDIT: ${have_audit} + IMA: ${have_ima} SELinux: ${have_selinux} XZ: ${have_xz} ACL: ${have_acl} diff --git a/src/build.h b/src/build.h index 50cd79d..0619013 100644 --- a/src/build.h +++ b/src/build.h @@ -46,6 +46,12 @@ #define _SELINUX_FEATURE_ "-SELINUX" #endif +#ifdef HAVE_IMA +#define _IMA_FEATURE_ "+IMA" +#else +#define _IMA_FEATURE_ "-IMA" +#endif + #ifdef HAVE_SYSV_COMPAT #define _SYSVINIT_FEATURE_ "+SYSVINIT" #else @@ -58,6 +64,6 @@ #define _LIBCRYPTSETUP_FEATURE_ "-LIBCRYPTSETUP" #endif -#define SYSTEMD_FEATURES _PAM_FEATURE_ " " _LIBWRAP_FEATURE_ " " _AUDIT_FEATURE_ " " _SELINUX_FEATURE_ " " _SYSVINIT_FEATURE_ " " _LIBCRYPTSETUP_FEATURE_ +#define SYSTEMD_FEATURES _PAM_FEATURE_ " " _LIBWRAP_FEATURE_ " " _AUDIT_FEATURE_ " " _SELINUX_FEATURE_ " " _IMA_FEATURE_ " " _SYSVINIT_FEATURE_ " " _LIBCRYPTSETUP_FEATURE_ #endif diff --git a/src/ima-setup.c b/src/ima-setup.c new file mode 100644 index 0000000..81eb043 --- /dev/null +++ b/src/ima-setup.c @@ -0,0 +1,125 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + Copyright (C) 2012 Roberto Sassu - Politecnico di Torino, Italy + TORSEC group -- http://security.polito.it + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU General Public License + along with systemd; If not, see <http://www.gnu.org/licenses/>. +***/ + +#include <unistd.h> +#include <stdio.h> +#include <errno.h> +#include <string.h> +#include <stdlib.h> +#include <fcntl.h> +#include <sys/stat.h> +#include <sys/mman.h> + +#include "ima-setup.h" +#include "mount-setup.h" +#include "macro.h" +#include "util.h" +#include "log.h" +#include "label.h" + +#define IMA_SECFS_DIR "/sys/kernel/security/ima" +#define IMA_SECFS_POLICY IMA_SECFS_DIR "/policy" +#define IMA_POLICY_PATH "/etc/ima/ima-policy" + +int ima_setup(void) { + +#ifdef HAVE_IMA + struct stat st; + ssize_t policy_size = 0, written = 0; + char *policy; + int policyfd = -1, imafd = -1; + int policy_line_number = 1; + int result = 0; + +#ifndef HAVE_SELINUX + /* Mount the securityfs filesystem */ + mount_setup_early(); +#endif + + if (stat(IMA_POLICY_PATH, &st) < 0) + return 0; + + policy_size = st.st_size; + if (stat(IMA_SECFS_DIR, &st) < 0) { + log_debug("IMA support is disabled in the kernel, ignoring."); + return 0; + } + + if (stat(IMA_SECFS_POLICY, &st) < 0) { + log_error("Another IMA custom policy has already been loaded, " + "ignoring."); + return 0; + } + + policyfd = open(IMA_POLICY_PATH, O_RDONLY|O_CLOEXEC); + if (policyfd < 0) { + log_error("Failed to open the IMA custom policy file %s (%m), " + "ignoring.", IMA_POLICY_PATH); + return 0; + } + + imafd = open(IMA_SECFS_POLICY, O_WRONLY|O_CLOEXEC); + if (imafd < 0) { + log_error("Failed to open the IMA kernel interface %s (%m), " + "ignoring.", IMA_SECFS_POLICY); + goto out; + } + + policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0); + if (policy == MAP_FAILED) { + log_error("mmap() failed (%m), freezing"); + result = -errno; + goto out; + } + + while(written < policy_size) { + ssize_t len = write(imafd, policy + written, + policy_size - written); + if (len <= 0) { + if (errno == EINVAL) + log_error("Invalid line #%d in the IMA custom policy file %s", + policy_line_number, IMA_POLICY_PATH); + + log_error("Failed to load the IMA custom policy " + "file %s (%m), ignoring.", IMA_POLICY_PATH); + goto out_mmap; + } + written += len; + policy_line_number++; + } + + log_info("Successfully loaded the IMA custom policy %s.", + IMA_POLICY_PATH); +out_mmap: + munmap(policy, policy_size); +out: + if (policyfd >= 0) + close_nointr_nofail(policyfd); + if (imafd >= 0) + close_nointr_nofail(imafd); + if (result) + return result; +#endif /* HAVE_IMA */ + + return 0; +} diff --git a/src/ima-setup.h b/src/ima-setup.h new file mode 100644 index 0000000..7d677cf --- /dev/null +++ b/src/ima-setup.h @@ -0,0 +1,29 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +#ifndef fooimasetuphfoo +#define fooimasetuphfoo + +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + Copyright (C) 2012 Roberto Sassu - Politecnico di Torino, Italy + TORSEC group -- http://security.polito.it + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU General Public License + along with systemd; If not, see <http://www.gnu.org/licenses/>. +***/ + +int ima_setup(void); + +#endif diff --git a/src/main.c b/src/main.c index ed317b4..7ae8841 100644 --- a/src/main.c +++ b/src/main.c @@ -41,6 +41,7 @@ #include "kmod-setup.h" #include "locale-setup.h" #include "selinux-setup.h" +#include "ima-setup.h" #include "machine-id-setup.h" #include "load-fragment.h" #include "fdset.h" @@ -1203,9 +1204,12 @@ int main(int argc, char *argv[]) { arg_running_as = MANAGER_SYSTEM; log_set_target(detect_container(NULL) > 0 ? LOG_TARGET_CONSOLE : LOG_TARGET_JOURNAL_OR_KMSG); - if (!is_reexec) + if (!is_reexec) { if (selinux_setup(&loaded_policy) < 0) goto finish; + if (ima_setup() < 0) + goto finish; + } log_open(); -- 1.7.7.6 |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Lennart P. <le...@po...> - 2012-03-05 14:40:04
|
On Wed, 22.02.12 15:52, Roberto Sassu (rob...@po...) wrote: Heya, > + policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0); > + if (policy == MAP_FAILED) { > + log_error("mmap() failed (%m), freezing"); > + result = -errno; > + goto out; > + } > + > + while(written < policy_size) { > + ssize_t len = write(imafd, policy + written, > + policy_size - written); > + if (len <= 0) { > + if (errno == EINVAL) > + log_error("Invalid line #%d in the IMA custom policy file %s", > + policy_line_number, IMA_POLICY_PATH); > + > + log_error("Failed to load the IMA custom policy " > + "file %s (%m), ignoring.", IMA_POLICY_PATH); > + goto out_mmap; > + } > + written += len; > + policy_line_number++; I don't understand the counting here of policy_line_number? You attempt to write the whole policy at once, no? How does this counting of line numbers work here then? Or does the write() call on the kernel file actually only accept one line at a time? If that's the case is it really a good idea to rely on that behaviour? Knowing how these things go eventually things might get optimized to read more than one line at once and then the counting here will be off. Maybe it makes sense to drop the counting entirely here? (Something else thing that gets me thinking: by mmap()ing the source file you imply that the policy can never grow beyond 2G or so. I presume that's not a problem, right?) Otherwise looks good. Lennart -- Lennart Poettering - Red Hat, Inc. |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Roberto S. <rob...@po...> - 2012-03-05 16:45:44
|
On 03/05/2012 03:39 PM, Lennart Poettering wrote: > On Wed, 22.02.12 15:52, Roberto Sassu (rob...@po...) wrote: > > Heya, > >> + policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0); >> + if (policy == MAP_FAILED) { >> + log_error("mmap() failed (%m), freezing"); >> + result = -errno; >> + goto out; >> + } >> + >> + while(written< policy_size) { >> + ssize_t len = write(imafd, policy + written, >> + policy_size - written); >> + if (len<= 0) { >> + if (errno == EINVAL) >> + log_error("Invalid line #%d in the IMA custom policy file %s", >> + policy_line_number, IMA_POLICY_PATH); >> + >> + log_error("Failed to load the IMA custom policy " >> + "file %s (%m), ignoring.", IMA_POLICY_PATH); >> + goto out_mmap; >> + } >> + written += len; >> + policy_line_number++; > > I don't understand the counting here of policy_line_number? You attempt > to write the whole policy at once, no? How does this counting of line > numbers work here then? Or does the write() call on the kernel file > actually only accept one line at a time? If that's the case is it really > a good idea to rely on that behaviour? Knowing how these things go > eventually things might get optimized to read more than one line at once > and then the counting here will be off. Maybe it makes sense to drop the > counting entirely here? > Hi Lennart yes, the kernel interface accepts only one line at time. I implemented this code because it is not possible to known from the kernel logs what is the invalid line if the policy contains several lines. Indeed, IMA sends an audit message for each parsed rule, so that some are dropped due to the rate limit of audit. I agree that is not a good idea writing a code that depends on the specific implementation of how the policy loading is handled. So, a solution may be to drop the counting code here and to solve the issue by allowing IMA to send an audit message only when an invalid rule is encountered. Mimi, do you agree with that? Thanks Roberto Sassu > (Something else thing that gets me thinking: by mmap()ing the source > file you imply that the policy can never grow beyond 2G or so. I presume > that's not a problem, right?) > > Otherwise looks good. > > Lennart > |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Mimi Z. <zo...@li...> - 2012-03-05 18:15:52
|
On Mon, 2012-03-05 at 17:15 +0100, Roberto Sassu wrote: > On 03/05/2012 03:39 PM, Lennart Poettering wrote: > > On Wed, 22.02.12 15:52, Roberto Sassu (rob...@po...) wrote: > > > > Heya, > > > >> + policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0); > >> + if (policy == MAP_FAILED) { > >> + log_error("mmap() failed (%m), freezing"); > >> + result = -errno; > >> + goto out; > >> + } > >> + > >> + while(written< policy_size) { > >> + ssize_t len = write(imafd, policy + written, > >> + policy_size - written); > >> + if (len<= 0) { > >> + if (errno == EINVAL) > >> + log_error("Invalid line #%d in the IMA custom policy file %s", > >> + policy_line_number, IMA_POLICY_PATH); > >> + > >> + log_error("Failed to load the IMA custom policy " > >> + "file %s (%m), ignoring.", IMA_POLICY_PATH); > >> + goto out_mmap; > >> + } > >> + written += len; > >> + policy_line_number++; > > > > I don't understand the counting here of policy_line_number? You attempt > > to write the whole policy at once, no? How does this counting of line > > numbers work here then? Or does the write() call on the kernel file > > actually only accept one line at a time? If that's the case is it really > > a good idea to rely on that behaviour? Knowing how these things go > > eventually things might get optimized to read more than one line at once > > and then the counting here will be off. Maybe it makes sense to drop the > > counting entirely here? > > > > Hi Lennart > > yes, the kernel interface accepts only one line at time. I implemented > this code because it is not possible to known from the kernel logs what > is the invalid line if the policy contains several lines. Indeed, IMA > sends an audit message for each parsed rule, so that some are dropped > due to the rate limit of audit. > > I agree that is not a good idea writing a code that depends on the > specific implementation of how the policy loading is handled. So, a > solution may be to drop the counting code here and to solve the issue > by allowing IMA to send an audit message only when an invalid rule is > encountered. > > Mimi, do you agree with that? With the audit log rate limiting, the current method is not very informative. How about implementing the securityfs 'read' ops to display the rules? Then, displaying only the invalid rule makes sense. thanks, Mimi |
From: Roberto S. <rob...@po...> - 2012-03-13 16:19:14
Attachments:
smime.p7s
|
The mount of the securityfs filesystem is now performed in the main systemd executable as it is used by IMA to provide the interface for loading custom policies. The unit file 'units/sys-kernel-security.mount' has been removed because it is not longer necessary. Signed-off-by: Roberto Sassu <rob...@po...> Acked-by: Gianluca Ramunno <ra...@po...> --- Makefile.am | 3 --- src/mount-setup.c | 6 ++++-- units/sys-kernel-security.mount | 17 ----------------- 3 files changed, 4 insertions(+), 22 deletions(-) delete mode 100644 units/sys-kernel-security.mount diff --git a/Makefile.am b/Makefile.am index d2bd340..c0fcd70 100644 --- a/Makefile.am +++ b/Makefile.am @@ -291,7 +291,6 @@ dist_systemunit_DATA = \ units/dev-mqueue.mount \ units/sys-kernel-config.mount \ units/sys-kernel-debug.mount \ - units/sys-kernel-security.mount \ units/sys-fs-fuse-connections.mount \ units/var-run.mount \ units/media.mount \ @@ -2342,7 +2341,6 @@ systemd-install-data-hook: dev-mqueue.mount \ sys-kernel-config.mount \ sys-kernel-debug.mount \ - sys-kernel-security.mount \ sys-fs-fuse-connections.mount \ systemd-modules-load.service \ systemd-tmpfiles-setup.service \ @@ -2352,7 +2350,6 @@ systemd-install-data-hook: $(LN_S) ../dev-mqueue.mount dev-mqueue.mount && \ $(LN_S) ../sys-kernel-config.mount sys-kernel-config.mount && \ $(LN_S) ../sys-kernel-debug.mount sys-kernel-debug.mount && \ - $(LN_S) ../sys-kernel-security.mount sys-kernel-security.mount && \ $(LN_S) ../sys-fs-fuse-connections.mount sys-fs-fuse-connections.mount && \ $(LN_S) ../systemd-modules-load.service systemd-modules-load.service && \ $(LN_S) ../systemd-tmpfiles-setup.service systemd-tmpfiles-setup.service && \ diff --git a/src/mount-setup.c b/src/mount-setup.c index 7c14ea8..75d5cae 100644 --- a/src/mount-setup.c +++ b/src/mount-setup.c @@ -51,13 +51,15 @@ typedef struct MountPoint { } MountPoint; /* The first three entries we might need before SELinux is up. The - * other ones we can delay until SELinux is loaded. */ -#define N_EARLY_MOUNT 3 + * fourth (securityfs) is needed by IMA to load a custom policy. The + * other ones we can delay until SELinux and IMA are loaded. */ +#define N_EARLY_MOUNT 4 static const MountPoint mount_table[] = { { "proc", "/proc", "proc", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, { "sysfs", "/sys", "sysfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, { "devtmpfs", "/dev", "devtmpfs", "mode=755", MS_NOSUID, true }, + { "securityfs", "/sys/kernel/security", "securityfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, { "tmpfs", "/dev/shm", "tmpfs", "mode=1777", MS_NOSUID|MS_NODEV, true }, { "devpts", "/dev/pts", "devpts", "mode=620,gid=" STRINGIFY(TTY_GID), MS_NOSUID|MS_NOEXEC, false }, { "tmpfs", "/run", "tmpfs", "mode=755", MS_NOSUID|MS_NODEV, true }, diff --git a/units/sys-kernel-security.mount b/units/sys-kernel-security.mount deleted file mode 100644 index 80cd761..0000000 --- a/units/sys-kernel-security.mount +++ /dev/null @@ -1,17 +0,0 @@ -# This file is part of systemd. -# -# systemd is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. - -[Unit] -Description=Security File System -DefaultDependencies=no -ConditionPathExists=/sys/kernel/security -Before=sysinit.target - -[Mount] -What=securityfs -Where=/sys/kernel/security -Type=securityfs -- 1.7.7.6 |
From: Roberto S. <rob...@po...> - 2012-03-13 16:19:21
Attachments:
smime.p7s
|
The new function ima_setup() loads an IMA custom policy from a file in the default location '/etc/ima/ima-policy', if present, and writes it to the path 'ima/policy' in the security filesystem. This function is executed at early stage in order to avoid that some file operations are not measured by IMA and it is placed after the initialization of SELinux because IMA needs the latter (or other security modules) to understand LSM-specific rules. This feature is disabled by default and can be enabled by providing the option '--enable-ima' to the configure script. Signed-off-by: Roberto Sassu <rob...@po...> Acked-by: Gianluca Ramunno <ra...@po...> --- Makefile.am | 1 + configure.ac | 14 +++++++ src/build.h | 8 +++- src/ima-setup.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/ima-setup.h | 29 ++++++++++++++ src/main.c | 6 ++- 6 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 src/ima-setup.c create mode 100644 src/ima-setup.h diff --git a/Makefile.am b/Makefile.am index c0fcd70..08c7ea7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -516,6 +516,7 @@ libsystemd_core_la_SOURCES = \ src/mount-setup.c \ src/hostname-setup.c \ src/selinux-setup.c \ + src/ima-setup.c \ src/loopback-setup.c \ src/kmod-setup.c \ src/locale-setup.c \ diff --git a/configure.ac b/configure.ac index 3860088..0fe29b1 100644 --- a/configure.ac +++ b/configure.ac @@ -127,6 +127,19 @@ PKG_CHECK_MODULES(UDEV, [ libudev >= 172 ]) PKG_CHECK_MODULES(DBUS, [ dbus-1 >= 1.3.2 ]) PKG_CHECK_MODULES(KMOD, [ libkmod >= 5 ]) +have_ima=no +AC_ARG_ENABLE([ima], AS_HELP_STRING([--disable-ima],[Disable optional IMA support]), + [case "${enableval}" in + yes) have_ima=yes ;; + no) have_ima=no ;; + *) AC_MSG_ERROR(bad value ${enableval} for --disable-ima) ;; + esac], + [have_ima=no]) + +if test "x${have_ima}" != xno ; then + AC_DEFINE(HAVE_IMA, 1, [Define if IMA is available]) +fi + have_selinux=no AC_ARG_ENABLE(selinux, AS_HELP_STRING([--disable-selinux], [Disable optional SELINUX support])) if test "x$enable_selinux" != "xno"; then @@ -629,6 +642,7 @@ AC_MSG_RESULT([ tcpwrap: ${have_tcpwrap} PAM: ${have_pam} AUDIT: ${have_audit} + IMA: ${have_ima} SELinux: ${have_selinux} XZ: ${have_xz} ACL: ${have_acl} diff --git a/src/build.h b/src/build.h index 50cd79d..0619013 100644 --- a/src/build.h +++ b/src/build.h @@ -46,6 +46,12 @@ #define _SELINUX_FEATURE_ "-SELINUX" #endif +#ifdef HAVE_IMA +#define _IMA_FEATURE_ "+IMA" +#else +#define _IMA_FEATURE_ "-IMA" +#endif + #ifdef HAVE_SYSV_COMPAT #define _SYSVINIT_FEATURE_ "+SYSVINIT" #else @@ -58,6 +64,6 @@ #define _LIBCRYPTSETUP_FEATURE_ "-LIBCRYPTSETUP" #endif -#define SYSTEMD_FEATURES _PAM_FEATURE_ " " _LIBWRAP_FEATURE_ " " _AUDIT_FEATURE_ " " _SELINUX_FEATURE_ " " _SYSVINIT_FEATURE_ " " _LIBCRYPTSETUP_FEATURE_ +#define SYSTEMD_FEATURES _PAM_FEATURE_ " " _LIBWRAP_FEATURE_ " " _AUDIT_FEATURE_ " " _SELINUX_FEATURE_ " " _IMA_FEATURE_ " " _SYSVINIT_FEATURE_ " " _LIBCRYPTSETUP_FEATURE_ #endif diff --git a/src/ima-setup.c b/src/ima-setup.c new file mode 100644 index 0000000..18ed49b --- /dev/null +++ b/src/ima-setup.c @@ -0,0 +1,115 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + Copyright (C) 2012 Roberto Sassu - Politecnico di Torino, Italy + TORSEC group -- http://security.polito.it + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU General Public License + along with systemd; If not, see <http://www.gnu.org/licenses/>. +***/ + +#include <unistd.h> +#include <stdio.h> +#include <errno.h> +#include <string.h> +#include <stdlib.h> +#include <fcntl.h> +#include <sys/stat.h> +#include <sys/mman.h> + +#include "ima-setup.h" +#include "mount-setup.h" +#include "macro.h" +#include "util.h" +#include "log.h" +#include "label.h" + +#define IMA_SECFS_DIR "/sys/kernel/security/ima" +#define IMA_SECFS_POLICY IMA_SECFS_DIR "/policy" +#define IMA_POLICY_PATH "/etc/ima/ima-policy" + +int ima_setup(void) { + +#ifdef HAVE_IMA + struct stat st; + ssize_t policy_size = 0, written = 0; + char *policy; + int policyfd = -1, imafd = -1; + int result = 0; + +#ifndef HAVE_SELINUX + /* Mount the securityfs filesystem */ + mount_setup_early(); +#endif + + if (stat(IMA_POLICY_PATH, &st) < 0) + return 0; + + policy_size = st.st_size; + if (stat(IMA_SECFS_DIR, &st) < 0) { + log_debug("IMA support is disabled in the kernel, ignoring."); + return 0; + } + + if (stat(IMA_SECFS_POLICY, &st) < 0) { + log_error("Another IMA custom policy has already been loaded, " + "ignoring."); + return 0; + } + + policyfd = open(IMA_POLICY_PATH, O_RDONLY|O_CLOEXEC); + if (policyfd < 0) { + log_error("Failed to open the IMA custom policy file %s (%m), " + "ignoring.", IMA_POLICY_PATH); + return 0; + } + + imafd = open(IMA_SECFS_POLICY, O_WRONLY|O_CLOEXEC); + if (imafd < 0) { + log_error("Failed to open the IMA kernel interface %s (%m), " + "ignoring.", IMA_SECFS_POLICY); + goto out; + } + + policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0); + if (policy == MAP_FAILED) { + log_error("mmap() failed (%m), freezing"); + result = -errno; + goto out; + } + + written = loop_write(imafd, policy, (size_t)policy_size, false); + if (written != policy_size) { + log_error("Failed to load the IMA custom policy file %s (%m), " + "ignoring.", IMA_POLICY_PATH); + goto out_mmap; + } + + log_info("Successfully loaded the IMA custom policy %s.", + IMA_POLICY_PATH); +out_mmap: + munmap(policy, policy_size); +out: + if (policyfd >= 0) + close_nointr_nofail(policyfd); + if (imafd >= 0) + close_nointr_nofail(imafd); + if (result) + return result; +#endif /* HAVE_IMA */ + + return 0; +} diff --git a/src/ima-setup.h b/src/ima-setup.h new file mode 100644 index 0000000..7d677cf --- /dev/null +++ b/src/ima-setup.h @@ -0,0 +1,29 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +#ifndef fooimasetuphfoo +#define fooimasetuphfoo + +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + Copyright (C) 2012 Roberto Sassu - Politecnico di Torino, Italy + TORSEC group -- http://security.polito.it + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU General Public License + along with systemd; If not, see <http://www.gnu.org/licenses/>. +***/ + +int ima_setup(void); + +#endif diff --git a/src/main.c b/src/main.c index ed317b4..7ae8841 100644 --- a/src/main.c +++ b/src/main.c @@ -41,6 +41,7 @@ #include "kmod-setup.h" #include "locale-setup.h" #include "selinux-setup.h" +#include "ima-setup.h" #include "machine-id-setup.h" #include "load-fragment.h" #include "fdset.h" @@ -1203,9 +1204,12 @@ int main(int argc, char *argv[]) { arg_running_as = MANAGER_SYSTEM; log_set_target(detect_container(NULL) > 0 ? LOG_TARGET_CONSOLE : LOG_TARGET_JOURNAL_OR_KMSG); - if (!is_reexec) + if (!is_reexec) { if (selinux_setup(&loaded_policy) < 0) goto finish; + if (ima_setup() < 0) + goto finish; + } log_open(); -- 1.7.7.6 |
From: Roberto S. <rob...@po...> - 2012-03-13 18:41:07
|
On 03/13/2012 06:39 PM, Dave Reisner wrote: > On Tue, Mar 13, 2012 at 05:15:35PM +0100, Roberto Sassu wrote: >> The mount of the securityfs filesystem is now performed in the main systemd >> executable as it is used by IMA to provide the interface for loading custom >> policies. The unit file 'units/sys-kernel-security.mount' has been removed >> because it is not longer necessary. >> >> Signed-off-by: Roberto Sassu<rob...@po...> >> Acked-by: Gianluca Ramunno<ra...@po...> >> --- >> Makefile.am | 3 --- >> src/mount-setup.c | 6 ++++-- >> units/sys-kernel-security.mount | 17 ----------------- >> 3 files changed, 4 insertions(+), 22 deletions(-) >> delete mode 100644 units/sys-kernel-security.mount >> >> diff --git a/Makefile.am b/Makefile.am >> index d2bd340..c0fcd70 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -291,7 +291,6 @@ dist_systemunit_DATA = \ >> units/dev-mqueue.mount \ >> units/sys-kernel-config.mount \ >> units/sys-kernel-debug.mount \ >> - units/sys-kernel-security.mount \ >> units/sys-fs-fuse-connections.mount \ >> units/var-run.mount \ >> units/media.mount \ >> @@ -2342,7 +2341,6 @@ systemd-install-data-hook: >> dev-mqueue.mount \ >> sys-kernel-config.mount \ >> sys-kernel-debug.mount \ >> - sys-kernel-security.mount \ >> sys-fs-fuse-connections.mount \ >> systemd-modules-load.service \ >> systemd-tmpfiles-setup.service \ >> @@ -2352,7 +2350,6 @@ systemd-install-data-hook: >> $(LN_S) ../dev-mqueue.mount dev-mqueue.mount&& \ >> $(LN_S) ../sys-kernel-config.mount sys-kernel-config.mount&& \ >> $(LN_S) ../sys-kernel-debug.mount sys-kernel-debug.mount&& \ >> - $(LN_S) ../sys-kernel-security.mount sys-kernel-security.mount&& \ >> $(LN_S) ../sys-fs-fuse-connections.mount sys-fs-fuse-connections.mount&& \ >> $(LN_S) ../systemd-modules-load.service systemd-modules-load.service&& \ >> $(LN_S) ../systemd-tmpfiles-setup.service systemd-tmpfiles-setup.service&& \ >> diff --git a/src/mount-setup.c b/src/mount-setup.c >> index 7c14ea8..75d5cae 100644 >> --- a/src/mount-setup.c >> +++ b/src/mount-setup.c >> @@ -51,13 +51,15 @@ typedef struct MountPoint { >> } MountPoint; >> >> /* The first three entries we might need before SELinux is up. The >> - * other ones we can delay until SELinux is loaded. */ >> -#define N_EARLY_MOUNT 3 >> + * fourth (securityfs) is needed by IMA to load a custom policy. The >> + * other ones we can delay until SELinux and IMA are loaded. */ >> +#define N_EARLY_MOUNT 4 >> >> static const MountPoint mount_table[] = { >> { "proc", "/proc", "proc", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, >> { "sysfs", "/sys", "sysfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, >> { "devtmpfs", "/dev", "devtmpfs", "mode=755", MS_NOSUID, true }, >> + { "securityfs", "/sys/kernel/security", "securityfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, > > Failure to mount securtiyfs might be fatal for _your_ purposes, but I'd > wager that not only are some people not interested in this, but some > people (myself included) might not even have securityfs in their kernel. > Hi Dave i think i can change this to false without breaking the other code, because at the beginning of the new file 'src/ima-setup.c' i check for the IMA support in the kernel by checking the existence of the '/sys/kernel/security/ima' directory. If the mount fails, this will be handled as the same as when the IMA support is disabled in the kernel. This could be acceptable because IMA requires the security filesystem as dependency. I'll wait for other comments before reposting the patches. Thanks Roberto Sassu > dave > >> { "tmpfs", "/dev/shm", "tmpfs", "mode=1777", MS_NOSUID|MS_NODEV, true }, >> { "devpts", "/dev/pts", "devpts", "mode=620,gid=" STRINGIFY(TTY_GID), MS_NOSUID|MS_NOEXEC, false }, >> { "tmpfs", "/run", "tmpfs", "mode=755", MS_NOSUID|MS_NODEV, true }, >> diff --git a/units/sys-kernel-security.mount b/units/sys-kernel-security.mount >> deleted file mode 100644 >> index 80cd761..0000000 >> --- a/units/sys-kernel-security.mount >> +++ /dev/null >> @@ -1,17 +0,0 @@ >> -# This file is part of systemd. >> -# >> -# systemd is free software; you can redistribute it and/or modify it >> -# under the terms of the GNU General Public License as published by >> -# the Free Software Foundation; either version 2 of the License, or >> -# (at your option) any later version. >> - >> -[Unit] >> -Description=Security File System >> -DefaultDependencies=no >> -ConditionPathExists=/sys/kernel/security >> -Before=sysinit.target >> - >> -[Mount] >> -What=securityfs >> -Where=/sys/kernel/security >> -Type=securityfs >> -- >> 1.7.7.6 >> > > > >> _______________________________________________ >> systemd-devel mailing list >> sys...@li... >> http://lists.freedesktop.org/mailman/listinfo/systemd-devel > |
From: Dave R. <d...@fa...> - 2012-03-13 18:01:20
|
On Tue, Mar 13, 2012 at 05:15:35PM +0100, Roberto Sassu wrote: > The mount of the securityfs filesystem is now performed in the main systemd > executable as it is used by IMA to provide the interface for loading custom > policies. The unit file 'units/sys-kernel-security.mount' has been removed > because it is not longer necessary. > > Signed-off-by: Roberto Sassu <rob...@po...> > Acked-by: Gianluca Ramunno <ra...@po...> > --- > Makefile.am | 3 --- > src/mount-setup.c | 6 ++++-- > units/sys-kernel-security.mount | 17 ----------------- > 3 files changed, 4 insertions(+), 22 deletions(-) > delete mode 100644 units/sys-kernel-security.mount > > diff --git a/Makefile.am b/Makefile.am > index d2bd340..c0fcd70 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -291,7 +291,6 @@ dist_systemunit_DATA = \ > units/dev-mqueue.mount \ > units/sys-kernel-config.mount \ > units/sys-kernel-debug.mount \ > - units/sys-kernel-security.mount \ > units/sys-fs-fuse-connections.mount \ > units/var-run.mount \ > units/media.mount \ > @@ -2342,7 +2341,6 @@ systemd-install-data-hook: > dev-mqueue.mount \ > sys-kernel-config.mount \ > sys-kernel-debug.mount \ > - sys-kernel-security.mount \ > sys-fs-fuse-connections.mount \ > systemd-modules-load.service \ > systemd-tmpfiles-setup.service \ > @@ -2352,7 +2350,6 @@ systemd-install-data-hook: > $(LN_S) ../dev-mqueue.mount dev-mqueue.mount && \ > $(LN_S) ../sys-kernel-config.mount sys-kernel-config.mount && \ > $(LN_S) ../sys-kernel-debug.mount sys-kernel-debug.mount && \ > - $(LN_S) ../sys-kernel-security.mount sys-kernel-security.mount && \ > $(LN_S) ../sys-fs-fuse-connections.mount sys-fs-fuse-connections.mount && \ > $(LN_S) ../systemd-modules-load.service systemd-modules-load.service && \ > $(LN_S) ../systemd-tmpfiles-setup.service systemd-tmpfiles-setup.service && \ > diff --git a/src/mount-setup.c b/src/mount-setup.c > index 7c14ea8..75d5cae 100644 > --- a/src/mount-setup.c > +++ b/src/mount-setup.c > @@ -51,13 +51,15 @@ typedef struct MountPoint { > } MountPoint; > > /* The first three entries we might need before SELinux is up. The > - * other ones we can delay until SELinux is loaded. */ > -#define N_EARLY_MOUNT 3 > + * fourth (securityfs) is needed by IMA to load a custom policy. The > + * other ones we can delay until SELinux and IMA are loaded. */ > +#define N_EARLY_MOUNT 4 > > static const MountPoint mount_table[] = { > { "proc", "/proc", "proc", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, > { "sysfs", "/sys", "sysfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, > { "devtmpfs", "/dev", "devtmpfs", "mode=755", MS_NOSUID, true }, > + { "securityfs", "/sys/kernel/security", "securityfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, Failure to mount securtiyfs might be fatal for _your_ purposes, but I'd wager that not only are some people not interested in this, but some people (myself included) might not even have securityfs in their kernel. dave > { "tmpfs", "/dev/shm", "tmpfs", "mode=1777", MS_NOSUID|MS_NODEV, true }, > { "devpts", "/dev/pts", "devpts", "mode=620,gid=" STRINGIFY(TTY_GID), MS_NOSUID|MS_NOEXEC, false }, > { "tmpfs", "/run", "tmpfs", "mode=755", MS_NOSUID|MS_NODEV, true }, > diff --git a/units/sys-kernel-security.mount b/units/sys-kernel-security.mount > deleted file mode 100644 > index 80cd761..0000000 > --- a/units/sys-kernel-security.mount > +++ /dev/null > @@ -1,17 +0,0 @@ > -# This file is part of systemd. > -# > -# systemd is free software; you can redistribute it and/or modify it > -# under the terms of the GNU General Public License as published by > -# the Free Software Foundation; either version 2 of the License, or > -# (at your option) any later version. > - > -[Unit] > -Description=Security File System > -DefaultDependencies=no > -ConditionPathExists=/sys/kernel/security > -Before=sysinit.target > - > -[Mount] > -What=securityfs > -Where=/sys/kernel/security > -Type=securityfs > -- > 1.7.7.6 > > _______________________________________________ > systemd-devel mailing list > sys...@li... > http://lists.freedesktop.org/mailman/listinfo/systemd-devel |
From: Lennart P. <le...@po...> - 2012-03-14 16:54:22
|
On Tue, 13.03.12 19:38, Roberto Sassu (rob...@po...) wrote: > >> static const MountPoint mount_table[] = { > >> { "proc", "/proc", "proc", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, > >> { "sysfs", "/sys", "sysfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, > >> { "devtmpfs", "/dev", "devtmpfs", "mode=755", MS_NOSUID, true }, > >>+ { "securityfs", "/sys/kernel/security", "securityfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, > > > >Failure to mount securtiyfs might be fatal for _your_ purposes, but I'd > >wager that not only are some people not interested in this, but some > >people (myself included) might not even have securityfs in their kernel. > > > > Hi Dave > > i think i can change this to false without breaking > the other code, because at the beginning of the new > file 'src/ima-setup.c' i check for the IMA support in > the kernel by checking the existence of the > '/sys/kernel/security/ima' directory. If the mount > fails, this will be handled as the same as when the > IMA support is disabled in the kernel. > This could be acceptable because IMA requires the > security filesystem as dependency. > > I'll wait for other comments before reposting the patches. Yes, please change this. It is important to us that systemd works well on kernels without any special security features enabled. Also, may I ask you to turn this feature on in configure, by default? I presume that machines with this feature built into systemd but with no policy file around will boot just fine, right? Hence enabling this by default shouldn't hurt. (The reason that I want this enabled by default is that I -- or other devs -- build this locally the code as comprehensively as possible so that things don't start to bitrot that easily) Lennart -- Lennart Poettering - Red Hat, Inc. |
From: Roberto S. <rob...@po...> - 2012-03-14 17:16:38
|
On 03/14/2012 05:54 PM, Lennart Poettering wrote: > On Tue, 13.03.12 19:38, Roberto Sassu (rob...@po...) wrote: > >>>> static const MountPoint mount_table[] = { >>>> { "proc", "/proc", "proc", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, >>>> { "sysfs", "/sys", "sysfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, >>>> { "devtmpfs", "/dev", "devtmpfs", "mode=755", MS_NOSUID, true }, >>>> + { "securityfs", "/sys/kernel/security", "securityfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, >>> >>> Failure to mount securtiyfs might be fatal for _your_ purposes, but I'd >>> wager that not only are some people not interested in this, but some >>> people (myself included) might not even have securityfs in their kernel. >>> >> >> Hi Dave >> >> i think i can change this to false without breaking >> the other code, because at the beginning of the new >> file 'src/ima-setup.c' i check for the IMA support in >> the kernel by checking the existence of the >> '/sys/kernel/security/ima' directory. If the mount >> fails, this will be handled as the same as when the >> IMA support is disabled in the kernel. >> This could be acceptable because IMA requires the >> security filesystem as dependency. >> >> I'll wait for other comments before reposting the patches. > > Yes, please change this. It is important to us that systemd works well > on kernels without any special security features enabled. > Hi Lennart ok, will do. > Also, may I ask you to turn this feature on in configure, by default? I > presume that machines with this feature built into systemd but with no > policy file around will boot just fine, right? Hence enabling this by > default shouldn't hurt. > Sure. Yes, the code returns immediately if the policy file is missing. > (The reason that I want this enabled by default is that I -- or other > devs -- build this locally the code as comprehensively as possible so > that things don't start to bitrot that easily) > This is good, as users will not need to rebuild the RPM with the IMA feature enabled but they can try this functionality if they want. Regards Roberto Sassu > Lennart > |
From: Roberto S. <rob...@po...> - 2012-03-15 18:09:36
Attachments:
smime.p7s
|
The mount of the securityfs filesystem is now performed in the main systemd executable as it is used by IMA to provide the interface for loading custom policies. The unit file 'units/sys-kernel-security.mount' has been removed because it is not longer necessary. Signed-off-by: Roberto Sassu <rob...@po...> Acked-by: Gianluca Ramunno <ra...@po...> --- Makefile.am | 3 --- src/mount-setup.c | 6 ++++-- units/sys-kernel-security.mount | 17 ----------------- 3 files changed, 4 insertions(+), 22 deletions(-) delete mode 100644 units/sys-kernel-security.mount diff --git a/Makefile.am b/Makefile.am index d2bd340..c0fcd70 100644 --- a/Makefile.am +++ b/Makefile.am @@ -291,7 +291,6 @@ dist_systemunit_DATA = \ units/dev-mqueue.mount \ units/sys-kernel-config.mount \ units/sys-kernel-debug.mount \ - units/sys-kernel-security.mount \ units/sys-fs-fuse-connections.mount \ units/var-run.mount \ units/media.mount \ @@ -2342,7 +2341,6 @@ systemd-install-data-hook: dev-mqueue.mount \ sys-kernel-config.mount \ sys-kernel-debug.mount \ - sys-kernel-security.mount \ sys-fs-fuse-connections.mount \ systemd-modules-load.service \ systemd-tmpfiles-setup.service \ @@ -2352,7 +2350,6 @@ systemd-install-data-hook: $(LN_S) ../dev-mqueue.mount dev-mqueue.mount && \ $(LN_S) ../sys-kernel-config.mount sys-kernel-config.mount && \ $(LN_S) ../sys-kernel-debug.mount sys-kernel-debug.mount && \ - $(LN_S) ../sys-kernel-security.mount sys-kernel-security.mount && \ $(LN_S) ../sys-fs-fuse-connections.mount sys-fs-fuse-connections.mount && \ $(LN_S) ../systemd-modules-load.service systemd-modules-load.service && \ $(LN_S) ../systemd-tmpfiles-setup.service systemd-tmpfiles-setup.service && \ diff --git a/src/mount-setup.c b/src/mount-setup.c index 7c14ea8..aaffb65 100644 --- a/src/mount-setup.c +++ b/src/mount-setup.c @@ -51,13 +51,15 @@ typedef struct MountPoint { } MountPoint; /* The first three entries we might need before SELinux is up. The - * other ones we can delay until SELinux is loaded. */ -#define N_EARLY_MOUNT 3 + * fourth (securityfs) is needed by IMA to load a custom policy. The + * other ones we can delay until SELinux and IMA are loaded. */ +#define N_EARLY_MOUNT 4 static const MountPoint mount_table[] = { { "proc", "/proc", "proc", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, { "sysfs", "/sys", "sysfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, true }, { "devtmpfs", "/dev", "devtmpfs", "mode=755", MS_NOSUID, true }, + { "securityfs", "/sys/kernel/security", "securityfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, false }, { "tmpfs", "/dev/shm", "tmpfs", "mode=1777", MS_NOSUID|MS_NODEV, true }, { "devpts", "/dev/pts", "devpts", "mode=620,gid=" STRINGIFY(TTY_GID), MS_NOSUID|MS_NOEXEC, false }, { "tmpfs", "/run", "tmpfs", "mode=755", MS_NOSUID|MS_NODEV, true }, diff --git a/units/sys-kernel-security.mount b/units/sys-kernel-security.mount deleted file mode 100644 index 80cd761..0000000 --- a/units/sys-kernel-security.mount +++ /dev/null @@ -1,17 +0,0 @@ -# This file is part of systemd. -# -# systemd is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2 of the License, or -# (at your option) any later version. - -[Unit] -Description=Security File System -DefaultDependencies=no -ConditionPathExists=/sys/kernel/security -Before=sysinit.target - -[Mount] -What=securityfs -Where=/sys/kernel/security -Type=securityfs -- 1.7.7.6 |
From: Roberto S. <rob...@po...> - 2012-03-15 18:10:30
Attachments:
smime.p7s
|
The new function ima_setup() loads an IMA custom policy from a file in the default location '/etc/ima/ima-policy', if present, and writes it to the path 'ima/policy' in the security filesystem. This function is executed at early stage in order to avoid that some file operations are not measured by IMA and it is placed after the initialization of SELinux because IMA needs the latter (or other security modules) to understand LSM-specific rules. This feature is enabled by default and can be disabled by providing the option '--disable-ima' to the configure script. Signed-off-by: Roberto Sassu <rob...@po...> Acked-by: Gianluca Ramunno <ra...@po...> --- Makefile.am | 1 + configure.ac | 14 +++++++ src/build.h | 8 +++- src/ima-setup.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/ima-setup.h | 29 ++++++++++++++ src/main.c | 6 ++- 6 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 src/ima-setup.c create mode 100644 src/ima-setup.h diff --git a/Makefile.am b/Makefile.am index c0fcd70..08c7ea7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -516,6 +516,7 @@ libsystemd_core_la_SOURCES = \ src/mount-setup.c \ src/hostname-setup.c \ src/selinux-setup.c \ + src/ima-setup.c \ src/loopback-setup.c \ src/kmod-setup.c \ src/locale-setup.c \ diff --git a/configure.ac b/configure.ac index 3860088..b562078 100644 --- a/configure.ac +++ b/configure.ac @@ -127,6 +127,19 @@ PKG_CHECK_MODULES(UDEV, [ libudev >= 172 ]) PKG_CHECK_MODULES(DBUS, [ dbus-1 >= 1.3.2 ]) PKG_CHECK_MODULES(KMOD, [ libkmod >= 5 ]) +have_ima=yes +AC_ARG_ENABLE([ima], AS_HELP_STRING([--disable-ima],[Disable optional IMA support]), + [case "${enableval}" in + yes) have_ima=yes ;; + no) have_ima=no ;; + *) AC_MSG_ERROR(bad value ${enableval} for --disable-ima) ;; + esac], + [have_ima=yes]) + +if test "x${have_ima}" != xno ; then + AC_DEFINE(HAVE_IMA, 1, [Define if IMA is available]) +fi + have_selinux=no AC_ARG_ENABLE(selinux, AS_HELP_STRING([--disable-selinux], [Disable optional SELINUX support])) if test "x$enable_selinux" != "xno"; then @@ -629,6 +642,7 @@ AC_MSG_RESULT([ tcpwrap: ${have_tcpwrap} PAM: ${have_pam} AUDIT: ${have_audit} + IMA: ${have_ima} SELinux: ${have_selinux} XZ: ${have_xz} ACL: ${have_acl} diff --git a/src/build.h b/src/build.h index 50cd79d..0619013 100644 --- a/src/build.h +++ b/src/build.h @@ -46,6 +46,12 @@ #define _SELINUX_FEATURE_ "-SELINUX" #endif +#ifdef HAVE_IMA +#define _IMA_FEATURE_ "+IMA" +#else +#define _IMA_FEATURE_ "-IMA" +#endif + #ifdef HAVE_SYSV_COMPAT #define _SYSVINIT_FEATURE_ "+SYSVINIT" #else @@ -58,6 +64,6 @@ #define _LIBCRYPTSETUP_FEATURE_ "-LIBCRYPTSETUP" #endif -#define SYSTEMD_FEATURES _PAM_FEATURE_ " " _LIBWRAP_FEATURE_ " " _AUDIT_FEATURE_ " " _SELINUX_FEATURE_ " " _SYSVINIT_FEATURE_ " " _LIBCRYPTSETUP_FEATURE_ +#define SYSTEMD_FEATURES _PAM_FEATURE_ " " _LIBWRAP_FEATURE_ " " _AUDIT_FEATURE_ " " _SELINUX_FEATURE_ " " _IMA_FEATURE_ " " _SYSVINIT_FEATURE_ " " _LIBCRYPTSETUP_FEATURE_ #endif diff --git a/src/ima-setup.c b/src/ima-setup.c new file mode 100644 index 0000000..03e43dc --- /dev/null +++ b/src/ima-setup.c @@ -0,0 +1,115 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + Copyright (C) 2012 Roberto Sassu - Politecnico di Torino, Italy + TORSEC group -- http://security.polito.it + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU General Public License + along with systemd; If not, see <http://www.gnu.org/licenses/>. +***/ + +#include <unistd.h> +#include <stdio.h> +#include <errno.h> +#include <string.h> +#include <stdlib.h> +#include <fcntl.h> +#include <sys/stat.h> +#include <sys/mman.h> + +#include "ima-setup.h" +#include "mount-setup.h" +#include "macro.h" +#include "util.h" +#include "log.h" +#include "label.h" + +#define IMA_SECFS_DIR "/sys/kernel/security/ima" +#define IMA_SECFS_POLICY IMA_SECFS_DIR "/policy" +#define IMA_POLICY_PATH "/etc/ima/ima-policy" + +int ima_setup(void) { + +#ifdef HAVE_IMA + struct stat st; + ssize_t policy_size = 0, written = 0; + char *policy; + int policyfd = -1, imafd = -1; + int result = 0; + +#ifndef HAVE_SELINUX + /* Mount the securityfs filesystem */ + mount_setup_early(); +#endif + + if (stat(IMA_POLICY_PATH, &st) < 0) + return 0; + + policy_size = st.st_size; + if (stat(IMA_SECFS_DIR, &st) < 0) { + log_debug("IMA support is disabled in the kernel, ignoring."); + return 0; + } + + if (stat(IMA_SECFS_POLICY, &st) < 0) { + log_error("Another IMA custom policy has already been loaded, " + "ignoring."); + return 0; + } + + policyfd = open(IMA_POLICY_PATH, O_RDONLY|O_CLOEXEC); + if (policyfd < 0) { + log_error("Failed to open the IMA custom policy file %s (%m), " + "ignoring.", IMA_POLICY_PATH); + return 0; + } + + imafd = open(IMA_SECFS_POLICY, O_WRONLY|O_CLOEXEC); + if (imafd < 0) { + log_error("Failed to open the IMA kernel interface %s (%m), " + "ignoring.", IMA_SECFS_POLICY); + goto out; + } + + policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0); + if (policy == MAP_FAILED) { + log_error("mmap() failed (%m), freezing"); + result = -errno; + goto out; + } + + written = loop_write(imafd, policy, (size_t)policy_size, false); + if (written != policy_size) { + log_error("Failed to load the IMA custom policy file %s (%m), " + "ignoring.", IMA_POLICY_PATH); + goto out_mmap; + } + + log_info("Successfully loaded the IMA custom policy %s.", + IMA_POLICY_PATH); +out_mmap: + munmap(policy, policy_size); +out: + if (policyfd >= 0) + close_nointr_nofail(policyfd); + if (imafd >= 0) + close_nointr_nofail(imafd); + if (result) + return result; +#endif /* HAVE_IMA */ + + return 0; +} diff --git a/src/ima-setup.h b/src/ima-setup.h new file mode 100644 index 0000000..7d677cf --- /dev/null +++ b/src/ima-setup.h @@ -0,0 +1,29 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +#ifndef fooimasetuphfoo +#define fooimasetuphfoo + +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + Copyright (C) 2012 Roberto Sassu - Politecnico di Torino, Italy + TORSEC group -- http://security.polito.it + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU General Public License + along with systemd; If not, see <http://www.gnu.org/licenses/>. +***/ + +int ima_setup(void); + +#endif diff --git a/src/main.c b/src/main.c index ed317b4..7ae8841 100644 --- a/src/main.c +++ b/src/main.c @@ -41,6 +41,7 @@ #include "kmod-setup.h" #include "locale-setup.h" #include "selinux-setup.h" +#include "ima-setup.h" #include "machine-id-setup.h" #include "load-fragment.h" #include "fdset.h" @@ -1203,9 +1204,12 @@ int main(int argc, char *argv[]) { arg_running_as = MANAGER_SYSTEM; log_set_target(detect_container(NULL) > 0 ? LOG_TARGET_CONSOLE : LOG_TARGET_JOURNAL_OR_KMSG); - if (!is_reexec) + if (!is_reexec) { if (selinux_setup(&loaded_policy) < 0) goto finish; + if (ima_setup() < 0) + goto finish; + } log_open(); -- 1.7.7.6 |
From: Lennart P. <le...@po...> - 2012-03-21 23:25:50
|
On Thu, 15.03.12 19:06, Roberto Sassu (rob...@po...) wrote: > The mount of the securityfs filesystem is now performed in the main systemd > executable as it is used by IMA to provide the interface for loading custom > policies. The unit file 'units/sys-kernel-security.mount' has been removed > because it is not longer necessary. Applied both patches! Thanks a lot for your work! Lennart -- Lennart Poettering - Red Hat, Inc. |
From: Roberto S. <rob...@po...> - 2012-03-22 08:53:32
|
On 03/22/2012 12:25 AM, Lennart Poettering wrote: > On Thu, 15.03.12 19:06, Roberto Sassu (rob...@po...) wrote: > >> The mount of the securityfs filesystem is now performed in the main systemd >> executable as it is used by IMA to provide the interface for loading custom >> policies. The unit file 'units/sys-kernel-security.mount' has been removed >> because it is not longer necessary. > > Applied both patches! > Hi Lennart thanks for accepting them! Regards Roberto Sassu > Thanks a lot for your work! > > Lennart > |
From: Roberto S. <rob...@po...> - 2012-02-15 13:27:22
Attachments:
smime.p7s
|
The new function ima_setup() loads an IMA custom policy from a file in the default location '/etc/sysconfig/ima-policy', if present, and writes it to the path 'ima/policy' in the security filesystem. This function is executed at early stage in order to avoid that some file operations are not measured by IMA and it is placed after the initialization of SELinux because IMA needs the latter (or other security modules) to understand LSM-specific rules. Signed-off-by: Roberto Sassu <rob...@po...> Acked-by: Gianluca Ramunno <ra...@po...> --- Makefile.am | 1 + src/ima-setup.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/ima-setup.h | 29 ++++++++++++++ src/main.c | 6 ++- 4 files changed, 149 insertions(+), 1 deletions(-) create mode 100644 src/ima-setup.c create mode 100644 src/ima-setup.h diff --git a/Makefile.am b/Makefile.am index d3d0ed8..7476caa 100644 --- a/Makefile.am +++ b/Makefile.am @@ -515,6 +515,7 @@ libsystemd_core_la_SOURCES = \ src/mount-setup.c \ src/hostname-setup.c \ src/selinux-setup.c \ + src/ima-setup.c \ src/loopback-setup.c \ src/kmod-setup.c \ src/locale-setup.c \ diff --git a/src/ima-setup.c b/src/ima-setup.c new file mode 100644 index 0000000..45afc0c --- /dev/null +++ b/src/ima-setup.c @@ -0,0 +1,114 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + Copyright (C) 2012 Roberto Sassu - Politecnico di Torino, Italy + TORSEC group -- http://security.polito.it + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU General Public License + along with systemd; If not, see <http://www.gnu.org/licenses/>. +***/ + +#include <unistd.h> +#include <stdio.h> +#include <errno.h> +#include <string.h> +#include <stdlib.h> +#include <fcntl.h> +#include <sys/stat.h> +#include <sys/mman.h> + +#include "ima-setup.h" +#include "mount-setup.h" +#include "macro.h" +#include "util.h" +#include "log.h" +#include "label.h" + +#define IMA_SECFS_DIR SECURITYFS_MNTPOINT "/ima" +#define IMA_SECFS_POLICY IMA_SECFS_DIR "/policy" +#define IMA_POLICY_PATH "/etc/sysconfig/ima-policy" + +int ima_setup(void) { + struct stat st; + ssize_t policy_size = 0, written = 0; + char *policy; + int policyfd = -1, imafd = -1; + int result = 0; + +#ifndef HAVE_SELINUX + /* Mount the securityfs filesystem */ + mount_setup_early(); +#endif + + if (stat(IMA_POLICY_PATH, &st) == -1) + return 0; + + policy_size = st.st_size; + if (stat(IMA_SECFS_DIR, &st) == -1) { + log_debug("IMA support is disabled in the kernel, ignoring."); + return 0; + } + + if (stat(IMA_SECFS_POLICY, &st) == -1) { + log_error("Another IMA custom policy has already been loaded, " + "ignoring."); + return 0; + } + + policyfd = open(IMA_POLICY_PATH, O_RDONLY); + if (policyfd < 0) { + log_error("Failed to open the IMA custom policy file %s (%s), " + "ignoring.", IMA_POLICY_PATH, strerror(errno)); + return 0; + } + + imafd = open(IMA_SECFS_POLICY, O_WRONLY); + if (imafd < 0) { + log_error("Failed to open the IMA kernel interface %s (%s), " + "ignoring.", IMA_SECFS_POLICY, strerror(errno)); + goto out; + } + + policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0); + if (policy == NULL) { + log_error("mmap() failed (%s), freezing", strerror(errno)); + result = -errno; + goto out; + } + + while(written < policy_size) { + ssize_t len = write(imafd, policy + written, + policy_size - written); + if (len <= 0) { + log_error("Failed to load the IMA custom policy " + "file %s (%s), ignoring.", IMA_POLICY_PATH, + strerror(errno)); + goto out_mmap; + } + written += len; + } + + log_info("Successfully loaded the IMA custom policy %s.", + IMA_POLICY_PATH); +out_mmap: + munmap(policy, policy_size); +out: + if (policyfd >= 0) + close_nointr_nofail(policyfd); + if (imafd >= 0) + close_nointr_nofail(imafd); + return result; +} diff --git a/src/ima-setup.h b/src/ima-setup.h new file mode 100644 index 0000000..7d677cf --- /dev/null +++ b/src/ima-setup.h @@ -0,0 +1,29 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +#ifndef fooimasetuphfoo +#define fooimasetuphfoo + +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + Copyright (C) 2012 Roberto Sassu - Politecnico di Torino, Italy + TORSEC group -- http://security.polito.it + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + General Public License for more details. + + You should have received a copy of the GNU General Public License + along with systemd; If not, see <http://www.gnu.org/licenses/>. +***/ + +int ima_setup(void); + +#endif diff --git a/src/main.c b/src/main.c index ed317b4..7ae8841 100644 --- a/src/main.c +++ b/src/main.c @@ -41,6 +41,7 @@ #include "kmod-setup.h" #include "locale-setup.h" #include "selinux-setup.h" +#include "ima-setup.h" #include "machine-id-setup.h" #include "load-fragment.h" #include "fdset.h" @@ -1203,9 +1204,12 @@ int main(int argc, char *argv[]) { arg_running_as = MANAGER_SYSTEM; log_set_target(detect_container(NULL) > 0 ? LOG_TARGET_CONSOLE : LOG_TARGET_JOURNAL_OR_KMSG); - if (!is_reexec) + if (!is_reexec) { if (selinux_setup(&loaded_policy) < 0) goto finish; + if (ima_setup() < 0) + goto finish; + } log_open(); -- 1.7.7.6 |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Gustavo S. B. <bar...@pr...> - 2012-02-16 21:51:03
|
On Thu, Feb 16, 2012 at 12:35 PM, Roberto Sassu <rob...@po...> wrote: > On 02/16/2012 03:30 PM, Gustavo Sverzut Barbieri wrote: >> >> On Thu, Feb 16, 2012 at 11:38 AM, Roberto Sassu<rob...@po...> >> wrote: >>> >>> On 02/16/2012 05:56 AM, Michael Cassaniti wrote: >>>> >>>> >>>> On 16/02/2012 04:12, Roberto Sassu wrote: >>>>> >>>>> >>>>> On 02/15/2012 05:55 PM, Gustavo Sverzut Barbieri wrote: >>>>>> >>>>>> >>>>>> On Wed, Feb 15, 2012 at 2:26 PM, Roberto >>>>>> Sassu<rob...@po...> wrote: >>>>>>> >>>>>>> >>>>>>> On 02/15/2012 03:30 PM, Gustavo Sverzut Barbieri wrote: >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Feb 15, 2012 at 11:23 AM, Roberto >>>>>>>> Sassu<rob...@po...> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> The new function ima_setup() loads an IMA custom policy from a >>>>>>>>> file in the >>>>>>>>> default location '/etc/sysconfig/ima-policy', if present, and >>>>>>>>> writes it to >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> isn't /etc/sysconfig too specific to Fedora? >>>>>>>> >>>>>>> Hi Gustavo >>>>>>> >>>>>>> probably yes. I see the code in 'src/locale-setup.c' where the >>>>>>> the configuration directory depends on the target distribution. >>>>>>> I can implement something like that in my patch. >>>>>> >>>>>> >>>>>> Can't IMA be changed? Lennart seems to be pushing for distribution >>>>>> independent location files. If you can get IMA people to agree on >>>>>> something, just use this one instead. >>>>>> >>>>>> People that use IMA with systemd must use this location. Eventually >>>>>> this will happen with every configuration file we support. >>>>>> >>>>> The location of the policy file is not IMA dependent. I chose that >>>>> because it seemed to me the right place where to put this file. >>>>> So, i can easily modify the location to be distribution independent >>>>> but i don't known which directory would be appropriate. >>>>> Any proposal? >>>>> >>>>> Regards >>>>> >>>>> Roberto Sassu >>>>> >>>>> >>>>>>>> Also, I certainly have no such things in my system and see no point >>>>>>>> in >>>>>>>> calling ima_setup() on it. Or even compiling the source file in such >>>>>>>> case. >>>>>>>> >>>>>>> Ok. I can enclose the code in ima-setup.c within an 'ifdef HAVE_IMA' >>>>>>> statement, as it happens for SELinux. However an issue is that there >>>>>>> is no a specific package for IMA that can be checked to set the >>>>>>> HAVE_IMA >>>>>>> definition to yes. Instead, the code can be enabled for example by >>>>>>> adding the parameter '--enable_ima' in the configure script. >>>>>> >>>>>> >>>>>> okay. >>>>>> >>>>>> -- >>>>>> Gustavo Sverzut Barbieri >>>>>> http://profusion.mobi embedded systems >>>>>> -------------------------------------- >>>>>> MSN: bar...@gm... >>>>>> Skype: gsbarbieri >>>>>> Mobile: +55 (19) 9225-2202 >>>> >>>> >>>> I'm under the impression this function belongs to a userspace tool. If >>>> not then I just don't see a good reason that this patch is required. I >>>> do understand that the IMA policy should be loaded as early as possible, >>>> but I believe that early userspace scripts should be doing that work. If >>>> it is a userspace function, then whatever makes you happy, other >>>> distro's will roll their own. >>> >>> >>> >>> Thanks Mimi for your contribution. I just want to add some >>> considerations. >>> >>> >>> >>> Hi Michael >>> >>> the reason for which the loading of IMA policies has been placed in >>> the main Systemd executable is that the measurement process performed >>> by IMA should start as early as possible. Otherwise, in order to build >>> the 'chain of trust' during the boot process from the BIOS to software >>> applications, it is required to measure those components loaded before >>> IMA is initialized with other means (for example from the boot loader). >>> >>> The more the IMA initialization is postponed, greater is the number of >>> components that must be measured using the second way. For instance, >>> if the policy loading is done in a userspace script you have to measure >>> the interpreter and the configuration files read by the latter. >>> >>> Since the policy loading can be implemented in different ways depending >>> on the init system (systemd, upstart, ...), an user must identify the >>> components to be measured for each case. Instead, if the IMA policy is >>> loaded in the main Systemd executable, only this file must be measured >>> by the boot loader. >> >> >> Then I wonder: why not make an ima-init binary that: >> - does ima_setup() >> - exec systemd || upstart || ... >> >> this way you only have to audit this very small file and not systemd >> itself, it's very early and so on. >> > > This does not work because SELinux is initialized inside Systemd and IMA > requires it for parsing LSM rules in the policy. initramfs may do it as well, no? then systemd will inherit it. I'm not using or reviewing the SELinux patches myself, so I may be wrong. Let's see if someone will check the code or Lennart will say what he did. -- Gustavo Sverzut Barbieri http://profusion.mobi embedded systems -------------------------------------- MSN: bar...@gm... Skype: gsbarbieri Mobile: +55 (19) 9225-2202 |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Lennart P. <le...@po...> - 2012-02-20 17:24:28
|
On Thu, 16.02.12 19:50, Gustavo Sverzut Barbieri (bar...@pr...) wrote: > >> Then I wonder: why not make an ima-init binary that: > >> - does ima_setup() > >> - exec systemd || upstart || ... > >> > >> this way you only have to audit this very small file and not systemd > >> itself, it's very early and so on. > >> > > > > This does not work because SELinux is initialized inside Systemd and IMA > > requires it for parsing LSM rules in the policy. > > initramfs may do it as well, no? then systemd will inherit it. We moved SELinux loading out of the initrd into systemd, in order to support fully featured initrd-less boots. I don't think we should reopen this problem set by having IMA in the initrd. I believe IMA should be treated pretty much exactly like SELinux here: the policy should be loaded from PID1 and it needs to be a compile time option, and it needs a kernel cmdline option to disable it (i.e. like selinux=0). Lennart -- Lennart Poettering - Red Hat, Inc. |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Lennart P. <le...@po...> - 2012-02-20 17:12:41
|
On Wed, 15.02.12 14:23, Roberto Sassu (rob...@po...) wrote: > The new function ima_setup() loads an IMA custom policy from a file in the > default location '/etc/sysconfig/ima-policy', if present, and writes it to > the path 'ima/policy' in the security filesystem. This function is executed > at early stage in order to avoid that some file operations are not measured > by IMA and it is placed after the initialization of SELinux because IMA > needs the latter (or other security modules) to understand LSM-specific > rules. This must be a configure option. I am pretty sure most embedded people don't require this feature. The kernel side of things is merged upstream I presume? (We generally only want to support stuff in our code that is merged upstream itself) > +#define IMA_SECFS_DIR SECURITYFS_MNTPOINT "/ima" > +#define IMA_SECFS_POLICY IMA_SECFS_DIR "/policy" Please use proper strings for this. (see my earlier mail) > +#define IMA_POLICY_PATH "/etc/sysconfig/ima-policy" This is a Fedoraism. Please introduce a proper configuration file for this. > + > +int ima_setup(void) { > + struct stat st; > + ssize_t policy_size = 0, written = 0; > + char *policy; > + int policyfd = -1, imafd = -1; > + int result = 0; > + > +#ifndef HAVE_SELINUX > + /* Mount the securityfs filesystem */ > + mount_setup_early(); > +#endif > + > + if (stat(IMA_POLICY_PATH, &st) == -1) > + return 0; We tend to do "< 0" instead of "== -1" checks for syscall failures. Might be good to use the same here, but this is not necessary for getting this merged. > + > + policyfd = open(IMA_POLICY_PATH, O_RDONLY); We tend to add O_CLOEXEC to all fds we open, just for being paranoid. Please do so here, too, to avoid surprise and avoid exceptions when people grep for all open() invocations looking for O_CLOEXEC. > + if (policyfd < 0) { > + log_error("Failed to open the IMA custom policy file %s (%s), " > + "ignoring.", IMA_POLICY_PATH, strerror(errno)); > + return 0; > + } Consider using %m instead of %s and strerror(errno). > + imafd = open(IMA_SECFS_POLICY, O_WRONLY); Also O_CLOEXEC please. > + if (imafd < 0) { > + log_error("Failed to open the IMA kernel interface %s (%s), " > + "ignoring.", IMA_SECFS_POLICY, strerror(errno)); > + goto out; > + } > + > + policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0); > + if (policy == NULL) { mmap() returns MAP_FAILED (i.e. (void) -1) on failure, not NULL. This check needs to be fixed. > + log_error("mmap() failed (%s), freezing", strerror(errno)); > + result = -errno; > + goto out; > + } > + > + while(written < policy_size) { > + ssize_t len = write(imafd, policy + written, > + policy_size - written); > + if (len <= 0) { > + log_error("Failed to load the IMA custom policy " > + "file %s (%s), ignoring.", IMA_POLICY_PATH, > + strerror(errno)); > + goto out_mmap; > + } > + written += len; > + } It might make sense to use loop_write() here instead, which does more or less this loop, and is defined in util.c anyway. Otherwise looks good. Lennart -- Lennart Poettering - Red Hat, Inc. |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Gustavo S. B. <bar...@pr...> - 2012-02-15 14:30:40
|
On Wed, Feb 15, 2012 at 11:23 AM, Roberto Sassu <rob...@po...> wrote: > The new function ima_setup() loads an IMA custom policy from a file in the > default location '/etc/sysconfig/ima-policy', if present, and writes it to isn't /etc/sysconfig too specific to Fedora? Also, I certainly have no such things in my system and see no point in calling ima_setup() on it. Or even compiling the source file in such case. -- Gustavo Sverzut Barbieri http://profusion.mobi embedded systems -------------------------------------- MSN: bar...@gm... Skype: gsbarbieri Mobile: +55 (19) 9225-2202 |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Roberto S. <rob...@po...> - 2012-02-15 16:29:05
|
On 02/15/2012 03:30 PM, Gustavo Sverzut Barbieri wrote: > On Wed, Feb 15, 2012 at 11:23 AM, Roberto Sassu<rob...@po...> wrote: >> The new function ima_setup() loads an IMA custom policy from a file in the >> default location '/etc/sysconfig/ima-policy', if present, and writes it to > > isn't /etc/sysconfig too specific to Fedora? > Hi Gustavo probably yes. I see the code in 'src/locale-setup.c' where the the configuration directory depends on the target distribution. I can implement something like that in my patch. > Also, I certainly have no such things in my system and see no point in > calling ima_setup() on it. Or even compiling the source file in such > case. > Ok. I can enclose the code in ima-setup.c within an 'ifdef HAVE_IMA' statement, as it happens for SELinux. However an issue is that there is no a specific package for IMA that can be checked to set the HAVE_IMA definition to yes. Instead, the code can be enabled for example by adding the parameter '--enable_ima' in the configure script. Regards Roberto Sassu |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Roberto S. <rob...@po...> - 2012-02-20 19:09:32
|
On 02/20/2012 06:24 PM, Lennart Poettering wrote: > On Thu, 16.02.12 19:50, Gustavo Sverzut Barbieri (bar...@pr...) wrote: > >>>> Then I wonder: why not make an ima-init binary that: >>>> - does ima_setup() >>>> - exec systemd || upstart || ... >>>> >>>> this way you only have to audit this very small file and not systemd >>>> itself, it's very early and so on. >>>> >>> >>> This does not work because SELinux is initialized inside Systemd and IMA >>> requires it for parsing LSM rules in the policy. >> >> initramfs may do it as well, no? then systemd will inherit it. > > We moved SELinux loading out of the initrd into systemd, in order to > support fully featured initrd-less boots. I don't think we should reopen > this problem set by having IMA in the initrd. I believe IMA should be > treated pretty much exactly like SELinux here: the policy should be > loaded from PID1 and it needs to be a compile time option, and it needs > a kernel cmdline option to disable it (i.e. like selinux=0). > If the SELinux module in dracut is to be considered definitively broken probably also the IMA module should be removed, because it will not be possible to load policies with LSM rules. But i don't know how this feature can be supported by distributions without Systemd installed. Regarding the kernel option, actually there is no a specific parameter to disable IMA. However, it can be introduced in the patches proposed by Mimi Zohar about the 'ima-appraisal' feature. This can allow to disable IMA or to put it in permissive/enforce mode as it happens for example in SELinux. Thanks Roberto Sassu > Lennart > |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Lennart P. <le...@po...> - 2012-02-20 17:14:06
|
On Wed, 15.02.12 17:26, Roberto Sassu (rob...@po...) wrote: > > On 02/15/2012 03:30 PM, Gustavo Sverzut Barbieri wrote: > >On Wed, Feb 15, 2012 at 11:23 AM, Roberto Sassu<rob...@po...> wrote: > >>The new function ima_setup() loads an IMA custom policy from a file in the > >>default location '/etc/sysconfig/ima-policy', if present, and writes it to > > > >isn't /etc/sysconfig too specific to Fedora? > > > > Hi Gustavo > > probably yes. I see the code in 'src/locale-setup.c' where the > the configuration directory depends on the target distribution. > I can implement something like that in my patch. We will sooner or later drop the per-distro ifdeffery. Please don't even start it for new code. Given that IMA is still new, please make sure to adopt configuration fails that are the same across all distributions. > >Also, I certainly have no such things in my system and see no point in > >calling ima_setup() on it. Or even compiling the source file in such > >case. > > > > Ok. I can enclose the code in ima-setup.c within an 'ifdef HAVE_IMA' > statement, as it happens for SELinux. However an issue is that there > is no a specific package for IMA that can be checked to set the > HAVE_IMA > definition to yes. Instead, the code can be enabled for example by > adding the parameter '--enable_ima' in the configure script. Sounds good. Lennart -- Lennart Poettering - Red Hat, Inc. |
Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added
support for loading IMA custom policies
From: Lennart P. <le...@po...> - 2012-02-20 19:18:17
|
On Mon, 20.02.12 20:06, Roberto Sassu (rob...@po...) wrote: > >We moved SELinux loading out of the initrd into systemd, in order to > >support fully featured initrd-less boots. I don't think we should reopen > >this problem set by having IMA in the initrd. I believe IMA should be > >treated pretty much exactly like SELinux here: the policy should be > >loaded from PID1 and it needs to be a compile time option, and it needs > >a kernel cmdline option to disable it (i.e. like selinux=0). > > > > If the SELinux module in dracut is to be considered definitively broken > probably also the IMA module should be removed, because it will not be > possible to load policies with LSM rules. But i don't know how this > feature can be supported by distributions without Systemd installed. Well, if the rumours I keep hearing are true Ubuntu might join the systemd camp too after their LTS release. Maybe the supporting non-systemd systems issues solves itself by that for you? > Regarding the kernel option, actually there is no a specific parameter > to disable IMA. However, it can be introduced in the patches proposed > by Mimi Zohar about the 'ima-appraisal' feature. This can allow to > disable IMA or to put it in permissive/enforce mode as it happens for > example in SELinux. Whether there is a kernel option to enable/disable IMA will not stop these patches from getting into systemd. But I am quite sure they will stop IMA from getting any wider coverage in the mainstream distributions (if you care for that). Oh, and one more thing: it matters to me that this doesn't break my build. So it needs to allow me booting when enabled in configure, but without any IMA policy around. Lennart -- Lennart Poettering - Red Hat, Inc. |