[PATCH 0/3] ELF executable signing and verification

[PATCH 0/3] ELF executable signing and verification

[Vivek Goyal]

Hi,

This is a very crude RFC for ELF executable signing and verification. This
has been done along the lines of module signature verification.

Why do we need it
=================
With arrival of secureboot, sys_kexec() is deemed dangerous. One can
effectively bypass the secureboot feature and run its own kernel. So
matthew garret proposed disabling sys_kexec() in secureboot mode.

https://lkml.org/lkml/2012/9/4/225

Later in a separate thread it was discussed how to handle the issue
of sys_kexec() with secureboot.

https://lkml.org/lkml/2012/10/24/451

My takeaway from discussion was that we need to sign /sbin/kexec. Signed
executable can get extra capability and we can allow/disallow access to
sys_kexec() based on that capability (Thanks to Eric Biederman for the
idea).

So that's my motivation to make user space signing work so that I can
get kdump working with secureboot enabled. There might be other people
who might find it useful in general.

What does it do
===============
I have written a utility "signelf" which can take a private key and
an x509 certificate and sign an ELF executable. This is very much done
along the lines of module signing. There are two major differences.
Signature are put in a section ".signature" instead of being appended
to executable. And we calculate digest of only PT_LOAD segments and not
the whole executable file.

Upon exec(), we determine if executable is signed. If it is, then locks
down the pages in memory (using MAP_LOCKED) and verfies the signature.
If signature does not match, process is killed. Unsigned processes
don't get affected at all.

Currently it is expected to use these patches only for statically linked
executables. No dynamic linking. In fact patches specifically disable
calling interpreter. This does not prevent against somebody using dlopen()
sutff. So don't sign binaries which do that.

HOWTO
=====
Currently module signing keys are automatically loaded in module keyring
so it is easiest to sign executable using the keys generated for module
signing.

- Compile and boot into kernel with following options enabled. 
	- CONFIG_MODULE_SIG=y
	- CONFIG_BINFMT_ELF_SIGNATURE=y
	- CONFIG_CRYPTO_SHA256=y

- Compile "signelf" utility (Attached in a patch)

- Install glibc-static

- Compile a test program (say hello-world.c). Link statically with glibc
	gcc hello-world.c -o hello-world -static

- Sign hello_world using keys generated during kernel build.
	signelf -i hello-world -o hello-world.signed -p linux-2.6/signing_key.priv -c linux-2.6/signing_key.x509

- Run signed executable
	./hello-world.signed 

This should run successfully. Now one can generate another pair of keys
and certificate and sign same binary using new keys. This new binary should
fail to execute as corresponding keys are not loaded in kernel.

	openssl req -new -nodes -utf8 -sha256 -days 36500 -batch -x509 -config linux-2.6/x509.genkey -outform DER -out new_signing_key.x509 -keyout new_signing_key.priv

	signelf -i hello-world -o hello-world.signed.new -p new_signing_key.priv -c new_signing_key.x509

- Run this signed executable
	./hello-world.signed.new
	Killed

TODO
====
- kexec related patches are yet to be done.
- Disable ptrace to signed processes so that one can not modify code/data
  of signed process.
- Sort out issues related to how key used for user space signing is loaded
  in kernel keyring.
- Sort out issues related to sharing keyring with modules.

Thanks
Vivek
Vivek Goyal (3):
  module: export couple of functions for use in process signature
    verification
  binfmt_elf: Verify signature of signed elf binary
  binfmt_elf: Do not allow exec() if signed binary has intepreter

 fs/Kconfig.binfmt       |    7 +
 fs/binfmt_elf.c         |  465 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/module.h  |    8 +
 kernel/module_signing.c |    4 +-
 4 files changed, 482 insertions(+), 2 deletions(-)

-- 
1.7.7.6

[PATCH 1/3] module: export couple of functions for use in process signature verification

[Vivek Goyal]

This probably is not the right thing to do. May be module keyring and
functions to retrieve key and mpi array should be moved into separate file
so that it could be shared.

But for the time being as quick RFC, just export couple of functions
from module_signing.c

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 include/linux/module.h  |    8 ++++++++
 kernel/module_signing.c |    4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 7760c6d..fd121f9 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -17,6 +17,7 @@
 #include <linux/moduleparam.h>
 #include <linux/tracepoint.h>
 #include <linux/export.h>
+#include <crypto/public_key.h>
 
 #include <linux/percpu.h>
 #include <asm/module.h>
@@ -450,6 +451,13 @@ extern void __module_put_and_exit(struct module *mod, long code)
 	__attribute__((noreturn));
 #define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code);
 
+#ifdef CONFIG_MODULE_SIG
+extern struct key *request_asymmetric_key(const char *signer, size_t signer_len,
+				const u8 *key_id, size_t key_id_len);
+extern int mod_extract_mpi_array(struct public_key_signature *pks,
+				const void *data, size_t len);
+#endif
+
 #ifdef CONFIG_MODULE_UNLOAD
 unsigned long module_refcount(struct module *mod);
 void __symbol_put(const char *symbol);
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index f2970bd..4362a35 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -104,7 +104,7 @@ error_no_pks:
  *
  * RSA signatures only have one MPI, so currently we only read one.
  */
-static int mod_extract_mpi_array(struct public_key_signature *pks,
+int mod_extract_mpi_array(struct public_key_signature *pks,
 				 const void *data, size_t len)
 {
 	size_t nbytes;
@@ -129,7 +129,7 @@ static int mod_extract_mpi_array(struct public_key_signature *pks,
 /*
  * Request an asymmetric key.
  */
-static struct key *request_asymmetric_key(const char *signer, size_t signer_len,
+struct key *request_asymmetric_key(const char *signer, size_t signer_len,
 					  const u8 *key_id, size_t key_id_len)
 {
 	key_ref_t key;
-- 
1.7.7.6

[PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

If a binary is signed, verify its signature. If signature is not valid, do
not allow execution. If binary is not signed, execution is allowed
unconditionally.

CONFIG_BINFMT_ELF_SIGNATURE controls whether elf binary signature support
is compiled in or not.

Signature are expected to be present in elf section ".section". This code
is written along the lines of module signature verification code. Just
that I have removed the magic string. It is not needed as signature is
expected to be present in a specific section.

I put the signature into a section, instead of appending it so that
strip operation works fine.

One signs and verifies  all the areas mapped by PT_LOAD segments of elf
binary. Typically Elf header is mapped in first PT_LOAD segment. As adding
.signature section can change three elf header fields (e_shoff, e_shnum
and e_shstrndx), these fields are excluded from digest calculation

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/Kconfig.binfmt |    7 +
 fs/binfmt_elf.c   |  455 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 462 insertions(+), 0 deletions(-)

diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt
index 0efd152..93eb90d 100644
--- a/fs/Kconfig.binfmt
+++ b/fs/Kconfig.binfmt
@@ -23,6 +23,13 @@ config BINFMT_ELF
 	  ld.so (check the file <file:Documentation/Changes> for location and
 	  latest version).
 
+config BINFMT_ELF_SIGNATURE
+	bool "Kernel support for ELF binaries signature verification"
+	depends on BINFMT_ELF && MODULE_SIG
+	default n
+	---help---
+	  ELF binary signature verification support.
+
 config COMPAT_BINFMT_ELF
 	bool
 	depends on COMPAT && BINFMT_ELF
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 0c42cdb..80da13c 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -33,6 +33,10 @@
 #include <linux/elf.h>
 #include <linux/utsname.h>
 #include <linux/coredump.h>
+#include <linux/module.h>
+#include <crypto/public_key.h>
+#include <crypto/hash.h>
+#include <keys/asymmetric-type.h>
 #include <asm/uaccess.h>
 #include <asm/param.h>
 #include <asm/page.h>
@@ -44,6 +48,27 @@
 #define user_siginfo_t siginfo_t
 #endif
 
+struct elf_sig_info {
+	u8      algo;		/* Public-key crypto algorithm [enum pkey_algo] */
+	u8	hash;		/* Digest algorithm [enum pkey_hash_algo] */
+	u8	id_type;	/* Key identifier type [enum pkey_id_type] */
+	u8	signer_len;	/* Length of signer's name */
+	u8	key_id_len;	/* Length of key identifier */
+	u8	__pad[3];
+	__be32	sig_len;	/* Length of signature data */
+};
+
+struct elf_sig_data {
+	enum pkey_hash_algo hash;
+	char *sig;
+	unsigned int sig_len;
+	struct key *key;
+	struct shash_desc *desc;
+	char *digest;
+	unsigned int digest_sz;
+};
+
+
 static int load_elf_binary(struct linux_binprm *bprm);
 static int load_elf_library(struct file *);
 static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
@@ -558,6 +583,400 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
 #endif
 }
 
+#ifdef CONFIG_BINFMT_ELF_SIGNATURE
+/* Elf Signature Verification related stuff */
+static int esd_shash_init(struct elf_sig_data *esd)
+{
+	struct shash_desc *desc;
+	struct crypto_shash *tfm;
+	size_t digest_size, desc_size;
+	char *digest;
+	int ret;
+
+	tfm = crypto_alloc_shash(pkey_hash_algo[esd->hash], 0, 0);
+	if (IS_ERR(tfm)) {
+		ret = PTR_ERR(tfm);
+		return (ret == -ENOENT ? -ENOPKG : ret);
+	}
+
+	desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
+	digest_size = crypto_shash_digestsize(tfm);
+
+	desc = kzalloc(desc_size, GFP_KERNEL);
+	if (!desc) {
+		ret = -ENOMEM;
+		goto out_free_tfm;
+	}
+
+	digest = kzalloc(digest_size, GFP_KERNEL);
+	if (!digest) {
+		ret = -ENOMEM;
+		goto out_free_desc;
+	}
+
+	desc->tfm   = tfm;
+	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+
+	ret = crypto_shash_init(desc);
+	if (ret < 0)
+		goto out_free_digest;
+
+	esd->desc = desc;
+	esd->digest = digest;
+	esd->digest_sz = digest_size;
+	return 0;
+
+out_free_digest:
+	kfree(digest);
+out_free_desc:
+	kfree(desc);
+out_free_tfm:
+	crypto_free_shash(tfm);
+	return ret;
+}
+
+static struct elf_sig_data *
+elf_parse_binary_signature(struct elfhdr *ehdr, struct file *file)
+{
+	loff_t rem_file_sz, file_sz;
+	loff_t offset;
+	struct elf_sig_data *esd;
+	struct elf_sig_info *esi;
+	int retval, i;
+	size_t sig_len;
+	struct key *key;
+	struct elf_shdr *elf_shtable, *elf_spnt, *elf_shstrpnt;
+	unsigned int sig_info_sz, shtable_sz;
+	uint16_t shstrndx;
+	bool found_sig_section = false;
+	void *signer_name, *key_id;
+
+	if (!ehdr->e_shnum)
+		return NULL;
+
+	if (ehdr->e_shstrndx == SHN_UNDEF)
+		return NULL;
+
+	/* Read in elf section table */
+	file_sz = i_size_read(file->f_path.dentry->d_inode);
+	shtable_sz = ehdr->e_shnum * sizeof(struct elf_shdr);
+	elf_shtable = kmalloc(shtable_sz, GFP_KERNEL);
+	if (!elf_shtable)
+		return ERR_PTR(-ENOMEM);
+
+	retval = kernel_read(file, ehdr->e_shoff, (char *)elf_shtable,
+					shtable_sz);
+	if (retval != shtable_sz) {
+		if (retval >= 0)
+			retval = -EIO;
+		goto out_free_shtable;
+	}
+
+	if (ehdr->e_shstrndx == 0xffff)
+		shstrndx = elf_shtable[0].sh_link;
+	else
+		shstrndx = ehdr->e_shstrndx;
+
+	if (shstrndx >= ehdr->e_shnum) {
+		retval = -EINVAL;
+		goto out_free_shtable;
+	}
+
+	elf_shstrpnt = elf_shtable + shstrndx;
+	elf_spnt = elf_shtable;
+
+	/* Scan for section with name ".signature" */
+	for (i = 0; i < ehdr->e_shnum; i++) {
+		char sec_name[11];
+		offset = elf_shstrpnt->sh_offset + elf_spnt->sh_name;
+		retval = kernel_read(file, offset, sec_name, 11);
+		if (retval != 11) {
+			if (retval >= 0)
+				retval = -EIO;
+			goto out_free_shtable;
+		}
+
+		if(!strcmp(sec_name, ".signature")) {
+			found_sig_section = true;
+			break;
+		}
+		elf_spnt++;
+	}
+
+	if (!found_sig_section) {
+		/* File is not signed */
+		retval = 0;
+		goto out_free_shtable;
+	}
+
+	esi = kzalloc(sizeof(struct elf_sig_info), GFP_KERNEL);
+	if (!esi) {
+		retval = -ENOMEM;
+		goto out_free_shtable;
+	}
+
+	esd = kzalloc(sizeof(struct elf_sig_data), GFP_KERNEL);
+	if (!esd) {
+		retval = -ENOMEM;
+		goto out_free_esi;
+	}
+
+	/* Read in sig info */
+	sig_info_sz = sizeof(struct elf_sig_info);
+
+	offset = elf_spnt->sh_offset + elf_spnt->sh_size - sig_info_sz;
+	rem_file_sz = file_sz - sig_info_sz;
+	retval = kernel_read(file, offset, (char *)esi, sig_info_sz);
+	if (retval != sig_info_sz) {
+		if (retval >= 0)
+			retval = -EIO;
+		goto out_free_esd;
+	}
+
+	sig_len = be32_to_cpu(esi->sig_len);
+	if (sig_len >= rem_file_sz) {
+		retval = -EBADMSG;
+		goto out_free_esd;
+	}
+	rem_file_sz -= sig_len;
+
+	if ((size_t)esi->signer_len + esi->key_id_len >= rem_file_sz) {
+		retval = -EBADMSG;
+		goto out_free_esd;
+	}
+
+	rem_file_sz -= ((size_t)esi->signer_len + esi->key_id_len);
+
+	/* For the moment, only support RSA and X.509 identifiers */
+	if (esi->algo != PKEY_ALGO_RSA || esi->id_type != PKEY_ID_X509) {
+		retval = -ENOPKG;
+		goto out_free_esd;
+	}
+
+	if (esi->hash >= PKEY_HASH__LAST || !pkey_hash_algo[esi->hash]) {
+		retval = -ENOPKG;
+		goto out_free_esd;
+	}
+
+	esd->hash = esi->hash;
+
+	/* Read in signature */
+	esd->sig = kzalloc(sig_len, GFP_KERNEL);
+	if (!esd->sig) {
+		retval = -ENOMEM;
+		goto out_free_esd;
+	}
+	esd->sig_len = sig_len;
+
+	offset = offset - sig_len;
+	retval = kernel_read(file, offset, esd->sig, sig_len);
+	if (retval != sig_len) {
+		if (retval >= 0)
+			retval = -EIO;
+		goto out_free_esd_sig;
+	}
+
+	/* Read in skid */
+	key_id = kzalloc(esi->key_id_len, GFP_KERNEL);
+	if (!key_id) {
+		retval = -ENOMEM;
+		goto out_free_esd_sig;
+	}
+
+	offset = offset - esi->key_id_len;
+	retval = kernel_read(file, offset, key_id, esi->key_id_len);
+	if (retval != esi->key_id_len) {
+		if (retval >= 0)
+			retval = -EIO;
+		goto out_free_key_id;
+	}
+
+	/* Read in signer_name */
+	signer_name = kzalloc(esi->signer_len, GFP_KERNEL);
+	if (!signer_name) {
+		retval = -ENOMEM;
+		goto out_free_key_id;
+	}
+
+	offset = offset - esi->signer_len;
+	retval = kernel_read(file, offset, signer_name, esi->signer_len);
+	if (retval != esi->signer_len) {
+		if (retval >= 0)
+			retval = -EIO;
+		goto out_free_signer_name;
+	}
+
+	/* search for key */
+	key = request_asymmetric_key(signer_name, esi->signer_len,
+				     key_id, esi->key_id_len);
+	if (IS_ERR(key)) {
+		retval = PTR_ERR(key);
+		goto out_free_signer_name;
+	}
+	esd->key = key;
+
+	retval = esd_shash_init(esd);
+	if (retval < 0)
+		goto out_put_key;
+
+	kfree(elf_shtable);
+	kfree(signer_name);
+	kfree(key_id);
+	kfree(esi);
+	return esd;
+
+out_put_key:
+	key_put(key);
+out_free_signer_name:
+	kfree(signer_name);
+out_free_key_id:
+	kfree(key_id);
+out_free_esd_sig:
+	kfree(esd->sig);
+out_free_esd:
+	kfree(esd);
+out_free_esi:
+	kfree(esi);
+out_free_shtable:
+	kfree(elf_shtable);
+	return ERR_PTR(retval);
+}
+
+static void free_elf_sig_data(struct elf_sig_data *esd)
+{
+	if (!esd)
+		return;
+
+	if (esd->sig)
+		kfree(esd->sig);
+
+	if (esd->key)
+		key_put(esd->key);
+
+	if (esd->desc && esd->desc->tfm)
+		crypto_free_shash(esd->desc->tfm);
+
+	if (esd->desc)
+		kfree(esd->desc);
+
+	if (esd->digest)
+		kfree(esd->digest);
+}
+
+static void elf_digest_first_phdr(struct elfhdr *elfhdr,
+		struct elf_phdr *elf_ppnt, struct elf_sig_data *esd,
+		unsigned long map_addr)
+{
+	unsigned int off_e_shoff = offsetof(struct elfhdr, e_shoff);
+	unsigned int off_e_flags = offsetof(struct elfhdr, e_flags);
+	unsigned int off_e_shnum = offsetof(struct elfhdr, e_shnum);
+
+	/*
+	 * If elf header is mapped in first segment, execlude e_shoff, e_shnum
+	 * and e_shstrndx from digest calculation as this can change when
+	 * signature section is added or executable is stripped after
+	 * signing.
+	 */
+
+	if (!elf_ppnt->p_offset) {
+		/* ELF header is mapped into first PT_LOAD segment */
+		unsigned long sz = off_e_shoff;
+
+		crypto_shash_update(esd->desc, (u8*)map_addr, sz);
+
+		/* Digest e_flags to e_shentsize */
+		sz = off_e_shnum - off_e_flags;
+		crypto_shash_update(esd->desc, (u8*)map_addr + off_e_flags, sz);
+
+		/* Digest rest of the segment */
+		crypto_shash_update(esd->desc, (u8*)map_addr + elfhdr->e_ehsize,
+				elf_ppnt->p_filesz - elfhdr->e_ehsize);
+	} else
+		/* Digest full segment */
+		crypto_shash_update(esd->desc, (u8*)map_addr,
+					elf_ppnt->p_filesz);
+}
+
+static void elf_digest_phdr(struct elfhdr *ehdr, struct elf_phdr *phdr,
+			struct elf_sig_data *esd, unsigned long map_addr,
+			bool first_phdr)
+{
+	/*
+	 * If phdr->p_vaddr is not aligned, then elf_map() will map
+	 * at aligned address. Take that into account
+	 */
+	map_addr = map_addr + ELF_PAGEOFFSET(phdr->p_vaddr);
+
+	if (first_phdr)
+		elf_digest_first_phdr(ehdr, phdr, esd, map_addr);
+	else
+		crypto_shash_update(esd->desc, (u8*)map_addr, phdr->p_filesz);
+}
+
+static int elf_verify_signature(struct elf_sig_data *esd)
+{
+	struct public_key_signature *pks;
+	int retval;
+
+	pks = kzalloc(sizeof(*pks), GFP_KERNEL);
+	if (!pks)
+		return -ENOMEM;
+
+	pks->pkey_hash_algo = esd->hash;
+	pks->digest = (u8*)esd->digest;
+	pks->digest_size = esd->digest_sz;
+
+	retval = mod_extract_mpi_array(pks, esd->sig, esd->sig_len);
+	if (retval < 0)
+		goto error_free_pks;
+
+	retval = verify_signature(esd->key, pks);
+	mpi_free(pks->rsa.s);
+error_free_pks:
+	kfree(pks);
+	return retval;
+}
+
+static int elf_finalize_digest_verify_signature(struct elf_sig_data *esd)
+{
+	int retval;
+
+	retval = crypto_shash_final(esd->desc, (u8*)esd->digest);
+	if (retval < 0)
+		return retval;
+
+	retval = elf_verify_signature(esd);
+	if (retval < 0)
+		return retval;
+
+	return 0;
+}
+
+#else /* CONFIG_BINFMT_ELF_SIGNATURE */
+static inline struct elf_sig_data *
+elf_parse_binary_signature(struct elfhdr *ehdr, struct file *file)
+{
+	return NULL;
+}
+
+static inline void free_elf_sig_data(struct elf_sig_data *esd) {}
+
+static inline void elf_digest_phdr(struct elfhdr *ehdr, struct elf_phdr *phdr,
+			struct elf_sig_data *esd, unsigned long map_addr,
+			bool first_phdr) {}
+
+static inline int elf_verify_signature(struct elf_sig_data *esd)
+{
+	return 0;
+}
+
+static inline int elf_finalize_digest_verify_signature(struct elf_sig_data *esd)
+{
+	return 0;
+}
+
+#endif /* CONFIG_BINFMT_ELF_SIGNATURE */
+
 static int load_elf_binary(struct linux_binprm *bprm)
 {
 	struct file *interpreter = NULL; /* to shut gcc up */
@@ -575,6 +994,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
 	unsigned long reloc_func_desc __maybe_unused = 0;
 	int executable_stack = EXSTACK_DEFAULT;
 	unsigned long def_flags = 0;
+	struct elf_sig_data *esd = NULL;
+	bool first_signed_phdr = true;
 	struct pt_regs *regs = current_pt_regs();
 	struct {
 		struct elfhdr elf_ex;
@@ -741,6 +1162,14 @@ static int load_elf_binary(struct linux_binprm *bprm)
 	
 	current->mm->start_stack = bprm->p;
 
+	esd = elf_parse_binary_signature(&loc->elf_ex, bprm->file);
+	if (IS_ERR(esd)) {
+		retval = PTR_ERR(esd);
+		send_sig(SIGKILL, current, 0);
+		esd = NULL;
+		goto out_free_dentry;
+	}
+
 	/* Now we do a little grungy work by mmapping the ELF image into
 	   the correct location in memory. */
 	for(i = 0, elf_ppnt = elf_phdata;
@@ -788,6 +1217,14 @@ static int load_elf_binary(struct linux_binprm *bprm)
 
 		elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE;
 
+		/*
+		 * mlock the pages of signed binary. We don't want these
+		 * to be swapped out and be potentially modifed, effectively
+		 * bypassing signature verification.
+		 */
+		if (esd)
+			elf_flags = elf_flags | MAP_LOCKED;
+
 		vaddr = elf_ppnt->p_vaddr;
 		if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) {
 			elf_flags |= MAP_FIXED;
@@ -831,6 +1268,14 @@ static int load_elf_binary(struct linux_binprm *bprm)
 				reloc_func_desc = load_bias;
 			}
 		}
+
+		/* Calculate digest of PT_LOAD segments */
+		if (esd) {
+			elf_digest_phdr(&loc->elf_ex, elf_ppnt, esd, error,
+					first_signed_phdr);
+			first_signed_phdr = false;
+		}
+
 		k = elf_ppnt->p_vaddr;
 		if (k < start_code)
 			start_code = k;
@@ -864,6 +1309,15 @@ static int load_elf_binary(struct linux_binprm *bprm)
 			elf_brk = k;
 	}
 
+	/* Finalize digest and do signature verification */
+	if (esd) {
+		retval = elf_finalize_digest_verify_signature(esd);
+		if (retval < 0) {
+			send_sig(SIGKILL, current, 0);
+			goto out_free_dentry;
+		}
+	}
+
 	loc->elf_ex.e_entry += load_bias;
 	elf_bss += load_bias;
 	elf_brk += load_bias;
@@ -985,6 +1439,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
 	start_thread(regs, elf_entry, bprm->p);
 	retval = 0;
 out:
+	free_elf_sig_data(esd);
 	kfree(loc);
 out_ret:
 	return retval;
-- 
1.7.7.6

[PATCH 3/3] binfmt_elf: Do not allow exec() if signed binary has intepreter

[Vivek Goyal]

Do not allow execution if signed binary has interpreter. We don't have
a way to verify the signature of libraries interpreter can map. So do not
allow exec() of such binary.

Currently this signing process works only for statically linked binaries.

Well it does not prevent an application to use dlopen(). In that case,
these binaries should not be signed.

If a method to verify signature of shared libraries comes along, then
I think we can verify the signature of interpreter and allow launching
interpreter.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 fs/binfmt_elf.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 80da13c..d2fcb47 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1170,6 +1170,16 @@ static int load_elf_binary(struct linux_binprm *bprm)
 		goto out_free_dentry;
 	}
 
+	/*
+	 * Signed binary. If there is an interpreter specified, deny
+	 * execution
+	 */
+	if (esd && elf_interpreter) {
+		retval = -EINVAL;
+		send_sig(SIGKILL, current, 0);
+		goto out_free_dentry;
+	}
+
 	/* Now we do a little grungy work by mmapping the ELF image into
 	   the correct location in memory. */
 	for(i = 0, elf_ppnt = elf_phdata;
-- 
1.7.7.6

[PATCH 4/3] User space utility "signelf" to sign elf executable

[Vivek Goyal]

This is basic implementation of signelf utility. It can be used to sign an
ELF executable file.

A new section ".signature" is added to elf executable and signature of
executable are put there.

This is currently modeled on module signing and in fact imitates
scripts/sign-file script in kernel sources.

ELF parsing code I have taken from vmcore-dmesg.c in kexec project.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 Makefile  |   21 +
 signelf.c | 1130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 1151 insertions(+)

Index: signelf/signelf.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ signelf/signelf.c	2013-01-18 02:27:41.000000000 -0500
@@ -0,0 +1,1130 @@
+/*
+ * signelf: Sign ELF executable
+ *
+ * Copyright (C) 2013 Vivek Goyal (vgoyal@redhat.com)
+ *
+ * This program 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 (version 2 of the License).
+ *
+ * This program 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 this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#define _BSD_SOURCE
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <getopt.h>
+#include <limits.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <byteswap.h>
+#include <elf.h>
+#include <openssl/evp.h>
+#include <openssl/pkcs7.h>
+#include <openssl/bio.h>
+#include <openssl/pem.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/err.h>
+#include <openssl/rsa.h>
+#include <openssl/asn1.h>
+#include <endian.h>
+
+struct signelf_info {
+	char *in_file, *out_file, *privkey_file, *certificate_file;
+	Elf64_Ehdr ehdr;
+	Elf64_Phdr *phdr;
+	unsigned char md_value[EVP_MAX_MD_SIZE];
+	unsigned int md_len;
+	char *signer_name;
+	uint8_t *key_identifier;
+	unsigned int key_identifier_len;
+	/* Signature prefixed with signature len */
+	uint8_t *signature;
+	unsigned int signature_len;
+	/* Signature info */
+	char sig_info[64];
+	unsigned int sig_info_len;
+};
+
+/* digest prefixed with algo identifier. Used as input file for signing */
+char *temp_algo_ident_digest_file = "/tmp/signelf.algo_digest";
+
+/* signature file. output file of rsautl */
+char *tempsig_file = "/tmp/signelf.sig";
+
+/*
+ * Contains the full signature (including info) which gets into a section.
+ * objcopy uses it
+ */
+char *tempsigsection_file = "/tmp/signelf.sigsection";
+
+#define MD_BUF_SIZE	16384
+
+/* sha256 prologue */
+static char algo_ident_sha256[] = {
+		0x30, 0x31, 0x30, 0x0d, 0x06, 0x09,
+		0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01,
+		0x05, 0x00, 0x04, 0x20
+	};
+static unsigned int algo_ident_sha256_len = 19;
+
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+#define ELFDATANATIVE ELFDATA2LSB
+#elif __BYTE_ORDER == __BIG_ENDIAN
+#define ELFDATANATIVE ELFDATA2MSB
+#else
+#error "Unknown machine endian"
+#endif
+
+static uint16_t file16_to_cpu(struct signelf_info *sinfo, uint16_t val)
+{
+	if (sinfo->ehdr.e_ident[EI_DATA] != ELFDATANATIVE)
+		val = bswap_16(val);
+	return val;
+}
+
+static uint32_t file32_to_cpu(struct signelf_info *sinfo, uint32_t val)
+{
+	if (sinfo->ehdr.e_ident[EI_DATA] != ELFDATANATIVE)
+		val = bswap_32(val);
+	return val;
+}
+
+static uint64_t file64_to_cpu(struct signelf_info *sinfo, uint64_t val)
+{
+	if (sinfo->ehdr.e_ident[EI_DATA] != ELFDATANATIVE)
+		val = bswap_64(val);
+	return val;
+}
+
+static int read_elf32(struct signelf_info *sinfo, int fd)
+{
+	Elf32_Ehdr ehdr32;
+	Elf32_Phdr *phdr32;
+	size_t phdrs32_size;
+	ssize_t ret = 0, i;
+
+	ret = pread(fd, &ehdr32, sizeof(ehdr32), 0);
+	if (ret != sizeof(ehdr32)) {
+		fprintf(stderr, "Read of Elf header failed: %s\n",
+			strerror(errno));
+		return 1;
+	}
+
+	sinfo->ehdr.e_type	= file16_to_cpu(sinfo, ehdr32.e_type);
+	sinfo->ehdr.e_machine	= file16_to_cpu(sinfo, ehdr32.e_machine);
+	sinfo->ehdr.e_version	= file32_to_cpu(sinfo, ehdr32.e_version);
+	sinfo->ehdr.e_entry	= file32_to_cpu(sinfo, ehdr32.e_entry);
+	sinfo->ehdr.e_phoff	= file32_to_cpu(sinfo, ehdr32.e_phoff);
+	sinfo->ehdr.e_shoff	= file32_to_cpu(sinfo, ehdr32.e_shoff);
+	sinfo->ehdr.e_flags	= file32_to_cpu(sinfo, ehdr32.e_flags);
+	sinfo->ehdr.e_ehsize	= file16_to_cpu(sinfo, ehdr32.e_ehsize);
+	sinfo->ehdr.e_phentsize= file16_to_cpu(sinfo, ehdr32.e_phentsize);
+	sinfo->ehdr.e_phnum	= file16_to_cpu(sinfo, ehdr32.e_phnum);
+	sinfo->ehdr.e_shentsize= file16_to_cpu(sinfo, ehdr32.e_shentsize);
+	sinfo->ehdr.e_shnum	= file16_to_cpu(sinfo, ehdr32.e_shnum);
+	sinfo->ehdr.e_shstrndx	= file16_to_cpu(sinfo, ehdr32.e_shstrndx);
+
+	if (sinfo->ehdr.e_version != EV_CURRENT) {
+		fprintf(stderr, "Bad Elf header version %u\n",
+			sinfo->ehdr.e_version);
+		return 1;
+	}
+	if (sinfo->ehdr.e_phentsize != sizeof(Elf32_Phdr)) {
+		fprintf(stderr, "Bad Elf progra header size %u expected %zu\n",
+			sinfo->ehdr.e_phentsize, sizeof(Elf32_Phdr));
+		return 1;
+	}
+	phdrs32_size = sinfo->ehdr.e_phnum * sizeof(Elf32_Phdr);
+	phdr32 = calloc(sinfo->ehdr.e_phnum, sizeof(Elf32_Phdr));
+	if (!phdr32) {
+		fprintf(stderr, "Calloc of %u phdrs32 failed: %s\n",
+			sinfo->ehdr.e_phnum, strerror(errno));
+		return 1;
+	}
+
+	sinfo->phdr = calloc(sinfo->ehdr.e_phnum, sizeof(Elf64_Phdr));
+	if (!sinfo->phdr) {
+		fprintf(stderr, "Calloc of %u phdrs failed: %s\n",
+			sinfo->ehdr.e_phnum, strerror(errno));
+		ret = 1;
+		goto out_free_phdr32;
+	}
+	ret = pread(fd, phdr32, phdrs32_size, sinfo->ehdr.e_phoff);
+	if (ret < 0 || (size_t)ret != phdrs32_size) {
+		fprintf(stderr, "Read of program header @ 0x%llu for %zu bytes failed: %s\n",
+			(unsigned long long)sinfo->ehdr.e_phoff, phdrs32_size, strerror(errno));
+		ret = 1;
+		goto out_free_phdr;
+	}
+	for (i = 0; i < sinfo->ehdr.e_phnum; i++) {
+		sinfo->phdr[i].p_type = file32_to_cpu(sinfo, phdr32[i].p_type);
+		sinfo->phdr[i].p_offset = file32_to_cpu(sinfo,
+						phdr32[i].p_offset);
+		sinfo->phdr[i].p_vaddr = file32_to_cpu(sinfo,
+						phdr32[i].p_vaddr);
+		sinfo->phdr[i].p_paddr = file32_to_cpu(sinfo,
+						phdr32[i].p_paddr);
+		sinfo->phdr[i].p_filesz = file32_to_cpu(sinfo,
+						phdr32[i].p_filesz);
+		sinfo->phdr[i].p_memsz = file32_to_cpu(sinfo,
+						phdr32[i].p_memsz);
+		sinfo->phdr[i].p_flags = file32_to_cpu(sinfo,
+						phdr32[i].p_flags);
+		sinfo->phdr[i].p_align = file32_to_cpu(sinfo,
+						phdr32[i].p_align);
+	}
+	free(phdr32);
+	return ret;
+
+out_free_phdr:
+	free(sinfo->phdr);
+out_free_phdr32:
+	free(phdr32);
+	return ret;
+}
+
+static int read_elf64(struct signelf_info *sinfo, int fd)
+{
+	Elf64_Ehdr ehdr64;
+	Elf64_Phdr *phdr64;
+	size_t phdrs_size;
+	ssize_t ret, i;
+
+	ret = pread(fd, &ehdr64, sizeof(ehdr64), 0);
+	if (ret < 0 || (size_t)ret != sizeof(sinfo->ehdr)) {
+		fprintf(stderr, "Read of Elf header failed: %s\n",
+			strerror(errno));
+		return 1;
+	}
+
+	sinfo->ehdr.e_type	= file16_to_cpu(sinfo, ehdr64.e_type);
+	sinfo->ehdr.e_machine	= file16_to_cpu(sinfo, ehdr64.e_machine);
+	sinfo->ehdr.e_version	= file32_to_cpu(sinfo, ehdr64.e_version);
+	sinfo->ehdr.e_entry	= file64_to_cpu(sinfo, ehdr64.e_entry);
+	sinfo->ehdr.e_phoff	= file64_to_cpu(sinfo, ehdr64.e_phoff);
+	sinfo->ehdr.e_shoff	= file64_to_cpu(sinfo, ehdr64.e_shoff);
+	sinfo->ehdr.e_flags	= file32_to_cpu(sinfo, ehdr64.e_flags);
+	sinfo->ehdr.e_ehsize	= file16_to_cpu(sinfo, ehdr64.e_ehsize);
+	sinfo->ehdr.e_phentsize	= file16_to_cpu(sinfo, ehdr64.e_phentsize);
+	sinfo->ehdr.e_phnum	= file16_to_cpu(sinfo, ehdr64.e_phnum);
+	sinfo->ehdr.e_shentsize	= file16_to_cpu(sinfo, ehdr64.e_shentsize);
+	sinfo->ehdr.e_shnum	= file16_to_cpu(sinfo, ehdr64.e_shnum);
+	sinfo->ehdr.e_shstrndx	= file16_to_cpu(sinfo, ehdr64.e_shstrndx);
+
+	if (sinfo->ehdr.e_version != EV_CURRENT) {
+		fprintf(stderr, "Bad Elf header version %u\n",
+			sinfo->ehdr.e_version);
+		return 1;
+	}
+	if (sinfo->ehdr.e_phentsize != sizeof(Elf64_Phdr)) {
+		fprintf(stderr, "Bad Elf progra header size %u expected %zu\n",
+			sinfo->ehdr.e_phentsize, sizeof(Elf64_Phdr));
+		return 1;
+	}
+	phdrs_size = sinfo-> ehdr.e_phnum * sizeof(Elf64_Phdr);
+	phdr64 = calloc(sinfo->ehdr.e_phnum, sizeof(Elf64_Phdr));
+	if (!phdr64) {
+		fprintf(stderr, "Calloc of %u phdrs64 failed: %s\n",
+			sinfo->ehdr.e_phnum, strerror(errno));
+		return 1;
+	}
+	sinfo->phdr = calloc(sinfo->ehdr.e_phnum, sizeof(Elf64_Phdr));
+	if (!sinfo->phdr) {
+		fprintf(stderr, "Calloc of %u phdrs failed: %s\n",
+			sinfo->ehdr.e_phnum, strerror(errno));
+		ret = 1;
+		goto out_free_phdr64;
+	}
+	ret = pread(fd, phdr64, phdrs_size, sinfo->ehdr.e_phoff);
+	if (ret < 0 || (size_t)ret != phdrs_size) {
+		fprintf(stderr, "Read of program header @ %llu for %zu bytes failed: %s\n",
+			(unsigned long long)(sinfo->ehdr.e_phoff), phdrs_size, strerror(errno));
+		ret = 1;
+		goto out_free_phdr;
+	}
+	for (i = 0; i < sinfo->ehdr.e_phnum; i++) {
+		sinfo->phdr[i].p_type = file32_to_cpu(sinfo, phdr64[i].p_type);
+		sinfo->phdr[i].p_flags = file32_to_cpu(sinfo,
+						phdr64[i].p_flags);
+		sinfo->phdr[i].p_offset = file64_to_cpu(sinfo,
+						phdr64[i].p_offset);
+		sinfo->phdr[i].p_vaddr = file64_to_cpu(sinfo,
+						phdr64[i].p_vaddr);
+		sinfo->phdr[i].p_paddr = file64_to_cpu(sinfo,
+						phdr64[i].p_paddr);
+		sinfo->phdr[i].p_filesz = file64_to_cpu(sinfo,
+						phdr64[i].p_filesz);
+		sinfo->phdr[i].p_memsz = file64_to_cpu(sinfo,
+						phdr64[i].p_memsz);
+		sinfo->phdr[i].p_align = file64_to_cpu(sinfo,
+						phdr64[i].p_align);
+	}
+	free(phdr64);
+	return ret;
+
+out_free_phdr:
+	free(sinfo->phdr);
+out_free_phdr64:
+	free(phdr64);
+	return ret;
+}
+
+#ifdef DEBUG
+static void print_digest(struct signelf_info *sinfo)
+{
+	int i;
+
+	printf("Digest is: ");
+	for(i = 0; i < sinfo->md_len; i++)
+		printf("%02x", sinfo->md_value[i]);
+	printf("\n");
+}
+#endif
+
+/*
+ * First PT_LOAD segment might have ELF header mapped already. Handle it
+ * differently
+ */
+static int calculate_digest_first_seg(struct signelf_info *sinfo,
+			int fd, int phdr_idx, EVP_MD_CTX *mdctx)
+{
+	unsigned char buf[MD_BUF_SIZE];
+	loff_t offset;
+	size_t to_read;
+	unsigned int buf_len;
+	size_t sz = 0, sz_done = 0, sz_rem = 0;
+	unsigned int bytes;
+	unsigned int off_e_shoff, off_e_flags, off_e_shnum;
+
+	if (sinfo->ehdr.e_ident[EI_CLASS] == ELFCLASS32) {
+		off_e_shoff = offsetof(Elf32_Ehdr, e_shoff);
+		off_e_flags = offsetof(Elf32_Ehdr, e_flags);
+		off_e_shnum = offsetof(Elf32_Ehdr, e_shnum);
+	} else {
+		off_e_shoff = offsetof(Elf64_Ehdr, e_shoff);
+		off_e_flags = offsetof(Elf64_Ehdr, e_flags);
+		off_e_shnum = offsetof(Elf64_Ehdr, e_shnum);
+	}
+
+	/*
+	 * Make sure ELF header is mapped into first PT_LOAD segment
+	 * otherwise error out
+	 */
+	if (sinfo->phdr[phdr_idx].p_offset != 0) {
+		fprintf(stderr, "Error: ELF header is not mapped"
+			" in first PT_LOAD segment\n");
+		return 1;
+	}
+
+	/*
+	 * First digest of ELF header. Except following e_shoff, e_shnum,
+	 * and e_shstrndx.
+	 */
+
+	offset = 0;
+	to_read = off_e_shoff;
+
+	buf_len = pread(fd, (void*)buf, to_read, offset);
+	if (buf_len != to_read) {
+		fprintf(stderr, "Failed to read %lu bytes.\n", to_read);
+		return 1;
+	}
+
+	EVP_DigestUpdate(mdctx, buf, buf_len);
+	bytes = buf_len;
+
+	/* Calculate digest of rest of the elf header */
+	offset = off_e_flags;
+	to_read = off_e_shnum - off_e_flags;
+	buf_len = pread(fd, (void*)buf, to_read, offset);
+	if (buf_len != to_read) {
+		fprintf(stderr, "Failed to read %lu bytes.\n", to_read);
+		return 1;
+	}
+
+	EVP_DigestUpdate(mdctx, buf, buf_len);
+	bytes += buf_len;
+
+	/* Calculate digest of rest of the segment */
+	offset = sinfo->ehdr.e_ehsize;
+	sz = sinfo->phdr[phdr_idx].p_filesz - sinfo->ehdr.e_ehsize;
+
+	sz_rem = sz;
+	to_read = MD_BUF_SIZE;
+	sz_done = 0;
+
+	while (sz_rem) {
+		if (sz_rem < to_read)
+			to_read = sz_rem;
+
+		buf_len = pread(fd, (void*)buf, to_read, offset);
+		if (buf_len == -1) {
+			fprintf(stderr, "Failed to read file:%s\n",
+					strerror(errno));
+			return 1;
+		}
+
+		/* TODO: Use fread() instead of pread() */
+		if (buf_len != to_read) {
+			fprintf(stderr, "Failed to read %lu bytes from"
+				"  file :%s. Read %u bytes\n",
+				to_read, strerror(errno), buf_len);
+			return 1;
+		}
+
+		if (buf_len == 0)
+			break;
+
+		EVP_DigestUpdate(mdctx, buf, buf_len);
+
+		bytes += buf_len;
+		sz_rem -= buf_len;
+		sz_done += buf_len;
+		offset += buf_len;
+
+		to_read = sz_rem;
+		if (to_read > MD_BUF_SIZE)
+			to_read = MD_BUF_SIZE;
+	}
+
+	if (sz_done != sz) {
+		fprintf(stderr, "Could not digest %lu bytes. Digested"
+			" only %lu bytes\n", sz, sz_done);
+		return 1;
+	}
+
+	return 0;
+}
+
+static int calculate_elf_hash(struct signelf_info *sinfo, int fd)
+{
+	EVP_MD_CTX *mdctx;
+	const EVP_MD *md;
+	unsigned int buf_len;
+	unsigned char buf[MD_BUF_SIZE];
+	int i;
+	size_t sz = 0, sz_done = 0, sz_rem = 0;
+	int ret;
+	int first_segment = 1;
+
+	OpenSSL_add_all_digests();
+
+	/* TODO: Make digest type variable */
+	md = EVP_get_digestbyname("sha256");
+	mdctx = EVP_MD_CTX_create();
+	if (!mdctx) {
+		fprintf(stderr, "mdctx creation failed\n");
+		return 1;
+	}
+
+	EVP_DigestInit_ex(mdctx, md, NULL);
+
+	/*
+	 * Now go through all PT_LOAD program header areas and calculate
+	 * their checksum too. During verification, kernel will lock the
+	 * process mapped areas (essentially these PT_LOAD segments) and
+	 * go over these areas to calculate digest.
+	 *
+	 * Also calculate checksum of ELF header except two last fields.
+	 * Number of section headers and section header string table
+	 * index. Once signature is added in a section these two fields
+	 * will change.
+	 */
+	/* TODO: calculate hash of all program headers first */
+	for (i = 0; i < sinfo->ehdr.e_phnum; i++) {
+		loff_t offset;
+		size_t to_read;
+
+		if (sinfo->phdr[i].p_type != PT_LOAD)
+			continue;
+
+		if (first_segment) {
+			ret = calculate_digest_first_seg(sinfo, fd,
+					i, mdctx);
+			if (ret) {
+				fprintf(stderr, "Error while calculating"
+					" digest\n");
+				return 1;
+			}
+			first_segment = 0;
+			continue;
+		}
+
+		/* Calcualte hash of PT_LOAD area contents */
+		/* Skip if segment size is 0 (bss) */
+		offset = sinfo->phdr[i].p_offset;
+		sz = sinfo->phdr[i].p_filesz;
+
+		sz_rem = sz;
+		to_read = MD_BUF_SIZE;
+		sz_done = 0;
+
+		while (sz_rem) {
+			if (sz_rem < to_read)
+				to_read = sz_rem;
+
+			buf_len = pread(fd, (void*)buf, to_read, offset);
+			if (buf_len == -1) {
+				fprintf(stderr, "Failed to read:%s\n",
+						strerror(errno));
+				return 1;
+			}
+
+			/* TODO: Use fread() instead of pread() */
+			if (buf_len != to_read) {
+				fprintf(stderr, "Failed to read %lu bytes."
+					" Read %u bytes:%s\n",
+					to_read, buf_len, strerror(errno));
+				return 1;
+			}
+
+			if (buf_len == 0)
+				break;
+
+			EVP_DigestUpdate(mdctx, buf, buf_len);
+
+			sz_rem -= buf_len;
+			sz_done += buf_len;
+			offset += buf_len;
+
+			to_read = sz_rem;
+			if (to_read > MD_BUF_SIZE)
+				to_read = MD_BUF_SIZE;
+		}
+
+		if (sz_done != sz) {
+			fprintf(stderr, "Could not digest %lu bytes. Digested"
+				" only %lu bytes\n", sz, sz_done);
+			return 1;
+		}
+	}
+
+	EVP_DigestFinal_ex(mdctx, sinfo->md_value, &sinfo->md_len);
+	EVP_MD_CTX_destroy(mdctx);
+	EVP_cleanup();
+
+#ifdef DEBUG
+	print_digest(sinfo);
+#endif
+	return 0;
+}
+
+static int prefix_algo_identifier_to_digest(struct signelf_info *sinfo)
+{
+	FILE *algo_digest_fp;
+	size_t nr_write;
+
+	algo_digest_fp = fopen(temp_algo_ident_digest_file, "w+");
+	if (!algo_digest_fp) {
+		fprintf(stderr, "Failed to open file %s\n",
+				temp_algo_ident_digest_file);
+		return 1;
+	}
+
+	/* Write prologue */
+	nr_write = fwrite(algo_ident_sha256, 1, algo_ident_sha256_len,
+				algo_digest_fp);
+	if (nr_write != algo_ident_sha256_len) {
+		fprintf(stderr, "Failed to write prologue to %s\n",
+				temp_algo_ident_digest_file);
+		fclose(algo_digest_fp);
+		return 1;
+	}
+
+	/* Write digest */
+	nr_write = fwrite(sinfo->md_value, 1, sinfo->md_len, algo_digest_fp);
+	if (nr_write != sinfo->md_len) {
+		fprintf(stderr, "Failed to write digest to %s\n",
+				temp_algo_ident_digest_file);
+		fclose(algo_digest_fp);
+		return 1;
+	}
+
+	fclose(algo_digest_fp);
+	return 0;
+}
+
+static int prefix_siglen_to_signature(struct signelf_info *sinfo)
+{
+	char buf[4096];
+	FILE *sig_fp;
+	unsigned int nr_read, total_siglen;
+	uint16_t siglen, siglen_be16;
+
+
+	sig_fp = fopen(tempsig_file, "r");
+	if (!sig_fp) {
+		fprintf(stderr, "Failed to open file %s\n", tempsig_file);
+		return 1;
+	}
+
+	/* Read signature from file */
+	nr_read = fread(buf, 1, 4096, sig_fp);
+	if (nr_read == 0 && ferror(sig_fp)) {
+		fprintf(stderr, "Failed to read file %s\n", tempsig_file);
+		fclose(sig_fp);
+		return 1;
+	}
+
+	siglen = (uint16_t)nr_read;
+	siglen_be16 = htobe16(siglen);
+
+	total_siglen = siglen + 2;		// 2 byte for signature len
+
+	/* allocate memory for signature */
+	sinfo->signature = malloc(total_siglen);
+	if (!sinfo->signature) {
+		fprintf(stderr, "Failed to allocated %d bytes\n", total_siglen);
+		fclose(sig_fp);
+		return 1;
+	}
+
+	*(uint16_t*)sinfo->signature = siglen_be16;
+	memcpy(sinfo->signature + 2, buf, siglen);
+	sinfo->signature_len = total_siglen;
+
+	fclose(sig_fp);
+	return 0;
+}
+
+static int sign_hash_rsautl(struct signelf_info *sinfo)
+{
+	int ret, exit_code;
+	char command[1024];
+
+	snprintf(command, 1024, "openssl rsautl -sign -in %s -inkey %s -out %s",
+			temp_algo_ident_digest_file, sinfo->privkey_file,
+			tempsig_file);
+	ret = system(command);
+
+	if (ret == -1) {
+		fprintf(stderr, "Failed to execute system(%s)\n", command);
+		return 1;
+	}
+
+	exit_code = WEXITSTATUS(ret);
+	return exit_code;
+}
+
+static int x509_get_subject_key_identifier(struct signelf_info *sinfo)
+{
+	BIO *cert_buf;
+	X509 *cert_x509;
+	int ret = 0;
+	ASN1_OCTET_STRING *skid;
+
+	cert_buf = BIO_new_file(sinfo->certificate_file, "r");
+	if (!cert_buf) {
+		fprintf(stderr, "BIO_new_file on %s failed\n",
+			sinfo->certificate_file);
+		ret = 1;
+		goto out;
+	}
+
+	cert_x509 = d2i_X509_bio(cert_buf, NULL);
+	if (!cert_x509) {
+		fprintf(stderr, "Failed to parse DER format certificate\n");
+		ret = 1;
+		goto out_free_cert_buf;
+	}
+
+	skid = X509_get_ext_d2i(cert_x509, NID_subject_key_identifier,
+					NULL, NULL);
+	if (!skid) {
+		fprintf(stderr, "Failed to find subject key identifier"
+			" extension in certificate\n");
+		ret = 1;
+		goto out_free_x509;
+	}
+
+	sinfo->key_identifier = malloc(skid->length);
+	if (!sinfo->key_identifier) {
+		fprintf(stderr, "Failed to allocated %d bytes:%s\n",
+				skid->length, strerror(errno));
+		ret = 1;
+		goto out_free_asn1_string;
+	}
+
+	memcpy(sinfo->key_identifier, skid->data, skid->length);
+	sinfo->key_identifier_len = skid->length;
+
+#ifdef DEBUG
+	{
+	int i;
+		printf("skid in hex format:\n");
+		for (i = 0; i < skid->length; i++) {
+			printf("%02X", skid->data[i]);
+			if (i < skid->length - 1)
+				printf(":");
+		}
+		printf("\n");
+	}
+#endif
+
+out_free_asn1_string:
+	ASN1_STRING_free(skid);
+out_free_x509:
+	X509_free(cert_x509);
+out_free_cert_buf:
+	BIO_free(cert_buf);
+out:
+	return ret;
+}
+
+static int x509_get_subject(struct signelf_info *sinfo)
+{
+	BIO *cert_buf;
+	X509 *cert_x509;
+	int ret = 0, len;
+	X509_NAME *subject_x509_name;
+	char *common_name = NULL, *org_name = NULL, *email_addr = NULL;
+	int common_name_len = 0, org_name_len = 0, email_addr_len = 0;
+
+	cert_buf = BIO_new_file(sinfo->certificate_file, "r");
+	if (!cert_buf) {
+		fprintf(stderr, "BIO_new_file on %s failed\n",
+			sinfo->certificate_file);
+		ret = 1;
+		goto out;
+	}
+
+	cert_x509 = d2i_X509_bio(cert_buf, NULL);
+	if (!cert_x509) {
+		fprintf(stderr, "Failed to parse DER format certificate\n");
+		ret = 1;
+		goto out_free_cert_buf;
+	}
+
+	subject_x509_name = X509_get_subject_name(cert_x509);
+	if (!subject_x509_name) {
+		fprintf(stderr, "Failed to get subject name from"
+			" certificate\n");
+		ret = 1;
+		goto out_free_cert_buf;
+	}
+
+	/* Get commonName */
+	len = X509_NAME_get_text_by_NID(subject_x509_name, NID_commonName,
+			NULL, 0);
+
+	if (len != -1) {
+		common_name_len = len;
+		common_name = malloc(len + 1);
+		if (!common_name) {
+			fprintf(stderr, "Failed to allocate %d bytes of"
+				" memory:%s\n", len + 1, strerror(errno));
+			ret = 1;
+			goto out_free_cert_buf;
+		}
+
+		len = X509_NAME_get_text_by_NID(subject_x509_name,
+				NID_commonName, common_name,
+				common_name_len + 1);
+
+		if (len == -1) {
+			fprintf(stderr, "Failed to get commonName\n");
+			ret = 1;
+			goto out_free_common_name;
+		}
+	}
+
+	/* Get orgName */
+	len = X509_NAME_get_text_by_NID(subject_x509_name, NID_organizationName,
+			NULL, 0);
+
+	if (len != -1) {
+		org_name_len = len;
+		org_name = malloc(len + 1);
+		if (!org_name) {
+			fprintf(stderr, "Failed to allocate %d bytes of"
+				" memory:%s\n", len + 1, strerror(errno));
+			ret = 1;
+			goto out_free_common_name;
+		}
+
+		len = X509_NAME_get_text_by_NID(subject_x509_name,
+				NID_organizationName, org_name,
+				org_name_len + 1);
+
+		if (len == -1) {
+			fprintf(stderr, "Failed to get organizationName\n");
+			ret = 1;
+			goto out_free_org_name;
+		}
+	}
+
+	/* Get email addr */
+
+	len = X509_NAME_get_text_by_NID(subject_x509_name,
+					NID_pkcs9_emailAddress, NULL, 0);
+
+	if (len != -1) {
+		email_addr_len = len;
+		email_addr = malloc(len + 1);
+		if (!email_addr) {
+			fprintf(stderr, "Failed to allocate %d bytes of"
+				" memory:%s\n", len + 1, strerror(errno));
+			ret = 1;
+			goto out_free_org_name;
+		}
+
+		len = X509_NAME_get_text_by_NID(subject_x509_name,
+				NID_pkcs9_emailAddress, email_addr,
+				email_addr_len + 1);
+
+		if (len == -1) {
+			fprintf(stderr, "Failed to get email addr\n");
+			ret = 1;
+			goto out_free_email_addr;
+		}
+	}
+
+	sinfo->signer_name = NULL;
+
+	if (common_name_len && org_name_len) {
+		/* If common name contains organization name, don't use it */
+		if (strstr(common_name, org_name)) {
+			sinfo->signer_name = strdup(common_name);
+		} else {
+			sinfo->signer_name = malloc(org_name_len + common_name_len + 3);
+			sprintf(sinfo->signer_name, "%s: %s", org_name,
+					common_name);
+		}
+	} else if (common_name_len) {
+		sinfo->signer_name = strdup(common_name);
+	} else if (org_name_len) {
+		sinfo->signer_name = strdup(org_name);
+	} else if(email_addr)
+		sinfo->signer_name = strdup(email_addr);
+
+	if (!sinfo->signer_name) {
+		fprintf(stderr, "Could not obtain signer name\n");
+		ret = 1;
+	}
+out_free_email_addr:
+	free(email_addr);
+out_free_org_name:
+	free(org_name);
+out_free_common_name:
+	free(common_name);
+out_free_cert_buf:
+	BIO_free(cert_buf);
+out:
+	return ret;
+}
+
+static int prepare_sig_info(struct signelf_info *sinfo)
+{
+	/* algo = 1 (RSA), hash = 4 (sha256), id_type=1 (X.509) */
+	unsigned int algo=1, hash = 4, id_type = 1, i=0;
+	uint8_t signer_name_len = strlen(sinfo->signer_name);
+	uint32_t signature_len_be = htobe32(sinfo->signature_len);
+
+	sinfo->sig_info[i++] = algo;
+	sinfo->sig_info[i++] = hash;
+	sinfo->sig_info[i++] = id_type;
+	sinfo->sig_info[i++] = signer_name_len;
+	sinfo->sig_info[i++] = sinfo->key_identifier_len;
+	sinfo->sig_info[i++] = 0;
+	sinfo->sig_info[i++] = 0;
+	sinfo->sig_info[i++] = 0;
+
+	sinfo->sig_info_len = i;
+	memcpy(&sinfo->sig_info[i], &signature_len_be, 4);
+	sinfo->sig_info_len += 4;
+
+	return 0;
+}
+
+static int add_signature_in_a_section(struct signelf_info *sinfo)
+{
+	FILE *outfp;
+	int ret = 0, exit_code;
+	unsigned int written;
+	unsigned int signer_name_len;
+	char command[1024];
+
+	outfp = fopen(tempsigsection_file, "w+");
+	if (!outfp) {
+		fprintf(stderr, "Failed to open %s:%s\n", tempsigsection_file,
+				strerror(errno));
+		return 1;
+	}
+
+	/* Write signer's name */
+	signer_name_len = strlen(sinfo->signer_name);
+	written = fwrite(sinfo->signer_name, 1, signer_name_len, outfp);
+	if (written != signer_name_len) {
+		fprintf(stderr, "Failed to write signer name to file %s\n",
+				tempsigsection_file);
+		ret = 1;
+		goto out_close_outfp;
+	}
+
+	/* Write skid */
+	written = fwrite(sinfo->key_identifier, 1, sinfo->key_identifier_len,
+				outfp);
+	if (written != sinfo->key_identifier_len) {
+		fprintf(stderr, "Failed to write key identifier to file %s\n",
+				tempsigsection_file);
+		ret = 1;
+		goto out_close_outfp;
+	}
+
+	/* Write signature */
+	written = fwrite(sinfo->signature, 1, sinfo->signature_len, outfp);
+	if (written != sinfo->signature_len) {
+		fprintf(stderr, "Failed to write signature to file %s\n",
+				tempsigsection_file);
+		ret = 1;
+		goto out_close_outfp;
+	}
+
+	/* Write info */
+	written = fwrite(sinfo->sig_info, 1, sinfo->sig_info_len, outfp);
+	if (written != sinfo->sig_info_len) {
+		fprintf(stderr, "Failed to write info to file %s\n",
+				 tempsigsection_file);
+		ret = 1;
+		goto out_close_outfp;
+	}
+
+	/* Add signature section */
+	fflush(outfp);
+	snprintf(command, 1024, "objcopy --add-section .signature=%s %s %s",
+			tempsigsection_file, sinfo->in_file, sinfo->out_file);
+	ret = system(command);
+	if (ret == -1) {
+		fprintf(stderr, "Failed to execute system(%s)\n", command);
+		goto out_close_outfp;
+	}
+
+	exit_code = WEXITSTATUS(ret);
+	ret = exit_code;
+	if (ret)
+		goto out_close_outfp;
+
+out_close_outfp:
+	fclose(outfp);
+	return ret;
+}
+
+static int sign_elf_executable(struct signelf_info *sinfo)
+{
+	int ret, fd;
+
+	fd = open(sinfo->in_file, O_RDONLY);
+	if (fd < 0) {
+		fprintf(stderr, "Cannot open %s: %s\n",
+				sinfo->in_file, strerror(errno));
+		return 1;
+	}
+
+	ret = pread(fd, sinfo->ehdr.e_ident, EI_NIDENT, 0);
+	if (ret != EI_NIDENT) {
+		fprintf(stderr, "Read of e_ident from %s failed: %s\n",
+			sinfo->in_file, strerror(errno));
+		ret = 1;
+		goto out;
+        }
+
+	if (memcmp(sinfo->ehdr.e_ident, ELFMAG, SELFMAG) != 0) {
+		fprintf(stderr, "Missing elf signature\n");
+		ret = 1;
+		goto out;
+        }
+
+	if (sinfo->ehdr.e_ident[EI_VERSION] != EV_CURRENT) {
+		fprintf(stderr, "Bad elf version\n");
+		ret = 1;
+		goto out;
+	}
+
+	if ((sinfo->ehdr.e_ident[EI_CLASS] != ELFCLASS32) &&
+	    (sinfo->ehdr.e_ident[EI_CLASS] != ELFCLASS64))
+	{
+		fprintf(stderr, "Unknown elf class %u\n",
+			sinfo->ehdr.e_ident[EI_CLASS]);
+		ret = 1;
+		goto out;
+	}
+
+	if ((sinfo->ehdr.e_ident[EI_DATA] != ELFDATA2LSB) &&
+	    (sinfo->ehdr.e_ident[EI_DATA] != ELFDATA2MSB))
+	{
+		fprintf(stderr, "Unkown elf data order %u\n",
+			sinfo->ehdr.e_ident[EI_DATA]);
+		ret = 1;
+		goto out;
+	}
+
+	if (sinfo->ehdr.e_ident[EI_CLASS] == ELFCLASS32)
+		ret = read_elf32(sinfo, fd);
+	else
+		ret = read_elf64(sinfo, fd);
+
+	if (!ret)
+		goto out;
+
+	ret = x509_get_subject(sinfo);
+	if (ret)
+		goto out;
+
+	ret = x509_get_subject_key_identifier(sinfo);
+	if (ret)
+		goto out;
+
+	if (calculate_elf_hash(sinfo, fd)) {
+		ret = 1;
+		goto out;
+	}
+
+	ret = prefix_algo_identifier_to_digest(sinfo);
+	if (ret)
+		goto out;
+
+	ret = sign_hash_rsautl(sinfo);
+	if (ret)
+		goto out;
+
+	ret = prefix_siglen_to_signature(sinfo);
+	if (ret)
+		goto out;
+
+	ret = prepare_sig_info(sinfo);
+	if (ret)
+		goto out;
+
+	ret = add_signature_in_a_section(sinfo);
+	if (ret) {
+		fprintf(stderr, "Error while putting signature into an elf"
+			" section\n");
+		goto out;
+	}
+
+out:
+	close(fd);
+	return ret;
+}
+
+static void print_help()
+{
+	printf("Usage: signelf [OPTION...]\n");
+	printf(" -i, --in=<infile>\t\t\t\tspecify input file\n");
+	printf(" -p, --privkey=<privkey>\t\t\tspecify private key file\n");
+	printf(" -c, --certificate=<certificate>\t\tspecify certificate file\n");
+	printf(" -o, --out=<outfile>\t\t\t\tspecify output file\n");
+}
+
+static void free_sinfo_members(struct signelf_info *sinfo)
+{
+	free(sinfo->in_file);
+	free(sinfo->out_file);
+	free(sinfo->privkey_file);
+	free(sinfo->certificate_file);
+	free(sinfo->signer_name);
+	free(sinfo->key_identifier);
+	free(sinfo->signature);
+}
+
+int main(int argc, char *argv[])
+{
+	int ret = 0;
+	char *option_string = "hi:p:c:o:", c;
+	struct signelf_info *sinfo, signelf_info;
+
+	struct option long_options[] =
+		{
+			{"help", no_argument, 0, 'h'},
+			{"in", required_argument, 0, 'i'},
+			{"privkey", required_argument, 0, 'p'},
+			{"certificate", required_argument, 0, 'c'},
+			{"out", required_argument, 0, 'o'},
+			{ 0, 0, 0, 0}
+		};
+
+	if (argc < 2) {
+		print_help();
+		exit(1);
+	}
+
+	sinfo = &signelf_info;
+	memset(sinfo, 0, sizeof(struct signelf_info));
+
+	while((c = getopt_long(argc, argv, option_string, &long_options[0],
+	       NULL)) != -1) {
+		switch(c) {
+		case '?':
+			/* Unknown option or missing argument*/
+			print_help();
+			exit(1);
+		case 'h':
+			print_help();
+			exit(0);
+		case 'i':
+			sinfo->in_file = strdup(optarg);
+			if (!sinfo->in_file) {
+				fprintf(stderr, "Can't duplicate string:%s\n",
+						strerror(errno));
+				exit(1);
+			}
+			break;
+		case 'p':
+			sinfo->privkey_file = strdup(optarg);
+			if (!sinfo->privkey_file) {
+				fprintf(stderr, "Can't duplicate string:%s\n",
+					strerror(errno));
+				exit(1);
+			}
+			break;
+		case 'c':
+			sinfo->certificate_file = strdup(optarg);
+			if (!sinfo->certificate_file) {
+				fprintf(stderr, "Can't duplicate string:%s\n",
+					strerror(errno));
+				exit(1);
+			}
+			break;
+		case 'o':
+			sinfo->out_file = strdup(optarg);
+			if (!sinfo->out_file) {
+				fprintf(stderr, "Can't duplicate string:%s\n",
+					strerror(errno));
+				exit(1);
+			}
+			break;
+		default:
+			printf("Unexpected option\n");
+			exit(1);
+		}
+	}
+
+	if (!sinfo->in_file || !sinfo->out_file || !sinfo->privkey_file ||
+	    !sinfo->certificate_file) {
+		print_help();
+		exit(1);
+	}
+
+	ret = sign_elf_executable(sinfo);
+
+	free_sinfo_members(sinfo);
+	remove(temp_algo_ident_digest_file);
+	remove(tempsig_file);
+	remove(tempsigsection_file);
+
+	exit(ret);
+}
Index: signelf/Makefile
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ signelf/Makefile	2013-01-18 02:27:41.000000000 -0500
@@ -0,0 +1,21 @@
+CC	= gcc
+OPTFLAGS= -g
+CFLAGS	= -Wall -D_GNU_SOURCE $(OPTFLAGS)
+PROGS	= signelf
+OBJS	= signelf.o
+
+INSTALL = install
+prefix = /usr/local
+bindir = $(prefix)/bin
+
+%.o: %.c
+	$(CC) -o $*.o -c $(CFLAGS) $<
+signelf: $(OBJS)
+	$(CC) $(CFLAGS) -o $@ $(filter %.o,$^) -lcrypto
+
+install:
+	$(INSTALL) -m755 -d $(DESTDIR)$(bindir)
+	$(INSTALL) $(PROGS) $(DESTDIR)$(bindir)
+
+clean:
+	rm -f $(OBJS) $(PROGS)

Re: [PATCH 0/3] ELF executable signing and verification

[richard -rw- weinberger]

On Tue, Jan 15, 2013 at 10:34 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> Upon exec(), we determine if executable is signed. If it is, then locks
> down the pages in memory (using MAP_LOCKED) and verfies the signature.
> If signature does not match, process is killed. Unsigned processes
> don't get affected at all.

Also ptrace() should be disallowed on such a process created from a signed ELF.

-- 
Thanks,
//richard

Re: [PATCH 0/3] ELF executable signing and verification

[Vivek Goyal]

On Tue, Jan 15, 2013 at 11:27:12PM +0100, richard -rw- weinberger wrote:
> On Tue, Jan 15, 2013 at 10:34 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > Upon exec(), we determine if executable is signed. If it is, then locks
> > down the pages in memory (using MAP_LOCKED) and verfies the signature.
> > If signature does not match, process is killed. Unsigned processes
> > don't get affected at all.
> 
> Also ptrace() should be disallowed on such a process created from a signed ELF.

Yes, that I have listed in TODO. We need to disable ptrace for signed
processes or make sure only signed process can trace it.

Thanks
Vivek

Re: [PATCH 0/3] ELF executable signing and verification

[richard -rw- weinberger]

On Wed, Jan 16, 2013 at 12:15 AM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Tue, Jan 15, 2013 at 11:27:12PM +0100, richard -rw- weinberger wrote:
>> On Tue, Jan 15, 2013 at 10:34 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> > Upon exec(), we determine if executable is signed. If it is, then locks
>> > down the pages in memory (using MAP_LOCKED) and verfies the signature.
>> > If signature does not match, process is killed. Unsigned processes
>> > don't get affected at all.
>>
>> Also ptrace() should be disallowed on such a process created from a signed ELF.
>
> Yes, that I have listed in TODO. We need to disable ptrace for signed
> processes or make sure only signed process can trace it.

Memo to myself: Reading long mails on my Android sucks. ;)
I totally missed the TODO section. Sorry for the noise.

-- 
Thanks,
//richard

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Eric W. Biederman]

Vivek Goyal <vgoyal@redhat.com> writes:

> If a binary is signed, verify its signature. If signature is not valid, do
> not allow execution. If binary is not signed, execution is allowed
> unconditionally.
>
> CONFIG_BINFMT_ELF_SIGNATURE controls whether elf binary signature support
> is compiled in or not.
>
> Signature are expected to be present in elf section ".section". This code
> is written along the lines of module signature verification code. Just
> that I have removed the magic string. It is not needed as signature is
> expected to be present in a specific section.
>
> I put the signature into a section, instead of appending it so that
> strip operation works fine.
>
> One signs and verifies  all the areas mapped by PT_LOAD segments of elf
> binary. Typically Elf header is mapped in first PT_LOAD segment. As adding
> .signature section can change three elf header fields (e_shoff, e_shnum
> and e_shstrndx), these fields are excluded from digest calculation

My gut feel says that a signature that we verify should reside in an ELF
segment.  Sections are for the linker not the kernel.

I don't totally know what the signature should cover but my gut feels
says the signature should come after ever non-signature segment and
cover all of the prior segments (PT_LOAD or not).  Because presumably
the loader needs to look at everything in a segment.  We can
restrict ourselves to only processing signed binaries on executables
with only PT_LOAD segments and signatures for now.

Eric

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Tue, 2013-01-15 at 20:30 -0800, Eric W. Biederman wrote:
> Vivek Goyal <vgoyal@redhat.com> writes:
> 
> > If a binary is signed, verify its signature. If signature is not valid, do
> > not allow execution. If binary is not signed, execution is allowed
> > unconditionally.
> >
> > CONFIG_BINFMT_ELF_SIGNATURE controls whether elf binary signature support
> > is compiled in or not.
> >
> > Signature are expected to be present in elf section ".section". This code
> > is written along the lines of module signature verification code. Just
> > that I have removed the magic string. It is not needed as signature is
> > expected to be present in a specific section.
> >
> > I put the signature into a section, instead of appending it so that
> > strip operation works fine.
> >
> > One signs and verifies  all the areas mapped by PT_LOAD segments of elf
> > binary. Typically Elf header is mapped in first PT_LOAD segment. As adding
> > .signature section can change three elf header fields (e_shoff, e_shnum
> > and e_shstrndx), these fields are excluded from digest calculation
> 
> My gut feel says that a signature that we verify should reside in an ELF
> segment.  Sections are for the linker not the kernel.
> 
> I don't totally know what the signature should cover but my gut feels
> says the signature should come after ever non-signature segment and
> cover all of the prior segments (PT_LOAD or not).  Because presumably
> the loader needs to look at everything in a segment.  We can
> restrict ourselves to only processing signed binaries on executables
> with only PT_LOAD segments and signatures for now.

Please remind me why you can't use IMA-appraisal, which was upstreamed
in Linux 3.7?  Why another method is needed?

With IMA-appraisal, there are a couple of issues that would still need
to be addressed:
- missing the ability to specify the validation method required.
- modify the ima_appraise_tcb policy policy to require elf executables
to be digitally signed.
- security_bprm_check() is called before the binary handler is known.

The first issue is addressed by a set of patches queued to be upstreamed
in linux-integrity/next-ima-appraise-status.

To address the last issue would either require moving the existing
bprm_check or defining a new hook after the binary handler is known.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> Please remind me why you can't use IMA-appraisal, which was upstreamed
> in Linux 3.7?  Why another method is needed?

Good question Vivek?  

I remeber there was a slight mismatch in the desired attributes.  In
particular we want signatures that are not generated on the local
machine.

> With IMA-appraisal, there are a couple of issues that would still need
> to be addressed:
> - missing the ability to specify the validation method required.
> - modify the ima_appraise_tcb policy policy to require elf executables
> to be digitally signed.
> - security_bprm_check() is called before the binary handler is known.
>
> The first issue is addressed by a set of patches queued to be upstreamed
> in linux-integrity/next-ima-appraise-status.
>
> To address the last issue would either require moving the existing
> bprm_check or defining a new hook after the binary handler is known.

Even if there is a small mismatch it certainly sounds like something to
investigate.  There are a lot of pieces flying around with IMA so an
appropriate model of what needs to happen isn't in my head.  As opposed
to a signature in an ELF executable and a key in the kernel.

Hooks aside in an IMA world where does the signing key live?  Where does
the signature live?

Eric

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Tue, 2013-01-15 at 23:10 -0800, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > Please remind me why you can't use IMA-appraisal, which was upstreamed
> > in Linux 3.7?  Why another method is needed?
> 
> Good question Vivek?  
> 
> I remeber there was a slight mismatch in the desired attributes.  In
> particular we want signatures that are not generated on the local
> machine.

Right, IMA-appraisal supports different methods of verification.  The
initial methods are hash and digital signature stored in the extended
attribute.  With the queued patches, we can force signature verification
to be of a specific type. It defines a new IMA policy option called
'appraise_type='.

> > With IMA-appraisal, there are a couple of issues that would still need
> > to be addressed:
> > - missing the ability to specify the validation method required.
> > - modify the ima_appraise_tcb policy policy to require elf executables
> > to be digitally signed.
> > - security_bprm_check() is called before the binary handler is known.
> >
> > The first issue is addressed by a set of patches queued to be upstreamed
> > in linux-integrity/next-ima-appraise-status.
> >
> > To address the last issue would either require moving the existing
> > bprm_check or defining a new hook after the binary handler is known.
> 
> Even if there is a small mismatch it certainly sounds like something to
> investigate.  There are a lot of pieces flying around with IMA so an
> appropriate model of what needs to happen isn't in my head.  As opposed
> to a signature in an ELF executable and a key in the kernel.

The original IMA was about measurement and attestation.  IMA-appraisal 
adds local integrity validation and enforcement of the measurement
against a "good" value stored as an extended attribute 'security.ima'.
The initial methods for validating 'security.ima' are hash and digital
signature based.

> Hooks aside in an IMA world where does the signing key live?  Where does
> the signature live?

Initially, the public key used to verify the signature is loaded onto an
IMA specific keyring.  We've discussed embedding public keys inside the
kernel, but haven't done so yet.

The next steps are to ensure the secure boot signature chain of trust
has not been broken.

(I've cc'ed Dmitry Kasatkin on this discussion.  It would also be
appropriate to cc the LSM mailing list.)

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Wed, Jan 16, 2013 at 09:00:59AM -0500, Mimi Zohar wrote:
> On Tue, 2013-01-15 at 23:10 -0800, Eric W. Biederman wrote:
> > Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> > 
> > > Please remind me why you can't use IMA-appraisal, which was upstreamed
> > > in Linux 3.7?  Why another method is needed?
> > 
> > Good question Vivek?  

- IMA did not have any method to lock down signed binary pages in memory.
  So while contents on disk could be verified, one could still modify
  process memory contents by modifying swap. And IMA does not seem to
  have any mechanism to protect against that.

- Also I really could not figure out where does the private signing key
  lives. I got the impression that we need to trust installer and
  signing somehow happens at installation time. And we wanted signing
  to happen at build server and could not trust installer for that.

  My understanding of IMA could be wrong. So it would help if you
  could list the exact steps about how to achieve the same goal using
  IMA.
 
> > 
> > I remeber there was a slight mismatch in the desired attributes.  In
> > particular we want signatures that are not generated on the local
> > machine.
> 
> Right, IMA-appraisal supports different methods of verification.  The
> initial methods are hash and digital signature stored in the extended
> attribute.  With the queued patches, we can force signature verification
> to be of a specific type. It defines a new IMA policy option called
> 'appraise_type='.
> 
> > > With IMA-appraisal, there are a couple of issues that would still need
> > > to be addressed:
> > > - missing the ability to specify the validation method required.
> > > - modify the ima_appraise_tcb policy policy to require elf executables
> > > to be digitally signed.
> > > - security_bprm_check() is called before the binary handler is known.
> > >
> > > The first issue is addressed by a set of patches queued to be upstreamed
> > > in linux-integrity/next-ima-appraise-status.
> > >
> > > To address the last issue would either require moving the existing
> > > bprm_check or defining a new hook after the binary handler is known.
> > 
> > Even if there is a small mismatch it certainly sounds like something to
> > investigate.  There are a lot of pieces flying around with IMA so an
> > appropriate model of what needs to happen isn't in my head.  As opposed
> > to a signature in an ELF executable and a key in the kernel.
> 
> The original IMA was about measurement and attestation.  IMA-appraisal 
> adds local integrity validation and enforcement of the measurement
> against a "good" value stored as an extended attribute 'security.ima'.
> The initial methods for validating 'security.ima' are hash and digital
> signature based.
> 
> > Hooks aside in an IMA world where does the signing key live?  Where does
> > the signature live?
> 
> Initially, the public key used to verify the signature is loaded onto an
> IMA specific keyring.  We've discussed embedding public keys inside the
> kernel, but haven't done so yet.

So where does the signing key (private key) live? And when does actual
signing happens and who does it.

> 
> The next steps are to ensure the secure boot signature chain of trust
> has not been broken.

Yes this one is important. This will also include making sure root can
not load/install its own keys until and unless new key is signed with
one of existing keys. Otherwise chain of trust is broken.

> 
> (I've cc'ed Dmitry Kasatkin on this discussion.  It would also be
> appropriate to cc the LSM mailing list.)

I am CCing LSM list and Andrew Morton.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Wed, 2013-01-16 at 09:48 -0500, Vivek Goyal wrote:
> On Wed, Jan 16, 2013 at 09:00:59AM -0500, Mimi Zohar wrote:
> > On Tue, 2013-01-15 at 23:10 -0800, Eric W. Biederman wrote:
> > > Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> > > 
> > > > Please remind me why you can't use IMA-appraisal, which was upstreamed
> > > > in Linux 3.7?  Why another method is needed?
> > > 
> > > Good question Vivek?  
> 
> - IMA did not have any method to lock down signed binary pages in memory.
>   So while contents on disk could be verified, one could still modify
>   process memory contents by modifying swap. And IMA does not seem to
>   have any mechanism to protect against that.

The kernel itself protects executables from being modified by calling
try_module_get().  The call to security_bprm_check() is immediately
before this call.

> - Also I really could not figure out where does the private signing key
>   lives. I got the impression that we need to trust installer and
>   signing somehow happens at installation time. And we wanted signing
>   to happen at build server and could not trust installer for that.

Dmitry's ima-evm-utils package signs files.  Depending on the options,
both the EVM and IMA extended attributes are created.

>   My understanding of IMA could be wrong. So it would help if you
>   could list the exact steps about how to achieve the same goal using
>   IMA.

http://linux-ima.sourceforge.net/  needs to be updated, but it describes
the integrity subsystem and includes a link to Dave Safford's original
whitepaper "An Overview of the Linux Integrity subsystem".

> > > 
> > > I remeber there was a slight mismatch in the desired attributes.  In
> > > particular we want signatures that are not generated on the local
> > > machine.
> > 
> > Right, IMA-appraisal supports different methods of verification.  The
> > initial methods are hash and digital signature stored in the extended
> > attribute.  With the queued patches, we can force signature verification
> > to be of a specific type. It defines a new IMA policy option called
> > 'appraise_type='.
> > 
> > > > With IMA-appraisal, there are a couple of issues that would still need
> > > > to be addressed:
> > > > - missing the ability to specify the validation method required.
> > > > - modify the ima_appraise_tcb policy policy to require elf executables
> > > > to be digitally signed.
> > > > - security_bprm_check() is called before the binary handler is known.
> > > >
> > > > The first issue is addressed by a set of patches queued to be upstreamed
> > > > in linux-integrity/next-ima-appraise-status.
> > > >
> > > > To address the last issue would either require moving the existing
> > > > bprm_check or defining a new hook after the binary handler is known.
> > > 
> > > Even if there is a small mismatch it certainly sounds like something to
> > > investigate.  There are a lot of pieces flying around with IMA so an
> > > appropriate model of what needs to happen isn't in my head.  As opposed
> > > to a signature in an ELF executable and a key in the kernel.
> > 
> > The original IMA was about measurement and attestation.  IMA-appraisal 
> > adds local integrity validation and enforcement of the measurement
> > against a "good" value stored as an extended attribute 'security.ima'.
> > The initial methods for validating 'security.ima' are hash and digital
> > signature based.
> > 
> > > Hooks aside in an IMA world where does the signing key live?  Where does
> > > the signature live?
> > 
> > Initially, the public key used to verify the signature is loaded onto an
> > IMA specific keyring.  We've discussed embedding public keys inside the
> > kernel, but haven't done so yet.
> 
> So where does the signing key (private key) live? And when does actual
> signing happens and who does it.

The signing process is currently not part of kbuild, but a separate
process, as mentioned above.

> > 
> > The next steps are to ensure the secure boot signature chain of trust
> > has not been broken.
> 
> Yes this one is important. This will also include making sure root can
> not load/install its own keys until and unless new key is signed with
> one of existing keys. Otherwise chain of trust is broken.

Right.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Wed, Jan 16, 2013 at 10:33:11AM -0500, Mimi Zohar wrote:

[..]
> > - Also I really could not figure out where does the private signing key
> >   lives. I got the impression that we need to trust installer and
> >   signing somehow happens at installation time. And we wanted signing
> >   to happen at build server and could not trust installer for that.
> 
> Dmitry's ima-evm-utils package signs files.  Depending on the options,
> both the EVM and IMA extended attributes are created.

I was going through following presentation.

http://selinuxproject.org/~jmorris/lss2011_slides/IMA_EVM_Digital_Signature_Support.pdf

On slide 8, it mentons signing.

	evmctl sign --imahash /path/to/file
	evmctl sign --imasig /path/to/file

Can't figure out where does the key for signing come from? Is it already
loaded in any of kernel keyrings.

If yes, I think this is non-starter. One can not distribute the private
key.

Also I am assuming that this is done at installation time? If yes, then
again it does not work as installer does not have private key.

On slide 11, it talks about importing public keys in kernel keyring from
initramfs. As we discussed this will need modification as these keys
need to be signed and signing public key should already be part of 
kernel keyring.

So looking at the signing process, it really does not look like that
I can sign the executable at build server. It looks it needs to be
signed by installer at install time and private key needs to be available
to installer?
 
> 
> >   My understanding of IMA could be wrong. So it would help if you
> >   could list the exact steps about how to achieve the same goal using
> >   IMA.
> 
> http://linux-ima.sourceforge.net/  needs to be updated, but it describes
> the integrity subsystem and includes a link to Dave Safford's original
> whitepaper "An Overview of the Linux Integrity subsystem".

I have gone through the paper in the past and still the quetions remain
unanswered. So it will really help, if you could take a very simple
example of hello-world executable and list the steps needed to be
performed to sign and verify executable.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Wed, Jan 16, 2013 at 10:33:11AM -0500, Mimi Zohar wrote:
> On Wed, 2013-01-16 at 09:48 -0500, Vivek Goyal wrote:
> > On Wed, Jan 16, 2013 at 09:00:59AM -0500, Mimi Zohar wrote:
> > > On Tue, 2013-01-15 at 23:10 -0800, Eric W. Biederman wrote:
> > > > Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> > > > 
> > > > > Please remind me why you can't use IMA-appraisal, which was upstreamed
> > > > > in Linux 3.7?  Why another method is needed?
> > > > 
> > > > Good question Vivek?  
> > 
> > - IMA did not have any method to lock down signed binary pages in memory.
> >   So while contents on disk could be verified, one could still modify
> >   process memory contents by modifying swap. And IMA does not seem to
> >   have any mechanism to protect against that.
> 
> The kernel itself protects executables from being modified by calling
> try_module_get().

try_module_get() just takes a reference on module, isn't it? So in this
case take reference on binary loader module.

> The call to security_bprm_check() is immediately before this call.

I read the comment in ima_bprm_check() being called from security_bprm_check(). 
It says that files already open for write can't executed and files already
open for exec can't be open for writes. That's fine. 

I was worried about anonymous pages being modified on swap and then
faulted back in. It is not necessarily signature verification but making
sure signed processes memory is not modified later by any unsigned process
in anyway. And that includes disabling ptrace too.

So IMA stuff does not do anything to protect against process memory being
modified using ptrace or by playing tricks with swap.

I am not sure what will happen if I can bypass the file system and directly
write on a disk block and modify executable. (Assuming one can get block
information somehow). Does anything protect such modification? Will IMA
detect it?

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Wed, 2013-01-16 at 10:54 -0500, Vivek Goyal wrote:
> On Wed, Jan 16, 2013 at 10:33:11AM -0500, Mimi Zohar wrote:
> 
> [..]
> > > - Also I really could not figure out where does the private signing key
> > >   lives. I got the impression that we need to trust installer and
> > >   signing somehow happens at installation time. And we wanted signing
> > >   to happen at build server and could not trust installer for that.
> > 
> > Dmitry's ima-evm-utils package signs files.  Depending on the options,
> > both the EVM and IMA extended attributes are created.
> 
> I was going through following presentation.
> 
> http://selinuxproject.org/~jmorris/lss2011_slides/IMA_EVM_Digital_Signature_Support.pdf
> 
> On slide 8, it mentons signing.
> 
> 	evmctl sign --imahash /path/to/file
> 	evmctl sign --imasig /path/to/file
> 
> Can't figure out where does the key for signing come from? Is it already
> loaded in any of kernel keyrings.
> 
> If yes, I think this is non-starter. One can not distribute the private
> key.

No, the default key location is /etc/keys/privkey_evm.pem, but can be
specified.  Prior to Dmitry's updating the package yesterday, the last
parameter was the key pathname.  After the update, you can specify the
key location with the new --key option.

> Also I am assuming that this is done at installation time? If yes, then
> again it does not work as installer does not have private key.

Not necessarily.  The original use case scenario was creating an image
with both EVM and IMA digital signatures and then flashing the image.

> On slide 11, it talks about importing public keys in kernel keyring from
> initramfs. As we discussed this will need modification as these keys
> need to be signed and signing public key should already be part of 
> kernel keyring.

It's been a long process upstreaming all the different pieces involved.
The initial design/step was to load the public key on a keyring.  Since
then we've added support for multiple keyrings(eg. EVM, IMA, etc).  The
next step is to tie this in with secure boot.

> So looking at the signing process, it really does not look like that
> I can sign the executable at build server. It looks it needs to be
> signed by installer at install time and private key needs to be available
> to installer?

No, the build server can sign the files, so the private key is not
required on the target.  These signatures need to be included in the
package.  Elena Reshetova gave a talk at the last LSS
(http://lwn.net/Articles/518265/), describing changes to RPM to write
the security extended attributes.

> > 
> > >   My understanding of IMA could be wrong. So it would help if you
> > >   could list the exact steps about how to achieve the same goal using
> > >   IMA.
> > 
> > http://linux-ima.sourceforge.net/  needs to be updated, but it describes
> > the integrity subsystem and includes a link to Dave Safford's original
> > whitepaper "An Overview of the Linux Integrity subsystem".
> 
> I have gone through the paper in the past and still the quetions remain
> unanswered. So it will really help, if you could take a very simple
> example of hello-world executable and list the steps needed to be
> performed to sign and verify executable.

Ok, will post this separately.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Wed, 2013-01-16 at 11:34 -0500, Vivek Goyal wrote:

> I read the comment in ima_bprm_check() being called from security_bprm_check(). 
> It says that files already open for write can't executed and files already
> open for exec can't be open for writes. That's fine. 
> 
> I was worried about anonymous pages being modified on swap and then
> faulted back in. It is not necessarily signature verification but making
> sure signed processes memory is not modified later by any unsigned process
> in anyway. And that includes disabling ptrace too.
> 
> So IMA stuff does not do anything to protect against process memory being
> modified using ptrace or by playing tricks with swap.

> I am not sure what will happen if I can bypass the file system and directly
> write on a disk block and modify executable. (Assuming one can get block
> information somehow). Does anything protect such modification? Will IMA
> detect it?

Sorry, this is out of scope for IMA.  Dmitry has looked into this, but
I'm not sure where it stands at the moment.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Wed, Jan 16, 2013 at 12:24:39PM -0500, Mimi Zohar wrote:
> On Wed, 2013-01-16 at 10:54 -0500, Vivek Goyal wrote:
> > On Wed, Jan 16, 2013 at 10:33:11AM -0500, Mimi Zohar wrote:
> > 
> > [..]
> > > > - Also I really could not figure out where does the private signing key
> > > >   lives. I got the impression that we need to trust installer and
> > > >   signing somehow happens at installation time. And we wanted signing
> > > >   to happen at build server and could not trust installer for that.
> > > 
> > > Dmitry's ima-evm-utils package signs files.  Depending on the options,
> > > both the EVM and IMA extended attributes are created.
> > 
> > I was going through following presentation.
> > 
> > http://selinuxproject.org/~jmorris/lss2011_slides/IMA_EVM_Digital_Signature_Support.pdf
> > 
> > On slide 8, it mentons signing.
> > 
> > 	evmctl sign --imahash /path/to/file
> > 	evmctl sign --imasig /path/to/file
> > 
> > Can't figure out where does the key for signing come from? Is it already
> > loaded in any of kernel keyrings.
> > 
> > If yes, I think this is non-starter. One can not distribute the private
> > key.
> 
> No, the default key location is /etc/keys/privkey_evm.pem, but can be
> specified.  Prior to Dmitry's updating the package yesterday, the last
> parameter was the key pathname.  After the update, you can specify the
> key location with the new --key option.
> 
> > Also I am assuming that this is done at installation time? If yes, then
> > again it does not work as installer does not have private key.
> 
> Not necessarily.  The original use case scenario was creating an image
> with both EVM and IMA digital signatures and then flashing the image.

But distributions don't ship pre-setup root file system. It is created
and labeled at installation time. So if we want to use IMA, we need to
sign files at installation time. That means private keys need to be
accessible to installer (using --key option). And that does not work for
this use case.

> 
> > On slide 11, it talks about importing public keys in kernel keyring from
> > initramfs. As we discussed this will need modification as these keys
> > need to be signed and signing public key should already be part of 
> > kernel keyring.
> 
> It's been a long process upstreaming all the different pieces involved.
> The initial design/step was to load the public key on a keyring.  Since
> then we've added support for multiple keyrings(eg. EVM, IMA, etc).  The
> next step is to tie this in with secure boot.
> 
> > So looking at the signing process, it really does not look like that
> > I can sign the executable at build server. It looks it needs to be
> > signed by installer at install time and private key needs to be available
> > to installer?
> 
> No, the build server can sign the files, so the private key is not
> required on the target.  These signatures need to be included in the
> package.  Elena Reshetova gave a talk at the last LSS
> (http://lwn.net/Articles/518265/), describing changes to RPM to write
> the security extended attributes.

Following is from article.

"The other hooks that Reshetova proposed are associated with the individual
files in a package. Those would allow things like security labeling or
calculating hashes on the file contents (for integrity purposes)."

IIUC, these hooks will execute on the target. So hash and all signatures
should still be generated on target and not build server?

Given the fact that signatures are stored in extended attributes, to me
the only way to sign executables in current IMA framework would to be
prepare file system image at build server and ship that image. And
then installer simply mounts that image (after making sure that proper
verification keys have been loaded in kernel).

And this image will pretty much be static. As you don't have private key
you can't install more packages and sign these.

So it will be something like one image per signed package (in worst case).

I don't think starting to ship signed file system images as compared
to signed files is a better idea. 

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Wed, Jan 16, 2013 at 01:08:35PM -0500, Mimi Zohar wrote:
> On Wed, 2013-01-16 at 11:34 -0500, Vivek Goyal wrote:
> 
> > I read the comment in ima_bprm_check() being called from security_bprm_check(). 
> > It says that files already open for write can't executed and files already
> > open for exec can't be open for writes. That's fine. 
> > 
> > I was worried about anonymous pages being modified on swap and then
> > faulted back in. It is not necessarily signature verification but making
> > sure signed processes memory is not modified later by any unsigned process
> > in anyway. And that includes disabling ptrace too.
> > 
> > So IMA stuff does not do anything to protect against process memory being
> > modified using ptrace or by playing tricks with swap.
> 
> > I am not sure what will happen if I can bypass the file system and directly
> > write on a disk block and modify executable. (Assuming one can get block
> > information somehow). Does anything protect such modification? Will IMA
> > detect it?
> 
> Sorry, this is out of scope for IMA.  Dmitry has looked into this, but
> I'm not sure where it stands at the moment.

Ok, so that's one reason that why I wrote these patcehs. IMA currently
is not doing following things to make sure address space of signed images
is not modified by others.

- Protecting against modifications to pages on swap.
- Protecting against modifications by ptrace.
- Protecting against modifications which bypassed filesystem and directly
  wrote to the block.
 
Locking down all the pages of signed binaries in memory hopefully should
solve above problems.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Wed, 2013-01-16 at 13:21 -0500, Vivek Goyal wrote:
> On Wed, Jan 16, 2013 at 12:24:39PM -0500, Mimi Zohar wrote:
> > On Wed, 2013-01-16 at 10:54 -0500, Vivek Goyal wrote:
> > > On Wed, Jan 16, 2013 at 10:33:11AM -0500, Mimi Zohar wrote:
> > > 
> > > [..]
> > > > > - Also I really could not figure out where does the private signing key
> > > > >   lives. I got the impression that we need to trust installer and
> > > > >   signing somehow happens at installation time. And we wanted signing
> > > > >   to happen at build server and could not trust installer for that.
> > > > 
> > > > Dmitry's ima-evm-utils package signs files.  Depending on the options,
> > > > both the EVM and IMA extended attributes are created.
> > > 
> > > I was going through following presentation.
> > > 
> > > http://selinuxproject.org/~jmorris/lss2011_slides/IMA_EVM_Digital_Signature_Support.pdf
> > > 
> > > On slide 8, it mentons signing.
> > > 
> > > 	evmctl sign --imahash /path/to/file
> > > 	evmctl sign --imasig /path/to/file
> > > 
> > > Can't figure out where does the key for signing come from? Is it already
> > > loaded in any of kernel keyrings.
> > > 
> > > If yes, I think this is non-starter. One can not distribute the private
> > > key.
> > 
> > No, the default key location is /etc/keys/privkey_evm.pem, but can be
> > specified.  Prior to Dmitry's updating the package yesterday, the last
> > parameter was the key pathname.  After the update, you can specify the
> > key location with the new --key option.
> > 
> > > Also I am assuming that this is done at installation time? If yes, then
> > > again it does not work as installer does not have private key.
> > 
> > Not necessarily.  The original use case scenario was creating an image
> > with both EVM and IMA digital signatures and then flashing the image.
> 
> But distributions don't ship pre-setup root file system. It is created
> and labeled at installation time. So if we want to use IMA, we need to
> sign files at installation time. That means private keys need to be
> accessible to installer (using --key option). And that does not work for
> this use case.

No, packages contain file data and the associated file metadata,
including extended attributes.  Files would be signed as part of the
build process and written out as extended attributes.

> > 
> > > On slide 11, it talks about importing public keys in kernel keyring from
> > > initramfs. As we discussed this will need modification as these keys
> > > need to be signed and signing public key should already be part of 
> > > kernel keyring.
> > 
> > It's been a long process upstreaming all the different pieces involved.
> > The initial design/step was to load the public key on a keyring.  Since
> > then we've added support for multiple keyrings(eg. EVM, IMA, etc).  The
> > next step is to tie this in with secure boot.
> > 
> > > So looking at the signing process, it really does not look like that
> > > I can sign the executable at build server. It looks it needs to be
> > > signed by installer at install time and private key needs to be available
> > > to installer?
> > 
> > No, the build server can sign the files, so the private key is not
> > required on the target.  These signatures need to be included in the
> > package.  Elena Reshetova gave a talk at the last LSS
> > (http://lwn.net/Articles/518265/), describing changes to RPM to write
> > the security extended attributes.
> 
> Following is from article.
> 
> "The other hooks that Reshetova proposed are associated with the individual
> files in a package. Those would allow things like security labeling or
> calculating hashes on the file contents (for integrity purposes)."
> 
> IIUC, these hooks will execute on the target. So hash and all signatures
> should still be generated on target and not build server?

No, the package, including the extended attributes containing the
signature, are installed on the target system.  The associated public
key needs to also be installed on the target, not the private key.

> Given the fact that signatures are stored in extended attributes, to me
> the only way to sign executables in current IMA framework would to be
> prepare file system image at build server and ship that image. And
> then installer simply mounts that image (after making sure that proper
> verification keys have been loaded in kernel).

That is one scenario.  Another scenario is to update packages to include
extended attributes and to write those extended attributes on
installation.  This is currently supported for some, but not all
security extended attributes.  Elena's talk discussed doing this in a
generic manner for all security extended attributes.

> And this image will pretty much be static. As you don't have private key
> you can't install more packages and sign these.

No! You're missing the point.  We don't need the private key to install
a package.  There, however, does need to be a safe and trusted mechanism
to install the public key.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Wed, Jan 16, 2013 at 01:45:12PM -0500, Mimi Zohar wrote:

[..]
> > Given the fact that signatures are stored in extended attributes, to me
> > the only way to sign executables in current IMA framework would to be
> > prepare file system image at build server and ship that image. And
> > then installer simply mounts that image (after making sure that proper
> > verification keys have been loaded in kernel).
> 
> That is one scenario.  Another scenario is to update packages to include
> extended attributes and to write those extended attributes on
> installation.

Ok, that's the point I am missing. So I can sign a file and signatures
are in a separate file. And these signatures are installed in extended
attributes at file installation time (IOW rpm installation time) on
target.

If all this works, this sounds reasonable so far. Except the point of
disabling ptrace and locking down memory. 

So what's the state of above work. Is there something I can play with.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Wed, 2013-01-16 at 13:28 -0500, Vivek Goyal wrote:
> On Wed, Jan 16, 2013 at 01:08:35PM -0500, Mimi Zohar wrote:
> > On Wed, 2013-01-16 at 11:34 -0500, Vivek Goyal wrote:
> > 
> > > I read the comment in ima_bprm_check() being called from security_bprm_check(). 
> > > It says that files already open for write can't executed and files already
> > > open for exec can't be open for writes. That's fine. 
> > > 
> > > I was worried about anonymous pages being modified on swap and then
> > > faulted back in. It is not necessarily signature verification but making
> > > sure signed processes memory is not modified later by any unsigned process
> > > in anyway. And that includes disabling ptrace too.
> > > 
> > > So IMA stuff does not do anything to protect against process memory being
> > > modified using ptrace or by playing tricks with swap.
> > 
> > > I am not sure what will happen if I can bypass the file system and directly
> > > write on a disk block and modify executable. (Assuming one can get block
> > > information somehow). Does anything protect such modification? Will IMA
> > > detect it?
> > 
> > Sorry, this is out of scope for IMA.  Dmitry has looked into this, but
> > I'm not sure where it stands at the moment.
> 
> Ok, so that's one reason that why I wrote these patcehs. IMA currently
> is not doing following things to make sure address space of signed images
> is not modified by others.
> 
> - Protecting against modifications to pages on swap.
> - Protecting against modifications by ptrace.
> - Protecting against modifications which bypassed filesystem and directly
>   wrote to the block.
> 
> Locking down all the pages of signed binaries in memory hopefully should
> solve above problems.

Signing and verifying ELF executables goes back a long time ~2003/4,
from a number of esteemed kernel developers, including Greg-KH and Serge
Hallyn.

IMA-appraisal isn't limited to appraising a single type of file, but is
a generic mechanism for appraising all files.  If there are issues that
aren't being addressed, then by all means, please help by addressing
them.  Duplicating a large portion of the code is not productive.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Wed, 2013-01-16 at 13:57 -0500, Vivek Goyal wrote:
> On Wed, Jan 16, 2013 at 01:45:12PM -0500, Mimi Zohar wrote:
> 
> [..]
> > > Given the fact that signatures are stored in extended attributes, to me
> > > the only way to sign executables in current IMA framework would to be
> > > prepare file system image at build server and ship that image. And
> > > then installer simply mounts that image (after making sure that proper
> > > verification keys have been loaded in kernel).
> > 
> > That is one scenario.  Another scenario is to update packages to include
> > extended attributes and to write those extended attributes on
> > installation.
> 
> Ok, that's the point I am missing. So I can sign a file and signatures
> are in a separate file. And these signatures are installed in extended
> attributes at file installation time (IOW rpm installation time) on
> target.
> 
> If all this works, this sounds reasonable so far. Except the point of
> disabling ptrace and locking down memory. 
> 
> So what's the state of above work. Is there something I can play with.

Sorry, I'm not sure of the RPM implementation details of where/how the
signatures are stored in the package, nor of the status of these
changes.  Perhaps someone else on the mailing list knows.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Wed, Jan 16, 2013 at 02:37:24PM -0500, Mimi Zohar wrote:
> On Wed, 2013-01-16 at 13:57 -0500, Vivek Goyal wrote:
> > On Wed, Jan 16, 2013 at 01:45:12PM -0500, Mimi Zohar wrote:
> > 
> > [..]
> > > > Given the fact that signatures are stored in extended attributes, to me
> > > > the only way to sign executables in current IMA framework would to be
> > > > prepare file system image at build server and ship that image. And
> > > > then installer simply mounts that image (after making sure that proper
> > > > verification keys have been loaded in kernel).
> > > 
> > > That is one scenario.  Another scenario is to update packages to include
> > > extended attributes and to write those extended attributes on
> > > installation.
> > 
> > Ok, that's the point I am missing. So I can sign a file and signatures
> > are in a separate file. And these signatures are installed in extended
> > attributes at file installation time (IOW rpm installation time) on
> > target.
> > 
> > If all this works, this sounds reasonable so far. Except the point of
> > disabling ptrace and locking down memory. 
> > 
> > So what's the state of above work. Is there something I can play with.
> 
> Sorry, I'm not sure of the RPM implementation details of where/how the
> signatures are stored in the package, nor of the status of these
> changes.  Perhaps someone else on the mailing list knows.

So irrespective of fact how RPM does it. What are basic commands/steps to
generate signature of a file and how to store it later in an extended
attribute?

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Wed, 2013-01-16 at 14:47 -0500, Vivek Goyal wrote:
> On Wed, Jan 16, 2013 at 02:37:24PM -0500, Mimi Zohar wrote:
> > On Wed, 2013-01-16 at 13:57 -0500, Vivek Goyal wrote:
> > > On Wed, Jan 16, 2013 at 01:45:12PM -0500, Mimi Zohar wrote:
> > > 
> > > [..]
> > > > > Given the fact that signatures are stored in extended attributes, to me
> > > > > the only way to sign executables in current IMA framework would to be
> > > > > prepare file system image at build server and ship that image. And
> > > > > then installer simply mounts that image (after making sure that proper
> > > > > verification keys have been loaded in kernel).
> > > > 
> > > > That is one scenario.  Another scenario is to update packages to include
> > > > extended attributes and to write those extended attributes on
> > > > installation.
> > > 
> > > Ok, that's the point I am missing. So I can sign a file and signatures
> > > are in a separate file. And these signatures are installed in extended
> > > attributes at file installation time (IOW rpm installation time) on
> > > target.
> > > 
> > > If all this works, this sounds reasonable so far. Except the point of
> > > disabling ptrace and locking down memory. 
> > > 
> > > So what's the state of above work. Is there something I can play with.
> > 
> > Sorry, I'm not sure of the RPM implementation details of where/how the
> > signatures are stored in the package, nor of the status of these
> > changes.  Perhaps someone else on the mailing list knows.
> 
> So irrespective of fact how RPM does it. What are basic commands/steps to
> generate signature of a file and how to store it later in an extended
> attribute?

evmctl calculates and writes out the 'security.evm' and 'security.ima'
extended attribute.  The ima-evm-utils package README contains some
directions for getting started.  We should probably move this thread to
the linux-ima-user mailing list.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Wed, Jan 16, 2013 at 02:24:50PM -0500, Mimi Zohar wrote:
[..]
> > > Sorry, this is out of scope for IMA.  Dmitry has looked into this, but
> > > I'm not sure where it stands at the moment.
> > 
> > Ok, so that's one reason that why I wrote these patcehs. IMA currently
> > is not doing following things to make sure address space of signed images
> > is not modified by others.
> > 
> > - Protecting against modifications to pages on swap.
> > - Protecting against modifications by ptrace.
> > - Protecting against modifications which bypassed filesystem and directly
> >   wrote to the block.
> > 
> > Locking down all the pages of signed binaries in memory hopefully should
> > solve above problems.
> 
> Signing and verifying ELF executables goes back a long time ~2003/4,
> from a number of esteemed kernel developers, including Greg-KH and Serge
> Hallyn.
> 
> IMA-appraisal isn't limited to appraising a single type of file, but is
> a generic mechanism for appraising all files.  If there are issues that
> aren't being addressed, then by all means, please help by addressing
> them.  Duplicating a large portion of the code is not productive.

So do you have ideas on how to address above mentioned issues. Do they
fit into the realm of IMA/EVM or I just need to write separate code (which
I have already done).

With above issues, IMA stuff for executable files sounds incomplete.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Wed, Jan 16, 2013 at 03:25:57PM -0500, Mimi Zohar wrote:

[..]
> > So irrespective of fact how RPM does it. What are basic commands/steps to
> > generate signature of a file and how to store it later in an extended
> > attribute?
> 
> evmctl calculates and writes out the 'security.evm' and 'security.ima'
> extended attribute.  The ima-evm-utils package README contains some
> directions for getting started.  We should probably move this thread to
> the linux-ima-user mailing list.

Ok, I will go through README file. I think we should still keep this
thread on lkml as it is not yet clear whether IMA is sufficient for
this requirement or not.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Tue, 2013-01-15 at 16:34 -0500, Vivek Goyal wrote:
> If a binary is signed, verify its signature. If signature is not valid, do
> not allow execution. If binary is not signed, execution is allowed
> unconditionally.

Basically you're building the policy into the executable.  Anyone can
rebuild the executable and, without signing it, install/replace an
existing one.  How is this safe?  The signature verification policy
needs to be defined independently of the executable.

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Wed, Jan 16, 2013 at 05:35:23PM -0500, Mimi Zohar wrote:
> On Tue, 2013-01-15 at 16:34 -0500, Vivek Goyal wrote:
> > If a binary is signed, verify its signature. If signature is not valid, do
> > not allow execution. If binary is not signed, execution is allowed
> > unconditionally.
> 
> Basically you're building the policy into the executable.  Anyone can
> rebuild the executable and, without signing it, install/replace an
> existing one.  How is this safe?  The signature verification policy
> needs to be defined independently of the executable.

Upon signature verification this executable will also acquire a new
capability(say CAP_SIGNED). And some of the services can be allowed only
if process has that new capability. (TODO item)

So if somebody removes the signature and launches same executable, sure
it will run but it will fail to get some of the kernel services. For
example, in this case it should not be able to call sys_kexec() system
call.

Right now my goal is to not run anything which is not signed. For that
we will need a fully signed user space. And I am not sure that's going
to happen in near future, atleast for the generic case.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Eric W. Biederman]

Vivek Goyal <vgoyal@redhat.com> writes:

> On Wed, Jan 16, 2013 at 05:35:23PM -0500, Mimi Zohar wrote:
>> On Tue, 2013-01-15 at 16:34 -0500, Vivek Goyal wrote:
>> > If a binary is signed, verify its signature. If signature is not valid, do
>> > not allow execution. If binary is not signed, execution is allowed
>> > unconditionally.
>> 
>> Basically you're building the policy into the executable.  Anyone can
>> rebuild the executable and, without signing it, install/replace an
>> existing one.  How is this safe?  The signature verification policy
>> needs to be defined independently of the executable.
>
> Upon signature verification this executable will also acquire a new
> capability(say CAP_SIGNED). And some of the services can be allowed only
> if process has that new capability. (TODO item)

Just a quick segway.  The ptrace problem is solved by existing
mechanisms if you have a capability that other binaries don't have.

Eric

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Elena Reshetova]

>> > Ok, that's the point I am missing. So I can sign a file and signatures
>> > are in a separate file. And these signatures are installed in extended
>> > attributes at file installation time (IOW rpm installation time) on
>> > target.
>> >
>> > If all this works, this sounds reasonable so far. Except the point of
>> > disabling ptrace and locking down memory.
>> >
>> > So what's the state of above work. Is there something I can play with.

Let me try to comment on this one a bit.
Thewhole idea behind extending rpm plugin interface was to have an extensive
 set of hooks that would allow rpm plugins to perform needed additional things.
Plugins can be different dependening on a ditsibution need, and if a
distribution
needs to bootstrap IMA signatures, this can also be done in  one of
plugins hooks.

Now about hook status: we have so far integrated to rpm master branch
only a subset of hooks.
Mainly the cause has been that I am far from working on it all the
time unfortunately.
Currently I am looking at the filesystem hooks and hoping to send some
version of that patch soon.

When the hooks will be integrated,it is really up to plugin
implementor to design how thing wil happen.
There will be a hook that would be called after file from a package is
placed to filesystem, where
plugin can do many things, like setting MAC labels or setting IMA
signatures on a file.
The way signature will be stored in a package is also currently open,
there can be a number of options here.
You can define a special rpm header TAG and during package build
embeed all the informaiton about
signatures there together with the file name. This way plugin can
parse the header tag info, get all signatures info
and when the right hook is called, setup the IMA signature attribute.
But as I said, this is just one way of doing it
and may not be the best one.

Best Regards,
Elena.

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Kasatkin, Dmitry]

On Wed, Jan 16, 2013 at 5:54 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Wed, Jan 16, 2013 at 10:33:11AM -0500, Mimi Zohar wrote:
>
> [..]
>> > - Also I really could not figure out where does the private signing key
>> >   lives. I got the impression that we need to trust installer and
>> >   signing somehow happens at installation time. And we wanted signing
>> >   to happen at build server and could not trust installer for that.
>>
>> Dmitry's ima-evm-utils package signs files.  Depending on the options,
>> both the EVM and IMA extended attributes are created.
>
> I was going through following presentation.
>
> http://selinuxproject.org/~jmorris/lss2011_slides/IMA_EVM_Digital_Signature_Support.pdf
>
> On slide 8, it mentons signing.
>
>         evmctl sign --imahash /path/to/file
>         evmctl sign --imasig /path/to/file
>
> Can't figure out where does the key for signing come from? Is it already
> loaded in any of kernel keyrings.
>
> If yes, I think this is non-starter. One can not distribute the private
> key.
>
> Also I am assuming that this is done at installation time? If yes, then
> again it does not work as installer does not have private key.
>
> On slide 11, it talks about importing public keys in kernel keyring from
> initramfs. As we discussed this will need modification as these keys
> need to be signed and signing public key should already be part of
> kernel keyring.
>
> So looking at the signing process, it really does not look like that
> I can sign the executable at build server. It looks it needs to be
> signed by installer at install time and private key needs to be available
> to installer?
>

This is not like that.
There was never an idea to have a private key on the "target" system.
Because as you said it is wrong...
Signing can be on the build server.
Default keys are /etc/keys/{privkey,pubkey}_evm.pem

Non default keys can be passed as a last parameter in signing in old tools and
as a '--key' parameter in latest tools.

Latest source code includes X509 and asymmetric keys support and is
located here.

http://sourceforge.net/p/linux-ima/ima-evm-utils/


>>
>> >   My understanding of IMA could be wrong. So it would help if you
>> >   could list the exact steps about how to achieve the same goal using
>> >   IMA.
>>
>> http://linux-ima.sourceforge.net/  needs to be updated, but it describes
>> the integrity subsystem and includes a link to Dave Safford's original
>> whitepaper "An Overview of the Linux Integrity subsystem".
>
> I have gone through the paper in the past and still the quetions remain
> unanswered. So it will really help, if you could take a very simple
> example of hello-world executable and list the steps needed to be
> performed to sign and verify executable.

There is no problem in signing on the build server at all.
There is a different problem. Signature is stored in extended attribute.
The problem is how to 'transfer' signature to the target system.
It is necessary to add signatures to the RPM.

There is even a bug report in RedHat bugzilla

https://bugzilla.redhat.com/show_bug.cgi?id=771926

- Dmitry


>
> Thanks
> Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Kasatkin, Dmitry]

On Wed, Jan 16, 2013 at 8:21 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Wed, Jan 16, 2013 at 12:24:39PM -0500, Mimi Zohar wrote:
>> On Wed, 2013-01-16 at 10:54 -0500, Vivek Goyal wrote:
>> > On Wed, Jan 16, 2013 at 10:33:11AM -0500, Mimi Zohar wrote:
>> >
>> > [..]
>> > > > - Also I really could not figure out where does the private signing key
>> > > >   lives. I got the impression that we need to trust installer and
>> > > >   signing somehow happens at installation time. And we wanted signing
>> > > >   to happen at build server and could not trust installer for that.
>> > >
>> > > Dmitry's ima-evm-utils package signs files.  Depending on the options,
>> > > both the EVM and IMA extended attributes are created.
>> >
>> > I was going through following presentation.
>> >
>> > http://selinuxproject.org/~jmorris/lss2011_slides/IMA_EVM_Digital_Signature_Support.pdf
>> >
>> > On slide 8, it mentons signing.
>> >
>> >     evmctl sign --imahash /path/to/file
>> >     evmctl sign --imasig /path/to/file
>> >
>> > Can't figure out where does the key for signing come from? Is it already
>> > loaded in any of kernel keyrings.
>> >
>> > If yes, I think this is non-starter. One can not distribute the private
>> > key.
>>
>> No, the default key location is /etc/keys/privkey_evm.pem, but can be
>> specified.  Prior to Dmitry's updating the package yesterday, the last
>> parameter was the key pathname.  After the update, you can specify the
>> key location with the new --key option.
>>
>> > Also I am assuming that this is done at installation time? If yes, then
>> > again it does not work as installer does not have private key.
>>
>> Not necessarily.  The original use case scenario was creating an image
>> with both EVM and IMA digital signatures and then flashing the image.
>
> But distributions don't ship pre-setup root file system. It is created
> and labeled at installation time. So if we want to use IMA, we need to
> sign files at installation time. That means private keys need to be
> accessible to installer (using --key option). And that does not work for
> this use case.
>
>>

This wrong again.. Private should be used during installation time at all...


>> > On slide 11, it talks about importing public keys in kernel keyring from
>> > initramfs. As we discussed this will need modification as these keys
>> > need to be signed and signing public key should already be part of
>> > kernel keyring.
>>
>> It's been a long process upstreaming all the different pieces involved.
>> The initial design/step was to load the public key on a keyring.  Since
>> then we've added support for multiple keyrings(eg. EVM, IMA, etc).  The
>> next step is to tie this in with secure boot.
>>
>> > So looking at the signing process, it really does not look like that
>> > I can sign the executable at build server. It looks it needs to be
>> > signed by installer at install time and private key needs to be available
>> > to installer?
>>
>> No, the build server can sign the files, so the private key is not
>> required on the target.  These signatures need to be included in the
>> package.  Elena Reshetova gave a talk at the last LSS
>> (http://lwn.net/Articles/518265/), describing changes to RPM to write
>> the security extended attributes.
>
> Following is from article.
>
> "The other hooks that Reshetova proposed are associated with the individual
> files in a package. Those would allow things like security labeling or
> calculating hashes on the file contents (for integrity purposes)."
>
> IIUC, these hooks will execute on the target. So hash and all signatures
> should still be generated on target and not build server?
>
> Given the fact that signatures are stored in extended attributes, to me
> the only way to sign executables in current IMA framework would to be
> prepare file system image at build server and ship that image. And
> then installer simply mounts that image (after making sure that proper
> verification keys have been loaded in kernel).
>
> And this image will pretty much be static. As you don't have private key
> you can't install more packages and sign these.
>
> So it will be something like one image per signed package (in worst case).
>
> I don't think starting to ship signed file system images as compared
> to signed files is a better idea.
>
> Thanks
> Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Kasatkin, Dmitry]

On Wed, Jan 16, 2013 at 11:53 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Wed, Jan 16, 2013 at 02:24:50PM -0500, Mimi Zohar wrote:
> [..]
>> > > Sorry, this is out of scope for IMA.  Dmitry has looked into this, but
>> > > I'm not sure where it stands at the moment.
>> >
>> > Ok, so that's one reason that why I wrote these patcehs. IMA currently
>> > is not doing following things to make sure address space of signed images
>> > is not modified by others.
>> >
>> > - Protecting against modifications to pages on swap.
>> > - Protecting against modifications by ptrace.
>> > - Protecting against modifications which bypassed filesystem and directly
>> >   wrote to the block.
>> >
>> > Locking down all the pages of signed binaries in memory hopefully should
>> > solve above problems.
>>
>> Signing and verifying ELF executables goes back a long time ~2003/4,
>> from a number of esteemed kernel developers, including Greg-KH and Serge
>> Hallyn.
>>
>> IMA-appraisal isn't limited to appraising a single type of file, but is
>> a generic mechanism for appraising all files.  If there are issues that
>> aren't being addressed, then by all means, please help by addressing
>> them.  Duplicating a large portion of the code is not productive.
>
> So do you have ideas on how to address above mentioned issues. Do they
> fit into the realm of IMA/EVM or I just need to write separate code (which
> I have already done).
>
> With above issues, IMA stuff for executable files sounds incomplete.
>

swap is a last resort. healthy system uses swap minimally.
It is very easy to add /etc/crypttab which allows to have encrypted swap

# <target name> <source device>         <key file>      <options>
swap   /dev/sda6       /dev/urandom    swap

And it will eliminate plenty of possible attacks.
Processes have also RW data, modification of those will create huge
risk for the system...

But certain locking extensions like you implemented can be added to IMA as well.

It was said about ptrace already.


> - Protecting against modifications which bypassed filesystem and directly  wrote to the block.

Can you please tell a bit more how this patch protect against direct
writing to the blocks?

Thanks,
Dmitry

> Thanks
> Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Kasatkin, Dmitry]

On Thu, Jan 17, 2013 at 4:58 PM, Kasatkin, Dmitry
<dmitry.kasatkin@intel.com> wrote:
> On Wed, Jan 16, 2013 at 11:53 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> On Wed, Jan 16, 2013 at 02:24:50PM -0500, Mimi Zohar wrote:
>> [..]
>>> > > Sorry, this is out of scope for IMA.  Dmitry has looked into this, but
>>> > > I'm not sure where it stands at the moment.
>>> >
>>> > Ok, so that's one reason that why I wrote these patcehs. IMA currently
>>> > is not doing following things to make sure address space of signed images
>>> > is not modified by others.
>>> >
>>> > - Protecting against modifications to pages on swap.
>>> > - Protecting against modifications by ptrace.
>>> > - Protecting against modifications which bypassed filesystem and directly
>>> >   wrote to the block.
>>> >
>>> > Locking down all the pages of signed binaries in memory hopefully should
>>> > solve above problems.
>>>
>>> Signing and verifying ELF executables goes back a long time ~2003/4,
>>> from a number of esteemed kernel developers, including Greg-KH and Serge
>>> Hallyn.
>>>
>>> IMA-appraisal isn't limited to appraising a single type of file, but is
>>> a generic mechanism for appraising all files.  If there are issues that
>>> aren't being addressed, then by all means, please help by addressing
>>> them.  Duplicating a large portion of the code is not productive.
>>
>> So do you have ideas on how to address above mentioned issues. Do they
>> fit into the realm of IMA/EVM or I just need to write separate code (which
>> I have already done).
>>
>> With above issues, IMA stuff for executable files sounds incomplete.
>>
>
> swap is a last resort. healthy system uses swap minimally.
> It is very easy to add /etc/crypttab which allows to have encrypted swap
>
> # <target name> <source device>         <key file>      <options>
> swap   /dev/sda6       /dev/urandom    swap
>
> And it will eliminate plenty of possible attacks.
> Processes have also RW data, modification of those will create huge
> risk for the system...
>
> But certain locking extensions like you implemented can be added to IMA as well.
>
> It was said about ptrace already.
>
>
>> - Protecting against modifications which bypassed filesystem and directly  wrote to the block.
>
> Can you please tell a bit more how this patch protect against direct
> writing to the blocks?
>
> Thanks,
> Dmitry
>
>> Thanks
>> Vivek

One important thing to mention.
Protecting ELF-only does not help too much in protecting the system.
There are plenty of init, upstart and systemd scripts which must be
verified as well. IMA does it.

- Dmitry

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Thu, Jan 17, 2013 at 04:58:02PM +0200, Kasatkin, Dmitry wrote:
> On Wed, Jan 16, 2013 at 11:53 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Wed, Jan 16, 2013 at 02:24:50PM -0500, Mimi Zohar wrote:
> > [..]
> >> > > Sorry, this is out of scope for IMA.  Dmitry has looked into this, but
> >> > > I'm not sure where it stands at the moment.
> >> >
> >> > Ok, so that's one reason that why I wrote these patcehs. IMA currently
> >> > is not doing following things to make sure address space of signed images
> >> > is not modified by others.
> >> >
> >> > - Protecting against modifications to pages on swap.
> >> > - Protecting against modifications by ptrace.
> >> > - Protecting against modifications which bypassed filesystem and directly
> >> >   wrote to the block.
> >> >
> >> > Locking down all the pages of signed binaries in memory hopefully should
> >> > solve above problems.
> >>
> >> Signing and verifying ELF executables goes back a long time ~2003/4,
> >> from a number of esteemed kernel developers, including Greg-KH and Serge
> >> Hallyn.
> >>
> >> IMA-appraisal isn't limited to appraising a single type of file, but is
> >> a generic mechanism for appraising all files.  If there are issues that
> >> aren't being addressed, then by all means, please help by addressing
> >> them.  Duplicating a large portion of the code is not productive.
> >
> > So do you have ideas on how to address above mentioned issues. Do they
> > fit into the realm of IMA/EVM or I just need to write separate code (which
> > I have already done).
> >
> > With above issues, IMA stuff for executable files sounds incomplete.
> >
> 
> swap is a last resort. healthy system uses swap minimally.
> It is very easy to add /etc/crypttab which allows to have encrypted swap
> 
> # <target name> <source device>         <key file>      <options>
> swap   /dev/sda6       /dev/urandom    swap
> 
> And it will eliminate plenty of possible attacks.
> Processes have also RW data, modification of those will create huge
> risk for the system...

Hm..., encrypted swap is interesting. It might have overhead too. It is
one way of doing thing, but I think it would be better if we don't rely
on user has setup encrypted swap and provide a way to lock down memory of
signed executables.

> 
> But certain locking extensions like you implemented can be added to IMA as well.

Cool. That would help. I am very new to IMA. If you have some ideas on
how to go about it, I can start looking into implementing it.

> 
> It was said about ptrace already.

Sorry, did not get what was said about ptrace already? We need to disable
ptrace for signed executables. So can IMA set a process flag upon
signature verification and exec() code can give additional capability. And
as Eric mentioned that should automatically take care of ptrace issue.

> 
> > - Protecting against modifications which bypassed filesystem and directly  wrote to the block.
> 
> Can you please tell a bit more how this patch protect against direct
> writing to the blocks?

If you have loaded all the pages from disk and locked them in memory and
verified the signature, then even if somebody modifies a block on disk
it does not matter. We will not read pages from disk anymore for this
exec(). We verified the signature of executable loaded in memory and
in-memory copy is intact.

So next time somebody tries to execute same binary it will fail (because
of modified block).

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Thu, Jan 17, 2013 at 05:06:09PM +0200, Kasatkin, Dmitry wrote:

[..]
> One important thing to mention.
> Protecting ELF-only does not help too much in protecting the system.
> There are plenty of init, upstart and systemd scripts which must be
> verified as well. IMA does it.

Actually that would be a different requirement altogether. I am not
trying to verify all the processes started by root. I am just trying
to sign and verify signature of select user process and if signature
are verified, kernel grants those processes extra capability and allow
calling sys_kexec() when secureboot is enabled.

So for my use case, I don't care if there are so many other unsigned
processes running in the system.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Tue, 2013-01-15 at 16:34 -0500, Vivek Goyal wrote:
> If a binary is signed, verify its signature. If signature is not valid, do
> not allow execution. If binary is not signed, execution is allowed
> unconditionally.
> 
> CONFIG_BINFMT_ELF_SIGNATURE controls whether elf binary signature support
> is compiled in or not.
> 
> Signature are expected to be present in elf section ".section". This code
> is written along the lines of module signature verification code. Just
> that I have removed the magic string. It is not needed as signature is
> expected to be present in a specific section.

Right, it's written along the lines of the original module signature
verification code, where the signature was in the ELF header, not the
version that was upstreamed, where the signature was appended.

> I put the signature into a section, instead of appending it so that
> strip operation works fine.

Wouldn't the original reasons for not embedding the signature in the ELF
header for modules apply here too?  

> One signs and verifies  all the areas mapped by PT_LOAD segments of elf
> binary. Typically Elf header is mapped in first PT_LOAD segment. As adding
> .signature section can change three elf header fields (e_shoff, e_shnum
> and e_shstrndx), these fields are excluded from digest calculation
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

At this point, why would you want yet another method for signing files?

Mimi

> ---
>  fs/Kconfig.binfmt |    7 +
>  fs/binfmt_elf.c   |  455 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 462 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt
> index 0efd152..93eb90d 100644
> --- a/fs/Kconfig.binfmt
> +++ b/fs/Kconfig.binfmt
> @@ -23,6 +23,13 @@ config BINFMT_ELF
>  	  ld.so (check the file <file:Documentation/Changes> for location and
>  	  latest version).
> 
> +config BINFMT_ELF_SIGNATURE
> +	bool "Kernel support for ELF binaries signature verification"
> +	depends on BINFMT_ELF && MODULE_SIG
> +	default n
> +	---help---
> +	  ELF binary signature verification support.
> +
>  config COMPAT_BINFMT_ELF
>  	bool
>  	depends on COMPAT && BINFMT_ELF
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 0c42cdb..80da13c 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -33,6 +33,10 @@
>  #include <linux/elf.h>
>  #include <linux/utsname.h>
>  #include <linux/coredump.h>
> +#include <linux/module.h>
> +#include <crypto/public_key.h>
> +#include <crypto/hash.h>
> +#include <keys/asymmetric-type.h>
>  #include <asm/uaccess.h>
>  #include <asm/param.h>
>  #include <asm/page.h>
> @@ -44,6 +48,27 @@
>  #define user_siginfo_t siginfo_t
>  #endif
> 
> +struct elf_sig_info {
> +	u8      algo;		/* Public-key crypto algorithm [enum pkey_algo] */
> +	u8	hash;		/* Digest algorithm [enum pkey_hash_algo] */
> +	u8	id_type;	/* Key identifier type [enum pkey_id_type] */
> +	u8	signer_len;	/* Length of signer's name */
> +	u8	key_id_len;	/* Length of key identifier */
> +	u8	__pad[3];
> +	__be32	sig_len;	/* Length of signature data */
> +};
> +
> +struct elf_sig_data {
> +	enum pkey_hash_algo hash;
> +	char *sig;
> +	unsigned int sig_len;
> +	struct key *key;
> +	struct shash_desc *desc;
> +	char *digest;
> +	unsigned int digest_sz;
> +};
> +
> +
>  static int load_elf_binary(struct linux_binprm *bprm);
>  static int load_elf_library(struct file *);
>  static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
> @@ -558,6 +583,400 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
>  #endif
>  }
> 
> +#ifdef CONFIG_BINFMT_ELF_SIGNATURE
> +/* Elf Signature Verification related stuff */
> +static int esd_shash_init(struct elf_sig_data *esd)
> +{
> +	struct shash_desc *desc;
> +	struct crypto_shash *tfm;
> +	size_t digest_size, desc_size;
> +	char *digest;
> +	int ret;
> +
> +	tfm = crypto_alloc_shash(pkey_hash_algo[esd->hash], 0, 0);
> +	if (IS_ERR(tfm)) {
> +		ret = PTR_ERR(tfm);
> +		return (ret == -ENOENT ? -ENOPKG : ret);
> +	}
> +
> +	desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> +	digest_size = crypto_shash_digestsize(tfm);
> +
> +	desc = kzalloc(desc_size, GFP_KERNEL);
> +	if (!desc) {
> +		ret = -ENOMEM;
> +		goto out_free_tfm;
> +	}
> +
> +	digest = kzalloc(digest_size, GFP_KERNEL);
> +	if (!digest) {
> +		ret = -ENOMEM;
> +		goto out_free_desc;
> +	}
> +
> +	desc->tfm   = tfm;
> +	desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> +
> +	ret = crypto_shash_init(desc);
> +	if (ret < 0)
> +		goto out_free_digest;
> +
> +	esd->desc = desc;
> +	esd->digest = digest;
> +	esd->digest_sz = digest_size;
> +	return 0;
> +
> +out_free_digest:
> +	kfree(digest);
> +out_free_desc:
> +	kfree(desc);
> +out_free_tfm:
> +	crypto_free_shash(tfm);
> +	return ret;
> +}
> +
> +static struct elf_sig_data *
> +elf_parse_binary_signature(struct elfhdr *ehdr, struct file *file)
> +{
> +	loff_t rem_file_sz, file_sz;
> +	loff_t offset;
> +	struct elf_sig_data *esd;
> +	struct elf_sig_info *esi;
> +	int retval, i;
> +	size_t sig_len;
> +	struct key *key;
> +	struct elf_shdr *elf_shtable, *elf_spnt, *elf_shstrpnt;
> +	unsigned int sig_info_sz, shtable_sz;
> +	uint16_t shstrndx;
> +	bool found_sig_section = false;
> +	void *signer_name, *key_id;
> +
> +	if (!ehdr->e_shnum)
> +		return NULL;
> +
> +	if (ehdr->e_shstrndx == SHN_UNDEF)
> +		return NULL;
> +
> +	/* Read in elf section table */
> +	file_sz = i_size_read(file->f_path.dentry->d_inode);
> +	shtable_sz = ehdr->e_shnum * sizeof(struct elf_shdr);
> +	elf_shtable = kmalloc(shtable_sz, GFP_KERNEL);
> +	if (!elf_shtable)
> +		return ERR_PTR(-ENOMEM);
> +
> +	retval = kernel_read(file, ehdr->e_shoff, (char *)elf_shtable,
> +					shtable_sz);
> +	if (retval != shtable_sz) {
> +		if (retval >= 0)
> +			retval = -EIO;
> +		goto out_free_shtable;
> +	}
> +
> +	if (ehdr->e_shstrndx == 0xffff)
> +		shstrndx = elf_shtable[0].sh_link;
> +	else
> +		shstrndx = ehdr->e_shstrndx;
> +
> +	if (shstrndx >= ehdr->e_shnum) {
> +		retval = -EINVAL;
> +		goto out_free_shtable;
> +	}
> +
> +	elf_shstrpnt = elf_shtable + shstrndx;
> +	elf_spnt = elf_shtable;
> +
> +	/* Scan for section with name ".signature" */
> +	for (i = 0; i < ehdr->e_shnum; i++) {
> +		char sec_name[11];
> +		offset = elf_shstrpnt->sh_offset + elf_spnt->sh_name;
> +		retval = kernel_read(file, offset, sec_name, 11);
> +		if (retval != 11) {
> +			if (retval >= 0)
> +				retval = -EIO;
> +			goto out_free_shtable;
> +		}
> +
> +		if(!strcmp(sec_name, ".signature")) {
> +			found_sig_section = true;
> +			break;
> +		}
> +		elf_spnt++;
> +	}
> +
> +	if (!found_sig_section) {
> +		/* File is not signed */
> +		retval = 0;
> +		goto out_free_shtable;
> +	}
> +
> +	esi = kzalloc(sizeof(struct elf_sig_info), GFP_KERNEL);
> +	if (!esi) {
> +		retval = -ENOMEM;
> +		goto out_free_shtable;
> +	}
> +
> +	esd = kzalloc(sizeof(struct elf_sig_data), GFP_KERNEL);
> +	if (!esd) {
> +		retval = -ENOMEM;
> +		goto out_free_esi;
> +	}
> +
> +	/* Read in sig info */
> +	sig_info_sz = sizeof(struct elf_sig_info);
> +
> +	offset = elf_spnt->sh_offset + elf_spnt->sh_size - sig_info_sz;
> +	rem_file_sz = file_sz - sig_info_sz;
> +	retval = kernel_read(file, offset, (char *)esi, sig_info_sz);
> +	if (retval != sig_info_sz) {
> +		if (retval >= 0)
> +			retval = -EIO;
> +		goto out_free_esd;
> +	}
> +
> +	sig_len = be32_to_cpu(esi->sig_len);
> +	if (sig_len >= rem_file_sz) {
> +		retval = -EBADMSG;
> +		goto out_free_esd;
> +	}
> +	rem_file_sz -= sig_len;
> +
> +	if ((size_t)esi->signer_len + esi->key_id_len >= rem_file_sz) {
> +		retval = -EBADMSG;
> +		goto out_free_esd;
> +	}
> +
> +	rem_file_sz -= ((size_t)esi->signer_len + esi->key_id_len);
> +
> +	/* For the moment, only support RSA and X.509 identifiers */
> +	if (esi->algo != PKEY_ALGO_RSA || esi->id_type != PKEY_ID_X509) {
> +		retval = -ENOPKG;
> +		goto out_free_esd;
> +	}
> +
> +	if (esi->hash >= PKEY_HASH__LAST || !pkey_hash_algo[esi->hash]) {
> +		retval = -ENOPKG;
> +		goto out_free_esd;
> +	}
> +
> +	esd->hash = esi->hash;
> +
> +	/* Read in signature */
> +	esd->sig = kzalloc(sig_len, GFP_KERNEL);
> +	if (!esd->sig) {
> +		retval = -ENOMEM;
> +		goto out_free_esd;
> +	}
> +	esd->sig_len = sig_len;
> +
> +	offset = offset - sig_len;
> +	retval = kernel_read(file, offset, esd->sig, sig_len);
> +	if (retval != sig_len) {
> +		if (retval >= 0)
> +			retval = -EIO;
> +		goto out_free_esd_sig;
> +	}
> +
> +	/* Read in skid */
> +	key_id = kzalloc(esi->key_id_len, GFP_KERNEL);
> +	if (!key_id) {
> +		retval = -ENOMEM;
> +		goto out_free_esd_sig;
> +	}
> +
> +	offset = offset - esi->key_id_len;
> +	retval = kernel_read(file, offset, key_id, esi->key_id_len);
> +	if (retval != esi->key_id_len) {
> +		if (retval >= 0)
> +			retval = -EIO;
> +		goto out_free_key_id;
> +	}
> +
> +	/* Read in signer_name */
> +	signer_name = kzalloc(esi->signer_len, GFP_KERNEL);
> +	if (!signer_name) {
> +		retval = -ENOMEM;
> +		goto out_free_key_id;
> +	}
> +
> +	offset = offset - esi->signer_len;
> +	retval = kernel_read(file, offset, signer_name, esi->signer_len);
> +	if (retval != esi->signer_len) {
> +		if (retval >= 0)
> +			retval = -EIO;
> +		goto out_free_signer_name;
> +	}
> +
> +	/* search for key */
> +	key = request_asymmetric_key(signer_name, esi->signer_len,
> +				     key_id, esi->key_id_len);
> +	if (IS_ERR(key)) {
> +		retval = PTR_ERR(key);
> +		goto out_free_signer_name;
> +	}
> +	esd->key = key;
> +
> +	retval = esd_shash_init(esd);
> +	if (retval < 0)
> +		goto out_put_key;
> +
> +	kfree(elf_shtable);
> +	kfree(signer_name);
> +	kfree(key_id);
> +	kfree(esi);
> +	return esd;
> +
> +out_put_key:
> +	key_put(key);
> +out_free_signer_name:
> +	kfree(signer_name);
> +out_free_key_id:
> +	kfree(key_id);
> +out_free_esd_sig:
> +	kfree(esd->sig);
> +out_free_esd:
> +	kfree(esd);
> +out_free_esi:
> +	kfree(esi);
> +out_free_shtable:
> +	kfree(elf_shtable);
> +	return ERR_PTR(retval);
> +}
> +
> +static void free_elf_sig_data(struct elf_sig_data *esd)
> +{
> +	if (!esd)
> +		return;
> +
> +	if (esd->sig)
> +		kfree(esd->sig);
> +
> +	if (esd->key)
> +		key_put(esd->key);
> +
> +	if (esd->desc && esd->desc->tfm)
> +		crypto_free_shash(esd->desc->tfm);
> +
> +	if (esd->desc)
> +		kfree(esd->desc);
> +
> +	if (esd->digest)
> +		kfree(esd->digest);
> +}
> +
> +static void elf_digest_first_phdr(struct elfhdr *elfhdr,
> +		struct elf_phdr *elf_ppnt, struct elf_sig_data *esd,
> +		unsigned long map_addr)
> +{
> +	unsigned int off_e_shoff = offsetof(struct elfhdr, e_shoff);
> +	unsigned int off_e_flags = offsetof(struct elfhdr, e_flags);
> +	unsigned int off_e_shnum = offsetof(struct elfhdr, e_shnum);
> +
> +	/*
> +	 * If elf header is mapped in first segment, execlude e_shoff, e_shnum
> +	 * and e_shstrndx from digest calculation as this can change when
> +	 * signature section is added or executable is stripped after
> +	 * signing.
> +	 */
> +
> +	if (!elf_ppnt->p_offset) {
> +		/* ELF header is mapped into first PT_LOAD segment */
> +		unsigned long sz = off_e_shoff;
> +
> +		crypto_shash_update(esd->desc, (u8*)map_addr, sz);
> +
> +		/* Digest e_flags to e_shentsize */
> +		sz = off_e_shnum - off_e_flags;
> +		crypto_shash_update(esd->desc, (u8*)map_addr + off_e_flags, sz);
> +
> +		/* Digest rest of the segment */
> +		crypto_shash_update(esd->desc, (u8*)map_addr + elfhdr->e_ehsize,
> +				elf_ppnt->p_filesz - elfhdr->e_ehsize);
> +	} else
> +		/* Digest full segment */
> +		crypto_shash_update(esd->desc, (u8*)map_addr,
> +					elf_ppnt->p_filesz);
> +}
> +
> +static void elf_digest_phdr(struct elfhdr *ehdr, struct elf_phdr *phdr,
> +			struct elf_sig_data *esd, unsigned long map_addr,
> +			bool first_phdr)
> +{
> +	/*
> +	 * If phdr->p_vaddr is not aligned, then elf_map() will map
> +	 * at aligned address. Take that into account
> +	 */
> +	map_addr = map_addr + ELF_PAGEOFFSET(phdr->p_vaddr);
> +
> +	if (first_phdr)
> +		elf_digest_first_phdr(ehdr, phdr, esd, map_addr);
> +	else
> +		crypto_shash_update(esd->desc, (u8*)map_addr, phdr->p_filesz);
> +}
> +
> +static int elf_verify_signature(struct elf_sig_data *esd)
> +{
> +	struct public_key_signature *pks;
> +	int retval;
> +
> +	pks = kzalloc(sizeof(*pks), GFP_KERNEL);
> +	if (!pks)
> +		return -ENOMEM;
> +
> +	pks->pkey_hash_algo = esd->hash;
> +	pks->digest = (u8*)esd->digest;
> +	pks->digest_size = esd->digest_sz;
> +
> +	retval = mod_extract_mpi_array(pks, esd->sig, esd->sig_len);
> +	if (retval < 0)
> +		goto error_free_pks;
> +
> +	retval = verify_signature(esd->key, pks);
> +	mpi_free(pks->rsa.s);
> +error_free_pks:
> +	kfree(pks);
> +	return retval;
> +}
> +
> +static int elf_finalize_digest_verify_signature(struct elf_sig_data *esd)
> +{
> +	int retval;
> +
> +	retval = crypto_shash_final(esd->desc, (u8*)esd->digest);
> +	if (retval < 0)
> +		return retval;
> +
> +	retval = elf_verify_signature(esd);
> +	if (retval < 0)
> +		return retval;
> +
> +	return 0;
> +}
> +
> +#else /* CONFIG_BINFMT_ELF_SIGNATURE */
> +static inline struct elf_sig_data *
> +elf_parse_binary_signature(struct elfhdr *ehdr, struct file *file)
> +{
> +	return NULL;
> +}
> +
> +static inline void free_elf_sig_data(struct elf_sig_data *esd) {}
> +
> +static inline void elf_digest_phdr(struct elfhdr *ehdr, struct elf_phdr *phdr,
> +			struct elf_sig_data *esd, unsigned long map_addr,
> +			bool first_phdr) {}
> +
> +static inline int elf_verify_signature(struct elf_sig_data *esd)
> +{
> +	return 0;
> +}
> +
> +static inline int elf_finalize_digest_verify_signature(struct elf_sig_data *esd)
> +{
> +	return 0;
> +}
> +
> +#endif /* CONFIG_BINFMT_ELF_SIGNATURE */
> +
>  static int load_elf_binary(struct linux_binprm *bprm)
>  {
>  	struct file *interpreter = NULL; /* to shut gcc up */
> @@ -575,6 +994,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
>  	unsigned long reloc_func_desc __maybe_unused = 0;
>  	int executable_stack = EXSTACK_DEFAULT;
>  	unsigned long def_flags = 0;
> +	struct elf_sig_data *esd = NULL;
> +	bool first_signed_phdr = true;
>  	struct pt_regs *regs = current_pt_regs();
>  	struct {
>  		struct elfhdr elf_ex;
> @@ -741,6 +1162,14 @@ static int load_elf_binary(struct linux_binprm *bprm)
>  	
>  	current->mm->start_stack = bprm->p;
> 
> +	esd = elf_parse_binary_signature(&loc->elf_ex, bprm->file);
> +	if (IS_ERR(esd)) {
> +		retval = PTR_ERR(esd);
> +		send_sig(SIGKILL, current, 0);
> +		esd = NULL;
> +		goto out_free_dentry;
> +	}
> +
>  	/* Now we do a little grungy work by mmapping the ELF image into
>  	   the correct location in memory. */
>  	for(i = 0, elf_ppnt = elf_phdata;
> @@ -788,6 +1217,14 @@ static int load_elf_binary(struct linux_binprm *bprm)
> 
>  		elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE;
> 
> +		/*
> +		 * mlock the pages of signed binary. We don't want these
> +		 * to be swapped out and be potentially modifed, effectively
> +		 * bypassing signature verification.
> +		 */
> +		if (esd)
> +			elf_flags = elf_flags | MAP_LOCKED;
> +
>  		vaddr = elf_ppnt->p_vaddr;
>  		if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) {
>  			elf_flags |= MAP_FIXED;
> @@ -831,6 +1268,14 @@ static int load_elf_binary(struct linux_binprm *bprm)
>  				reloc_func_desc = load_bias;
>  			}
>  		}
> +
> +		/* Calculate digest of PT_LOAD segments */
> +		if (esd) {
> +			elf_digest_phdr(&loc->elf_ex, elf_ppnt, esd, error,
> +					first_signed_phdr);
> +			first_signed_phdr = false;
> +		}
> +
>  		k = elf_ppnt->p_vaddr;
>  		if (k < start_code)
>  			start_code = k;
> @@ -864,6 +1309,15 @@ static int load_elf_binary(struct linux_binprm *bprm)
>  			elf_brk = k;
>  	}
> 
> +	/* Finalize digest and do signature verification */
> +	if (esd) {
> +		retval = elf_finalize_digest_verify_signature(esd);
> +		if (retval < 0) {
> +			send_sig(SIGKILL, current, 0);
> +			goto out_free_dentry;
> +		}
> +	}
> +
>  	loc->elf_ex.e_entry += load_bias;
>  	elf_bss += load_bias;
>  	elf_brk += load_bias;
> @@ -985,6 +1439,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
>  	start_thread(regs, elf_entry, bprm->p);
>  	retval = 0;
>  out:
> +	free_elf_sig_data(esd);
>  	kfree(loc);
>  out_ret:
>  	return retval;

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Thu, Jan 17, 2013 at 10:37:01AM -0500, Mimi Zohar wrote:
> On Tue, 2013-01-15 at 16:34 -0500, Vivek Goyal wrote:
> > If a binary is signed, verify its signature. If signature is not valid, do
> > not allow execution. If binary is not signed, execution is allowed
> > unconditionally.
> > 
> > CONFIG_BINFMT_ELF_SIGNATURE controls whether elf binary signature support
> > is compiled in or not.
> > 
> > Signature are expected to be present in elf section ".section". This code
> > is written along the lines of module signature verification code. Just
> > that I have removed the magic string. It is not needed as signature is
> > expected to be present in a specific section.
> 
> Right, it's written along the lines of the original module signature
> verification code, where the signature was in the ELF header, not the
> version that was upstreamed, where the signature was appended.
> 
> > I put the signature into a section, instead of appending it so that
> > strip operation works fine.
> 
> Wouldn't the original reasons for not embedding the signature in the ELF
> header for modules apply here too?  

I think rusty wanted to append signatures. He thought it is much easier.
Adding a .signature section makes life easier for user space tools. One
can strip files even after signing.

Not that I am married to the idea of putting signature in a section. Just
that it sounded reasonable enough to do for an RFC. So if appending 
signature proves to be better, it is easy to implement that.
> 
> > One signs and verifies  all the areas mapped by PT_LOAD segments of elf
> > binary. Typically Elf header is mapped in first PT_LOAD segment. As adding
> > .signature section can change three elf header fields (e_shoff, e_shnum
> > and e_shstrndx), these fields are excluded from digest calculation
> > 
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> 
> At this point, why would you want yet another method for signing files?

Are you saying that append signature instead of putting them in a section
or are you saying that just use IMA.

- For the first, I am fine with appending too if that works better. So
  what's wrong with current implementation. Just because we append the
  signatures in case of modules, we should follow the same thing for
  executables too?

- If above comment is w.r.t use of IMA, then I have no issues in using
  IMA as long as it can meet all the requirements. Looks like there is
  a long TODO list before we get there. In fact for some things its not
  even clear whether they fit in IMA or somehwere else.

  - Make sure IMA/EVM stuff chains into secureboot chain of trust.

  - Sort out all the memory locking related issues I mentioned in
    other mail. You seemed to be of opinion that it is out of scope
    for IMA, but I think it probably is nice extenstion.

    Or somehow a way needs to be found to make sure nobody can modify
    process address space without processe's knowledge.

  - Once all this works, then one needs to figure out all the RPM stuff
    and plugins to make sure files can be singed on build server and
    installed properly on target system.

Thanks
Vivek

Re: [PATCH 0/3] ELF executable signing and verification

[Kasatkin, Dmitry]

On Tue, Jan 15, 2013 at 11:34 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> Hi,
>
> This is a very crude RFC for ELF executable signing and verification. This
> has been done along the lines of module signature verification.
>
> Why do we need it
> =================
> With arrival of secureboot, sys_kexec() is deemed dangerous. One can
> effectively bypass the secureboot feature and run its own kernel. So
> matthew garret proposed disabling sys_kexec() in secureboot mode.
>
> https://lkml.org/lkml/2012/9/4/225
>
> Later in a separate thread it was discussed how to handle the issue
> of sys_kexec() with secureboot.
>
> https://lkml.org/lkml/2012/10/24/451
>
> My takeaway from discussion was that we need to sign /sbin/kexec. Signed
> executable can get extra capability and we can allow/disallow access to
> sys_kexec() based on that capability (Thanks to Eric Biederman for the
> idea).
>
> So that's my motivation to make user space signing work so that I can
> get kdump working with secureboot enabled. There might be other people
> who might find it useful in general.
>
> What does it do
> ===============
> I have written a utility "signelf" which can take a private key and
> an x509 certificate and sign an ELF executable. This is very much done
> along the lines of module signing. There are two major differences.
> Signature are put in a section ".signature" instead of being appended
> to executable. And we calculate digest of only PT_LOAD segments and not
> the whole executable file.
>
> Upon exec(), we determine if executable is signed. If it is, then locks
> down the pages in memory (using MAP_LOCKED) and verfies the signature.
> If signature does not match, process is killed. Unsigned processes
> don't get affected at all.
>
> Currently it is expected to use these patches only for statically linked
> executables. No dynamic linking. In fact patches specifically disable
> calling interpreter. This does not prevent against somebody using dlopen()
> sutff. So don't sign binaries which do that.

How dynamic linking and interpreter are related together?

This is rather policy than enforcement.
Protection works only for statically linked binaries, because dynamic
libraries are not verified.

- Dmitry

>
> HOWTO
> =====
> Currently module signing keys are automatically loaded in module keyring
> so it is easiest to sign executable using the keys generated for module
> signing.
>
> - Compile and boot into kernel with following options enabled.
>         - CONFIG_MODULE_SIG=y
>         - CONFIG_BINFMT_ELF_SIGNATURE=y
>         - CONFIG_CRYPTO_SHA256=y
>
> - Compile "signelf" utility (Attached in a patch)
>
> - Install glibc-static
>
> - Compile a test program (say hello-world.c). Link statically with glibc
>         gcc hello-world.c -o hello-world -static
>
> - Sign hello_world using keys generated during kernel build.
>         signelf -i hello-world -o hello-world.signed -p linux-2.6/signing_key.priv -c linux-2.6/signing_key.x509
>
> - Run signed executable
>         ./hello-world.signed
>
> This should run successfully. Now one can generate another pair of keys
> and certificate and sign same binary using new keys. This new binary should
> fail to execute as corresponding keys are not loaded in kernel.
>
>         openssl req -new -nodes -utf8 -sha256 -days 36500 -batch -x509 -config linux-2.6/x509.genkey -outform DER -out new_signing_key.x509 -keyout new_signing_key.priv
>
>         signelf -i hello-world -o hello-world.signed.new -p new_signing_key.priv -c new_signing_key.x509
>
> - Run this signed executable
>         ./hello-world.signed.new
>         Killed
>
> TODO
> ====
> - kexec related patches are yet to be done.
> - Disable ptrace to signed processes so that one can not modify code/data
>   of signed process.
> - Sort out issues related to how key used for user space signing is loaded
>   in kernel keyring.
> - Sort out issues related to sharing keyring with modules.
>
> Thanks
> Vivek
> Vivek Goyal (3):
>   module: export couple of functions for use in process signature
>     verification
>   binfmt_elf: Verify signature of signed elf binary
>   binfmt_elf: Do not allow exec() if signed binary has intepreter
>
>  fs/Kconfig.binfmt       |    7 +
>  fs/binfmt_elf.c         |  465 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/module.h  |    8 +
>  kernel/module_signing.c |    4 +-
>  4 files changed, 482 insertions(+), 2 deletions(-)
>
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Kasatkin, Dmitry]

On Thu, Jan 17, 2013 at 5:18 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Thu, Jan 17, 2013 at 04:58:02PM +0200, Kasatkin, Dmitry wrote:
>> On Wed, Jan 16, 2013 at 11:53 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> > On Wed, Jan 16, 2013 at 02:24:50PM -0500, Mimi Zohar wrote:
>> > [..]
>> >> > > Sorry, this is out of scope for IMA.  Dmitry has looked into this, but
>> >> > > I'm not sure where it stands at the moment.
>> >> >
>> >> > Ok, so that's one reason that why I wrote these patcehs. IMA currently
>> >> > is not doing following things to make sure address space of signed images
>> >> > is not modified by others.
>> >> >
>> >> > - Protecting against modifications to pages on swap.
>> >> > - Protecting against modifications by ptrace.
>> >> > - Protecting against modifications which bypassed filesystem and directly
>> >> >   wrote to the block.
>> >> >
>> >> > Locking down all the pages of signed binaries in memory hopefully should
>> >> > solve above problems.
>> >>
>> >> Signing and verifying ELF executables goes back a long time ~2003/4,
>> >> from a number of esteemed kernel developers, including Greg-KH and Serge
>> >> Hallyn.
>> >>
>> >> IMA-appraisal isn't limited to appraising a single type of file, but is
>> >> a generic mechanism for appraising all files.  If there are issues that
>> >> aren't being addressed, then by all means, please help by addressing
>> >> them.  Duplicating a large portion of the code is not productive.
>> >
>> > So do you have ideas on how to address above mentioned issues. Do they
>> > fit into the realm of IMA/EVM or I just need to write separate code (which
>> > I have already done).
>> >
>> > With above issues, IMA stuff for executable files sounds incomplete.
>> >
>>
>> swap is a last resort. healthy system uses swap minimally.
>> It is very easy to add /etc/crypttab which allows to have encrypted swap
>>
>> # <target name> <source device>         <key file>      <options>
>> swap   /dev/sda6       /dev/urandom    swap
>>
>> And it will eliminate plenty of possible attacks.
>> Processes have also RW data, modification of those will create huge
>> risk for the system...
>
> Hm..., encrypted swap is interesting. It might have overhead too. It is
> one way of doing thing, but I think it would be better if we don't rely
> on user has setup encrypted swap and provide a way to lock down memory of
> signed executables.
>
>>
>> But certain locking extensions like you implemented can be added to IMA as well.
>
> Cool. That would help. I am very new to IMA. If you have some ideas on
> how to go about it, I can start looking into implementing it.
>
>>
>> It was said about ptrace already.
>
> Sorry, did not get what was said about ptrace already? We need to disable
> ptrace for signed executables. So can IMA set a process flag upon
> signature verification and exec() code can give additional capability. And
> as Eric mentioned that should automatically take care of ptrace issue.
>
>>
>> > - Protecting against modifications which bypassed filesystem and directly  wrote to the block.
>>
>> Can you please tell a bit more how this patch protect against direct
>> writing to the blocks?
>
> If you have loaded all the pages from disk and locked them in memory and
> verified the signature, then even if somebody modifies a block on disk
> it does not matter. We will not read pages from disk anymore for this
> exec(). We verified the signature of executable loaded in memory and
> in-memory copy is intact.
>
> So next time somebody tries to execute same binary it will fail (because
> of modified block).
>

Ok. This is about the same as with swap.

It should be a 10 lines patch to IMA to lock down the memory.
It may be controlled by the policy.
IMA might also set new capability if needed.

- Dmitry


> Thanks
> Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Thu, 2013-01-17 at 10:51 -0500, Vivek Goyal wrote:
> On Thu, Jan 17, 2013 at 10:37:01AM -0500, Mimi Zohar wrote:
> > On Tue, 2013-01-15 at 16:34 -0500, Vivek Goyal wrote:
> > > If a binary is signed, verify its signature. If signature is not valid, do
> > > not allow execution. If binary is not signed, execution is allowed
> > > unconditionally.
> > > 
> > > CONFIG_BINFMT_ELF_SIGNATURE controls whether elf binary signature support
> > > is compiled in or not.
> > > 
> > > Signature are expected to be present in elf section ".section". This code
> > > is written along the lines of module signature verification code. Just
> > > that I have removed the magic string. It is not needed as signature is
> > > expected to be present in a specific section.
> > 
> > Right, it's written along the lines of the original module signature
> > verification code, where the signature was in the ELF header, not the
> > version that was upstreamed, where the signature was appended.
> > 
> > > I put the signature into a section, instead of appending it so that
> > > strip operation works fine.
> > 
> > Wouldn't the original reasons for not embedding the signature in the ELF
> > header for modules apply here too?  
> 
> I think rusty wanted to append signatures. He thought it is much easier.
> Adding a .signature section makes life easier for user space tools. One
> can strip files even after signing.
> 
> Not that I am married to the idea of putting signature in a section. Just
> that it sounded reasonable enough to do for an RFC. So if appending 
> signature proves to be better, it is easy to implement that.
> > 
> > > One signs and verifies  all the areas mapped by PT_LOAD segments of elf
> > > binary. Typically Elf header is mapped in first PT_LOAD segment. As adding
> > > .signature section can change three elf header fields (e_shoff, e_shnum
> > > and e_shstrndx), these fields are excluded from digest calculation
> > > 
> > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> > 
> > At this point, why would you want yet another method for signing files?
> 
> Are you saying that append signature instead of putting them in a section
> or are you saying that just use IMA.
> 
> - For the first, I am fine with appending too if that works better. So
>   what's wrong with current implementation. Just because we append the
>   signatures in case of modules, we should follow the same thing for
>   executables too?

No, I was saying that if this patch set were to be upstreamed, then the
signature verification, at least for ELF modules and ELF executables,
should be the same.  The patch would then be a lot smaller.

> - If above comment is w.r.t use of IMA, then I have no issues in using
>   IMA as long as it can meet all the requirements. Looks like there is
>   a long TODO list before we get there. In fact for some things its not
>   even clear whether they fit in IMA or somehwere else.
> 
>   - Make sure IMA/EVM stuff chains into secureboot chain of trust.

For sure.

>   - Sort out all the memory locking related issues I mentioned in
>     other mail. You seemed to be of opinion that it is out of scope
>     for IMA, but I think it probably is nice extenstion.

Yes, it would be, but I'm not sure that your method of mmaping a file
with MAP_LOCKED is scalable, when all executables are signed.  If it is,
then why not do it in general?  Otherwise there needs to be a more
scalable solution.

>     Or somehow a way needs to be found to make sure nobody can modify
>     process address space without processe's knowledge.

I'm not sure I understand.  Does your patch already address this?

>   - Once all this works, then one needs to figure out all the RPM stuff
>     and plugins to make sure files can be singed on build server and
>     installed properly on target system.

Yes, we need the distros involvment in this to sign all files.
Immutable files should be signed with digital signatures.  Mutable files
should have hashes.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Kasatkin, Dmitry]

commit f6bf2c4c0339dabac435f518bb1fcb617fdef8f1
Author: Dmitry Kasatkin <dmitry.kasatkin@intel.com>
Date:   Thu Jan 17 18:50:43 2013 +0200

    ima: lock down memory if binary is digitally signed

    This patch set a flag in the linux_binprm structure if binary is
    digitally signed. The flag is used to lock down memory when loading
    ELF binary.

    Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com>

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 0c42cdb..ba94d13 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -44,6 +44,8 @@
 #define user_siginfo_t siginfo_t
 #endif

+#define LSM_UNSAFE_DIGSIG	16
+
 static int load_elf_binary(struct linux_binprm *bprm);
 static int load_elf_library(struct file *);
 static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
@@ -788,6 +790,9 @@ static int load_elf_binary(struct linux_binprm *bprm)

 		elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE;

+		if (bprm->unsafe & LSM_UNSAFE_DIGSIG)
+			elf_flags |= MAP_LOCKED;
+
 		vaddr = elf_ppnt->p_vaddr;
 		if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) {
 			elf_flags |= MAP_FIXED;
diff --git a/security/integrity/ima/ima_main.c
b/security/integrity/ima/ima_main.c
index b359fb5..50d9959 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -27,6 +27,8 @@

 #include "ima.h"

+#define LSM_UNSAFE_DIGSIG	16
+
 int ima_initialized;

 #ifdef CONFIG_IMA_APPRAISE
@@ -315,10 +317,19 @@ int ima_file_mmap(struct file *file, unsigned long prot)
  */
 int ima_bprm_check(struct linux_binprm *bprm)
 {
-	return process_measurement(bprm->file,
+	int err;
+	struct integrity_iint_cache *iint;
+
+	err = process_measurement(bprm->file,
 				 (strcmp(bprm->filename, bprm->interp) == 0) ?
 				 bprm->filename : bprm->interp,
 				 MAY_EXEC, BPRM_CHECK);
+
+	iint = integrity_iint_find(bprm->file->f_dentry->d_inode);
+	if (iint && (iint->flags & IMA_DIGSIG))
+		bprm->unsafe |= LSM_UNSAFE_DIGSIG;
+
+	return err;
 }

 /**

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Kasatkin, Dmitry]

Hello.

This is just a quick-patch for IMA to lock digitally signed binaries
in similar manner as the patch of this thread does...

No policy here. No optimization here. Just tests if binary has signature.
Rather simple.

- Dmitry


On Thu, Jan 17, 2013 at 7:01 PM, Kasatkin, Dmitry
<dmitry.kasatkin@intel.com> wrote:
> commit f6bf2c4c0339dabac435f518bb1fcb617fdef8f1
> Author: Dmitry Kasatkin <dmitry.kasatkin@intel.com>
> Date:   Thu Jan 17 18:50:43 2013 +0200
>
>     ima: lock down memory if binary is digitally signed
>
>     This patch set a flag in the linux_binprm structure if binary is
>     digitally signed. The flag is used to lock down memory when loading
>     ELF binary.
>
>     Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com>
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 0c42cdb..ba94d13 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -44,6 +44,8 @@
>  #define user_siginfo_t siginfo_t
>  #endif
>
> +#define LSM_UNSAFE_DIGSIG      16
> +
>  static int load_elf_binary(struct linux_binprm *bprm);
>  static int load_elf_library(struct file *);
>  static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
> @@ -788,6 +790,9 @@ static int load_elf_binary(struct linux_binprm *bprm)
>
>                 elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE;
>
> +               if (bprm->unsafe & LSM_UNSAFE_DIGSIG)
> +                       elf_flags |= MAP_LOCKED;
> +
>                 vaddr = elf_ppnt->p_vaddr;
>                 if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) {
>                         elf_flags |= MAP_FIXED;
> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index b359fb5..50d9959 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -27,6 +27,8 @@
>
>  #include "ima.h"
>
> +#define LSM_UNSAFE_DIGSIG      16
> +
>  int ima_initialized;
>
>  #ifdef CONFIG_IMA_APPRAISE
> @@ -315,10 +317,19 @@ int ima_file_mmap(struct file *file, unsigned long prot)
>   */
>  int ima_bprm_check(struct linux_binprm *bprm)
>  {
> -       return process_measurement(bprm->file,
> +       int err;
> +       struct integrity_iint_cache *iint;
> +
> +       err = process_measurement(bprm->file,
>                                  (strcmp(bprm->filename, bprm->interp) == 0) ?
>                                  bprm->filename : bprm->interp,
>                                  MAY_EXEC, BPRM_CHECK);
> +
> +       iint = integrity_iint_find(bprm->file->f_dentry->d_inode);
> +       if (iint && (iint->flags & IMA_DIGSIG))
> +               bprm->unsafe |= LSM_UNSAFE_DIGSIG;
> +
> +       return err;
>  }
>
>  /**

Re: [PATCH 0/3] ELF executable signing and verification

[Vivek Goyal]

On Thu, Jan 17, 2013 at 06:22:47PM +0200, Kasatkin, Dmitry wrote:

[..]
> > Currently it is expected to use these patches only for statically linked
> > executables. No dynamic linking. In fact patches specifically disable
> > calling interpreter. This does not prevent against somebody using dlopen()
> > sutff. So don't sign binaries which do that.
> 
> How dynamic linking and interpreter are related together?

Well interpreter will do the dynamic linking automatically? So I blocked
that.

> 
> This is rather policy than enforcement.
> Protection works only for statically linked binaries, because dynamic
> libraries are not verified.

Agreed.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Thu, Jan 17, 2013 at 11:32:45AM -0500, Mimi Zohar wrote:

[..]
> > > At this point, why would you want yet another method for signing files?
> > 
> > Are you saying that append signature instead of putting them in a section
> > or are you saying that just use IMA.
> > 
> > - For the first, I am fine with appending too if that works better. So
> >   what's wrong with current implementation. Just because we append the
> >   signatures in case of modules, we should follow the same thing for
> >   executables too?
> 
> No, I was saying that if this patch set were to be upstreamed, then the
> signature verification, at least for ELF modules and ELF executables,
> should be the same.  The patch would then be a lot smaller.

I don't think that patch is lot smaller. Initially I had written code
where signatures were appended. Parsing the signature is little different
from module. In case of modules, whole file is already in memory and
in case of executables, we are reading selected portions of file in
buffer. 

So we don't share code while parsing, hence there is no significant
code bloat when .signature section is added as opposed to appending
signature.

> 
> > - If above comment is w.r.t use of IMA, then I have no issues in using
> >   IMA as long as it can meet all the requirements. Looks like there is
> >   a long TODO list before we get there. In fact for some things its not
> >   even clear whether they fit in IMA or somehwere else.
> > 
> >   - Make sure IMA/EVM stuff chains into secureboot chain of trust.
> 
> For sure.
> 
> >   - Sort out all the memory locking related issues I mentioned in
> >     other mail. You seemed to be of opinion that it is out of scope
> >     for IMA, but I think it probably is nice extenstion.
> 
> Yes, it would be, but I'm not sure that your method of mmaping a file
> with MAP_LOCKED is scalable, when all executables are signed.  If it is,
> then why not do it in general?  Otherwise there needs to be a more
> scalable solution.

It is like running without swap enabled. There is definitely a cost
involved and I think that justfies that to begin with why not everything
should be signed. Probably a overkill.

And hence the idea of signing only very selected files on need basis.

Those who want to lock down full user space, they can happily sign full
user space and pay the penalty. But for general case, it might not make
sense.

Encrypted swap is one workaround. Which probably is more scalable then
locking down all user space in memory. But that question arises only
if we want to sign full user space in general case. And I think we 
agree that it does not make much sense.

> 
> >     Or somehow a way needs to be found to make sure nobody can modify
> >     process address space without processe's knowledge.
> 
> I'm not sure I understand.  Does your patch already address this?

Well, I was repeating the same thing in ohter words. So if we do
following.

	- No shared libaries
	- No ptrace
	- Lock down current and future mappings so nothing is swapped out.

Will it not make sure that a process address space is not modified without
process knowledge. I think my patch has little bug. I might not have
enabled a flag to lock down future mappings.

> 
> >   - Once all this works, then one needs to figure out all the RPM stuff
> >     and plugins to make sure files can be singed on build server and
> >     installed properly on target system.
> 
> Yes, we need the distros involvment in this to sign all files.
> Immutable files should be signed with digital signatures.

So you still think that I should take IMA path to solve my use case or it
is reasonable to sign executable directly and I continue down my path.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Thu, Jan 17, 2013 at 07:01:40PM +0200, Kasatkin, Dmitry wrote:
> commit f6bf2c4c0339dabac435f518bb1fcb617fdef8f1
> Author: Dmitry Kasatkin <dmitry.kasatkin@intel.com>
> Date:   Thu Jan 17 18:50:43 2013 +0200
> 
>     ima: lock down memory if binary is digitally signed
> 
>     This patch set a flag in the linux_binprm structure if binary is
>     digitally signed. The flag is used to lock down memory when loading
>     ELF binary.
> 
>     Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@intel.com>
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 0c42cdb..ba94d13 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -44,6 +44,8 @@
>  #define user_siginfo_t siginfo_t
>  #endif
> 
> +#define LSM_UNSAFE_DIGSIG	16
> +
>  static int load_elf_binary(struct linux_binprm *bprm);
>  static int load_elf_library(struct file *);
>  static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
> @@ -788,6 +790,9 @@ static int load_elf_binary(struct linux_binprm *bprm)
> 
>  		elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE;
> 
> +		if (bprm->unsafe & LSM_UNSAFE_DIGSIG)
> +			elf_flags |= MAP_LOCKED;
> +

Couple of thoughts.

- I think my patch does not take care of locking down future mappings. I
  think we might have to do.

	current->mm->def_flags = VM_LOCKED;	

 Along the lines of do_mlockall().

- Also there is still a small window open where changes to file contents
  by directly writing to block will not be detected. We are doing IMA
  check first and then faulting in pages in memory. It might have happend
  that write to disk block happened after IMA check but before page was
  read back from disk.

  so some kind of post verification also is probably needed. Or just map
  it first and then do the verification.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Frank Ch. Eigler]

Vivek Goyal <vgoyal@redhat.com> writes:

> [...]
>> Can you please tell a bit more how this patch protect against direct
>> writing to the blocks?
>
> If you have loaded all the pages from disk and locked them in memory and
> verified the signature, then even if somebody modifies a block on disk
> it does not matter. We will not read pages from disk anymore for this
> exec(). We verified the signature of executable loaded in memory and
> in-memory copy is intact.

Does this imply dramatically increasing physical RAM pressure and load
latency, because binaries (and presumably all their shared libraries)
have to be locked & loaded?  (Else if they are paged out to
encrypted-swap, is that sufficient protection against manipulation?)

- FChE

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Thu, Jan 17, 2013 at 03:33:47PM -0500, Frank Ch. Eigler wrote:
> Vivek Goyal <vgoyal@redhat.com> writes:
> 
> > [...]
> >> Can you please tell a bit more how this patch protect against direct
> >> writing to the blocks?
> >
> > If you have loaded all the pages from disk and locked them in memory and
> > verified the signature, then even if somebody modifies a block on disk
> > it does not matter. We will not read pages from disk anymore for this
> > exec(). We verified the signature of executable loaded in memory and
> > in-memory copy is intact.
> 
> Does this imply dramatically increasing physical RAM pressure and load
> latency, because binaries (and presumably all their shared libraries)
> have to be locked & loaded?  (Else if they are paged out to
> encrypted-swap, is that sufficient protection against manipulation?)

Even if you employ encrypted-swap, we still need to lock down any code
and data which lives in executable file on disk to avoid the case of
it being modified directly by writing to a block. Looks like IMA will not
detect that case.

May be we can only lock down any information which is loaded from
executable file. Rest of the pages can be swapped to encrypted swap.

As long as number of signed binaries are small, I think RAM pressure might
not be a problem but yes, if we sign everything, it will become an issue.

I am not sure how kernel can enforce the requirement of encrypted swap. If
we leave it to user as a recommendation, then we have the potential that
some hacker can bypass the whole thing. So it is not enforceable.

May be there could be a config option if that's enabled swapping works only
if it is encrypted. 

So locking few select statically compiled executables completely in memory
I think should not be too much of trouble and solve the problem I have at
hand.

For the more generic case of completely locked system, we will have to
conditionally modify the code to lock only any info loaded from executable 
and allow swapping other data to encrypted swap. This one we can look into
once somebody really wants to use it.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Kasatkin, Dmitry]

On Thu, Jan 17, 2013 at 10:55 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> On Thu, Jan 17, 2013 at 03:33:47PM -0500, Frank Ch. Eigler wrote:
>> Vivek Goyal <vgoyal@redhat.com> writes:
>>
>> > [...]
>> >> Can you please tell a bit more how this patch protect against direct
>> >> writing to the blocks?
>> >
>> > If you have loaded all the pages from disk and locked them in memory and
>> > verified the signature, then even if somebody modifies a block on disk
>> > it does not matter. We will not read pages from disk anymore for this
>> > exec(). We verified the signature of executable loaded in memory and
>> > in-memory copy is intact.
>>
>> Does this imply dramatically increasing physical RAM pressure and load
>> latency, because binaries (and presumably all their shared libraries)
>> have to be locked & loaded?  (Else if they are paged out to
>> encrypted-swap, is that sufficient protection against manipulation?)
>
> Even if you employ encrypted-swap, we still need to lock down any code
> and data which lives in executable file on disk to avoid the case of
> it being modified directly by writing to a block. Looks like IMA will not
> detect that case.
>

See my IMA patch I set today, which does locking the same way as you do.

- Dmitry

> May be we can only lock down any information which is loaded from
> executable file. Rest of the pages can be swapped to encrypted swap.
>
> As long as number of signed binaries are small, I think RAM pressure might
> not be a problem but yes, if we sign everything, it will become an issue.
>
> I am not sure how kernel can enforce the requirement of encrypted swap. If
> we leave it to user as a recommendation, then we have the potential that
> some hacker can bypass the whole thing. So it is not enforceable.
>
> May be there could be a config option if that's enabled swapping works only
> if it is encrypted.
>
> So locking few select statically compiled executables completely in memory
> I think should not be too much of trouble and solve the problem I have at
> hand.
>
> For the more generic case of completely locked system, we will have to
> conditionally modify the code to lock only any info loaded from executable
> and allow swapping other data to encrypted swap. This one we can look into
> once somebody really wants to use it.
>
> Thanks
> Vivek
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Thu, Jan 17, 2013 at 11:46:57PM +0200, Kasatkin, Dmitry wrote:
> On Thu, Jan 17, 2013 at 10:55 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > On Thu, Jan 17, 2013 at 03:33:47PM -0500, Frank Ch. Eigler wrote:
> >> Vivek Goyal <vgoyal@redhat.com> writes:
> >>
> >> > [...]
> >> >> Can you please tell a bit more how this patch protect against direct
> >> >> writing to the blocks?
> >> >
> >> > If you have loaded all the pages from disk and locked them in memory and
> >> > verified the signature, then even if somebody modifies a block on disk
> >> > it does not matter. We will not read pages from disk anymore for this
> >> > exec(). We verified the signature of executable loaded in memory and
> >> > in-memory copy is intact.
> >>
> >> Does this imply dramatically increasing physical RAM pressure and load
> >> latency, because binaries (and presumably all their shared libraries)
> >> have to be locked & loaded?  (Else if they are paged out to
> >> encrypted-swap, is that sufficient protection against manipulation?)
> >
> > Even if you employ encrypted-swap, we still need to lock down any code
> > and data which lives in executable file on disk to avoid the case of
> > it being modified directly by writing to a block. Looks like IMA will not
> > detect that case.
> >
> 
> See my IMA patch I set today, which does locking the same way as you do.

Yes but I also mentioned that still there is little problem. Signature
verification should happen after the pages have been locked and not
before that.

Also I was thinking about encrypted swap. Any root process will have access
to encrypted swap? If yes, then it atleast does not work for the use case
I am trying to solve.

By selectively signing root executable, I am differentiating it with rest
of the root executable and not trusting root process here till it is
signed. So if another root process can get to swap and modify its contents
and it modified the address space of signed process. 

So for the use case I am trying to solve, encrypted swap is not the
solution. We have to lock down all of the process memory.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[H. Peter Anvin]

You then get into issues like: do we have to ban prelink as a result?

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

>On Thu, 2013-01-17 at 10:51 -0500, Vivek Goyal wrote:
>> On Thu, Jan 17, 2013 at 10:37:01AM -0500, Mimi Zohar wrote:
>> > On Tue, 2013-01-15 at 16:34 -0500, Vivek Goyal wrote:
>> > > If a binary is signed, verify its signature. If signature is not
>valid, do
>> > > not allow execution. If binary is not signed, execution is
>allowed
>> > > unconditionally.
>> > > 
>> > > CONFIG_BINFMT_ELF_SIGNATURE controls whether elf binary signature
>support
>> > > is compiled in or not.
>> > > 
>> > > Signature are expected to be present in elf section ".section".
>This code
>> > > is written along the lines of module signature verification code.
>Just
>> > > that I have removed the magic string. It is not needed as
>signature is
>> > > expected to be present in a specific section.
>> > 
>> > Right, it's written along the lines of the original module
>signature
>> > verification code, where the signature was in the ELF header, not
>the
>> > version that was upstreamed, where the signature was appended.
>> > 
>> > > I put the signature into a section, instead of appending it so
>that
>> > > strip operation works fine.
>> > 
>> > Wouldn't the original reasons for not embedding the signature in
>the ELF
>> > header for modules apply here too?  
>> 
>> I think rusty wanted to append signatures. He thought it is much
>easier.
>> Adding a .signature section makes life easier for user space tools.
>One
>> can strip files even after signing.
>> 
>> Not that I am married to the idea of putting signature in a section.
>Just
>> that it sounded reasonable enough to do for an RFC. So if appending 
>> signature proves to be better, it is easy to implement that.
>> > 
>> > > One signs and verifies  all the areas mapped by PT_LOAD segments
>of elf
>> > > binary. Typically Elf header is mapped in first PT_LOAD segment.
>As adding
>> > > .signature section can change three elf header fields (e_shoff,
>e_shnum
>> > > and e_shstrndx), these fields are excluded from digest
>calculation
>> > > 
>> > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
>> > 
>> > At this point, why would you want yet another method for signing
>files?
>> 
>> Are you saying that append signature instead of putting them in a
>section
>> or are you saying that just use IMA.
>> 
>> - For the first, I am fine with appending too if that works better.
>So
>>   what's wrong with current implementation. Just because we append
>the
>>   signatures in case of modules, we should follow the same thing for
>>   executables too?
>
>No, I was saying that if this patch set were to be upstreamed, then the
>signature verification, at least for ELF modules and ELF executables,
>should be the same.  The patch would then be a lot smaller.
>
>> - If above comment is w.r.t use of IMA, then I have no issues in
>using
>>   IMA as long as it can meet all the requirements. Looks like there
>is
>>   a long TODO list before we get there. In fact for some things its
>not
>>   even clear whether they fit in IMA or somehwere else.
>> 
>>   - Make sure IMA/EVM stuff chains into secureboot chain of trust.
>
>For sure.
>
>>   - Sort out all the memory locking related issues I mentioned in
>>     other mail. You seemed to be of opinion that it is out of scope
>>     for IMA, but I think it probably is nice extenstion.
>
>Yes, it would be, but I'm not sure that your method of mmaping a file
>with MAP_LOCKED is scalable, when all executables are signed.  If it
>is,
>then why not do it in general?  Otherwise there needs to be a more
>scalable solution.
>
>>     Or somehow a way needs to be found to make sure nobody can modify
>>     process address space without processe's knowledge.
>
>I'm not sure I understand.  Does your patch already address this?
>
>>   - Once all this works, then one needs to figure out all the RPM
>stuff
>>     and plugins to make sure files can be singed on build server and
>>     installed properly on target system.
>
>Yes, we need the distros involvment in this to sign all files.
>Immutable files should be signed with digital signatures.  Mutable
>files
>should have hashes.
>
>thanks,
>
>Mimi

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Thu, 2013-01-17 at 16:52 -0500, Vivek Goyal wrote:
> On Thu, Jan 17, 2013 at 11:46:57PM +0200, Kasatkin, Dmitry wrote:
> > On Thu, Jan 17, 2013 at 10:55 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> > > On Thu, Jan 17, 2013 at 03:33:47PM -0500, Frank Ch. Eigler wrote:
> > >> Vivek Goyal <vgoyal@redhat.com> writes:
> > >>
> > >> > [...]
> > >> >> Can you please tell a bit more how this patch protect against direct
> > >> >> writing to the blocks?
> > >> >
> > >> > If you have loaded all the pages from disk and locked them in memory and
> > >> > verified the signature, then even if somebody modifies a block on disk
> > >> > it does not matter. We will not read pages from disk anymore for this
> > >> > exec(). We verified the signature of executable loaded in memory and
> > >> > in-memory copy is intact.
> > >>
> > >> Does this imply dramatically increasing physical RAM pressure and load
> > >> latency, because binaries (and presumably all their shared libraries)
> > >> have to be locked & loaded?  (Else if they are paged out to
> > >> encrypted-swap, is that sufficient protection against manipulation?)
> > >
> > > Even if you employ encrypted-swap, we still need to lock down any code
> > > and data which lives in executable file on disk to avoid the case of
> > > it being modified directly by writing to a block. Looks like IMA will not
> > > detect that case.
> > >
> > 
> > See my IMA patch I set today, which does locking the same way as you do.
> 
> Yes but I also mentioned that still there is little problem. Signature
> verification should happen after the pages have been locked and not
> before that.

My initial comments mentioned this.  We can either move the existing
ima_file_mmap() or add a new hook.

> Also I was thinking about encrypted swap. Any root process will have access
> to encrypted swap? If yes, then it atleast does not work for the use case
> I am trying to solve.

Dmitry's patch example does exactly what you did, setting MAP_LOCKED
before the mmap, but does it for all ELF executables.  This could be
configurable.  I would suggest looking at the IMA policy.

> By selectively signing root executable, I am differentiating it with rest
> of the root executable and not trusting root process here till it is
> signed. So if another root process can get to swap and modify its contents
> and it modified the address space of signed process.

You're hard coding policy in the kernel and relying on userspace to only
sign specific files.

> So for the use case I am trying to solve, encrypted swap is not the
> solution. We have to lock down all of the process memory.

Like IMA-appraisal, your patches enforce integrity.  The LSM hooks were
originally defined for mandatory access control.  A parallel set of
hooks, called LIM, was proposed but were not upstreamed, as there
weren't any users other than IMA.  As a result, the IMA calls were
embedded directly into the vfs layer, except where the LSM and IMA hooks
were co-located.

James/Rusty please correct me if I'm wrong, but the new kernel module
signature verification should not be construed as a general license for
adding integrity verification in an ad-hoc manner, but was an exception
for the lack of a file descriptor.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Sun, 2013-01-20 at 08:17 -0800, H. Peter Anvin wrote:
> You then get into issues like: do we have to ban prelink as a result?

Once you change a file, the original signature shouldn't match.  If you
really trust prelink, then make prelink a trusted application that can
resign the modified file.  How to create/store/use private keys on the
target system is a separate issue.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[H. Peter Anvin]

On 01/20/2013 08:55 AM, Mimi Zohar wrote:
> On Sun, 2013-01-20 at 08:17 -0800, H. Peter Anvin wrote:
>> You then get into issues like: do we have to ban prelink as a result?
> 
> Once you change a file, the original signature shouldn't match.  If you
> really trust prelink, then make prelink a trusted application that can
> resign the modified file.  How to create/store/use private keys on the
> target system is a separate issue.
> 

That is true in a particularly brittle sense of the word, but it would
also be possible for the signature system to explicitly recognize the
transformation performed by prelink -- and no other -- before
verification.  That being said, that may be quite complex.

	-hpa

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Thu, 2013-01-17 at 12:36 -0500, Vivek Goyal wrote:
> On Thu, Jan 17, 2013 at 11:32:45AM -0500, Mimi Zohar wrote:
> 
> [..]
> > > > At this point, why would you want yet another method for signing files?
> > > 
> > > Are you saying that append signature instead of putting them in a section
> > > or are you saying that just use IMA.
> > > 
> > > - For the first, I am fine with appending too if that works better. So
> > >   what's wrong with current implementation. Just because we append the
> > >   signatures in case of modules, we should follow the same thing for
> > >   executables too?
> > 
> > No, I was saying that if this patch set were to be upstreamed, then the
> > signature verification, at least for ELF modules and ELF executables,
> > should be the same.  The patch would then be a lot smaller.
> 
> I don't think that patch is lot smaller. Initially I had written code
> where signatures were appended. Parsing the signature is little different
> from module. In case of modules, whole file is already in memory and
> in case of executables, we are reading selected portions of file in
> buffer. 

Have you looked at the original kernel module signature verification
code as posted by David?  It did something similar, but was not
upstreamed.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Sun, Jan 20, 2013 at 12:20:00PM -0500, Mimi Zohar wrote:
> On Thu, 2013-01-17 at 12:36 -0500, Vivek Goyal wrote:
> > On Thu, Jan 17, 2013 at 11:32:45AM -0500, Mimi Zohar wrote:
> > 
> > [..]
> > > > > At this point, why would you want yet another method for signing files?
> > > > 
> > > > Are you saying that append signature instead of putting them in a section
> > > > or are you saying that just use IMA.
> > > > 
> > > > - For the first, I am fine with appending too if that works better. So
> > > >   what's wrong with current implementation. Just because we append the
> > > >   signatures in case of modules, we should follow the same thing for
> > > >   executables too?
> > > 
> > > No, I was saying that if this patch set were to be upstreamed, then the
> > > signature verification, at least for ELF modules and ELF executables,
> > > should be the same.  The patch would then be a lot smaller.
> > 
> > I don't think that patch is lot smaller. Initially I had written code
> > where signatures were appended. Parsing the signature is little different
> > from module. In case of modules, whole file is already in memory and
> > in case of executables, we are reading selected portions of file in
> > buffer. 
> 
> Have you looked at the original kernel module signature verification
> code as posted by David?  It did something similar, but was not
> upstreamed.

I have. I think keeping code in a section makes stripping of section
easy. Anyway, these sections are not loaded in memory at file exec
time so it should be fine.

So appending signature is easy and I can change the implementation to
do it like modules.

But please give a more stronger reason that why it should be appened
to executable then put in a section.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Vivek Goyal]

On Tue, Jan 15, 2013 at 11:55:59PM -0500, Mimi Zohar wrote:

[..]
> Please remind me why you can't use IMA-appraisal, which was upstreamed
> in Linux 3.7?  Why another method is needed?

So is this IMA-appraisal also supports digital signatures? The IMA white
paper seems to put digital signatures in separate category
(IMA-Appraisal-Signature-Extension).

> 
> With IMA-appraisal, there are a couple of issues that would still need
> to be addressed:
> - missing the ability to specify the validation method required.
> - modify the ima_appraise_tcb policy policy to require elf executables
> to be digitally signed.

For my use case, all executable don't have to be digitally signed. If
something is digitally signed then do the signature verification.

Thanks
Vivek

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Mon, 2013-01-21 at 11:42 -0500, Vivek Goyal wrote:
> On Tue, Jan 15, 2013 at 11:55:59PM -0500, Mimi Zohar wrote:
> 
> [..]
> > Please remind me why you can't use IMA-appraisal, which was upstreamed
> > in Linux 3.7?  Why another method is needed?
> 
> So is this IMA-appraisal also supports digital signatures? The IMA white
> paper seems to put digital signatures in separate category
> (IMA-Appraisal-Signature-Extension).

The white paper was written a couple of years ago, before either EVM or
IMA-appraisal were upstreamed.

- Linux 3.2: Extended Verification Module (EVM) - protects file metadata from offline modification
- Linux 3.3: Dmitry Kasatkin's digital signature verification for use with EVM/IMA-appraisal.
- Linux 3.7: IMA-appraisal/with digital signatures

> > With IMA-appraisal, there are a couple of issues that would still need
> > to be addressed:
> > - missing the ability to specify the validation method required.

Patches to address this issue are available from linux-integrity-test/
#next-ima-appraise-status and were posted on the LSM mailing list as an
RFC (12/18/2012).  Review of these patches would be appreciated.

> > - modify the ima_appraise_tcb policy policy to require elf executables
> > to be digitally signed.
> 
> For my use case, all executable don't have to be digitally signed. If
> something is digitally signed then do the signature verification.

We already discussed this.  Hard coding policy into the Linux kernel is
wrong.

thanks,

Mimi

Re: [PATCH 2/3] binfmt_elf: Verify signature of signed elf binary

[Mimi Zohar]

On Mon, 2013-01-21 at 10:45 -0500, Vivek Goyal wrote:
> On Sun, Jan 20, 2013 at 12:20:00PM -0500, Mimi Zohar wrote:
> > On Thu, 2013-01-17 at 12:36 -0500, Vivek Goyal wrote:
> > > On Thu, Jan 17, 2013 at 11:32:45AM -0500, Mimi Zohar wrote:
> > > 
> > > [..]
> > > > > > At this point, why would you want yet another method for signing files?
> > > > > 
> > > > > Are you saying that append signature instead of putting them in a section
> > > > > or are you saying that just use IMA.
> > > > > 
> > > > > - For the first, I am fine with appending too if that works better. So
> > > > >   what's wrong with current implementation. Just because we append the
> > > > >   signatures in case of modules, we should follow the same thing for
> > > > >   executables too?
> > > > 
> > > > No, I was saying that if this patch set were to be upstreamed, then the
> > > > signature verification, at least for ELF modules and ELF executables,
> > > > should be the same.  The patch would then be a lot smaller.
> > > 
> > > I don't think that patch is lot smaller. Initially I had written code
> > > where signatures were appended. Parsing the signature is little different
> > > from module. In case of modules, whole file is already in memory and
> > > in case of executables, we are reading selected portions of file in
> > > buffer. 
> > 
> > Have you looked at the original kernel module signature verification
> > code as posted by David?  It did something similar, but was not
> > upstreamed.
> 
> I have. I think keeping code in a section makes stripping of section
> easy. Anyway, these sections are not loaded in memory at file exec
> time so it should be fine.
> 
> So appending signature is easy and I can change the implementation to
> do it like modules.
> 
> But please give a more stronger reason that why it should be appened
> to executable then put in a section.

There are two existing upstreamed methods for verifying the integrity of
files.  One uses a file descriptor, the other is memory based, for the
specific use case, where a file descriptor is not available.  The real
question is what benefit there is to adding another method?  That
question needs to be addressed in the patch description.

thanks,

Mimi

Re: [PATCH 0/3] ELF executable signing and verification

[Rusty Russell]

Vivek Goyal <vgoyal@redhat.com> writes:
> Hi,
>
> This is a very crude RFC for ELF executable signing and verification. This
> has been done along the lines of module signature verification.

Yes, but I'm the first to admit that's the wrong lines.

The reasons we didn't choose that for module signatures:
1) I was unaware of it,
2) We didn't have a file descriptor in the module syscall, and
3) It needs attributes, and we don't understand xattrs in cpio (though
   bsdcpio does).

#1 and #2 are no longer true; #3 is a simple matter of coding.

Since signing binaries is the New Hotness, I'd prefer not to keep
reiterating this discussion every month.  Let's beef up IMA instead...

Thanks,
Rusty.