linux-ima-devel — Discussion list for problems or development.

Re: [Linux-ima-devel] [RFC 2/4] ima: use namespaced flags for IMA_AUDITED on each namespace

[Mehmet K. <mka...@li...>]

> On Jul 21, 2017, at 3:43 PM, Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...> wrote:
> 
> Thanks, Mehmet.
> 
>> As a result, the namespace cleanup does not touch inodes or iints. However,
>> we didn't save the inode-->namespace association, so we don't have a way 
>> of removing the flags when the inode is removed. But we keep a copy of 
>> ino and generation, so if inode matches but ino and generation do not 
>> match, we can reset and reuse it.
> Does it also solves the file change case? Should you add the i_version? When a file is changed, the namespaced flags must be cleared in all namespaces so the file can be audited again in all namespaces. The i_version/generation checkings should be done every time the namespaced flags are referenced and also clearing the flags if not matching, right?
> On the other hand, all these new fields consume memory which might be a considerable amount when there are many files under many namespaces.

Yes, adding the i_version would be necessary. You are correct. There
is a memory vs lookup/cleanup trade off here. 

>> The extra lookup is the namespace one. We should be able to tell the 
>> current namespace without needing an rbtree or a hash table.
> This is a good way to avoid the need for additional 2-look ups, while it is still not clear to me why a whole new namespace is needed and we should think about it carefully to avoid side effects. It seems actually an extension of mount namespace, it is like this probably related to possibilities of upstream acceptance. I believe AppArmor tried to add a real new namespace with a new clone() flag and it was not accepted. Any concerns with that? This new IMA namespace could be used in the future for namespacing the policy, while I believe the user namespace could be a better option for namespacing the policy.

Having explicit IMA namespacing would probably be better. I don't know
what the right way is. Having a separate clone flag would be helpful,
but I understand if one flag per each kernel feature may not be feasible.
 
Mehmet
 

Re: [Linux-ima-devel] [RFC PATCH 2/5] ima: Add ns_status for storing namespaced iint data

[Mehmet K. <mka...@li...>]

> On Jul 21, 2017, at 3:45 PM, Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...> wrote:
> 
> Mehmet,
> 
> +struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
> +				    struct inode *inode)
> +{
> +	struct ns_status *status;
> +	int skip_insert = 0;
> +
> +	status = ima_ns_status_find(ns, inode);
> +	if (status) {
> +		/*
> +		 * Unlike integrity_iint_cache we are not free'ing the
> +		 * ns_status data when the inode is free'd. So, in addition to
> +		 * checking the inode pointer, we need to make sure the
> +		 * (i_generation, i_ino) pair matches as well. In the future
> +		 * we might want to add support for lazily walking the rbtree
> +		 * to clean it up.
> +		 */
> +		if (inode->i_ino == status->i_ino &&
> +		    inode->i_generation == status->i_generation)
> 
> As pointed in another email, probably we need to check the 'i_version' here to account for the file change case. when files change, we want to clear out the namespaced flags.

Sure, good catch.

Mehmet

Re: [Linux-ima-devel] [RFC PATCH 0/5] ima: namespacing IMA audit messages

[Mehmet K. <mka...@li...>]

> On Jul 21, 2017, at 3:44 PM, Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...> wrote:
> 
> Mehmet,
> 
>> As a result, clone() and unshare() with CLONE_NEWNS results in a new 
>> mount and a new IMA namespace, while setns() called with the fd of 
>> /proc/*/ns/mnt would NOT have the same result. A second setns() 
>> call with the fd /proc/*/ns/ima would be required.
> If I understood correctly, a given mount namespace is always paired with a given IMA namespace, but processes which setns() to a given mount namespace, would need an additional step to fix the pairing. That means when a process changes to a different mount namespace, the namespaced IMA flags are still the flags 'controlled' by the old mount namespace until the IMA namespace is fixed with a new setns(). It is also possible that a process points to two completely unrelated mount and IMA namespaces.

Yes, we would prefer a solution that does not require two synced calls
to setns. I was thinking of putting the nsproxy pointer in the nsfs
inode->i_private. So that, in setns we can get to the ima_namespace. 

> This namespaces relation might confuse the audit log since a given mount namespace id might appear twice for the same file with no file change, or an expected entry may be not logged since the process changed to an IMA namespace where the inode namespaced flag was already IMA_AUDITED.
> Probably one side effect from that is in the audit log, which should be fixed replacing 'ns_mnt' with 'ns_ima' on the audit record.

Yes, we could have both, maybe? 

Mehmet

[Linux-ima-devel] Fw:inquiry/quote V-02214-002539

[Billie_Guulkdaw <ta...@16...>]

D9N6vy8ear 씇뢵휗콛퉤숽쿼띌놓frie1834nds,ﺡﻍﺙﻱﺩﻍﺙﻙThiﻙﺡﻍﺏs i5997s A18694107nd

y, Iは゜ォゥョワひnter996379893naсжыйвхвtion늨펶al Saﺡﻍﺥles,ﺵﺥﻑﻅﻁﺵﻁ fr735om JOﻍﺙﻡﺩﺝA-h7427404ard

warﺵﺱﻱﻝـﺩﻭﻅe,We015726 are ﺕﻡﺡﻡﺵﻅﺽـpr









ofンげヨカゕトアessio





nal xre8OphardOjware5lZhZbUq accﻉﺽﻙﺡﺏﺝessゟモげカかァori







esﻭﻁﻑﻉﻥﺵﺡﺽﻕ m


akeむほニヲれr in ﻙﺝChуаадржina.
We yjBiproぷびがぷvi




de mキぜざだぐむ゘しachi






ninицдпльйсуg,st0202ampﺍﺭﻝﺹing,zz cヒてプイast잷딭ingヺナゆぺゎピせ,forgﺏﻭﺕﻉing,







weld


ingql;Al





so꿢씸셇횹쇋붃앿훒듺 iндгуёчйгэnc쏥삐훗쭰떇뭛옏챫lu735892ded tV4FBM8qLp0he va






rious





 of 40173778surfaAh5KLcOWce 즳쎖쩍옆먛꺂쐎treaマヸtmenﺹﺱﻍﺭﺯﻉـt,ﺯﺥﺫﺽﻥﺭﻑLikかダ゜e Zinヸぽヒなc plфъшншated,ﺡﺝﺭﺍChهﻡﺫﻑﺕﻅormeﻕﻑﺍ pla뽺엝힇뼪뜺뚱댰teduvZB4,Nic몖륤럧쇍룍닪떯청롵kelмпшйюшявыу p






latLzAed,AlDqao9Qnod


izi쟲쓭권놺묕줰졾ng, チズよヅheat0RLu8m tr42ZwV13eatぶっセべワホmen겗궱쨧뢞t eлсогпчейtc.
If 쪝겱돕you휜쉍춦뱕쐈쭱쥹쓞탱 hぐヴーヲユどave aj3hny pﺵﺫﻅـﻅﺩﺫﻕote왣긇꾿썃ntialﻕﻕﻥﺕﻥهﺹﺍ met






al귋펯렊헸쐘 pr

ocesョアげぐびォペヷマsinvQZg orылщёмзщъder깊좥깤썰쟮꺧돢쐁, epcxFXL68ple줳블뾩ase kдьрклеindly゙ごデ se8mJuNqTQnd usデメゅゑじ the0610412385 drﻑﺱﺫawfU2d2omoingvIpmarka2, weLDs w015756il

l りサフづけヸヶof3191fer 








youзж comмвгиpeهﺕﺙﺙﺵﻅﻭﺯtitivﻍﻁﺍﺫﻕﺥﺍﺵﺩe q큃룖힗돤딇룾uotat32361576ioDlLn .
We

 are7310831835 sur튈뎳보뮁틥돒병늂뱪e ﺯﻭﺍﻱﻭﺩanﻍهﻍهﺱﺭﺍﺝy yёянтлодour q






uestьмиio2913980ns w툅섀씱웫솺갠쪘즜il71651l ge245554t our럀뤮 p









romююыщаpt 2430199027repロミパly.
Besﻭﺱt w잇뫲쏇횥셻묍컱맒is







hes.
Andy_____혖큋____




_____뻗착줱툚뽀보춯쯴____ﻡﺱ____Fe6pxQ___뭭뙑룼켸샌딝폐__43NHKK____ィザ゘ぜフゐすボヨ_____ﺯﺏﺱﻙﺥ____튥픺퇲____





___むぴヮヨづせず____귵럭촵빀쇏____эчвъсша____MJoKwG___67256362____
J딾궨꼃깞OA-80HARD



WARE
Huaxкшлыйрауоin Induん぀けサstr4839ial Zon5164e,Da졧넠댏퇘쫘tang CunDong gゞめプuan CityGббъщuang dепмэадongChina .
Webwww6819.joaщьла-h1rpJardware.꼁섢쌸뵩걅com
Unsubscrible from email.Click here.  

Re: [Linux-ima-devel] [RFC PATCH 3/5] ima: mamespace audit status flags

[Magalhaes, G. (B. R&D-CL) <gui...@hp...>]

Mehmet,

+#define IMA_NS_STATUS_ACTIONS	IMA_AUDIT
+#define IMA_NS_STATUS_FLAGS	IMA_AUDITED
+
+unsigned long iint_flags(struct integrity_iint_cache *iint,
+			 struct ns_status *status)
+{
+	if (!status)
+		return iint->flags;
+
+	return iint->flags & (status->flags & IMA_NS_STATUS_FLAGS); }

I believe the first '&' should be replaced with '|', so you can return the consolidated 'flags | ns_flags'.

+
+unsigned long set_iint_flags(struct integrity_iint_cache *iint,
+			     struct ns_status *status, unsigned long flags) {
+	iint->flags = flags;

I believe the global flags should consider if status is NULL, otherwise the namespaced flags such as IMA_AUDITED should not be set to the global flags otherwise this namespace flag IMA_AUDITED could be incorrectly injected into other namespaces.
--
Guilherme

+	if (status)
+		status->flags = flags & IMA_NS_STATUS_FLAGS;
+	return flags;
+}
--
2.9.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to maj...@vg... More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Linux-ima-devel] [RFC PATCH 5/5] ima: Add ns_mnt, dev, ino fields to IMA audit measurement msgs

[Magalhaes, G. (B. R&D-CL) <gui...@hp...>]

Mehmet,

 	if (flags & IMA_AUDITED)
 		return;
@@ -314,6 +316,14 @@ void ima_audit_measurement(struct integrity_iint_cache *iint,
 	audit_log_format(ab, " hash=");
 	snprintf(algo_hash, sizeof(algo_hash), "%s:%s", algo_name, hash);
 	audit_log_untrustedstring(ab, algo_hash);
+	ns = mntns_operations.get(current);

As pointed out in other email, probably we need to get the IMA namespace id to add it to the record as 'ns_ima' below.
--
Guilherme

+	if (!IS_ERR_OR_NULL(ns)) {
+		audit_log_format(ab, " ns_mnt=%u", ns->inum);
+		mntns_operations.put(ns);
+	}
+	audit_log_format(ab, " dev=");
+	audit_log_untrustedstring(ab, iint->inode->i_sb->s_id);
+	audit_log_format(ab, " ino=%lu", iint->inode->i_ino);
 
 	audit_log_task_info(ab, current);
 	audit_log_end(ab);
--
2.9.4

Re: [Linux-ima-devel] [RFC PATCH 2/5] ima: Add ns_status for storing namespaced iint data

[Magalhaes, G. (B. R&D-CL) <gui...@hp...>]

Mehmet,

+struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
+				    struct inode *inode)
+{
+	struct ns_status *status;
+	int skip_insert = 0;
+
+	status = ima_ns_status_find(ns, inode);
+	if (status) {
+		/*
+		 * Unlike integrity_iint_cache we are not free'ing the
+		 * ns_status data when the inode is free'd. So, in addition to
+		 * checking the inode pointer, we need to make sure the
+		 * (i_generation, i_ino) pair matches as well. In the future
+		 * we might want to add support for lazily walking the rbtree
+		 * to clean it up.
+		 */
+		if (inode->i_ino == status->i_ino &&
+		    inode->i_generation == status->i_generation)

As pointed in another email, probably we need to check the 'i_version' here to account for the file change case. when files change, we want to clear out the namespaced flags.

--
Guilherme

+			return status;
+
+		/* Same inode number is reused, overwrite the ns_status */
+		skip_insert = 1;
+	} else {
+		status = kmem_cache_alloc(ns->ns_status_cache, GFP_NOFS);
+		if (!status)
+			return ERR_PTR(-ENOMEM);
+	}
+
+	write_lock(&ns->ns_status_lock);
+
+	if (!skip_insert)
+		insert_ns_status(ns, inode, status);
+
+	status->inode = inode;
+	status->i_ino = inode->i_ino;
+	status->i_generation = inode->i_generation;
+	status->flags = 0UL;
+	write_unlock(&ns->ns_status_lock);
+
+	return status;
+}
--
2.9.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to maj...@vg... More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Linux-ima-devel] [RFC PATCH 0/5] ima: namespacing IMA audit messages

[Magalhaes, G. (B. R&D-CL) <gui...@hp...>]

Mehmet,

> As a result, clone() and unshare() with CLONE_NEWNS results in a new 
> mount and a new IMA namespace, while setns() called with the fd of 
> /proc/*/ns/mnt would NOT have the same result. A second setns() 
> call with the fd /proc/*/ns/ima would be required.
If I understood correctly, a given mount namespace is always paired with a given IMA namespace, but processes which setns() to a given mount namespace, would need an additional step to fix the pairing. That means when a process changes to a different mount namespace, the namespaced IMA flags are still the flags 'controlled' by the old mount namespace until the IMA namespace is fixed with a new setns(). It is also possible that a process points to two completely unrelated mount and IMA namespaces. This namespaces relation might confuse the audit log since a given mount namespace id might appear twice for the same file with no file change, or an expected entry may be not logged since the process changed to an IMA namespace where the inode namespaced flag was already IMA_AUDITED.
Probably one side effect from that is in the audit log, which should be fixed replacing 'ns_mnt' with 'ns_ima' on the audit record.

--
Guilherme

-----Original Message-----
From: own...@vg... [mailto:own...@vg...] On Behalf Of Mehmet Kayaalp
Sent: quinta-feira, 20 de julho de 2017 19:50
To: ima-devel <lin...@li...>
Cc: containers <con...@li...>; linux-kernel <lin...@vg...>; linux-security-module <lin...@vg...>; Tycho Andersen <ty...@do...>; Serge E . Hallyn <se...@ha...>; Yuqiong Sun <sun...@gm...>; David Safford <dav...@ge...>; Mehmet Kayaalp <mka...@cs...>; Stefan Berger <st...@li...>; Mehmet Kayaalp <mka...@li...>
Subject: [RFC PATCH 0/5] ima: namespacing IMA audit messages

This patch set implements an IMA namespace data structure that gets created alongside a mount namespace with CLONE_NEWNS, and lays down the foundation for namespacing the different aspects of IMA (eg. IMA-audit, IMA-measurement, IMA-appraisal).

The original PoC patches [1], created a new CLONE_NEWIMA flag to explicitly control when a new IMA namespace should be created. Based on comments, we elected to hang the IMA namepace off of existing namespaces, and the mount namespace made the most sense. However, we actually allocate a new namespace struct in nsproxy, allocate a new inum, and have an ima symlink in /proc/*/ns/, instead of adding a pointer from the mnt_namespace. As a result, clone() and unshare() with CLONE_NEWNS results in a new mount and a new IMA namespace, while setns() called with the fd of /proc/*/ns/mnt would NOT have the same result. A second setns() call with the fd /proc/*/ns/ima would be required.

The first patch creates the ima_namespace data, while the second patch puts the iint->flags in the namespace. The third patch uses these flags for namespacing the IMA-audit messages, enabling the same file to be audited each time it is accessed in a new namespace. Rest of the patches are small fixes and improvements to the audit messages generated by IMA.
Subsequent patch sets will namespace IMA-measurement and IMA-appraisal.

[1] https://sourceforge.net/p/linux-ima/mailman/message/35939754/

Guilherme Magalhaes (1):
  ima: Add ns_mnt, dev, ino fields to IMA audit measurement msgs

Mehmet Kayaalp (2):
  ima: Add ns_status for storing namespaced iint data
  ima: mamespace audit status flags

Mimi Zohar (1):
  ima: differentiate auditing policy rules from "audit" actions

Yuqiong Sun (1):
  ima: extend clone() with IMA namespace support

 fs/proc/namespaces.c                |   3 +
 include/linux/ima.h                 |  40 +++++
 include/linux/nsproxy.h             |   1 +
 include/linux/proc_ns.h             |   2 +
 include/uapi/linux/audit.h          |   3 +-
 init/Kconfig                        |  10 ++
 kernel/nsproxy.c                    |  15 ++
 security/integrity/ima/Makefile     |   1 +
 security/integrity/ima/ima.h        |  49 +++++-
 security/integrity/ima/ima_api.c    |  18 +-
 security/integrity/ima/ima_init.c   |   4 +
 security/integrity/ima/ima_main.c   |  15 +-
 security/integrity/ima/ima_ns.c     | 324 ++++++++++++++++++++++++++++++++++++
 security/integrity/ima/ima_policy.c |   2 +-
 14 files changed, 478 insertions(+), 9 deletions(-)  create mode 100644 security/integrity/ima/ima_ns.c

--
2.9.4

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to maj...@vg... More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Linux-ima-devel] [RFC 2/4] ima: use namespaced flags for IMA_AUDITED on each namespace

[Magalhaes, G. (B. R&D-CL) <gui...@hp...>]

Thanks, Mehmet.

> As a result, the namespace cleanup does not touch inodes or iints. However,
> we didn't save the inode-->namespace association, so we don't have a way 
>of removing the flags when the inode is removed. But we keep a copy of 
> ino and generation, so if inode matches but ino and generation do not 
> match, we can reset and reuse it.
Does it also solves the file change case? Should you add the i_version? When a file is changed, the namespaced flags must be cleared in all namespaces so the file can be audited again in all namespaces. The i_version/generation checkings should be done every time the namespaced flags are referenced and also clearing the flags if not matching, right?
On the other hand, all these new fields consume memory which might be a considerable amount when there are many files under many namespaces.

> The extra lookup is the namespace one. We should be able to tell the 
> current namespace without needing an rbtree or a hash table.
This is a good way to avoid the need for additional 2-look ups, while it is still not clear to me why a whole new namespace is needed and we should think about it carefully to avoid side effects. It seems actually an extension of mount namespace, it is like this probably related to possibilities of upstream acceptance. I believe AppArmor tried to add a real new namespace with a new clone() flag and it was not accepted. Any concerns with that? This new IMA namespace could be used in the future for namespacing the policy, while I believe the user namespace could be a better option for namespacing the policy.

--
Guilherme

-----Original Message-----
From: Mehmet Kayaalp [mailto:mka...@li...] 
Sent: sexta-feira, 21 de julho de 2017 10:04
To: Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...>
Cc: Mimi Zohar <zo...@li...>; lin...@li...
Subject: Re: [Linux-ima-devel] [RFC 2/4] ima: use namespaced flags for IMA_AUDITED on each namespace


> On Jul 19, 2017, at 4:09 PM, Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...> wrote:
> 
> Hi Mimi,
> For all solutions with a single additional tree look up, the namespaced flags tree is placed inside the iint structure. This is what the POC posted by Mehmet does, and this mechanism is also the solution for 'namespacing' the integrity_iint_find() function.

See the patch set I posted yesterday [1], that does not place it inside the iint, and creates an rbtree as part of the namespace struct. 

> However, this option suffers with the costly lock/unlock of hundreds of inodes when a namespace is released to cleanup namespaced flags, what makes this solution not acceptable for upstreaming as you indicated.

As a result, the namespace cleanup does not touch inodes or iints. However, we didn't save the inode-->namespace association, so we don't have a way of removing the flags when the inode is removed. But we keep a copy of ino and generation, so if inode matches but ino and generation do not match, we can reset and reuse it.

> We are really convinced that the solution #2 below is the best possible solution, which has 2 additional tree look ups for namespaced flags references (2 in addition to the existent iint tree look up, the first look up gets the list of flags for a given namespace and the second lookup get the target flag from the list for a given iint). 

The extra lookup is the namespace one. We should be able to tell the current namespace without needing an rbtree or a hash table.

[1]: https://sourceforge.net/p/linux-ima/mailman/message/35956610/

Mehmet

Re: [Linux-ima-devel] [RFC 2/4] ima: use namespaced flags for IMA_AUDITED on each namespace

[Mehmet K. <mka...@li...>]

> On Jul 19, 2017, at 4:09 PM, Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...> wrote:
> 
> Hi Mimi,
> For all solutions with a single additional tree look up, the namespaced flags tree is placed inside the iint structure. This is what the POC posted by Mehmet does, and this mechanism is also the solution for 'namespacing' the integrity_iint_find() function.

See the patch set I posted yesterday [1], that does not place it inside the
iint, and creates an rbtree as part of the namespace struct. 

> However, this option suffers with the costly lock/unlock of hundreds of inodes when a namespace is released to cleanup namespaced flags, what makes this solution not acceptable for upstreaming as you indicated.

As a result, the namespace cleanup does not touch inodes or iints. However, we
didn't save the inode-->namespace association, so we don't have a way of
removing the flags when the inode is removed. But we keep a copy of ino and
generation, so if inode matches but ino and generation do not match, we can
reset and reuse it.

> We are really convinced that the solution #2 below is the best possible solution, which has 2 additional tree look ups for namespaced flags references (2 in addition to the existent iint tree look up, the first look up gets the list of flags for a given namespace and the second lookup get the target flag from the list for a given iint). 

The extra lookup is the namespace one. We should be able to tell the current
namespace without needing an rbtree or a hash table.

[1]: https://sourceforge.net/p/linux-ima/mailman/message/35956610/

Mehmet

[Linux-ima-devel] [RFC PATCH 5/5] ima: Add ns_mnt, dev, ino fields to IMA audit measurement msgs

[Mehmet K. <mka...@li...>]

From: Guilherme Magalhaes <gui...@hp...>

Extending audit measurement record with mount namespace id, file inode,
and device name. These fields uniquely identify a pathname considering
different mount namespaces. The file inode on a given device is unique
and these fields are required to identify a namespace id since this id
can be released and later reused by a different process.

Signed-off-by: Guilherme Magalhaes <gui...@hp...>

Changelog:
* Change the field name from "mnt_ns" to "ns_mnt"

Signed-off-by: Mehmet Kayaalp <mka...@li...>
---
 security/integrity/ima/ima_api.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 4a77072..084b126 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -18,6 +18,7 @@
 #include <linux/fs.h>
 #include <linux/xattr.h>
 #include <linux/evm.h>
+#include <linux/proc_ns.h>
 
 #include "ima.h"
 
@@ -296,6 +297,7 @@ void ima_audit_measurement(struct integrity_iint_cache *iint,
 	char algo_hash[sizeof(hash) + strlen(algo_name) + 2];
 	int i;
 	unsigned long flags = iint_flags(iint, status);
+	struct ns_common *ns;
 
 	if (flags & IMA_AUDITED)
 		return;
@@ -314,6 +316,14 @@ void ima_audit_measurement(struct integrity_iint_cache *iint,
 	audit_log_format(ab, " hash=");
 	snprintf(algo_hash, sizeof(algo_hash), "%s:%s", algo_name, hash);
 	audit_log_untrustedstring(ab, algo_hash);
+	ns = mntns_operations.get(current);
+	if (!IS_ERR_OR_NULL(ns)) {
+		audit_log_format(ab, " ns_mnt=%u", ns->inum);
+		mntns_operations.put(ns);
+	}
+	audit_log_format(ab, " dev=");
+	audit_log_untrustedstring(ab, iint->inode->i_sb->s_id);
+	audit_log_format(ab, " ino=%lu", iint->inode->i_ino);
 
 	audit_log_task_info(ab, current);
 	audit_log_end(ab);
-- 
2.9.4

[Linux-ima-devel] [RFC PATCH 3/5] ima: mamespace audit status flags

[Mehmet K. <mka...@li...>]

The iint cache stores whether the file is measured, appraised, audited
etc. This patch moves the IMA_AUDITED flag into the per-namespace
ns_status, enabling IMA audit mechanism to audit the same file each time
it is accessed in a new namespace.

The ns_status is not looked up if the CONFIG_IMA_NS is disabled or if
none of the IMA_NS_STATUS_ACTIONS (currently only IMA_AUDIT) are enabled.

Read and write operations on the iint flags is replaced with function
calls. For reading, iint_flags() returns the bitwise AND of iint->flags
and ns_status->flags. The ns_status flags are masked with
IMA_NS_STATUS_FLAGS (currently only IMA_AUDITED). Similarly
set_iint_flags() only writes the masked portion to the ns_status flags,
while the iint flags is set as before. The ns_status parameter added to
ima_audit_measurement() is used with the above functions to query and
set the ns_status flags.

Signed-off-by: Mehmet Kayaalp <mka...@li...>
---
 init/Kconfig                      |  4 +++-
 security/integrity/ima/ima.h      | 24 +++++++++++++++++++++++-
 security/integrity/ima/ima_api.c  |  8 +++++---
 security/integrity/ima/ima_main.c | 15 ++++++++++++---
 security/integrity/ima/ima_ns.c   | 21 +++++++++++++++++++++
 5 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 339f84d..6bfc579 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1294,7 +1294,9 @@ config IMA_NS
 	help
 	  Allow the creation of IMA namespaces for each mount namespace.
 	  Namespaced IMA data enables having IMA features work separately
-	  for each mount namespace.
+	  for each mount namespace. Currently, only the audit status flags
+	  are stored in the namespace, which allows the same file to be
+	  audited each time it is accessed in a new namespace.
 
 endif # NAMESPACES
 
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 5ab769a..78921b7 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -210,7 +210,8 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   struct evm_ima_xattr_data *xattr_value,
 			   int xattr_len, int pcr);
 void ima_audit_measurement(struct integrity_iint_cache *iint,
-			   const unsigned char *filename);
+			   const unsigned char *filename,
+			   struct ns_status *status);
 int ima_alloc_init_template(struct ima_event_data *event_data,
 			    struct ima_template_entry **entry);
 int ima_store_template(struct ima_template_entry *entry, int violation,
@@ -299,10 +300,17 @@ static inline int ima_read_xattr(struct dentry *dentry,
 
 #endif /* CONFIG_IMA_APPRAISE */
 
+#define IMA_NS_STATUS_ACTIONS   IMA_AUDIT
+#define IMA_NS_STATUS_FLAGS     IMA_AUDITED
+
 #ifdef CONFIG_IMA_NS
 int ima_ns_init(void);
 struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
 				    struct inode *inode);
+unsigned long iint_flags(struct integrity_iint_cache *iint,
+			 struct ns_status *status);
+unsigned long set_iint_flags(struct integrity_iint_cache *iint,
+			     struct ns_status *status, unsigned long flags);
 #else
 static inline int ima_ns_init(void)
 {
@@ -314,6 +322,20 @@ static inline struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
 {
 	return NULL;
 }
+
+static inline unsigned long iint_flags(struct integrity_iint_cache *iint,
+				       struct ns_status *status)
+{
+	return iint->flags;
+}
+
+static inline unsigned long set_iint_flags(struct integrity_iint_cache *iint,
+					   struct ns_status *status,
+					   unsigned long flags)
+{
+	iint->flags = flags;
+	return flags;
+}
 #endif /* CONFIG_IMA_NS */
 
 /* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c2edba8..4a77072 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -287,15 +287,17 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 }
 
 void ima_audit_measurement(struct integrity_iint_cache *iint,
-			   const unsigned char *filename)
+			   const unsigned char *filename,
+			   struct ns_status *status)
 {
 	struct audit_buffer *ab;
 	char hash[(iint->ima_hash->length * 2) + 1];
 	const char *algo_name = hash_algo_name[iint->ima_hash->algo];
 	char algo_hash[sizeof(hash) + strlen(algo_name) + 2];
 	int i;
+	unsigned long flags = iint_flags(iint, status);
 
-	if (iint->flags & IMA_AUDITED)
+	if (flags & IMA_AUDITED)
 		return;
 
 	for (i = 0; i < iint->ima_hash->length; i++)
@@ -316,7 +318,7 @@ void ima_audit_measurement(struct integrity_iint_cache *iint,
 	audit_log_task_info(ab, current);
 	audit_log_end(ab);
 
-	iint->flags |= IMA_AUDITED;
+	set_iint_flags(iint, status, flags | IMA_AUDITED);
 }
 
 /*
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2aebb79..fb002f2 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -160,6 +160,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 {
 	struct inode *inode = file_inode(file);
 	struct integrity_iint_cache *iint = NULL;
+	struct ns_status *status = NULL;
 	struct ima_template_desc *template_desc;
 	char *pathbuf = NULL;
 	char filename[NAME_MAX];
@@ -170,6 +171,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	int xattr_len = 0;
 	bool violation_check;
 	enum hash_algo hash_algo;
+	unsigned long flags;
 
 	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
 		return 0;
@@ -196,6 +198,12 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 		iint = integrity_inode_get(inode);
 		if (!iint)
 			goto out;
+
+		if (action & IMA_NS_STATUS_ACTIONS) {
+			status = ima_get_ns_status(get_current_ns(), inode);
+			if (IS_ERR(status))
+				goto out;
+		}
 	}
 
 	if (violation_check) {
@@ -211,9 +219,10 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,
 	 *  IMA_AUDIT, IMA_AUDITED)
 	 */
-	iint->flags |= action;
+	flags = iint_flags(iint, status);
+	flags = set_iint_flags(iint, status, flags | action);
 	action &= IMA_DO_MASK;
-	action &= ~((iint->flags & (IMA_DONE_MASK ^ IMA_MEASURED)) >> 1);
+	action &= ~((flags & (IMA_DONE_MASK ^ IMA_MEASURED)) >> 1);
 
 	/* If target pcr is already measured, unset IMA_MEASURE action */
 	if ((action & IMA_MEASURE) && (iint->measured_pcrs & (0x1 << pcr)))
@@ -251,7 +260,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 		rc = ima_appraise_measurement(func, iint, file, pathname,
 					      xattr_value, xattr_len, opened);
 	if (action & IMA_AUDIT)
-		ima_audit_measurement(iint, pathname);
+		ima_audit_measurement(iint, pathname, status);
 
 out_digsig:
 	if ((mask & MAY_WRITE) && (iint->flags & IMA_DIGSIG) &&
diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
index 5ec5a4b..562a0812 100644
--- a/security/integrity/ima/ima_ns.c
+++ b/security/integrity/ima/ima_ns.c
@@ -301,3 +301,24 @@ struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
 
 	return status;
 }
+
+#define IMA_NS_STATUS_ACTIONS	IMA_AUDIT
+#define IMA_NS_STATUS_FLAGS	IMA_AUDITED
+
+unsigned long iint_flags(struct integrity_iint_cache *iint,
+			 struct ns_status *status)
+{
+	if (!status)
+		return iint->flags;
+
+	return iint->flags & (status->flags & IMA_NS_STATUS_FLAGS);
+}
+
+unsigned long set_iint_flags(struct integrity_iint_cache *iint,
+			     struct ns_status *status, unsigned long flags)
+{
+	iint->flags = flags;
+	if (status)
+		status->flags = flags & IMA_NS_STATUS_FLAGS;
+	return flags;
+}
-- 
2.9.4

[Linux-ima-devel] [RFC PATCH 2/5] ima: Add ns_status for storing namespaced iint data

[Mehmet K. <mka...@li...>]

This patch adds an rbtree to the IMA namespace structure that stores a
namespaced version of iint->flags in ns_status struct. Similar to the
integrity_iint_cache, both the iint ns_struct are looked up using the
inode pointer value. The lookup, allocate, and insertion code is also
similar, except ns_struct is not free'd when the inode is free'd.
Instead, the lookup verifies the i_ino and i_generation fields are also a
match. A lazy clean up of the rbtree that removes free'd inodes could be
implemented to reclaim the invalid entries.

Signed-off-by: Mehmet Kayaalp <mka...@li...>
---
 include/linux/ima.h             |   3 +
 security/integrity/ima/ima.h    |  16 ++++++
 security/integrity/ima/ima_ns.c | 120 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 139 insertions(+)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 11e4841..3fdf56f 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -111,6 +111,9 @@ struct ima_namespace {
 	struct user_namespace *user_ns;
 	struct ns_common ns;
 	struct ima_namespace *parent;
+	struct rb_root ns_status_tree;
+	rwlock_t ns_status_lock;
+	struct kmem_cache *ns_status_cache;
 };
 
 extern struct ima_namespace init_ima_ns;
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 8a8234a..5ab769a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -128,6 +128,14 @@ static inline void ima_load_kexec_buffer(void) {}
  */
 extern bool ima_canonical_fmt;
 
+struct ns_status {
+	struct rb_node rb_node;
+	struct inode *inode;
+	ino_t i_ino;
+	u32 i_generation;
+	unsigned long flags;
+};
+
 /* Internal IMA function definitions */
 int ima_init(void);
 int ima_fs_init(void);
@@ -293,11 +301,19 @@ static inline int ima_read_xattr(struct dentry *dentry,
 
 #ifdef CONFIG_IMA_NS
 int ima_ns_init(void);
+struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
+				    struct inode *inode);
 #else
 static inline int ima_ns_init(void)
 {
 	return 0;
 }
+
+static inline struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
+						  struct inode *inode)
+{
+	return NULL;
+}
 #endif /* CONFIG_IMA_NS */
 
 /* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
index 383217b..5ec5a4b 100644
--- a/security/integrity/ima/ima_ns.c
+++ b/security/integrity/ima/ima_ns.c
@@ -20,6 +20,9 @@ static void get_ima_ns(struct ima_namespace *ns);
 
 int ima_init_namespace(struct ima_namespace *ns)
 {
+	ns->ns_status_tree = RB_ROOT;
+	rwlock_init(&ns->ns_status_lock);
+	ns->ns_status_cache = KMEM_CACHE(ns_status, SLAB_PANIC);
 	return 0;
 }
 
@@ -98,10 +101,24 @@ struct ima_namespace *copy_ima(unsigned long flags,
 	return new_ns;
 }
 
+static void free_ns_status_cache(struct ima_namespace *ns)
+{
+	struct ns_status *status, *next;
+
+	write_lock(&ns->ns_status_lock);
+	rbtree_postorder_for_each_entry_safe(status, next,
+					     &ns->ns_status_tree, rb_node)
+		kmem_cache_free(ns->ns_status_cache, status);
+	ns->ns_status_tree = RB_ROOT;
+	write_unlock(&ns->ns_status_lock);
+	kmem_cache_destroy(ns->ns_status_cache);
+}
+
 static void destroy_ima_ns(struct ima_namespace *ns)
 {
 	put_user_ns(ns->user_ns);
 	ns_free_inum(&ns->ns);
+	free_ns_status_cache(ns);
 	kfree(ns);
 }
 
@@ -181,3 +198,106 @@ struct ima_namespace init_ima_ns = {
 	.parent = NULL,
 };
 EXPORT_SYMBOL(init_ima_ns);
+
+/*
+ * __ima_ns_status_find - return the ns_status associated with an inode
+ */
+static struct ns_status *__ima_ns_status_find(struct ima_namespace *ns,
+					      struct inode *inode)
+{
+	struct ns_status *status;
+	struct rb_node *n = ns->ns_status_tree.rb_node;
+
+	while (n) {
+		status = rb_entry(n, struct ns_status, rb_node);
+
+		if (inode < status->inode)
+			n = n->rb_left;
+		else if (inode->i_ino > status->i_ino)
+			n = n->rb_right;
+		else
+			break;
+	}
+	if (!n)
+		return NULL;
+
+	return status;
+}
+
+/*
+ * ima_ns_status_find - return the ns_status associated with an inode
+ */
+static struct ns_status *ima_ns_status_find(struct ima_namespace *ns,
+					    struct inode *inode)
+{
+	struct ns_status *status;
+
+	read_lock(&ns->ns_status_lock);
+	status = __ima_ns_status_find(ns, inode);
+	read_unlock(&ns->ns_status_lock);
+
+	return status;
+}
+
+void insert_ns_status(struct ima_namespace *ns, struct inode *inode,
+		      struct ns_status *status)
+{
+	struct rb_node **p;
+	struct rb_node *node, *parent = NULL;
+	struct ns_status *test_status;
+
+	p = &ns->ns_status_tree.rb_node;
+	while (*p) {
+		parent = *p;
+		test_status = rb_entry(parent, struct ns_status, rb_node);
+		if (inode < test_status->inode)
+			p = &(*p)->rb_left;
+		else
+			p = &(*p)->rb_right;
+	}
+	node = &status->rb_node;
+	rb_link_node(node, parent, p);
+	rb_insert_color(node, &ns->ns_status_tree);
+}
+
+struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
+				    struct inode *inode)
+{
+	struct ns_status *status;
+	int skip_insert = 0;
+
+	status = ima_ns_status_find(ns, inode);
+	if (status) {
+		/*
+		 * Unlike integrity_iint_cache we are not free'ing the
+		 * ns_status data when the inode is free'd. So, in addition to
+		 * checking the inode pointer, we need to make sure the
+		 * (i_generation, i_ino) pair matches as well. In the future
+		 * we might want to add support for lazily walking the rbtree
+		 * to clean it up.
+		 */
+		if (inode->i_ino == status->i_ino &&
+		    inode->i_generation == status->i_generation)
+			return status;
+
+		/* Same inode number is reused, overwrite the ns_status */
+		skip_insert = 1;
+	} else {
+		status = kmem_cache_alloc(ns->ns_status_cache, GFP_NOFS);
+		if (!status)
+			return ERR_PTR(-ENOMEM);
+	}
+
+	write_lock(&ns->ns_status_lock);
+
+	if (!skip_insert)
+		insert_ns_status(ns, inode, status);
+
+	status->inode = inode;
+	status->i_ino = inode->i_ino;
+	status->i_generation = inode->i_generation;
+	status->flags = 0UL;
+	write_unlock(&ns->ns_status_lock);
+
+	return status;
+}
-- 
2.9.4

[Linux-ima-devel] [RFC PATCH 1/5] ima: extend clone() with IMA namespace support

[Mehmet K. <mka...@li...>]

From: Yuqiong Sun <su...@us...>

Add new CONFIG_IMA_NS config option.  Let clone() create a new IMA
namespace upon CLONE_NEWNS flag. Add ima_ns data structure in nsproxy.
ima_ns is allocated and freed upon IMA namespace creation and exit.
Currently, the ima_ns contains no useful IMA data but only a dummy
interface. This patch creates the framework for namespacing the different
aspects of IMA (eg. IMA-audit, IMA-measurement, IMA-appraisal).

Signed-off-by: Yuqiong Sun <su...@us...>

Changelog:
* Use CLONE_NEWNS instead of a new CLONE_NEWIMA flag
* Use existing ima.h headers
* Move the ima_namespace.c to security/integrity/ima/ima_ns.c
* Fix typo INFO->INO
* Each namespace free's itself, removed recursively free'ing
until init_ima_ns from free_ima_ns()

Signed-off-by: Mehmet Kayaalp <mka...@li...>
---
 fs/proc/namespaces.c              |   3 +
 include/linux/ima.h               |  37 ++++++++
 include/linux/nsproxy.h           |   1 +
 include/linux/proc_ns.h           |   2 +
 init/Kconfig                      |   8 ++
 kernel/nsproxy.c                  |  15 ++++
 security/integrity/ima/Makefile   |   1 +
 security/integrity/ima/ima.h      |   9 ++
 security/integrity/ima/ima_init.c |   4 +
 security/integrity/ima/ima_ns.c   | 183 ++++++++++++++++++++++++++++++++++++++
 10 files changed, 263 insertions(+)
 create mode 100644 security/integrity/ima/ima_ns.c

diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c
index 3803b24..222a64e 100644
--- a/fs/proc/namespaces.c
+++ b/fs/proc/namespaces.c
@@ -32,6 +32,9 @@ static const struct proc_ns_operations *ns_entries[] = {
 #ifdef CONFIG_CGROUPS
 	&cgroupns_operations,
 #endif
+#ifdef CONFIG_IMA_NS
+	&imans_operations,
+#endif
 };
 
 static const char *proc_ns_get_link(struct dentry *dentry,
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e..11e4841 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -105,4 +105,41 @@ static inline int ima_inode_removexattr(struct dentry *dentry,
 	return 0;
 }
 #endif /* CONFIG_IMA_APPRAISE */
+
+struct ima_namespace {
+	struct kref kref;
+	struct user_namespace *user_ns;
+	struct ns_common ns;
+	struct ima_namespace *parent;
+};
+
+extern struct ima_namespace init_ima_ns;
+
+#ifdef CONFIG_IMA_NS
+void put_ima_ns(struct ima_namespace *ns);
+struct ima_namespace *copy_ima(unsigned long flags,
+			       struct user_namespace *user_ns,
+			       struct ima_namespace *old_ns);
+static inline struct ima_namespace *get_current_ns(void)
+{
+	return current->nsproxy->ima_ns;
+}
+#else
+static inline void put_ima_ns(struct ima_namespace *ns)
+{
+	return;
+}
+
+static inline struct ima_namespace *copy_ima(unsigned long flags,
+					     struct user_namespace *user_ns,
+					     struct ima_namespace *old_ns)
+{
+	return old_ns;
+}
+
+static inline struct ima_namespace *get_current_ns(void)
+{
+	return NULL;
+}
+#endif /* CONFIG_IMA_NS */
 #endif /* _LINUX_IMA_H */
diff --git a/include/linux/nsproxy.h b/include/linux/nsproxy.h
index ac0d65b..a97031d 100644
--- a/include/linux/nsproxy.h
+++ b/include/linux/nsproxy.h
@@ -35,6 +35,7 @@ struct nsproxy {
 	struct pid_namespace *pid_ns_for_children;
 	struct net 	     *net_ns;
 	struct cgroup_namespace *cgroup_ns;
+	struct ima_namespace *ima_ns;
 };
 extern struct nsproxy init_nsproxy;
 
diff --git a/include/linux/proc_ns.h b/include/linux/proc_ns.h
index 58ab28d..c7c1239 100644
--- a/include/linux/proc_ns.h
+++ b/include/linux/proc_ns.h
@@ -31,6 +31,7 @@ extern const struct proc_ns_operations pidns_for_children_operations;
 extern const struct proc_ns_operations userns_operations;
 extern const struct proc_ns_operations mntns_operations;
 extern const struct proc_ns_operations cgroupns_operations;
+extern const struct proc_ns_operations imans_operations;
 
 /*
  * We always define these enumerators
@@ -42,6 +43,7 @@ enum {
 	PROC_USER_INIT_INO	= 0xEFFFFFFDU,
 	PROC_PID_INIT_INO	= 0xEFFFFFFCU,
 	PROC_CGROUP_INIT_INO	= 0xEFFFFFFBU,
+	PROC_IMA_INIT_INO	= 0xEFFFFFFAU,
 };
 
 #ifdef CONFIG_PROC_FS
diff --git a/init/Kconfig b/init/Kconfig
index 1d3475f..339f84d 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1287,6 +1287,14 @@ config NET_NS
 	help
 	  Allow user space to create what appear to be multiple instances
 	  of the network stack.
+config IMA_NS
+	bool "IMA namespace"
+	depends on IMA
+	default y
+	help
+	  Allow the creation of IMA namespaces for each mount namespace.
+	  Namespaced IMA data enables having IMA features work separately
+	  for each mount namespace.
 
 endif # NAMESPACES
 
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index f6c5d33..85be341 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -27,6 +27,7 @@
 #include <linux/syscalls.h>
 #include <linux/cgroup.h>
 #include <linux/perf_event.h>
+#include <linux/ima.h>
 
 static struct kmem_cache *nsproxy_cachep;
 
@@ -44,6 +45,9 @@ struct nsproxy init_nsproxy = {
 #ifdef CONFIG_CGROUPS
 	.cgroup_ns		= &init_cgroup_ns,
 #endif
+#ifdef CONFIG_IMA_NS
+	.ima_ns                 = &init_ima_ns,
+#endif
 };
 
 static inline struct nsproxy *create_nsproxy(void)
@@ -110,8 +114,17 @@ static struct nsproxy *create_new_namespaces(unsigned long flags,
 		goto out_net;
 	}
 
+	new_nsp->ima_ns = copy_ima(flags, user_ns, tsk->nsproxy->ima_ns);
+	if (IS_ERR(new_nsp->ima_ns)) {
+		err = PTR_ERR(new_nsp->ima_ns);
+		goto out_ima;
+	}
+
 	return new_nsp;
 
+out_ima:
+	if (new_nsp->ima_ns)
+		put_ima_ns(new_nsp->ima_ns);
 out_net:
 	put_cgroup_ns(new_nsp->cgroup_ns);
 out_cgroup:
@@ -181,6 +194,8 @@ void free_nsproxy(struct nsproxy *ns)
 	if (ns->pid_ns_for_children)
 		put_pid_ns(ns->pid_ns_for_children);
 	put_cgroup_ns(ns->cgroup_ns);
+	if (ns->ima_ns)
+		put_ima_ns(ns->ima_ns);
 	put_net(ns->net_ns);
 	kmem_cache_free(nsproxy_cachep, ns);
 }
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 29f198b..20493f0 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -8,5 +8,6 @@ obj-$(CONFIG_IMA) += ima.o
 ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
 	 ima_policy.o ima_template.o ima_template_lib.o
 ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
+ima-$(CONFIG_IMA_NS) += ima_ns.o
 ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
 obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487..8a8234a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -291,6 +291,15 @@ static inline int ima_read_xattr(struct dentry *dentry,
 
 #endif /* CONFIG_IMA_APPRAISE */
 
+#ifdef CONFIG_IMA_NS
+int ima_ns_init(void);
+#else
+static inline int ima_ns_init(void)
+{
+	return 0;
+}
+#endif /* CONFIG_IMA_NS */
+
 /* LSM based policy rules require audit */
 #ifdef CONFIG_IMA_LSM_RULES
 
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 2967d49..7f884a7 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -137,5 +137,9 @@ int __init ima_init(void)
 
 	ima_init_policy();
 
+	rc = ima_ns_init();
+	if (rc != 0)
+		return rc;
+
 	return ima_fs_init();
 }
diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
new file mode 100644
index 0000000..383217b
--- /dev/null
+++ b/security/integrity/ima/ima_ns.c
@@ -0,0 +1,183 @@
+/*
+ * Copyright (C) 2008 IBM Corporation
+ * Author: Yuqiong Sun <su...@us...>
+ *
+ * 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.
+ */
+
+#include <linux/export.h>
+#include <linux/user_namespace.h>
+#include <linux/proc_ns.h>
+#include <linux/kref.h>
+#include <linux/slab.h>
+#include <linux/ima.h>
+
+#include "ima.h"
+
+static void get_ima_ns(struct ima_namespace *ns);
+
+int ima_init_namespace(struct ima_namespace *ns)
+{
+	return 0;
+}
+
+int ima_ns_init(void)
+{
+	return ima_init_namespace(&init_ima_ns);
+}
+
+static struct ima_namespace *create_ima_ns(void)
+{
+	struct ima_namespace *ima_ns;
+
+	ima_ns = kmalloc(sizeof(*ima_ns), GFP_KERNEL);
+	if (ima_ns)
+		kref_init(&ima_ns->kref);
+
+	return ima_ns;
+}
+
+/**
+ * Clone a new ns copying an original ima namespace, setting refcount to 1
+ *
+ * @old_ns: old ima namespace to clone
+ * @user_ns: user namespace that current task runs in
+ * Return ERR_PTR(-ENOMEM) on error (failure to kmalloc), new ns otherwise
+ */
+static struct ima_namespace *clone_ima_ns(struct user_namespace *user_ns,
+					  struct ima_namespace *old_ns)
+{
+	struct ima_namespace *ns;
+	int err;
+
+	ns = create_ima_ns();
+	if (!ns)
+		return ERR_PTR(-ENOMEM);
+
+	err = ns_alloc_inum(&ns->ns);
+	if (err) {
+		kfree(ns);
+		return ERR_PTR(err);
+	}
+
+	ns->ns.ops = &imans_operations;
+	get_ima_ns(old_ns);
+	ns->parent = old_ns;
+	ns->user_ns = get_user_ns(user_ns);
+
+	ima_init_namespace(ns);
+
+	return ns;
+}
+
+/**
+ * Copy task's ima namespace, or clone it if flags specifies CLONE_NEWNS.
+ *
+ * @flags: flags used in the clone syscall
+ * @user_ns: user namespace that current task runs in
+ * @old_ns: old ima namespace to clone
+ */
+
+struct ima_namespace *copy_ima(unsigned long flags,
+			       struct user_namespace *user_ns,
+			       struct ima_namespace *old_ns)
+{
+	struct ima_namespace *new_ns;
+
+	BUG_ON(!old_ns);
+	get_ima_ns(old_ns);
+
+	if (!(flags & CLONE_NEWNS))
+		return old_ns;
+
+	new_ns = clone_ima_ns(user_ns, old_ns);
+	put_ima_ns(old_ns);
+
+	return new_ns;
+}
+
+static void destroy_ima_ns(struct ima_namespace *ns)
+{
+	put_user_ns(ns->user_ns);
+	ns_free_inum(&ns->ns);
+	kfree(ns);
+}
+
+static void free_ima_ns(struct kref *kref)
+{
+	struct ima_namespace *ns;
+
+	ns = container_of(kref, struct ima_namespace, kref);
+	destroy_ima_ns(ns);
+}
+
+static void get_ima_ns(struct ima_namespace *ns)
+{
+	kref_get(&ns->kref);
+}
+
+void put_ima_ns(struct ima_namespace *ns)
+{
+	kref_put(&ns->kref, free_ima_ns);
+}
+
+static inline struct ima_namespace *to_ima_ns(struct ns_common *ns)
+{
+	return container_of(ns, struct ima_namespace, ns);
+}
+
+static struct ns_common *imans_get(struct task_struct *task)
+{
+	struct ima_namespace *ns = NULL;
+	struct nsproxy *nsproxy;
+
+	task_lock(task);
+	nsproxy = task->nsproxy;
+	if (nsproxy) {
+		ns = nsproxy->ima_ns;
+		get_ima_ns(ns);
+	}
+	task_unlock(task);
+
+	return ns ? &ns->ns : NULL;
+}
+
+static void imans_put(struct ns_common *ns)
+{
+	put_ima_ns(to_ima_ns(ns));
+}
+
+static int imans_install(struct nsproxy *nsproxy, struct ns_common *new)
+{
+	struct ima_namespace *ns = to_ima_ns(new);
+
+	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN) ||
+	    !ns_capable(current_user_ns(), CAP_SYS_ADMIN))
+		return -EPERM;
+
+	get_ima_ns(ns);
+	put_ima_ns(nsproxy->ima_ns);
+	nsproxy->ima_ns = ns;
+	return 0;
+}
+
+const struct proc_ns_operations imans_operations = {
+	.name = "ima",
+	.type = CLONE_NEWNS,
+	.get = imans_get,
+	.put = imans_put,
+	.install = imans_install,
+};
+
+struct ima_namespace init_ima_ns = {
+	.kref = KREF_INIT(2),
+	.user_ns = &init_user_ns,
+	.ns.inum = PROC_IMA_INIT_INO,
+#ifdef CONFIG_IMA_NS
+	.ns.ops = &imans_operations,
+#endif
+	.parent = NULL,
+};
+EXPORT_SYMBOL(init_ima_ns);
-- 
2.9.4

[Linux-ima-devel] [RFC PATCH 0/5] ima: namespacing IMA audit messages

[Mehmet K. <mka...@li...>]

This patch set implements an IMA namespace data structure that gets
created alongside a mount namespace with CLONE_NEWNS, and lays down the
foundation for namespacing the different aspects of IMA (eg. IMA-audit,
IMA-measurement, IMA-appraisal).

The original PoC patches [1], created a new CLONE_NEWIMA flag to
explicitly control when a new IMA namespace should be created. Based on
comments, we elected to hang the IMA namepace off of existing namespaces,
and the mount namespace made the most sense. However, we actually
allocate a new namespace struct in nsproxy, allocate a new inum, and have
an ima symlink in /proc/*/ns/, instead of adding a pointer from the
mnt_namespace. As a result, clone() and unshare() with CLONE_NEWNS
results in a new mount and a new IMA namespace, while setns() called with
the fd of /proc/*/ns/mnt would NOT have the same result. A second setns()
call with the fd /proc/*/ns/ima would be required.

The first patch creates the ima_namespace data, while the second patch
puts the iint->flags in the namespace. The third patch uses these flags
for namespacing the IMA-audit messages, enabling the same file to be
audited each time it is accessed in a new namespace. Rest of the patches
are small fixes and improvements to the audit messages generated by IMA.
Subsequent patch sets will namespace IMA-measurement and IMA-appraisal.

[1] https://sourceforge.net/p/linux-ima/mailman/message/35939754/

Guilherme Magalhaes (1):
  ima: Add ns_mnt, dev, ino fields to IMA audit measurement msgs

Mehmet Kayaalp (2):
  ima: Add ns_status for storing namespaced iint data
  ima: mamespace audit status flags

Mimi Zohar (1):
  ima: differentiate auditing policy rules from "audit" actions

Yuqiong Sun (1):
  ima: extend clone() with IMA namespace support

 fs/proc/namespaces.c                |   3 +
 include/linux/ima.h                 |  40 +++++
 include/linux/nsproxy.h             |   1 +
 include/linux/proc_ns.h             |   2 +
 include/uapi/linux/audit.h          |   3 +-
 init/Kconfig                        |  10 ++
 kernel/nsproxy.c                    |  15 ++
 security/integrity/ima/Makefile     |   1 +
 security/integrity/ima/ima.h        |  49 +++++-
 security/integrity/ima/ima_api.c    |  18 +-
 security/integrity/ima/ima_init.c   |   4 +
 security/integrity/ima/ima_main.c   |  15 +-
 security/integrity/ima/ima_ns.c     | 324 ++++++++++++++++++++++++++++++++++++
 security/integrity/ima/ima_policy.c |   2 +-
 14 files changed, 478 insertions(+), 9 deletions(-)
 create mode 100644 security/integrity/ima/ima_ns.c

-- 
2.9.4

Re: [Linux-ima-devel] [RFC 2/4] ima: use namespaced flags for IMA_AUDITED on each namespace

[Magalhaes, G. (B. R&D-CL) <gui...@hp...>]

Hi Mimi,
For all solutions with a single additional tree look up, the namespaced flags tree is placed inside the iint structure. This is what the POC posted by Mehmet does, and this mechanism is also the solution for 'namespacing' the integrity_iint_find() function.
However, this option suffers with the costly lock/unlock of hundreds of inodes when a namespace is released to cleanup namespaced flags, what makes this solution not acceptable for upstreaming as you indicated.

We are really convinced that the solution #2 below is the best possible solution, which has 2 additional tree look ups for namespaced flags references (2 in addition to the existent iint tree look up, the first look up gets the list of flags for a given namespace and the second lookup get the target flag from the list for a given iint). I understand your concern about a relative performance impact with the 2 tree look up. We could, as a mitigation and if really necessary, add a shortcut/accelerator for flag references on the initial namespace (since it is never released AFAIK), so there would be no additional tree lookup on this namespace, which is probably the most common namespace.

We have a new version ready to post, now with a rbtree replacing a radix tree and following the solution #2 below.
I you agree, I can post this new version so we can discuss it further considering the overall solution.

Thanks.
--
Guilherme

-----Original Message-----
From: Mimi Zohar [mailto:zo...@li...] 
Sent: segunda-feira, 17 de julho de 2017 14:04
To: Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...>
Cc: lin...@li...
Subject: Re: [Linux-ima-devel] [RFC 2/4] ima: use namespaced flags for IMA_AUDITED on each namespace

On Mon, 2017-07-17 at 14:35 +0000, Magalhaes, Guilherme (Brazil R&DCL)
wrote:
> Mimi,
> Let me update you on the ideas below. First, on the 2-lookups 
> solution, we have one radix tree inside another. On the first radix 
> tree, the index is namespace id, which seems ok (not sparse). The 
> second radix tree is indexed by iint address, which must be replaced 
> by a rbtree as you indicated.
> The second point is about the possible alternative solution I 
> mentioned. Actually, I realized that we would have possible memory 
> leaks on that solution since it would not be possible to delete the 
> namespaced flags when a namespace is released. It would wait when the 
> namespace is reused for the same iint and then it would finally reset 
> the flags under that namespace. Then, the namespaced flags would never 
> be deleted (to avoid the need for locking the inode when the namespace 
> is released).
> 
> Therefore, we still have 2 options:
> 1) namespaced flags tree inside the iint structure, single lookup, but 
> when namespace is released it would have to lock and unlock each 
> related inode accessed under that namespace to safely delete the 
> namespaced flag. There can be hundred/thousands of iints namespaced 
> flags to delete.

Definitely not a good idea.

> 2) namespaced flags tree outside the iint structure, we would need 2 
> lookups when a namespace flag is referenced.

There needs to be two look ups, just not two tree look ups.

Mimi

Re: [Linux-ima-devel] IMA SHA-1 log format "specification"

[Mehmet K. <mka...@li...>]

> On Jul 17, 2017, at 4:44 PM, Ken Goldman <kg...@li...> wrote:
> 
> Question:  What is the maximum number of event records?

There is no limit and the number is stored in a long field. 

> 2. IMA Event
> 
> An IMA event record has the following fields.
> 
> 2.1. PCR Index
> 
> This is a 4-byte integer representing the PCR Index.
> 
> Note:  The value is currently always 10.
> 2.2.  Template Hash
> 
> This is a 20-byte SHA-1 hash of the Template Data field.
> 
> There is no associated length or descriptor.
> 
> The Template Hash is used directly in the PCR 10 extend operation.
> 2.3. Template Name Length
> 
> This is a 4-byte integer representing the length of the Template Name field.
> 
> Question:  What is the maximum length?

in security/integrity/ima/ima_template.c
#define MAX_TEMPLATE_NAME_LEN 15

> 
> 2.4. Template Name
> 
> This is a printable string representing the template name.
> 
> The string is NOT nul terminated.  It is guaranteed to be printable.
> 
> For legal names, see section 3 Template Data.
> 
> 2.5. Template Data Length
> 
> This is a 4-byte integer representing the length of the Template Data field.
> 
> Note that there is redundancy, in that the ima-sig data fields are self-describing.  This can be checked for consistency.
> 
> 2.6. Template Data
> 
> See Section 3 Template Data for the contents of this field.
> 
> 
> 3. Template Data
> 
> Template data can have the following fields.  Unless specified, each is preceded by a 4-byte length.
> 
> d	20-byte digest, SHA-1 or zero padded MD-5 (no length)
> d-ng	hash algorithm + digest
> n	256-byte nul terminated file name (no length)
> n-ng	file name
> sig	signature header + signature
> uuid	(undocumented)
> pid	(undocumented)
> ppid	(undocumented)
> gid	(undocumented)
> 
> The template names become (where | is the concatenation symbol)
> 
> ima		d | n
> ima-ng		d-ng | n-ng
> ima-sig		d-ng | n-ng | sig
> 
> 3.1. Hash Length
> 
> This is a 4-byte integer representing the combined length of the Hash Algorithm and File Data Hash fields.  Note that the File Data Hash contains the two following fields, and those fields do not have explicit lengths.
> 
> 3.2. Hash Algorithm
> 
> This is a nul terminated string representing the hash algorithm of the File Data Hash field.  Two values are currently used:
> 
> * sha256:
> * sha1:
> 
> Note this redundancy, which can be checked for consistency:
> 
> * The Hash Length minus the length of the Hash Algorithm field (including the nul terminator) yields the size of the File Data Hash.
> * The length of a hash based on the Hash Algorithm yields the size of the File Data Hash.
> 
> 3.3. File Data Hash
> 
> This is a hash of the file data that was used to compute the Signature.
> 
> The length and hash algorithm are determined by the Hash Algorithm field.
> 
> 3.1. File Name Length
> 
> This is a 4-byte integer representing the length of the file name, including the nul terminator.  The maximum value is MAXPATHLEN +1?
> 
> Note that there is redundancy, in that the file name is nul terminated. This can be checked for consistency.
> 
> 3.2. File Name
> 
> This is a nul terminated string representing the name of the file that was measured.
> 
> The length is determined by the File Name Length.
> 
> 3.3. Signature Length
> 
> This is a 4-byte integer representing the total length of the Signature Header and Signature fields.  The value may be zero, indicating that those two fields are not present.
> 
> 3.4. Signature Header
> 
> This field is fixed at 9 bytes, consisting of 5 fields.
> 3.4.1. Signature Type
> 
> This is a 1-byte field.  The value is 0x03.
> 
> Question:  What are the valid values and meanings.  How does the type affect the other fields.

The whole signature data is part of the xattr data. This is actually 
not the signature type, but the xattr type defined in:
security/integrity/integrity.h as:

enum evm_ima_xattr_type {
	IMA_XATTR_DIGEST = 0x01,
	EVM_XATTR_HMAC,
	EVM_IMA_XATTR_DIGSIG,
	IMA_XATTR_DIGEST_NG,
	IMA_XATTR_LAST
};

So the only value is 3 for an event with signature.

> 
> Question:  Is the signature algorithm encoded here, or anywhere, or is RSA assumed?
> 

I guess the version field could identify the algorithm, currently
asymmetric_verify function defined in security/integrity/digsig_asymmetric.c
uses the signature_v2_hdr and RSA is hardcoded as:

pks.pkey_algo = "rsa";

> 3.4.2. Signature Version
> 
> This is a 1-byte field.  The value is 0x02.
> 
> Question:  What are the valid values and meanings.  How does the version affect the other fields.

Only 1 and 2 are valid right now. In function integrity_digsig_verify:

switch (sig[1]) {
case 1:
	/* v1 API expect signature without xattr type */
	return digsig_verify(keyring[id], sig + 1, siglen - 1,
			     digest, digestlen);
case 2:
	return asymmetric_verify(keyring[id], sig, siglen,
				 digest, digestlen);
}

So, for 1, it skips over the xattr type (EVM_IMA_XATTR_DIGSIG).
The rest is interpreted by digsig_verify in lib/digsig.c as of type
struct signature_hdr defined in include/linux/digsig.h

struct signature_hdr {
	uint8_t		version;	/* signature format version */
	uint32_t	timestamp;	/* signature made */
	uint8_t		algo;
	uint8_t		hash;
	uint8_t		keyid[8];
	uint8_t		nmpi;
	char		mpi[0];
} __packed;

> 
> 3.4.3. Hash Algorithm
> 
> This is a 1-byte field representing the hash algorithm used for the File Data Hash.  The legal values are:
> 
> * 0x02: SHA-1
> * 0x04: SHA-256

these are defined by the crypto subsystem in 
include/uapi/linux/hash_info.h:

enum hash_algo {
	HASH_ALGO_MD4,
	HASH_ALGO_MD5,
	HASH_ALGO_SHA1,
	HASH_ALGO_RIPE_MD_160,
	HASH_ALGO_SHA256,
	HASH_ALGO_SHA384,
	HASH_ALGO_SHA512,
	HASH_ALGO_SHA224,
	HASH_ALGO_RIPE_MD_128,
	HASH_ALGO_RIPE_MD_256,
	HASH_ALGO_RIPE_MD_320,
	HASH_ALGO_WP_256,
	HASH_ALGO_WP_384,
	HASH_ALGO_WP_512,
	HASH_ALGO_TGR_128,
	HASH_ALGO_TGR_160,
	HASH_ALGO_TGR_192,
	HASH_ALGO_SM3_256,
	HASH_ALGO__LAST
};

> 
> Note that there is redundancy, in that this field must be consistent with the Hash Algorithm field on the Template Data.
> 
> 3.4.4. Public Key Identifier
> 
> This is a 4-byte field that identifies the public key.  It is the last 4 bytes of the key's X.509 certificate Subject Key Identifier.
> 
> 3.4.5. Signature Size
> 
> This is a 2-byte integer representing the size of the Signature field.
> 
> Question:  What are the legal values?  1024 and 2048?  Others?

This one is defined by the signature. For RSA, the key determines the
resulting signature size. When writing the xattr, it is set to the return value
of RSA_private_encrypt:

From: https://linux.die.net/man/3/rsa_private_encrypt
>#include <openssl/rsa.h>
>
>int RSA_private_encrypt(int flen, unsigned char *from,
>   unsigned char *to, RSA *rsa, int padding);
>
>RSA_private_encrypt() returns the size of the signature
>(i.e., RSA_size(rsa)).

From: https://linux.die.net/man/3/rsa_size
>#include <openssl/rsa.h>
>
>int RSA_size(const RSA *rsa);
>
>This function returns the RSA modulus size in bytes. It can be 
>used to determine how much memory must be allocated for an
>RSA encrypted value.

Mehmet

[Linux-ima-devel] IMA SHA-1 log format "specification"

[Ken G. <kg...@li...>]

While writing an attestation server, I found that I had to write an IMA 
log format parser (now open source).  Below is an informal specification 
based on the best information I have.

1 - I offer it for anyone who may be able to use it.

2 - There are questions throughout the text.  If anyone would like to
offer an answer, please do it on the list so everyone can validate it.

It probably makes sense to send one answer per post with a good subject 
line, in case it starts a thread.

3 - The below text was created from a Word document.  If anyone wants 
the original, it is a bit easier to read - different fonts, hyperlinks, 
the usual word processor things.

~~~


IMA Event Log Format

Ken Goldman
kgo...@us...
July 13, 2017





1.	Introduction	2
2.	IMA Event	2
2.1.	PCR Index	2
2.2.	Template Hash	3
2.3.	Template Name Length	3
2.4.	Template Name	3
2.5.	Template Data Length	3
2.6.	Template Data	3
3.	Template Data	4
3.1.	Hash Length	4
3.2.	Hash Algorithm	4
3.3.	File Data Hash	5
3.1.	File Name Length	5
3.2.	File Name	5
3.3.	Signature Length	5
3.4.	Signature Header	5
3.4.1.	Signature Type	5
3.4.2.	Signature Version	6
3.4.3.	Hash Algorithm	6
3.4.4.	Public Key Identifier	6
3.4.5.	Signature Size	6
3.5.	Signature	6
1. Introduction

This details the IMA event log format, field by field.

Multi-byte integer values (PCR index, length, etc.) are in the byte 
order of the host where the event log was created.  The receiver can 
determine the byte order by looking at the PCR Index field, or the 
sender can convert to network byte order before transmission.

Fields are always concatenated with no padding.

Question:  What is the maximum number of event records?
2. IMA Event

An IMA event record has the following fields.

2.1. PCR Index

This is a 4-byte integer representing the PCR Index.

Note:  The value is currently always 10.
2.2.  Template Hash

This is a 20-byte SHA-1 hash of the Template Data field.

There is no associated length or descriptor.

The Template Hash is used directly in the PCR 10 extend operation.
2.3. Template Name Length

This is a 4-byte integer representing the length of the Template Name field.

Question:  What is the maximum length?

2.4. Template Name

This is a printable string representing the template name.

The string is NOT nul terminated.  It is guaranteed to be printable.

For legal names, see section 3 Template Data.

2.5. Template Data Length

This is a 4-byte integer representing the length of the Template Data field.

Note that there is redundancy, in that the ima-sig data fields are 
self-describing.  This can be checked for consistency.

2.6. Template Data

See Section 3 Template Data for the contents of this field.


3. Template Data

Template data can have the following fields.  Unless specified, each is 
preceded by a 4-byte length.

d	20-byte digest, SHA-1 or zero padded MD-5 (no length)
d-ng	hash algorithm + digest
n	256-byte nul terminated file name (no length)
n-ng	file name
sig	signature header + signature
uuid	(undocumented)
pid	(undocumented)
ppid	(undocumented)
gid	(undocumented)

The template names become (where | is the concatenation symbol)

ima		d | n
ima-ng		d-ng | n-ng
ima-sig		d-ng | n-ng | sig

3.1. Hash Length

This is a 4-byte integer representing the combined length of the Hash 
Algorithm and File Data Hash fields.  Note that the File Data Hash 
contains the two following fields, and those fields do not have explicit 
lengths.

3.2. Hash Algorithm

This is a nul terminated string representing the hash algorithm of the 
File Data Hash field.  Two values are currently used:

* sha256:
* sha1:

Note this redundancy, which can be checked for consistency:

* The Hash Length minus the length of the Hash Algorithm field 
(including the nul terminator) yields the size of the File Data Hash.
* The length of a hash based on the Hash Algorithm yields the size of 
the File Data Hash.

3.3. File Data Hash

This is a hash of the file data that was used to compute the Signature.

The length and hash algorithm are determined by the Hash Algorithm field.

3.1. File Name Length

This is a 4-byte integer representing the length of the file name, 
including the nul terminator.  The maximum value is MAXPATHLEN +1?

Note that there is redundancy, in that the file name is nul terminated. 
This can be checked for consistency.

3.2. File Name

This is a nul terminated string representing the name of the file that 
was measured.

The length is determined by the File Name Length.

3.3. Signature Length

This is a 4-byte integer representing the total length of the Signature 
Header and Signature fields.  The value may be zero, indicating that 
those two fields are not present.

3.4. Signature Header

This field is fixed at 9 bytes, consisting of 5 fields.
3.4.1. Signature Type

This is a 1-byte field.  The value is 0x03.

Question:  What are the valid values and meanings.  How does the type 
affect the other fields.

Question:  Is the signature algorithm encoded here, or anywhere, or is 
RSA assumed?

3.4.2. Signature Version

This is a 1-byte field.  The value is 0x02.

Question:  What are the valid values and meanings.  How does the version 
affect the other fields.

3.4.3. Hash Algorithm

This is a 1-byte field representing the hash algorithm used for the File 
Data Hash.  The legal values are:

* 0x02: SHA-1
* 0x04: SHA-256

Note that there is redundancy, in that this field must be consistent 
with the Hash Algorithm field on the Template Data.

3.4.4. Public Key Identifier

This is a 4-byte field that identifies the public key.  It is the last 4 
bytes of the key's X.509 certificate Subject Key Identifier.

3.4.5. Signature Size

This is a 2-byte integer representing the size of the Signature field.

Question:  What are the legal values?  1024 and 2048?  Others?

3.5. Signature

This field represents the signature over the File Data Hash using the 
key specified by the Public Key Identifier and the hash algorithm 
represented by the (two) Hash Algorithm fields.





	Page 2	

Re: [Linux-ima-devel] [RFC 2/4] ima: use namespaced flags for IMA_AUDITED on each namespace

[Mimi Z. <zo...@li...>]

On Mon, 2017-07-17 at 14:35 +0000, Magalhaes, Guilherme (Brazil R&DCL)
wrote:
> Mimi,
> Let me update you on the ideas below. 
> First, on the 2-lookups solution, we have one radix tree inside
> another. On the first radix tree, the index is namespace id, which
> seems ok (not sparse). The second radix tree is indexed by iint
> address, which must be replaced by a rbtree as you indicated.
> The second point is about the possible alternative solution I
> mentioned. Actually, I realized that we would have possible memory
> leaks on that solution since it would not be possible to delete the
> namespaced flags when a namespace is released. It would wait when
> the namespace is reused for the same iint and then it would finally
> reset the flags under that namespace. Then, the namespaced flags
> would never be deleted (to avoid the need for locking the inode when
> the namespace is released).
> 
> Therefore, we still have 2 options:
> 1) namespaced flags tree inside the iint structure, single lookup,
> but when namespace is released it would have to lock and unlock each
> related inode accessed under that namespace to safely delete the
> namespaced flag. There can be hundred/thousands of iints namespaced
> flags to delete.

Definitely not a good idea.

> 2) namespaced flags tree outside the iint structure, we would need 2
> lookups when a namespace flag is referenced.

There needs to be two look ups, just not two tree look ups.

Mimi

Re: [Linux-ima-devel] [RFC 2/4] ima: use namespaced flags for IMA_AUDITED on each namespace

[Magalhaes, G. (B. R&D-CL) <gui...@hp...>]

Mimi,
Let me update you on the ideas below. 
First, on the 2-lookups solution, we have one radix tree inside another. On the first radix tree, the index is namespace id, which seems ok (not sparse). The second radix tree is indexed by iint address, which must be replaced by a rbtree as you indicated.
The second point is about the possible alternative solution I mentioned. Actually, I realized that we would have possible memory leaks on that solution since it would not be possible to delete the namespaced flags when a namespace is released. It would wait when the namespace is reused for the same iint and then it would finally reset the flags under that namespace. Then, the namespaced flags would never be deleted (to avoid the need for locking the inode when the namespace is released).

Therefore, we still have 2 options:
1) namespaced flags tree inside the iint structure, single lookup, but when namespace is released it would have to lock and unlock each related inode accessed under that namespace to safely delete the namespaced flag. There can be hundred/thousands of iints namespaced flags to delete.
2) namespaced flags tree outside the iint structure, we would need 2 lookups when a namespace flag is referenced.

--
Guilherme

-----Original Message-----
From: Magalhaes, Guilherme (Brazil R&D-CL) 
Sent: quarta-feira, 12 de julho de 2017 10:57
To: Mimi Zohar <zo...@li...>
Cc: lin...@li...
Subject: Re: [Linux-ima-devel] [RFC 2/4] ima: use namespaced flags for IMA_AUDITED on each namespace

Hi Mimi,

>Originally IMA used a radix tree for storing the iint information, but 
>later it was replace with a red-black tree. The reasons for using a 
>red-black tree instead are explained in commit 8549164 "IMA: use rbtree 
>instead of radix tree for inode information cache" patch description.

I reviewed the log on this commit and these are the most relevant parts:
     "... uses a radix tree for indexing.
      It uses the *address* of the struct inode as the indexing key.  That
      means the key space is extremely sparse. ... for XFS the struct inode
      addresses are approximately 1000 bytes apart, which means the closest
      the radix tree index keys get is ~1000.  ..., so the radix tree is using
      roughly 550 bytes for every 120byte structure being cached..."
Basically it says that using radix tree would not make an optimized use of memory in case the key values are sparse. On the case of XFS inode address, the values would always be at least 1000 apart. Now, the values used as key on the solution we propose are namespace ids, which are typically incremented by one and easily reused when released. Then our solution doesn't seem to suffer the memory footprint inefficiency described on this commit log. This is one aspect, I don't have a strong/full comparison to decide among radix tree and rbtree, so please advise if rbtree is still the preferable data structure. 

> We really shouldn't need two radix tree lookups to get the namespaced iint flags.
>Finding the namespaced iint information should be very similar to 
>finding the iint itself.  Both should be based on the pointer to the 
>inode.  I could see namespacing the function integrity_find_iint().

I checked the source code of the posted POC (from Mehmet) and it confirmed our understanding of namespacing IMA in general. 
For the discussion about how many lookups are required to retrieve the namespaced iint flags, we actually have two possible solutions. The first solution is the list of namespace flags inside the iint structure (the 'status' in the posted POC), which requires just one more lookup, and the second is the list of namespace flags outside the iint structure, which requires two more lookups (first by namespace id then by iint). 
The chosen solution must handle file change, file deletion and namespace release. 
Name space release may happen concurrently to file change or deletion. When a namespace is released, the namespaced iint flags must be deleted or cleared. Now, when the namespaced iint flags are inside the iint structure and a namespace is released, the namespaced flags deletion may conflict with the deletion of a related inode. That means the inode must be locked (each one in the list of iints in the namespace structure) during the processes of releasing the namespace so the flags can be safely cleared.
So, reviewing the POC code:

+void ima_free_ns_status(struct ima_namespace *ns) {
+	struct ns_status *current_status;
+	struct ns_status *next_status;
+
+	list_for_each_entry_safe(current_status, next_status,
+				 &ns->iint_list, iint_next) {
+		if (!list_empty(&current_status->ns_next))
+			list_del_rcu(&current_status->ns_next);   <--
+		synchronize_rcu();
+		list_del_rcu(&current_status->iint_next);
+		...	
+	}
+}

Deleting 'current_status->ns_next' may impact 'iint->ns_list' field, which can be referenced or even deleted concurrently and then the inode lock should be held (for each respective inode/iint). Take into consideration that the iint may be reused after file deletion which increases the conflict above. For this reason we proceeded to the second solution (flags outside iint structure) which requires two more lookups.
However, we believe another option could avoid the two additional lookups. We store the namespaced flags inside the iint structure, but we don't delete the flags when a namespace is released. We would rely in a sequence/version number stored along with the namespaced flags list which would indicate the version of the current namespace. The version of the namespace would be bumped up whenever the namespace is released and then reused.
If the versions do not match (namespace structure version and iint ns_list version), it means the ns_list in the iint structure is old and the flags are not reliable. The namespaced flags (inside iint) are then all deleted at this time and its version is updated.
Fortunately, the mnt_namespace structure (fs/mount.h) already has this 'seq' number, but we should make this field public to access it in the IMA code. 
If this solution (including the change in fs/namespace.c) is acceptable, this is then another option. Otherwise, the 2-lookups solution seems the only safe option.

--
Guilherme

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________
Linux-ima-devel mailing list
Lin...@li...
https://lists.sourceforge.net/lists/listinfo/linux-ima-devel

Re: [Linux-ima-devel] [PATCH v3 2/4] ima: use fs method to read integrity data

[Mimi Z. <zo...@li...>]

On Thu, 2017-07-13 at 18:03 +0200, Jan Kara wrote:
> On Thu 13-07-17 09:54:48, Mimi Zohar wrote:
> > diff --git a/fs/ext2/file.c b/fs/ext2/file.c
> > index b21891a6bfca..d57c4259945d 100644
> > --- a/fs/ext2/file.c
> > +++ b/fs/ext2/file.c
> > @@ -219,6 +219,7 @@ const struct file_operations ext2_file_operations = {
> >  	.get_unmapped_area = thp_get_unmapped_area,
> >  	.splice_read	= generic_file_splice_read,
> >  	.splice_write	= iter_file_splice_write,
> > +	.integrity_read	= generic_file_read_iter,
> >  };
> >  
> >  const struct inode_operations ext2_file_inode_operations = {
> > diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> > index 831fd6beebf0..e7b2bd43cdc4 100644
> > --- a/fs/ext4/file.c
> > +++ b/fs/ext4/file.c
> > @@ -753,6 +753,7 @@ const struct file_operations ext4_file_operations = {
> >  	.splice_read	= generic_file_splice_read,
> >  	.splice_write	= iter_file_splice_write,
> >  	.fallocate	= ext4_fallocate,
> > +	.integrity_read	= ext4_file_read_iter,
> >  };
> 
> I think both ext2 and ext4 need a bit more special handling (similarly to
> XFS) due to DAX. E.g. ext4_dax_read_iter() will try to get i_rwsem which is
> wrong for integrity_read handler as far as I understand.

True, thanks.  This will be addressed in the next version.

> > index c2062a108d19..9b49d09ba180 100644
> > --- a/fs/gfs2/file.c
> > +++ b/fs/gfs2/file.c
> > @@ -1124,6 +1124,7 @@ const struct file_operations gfs2_file_fops = {
> >  	.splice_write	= gfs2_file_splice_write,
> >  	.setlease	= simple_nosetlease,
> >  	.fallocate	= gfs2_fallocate,
> > +	.integrity_read	= generic_file_read_iter,
> >  };
> >  
> >  const struct file_operations gfs2_dir_fops = {
> > @@ -1152,6 +1153,7 @@ const struct file_operations gfs2_file_fops_nolock = {
> >  	.splice_write	= gfs2_file_splice_write,
> >  	.setlease	= generic_setlease,
> >  	.fallocate	= gfs2_fallocate,
> > +	.integrity_read	= generic_file_read_iter,
> >  };
> >  
> >  const struct file_operations gfs2_dir_fops_nolock = {
> ...
> > diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> > index bfeb647459d9..2832a7c92acd 100644
> > --- a/fs/ocfs2/file.c
> > +++ b/fs/ocfs2/file.c
> > @@ -2536,6 +2536,7 @@ const struct file_operations ocfs2_fops = {
> >  	.fallocate	= ocfs2_fallocate,
> >  	.clone_file_range = ocfs2_file_clone_range,
> >  	.dedupe_file_range = ocfs2_file_dedupe_range,
> > +	.integrity_read	= ocfs2_file_read_iter,
> >  };
> >  
> >  const struct file_operations ocfs2_dops = {
> 
> For cluster filesystems like gfs2 or ocfs2 I actually wonder whether IMA
> works as it should - without special cluster locking another node may be
> modifying the file while you read it even when you hold i_rwsem. So don't
> these filesystems need some special treatment?

Both ocf2 and gfs2 can be created as a local filesystem, not only as
part of a cluster.  As part of a clustered node, you're probably
right.  

Mimi

Re: [Linux-ima-devel] [PATCH v3 2/4] ima: use fs method to read integrity data

[Jan K. <ja...@su...>]

On Thu 13-07-17 09:54:48, Mimi Zohar wrote:
> diff --git a/fs/ext2/file.c b/fs/ext2/file.c
> index b21891a6bfca..d57c4259945d 100644
> --- a/fs/ext2/file.c
> +++ b/fs/ext2/file.c
> @@ -219,6 +219,7 @@ const struct file_operations ext2_file_operations = {
>  	.get_unmapped_area = thp_get_unmapped_area,
>  	.splice_read	= generic_file_splice_read,
>  	.splice_write	= iter_file_splice_write,
> +	.integrity_read	= generic_file_read_iter,
>  };
>  
>  const struct inode_operations ext2_file_inode_operations = {
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 831fd6beebf0..e7b2bd43cdc4 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -753,6 +753,7 @@ const struct file_operations ext4_file_operations = {
>  	.splice_read	= generic_file_splice_read,
>  	.splice_write	= iter_file_splice_write,
>  	.fallocate	= ext4_fallocate,
> +	.integrity_read	= ext4_file_read_iter,
>  };

I think both ext2 and ext4 need a bit more special handling (similarly to
XFS) due to DAX. E.g. ext4_dax_read_iter() will try to get i_rwsem which is
wrong for integrity_read handler as far as I understand.

> index c2062a108d19..9b49d09ba180 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -1124,6 +1124,7 @@ const struct file_operations gfs2_file_fops = {
>  	.splice_write	= gfs2_file_splice_write,
>  	.setlease	= simple_nosetlease,
>  	.fallocate	= gfs2_fallocate,
> +	.integrity_read	= generic_file_read_iter,
>  };
>  
>  const struct file_operations gfs2_dir_fops = {
> @@ -1152,6 +1153,7 @@ const struct file_operations gfs2_file_fops_nolock = {
>  	.splice_write	= gfs2_file_splice_write,
>  	.setlease	= generic_setlease,
>  	.fallocate	= gfs2_fallocate,
> +	.integrity_read	= generic_file_read_iter,
>  };
>  
>  const struct file_operations gfs2_dir_fops_nolock = {
...
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index bfeb647459d9..2832a7c92acd 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -2536,6 +2536,7 @@ const struct file_operations ocfs2_fops = {
>  	.fallocate	= ocfs2_fallocate,
>  	.clone_file_range = ocfs2_file_clone_range,
>  	.dedupe_file_range = ocfs2_file_dedupe_range,
> +	.integrity_read	= ocfs2_file_read_iter,
>  };
>  
>  const struct file_operations ocfs2_dops = {

For cluster filesystems like gfs2 or ocfs2 I actually wonder whether IMA
works as it should - without special cluster locking another node may be
modifying the file while you read it even when you hold i_rwsem. So don't
these filesystems need some special treatment?

								Honza
-- 
Jan Kara <ja...@su...>
SUSE Labs, CR

[Linux-ima-devel] [PATCH v3 2/4] ima: use fs method to read integrity data

[Mimi Z. <zo...@li...>]

From: Christoph Hellwig <hc...@ls...>

Add a new ->integrity_read file operation to read data for integrity
hash collection.  This is defined to be equivalent to ->read_iter,
except that it will be called with the i_rwsem held exclusively.

Signed-off-by: Christoph Hellwig <hc...@ls...>
Cc: Matthew Garrett <mat...@ne...>
Cc: Jan Kara <ja...@su...>
Cc: "Theodore Ts'o" <ty...@mi...>
Cc: Andreas Dilger <adi...@di...>
Cc: Jaegeuk Kim <ja...@ke...>
Cc: Chao Yu <yu...@hu...>
Cc: Steven Whitehouse <swh...@re...>
Cc: Bob Peterson <rpe...@re...>
Cc: David Woodhouse <dw...@in...>
Cc: Dave Kleikamp <sh...@ke...>
Cc: Ryusuke Konishi <kon...@la...>
Cc: Mark Fasheh <mf...@ve...>
Cc: Joel Becker <jl...@ev...>
Cc: Richard Weinberger <ri...@no...>
Cc: "Darrick J. Wong" <dar...@or...>
Cc: Hugh Dickins <hu...@go...>
Cc: Chris Mason <cl...@fb...>
Signed-off-by: Mimi Zohar <zo...@li...>
---
Changelog v3:
- define simple_read_iter_from_buffer as suggested by Al Viro
- replace the existing efivarfs ->read method with ->read_iter method.
- squashed other fs definitions of ->integrity_read with this patch.
- Cc'ing maintainers
- moved '---' divider before change log, as requested in patch review

Changelog v2:
- change iovec to kvec

Changelog v1:
- update the patch description, removing the concept that the presence of
->integrity_read indicates that the file system can support IMA. (Mimi)

 fs/btrfs/file.c           |  1 +
 fs/efivarfs/file.c        | 12 +++++++-----
 fs/ext2/file.c            |  1 +
 fs/ext4/file.c            |  1 +
 fs/f2fs/file.c            |  1 +
 fs/gfs2/file.c            |  2 ++
 fs/jffs2/file.c           |  1 +
 fs/jfs/file.c             |  1 +
 fs/libfs.c                | 32 ++++++++++++++++++++++++++++++++
 fs/nilfs2/file.c          |  1 +
 fs/ocfs2/file.c           |  1 +
 fs/ramfs/file-mmu.c       |  1 +
 fs/ramfs/file-nommu.c     |  1 +
 fs/ubifs/file.c           |  1 +
 fs/xfs/xfs_file.c         | 21 +++++++++++++++++++++
 include/linux/fs.h        |  3 +++
 mm/shmem.c                |  1 +
 security/integrity/iint.c | 20 ++++++++++++++------
 18 files changed, 91 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index da1096eb1a40..003e859b56c4 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -3087,6 +3087,7 @@ const struct file_operations btrfs_file_operations = {
 #endif
 	.clone_file_range = btrfs_clone_file_range,
 	.dedupe_file_range = btrfs_dedupe_file_range,
+	.integrity_read = generic_file_read_iter,
 };
 
 void btrfs_auto_defrag_exit(void)
diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index 5f22e74bbade..17955a92a5b3 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -64,9 +64,10 @@ static ssize_t efivarfs_file_write(struct file *file,
 	return bytes;
 }
 
-static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
-		size_t count, loff_t *ppos)
+static ssize_t efivarfs_file_read_iter(struct kiocb *iocb,
+				       struct iov_iter *iter)
 {
+	struct file *file = iocb->ki_filp;
 	struct efivar_entry *var = file->private_data;
 	unsigned long datasize = 0;
 	u32 attributes;
@@ -96,8 +97,8 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
 		goto out_free;
 
 	memcpy(data, &attributes, sizeof(attributes));
-	size = simple_read_from_buffer(userbuf, count, ppos,
-				       data, datasize + sizeof(attributes));
+	size = simple_read_iter_from_buffer(iocb, iter, data,
+					    datasize + sizeof(attributes));
 out_free:
 	kfree(data);
 
@@ -174,8 +175,9 @@ efivarfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long p)
 
 const struct file_operations efivarfs_file_operations = {
 	.open	= simple_open,
-	.read	= efivarfs_file_read,
+	.read_iter = efivarfs_file_read_iter,
 	.write	= efivarfs_file_write,
 	.llseek	= no_llseek,
 	.unlocked_ioctl = efivarfs_file_ioctl,
+	.integrity_read	= efivarfs_file_read_iter,
 };
diff --git a/fs/ext2/file.c b/fs/ext2/file.c
index b21891a6bfca..d57c4259945d 100644
--- a/fs/ext2/file.c
+++ b/fs/ext2/file.c
@@ -219,6 +219,7 @@ const struct file_operations ext2_file_operations = {
 	.get_unmapped_area = thp_get_unmapped_area,
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
+	.integrity_read	= generic_file_read_iter,
 };
 
 const struct inode_operations ext2_file_inode_operations = {
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 831fd6beebf0..e7b2bd43cdc4 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -753,6 +753,7 @@ const struct file_operations ext4_file_operations = {
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= ext4_fallocate,
+	.integrity_read	= ext4_file_read_iter,
 };
 
 const struct inode_operations ext4_file_inode_operations = {
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 61af721329fa..e93fdeb3eba4 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2406,4 +2406,5 @@ const struct file_operations f2fs_file_operations = {
 #endif
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
+	.integrity_read	= generic_file_read_iter,
 };
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index c2062a108d19..9b49d09ba180 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -1124,6 +1124,7 @@ const struct file_operations gfs2_file_fops = {
 	.splice_write	= gfs2_file_splice_write,
 	.setlease	= simple_nosetlease,
 	.fallocate	= gfs2_fallocate,
+	.integrity_read	= generic_file_read_iter,
 };
 
 const struct file_operations gfs2_dir_fops = {
@@ -1152,6 +1153,7 @@ const struct file_operations gfs2_file_fops_nolock = {
 	.splice_write	= gfs2_file_splice_write,
 	.setlease	= generic_setlease,
 	.fallocate	= gfs2_fallocate,
+	.integrity_read	= generic_file_read_iter,
 };
 
 const struct file_operations gfs2_dir_fops_nolock = {
diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index c12476e309c6..5a63034cccf5 100644
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -57,6 +57,7 @@ const struct file_operations jffs2_file_operations =
 	.mmap =		generic_file_readonly_mmap,
 	.fsync =	jffs2_fsync,
 	.splice_read =	generic_file_splice_read,
+	.integrity_read = generic_file_read_iter,
 };
 
 /* jffs2_file_inode_operations */
diff --git a/fs/jfs/file.c b/fs/jfs/file.c
index 739492c7a3fd..423512a810e4 100644
--- a/fs/jfs/file.c
+++ b/fs/jfs/file.c
@@ -162,4 +162,5 @@ const struct file_operations jfs_file_operations = {
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= jfs_compat_ioctl,
 #endif
+	.integrity_read	= generic_file_read_iter,
 };
diff --git a/fs/libfs.c b/fs/libfs.c
index a04395334bb1..e1b4f8695013 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -16,6 +16,7 @@
 #include <linux/exportfs.h>
 #include <linux/writeback.h>
 #include <linux/buffer_head.h> /* sync_mapping_buffers */
+#include <linux/uio.h>
 
 #include <linux/uaccess.h>
 
@@ -676,6 +677,37 @@ ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
 EXPORT_SYMBOL(simple_write_to_buffer);
 
 /**
+ * simple_read_iter_from_buffer - copy data from the buffer to user space
+ * @iocb: struct containing the file, the current position and other info
+ * @to: the user space buffer to read to
+ * @from: the buffer to read from
+ * @available: the size of the buffer
+ *
+ * The simple_read_iter_from_buffer() function reads up to @available bytes
+ * from the current buffer into the user space buffer.
+ *
+ * On success, the current buffer offset is advanced by the number of bytes
+ * read, or a negative value is returned on error.
+ **/
+ssize_t simple_read_iter_from_buffer(struct kiocb *iocb, struct iov_iter *to,
+                                const void *from, size_t available)
+{
+	loff_t pos = iocb->ki_pos;
+	size_t ret;
+
+	if (pos < 0)
+		return -EINVAL;
+	if (pos >= available)
+		return 0;
+	ret = copy_to_iter(from + pos, available - pos, to);
+	if (!ret && iov_iter_count(to))
+		return -EFAULT;
+	iocb->ki_pos = pos + ret;
+	return ret;
+}
+EXPORT_SYMBOL(simple_read_iter_from_buffer);
+
+/**
  * memory_read_from_buffer - copy data from the buffer
  * @to: the kernel space buffer to read to
  * @count: the maximum number of bytes to read
diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
index c5fa3dee72fc..55e058ac487f 100644
--- a/fs/nilfs2/file.c
+++ b/fs/nilfs2/file.c
@@ -150,6 +150,7 @@ const struct file_operations nilfs_file_operations = {
 	/* .release	= nilfs_release_file, */
 	.fsync		= nilfs_sync_file,
 	.splice_read	= generic_file_splice_read,
+	.integrity_read	= generic_file_read_iter,
 };
 
 const struct inode_operations nilfs_file_inode_operations = {
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index bfeb647459d9..2832a7c92acd 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2536,6 +2536,7 @@ const struct file_operations ocfs2_fops = {
 	.fallocate	= ocfs2_fallocate,
 	.clone_file_range = ocfs2_file_clone_range,
 	.dedupe_file_range = ocfs2_file_dedupe_range,
+	.integrity_read	= ocfs2_file_read_iter,
 };
 
 const struct file_operations ocfs2_dops = {
diff --git a/fs/ramfs/file-mmu.c b/fs/ramfs/file-mmu.c
index 12af0490322f..4f24d1b589b1 100644
--- a/fs/ramfs/file-mmu.c
+++ b/fs/ramfs/file-mmu.c
@@ -47,6 +47,7 @@ const struct file_operations ramfs_file_operations = {
 	.splice_write	= iter_file_splice_write,
 	.llseek		= generic_file_llseek,
 	.get_unmapped_area	= ramfs_mmu_get_unmapped_area,
+	.integrity_read	= generic_file_read_iter,
 };
 
 const struct inode_operations ramfs_file_inode_operations = {
diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
index 2ef7ce75c062..5ee704fa84e0 100644
--- a/fs/ramfs/file-nommu.c
+++ b/fs/ramfs/file-nommu.c
@@ -50,6 +50,7 @@ const struct file_operations ramfs_file_operations = {
 	.splice_read		= generic_file_splice_read,
 	.splice_write		= iter_file_splice_write,
 	.llseek			= generic_file_llseek,
+	.integrity_read		= generic_file_read_iter,
 };
 
 const struct inode_operations ramfs_file_inode_operations = {
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 2cda3d67e2d0..eeb4e872e47a 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1754,4 +1754,5 @@ const struct file_operations ubifs_file_operations = {
 #ifdef CONFIG_COMPAT
 	.compat_ioctl   = ubifs_compat_ioctl,
 #endif
+	.integrity_read = generic_file_read_iter,
 };
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 35703a801372..3d6ace2a79bc 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -288,6 +288,26 @@ xfs_file_read_iter(
 	return ret;
 }
 
+static ssize_t
+xfs_integrity_read(
+	struct kiocb		*iocb,
+	struct iov_iter		*to)
+{
+	struct inode		*inode = file_inode(iocb->ki_filp);
+	struct xfs_mount	*mp = XFS_I(inode)->i_mount;
+
+	lockdep_assert_held(&inode->i_rwsem);
+
+	XFS_STATS_INC(mp, xs_read_calls);
+
+	if (XFS_FORCED_SHUTDOWN(mp))
+		return -EIO;
+
+	if (IS_DAX(inode))
+		return dax_iomap_rw(iocb, to, &xfs_iomap_ops);
+	return generic_file_read_iter(iocb, to);
+}
+
 /*
  * Zero any on disk space between the current EOF and the new, larger EOF.
  *
@@ -1534,6 +1554,7 @@ const struct file_operations xfs_file_operations = {
 	.fallocate	= xfs_file_fallocate,
 	.clone_file_range = xfs_file_clone_range,
 	.dedupe_file_range = xfs_file_dedupe_range,
+	.integrity_read	= xfs_integrity_read,
 };
 
 const struct file_operations xfs_dir_file_operations = {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 803e5a9b2654..d85d2c43afd9 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1690,6 +1690,7 @@ struct file_operations {
 			u64);
 	ssize_t (*dedupe_file_range)(struct file *, u64, u64, struct file *,
 			u64);
+	ssize_t (*integrity_read)(struct kiocb *, struct iov_iter *);
 };
 
 struct inode_operations {
@@ -3011,6 +3012,8 @@ extern void simple_release_fs(struct vfsmount **mount, int *count);
 
 extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
 			loff_t *ppos, const void *from, size_t available);
+extern ssize_t simple_read_iter_from_buffer(struct kiocb *iocb,
+		struct iov_iter *to, const void *from, size_t available);
 extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
 		const void __user *from, size_t count);
 
diff --git a/mm/shmem.c b/mm/shmem.c
index e67d6ba4e98e..16958b20946f 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3846,6 +3846,7 @@ static const struct file_operations shmem_file_operations = {
 	.splice_read	= generic_file_splice_read,
 	.splice_write	= iter_file_splice_write,
 	.fallocate	= shmem_fallocate,
+	.integrity_read	= shmem_file_read_iter,
 #endif
 };
 
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 6fc888ca468e..df04f35a1d40 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -21,6 +21,7 @@
 #include <linux/rbtree.h>
 #include <linux/file.h>
 #include <linux/uaccess.h>
+#include <linux/uio.h>
 #include "integrity.h"
 
 static struct rb_root integrity_iint_tree = RB_ROOT;
@@ -184,18 +185,25 @@ security_initcall(integrity_iintcache_init);
 int integrity_kernel_read(struct file *file, loff_t offset,
 			  void *addr, unsigned long count)
 {
-	mm_segment_t old_fs;
-	char __user *buf = (char __user *)addr;
+	struct inode *inode = file_inode(file);
+	struct kvec iov = { .iov_base = addr, .iov_len = count };
+	struct kiocb kiocb;
+	struct iov_iter iter;
 	ssize_t ret;
 
+	lockdep_assert_held(&inode->i_rwsem);
+
 	if (!(file->f_mode & FMODE_READ))
 		return -EBADF;
+	if (!file->f_op->integrity_read)
+		return -EBADF;
 
-	old_fs = get_fs();
-	set_fs(get_ds());
-	ret = __vfs_read(file, buf, count, &offset);
-	set_fs(old_fs);
+	init_sync_kiocb(&kiocb, file);
+	kiocb.ki_pos = offset;
+	iov_iter_kvec(&iter, READ | ITER_KVEC, &iov, 1, count);
 
+	ret = file->f_op->integrity_read(&kiocb, &iter);
+	BUG_ON(ret == -EIOCBQUEUED);
 	return ret;
 }
 
-- 
2.7.4

[Linux-ima-devel] [PATCH v3 4/4] ima: define "fs_unsafe" builtin policy

[Mimi Z. <zo...@li...>]

Permit normally denied access/execute permission for files in policy
on IMA unsupported filesystems.  This patch defines "fs_unsafe", a
builtin policy.

Mimi Zohar <zo...@li...>

Changelog v3:
- include dont_failsafe rule when displaying policy
---
 Documentation/admin-guide/kernel-parameters.txt |  8 +++++++-
 security/integrity/ima/ima_policy.c             | 12 ++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e438a1fca554..0aed01aabc16 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1478,7 +1478,7 @@
 
 	ima_policy=	[IMA]
 			The builtin policies to load during IMA setup.
-			Format: "tcb | appraise_tcb | secure_boot"
+			Format: "tcb | appraise_tcb | secure_boot | fs_unsafe"
 
 			The "tcb" policy measures all programs exec'd, files
 			mmap'd for exec, and all files opened with the read
@@ -1493,6 +1493,12 @@
 			of files (eg. kexec kernel image, kernel modules,
 			firmware, policy, etc) based on file signatures.
 
+			The "fs_unsafe" policy permits normally denied
+			access/execute permission for files in policy on IMA
+			unsupported filesystems.  Note this option, as the
+			name implies, is not safe and not recommended for
+			any environments other than testing.
+
 	ima_tcb		[IMA] Deprecated.  Use ima_policy= instead.
 			Load a policy which meets the needs of the Trusted
 			Computing Base.  This means IMA will measure all
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 3c29aa4e7980..6b77b890ff96 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -169,6 +169,10 @@ static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
 };
 
+static struct ima_rule_entry dont_failsafe_rules[] __ro_after_init = {
+	{.action = DONT_FAILSAFE}
+};
+
 static LIST_HEAD(ima_default_rules);
 static LIST_HEAD(ima_policy_rules);
 static LIST_HEAD(ima_temp_rules);
@@ -188,6 +192,7 @@ __setup("ima_tcb", default_measure_policy_setup);
 
 static bool ima_use_appraise_tcb __initdata;
 static bool ima_use_secure_boot __initdata;
+static bool ima_use_dont_failsafe __initdata;
 static int __init policy_setup(char *str)
 {
 	char *p;
@@ -201,6 +206,10 @@ static int __init policy_setup(char *str)
 			ima_use_appraise_tcb = 1;
 		else if (strcmp(p, "secure_boot") == 0)
 			ima_use_secure_boot = 1;
+		else if (strcmp(p, "fs_unsafe") == 0) {
+			ima_use_dont_failsafe = 1;
+			set_failsafe(0);
+		}
 	}
 
 	return 1;
@@ -470,6 +479,9 @@ void __init ima_init_policy(void)
 			temp_ima_appraise |= IMA_APPRAISE_POLICY;
 	}
 
+	if (ima_use_dont_failsafe)
+		list_add_tail(&dont_failsafe_rules[0].list, &ima_default_rules);
+
 	ima_rules = &ima_default_rules;
 	ima_update_policy_flag();
 }
-- 
2.7.4

[Linux-ima-devel] [PATCH v3 3/4] ima: define "dont_failsafe" policy action rule

[Mimi Z. <zo...@li...>]

Permit normally denied access/execute permission for files in policy
on IMA unsupported filesystems.  This patch defines the "dont_failsafe"
policy action rule.

Mimi Zohar <zo...@li...>
---
Changelog v3:
- include dont_failsafe rule when displaying policy
- fail attempt to add dont_failsafe rule when appending to the policy

 Documentation/ABI/testing/ima_policy |  3 ++-
 security/integrity/ima/ima.h         |  1 +
 security/integrity/ima/ima_main.c    | 11 ++++++++++-
 security/integrity/ima/ima_policy.c  | 29 ++++++++++++++++++++++++++++-
 4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index e76432b9954d..f271207743e5 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -17,7 +17,8 @@ Description:
 
 		rule format: action [condition ...]
 
-		action: measure | dont_measure | appraise | dont_appraise | audit
+		action: measure | dont_meaure | appraise | dont_appraise |
+			audit | dont_failsafe
 		condition:= base | lsm  [option]
 			base:	[[func=] [mask=] [fsmagic=] [fsuuid=] [uid=]
 				[euid=] [fowner=]]
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487ad259..c5f34f7c5b0f 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -224,6 +224,7 @@ void *ima_policy_start(struct seq_file *m, loff_t *pos);
 void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos);
 void ima_policy_stop(struct seq_file *m, void *v);
 int ima_policy_show(struct seq_file *m, void *v);
+void set_failsafe(bool flag);
 
 /* Appraise integrity measurements */
 #define IMA_APPRAISE_ENFORCE	0x01
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 63777d1210b1..59e271a20600 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -38,6 +38,11 @@ int ima_appraise;
 int ima_hash_algo = HASH_ALGO_SHA1;
 static int hash_setup_done;
 
+static bool ima_failsafe = 1;
+void set_failsafe(bool flag) {
+	ima_failsafe = flag;
+}
+
 static int __init hash_setup(char *str)
 {
 	struct ima_template_desc *template_desc = ima_template_desc_current();
@@ -263,8 +268,12 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 		__putname(pathbuf);
 out:
 	inode_unlock(inode);
-	if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE))
+	if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+		if (!ima_failsafe && rc == -EBADF)
+			return 0;
+
 		return -EACCES;
+	}
 	return 0;
 }
 
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index f4436626ccb7..3c29aa4e7980 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -40,12 +40,14 @@
 #define APPRAISE	0x0004	/* same as IMA_APPRAISE */
 #define DONT_APPRAISE	0x0008
 #define AUDIT		0x0040
+#define DONT_FAILSAFE	0x0400
 
 #define INVALID_PCR(a) (((a) < 0) || \
 	(a) >= (FIELD_SIZEOF(struct integrity_iint_cache, measured_pcrs) * 8))
 
 int ima_policy_flag;
 static int temp_ima_appraise;
+static bool temp_failsafe = 1;
 
 #define MAX_LSM_RULES 6
 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,
@@ -513,6 +515,9 @@ void ima_update_policy(void)
 	if (ima_rules != policy) {
 		ima_policy_flag = 0;
 		ima_rules = policy;
+
+		/* Only update on initial policy replacement, not append */
+		set_failsafe(temp_failsafe);
 	}
 	ima_update_policy_flag();
 }
@@ -529,7 +534,7 @@ enum {
 	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
 	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
 	Opt_appraise_type, Opt_permit_directio,
-	Opt_pcr
+	Opt_pcr, Opt_dont_failsafe
 };
 
 static match_table_t policy_tokens = {
@@ -560,6 +565,7 @@ static match_table_t policy_tokens = {
 	{Opt_appraise_type, "appraise_type=%s"},
 	{Opt_permit_directio, "permit_directio"},
 	{Opt_pcr, "pcr=%s"},
+	{Opt_dont_failsafe, "dont_failsafe"},
 	{Opt_err, NULL}
 };
 
@@ -630,6 +636,11 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 		if ((*p == '\0') || (*p == ' ') || (*p == '\t'))
 			continue;
 		token = match_token(p, policy_tokens, args);
+		if (entry->action == DONT_FAILSAFE) {
+			/* no args permitted, force invalid rule */
+			token = Opt_dont_failsafe;
+		}
+
 		switch (token) {
 		case Opt_measure:
 			ima_log_string(ab, "action", "measure");
@@ -671,6 +682,19 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 
 			entry->action = AUDIT;
 			break;
+		case Opt_dont_failsafe:
+			ima_log_string(ab, "action", "dont_failsafe");
+
+			if (entry->action != UNKNOWN)
+				result = -EINVAL;
+
+			/* Permit on initial policy replacement only */
+			if (ima_rules != &ima_policy_rules)
+				temp_failsafe = 0;
+			else
+				result = -EINVAL;
+			entry->action = DONT_FAILSAFE;
+			break;
 		case Opt_func:
 			ima_log_string(ab, "func", args[0].from);
 
@@ -951,6 +975,7 @@ void ima_delete_rules(void)
 	int i;
 
 	temp_ima_appraise = 0;
+	temp_failsafe = 1;
 	list_for_each_entry_safe(entry, tmp, &ima_temp_rules, list) {
 		for (i = 0; i < MAX_LSM_RULES; i++)
 			kfree(entry->lsm[i].args_p);
@@ -1042,6 +1067,8 @@ int ima_policy_show(struct seq_file *m, void *v)
 		seq_puts(m, pt(Opt_dont_appraise));
 	if (entry->action & AUDIT)
 		seq_puts(m, pt(Opt_audit));
+	if (entry->action & DONT_FAILSAFE)
+		seq_puts(m, pt(Opt_dont_failsafe));
 
 	seq_puts(m, " ");
 
-- 
2.7.4