Discussion:
[PATCH v3 0/6] ima: provide signature based 'init' appraisal
Dmitry Kasatkin
2014-10-10 14:09:27 UTC
Permalink
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

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dmitry Kasatkin
2014-10-10 14:09:30 UTC
Permalink
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 <***@samsung.com>
---
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
Dmitry Kasatkin
2014-10-10 14:09:28 UTC
Permalink
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 <***@samsung.com>
---
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
Dmitry Kasatkin
2014-10-10 14:09:33 UTC
Permalink
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 <***@samsung.com>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Mimi Zohar
2014-10-13 17:33:27 UTC
Permalink
Post by Dmitry Kasatkin
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().
Al, can we get an ack on this?

thanks,

Mimi
Post by Dmitry Kasatkin
---
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;
}
Dmitry Kasatkin
2014-10-10 14:09:31 UTC
Permalink
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 <***@samsung.com>
---
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
Mimi Zohar
2014-10-13 17:33:17 UTC
Permalink
Post by Dmitry Kasatkin
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
Post by Dmitry Kasatkin
* Hook renamed as 'integrity_load_keys()' to handle both IMA and EVM
keys by integrity subsystem.
* Hook patch moved after defining loading functions
---
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
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();
+}
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dmitry Kasatkin
2014-10-10 14:09:32 UTC
Permalink
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 <***@samsung.com>
---
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
Mimi Zohar
2014-10-13 17:33:23 UTC
Permalink
Post by Dmitry Kasatkin
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
Post by Dmitry Kasatkin
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.
* policy change of this patch separated from the key loading patch
---
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);
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dmitry Kasatkin
2014-10-10 14:09:29 UTC
Permalink
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 <***@samsung.com>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Mimi Zohar
2014-10-13 17:32:51 UTC
Permalink
Post by Dmitry Kasatkin
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.
- 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
Post by Dmitry Kasatkin
* 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.
* 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
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(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dmitry Kasatkin
2014-10-14 22:23:29 UTC
Permalink
Hi Mimi,

Thanks for reply. I will fix your comments.
I am on LinuxCon EU at the moment.
Fixing as have a time :)

- Dmitry
Post by Mimi Zohar
Post by Dmitry Kasatkin
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.
- 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
Post by Dmitry Kasatkin
* 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.
* 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
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
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...