Currently secure IMA/EVM initialization has to be done from the initramfs,
embedded in the signed kernel image. Many systems do not want to use
initramfs or usage of embedded initramfs makes it difficult to have
multi-target kernels.
This is a very simple patchset which makes it possible to perform secure
initialization by requiring initial user-space to be signed.
It does it by:
- introducing a hook to load keys
- loading IMA signed public key certificate into the '.ima' trusted keyring
- making default IMA appraisal policy to require everything to be signed
When builtin initramfs is not in use, keys cannot be read from initcalls,
because root filesystem is not yet mounted. In order to read keys before
executing init process, ima_prepare_keys() hook is introduced. Reading
public keys from the kernel is justified because signature verification
key is needed in order to verify anything else which is read from the
file system. Public keys are X509 certificates and itself signed by the
trusted key from the .system keyring. Kernel BIG KEYS support is an example
of reading keys directly by the kernel.
CONFIG_IMA_APPRAISE_SIGNED_INIT kernel option is provided to make the IMA
default appraisal policy to required signature validation. Signed init
process need to initialize EVM key and load appropriate IMA policy which
would not require everything to be signed.
Unless real '/sbin/init' is signed, a simple and practical way is to place
all signed programs, libraries, scripts and configuration files under
dedicated directory, for example '/ima', and run signed init process by
providing a kernel command line parameter 'init=/ima/init'.
In the first post of these patches Andrew Morton noted that
integrity_read_file() is a very simple open-file-and-slurp-it-into-memory
and if there are other similar functions that can be made in ./lib.
I found out that only sound:sound_firmware.c:do_mod_firmware_load(),
which is enabled by CONFIG_SOUND_PRIME which is related to deprecated OSS
interface and is not enabled anymore in latest Ubuntu kernels, at least.
So I am keeping integrity_read_file() in integrity subsystem.
cpio based initramfs currently does not support extended attributes.
There is an initial agreement to introduce light-weight tar parser to
the kernel to support extended attributes which will make it possible to
use IMA appraisal with external initramfs. It will benefit from this
patchset and allow to update initramfs with signed files also on the
running system as distros do.
Changes in v3:
* ima_prepare_keys() renamed to integrity_load_keys() to be the hook
for both modules of integrity subsystem IMA/EVM.
* removed unnecessary configuration options and declared init functions
with '__init'.
* updated to lately introduced 'ima_policy_flag' variable to disabled and
enable IMA appraisal.
* separated key loading patch from policy change patch
* added patch which refactor vfs_read(). Agreed with Mimi to offer to
move calling file operations hooks to a separate helper function which
is then used by vfs_read() and integrity_kernel_read(). Applying this
patch does not affect functionality and can be applied if agreed so.
Changes in v2:
* ima_kernel_read() moved as integrity_kernel_read() from ima_crypto.c to
iint.c for use by integrity_read_file. The reason for keeping internal
version is because 'integrity' version does not call fsnotify_access(),
add_rchar() and inc_syscr().
* integrity_read_file() moved from digsig.c to iint.c because it is used
by IMA crypto subsystem and should not depend on digsig support being
enabled.
-Dmitry
Dmitry Kasatkin (6):
integrity: provide integrity_read_file()
integrity: provide x509 certificate loading from the kernel
ima: load x509 certificate from the kernel
integrity: provide hook to load keys when rootfs is ready
ima: require signature based appraisal
VFS: refactor vfs_read()
fs/read_write.c | 24 ++++++++---
include/linux/fs.h | 1 +
include/linux/integrity.h | 6 +++
init/main.c | 6 ++-
security/integrity/digsig.c | 37 +++++++++++++++-
security/integrity/iint.c | 85 +++++++++++++++++++++++++++++++++++++
security/integrity/ima/Kconfig | 22 ++++++++++
security/integrity/ima/ima_crypto.c | 35 ++-------------
security/integrity/ima/ima_init.c | 17 ++++++++
security/integrity/ima/ima_policy.c | 5 +++
security/integrity/integrity.h | 14 ++++++
11 files changed, 212 insertions(+), 40 deletions(-)
--
1.9.1
Define configuration option to load X509 certificate into the
IMA trusted kernel keyring. It implements ima_load_x509() hook
to load X509 certificate into the .ima trusted kernel keyring
from root filesystem.
Changes in v2:
* added '__init'
* use ima_policy_flag to disable appraisal to load keys
Signed-off-by: Dmitry Kasatkin <[email protected]>
---
security/integrity/ima/Kconfig | 15 +++++++++++++++
security/integrity/ima/ima_init.c | 17 +++++++++++++++++
security/integrity/integrity.h | 8 ++++++++
3 files changed, 40 insertions(+)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index e099875..44941c1 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -131,3 +131,18 @@ config IMA_TRUSTED_KEYRING
help
This option requires that all keys added to the .ima
keyring be signed by a key on the system trusted keyring.
+
+config IMA_LOAD_X509
+ bool "Load X509 certificate to the '.ima' trusted keyring"
+ depends on IMA_TRUSTED_KEYRING
+ default n
+ help
+ This option enables X509 certificate loading from the kernel
+ to the '.ima' trusted keyring.
+
+config IMA_X509_PATH
+ string "IMA X509 certificate path"
+ depends on IMA_LOAD_X509
+ default "/etc/ima/x509_ima.der"
+ help
+ This option defines IMA X509 certificate path.
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 9164fc8..0b6c305 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -24,6 +24,12 @@
#include <crypto/hash_info.h>
#include "ima.h"
+#ifdef CONFIG_IMA_X509_PATH
+#define IMA_X509_PATH CONFIG_IMA_X509_PATH
+#else
+#define IMA_X509_PATH "/etc/ima/x509_ima.der"
+#endif
+
/* name for boot aggregate entry */
static const char *boot_aggregate_name = "boot_aggregate";
int ima_used_chip;
@@ -91,6 +97,17 @@ err_out:
return result;
}
+#ifdef CONFIG_IMA_LOAD_X509
+void __init ima_load_x509(void)
+{
+ int unset_flags = ima_policy_flag & IMA_APPRAISE;
+
+ ima_policy_flag &= ~unset_flags;
+ integrity_load_x509(INTEGRITY_KEYRING_IMA, IMA_X509_PATH);
+ ima_policy_flag |= unset_flags;
+}
+#endif
+
int __init ima_init(void)
{
u8 pcr_i[TPM_DIGEST_SIZE];
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 1057abb..caa1f6c 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -162,6 +162,14 @@ static inline int asymmetric_verify(struct key *keyring, const char *sig,
}
#endif
+#ifdef CONFIG_IMA_LOAD_X509
+void __init ima_load_x509(void);
+#else
+static inline void ima_load_x509(void)
+{
+}
+#endif
+
#ifdef CONFIG_INTEGRITY_AUDIT
/* declarations */
void integrity_audit_msg(int audit_msgno, struct inode *inode,
--
1.9.1
This patch provides function to read file into the allocated
buffer. In order to use internal version of kernel_read to read
file without security checks, it also moves and renames
ima_kernel_read as integrity_kernel_read.
Changes in v2:
* configuration option removed
* function declared as '__init'
Signed-off-by: Dmitry Kasatkin <[email protected]>
---
security/integrity/iint.c | 78 +++++++++++++++++++++++++++++++++++++
security/integrity/ima/ima_crypto.c | 35 ++---------------
security/integrity/integrity.h | 4 ++
3 files changed, 85 insertions(+), 32 deletions(-)
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index cc3eb4d..0a76686 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -19,6 +19,8 @@
#include <linux/module.h>
#include <linux/spinlock.h>
#include <linux/rbtree.h>
+#include <linux/file.h>
+#include <linux/uaccess.h>
#include "integrity.h"
static struct rb_root integrity_iint_tree = RB_ROOT;
@@ -167,3 +169,79 @@ static int __init integrity_iintcache_init(void)
return 0;
}
security_initcall(integrity_iintcache_init);
+
+
+/*
+ * integrity_kernel_read - read data from the file
+ *
+ * This is a function for reading file content instead of kernel_read().
+ * It does not perform locking checks to ensure it cannot be blocked.
+ * It does not perform security checks because it is irrelevant for IMA.
+ *
+ */
+int integrity_kernel_read(struct file *file, loff_t offset,
+ char *addr, unsigned long count)
+{
+ mm_segment_t old_fs;
+ char __user *buf = addr;
+ ssize_t ret = -EINVAL;
+
+ if (!(file->f_mode & FMODE_READ))
+ return -EBADF;
+
+ old_fs = get_fs();
+ set_fs(get_ds());
+ if (file->f_op->read)
+ ret = file->f_op->read(file, buf, count, &offset);
+ else if (file->f_op->aio_read)
+ ret = do_sync_read(file, buf, count, &offset);
+ else if (file->f_op->read_iter)
+ ret = new_sync_read(file, buf, count, &offset);
+ set_fs(old_fs);
+ return ret;
+}
+
+/*
+ * integrity_read_file - read entire file content into the buffer
+ *
+ * This is function opens a file, allocates the buffer of required
+ * size, read entire file content to the buffer and closes the file
+ *
+ * It is used only by init code.
+ *
+ */
+int __init integrity_read_file(const char *path, char **data)
+{
+ struct file *file;
+ loff_t size;
+ char *buf;
+ int rc = -EINVAL;
+
+ file = filp_open(path, O_RDONLY, 0);
+ if (IS_ERR(file)) {
+ rc = PTR_ERR(file);
+ pr_err("Unable to open file: %s (%d)", path, rc);
+ return rc;
+ }
+
+ size = i_size_read(file_inode(file));
+ if (size <= 0)
+ goto out;
+
+ buf = kmalloc(size, GFP_KERNEL);
+ if (!buf) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ rc = integrity_kernel_read(file, 0, buf, size);
+ if (rc < 0)
+ kfree(buf);
+ else if (rc != size)
+ rc = -EIO;
+ else
+ *data = buf;
+out:
+ fput(file);
+ return rc;
+}
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index d34e7df..5df4d96 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -67,36 +67,6 @@ MODULE_PARM_DESC(ahash_bufsize, "Maximum ahash buffer size");
static struct crypto_shash *ima_shash_tfm;
static struct crypto_ahash *ima_ahash_tfm;
-/**
- * ima_kernel_read - read file content
- *
- * This is a function for reading file content instead of kernel_read().
- * It does not perform locking checks to ensure it cannot be blocked.
- * It does not perform security checks because it is irrelevant for IMA.
- *
- */
-static int ima_kernel_read(struct file *file, loff_t offset,
- char *addr, unsigned long count)
-{
- mm_segment_t old_fs;
- char __user *buf = addr;
- ssize_t ret = -EINVAL;
-
- if (!(file->f_mode & FMODE_READ))
- return -EBADF;
-
- old_fs = get_fs();
- set_fs(get_ds());
- if (file->f_op->read)
- ret = file->f_op->read(file, buf, count, &offset);
- else if (file->f_op->aio_read)
- ret = do_sync_read(file, buf, count, &offset);
- else if (file->f_op->read_iter)
- ret = new_sync_read(file, buf, count, &offset);
- set_fs(old_fs);
- return ret;
-}
-
int __init ima_init_crypto(void)
{
long rc;
@@ -324,7 +294,8 @@ static int ima_calc_file_hash_atfm(struct file *file,
}
/* read buffer */
rbuf_len = min_t(loff_t, i_size - offset, rbuf_size[active]);
- rc = ima_kernel_read(file, offset, rbuf[active], rbuf_len);
+ rc = integrity_kernel_read(file, offset, rbuf[active],
+ rbuf_len);
if (rc != rbuf_len)
goto out3;
@@ -417,7 +388,7 @@ static int ima_calc_file_hash_tfm(struct file *file,
while (offset < i_size) {
int rbuf_len;
- rbuf_len = ima_kernel_read(file, offset, rbuf, PAGE_SIZE);
+ rbuf_len = integrity_kernel_read(file, offset, rbuf, PAGE_SIZE);
if (rbuf_len < 0) {
rc = rbuf_len;
break;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index f51ad65..20d2204 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -119,6 +119,10 @@ struct integrity_iint_cache {
*/
struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
+int integrity_kernel_read(struct file *file, loff_t offset,
+ char *addr, unsigned long count);
+int __init integrity_read_file(const char *path, char **data);
+
#define INTEGRITY_KEYRING_EVM 0
#define INTEGRITY_KEYRING_MODULE 1
#define INTEGRITY_KEYRING_IMA 2
--
1.9.1
integrity_kernel_read() duplicates the file read operations code
in vfs_read(). This patch refactors vfs_read() code creating a
helper function __vfs_read(). It is used by both vfs_read() and
integrity_kernel_read().
Signed-off-by: Dmitry Kasatkin <[email protected]>
---
fs/read_write.c | 24 ++++++++++++++++++------
include/linux/fs.h | 1 +
security/integrity/iint.c | 10 +++-------
3 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 009d854..f45b2ae 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -412,6 +412,23 @@ ssize_t new_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *p
EXPORT_SYMBOL(new_sync_read);
+ssize_t __vfs_read(struct file *file, char __user *buf, size_t count,
+ loff_t *pos)
+{
+ ssize_t ret;
+
+ if (file->f_op->read)
+ ret = file->f_op->read(file, buf, count, pos);
+ else if (file->f_op->aio_read)
+ ret = do_sync_read(file, buf, count, pos);
+ else if (file->f_op->read_iter)
+ ret = new_sync_read(file, buf, count, pos);
+ else
+ ret = -EINVAL;
+
+ return ret;
+}
+
ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
{
ssize_t ret;
@@ -426,12 +443,7 @@ ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
ret = rw_verify_area(READ, file, pos, count);
if (ret >= 0) {
count = ret;
- if (file->f_op->read)
- ret = file->f_op->read(file, buf, count, pos);
- else if (file->f_op->aio_read)
- ret = do_sync_read(file, buf, count, pos);
- else
- ret = new_sync_read(file, buf, count, pos);
+ ret = __vfs_read(file, buf, count, pos);
if (ret > 0) {
fsnotify_access(file);
add_rchar(current, ret);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e11d60c..ac3a36e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1527,6 +1527,7 @@ ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,
struct iovec *fast_pointer,
struct iovec **ret_pointer);
+extern ssize_t __vfs_read(struct file *, char __user *, size_t, loff_t *);
extern ssize_t vfs_read(struct file *, char __user *, size_t, loff_t *);
extern ssize_t vfs_write(struct file *, const char __user *, size_t, loff_t *);
extern ssize_t vfs_readv(struct file *, const struct iovec __user *,
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index a1f5cd1..e359477 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -184,20 +184,16 @@ int integrity_kernel_read(struct file *file, loff_t offset,
{
mm_segment_t old_fs;
char __user *buf = addr;
- ssize_t ret = -EINVAL;
+ ssize_t ret;
if (!(file->f_mode & FMODE_READ))
return -EBADF;
old_fs = get_fs();
set_fs(get_ds());
- if (file->f_op->read)
- ret = file->f_op->read(file, buf, count, &offset);
- else if (file->f_op->aio_read)
- ret = do_sync_read(file, buf, count, &offset);
- else if (file->f_op->read_iter)
- ret = new_sync_read(file, buf, count, &offset);
+ ret = __vfs_read(file, buf, count, &offset);
set_fs(old_fs);
+
return ret;
}
--
1.9.1
Keys can only be loaded when rootfs is mounted. Initcalls
are not suitable for that. Provide a special hook.
Changes in v2:
* Hook renamed as 'integrity_load_keys()' to handle both IMA and EVM
keys by integrity subsystem.
* Hook patch moved after defining loading functions
Signed-off-by: Dmitry Kasatkin <[email protected]>
---
include/linux/integrity.h | 6 ++++++
init/main.c | 6 +++++-
security/integrity/iint.c | 11 +++++++++++
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index 83222ce..c2d6082 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -24,6 +24,7 @@ enum integrity_status {
#ifdef CONFIG_INTEGRITY
extern struct integrity_iint_cache *integrity_inode_get(struct inode *inode);
extern void integrity_inode_free(struct inode *inode);
+extern void __init integrity_load_keys(void);
#else
static inline struct integrity_iint_cache *
@@ -36,5 +37,10 @@ static inline void integrity_inode_free(struct inode *inode)
{
return;
}
+
+static inline void integrity_load_keys(void)
+{
+}
#endif /* CONFIG_INTEGRITY */
+
#endif /* _LINUX_INTEGRITY_H */
diff --git a/init/main.c b/init/main.c
index e8ae1fe..2c1928d 100644
--- a/init/main.c
+++ b/init/main.c
@@ -78,6 +78,7 @@
#include <linux/context_tracking.h>
#include <linux/random.h>
#include <linux/list.h>
+#include <linux/integrity.h>
#include <asm/io.h>
#include <asm/bugs.h>
@@ -1026,8 +1027,11 @@ static noinline void __init kernel_init_freeable(void)
* Ok, we have completed the initial bootup, and
* we're essentially up and running. Get rid of the
* initmem segments and start the user-mode stuff..
+ *
+ * rootfs is available now, try loading the public keys
+ * and default modules
*/
- /* rootfs is available now, try loading default modules */
+ integrity_load_keys();
load_default_modules();
}
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 0a76686..a1f5cd1 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -245,3 +245,14 @@ out:
fput(file);
return rc;
}
+
+/*
+ * integrity_load_keys - load integrity keys hook
+ *
+ * Hooks is called from init/main.c:kernel_init_freeable()
+ * when rootfs is ready
+ */
+void __init integrity_load_keys(void)
+{
+ ima_load_x509();
+}
--
1.9.1
This patch provides kernel parameter CONFIG_IMA_APPRAISE_SIGNED_INIT
to force IMA appraisal using signatures. This is useful, when EVM key
is not initalized yet and we want securely initialize integrity or any
other functionality.
It forces embedded policy to require signature. Signed initialization
script can initialize EVM key, update the IMA policy and change further
requirement of everything to be signed.
Changes in v2:
* policy change of this patch separated from the key loading patch
Signed-off-by: Dmitry Kasatkin <[email protected]>
---
security/integrity/ima/Kconfig | 7 +++++++
security/integrity/ima/ima_policy.c | 5 +++++
2 files changed, 12 insertions(+)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 44941c1..6a1971f 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -146,3 +146,10 @@ config IMA_X509_PATH
default "/etc/ima/x509_ima.der"
help
This option defines IMA X509 certificate path.
+
+config IMA_APPRAISE_SIGNED_INIT
+ bool "Require signed user-space initialization"
+ depends on IMA_LOAD_X509
+ default n
+ help
+ This option requires user-space init to be signed.
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 0d14d25..222ff79 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -100,7 +100,12 @@ static struct ima_rule_entry default_appraise_rules[] = {
{.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
{.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
{.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC},
+#ifndef CONFIG_IMA_APPRAISE_SIGNED_INIT
{.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER},
+#else
+ /* force signature */
+ {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER | IMA_DIGSIG_REQUIRED},
+#endif
};
static LIST_HEAD(ima_default_rules);
--
1.9.1
Provide function to load x509 certificates from the kernel into the
integrity kernel keyrings.
Changes in v2:
* configuration option removed
* function declared as '__init'
Signed-off-by: Dmitry Kasatkin <[email protected]>
---
security/integrity/digsig.c | 37 ++++++++++++++++++++++++++++++++++++-
security/integrity/integrity.h | 2 ++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 4f643d1..fa383fd 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -14,7 +14,7 @@
#include <linux/err.h>
#include <linux/sched.h>
-#include <linux/rbtree.h>
+#include <linux/slab.h>
#include <linux/cred.h>
#include <linux/key-type.h>
#include <linux/digsig.h>
@@ -84,3 +84,38 @@ int __init integrity_init_keyring(const unsigned int id)
}
return err;
}
+
+int __init integrity_load_x509(const unsigned int id, char *path)
+{
+ key_ref_t key;
+ char *data;
+ int rc;
+
+ if (!keyring[id])
+ return -EINVAL;
+
+ rc = integrity_read_file(path, &data);
+ if (rc < 0)
+ return rc;
+
+ key = key_create_or_update(make_key_ref(keyring[id], 1),
+ "asymmetric",
+ NULL,
+ data,
+ rc,
+ ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+ KEY_USR_VIEW | KEY_USR_READ),
+ KEY_ALLOC_NOT_IN_QUOTA |
+ KEY_ALLOC_TRUSTED);
+ if (IS_ERR(key)) {
+ rc = PTR_ERR(key);
+ pr_err("Problem loading X.509 certificate (%d): %s\n",
+ rc, path);
+ } else {
+ pr_notice("Loaded X.509 cert '%s': %s\n",
+ key_ref_to_ptr(key)->description, path);
+ key_ref_put(key);
+ }
+ kfree(data);
+ return 0;
+}
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 20d2204..1057abb 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -134,6 +134,7 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
const char *digest, int digestlen);
int __init integrity_init_keyring(const unsigned int id);
+int __init integrity_load_x509(const unsigned int id, char *path);
#else
static inline int integrity_digsig_verify(const unsigned int id,
@@ -147,6 +148,7 @@ static inline int integrity_init_keyring(const unsigned int id)
{
return 0;
}
+
#endif /* CONFIG_INTEGRITY_SIGNATURE */
#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
--
1.9.1
On Fri, 2014-10-10 at 17:09 +0300, Dmitry Kasatkin wrote:
> Currently secure IMA/EVM initialization has to be done from the initramfs,
> embedded in the signed kernel image. Many systems do not want to use
> initramfs or usage of embedded initramfs makes it difficult to have
> multi-target kernels.
>
> This is a very simple patchset which makes it possible to perform secure
> initialization by requiring initial user-space to be signed.
>
> It does it by:
> - introducing a hook to load keys
> - loading IMA signed public key certificate into the '.ima' trusted keyring
> - making default IMA appraisal policy to require everything to be signed
>
> When builtin initramfs is not in use, keys cannot be read from initcalls,
> because root filesystem is not yet mounted. In order to read keys before
> executing init process, ima_prepare_keys() hook is introduced. Reading
> public keys from the kernel is justified because signature verification
> key is needed in order to verify anything else which is read from the
> file system. Public keys are X509 certificates and itself signed by the
> trusted key from the .system keyring. Kernel BIG KEYS support is an example
> of reading keys directly by the kernel.
>
> CONFIG_IMA_APPRAISE_SIGNED_INIT kernel option is provided to make the IMA
> default appraisal policy to required signature validation. Signed init
> process need to initialize EVM key and load appropriate IMA policy which
> would not require everything to be signed.
>
> Unless real '/sbin/init' is signed, a simple and practical way is to place
> all signed programs, libraries, scripts and configuration files under
> dedicated directory, for example '/ima', and run signed init process by
> providing a kernel command line parameter 'init=/ima/init'.
>
> In the first post of these patches Andrew Morton noted that
> integrity_read_file() is a very simple open-file-and-slurp-it-into-memory
> and if there are other similar functions that can be made in ./lib.
> I found out that only sound:sound_firmware.c:do_mod_firmware_load(),
> which is enabled by CONFIG_SOUND_PRIME which is related to deprecated OSS
> interface and is not enabled anymore in latest Ubuntu kernels, at least.
> So I am keeping integrity_read_file() in integrity subsystem.
>
> cpio based initramfs currently does not support extended attributes.
> There is an initial agreement to introduce light-weight tar parser to
> the kernel to support extended attributes which will make it possible to
> use IMA appraisal with external initramfs. It will benefit from this
> patchset and allow to update initramfs with signed files also on the
> running system as distros do.
Thanks, Dmitry. The patches look good, but I still will need to test.
(Once I figure out how to build a kernel without an initramfs, that
boots a distro kernel.) Only some minor suggested patch descriptions
changes ...
Mimi
> Changes in v3:
> * ima_prepare_keys() renamed to integrity_load_keys() to be the hook
> for both modules of integrity subsystem IMA/EVM.
> * removed unnecessary configuration options and declared init functions
> with '__init'.
> * updated to lately introduced 'ima_policy_flag' variable to disabled and
> enable IMA appraisal.
> * separated key loading patch from policy change patch
> * added patch which refactor vfs_read(). Agreed with Mimi to offer to
> move calling file operations hooks to a separate helper function which
> is then used by vfs_read() and integrity_kernel_read(). Applying this
> patch does not affect functionality and can be applied if agreed so.
>
> Changes in v2:
> * ima_kernel_read() moved as integrity_kernel_read() from ima_crypto.c to
> iint.c for use by integrity_read_file. The reason for keeping internal
> version is because 'integrity' version does not call fsnotify_access(),
> add_rchar() and inc_syscr().
> * integrity_read_file() moved from digsig.c to iint.c because it is used
> by IMA crypto subsystem and should not depend on digsig support being
> enabled.
>
> -Dmitry
>
> Dmitry Kasatkin (6):
> integrity: provide integrity_read_file()
> integrity: provide x509 certificate loading from the kernel
> ima: load x509 certificate from the kernel
> integrity: provide hook to load keys when rootfs is ready
> ima: require signature based appraisal
> VFS: refactor vfs_read()
>
> fs/read_write.c | 24 ++++++++---
> include/linux/fs.h | 1 +
> include/linux/integrity.h | 6 +++
> init/main.c | 6 ++-
> security/integrity/digsig.c | 37 +++++++++++++++-
> security/integrity/iint.c | 85 +++++++++++++++++++++++++++++++++++++
> security/integrity/ima/Kconfig | 22 ++++++++++
> security/integrity/ima/ima_crypto.c | 35 ++-------------
> security/integrity/ima/ima_init.c | 17 ++++++++
> security/integrity/ima/ima_policy.c | 5 +++
> security/integrity/integrity.h | 14 ++++++
> 11 files changed, 212 insertions(+), 40 deletions(-)
>
On Fri, 2014-10-10 at 17:09 +0300, Dmitry Kasatkin wrote:
> Keys can only be loaded when rootfs is mounted. Initcalls
> are not suitable for that. Provide a special hook.
This patch description needs to be expanded a bit. Please include an
explanation as to why the keys need to be loaded here, before accessing
any file, and why it is safe to do so. (This information can be taken
from the cover letter.)
thanks,
Mimi
> Changes in v2:
> * Hook renamed as 'integrity_load_keys()' to handle both IMA and EVM
> keys by integrity subsystem.
> * Hook patch moved after defining loading functions
>
> Signed-off-by: Dmitry Kasatkin <[email protected]>
> ---
> include/linux/integrity.h | 6 ++++++
> init/main.c | 6 +++++-
> security/integrity/iint.c | 11 +++++++++++
> 3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index 83222ce..c2d6082 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -24,6 +24,7 @@ enum integrity_status {
> #ifdef CONFIG_INTEGRITY
> extern struct integrity_iint_cache *integrity_inode_get(struct inode *inode);
> extern void integrity_inode_free(struct inode *inode);
> +extern void __init integrity_load_keys(void);
>
> #else
> static inline struct integrity_iint_cache *
> @@ -36,5 +37,10 @@ static inline void integrity_inode_free(struct inode *inode)
> {
> return;
> }
> +
> +static inline void integrity_load_keys(void)
> +{
> +}
> #endif /* CONFIG_INTEGRITY */
> +
> #endif /* _LINUX_INTEGRITY_H */
> diff --git a/init/main.c b/init/main.c
> index e8ae1fe..2c1928d 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -78,6 +78,7 @@
> #include <linux/context_tracking.h>
> #include <linux/random.h>
> #include <linux/list.h>
> +#include <linux/integrity.h>
>
> #include <asm/io.h>
> #include <asm/bugs.h>
> @@ -1026,8 +1027,11 @@ static noinline void __init kernel_init_freeable(void)
> * Ok, we have completed the initial bootup, and
> * we're essentially up and running. Get rid of the
> * initmem segments and start the user-mode stuff..
> + *
> + * rootfs is available now, try loading the public keys
> + * and default modules
> */
>
> - /* rootfs is available now, try loading default modules */
> + integrity_load_keys();
> load_default_modules();
> }
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index 0a76686..a1f5cd1 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -245,3 +245,14 @@ out:
> fput(file);
> return rc;
> }
> +
> +/*
> + * integrity_load_keys - load integrity keys hook
> + *
> + * Hooks is called from init/main.c:kernel_init_freeable()
> + * when rootfs is ready
> + */
> +void __init integrity_load_keys(void)
> +{
> + ima_load_x509();
> +}
On Fri, 2014-10-10 at 17:09 +0300, Dmitry Kasatkin wrote:
> This patch provides kernel parameter CONFIG_IMA_APPRAISE_SIGNED_INIT
> to force IMA appraisal using signatures. This is useful, when EVM key
> is not initalized yet and we want securely initialize integrity or any
> other functionality.
Instead of "kernel parameter", I think you meant "config option". A new
kernel parameter would need to be documented in
Documentation/kernel-parameters.
Mimi
> It forces embedded policy to require signature. Signed initialization
> script can initialize EVM key, update the IMA policy and change further
> requirement of everything to be signed.
>
> Changes in v2:
> * policy change of this patch separated from the key loading patch
>
> Signed-off-by: Dmitry Kasatkin <[email protected]>
> ---
> security/integrity/ima/Kconfig | 7 +++++++
> security/integrity/ima/ima_policy.c | 5 +++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 44941c1..6a1971f 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -146,3 +146,10 @@ config IMA_X509_PATH
> default "/etc/ima/x509_ima.der"
> help
> This option defines IMA X509 certificate path.
> +
> +config IMA_APPRAISE_SIGNED_INIT
> + bool "Require signed user-space initialization"
> + depends on IMA_LOAD_X509
> + default n
> + help
> + This option requires user-space init to be signed.
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 0d14d25..222ff79 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -100,7 +100,12 @@ static struct ima_rule_entry default_appraise_rules[] = {
> {.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC},
> {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC},
> {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC},
> +#ifndef CONFIG_IMA_APPRAISE_SIGNED_INIT
> {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER},
> +#else
> + /* force signature */
> + {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER | IMA_DIGSIG_REQUIRED},
> +#endif
> };
>
> static LIST_HEAD(ima_default_rules);
On Fri, 2014-10-10 at 17:09 +0300, Dmitry Kasatkin wrote:
> integrity_kernel_read() duplicates the file read operations code
> in vfs_read(). This patch refactors vfs_read() code creating a
> helper function __vfs_read(). It is used by both vfs_read() and
> integrity_kernel_read().
>
> Signed-off-by: Dmitry Kasatkin <[email protected]>
Al, can we get an ack on this?
thanks,
Mimi
> ---
> fs/read_write.c | 24 ++++++++++++++++++------
> include/linux/fs.h | 1 +
> security/integrity/iint.c | 10 +++-------
> 3 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 009d854..f45b2ae 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -412,6 +412,23 @@ ssize_t new_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *p
>
> EXPORT_SYMBOL(new_sync_read);
>
> +ssize_t __vfs_read(struct file *file, char __user *buf, size_t count,
> + loff_t *pos)
> +{
> + ssize_t ret;
> +
> + if (file->f_op->read)
> + ret = file->f_op->read(file, buf, count, pos);
> + else if (file->f_op->aio_read)
> + ret = do_sync_read(file, buf, count, pos);
> + else if (file->f_op->read_iter)
> + ret = new_sync_read(file, buf, count, pos);
> + else
> + ret = -EINVAL;
> +
> + return ret;
> +}
> +
> ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
> {
> ssize_t ret;
> @@ -426,12 +443,7 @@ ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
> ret = rw_verify_area(READ, file, pos, count);
> if (ret >= 0) {
> count = ret;
> - if (file->f_op->read)
> - ret = file->f_op->read(file, buf, count, pos);
> - else if (file->f_op->aio_read)
> - ret = do_sync_read(file, buf, count, pos);
> - else
> - ret = new_sync_read(file, buf, count, pos);
> + ret = __vfs_read(file, buf, count, pos);
> if (ret > 0) {
> fsnotify_access(file);
> add_rchar(current, ret);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e11d60c..ac3a36e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1527,6 +1527,7 @@ ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,
> struct iovec *fast_pointer,
> struct iovec **ret_pointer);
>
> +extern ssize_t __vfs_read(struct file *, char __user *, size_t, loff_t *);
> extern ssize_t vfs_read(struct file *, char __user *, size_t, loff_t *);
> extern ssize_t vfs_write(struct file *, const char __user *, size_t, loff_t *);
> extern ssize_t vfs_readv(struct file *, const struct iovec __user *,
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index a1f5cd1..e359477 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -184,20 +184,16 @@ int integrity_kernel_read(struct file *file, loff_t offset,
> {
> mm_segment_t old_fs;
> char __user *buf = addr;
> - ssize_t ret = -EINVAL;
> + ssize_t ret;
>
> if (!(file->f_mode & FMODE_READ))
> return -EBADF;
>
> old_fs = get_fs();
> set_fs(get_ds());
> - if (file->f_op->read)
> - ret = file->f_op->read(file, buf, count, &offset);
> - else if (file->f_op->aio_read)
> - ret = do_sync_read(file, buf, count, &offset);
> - else if (file->f_op->read_iter)
> - ret = new_sync_read(file, buf, count, &offset);
> + ret = __vfs_read(file, buf, count, &offset);
> set_fs(old_fs);
> +
> return ret;
> }
>
Hi Mimi,
Thanks for reply. I will fix your comments.
I am on LinuxCon EU at the moment.
Fixing as have a time :)
- Dmitry
On 13 October 2014 19:32, Mimi Zohar <[email protected]> wrote:
> On Fri, 2014-10-10 at 17:09 +0300, Dmitry Kasatkin wrote:
>> Currently secure IMA/EVM initialization has to be done from the initramfs,
>> embedded in the signed kernel image. Many systems do not want to use
>> initramfs or usage of embedded initramfs makes it difficult to have
>> multi-target kernels.
>>
>> This is a very simple patchset which makes it possible to perform secure
>> initialization by requiring initial user-space to be signed.
>>
>> It does it by:
>> - introducing a hook to load keys
>> - loading IMA signed public key certificate into the '.ima' trusted keyring
>> - making default IMA appraisal policy to require everything to be signed
>>
>> When builtin initramfs is not in use, keys cannot be read from initcalls,
>> because root filesystem is not yet mounted. In order to read keys before
>> executing init process, ima_prepare_keys() hook is introduced. Reading
>> public keys from the kernel is justified because signature verification
>> key is needed in order to verify anything else which is read from the
>> file system. Public keys are X509 certificates and itself signed by the
>> trusted key from the .system keyring. Kernel BIG KEYS support is an example
>> of reading keys directly by the kernel.
>>
>> CONFIG_IMA_APPRAISE_SIGNED_INIT kernel option is provided to make the IMA
>> default appraisal policy to required signature validation. Signed init
>> process need to initialize EVM key and load appropriate IMA policy which
>> would not require everything to be signed.
>>
>> Unless real '/sbin/init' is signed, a simple and practical way is to place
>> all signed programs, libraries, scripts and configuration files under
>> dedicated directory, for example '/ima', and run signed init process by
>> providing a kernel command line parameter 'init=/ima/init'.
>>
>> In the first post of these patches Andrew Morton noted that
>> integrity_read_file() is a very simple open-file-and-slurp-it-into-memory
>> and if there are other similar functions that can be made in ./lib.
>> I found out that only sound:sound_firmware.c:do_mod_firmware_load(),
>> which is enabled by CONFIG_SOUND_PRIME which is related to deprecated OSS
>> interface and is not enabled anymore in latest Ubuntu kernels, at least.
>> So I am keeping integrity_read_file() in integrity subsystem.
>>
>> cpio based initramfs currently does not support extended attributes.
>> There is an initial agreement to introduce light-weight tar parser to
>> the kernel to support extended attributes which will make it possible to
>> use IMA appraisal with external initramfs. It will benefit from this
>> patchset and allow to update initramfs with signed files also on the
>> running system as distros do.
>
> Thanks, Dmitry. The patches look good, but I still will need to test.
> (Once I figure out how to build a kernel without an initramfs, that
> boots a distro kernel.) Only some minor suggested patch descriptions
> changes ...
>
> Mimi
>
>> Changes in v3:
>> * ima_prepare_keys() renamed to integrity_load_keys() to be the hook
>> for both modules of integrity subsystem IMA/EVM.
>> * removed unnecessary configuration options and declared init functions
>> with '__init'.
>> * updated to lately introduced 'ima_policy_flag' variable to disabled and
>> enable IMA appraisal.
>> * separated key loading patch from policy change patch
>> * added patch which refactor vfs_read(). Agreed with Mimi to offer to
>> move calling file operations hooks to a separate helper function which
>> is then used by vfs_read() and integrity_kernel_read(). Applying this
>> patch does not affect functionality and can be applied if agreed so.
>>
>> Changes in v2:
>> * ima_kernel_read() moved as integrity_kernel_read() from ima_crypto.c to
>> iint.c for use by integrity_read_file. The reason for keeping internal
>> version is because 'integrity' version does not call fsnotify_access(),
>> add_rchar() and inc_syscr().
>> * integrity_read_file() moved from digsig.c to iint.c because it is used
>> by IMA crypto subsystem and should not depend on digsig support being
>> enabled.
>>
>> -Dmitry
>>
>> Dmitry Kasatkin (6):
>> integrity: provide integrity_read_file()
>> integrity: provide x509 certificate loading from the kernel
>> ima: load x509 certificate from the kernel
>> integrity: provide hook to load keys when rootfs is ready
>> ima: require signature based appraisal
>> VFS: refactor vfs_read()
>>
>> fs/read_write.c | 24 ++++++++---
>> include/linux/fs.h | 1 +
>> include/linux/integrity.h | 6 +++
>> init/main.c | 6 ++-
>> security/integrity/digsig.c | 37 +++++++++++++++-
>> security/integrity/iint.c | 85 +++++++++++++++++++++++++++++++++++++
>> security/integrity/ima/Kconfig | 22 ++++++++++
>> security/integrity/ima/ima_crypto.c | 35 ++-------------
>> security/integrity/ima/ima_init.c | 17 ++++++++
>> security/integrity/ima/ima_policy.c | 5 +++
>> security/integrity/integrity.h | 14 ++++++
>> 11 files changed, 212 insertions(+), 40 deletions(-)
>>
>
>
--
Thanks,
Dmitry
On Fri, 2014-10-10 at 17:09 +0300, Dmitry Kasatkin wrote:
> Define configuration option to load X509 certificate into the
> IMA trusted kernel keyring. It implements ima_load_x509() hook
> to load X509 certificate into the .ima trusted kernel keyring
> from root filesystem.
>
> Changes in v2:
> * added '__init'
> * use ima_policy_flag to disable appraisal to load keys
>
> Signed-off-by: Dmitry Kasatkin <[email protected]>
> ---
> security/integrity/ima/Kconfig | 15 +++++++++++++++
> security/integrity/ima/ima_init.c | 17 +++++++++++++++++
> security/integrity/integrity.h | 8 ++++++++
> 3 files changed, 40 insertions(+)
>
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index e099875..44941c1 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -131,3 +131,18 @@ config IMA_TRUSTED_KEYRING
> help
> This option requires that all keys added to the .ima
> keyring be signed by a key on the system trusted keyring.
> +
> +config IMA_LOAD_X509
> + bool "Load X509 certificate to the '.ima' trusted keyring"
> + depends on IMA_TRUSTED_KEYRING
> + default n
> + help
> + This option enables X509 certificate loading from the kernel
> + to the '.ima' trusted keyring.
> +
> +config IMA_X509_PATH
> + string "IMA X509 certificate path"
> + depends on IMA_LOAD_X509
> + default "/etc/ima/x509_ima.der"
The default dracut master key (modules/97masterkey/masterkey.sh) and EVM
key (modules/98integrity/evm_enable.sh) paths are defined as /etc/keys.
Next, EVM will want something similar as this patchset. Seems like
there should be a consistent "keys" directory.
Mimi
> + help
> + This option defines IMA X509 certificate path.
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 9164fc8..0b6c305 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -24,6 +24,12 @@
> #include <crypto/hash_info.h>
> #include "ima.h"
>
> +#ifdef CONFIG_IMA_X509_PATH
> +#define IMA_X509_PATH CONFIG_IMA_X509_PATH
> +#else
> +#define IMA_X509_PATH "/etc/ima/x509_ima.der"
> +#endif
> +
> /* name for boot aggregate entry */
> static const char *boot_aggregate_name = "boot_aggregate";
> int ima_used_chip;
> @@ -91,6 +97,17 @@ err_out:
> return result;
> }
>
> +#ifdef CONFIG_IMA_LOAD_X509
> +void __init ima_load_x509(void)
> +{
> + int unset_flags = ima_policy_flag & IMA_APPRAISE;
> +
> + ima_policy_flag &= ~unset_flags;
> + integrity_load_x509(INTEGRITY_KEYRING_IMA, IMA_X509_PATH);
> + ima_policy_flag |= unset_flags;
> +}
> +#endif
> +
> int __init ima_init(void)
> {
> u8 pcr_i[TPM_DIGEST_SIZE];
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 1057abb..caa1f6c 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -162,6 +162,14 @@ static inline int asymmetric_verify(struct key *keyring, const char *sig,
> }
> #endif
>
> +#ifdef CONFIG_IMA_LOAD_X509
> +void __init ima_load_x509(void);
> +#else
> +static inline void ima_load_x509(void)
> +{
> +}
> +#endif
> +
> #ifdef CONFIG_INTEGRITY_AUDIT
> /* declarations */
> void integrity_audit_msg(int audit_msgno, struct inode *inode,