Linux Security Modules development
 help / color / mirror / Atom feed
* RE: [RFC PATCH 0/1] security: add SECURE_KEEP_FSUID to preserve fsuid/fsgid across execve
From: James Morris @ 2019-06-15  3:53 UTC (permalink / raw)
  To: Lubashev, Igor
  Cc: Serge Hallyn, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <720751180a9543cfa205cd527248df7c@ustx2ex-dag1mb5.msg.corp.akamai.com>

On Sat, 15 Jun 2019, Lubashev, Igor wrote:

> > On Friday, June 14, 2019, James Morris wrote:

> Unfortunately, perf is using uid==0 and euid==0 as a "capability bits".
>
> 
> In tools/perf/util/evsel.c:
> 	static bool perf_event_can_profile_kernel(void)
> 	{
> 		return geteuid() == 0 || perf_event_paranoid() == -1;
> 	}
> 
> In tools/perf/util/symbol.c:
> 	static bool symbol__read_kptr_restrict(void)
> 	{
> 	...
> 		value = ((geteuid() != 0) || (getuid() != 0)) ?
> 				(atoi(line) != 0) :
> 				(atoi(line) == 2);
> 	...
> 	}

These are bugs. They should be checking for CAP_SYS_ADMIN.


> 
> > Have you considered the example security configuration in
> > Documentation/admin-guide/perf-security.rst ?
> 
> Unfortunately, this configuration does not work, unless you reset 
> /proc/sys/kernel/perf_event_paranoid to a permissive level (see code 
> above). We have perf_event_paranoid set to 2. If it worked, we could had 
> implemented the same capability-based policy in the wrapper.

This is not necessary for a process which has CAP_SYS_ADMIN.


-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply

* RE: [RFC PATCH 0/1] security: add SECURE_KEEP_FSUID to preserve fsuid/fsgid across execve
From: Lubashev, Igor @ 2019-06-15  1:16 UTC (permalink / raw)
  To: James Morris
  Cc: Serge Hallyn, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <alpine.LRH.2.21.1906141445010.7150@namei.org>

> On Friday, June 14, 2019, James Morris wrote:
> On Thu, 13 Jun 2019, Igor Lubashev wrote:
> 
> > I've posted this in March but received no response. Reposting.
> >
> > This patch introduces SECURE_KEEP_FSUID to allow fsuid/fsgid to be
> > preserved across execve. It is currently impossible to execve a
> > program such that effective and filesystem uid differ.
> >
> > The need for this functionality arose from a desire to allow certain
> > non-privileged users to run perf. To do this, we install perf without
> > set-uid-root and have a set-uid-root wrapper decide who is allowed to
> > run perf (and with what arguments).
> >
> > The wrapper must execve perf with real and effective root uid, because
> > perf and KASLR require this. However, that presently resets fsuid to
> > root, giving the user ability to read and overwrite any file owned by
> > root (perf report -i, perf record -o). Also, perf record will create
> > perf.data that cannot be deleted by the user.
> >
> > We cannot reset /proc/sys/kernel/perf_event_paranoid to a permissive
> > level, since we must be selective which users have the permissions.
> >
> > Of course, we could fix our problem by a patch to perf to allow
> > passing a username on the command line and having perf execute
> > setfsuid before opening files. However, perf is not the only program
> > that uses kernel features that require root uid/euid, so a general
> > solution that does not involve updating all such programs seems
> > warranted.

> This seems like a very specific corner case, depending on fsuid!=0 for an
> euid=0 process, along with a whitelist policy for perf arguments. It would be a
> great way to escalate to root via a bug in an executed app or via a wrapper
> misconfiguration.

Any set-uid-root app is a hazard.  This wrapper's purpose is to reduce the risk inherent in running apps with elevated privs.
It removes all capabilities (CAT_SETUID, CAT_SETPCAP, CAP_DAC_OVERRIDE, etc.) except for the required ones before execve().  It has a list of users allowed run apps (and a per-user whitelist of arguments, and it manages resources and time used by apps).

The wrapper works great for things like tcpdump -- it is executed with the user's uid and just CAP_NET_CAP and CAP_NET_ADMIN set.

Unfortunately, perf is using uid==0 and euid==0 as a "capability bits".

In tools/perf/util/evsel.c:
	static bool perf_event_can_profile_kernel(void)
	{
		return geteuid() == 0 || perf_event_paranoid() == -1;
	}

In tools/perf/util/symbol.c:
	static bool symbol__read_kptr_restrict(void)
	{
	...
		value = ((geteuid() != 0) || (getuid() != 0)) ?
				(atoi(line) != 0) :
				(atoi(line) == 2);
	...
	}

> Have you considered the example security configuration in
> Documentation/admin-guide/perf-security.rst ?

Unfortunately, this configuration does not work, unless you reset /proc/sys/kernel/perf_event_paranoid to a permissive level (see code above). We have perf_event_paranoid set to 2. If it worked, we could had implemented the same capability-based policy in the wrapper.


> It also adds complexity to kernel credential handling -- it's yet another thing
> to consider when trying to reason about this.

I really wish that there were only two concepts that mattered: capability sets and fsuid/fsgid. The proposed patch allows one to switch to such mode -- a much simpler mode. Yes, the patch does add a "new feature", but what matters most for the complexity question is whether this feature is a move in the right direction.  I am leaning that way, but I am not 100% positive -- hence this RFC patch.


> What are some other examples of programs that could utilize this scheme?

That's everything, like our wrapper, that needs to allow non-root users to run apps (like perf) that use uid/euid as capabilities.  It is a required, if the apps could interact with a filesystem (and accessing root-owned files is not a desired effect).  It is a good idea from the security perspective even if those apps do not normally interact with a filesystem.

I do not have a clear view about a variety of Linux apps ever written, but I suspect that there are many apps that fall into "use uid/euid as capabilities" category.  There are at least two in the kernel's tools directory. There is also use of uid/eiud in the kernel itself, and anything that uses this functionality cannot be fixed w/o fixing the kernel. It may be a bit hard to find all such uses, but a good start is:

	grep -rE '(uid_eq|uid\(\)).*\b(GLOBAL_ROOT_ID|0)\b'

- Igor

^ permalink raw reply

* Re: [PATCH] Smack: Restore the smackfsdef mount option and add missing prefixes
From: Linus Torvalds @ 2019-06-15  0:24 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: James Morris, David Howells, Al Viro, stable, Jose Bollo,
	LSM List, Linux List Kernel Mailing
In-Reply-To: <cb3749a6-e45b-3e07-27f9-841adf6f4640@schaufler-ca.com>

On Fri, Jun 14, 2019 at 1:08 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Al, are you going to take this, or should I find another way
> to get it in for 5.2?

I guess I can take it directly.

I was assuming it would come through either Al (which is how I got the
commit it fixes) or Casey (as smack maintainer), so I ignored the
patch.

                 Linus

^ permalink raw reply

* Re: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Dr. Greg @ 2019-06-14 23:19 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Stephen Smalley, Cedric Xing, linux-security-module, selinux,
	linux-kernel, linux-sgx, jarkko.sakkinen, luto, jmorris, serge,
	paul, eparis, jethro, dave.hansen, tglx, torvalds, akpm, nhorman,
	pmccallum, serge.ayoun, shay.katz-zamir, haitao.huang,
	andriy.shevchenko, kai.svahn, bp, josh, kai.huang, rientjes,
	william.c.roberts, philip.b.tricca
In-Reply-To: <20190614004600.GF18385@linux.intel.com>

On Thu, Jun 13, 2019 at 05:46:00PM -0700, Sean Christopherson wrote:

Good afternoon, I hope the week is ending well for everyone.

> On Thu, Jun 13, 2019 at 01:02:17PM -0400, Stephen Smalley wrote:
> > Given the complexity tradeoff, what is the clear motivating
> > example for why #1 isn't the obvious choice? That the enclave
> > loader has no way of knowing a priori whether the enclave will
> > require W->X or WX?  But aren't we better off requiring enclaves
> > to be explicitly marked as needing such so that we can make a more
> > informed decision about whether to load them in the first place?

> Andy and/or Cedric, can you please weigh in with a concrete (and
> practical) use case that will break if we go with #1?  The auditing
> issues for #2/#3 are complex to say the least...

So we are back to choosing door 1, door 2 or door 3.

That brings us back to our previous e-mail, where we suggested that
the most fundamental question to answer with the LSM issue is how much
effective security is being purchased at what complexity cost.

We are practical guys at our company, we direct the development and
deployment of practical SGX systems, including an independent
implementation of SGX runtime/attestation/provisioning et.al.  Our
comments, for whatever they are worth, are meant to reflect the real
world deployment of this technology.

Lets start big picture.

One of the clients we are consulting with on this technology is
running well north of 1400 Linux systems.  Every one of which has
selinux=0 in /proc/cmdline and will do so until approximately the heat
death of the Universe.

Our AI LSM will use any SGX LSM driver hooks that eventuate from these
discussions, so we support the notion of the LSM getting a look at
permissions of executable code.  However, our client isn't unique in
their configuration choice, so we believe this fact calls the question
as to how much SGX specific complexity should be injected into the
LSM.

So, as we noted in our previous e-mail, there are only two relevant
security questions the LSM needs to answer:

1.) Should a page of memory with executable content be allowed into an
enclave?

2.) Should an enclave be allowed to possess one or more pages of
executable memory which will have WX permissions sometime during its
lifetime?

Sean is suggesting the strategy of an ioctl to call out pages that
conform to question 2 (EAUG'ed pages).  That doesn't seem like an
onerous requirement, since all of the current enclave loaders already
have all of the metadata infrastructure to map/load page ranges.  The
EAUG WX range would simply be another layout type that gets walked
over when the enclave image is built.

Given that, we were somewhat surprised to hear Sean say that he had
been advised that door 1 was a non-starter.  Presumably this was
because of the need to delineate a specific cohort of pages that will
be permitted WX.  If that is the case, the question that needs to be
called, as Stephen alludes to above, is whether or not WX privileges
should be considered a characterizing feature of the VMA that defines
an enclave rather then a per page attribute.

Do we realistically believe that an LSM will be implemented that
reacts differently when the 357th page of WX memory is added as
opposed to the first?  The operative security question is whether or
not the platform owner is willing to allow arbitrary executable code,
that they may have no visibility into, to be executed on their
platform.

We talk to people that, as a technology, SGX is about building
'security archipelagos', islands of trusted execution on potentially
multiple platforms that interact to deliver a service, all of which
consider their surrounding platforms and the network in between them
as adversarial.  This model is, by definition, adverserial to the
notion and function of the LSM.

With respect to SGX dynamic code loading, the future for security
concious architectures, will be to pull the code from remotely
attested repository servers over the network.  The only relevant
security question that can be answered is whether or not a platform
owner feels comfortable with that model.

Best wishes for a pleasant weekend to everyone.

Dr. Greg

As always,
Dr. G.W. Wettstein, Ph.D.   Enjellic Systems Development, LLC.
4206 N. 19th Ave.           Specializing in information infra-structure
Fargo, ND  58102            development.
PH: 701-281-1686
FAX: 701-281-3949           EMAIL: greg@enjellic.com
------------------------------------------------------------------------------
"My spin on the meeting?  I lie somewhere between the individual who
 feels that we are all going to join hands and march forward carrying
 the organization into the information age and Dr. Wettstein.  Who
 feels that they are holding secret meetings at 6 o'clock in the
 morning plotting strategy on how to replace our system."
                                -- Paul S. Etzell, M.D.
                                   Medical Director, Roger Maris Cancer Center

^ permalink raw reply

* Re: [PATCH] Smack: Restore the smackfsdef mount option and add missing prefixes
From: Casey Schaufler @ 2019-06-14 23:08 UTC (permalink / raw)
  To: James Morris, David Howells, viro
  Cc: stable, Jose Bollo, torvalds, linux-security-module, linux-kernel,
	casey
In-Reply-To: <6cfd5113-8473-f962-dee7-e490e6f76f9c@schaufler-ca.com>

On 6/3/2019 4:07 PM, Casey Schaufler wrote:
> On 6/3/2019 3:42 PM, James Morris wrote:
>> On Fri, 31 May 2019, David Howells wrote:
>>
>>> Should this go via Al's tree, James's tree, Casey's tree or directly to Linus?
>> If it's specific to one LSM (as this is), via Casey, who can decide to 
>> forward to Al or Linus.
> I would very much appreciate it if Al could send this fix along.
> I am not fully set up for sending directly to Linus.

Al, are you going to take this, or should I find another way
to get it in for 5.2?


^ permalink raw reply

* Re: [PATCH v4 05/28] docs: cgroup-v1: convert docs to ReST and rename to *.rst
From: Tejun Heo @ 2019-06-14 20:30 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Mauro Carvalho Chehab, Linux Doc Mailing List,
	Mauro Carvalho Chehab, linux-kernel, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, x86, Jens Axboe, Li Zefan,
	Johannes Weiner, Alexei Starovoitov, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, James Morris,
	Serge E. Hallyn, linux-block, cgroups, netdev, bpf,
	linux-security-module
In-Reply-To: <20190614141401.48bfb266@lwn.net>

On Fri, Jun 14, 2019 at 02:14:01PM -0600, Jonathan Corbet wrote:
> On Wed, 12 Jun 2019 14:52:41 -0300
> Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> 
> > Convert the cgroup-v1 files to ReST format, in order to
> > allow a later addition to the admin-guide.
> > 
> > The conversion is actually:
> >   - add blank lines and identation in order to identify paragraphs;
> >   - fix tables markups;
> >   - add some lists markups;
> >   - mark literal blocks;
> >   - adjust title markups.
> > 
> > At its new index.rst, let's add a :orphan: while this is not linked to
> > the main index.rst file, in order to avoid build warnings.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > Acked-by: Tejun Heo <tj@kernel.org>
> 
> This one, too, has linux-next stuff that keeps it from applying to
> docs-next.  Tejun, would you like to carry it on top of your work?

Applied to cgroup/for-5.3.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH v4 05/28] docs: cgroup-v1: convert docs to ReST and rename to *.rst
From: Jonathan Corbet @ 2019-06-14 20:14 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	x86, Jens Axboe, Tejun Heo, Li Zefan, Johannes Weiner,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, James Morris, Serge E. Hallyn, linux-block,
	cgroups, netdev, bpf, linux-security-module
In-Reply-To: <c1dd623359f44f05863456b8bceba0d8f3e42f38.1560361364.git.mchehab+samsung@kernel.org>

On Wed, 12 Jun 2019 14:52:41 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:

> Convert the cgroup-v1 files to ReST format, in order to
> allow a later addition to the admin-guide.
> 
> The conversion is actually:
>   - add blank lines and identation in order to identify paragraphs;
>   - fix tables markups;
>   - add some lists markups;
>   - mark literal blocks;
>   - adjust title markups.
> 
> At its new index.rst, let's add a :orphan: while this is not linked to
> the main index.rst file, in order to avoid build warnings.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> Acked-by: Tejun Heo <tj@kernel.org>

This one, too, has linux-next stuff that keeps it from applying to
docs-next.  Tejun, would you like to carry it on top of your work?

(This means I won't be able to apply #6 either, naturally).

Thanks,

jon

^ permalink raw reply

* Re: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Sean Christopherson @ 2019-06-14 20:01 UTC (permalink / raw)
  To: Xing, Cedric
  Cc: Stephen Smalley, linux-security-module@vger.kernel.org,
	selinux@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-sgx@vger.kernel.org, jarkko.sakkinen@linux.intel.com,
	luto@kernel.org, jmorris@namei.org, serge@hallyn.com,
	paul@paul-moore.com, eparis@parisplace.org, jethro@fortanix.com,
	Hansen, Dave, tglx@linutronix.de, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, nhorman@redhat.com,
	pmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
	Huang, Haitao, andriy.shevchenko@linux.intel.com, Svahn, Kai,
	bp@alien8.de, josh@joshtriplett.org, Huang, Kai,
	rientjes@google.com, Roberts, William C, Tricca, Philip B
In-Reply-To: <20190614175339.GA29126@linux.intel.com>

On Fri, Jun 14, 2019 at 10:53:39AM -0700, Sean Christopherson wrote:
> On Fri, Jun 14, 2019 at 10:45:56AM -0700, Sean Christopherson wrote:
> > The state tracking of #2/#3 doesn't scare me, it's purely the auditing.
> > Holding an audit message for an indeterminate amount of time is a
> > nightmare.
> > 
> > Here's a thought.  What if we simply require FILE__EXECUTE or AA_EXEC_MAP
> > to load any enclave page from a file?  Alternatively, we could add an SGX
> > specific file policity, e.g. FILE__ENCLAVELOAD and AA_MAY_LOAD_ENCLAVE.
> > As in my other email, SELinux's W^X restrictions can be tied to the process,
> > i.e. they can be checked at mmap()/mprotect() without throwing a wrench in
> > auditing.
> 
> We would also need to require VM_MAYEXEC on all enclave pages, or forego
> enforcing path_noexec() for enclaves.

Scratch that thought.   Tying W^X restrictions to the process only works
if its done at load time.  E.g. If process A maps a page W and process B
maps the same page X, then which process needs W^X depends on the order of
mmap()/mprotect() between the two processes.

^ permalink raw reply

* [PATCH v4 14/14] ima: add Documentation/security/IMA-digest-lists.txt
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

This patch adds the documentation of the IMA Digest Lists extension.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/security/IMA-digest-lists.txt | 226 ++++++++++++++++++++
 1 file changed, 226 insertions(+)
 create mode 100644 Documentation/security/IMA-digest-lists.txt

diff --git a/Documentation/security/IMA-digest-lists.txt b/Documentation/security/IMA-digest-lists.txt
new file mode 100644
index 000000000000..dd9633e21304
--- /dev/null
+++ b/Documentation/security/IMA-digest-lists.txt
@@ -0,0 +1,226 @@
+==========================
+IMA Digest Lists Extension
+==========================
+
+INTRODUCTION
+============
+
+Integrity Measurement Architecture (IMA) is a security module that performs
+measurement of files accessed with the execve(), mmap() and open() system
+calls. File measurements can be used for different purposes. They can be
+added in a measurement list and sent to a remote verifier for an integrity
+evaluation. They can be compared to reference values provided by the
+software vendor and access can be denied if there is a mismatch. File
+measurements can also be included in system logs for auditing purposes.
+
+IMA Digest Lists is an extension providing additional functionality for
+the IMA submodules. Its main task is to load reference file digests in the
+kernel memory, and to communicate to IMA submodules (measurement and
+appraisal) whether the digest of a file being accessed is found in the
+uploaded digest lists.
+
+The IMA-Measure submodule uses the IMA Digest Lists extension to create
+a new measurement list (with a different PCR, for example 11) which
+contains the measurement of uploaded digest lists and unknown files. Since
+the loading of digest lists is sequential, the chosen PCR will have a
+predictable value for the whole boot cycle and can be used for sealing
+policies based on the OS software integrity.
+
+Both the standard and the new measurement list can be generated at the same
+time, allowing for implicit attestation based on the usage of a TPM key
+sealed to the OS, and for explicit attestation when a more precise
+integrity evaluation is necessary.
+
+The IMA-Appraise submodule uses the IMA Digest Lists extension to
+grant/deny access to the files depending on whether the file digest is
+found in the uploaded digest lists, instead of checking security.ima.
+
+The main advantage of the extension is that reference measurements used for
+the digest comparison can be extracted from already existing sources (for
+example RPM headers). Another benefit is that the overhead compared to file
+signatures is lower as only one signature is verified for all files
+included in the digest list. A disadvantage is that file metadata are not
+verified.
+
+
+
+WORKFLOW
+========
+
+The IMA workflow is modified as follows. The added steps are marked as
+[NEW].
+
+  +------------+
+  |  IMA hook  |
+  +------------+
+        |
+   +---------+
+   | collect |
+   +---------+
+        |                                           +---------------------+
++---------------+                       ------------| don't measure [NEW] |
+| digest lookup |                       | yes       +---------------------+
+|     [NEW]     |                 -------------- no +---------------------+
++---------------+           -----/ digest       \---| add to measurement  |
+        |                   |    \ found? [NEW] /   | list (PCR 11) [NEW] |
+        |                   |     --------------    +---------------------+
+  +----------+       +-------------+                +--------------------+
+  |  switch  |-------| IMA-Measure |----------------| add to measurement |
+  | (action) |       +-------------+                | list (PCR 10)      |
+  +----------+                                      +--------------------+
+        |
+        |
+        |       no xattr
++--------------+     ---------------  yes
+| IMA-Appraise |----/ filec created \-------------------------------
++--------------+    \ created [NEW] /                              |
+  xattr |            ---------------                               |
+        |                   | no                                   |
+        |           ------------------  no  --------------  yes  +--------+
+        |          / EVM initialized? \----/ digest       \------| grant  |
+        |          \ [NEW]            /    \ found? [NEW] /      | access |
+        |           ------------------      --------------       +--------+
+        |                   | yes                 | no
+        |                   |                     |              +--------+
+        |           -------------------  no       ---------------| deny   |
+        |          / digest-nometadata \-------------------------| access |
+        |          \ mode? [NEW]       /                         +--------+
+        |           -------------------
+        |               ^   | yes                                +--------+
+        |               |   -------------------------------------| grant  |
+        |               |                                        | access |
+        |               |                                        +--------+
+        |               | yes
+        |           -------------------------  no                +--------+
+        |----------/ security.ima (new type) \-------------------| deny   |
+        |          \ present and valid?      /                   | access |
+        |           -------------------------                    +--------+
+        |                                                            |
+        |           --------------------------  no                   |
+        -----------/ security.ima (cur types) \-----------------------
+                   \ present and valid?       /
+                    --------------------------
+                             | yes                               +--------+
+                             ------------------------------------| grant  |
+                                                                 | access |
+                                                                 +--------+
+
+After the file digest is calculated, it is searched in the hash table
+containing all digests extracted from the uploaded digest lists. Then, if
+the digest is found, a structure is returned to IMA with information
+associated to that digest. The structure is returned only to the IMA
+submodules that processed the digest lists (i.e. the action returned by
+ima_get_action() was 'measure' or 'appraise').
+
+IMA-Measure behavior depends on whether the digest list PCR has been
+specified in the kernel command line. If the PCR was not specified, the
+submodule behaves as before. If the PCR was specified, IMA-Measure creates
+a new measurement with that PCR, only if the file digest is not found in
+the digest lists. It additionally creates a measurement with the default
+PCR if '+' is added as a prefix to the PCR.
+
+IMA-Appraise behavior depends on whether either the 'digest' or
+'digest-nometadata' appraisal modes have been specified in the kernel
+command line. If they were not specified, IMA-Appraise relies solely on the
+security.ima xattr for verification. If the 'digest' mode was specified,
+verification succeeds if the file digest is found in the digest lists and
+EVM is not initialized, as there is no other way to verify file metadata.
+
+If the 'digest-nometadata' mode was specified, verification succeeds
+regardless of the fact that EVM is initialized or not. However, after a
+write, files for which access was granted without verifying metadata will
+have a new security.ima type, so that they can be identified also after
+reboot. Specifying 'digest-nometadata' is required also to access files
+with the new security.ima type.
+
+
+
+ARCHITECTURE
+============
+
++--------------+           8) digest lookup
+| IMA-Appraise |--------------------------------------
++--------------+                                     |
+                                                     |
++-------------+            7) digest lookup          |
+| IMA-Measure | -------------------------------------|
++-------------+                                     ||
+                                                   |||
++-------------+ 2a/5b) parse compact list +-------------------+
+| IMA (secfs) | ------------------------> | IMA Digests Lists |
++-------------+                           +-------------------+
+   ^        ^                                 |     | 3a/6b add digests
+   |        |                                 |  +------------+
+   |        |                                 |  | hash table |
+   |        |                                 |  +------------+
+   |        | 4b) upload compact list         |
+   |        |                                 |                kernel space
+---|--------|---------------------------------|-------------------------------
+   |        | 1b) open digest_list_data       |                user space
+   | +-------------+                          |
+   | |    Parser   |<--------------------------
+   | +-------------+   2b) check parser binary, shared libraries
+   |     ^  ^  ^           and actions (measure/appraise)
+   |    RPM | ...      3b) read and convert digest list
+   |     manifest
+   |
+1a) upload compact list
+
+The main addition to IMA is a new hash table (similar to that used to check
+for duplicate measurement entries), which contains file digests extracted
+from the digest lists.
+
+File digests can be uploaded to IMA through a new securityfs file named
+'digest_list_data' and must be sent in a format called compact list. Digest
+lists can be uploaded directly to the kernel, by specifying their path, or
+by executing a user space parser.
+
+The parser, called upload_digest_lists, converts digest lists from the
+original format defined by the software vendor (e.g. RPM package header) to
+the compact list format. It is executed by the kernel even before init,
+so that the hash table is populated before any file is accessed.
+
+Converted digest lists can be uploaded exclusively by the parser. The
+reason is that digest lists are measured only when they are accessed. If
+they were uploaded by any process there wouldn't be any guarantee that the
+converted lists contain the same digests as the original list. IMA checks
+that the digest of the executable that opened 'digest_list_data' as well as
+shared libraries supporting new digest list formats have type
+COMPACT_PARSER.
+
+As mentioned above, digest lists cannot be used unconditionally but must
+have been processed by the IMA submodule willing to use them. This is
+achieved by keeping track of the process that opens and closes
+digest_list_data, and checking the actions done by that process. If a file
+opened by the parser is not being measured, IMA-Measure will not be able
+to use digest lists until reboot. The same happens for IMA-Appraise.
+
+
+
+CONFIGURATION
+=============
+
+The first step consists in generating digest lists with the
+gen_digest_lists tool included in the digest-list-tools package.
+digest-list-tools can be retrieved at the URL:
+
+https://github.com/euleros/digest-list-tools
+
+gen_digest_lists can generate digest lists from different sources (for
+example: RPM package DB and IMA measurement list). By default, it saves
+generated digest lists in the /etc/ima/digest_lists directory.
+digest-list-tools includes also two bash scripts setup_ima_digest_lists and
+setup_digest_lists_demo to simplify the digest list generation for the
+users.
+
+To use the parser at boot, it is also necessary to generate a digest list
+for it. It is automatically generated by the setup_ima_digest_lists script,
+or it must be manually generated otherwise. If digest lists are used for
+appraisal, they must be signed with a key saved to /etc/keys/x509_ima.der
+and the key must be signed with a key in the primary or secondary kernel
+keyring.
+
+To use digest lists during early boot, it is necessary to regenerate the
+initial ram disk. Digest lists and the parser will be included in the ram
+disk by the new dracut/initramfs-tools modules, included in the
+digest-list-tools package.
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 13/14] ima: introduce new policies initrd and appraise_initrd
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

This patch introduces the new policies 'initrd' and 'appraise_initrd' to
measure/appraise files in the initial ram disk.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../admin-guide/kernel-parameters.txt         |  5 +++-
 security/integrity/ima/ima_policy.c           | 26 +++++++++++++++++--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 765682b4187d..47311cdf63d9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1619,7 +1619,7 @@
 	ima_policy=	[IMA]
 			The builtin policies to load during IMA setup.
 			Format: "tcb | appraise_tcb | secure_boot |
-				 fail_securely"
+				 fail_securely | initrd | appraise_initrd"
 
 			The "tcb" policy measures all programs exec'd, files
 			mmap'd for exec, and all files opened with the read
@@ -1638,6 +1638,9 @@
 			filesystems with the SB_I_UNVERIFIABLE_SIGNATURE
 			flag.
 
+			The "initrd" and "appraise_initrd" policies include
+			rootfs among the filesystems to be measured/appraised.
+
 	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 5537b91272f0..70412df07718 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -165,6 +165,14 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = {
 #endif
 };
 
+static struct ima_rule_entry initrd_measure_rule __ro_after_init = {
+	.action = MEASURE, .fsname = "rootfs", .flags = IMA_FSNAME
+};
+
+static struct ima_rule_entry initrd_appraise_rule __ro_after_init = {
+	.action = APPRAISE, .fsname = "rootfs", .flags = IMA_FSNAME
+};
+
 static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
 #ifdef CONFIG_IMA_APPRAISE_REQUIRE_MODULE_SIGS
 	{.action = APPRAISE, .func = MODULE_CHECK,
@@ -218,6 +226,8 @@ __setup("ima_tcb", default_measure_policy_setup);
 static bool ima_use_appraise_tcb __initdata;
 static bool ima_use_secure_boot __initdata;
 static bool ima_fail_unverifiable_sigs __ro_after_init;
+static bool ima_measure_initrd __initdata;
+static bool ima_appraise_initrd __initdata;
 static int __init policy_setup(char *str)
 {
 	char *p;
@@ -233,6 +243,10 @@ static int __init policy_setup(char *str)
 			ima_use_secure_boot = true;
 		else if (strcmp(p, "fail_securely") == 0)
 			ima_fail_unverifiable_sigs = true;
+		else if (strcmp(p, "initrd") == 0)
+			ima_measure_initrd = true;
+		else if (strcmp(p, "appraise_initrd") == 0)
+			ima_appraise_initrd = true;
 	}
 
 	return 1;
@@ -640,9 +654,13 @@ void __init ima_init_policy(void)
 	int build_appraise_entries, arch_entries;
 
 	/* if !ima_policy, we load NO default rules */
-	if (ima_policy)
+	if (ima_policy) {
+		if (ima_measure_initrd)
+			add_rules(&initrd_measure_rule, 1, IMA_DEFAULT_POLICY);
+
 		add_rules(dont_measure_rules, ARRAY_SIZE(dont_measure_rules),
 			  IMA_DEFAULT_POLICY);
+	}
 
 	switch (ima_policy) {
 	case ORIGINAL_TCB:
@@ -695,10 +713,14 @@ void __init ima_init_policy(void)
 				  IMA_DEFAULT_POLICY | IMA_CUSTOM_POLICY);
 	}
 
-	if (ima_use_appraise_tcb)
+	if (ima_use_appraise_tcb) {
+		if (ima_appraise_initrd)
+			add_rules(&initrd_appraise_rule, 1, IMA_DEFAULT_POLICY);
+
 		add_rules(default_appraise_rules,
 			  ARRAY_SIZE(default_appraise_rules),
 			  IMA_DEFAULT_POLICY);
+	}
 
 	ima_rules = &ima_default_rules;
 	ima_update_policy_flag();
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 12/14] ima: add support for appraisal with digest lists
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

IMA-Appraise grants access to files with a valid signature or with actual
file digest equal to the digest included in security.ima.

This patch adds support for appraisal based on digest lists. Instead of
using the reference value from security.ima, this patch checks if the
calculated file digest is included in the uploaded digest lists.

This functionality must be explicitly enabled by providing one of the
following values for the ima_appraise= kernel option:

- digest: this mode enables appraisal verification with digest lists until
  EVM is initialized; after that, access to files without signature/HMAC
  will be denied; this mode would be used for systems that perform HMAC
  verification and rely on digest lists for the system initialization until
  the HMAC key is unsealed;

- digest-nometadata: this mode enables appraisal verification with digest
  lists even after EVM has been initialized; access will be granted if
  a file does not have IMA/EVM xattrs but its digest is found in the digest
  lists; after update, mutable files will have a new security.ima type, so
  that they can be made inaccessible unless this mode is specified; this
  mode would be used for systems that perform appraisal verification
  exclusively based on digest lists.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../admin-guide/kernel-parameters.txt         |  3 +-
 include/linux/evm.h                           |  6 +++
 security/integrity/evm/evm_main.c             |  2 +-
 security/integrity/ima/ima.h                  |  5 ++-
 security/integrity/ima/ima_appraise.c         | 37 ++++++++++++++++++-
 security/integrity/ima/ima_main.c             |  4 +-
 security/integrity/integrity.h                |  2 +
 7 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 05862a0c402d..765682b4187d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1588,7 +1588,8 @@
 
 	ima_appraise=	[IMA] appraise integrity measurements
 			Format: { "off" | "enforce" | "fix" | "log" |
-				  "enforce-evm" | "log-evm" }
+				  "enforce-evm" | "log-evm" | "digest" |
+				  "digest-nometadata" }
 			default: "enforce"
 
 	ima_appraise_tcb [IMA] Deprecated.  Use ima_policy= instead.
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 8302bc29bb35..6e89d046b716 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -15,6 +15,7 @@
 struct integrity_iint_cache;
 
 #ifdef CONFIG_EVM
+extern bool evm_key_loaded(void);
 extern int evm_set_key(void *key, size_t keylen);
 extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
 					     const char *xattr_name,
@@ -45,6 +46,11 @@ static inline int posix_xattr_acl(const char *xattrname)
 #endif
 #else
 
+static inline bool evm_key_loaded(void)
+{
+	return false;
+}
+
 static inline int evm_set_key(void *key, size_t keylen)
 {
 	return -EOPNOTSUPP;
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index abb5ce3fd942..fbe0bb5cd7c4 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -86,7 +86,7 @@ static void __init evm_init_config(void)
 	pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
 }
 
-static bool evm_key_loaded(void)
+bool evm_key_loaded(void)
 {
 	return (bool)(evm_initialized & EVM_KEY_MASK);
 }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 42e4f2b64cd1..16952df04cb6 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -248,7 +248,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 			     struct integrity_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
 			     struct evm_ima_xattr_data *xattr_value,
-			     int xattr_len);
+			     int xattr_len, struct ima_digest *found_digest);
 int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
 void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
 enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
@@ -260,7 +260,8 @@ static inline int ima_appraise_measurement(enum ima_hooks func,
 					   struct file *file,
 					   const unsigned char *filename,
 					   struct evm_ima_xattr_data *xattr_value,
-					   int xattr_len)
+					   int xattr_len,
+					   struct ima_digest *found_digest)
 {
 	return INTEGRITY_UNKNOWN;
 }
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 2566dbfd2464..01210a265f1f 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -14,8 +14,10 @@
 #include <linux/evm.h>
 
 #include "ima.h"
+#include "ima_digest_list.h"
 
 static bool ima_appraise_req_evm __ro_after_init;
+static bool ima_appraise_no_metadata __ro_after_init;
 static int __init default_appraise_setup(char *str)
 {
 #ifdef CONFIG_IMA_APPRAISE_BOOTPARAM
@@ -29,6 +31,14 @@ static int __init default_appraise_setup(char *str)
 	if (strcmp(str, "enforce-evm") == 0 ||
 	    strcmp(str, "log-evm") == 0)
 		ima_appraise_req_evm = true;
+#ifdef CONFIG_IMA_DIGEST_LIST
+	if (!strncmp(str, "digest", 6)) {
+		ima_digest_list_actions |= IMA_APPRAISE;
+
+		if (!strcmp(str + 6, "-nometadata"))
+			ima_appraise_no_metadata = true;
+	}
+#endif
 	return 1;
 }
 
@@ -73,6 +83,8 @@ static int ima_fix_xattr(struct dentry *dentry,
 	} else {
 		offset = 0;
 		iint->ima_hash->xattr.ng.type = IMA_XATTR_DIGEST_NG;
+		if (iint->flags & IMA_DIGEST_LISTS)
+			iint->ima_hash->xattr.ng.type = IMA_XATTR_DIGEST_LIST;
 		iint->ima_hash->xattr.ng.algo = algo;
 	}
 	rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA,
@@ -163,7 +175,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 			     struct integrity_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
 			     struct evm_ima_xattr_data *xattr_value,
-			     int xattr_len)
+			     int xattr_len, struct ima_digest *found_digest)
 {
 	static const char op[] = "appraise_data";
 	const char *cause = "unknown";
@@ -192,6 +204,22 @@ int ima_appraise_measurement(enum ima_hooks func,
 		    (!(iint->flags & IMA_DIGSIG_REQUIRED) ||
 		     (inode->i_size == 0)))
 			status = INTEGRITY_PASS;
+		if (found_digest) {
+			if (!evm_key_loaded() || ima_appraise_no_metadata)
+				status = INTEGRITY_PASS;
+
+			if (!ima_digest_is_immutable(found_digest)) {
+				if ((iint->flags & IMA_DIGSIG_REQUIRED)) {
+					cause = "IMA-signature-required";
+					status = INTEGRITY_FAIL;
+					goto out;
+				}
+				clear_bit(IMA_DIGSIG, &iint->atomic_flags);
+				iint->flags |= IMA_DIGEST_LISTS;
+			} else {
+				set_bit(IMA_DIGSIG, &iint->atomic_flags);
+			}
+		}
 		goto out;
 	}
 
@@ -217,6 +245,13 @@ int ima_appraise_measurement(enum ima_hooks func,
 	}
 
 	switch (xattr_value->type) {
+	case IMA_XATTR_DIGEST_LIST:
+		if (!ima_appraise_no_metadata) {
+			cause = "IMA-xattr-untrusted";
+			status = INTEGRITY_FAIL;
+			break;
+		}
+		iint->flags |= IMA_DIGEST_LISTS;
 	case IMA_XATTR_DIGEST_NG:
 		/* first byte contains algorithm id */
 		hash_start = 1;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index eca55e788c28..53c6b2eeaec0 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -395,7 +395,9 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
 		inode_lock(inode);
 		rc = ima_appraise_measurement(func, iint, file, pathname,
-					      xattr_value, xattr_len);
+					      xattr_value, xattr_len,
+					      ima_digest_allow(found_digest,
+							       IMA_APPRAISE));
 		inode_unlock(inode);
 		if (!rc)
 			rc = mmap_violation_check(func, file, &pathbuf,
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 8c4cf5127a8b..f695b70feb1a 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -32,6 +32,7 @@
 #define IMA_NEW_FILE		0x04000000
 #define EVM_IMMUTABLE_DIGSIG	0x08000000
 #define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
+#define IMA_DIGEST_LISTS	0x20000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_HASH | IMA_APPRAISE_SUBMASK)
@@ -71,6 +72,7 @@ enum evm_ima_xattr_type {
 	IMA_XATTR_DIGEST_NG,
 	EVM_XATTR_PORTABLE_DIGSIG,
 	EVM_XATTR_HMAC_RND_KEY,
+	IMA_XATTR_DIGEST_LIST,
 	IMA_XATTR_LAST
 };
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 11/14] ima: add support for measurement with digest lists
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

IMA-Measure creates a new measurement entry every time a file is measured,
unless the same entry is already in the measurement list.

This patch introduces a new type of measurement list, recognizable by the
PCR number specified with the new ima_digest_list_pcr= kernel option. This
type of measurement list includes measurements of digest lists and files
not found in those lists.

The benefit of this patch is the availability of a predictable PCR that
can be used to seal data or TPM keys to the OS software. Unlike standard
measurements, digest lists measurements only indicate that files with a
digest in those lists could have been accessed, but not if and when. With
standard measurements, however, the chosen PCR is unlikely predictable.

Both standard and digest list measurements can be generated at the same
time by adding '+' as a prefix to the value of ima_digest_list_pcr=
(example: with ima_digest_list_pcr=+11, IMA generates standard measurements
with PCR 10 and digest list measurements with PCR 11).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../admin-guide/kernel-parameters.txt         |  8 ++++
 security/integrity/ima/ima.h                  |  9 ++--
 security/integrity/ima/ima_api.c              | 43 ++++++++++++++++---
 security/integrity/ima/ima_digest_list.c      | 25 +++++++++++
 security/integrity/ima/ima_init.c             |  2 +-
 security/integrity/ima/ima_main.c             |  9 +++-
 security/integrity/ima/ima_policy.c           |  3 +-
 7 files changed, 86 insertions(+), 13 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0585194ca736..05862a0c402d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1599,6 +1599,14 @@
 			Use the canonical format for the binary runtime
 			measurements, instead of host native format.
 
+	ima_digest_list_pcr=
+			[IMA]
+			Specify which PCR is extended with digests not found
+			in loaded digest lists. Measurement entries for known
+			files are not created unless the '+' is added before
+			the chosen PCR.
+			Format: { [+]<unsigned int> }
+
 	ima_hash=	[IMA]
 			Format: { md5 | sha1 | rmd160 | sha256 | sha384
 				   | sha512 | ... }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 1729ecc4e3e7..42e4f2b64cd1 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -52,6 +52,8 @@ extern int ima_policy_flag;
 extern int ima_hash_algo;
 extern int ima_appraise;
 extern struct tpm_chip *ima_tpm_chip;
+extern int ima_digest_list_pcr;
+extern bool ima_plus_standard_pcr;
 
 extern int ima_digest_list_actions;
 
@@ -204,15 +206,16 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
 			   const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,
 			   int xattr_len, int pcr,
-			   struct ima_template_desc *template_desc);
+			   struct ima_template_desc *template_desc,
+			   struct ima_digest *digest);
 void ima_audit_measurement(struct integrity_iint_cache *iint,
 			   const unsigned char *filename);
 int ima_alloc_init_template(struct ima_event_data *event_data,
 			    struct ima_template_entry **entry,
 			    struct ima_template_desc *template_desc);
 int ima_store_template(struct ima_template_entry *entry, int violation,
-		       struct inode *inode,
-		       const unsigned char *filename, int pcr);
+		       struct inode *inode, const unsigned char *filename,
+		       int pcr, struct ima_digest *digest);
 void ima_free_template_entry(struct ima_template_entry *entry);
 const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
 
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 3233cd69a91b..a3e23b8e2ec7 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -90,11 +90,13 @@ int ima_alloc_init_template(struct ima_event_data *event_data,
  */
 int ima_store_template(struct ima_template_entry *entry,
 		       int violation, struct inode *inode,
-		       const unsigned char *filename, int pcr)
+		       const unsigned char *filename, int pcr,
+		       struct ima_digest *digest)
 {
 	static const char op[] = "add_template_measure";
 	static const char audit_cause[] = "hashing_error";
 	char *template_name = entry->template_desc->name;
+	struct ima_template_entry *duplicated_entry = NULL;
 	int result;
 	struct {
 		struct ima_digest_data hdr;
@@ -117,8 +119,27 @@ int ima_store_template(struct ima_template_entry *entry,
 		}
 		memcpy(entry->digest, hash.hdr.digest, hash.hdr.length);
 	}
+
+	if (ima_plus_standard_pcr && !digest) {
+		duplicated_entry = kmemdup(entry,
+			sizeof(*entry) + entry->template_desc->num_fields *
+			sizeof(struct ima_field_data), GFP_KERNEL);
+		if (duplicated_entry)
+			duplicated_entry->pcr = ima_digest_list_pcr;
+	} else if (!ima_plus_standard_pcr && ima_digest_list_pcr >= 0) {
+		pcr = ima_digest_list_pcr;
+	}
+
 	entry->pcr = pcr;
+
 	result = ima_add_template_entry(entry, violation, op, inode, filename);
+	if (!result && duplicated_entry) {
+		result = ima_add_template_entry(duplicated_entry, violation, op,
+						inode, filename);
+		if (result < 0)
+			kfree(duplicated_entry);
+	}
+
 	return result;
 }
 
@@ -150,8 +171,8 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
 		result = -ENOMEM;
 		goto err_out;
 	}
-	result = ima_store_template(entry, violation, inode,
-				    filename, CONFIG_IMA_MEASURE_PCR_IDX);
+	result = ima_store_template(entry, violation, inode, filename,
+				    CONFIG_IMA_MEASURE_PCR_IDX, NULL);
 	if (result < 0)
 		ima_free_template_entry(entry);
 err_out:
@@ -285,13 +306,14 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 			   struct file *file, const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,
 			   int xattr_len, int pcr,
-			   struct ima_template_desc *template_desc)
+			   struct ima_template_desc *template_desc,
+			   struct ima_digest *digest)
 {
 	static const char op[] = "add_template_measure";
 	static const char audit_cause[] = "ENOMEM";
 	int result = -ENOMEM;
 	struct inode *inode = file_inode(file);
-	struct ima_template_entry *entry;
+	struct ima_template_entry *entry = NULL;
 	struct ima_event_data event_data = { .iint = iint,
 					     .file = file,
 					     .filename = filename,
@@ -302,6 +324,11 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 	if (iint->measured_pcrs & (0x1 << pcr))
 		return;
 
+	if (digest && !ima_plus_standard_pcr && ima_digest_list_pcr >= 0) {
+		result = -EEXIST;
+		goto out;
+	}
+
 	result = ima_alloc_init_template(&event_data, &entry, template_desc);
 	if (result < 0) {
 		integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
@@ -309,12 +336,14 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
 		return;
 	}
 
-	result = ima_store_template(entry, violation, inode, filename, pcr);
+	result = ima_store_template(entry, violation, inode, filename, pcr,
+				    digest);
+out:
 	if ((!result || result == -EEXIST) && !(file->f_flags & O_DIRECT)) {
 		iint->flags |= IMA_MEASURED;
 		iint->measured_pcrs |= (0x1 << pcr);
 	}
-	if (result < 0)
+	if (result < 0 && entry)
 		ima_free_template_entry(entry);
 }
 
diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c
index 532aaf5145ae..c9c079ae82c7 100644
--- a/security/integrity/ima/ima_digest_list.c
+++ b/security/integrity/ima/ima_digest_list.c
@@ -28,6 +28,31 @@ struct ima_h_table ima_digests_htable = {
 	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
 };
 
+static int __init digest_list_pcr_setup(char *str)
+{
+	int pcr, ret;
+
+	ret = kstrtouint(str, 10, &pcr);
+	if (ret) {
+		pr_err("Invalid PCR number %s\n", str);
+		return 1;
+	}
+
+	if (pcr == CONFIG_IMA_MEASURE_PCR_IDX) {
+		pr_err("Default PCR cannot be used for digest lists\n");
+		return 1;
+	}
+
+	ima_digest_list_pcr = pcr;
+	ima_digest_list_actions |= IMA_MEASURE;
+
+	if (*str == '+')
+		ima_plus_standard_pcr = true;
+
+	return 1;
+}
+__setup("ima_digest_list_pcr=", digest_list_pcr_setup);
+
 /*********************
  * Get/add functions *
  *********************/
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 5d55ade5f3b9..5ca9250b913a 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -76,7 +76,7 @@ static int __init ima_add_boot_aggregate(void)
 
 	result = ima_store_template(entry, violation, NULL,
 				    boot_aggregate_name,
-				    CONFIG_IMA_MEASURE_PCR_IDX);
+				    CONFIG_IMA_MEASURE_PCR_IDX, NULL);
 	if (result < 0) {
 		ima_free_template_entry(entry);
 		audit_cause = "store_entry";
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 15eb00fb6b6d..eca55e788c28 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -39,6 +39,10 @@ int ima_appraise;
 
 /* Actions (measure/appraisal) for which digest lists can be used */
 int ima_digest_list_actions;
+/* PCR used for digest list measurements */
+int ima_digest_list_pcr = -1;
+/* Flag to include standard measurement if digest list PCR is specified */
+bool ima_plus_standard_pcr;
 
 int ima_hash_algo = HASH_ALGO_SHA1;
 static int hash_setup_done;
@@ -384,7 +388,10 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (action & IMA_MEASURE)
 		ima_store_measurement(iint, file, pathname,
 				      xattr_value, xattr_len, pcr,
-				      template_desc);
+				      template_desc,
+				      ima_digest_allow(found_digest,
+						       IMA_MEASURE));
+
 	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
 		inode_lock(inode);
 		rc = ima_appraise_measurement(func, iint, file, pathname,
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 0e1c81a29ac1..5537b91272f0 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1135,7 +1135,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			ima_log_string(ab, "pcr", args[0].from);
 
 			result = kstrtoint(args[0].from, 10, &entry->pcr);
-			if (result || INVALID_PCR(entry->pcr))
+			if (result || INVALID_PCR(entry->pcr) ||
+			    entry->pcr == ima_digest_list_pcr)
 				result = -EINVAL;
 			else
 				entry->flags |= IMA_PCR;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 10/14] ima: load parser digests and execute the parser at boot time
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

Digest lists should be uploaded to IMA as soon as possible, otherwise file
digests would appear in the measurement list or access would be denied if
appraisal is in enforcing mode.

This patch adds a call to ima_load_parser_digest_list() in
integrity_load_keys(), so that the function is executed when rootfs becomes
available, before the init process is executed.

ima_load_parser_digest_list() loads a compact list containing the digests
of the parser and the shared libraries. This list is measured and
appraised depending on the current IMA policy. Then, the function executes
the parser executable with the User-Mode-Helper (UMH).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/iint.c                |  1 +
 security/integrity/ima/Kconfig           | 15 +++++++++
 security/integrity/ima/ima_digest_list.c | 42 ++++++++++++++++++++++++
 security/integrity/integrity.h           |  8 +++++
 4 files changed, 66 insertions(+)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index e12c4900510f..de73baccc847 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -213,6 +213,7 @@ void __init integrity_load_keys(void)
 {
 	ima_load_x509();
 	evm_load_x509();
+	ima_load_parser_digest_list();
 }
 
 static int __init integrity_fs_init(void)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index b3a7b46d21cf..c1bb9fedeccc 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -307,3 +307,18 @@ config IMA_DIGEST_LIST
 	   of accessed files are found in one of those lists, no new entries are
 	   added to the measurement list, and access to the file is granted if
 	   appraisal is in enforcing mode.
+
+config IMA_PARSER_DIGEST_LIST_PATH
+	string "Path of the parser digest list"
+	depends on IMA_DIGEST_LIST
+	default "/etc/ima/digest_lists/compact-upload_digest_lists"
+	help
+	   This option defines the path of the digest list containing the
+	   digest of the parser.
+
+config IMA_PARSER_BINARY_PATH
+	string "Path of the parser binary"
+	depends on IMA_DIGEST_LIST
+	default "/usr/bin/upload_digest_lists"
+	help
+	   This option defines the path of the parser binary.
diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c
index 3aaa26d6e8e3..532aaf5145ae 100644
--- a/security/integrity/ima/ima_digest_list.c
+++ b/security/integrity/ima/ima_digest_list.c
@@ -240,3 +240,45 @@ struct ima_digest *ima_digest_allow(struct ima_digest *digest, int action)
 
 	return digest;
 }
+
+/********************
+ * Parser execution *
+ ********************/
+static void ima_exec_parser(void)
+{
+	char *argv[2] = {NULL}, *envp[1] = {NULL};
+
+	argv[0] = (char *)CONFIG_IMA_PARSER_BINARY_PATH;
+	call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
+}
+
+void __init ima_load_parser_digest_list(void)
+{
+	void *datap;
+	loff_t size;
+	int ret;
+
+	if (!(ima_digest_list_actions & ima_policy_flag))
+		return;
+
+	ima_set_parser(current);
+	ret = kernel_read_file_from_path(CONFIG_IMA_PARSER_DIGEST_LIST_PATH,
+					 &datap, &size, 0, READING_DIGEST_LIST);
+	ima_set_parser(NULL);
+
+	if (ret < 0) {
+		if (ret != -ENOENT)
+			pr_err("Unable to open file: %s (%d)",
+			       CONFIG_IMA_PARSER_DIGEST_LIST_PATH, ret);
+		return;
+	}
+
+	ret = ima_parse_compact_list(size, datap);
+
+	vfree(datap);
+
+	if (ret < 0)
+		return;
+
+	ima_exec_parser();
+}
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 1a9bff2e01ec..8c4cf5127a8b 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -213,6 +213,14 @@ static inline void evm_load_x509(void)
 }
 #endif
 
+#ifdef CONFIG_IMA_DIGEST_LIST
+void __init ima_load_parser_digest_list(void);
+#else
+static inline void ima_load_parser_digest_list(void)
+{
+}
+#endif
+
 #ifdef CONFIG_INTEGRITY_AUDIT
 /* declarations */
 void integrity_audit_msg(int audit_msgno, struct inode *inode,
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 09/14] ima: introduce new securityfs files
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

This patch introduces two new files in the securityfs filesystem:
digest_list_data, loads a digest list from the specified path, if it is in
the compact format, or loads a digest list converted by the user space
parser; digests_count: shows the number of digests stored in the
ima_digests_htable hash table.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/fs.h              |  1 +
 security/integrity/ima/ima_fs.c | 44 ++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index f7fdfe93e25d..0591a3c3cc2f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2921,6 +2921,7 @@ extern int do_pipe_flags(int *, int);
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
 	id(POLICY, security-policy)		\
 	id(X509_CERTIFICATE, x509-certificate)	\
+	id(DIGEST_LIST, digest-list)	\
 	id(MAX_ID, )
 
 #define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 0f503b7cd396..b02b9a447d56 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -24,6 +24,7 @@
 #include <linux/vmalloc.h>
 
 #include "ima.h"
+#include "ima_digest_list.h"
 
 static DEFINE_MUTEX(ima_write_mutex);
 
@@ -34,6 +35,8 @@ static struct dentry *ascii_runtime_measurements;
 static struct dentry *runtime_measurements_count;
 static struct dentry *violations;
 static struct dentry *ima_policy;
+static struct dentry *digests_count;
+static struct dentry *digest_list_data;
 
 bool ima_canonical_fmt;
 static int __init default_canonical_fmt_setup(char *str)
@@ -58,6 +61,10 @@ static ssize_t ima_show_htable_value(struct file *filp, char __user *buf,
 		val = &ima_htable.violations;
 	else if (filp->f_path.dentry == runtime_measurements_count)
 		val = &ima_htable.len;
+#ifdef CONFIG_IMA_DIGEST_LIST
+	else if (filp->f_path.dentry == digests_count)
+		val = &ima_digests_htable.len;
+#endif
 
 	len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", atomic_long_read(val));
 	return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
@@ -296,6 +303,9 @@ static ssize_t ima_read_file(char *path, enum kernel_read_file_id file_id)
 			pr_debug("rule: %s\n", p);
 			rc = ima_parse_add_rule(p);
 			break;
+		case READING_DIGEST_LIST:
+			rc = ima_parse_compact_list(size, data);
+			break;
 		default:
 			break;
 		}
@@ -319,6 +329,10 @@ static ssize_t ima_write_data(struct file *file, const char __user *buf,
 	char *data;
 	ssize_t result;
 	struct dentry *dentry = file_dentry(file);
+	enum kernel_read_file_id id = READING_POLICY;
+
+	if (dentry == digest_list_data)
+		id = READING_DIGEST_LIST;
 
 	/* No partial writes. */
 	result = -EINVAL;
@@ -345,7 +359,7 @@ static ssize_t ima_write_data(struct file *file, const char __user *buf,
 		goto out_free;
 
 	if (data[0] == '/') {
-		result = ima_read_file(data, READING_POLICY);
+		result = ima_read_file(data, id);
 	} else if (dentry == ima_policy) {
 		if (ima_appraise & IMA_APPRAISE_POLICY) {
 			pr_err("signed policy file (specified as an absolute pathname) required\n");
@@ -357,6 +371,11 @@ static ssize_t ima_write_data(struct file *file, const char __user *buf,
 		} else {
 			result = ima_parse_add_rule(data);
 		}
+	} else if (dentry == digest_list_data) {
+		if (ima_check_current_is_parser())
+			result = ima_parse_compact_list(datalen, data);
+		else
+			result = -EACCES;
 	} else {
 		pr_err("Unknown data type\n");
 		result = -EINVAL;
@@ -373,6 +392,7 @@ static ssize_t ima_write_data(struct file *file, const char __user *buf,
 
 enum ima_fs_flags {
 	IMA_POLICY_BUSY,
+	IMA_DIGEST_LIST_DATA_BUSY,
 	IMA_FS_BUSY,
 };
 
@@ -382,6 +402,8 @@ static enum ima_fs_flags ima_get_dentry_flag(struct dentry *dentry)
 
 	if (dentry == ima_policy)
 		flag = IMA_POLICY_BUSY;
+	else if (dentry == digest_list_data)
+		flag = IMA_DIGEST_LIST_DATA_BUSY;
 
 	return flag;
 }
@@ -412,6 +434,8 @@ static int ima_open_data_upload(struct inode *inode, struct file *filp)
 		read_allowed = true;
 		seq_ops = &ima_policy_seqops;
 #endif
+	} else if (dentry == digest_list_data) {
+		ima_set_parser(current);
 	}
 
 	if (!(filp->f_flags & O_WRONLY)) {
@@ -444,6 +468,9 @@ static int ima_release_data_upload(struct inode *inode, struct file *file)
 	if ((file->f_flags & O_ACCMODE) == O_RDONLY)
 		return seq_release(inode, file);
 
+	if (dentry == digest_list_data)
+		ima_set_parser(NULL);
+
 	if (dentry != ima_policy) {
 		clear_bit(flag, &ima_fs_flags);
 		return 0;
@@ -529,8 +556,23 @@ int __init ima_fs_init(void)
 	if (IS_ERR(ima_policy))
 		goto out;
 
+#ifdef CONFIG_IMA_DIGEST_LIST
+	digests_count = securityfs_create_file("digests_count",
+					       S_IRUSR | S_IRGRP, ima_dir,
+					       NULL, &ima_htable_value_ops);
+	if (IS_ERR(digests_count))
+		goto out;
+
+	digest_list_data = securityfs_create_file("digest_list_data", S_IWUSR,
+						  ima_dir, NULL,
+						  &ima_data_upload_ops);
+	if (IS_ERR(digest_list_data))
+		goto out;
+#endif
 	return 0;
 out:
+	securityfs_remove(digest_list_data);
+	securityfs_remove(digests_count);
 	securityfs_remove(violations);
 	securityfs_remove(runtime_measurements_count);
 	securityfs_remove(ascii_runtime_measurements);
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 08/14] ima: prevent usage of digest lists that are not measured/appraised
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

The Digest Lists extension creates a new measurement only when a file is
unknown (i.e. its digest is not found in the uploaded digest lists).
However, if digest lists are not measured, a remote verifier cannot
determine which files could have possibly been accessed. If they are not
appraised, a user would be able to access files that are not signed or
protected with a HMAC.

This patch prevents this issue by monitoring the process that opened
digest_list_data and that can upload digests. If the process opens a file
that is not measured, digest list queries by IMA-Measure will be always
negative (ima_digest_allow() will always return a NULL pointer). The same
happens for IMA-Appraise.

This patch also ensures that the parser can only execute shared libraries
with type COMPACT_PARSER (i.e. libraries adding support for custom digest
list formats).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h             |  2 ++
 security/integrity/ima/ima_digest_list.c | 36 ++++++++++++++++++++++++
 security/integrity/ima/ima_digest_list.h | 15 ++++++++++
 security/integrity/ima/ima_main.c        | 17 ++++++++++-
 4 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index b4a0d2a02ff2..1729ecc4e3e7 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -53,6 +53,8 @@ extern int ima_hash_algo;
 extern int ima_appraise;
 extern struct tpm_chip *ima_tpm_chip;
 
+extern int ima_digest_list_actions;
+
 /* IMA event related data */
 struct ima_event_data {
 	struct integrity_iint_cache *iint;
diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c
index 3c77a6cec29a..3aaa26d6e8e3 100644
--- a/security/integrity/ima/ima_digest_list.c
+++ b/security/integrity/ima/ima_digest_list.c
@@ -163,6 +163,9 @@ bool ima_check_current_is_parser(void)
 	struct file *parser_file;
 	struct mm_struct *mm;
 
+	if (!(ima_digest_list_actions & ima_policy_flag))
+		return false;
+
 	mm = get_task_mm(current);
 	if (!mm)
 		return false;
@@ -204,3 +207,36 @@ struct task_struct *ima_get_parser(void)
 {
 	return current_parser;
 }
+
+/**********************
+ * Digest usage check *
+ **********************/
+void ima_check_parser_action(struct inode *inode, enum ima_hooks hook,
+			     int mask, int action, bool check_digest,
+			     struct ima_digest *digest)
+{
+	int action_mask = (IMA_DO_MASK & ~IMA_APPRAISE_SUBMASK);
+
+	if (current != current_parser)
+		return;
+
+	if (!(mask & (MAY_READ | MAY_EXEC)))
+		return;
+
+	if (hook == MMAP_CHECK && mask == MAY_EXEC && check_digest &&
+	    (!digest || digest->type != COMPACT_PARSER))
+		action_mask = 0;
+
+	if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
+		action_mask = 0;
+
+	ima_digest_list_actions &= (action & action_mask);
+}
+
+struct ima_digest *ima_digest_allow(struct ima_digest *digest, int action)
+{
+	if (!(ima_digest_list_actions & action))
+		return NULL;
+
+	return digest;
+}
diff --git a/security/integrity/ima/ima_digest_list.h b/security/integrity/ima/ima_digest_list.h
index be07a4afd7b6..49798688f9c8 100644
--- a/security/integrity/ima/ima_digest_list.h
+++ b/security/integrity/ima/ima_digest_list.h
@@ -29,6 +29,10 @@ int ima_parse_compact_list(loff_t size, void *buf);
 bool ima_check_current_is_parser(void);
 void ima_set_parser(struct task_struct *parser);
 struct task_struct *ima_get_parser(void);
+void ima_check_parser_action(struct inode *inode, enum ima_hooks hook,
+			     int mask, int action, bool check_digest,
+			     struct ima_digest *digest);
+struct ima_digest *ima_digest_allow(struct ima_digest *digest, int action);
 #else
 static inline struct ima_digest *ima_lookup_digest(u8 *digest,
 						   enum hash_algo algo)
@@ -50,5 +54,16 @@ static inline struct task_struct *ima_get_parser(void)
 {
 	return NULL;
 }
+static inline void ima_check_parser_action(struct inode *inode,
+					   enum ima_hooks hook, int mask,
+					   int action, bool check_digest,
+					   struct ima_digest *digest)
+{
+}
+static inline struct ima_digest *ima_digest_allow(struct ima_digest *digest,
+						  int action)
+{
+	return NULL;
+}
 #endif /*CONFIG_IMA_DIGEST_LIST*/
 #endif /*LINUX_IMA_DIGEST_LIST_H*/
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index dc53ed8b0dd0..15eb00fb6b6d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -29,6 +29,7 @@
 #include <linux/fs.h>
 
 #include "ima.h"
+#include "ima_digest_list.h"
 
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise = IMA_APPRAISE_ENFORCE;
@@ -36,6 +37,9 @@ int ima_appraise = IMA_APPRAISE_ENFORCE;
 int ima_appraise;
 #endif
 
+/* Actions (measure/appraisal) for which digest lists can be used */
+int ima_digest_list_actions;
+
 int ima_hash_algo = HASH_ALGO_SHA1;
 static int hash_setup_done;
 
@@ -252,11 +256,15 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	const char *pathname = NULL;
 	int rc = 0, action, must_appraise = 0;
 	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+	struct ima_digest *found_digest = NULL;
 	struct evm_ima_xattr_data *xattr_value = NULL;
 	int xattr_len = 0;
 	bool violation_check;
 	enum hash_algo hash_algo;
 
+	ima_check_parser_action(inode, func, mask, ima_policy_flag, false,
+				NULL);
+
 	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
 		return 0;
 
@@ -268,6 +276,9 @@ static int process_measurement(struct file *file, const struct cred *cred,
 				&template_desc);
 	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
 			   (ima_policy_flag & IMA_MEASURE));
+
+	ima_check_parser_action(inode, func, mask, action, false, NULL);
+
 	if (!action && !violation_check)
 		return 0;
 
@@ -312,7 +323,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags) ||
 	    ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) &&
 	     !(inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) &&
-	     !(action & IMA_FAIL_UNVERIFIABLE_SIGS))) {
+	     !(action & IMA_FAIL_UNVERIFIABLE_SIGS)) ||
+	    ima_get_parser() == current) {
 		iint->flags &= ~IMA_DONE_MASK;
 		iint->measured_pcrs = 0;
 	}
@@ -366,6 +378,9 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
 
+	found_digest = ima_lookup_digest(iint->ima_hash->digest, hash_algo);
+	ima_check_parser_action(inode, func, mask, action, true, found_digest);
+
 	if (action & IMA_MEASURE)
 		ima_store_measurement(iint, file, pathname,
 				      xattr_value, xattr_len, pcr,
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 07/14] ima: restrict upload of converted digest lists
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

If digest lists cannot be directly parsed by the kernel, access to the
securityfs file must be exclusively granted to the parser, to avoid that an
arbitrary process makes undesired modifications before uploading converted
lists to IMA. Digest lists are measured before they are converted and no
new measurement is taken after conversion.

This patch introduces ima_check_set_parser(), to verify whether the process
opening the interface to upload digest lists is the user space parser. It
checks whether the digest of the executable is found in a digest list and
if the type of found digest is COMPACT_PARSER.

It also introduces ima_set_parser() and ima_get_parser() to return the
task_struct of the process that opened digest_list_data. This will be used
to determine whether digest lists have been measured/appraised and, if not,
to prevent their usage.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_digest_list.c | 54 ++++++++++++++++++++++++
 security/integrity/ima/ima_digest_list.h | 14 ++++++
 2 files changed, 68 insertions(+)

diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c
index 6c7dd2cfbb68..3c77a6cec29a 100644
--- a/security/integrity/ima/ima_digest_list.c
+++ b/security/integrity/ima/ima_digest_list.c
@@ -17,6 +17,8 @@
 
 #include <linux/vmalloc.h>
 #include <linux/module.h>
+#include <linux/file.h>
+#include <linux/sched/mm.h>
 
 #include "ima.h"
 #include "ima_digest_list.h"
@@ -150,3 +152,55 @@ int ima_parse_compact_list(loff_t size, void *buf)
 
 	return bufp - buf;
 }
+
+/****************
+ * Parser check *
+ ****************/
+bool ima_check_current_is_parser(void)
+{
+	struct integrity_iint_cache *parser_iint;
+	struct ima_digest *parser_digest = NULL;
+	struct file *parser_file;
+	struct mm_struct *mm;
+
+	mm = get_task_mm(current);
+	if (!mm)
+		return false;
+
+	parser_file = get_mm_exe_file(mm);
+	mmput(mm);
+
+	if (!parser_file)
+		return false;
+
+	parser_iint = integrity_iint_find(file_inode(parser_file));
+	fput(parser_file);
+
+	if (!parser_iint)
+		return false;
+
+	/* flag cannot be cleared due to write protection of executables */
+	if (!(parser_iint->flags & IMA_COLLECTED))
+		return false;
+
+	parser_digest = ima_lookup_digest(parser_iint->ima_hash->digest,
+					  parser_iint->ima_hash->algo);
+
+	return (parser_digest && parser_digest->type == COMPACT_PARSER);
+}
+
+/*
+ * Current parser set and reset respectively during open() and close() of
+ * /sys/kernel/security/ima/digest_list_data.
+ */
+static struct task_struct *current_parser;
+
+void ima_set_parser(struct task_struct *parser)
+{
+	current_parser = parser;
+}
+
+struct task_struct *ima_get_parser(void)
+{
+	return current_parser;
+}
diff --git a/security/integrity/ima/ima_digest_list.h b/security/integrity/ima/ima_digest_list.h
index 13cdc3d954bd..be07a4afd7b6 100644
--- a/security/integrity/ima/ima_digest_list.h
+++ b/security/integrity/ima/ima_digest_list.h
@@ -26,6 +26,9 @@ extern struct ima_h_table ima_digests_htable;
 
 struct ima_digest *ima_lookup_digest(u8 *digest, enum hash_algo algo);
 int ima_parse_compact_list(loff_t size, void *buf);
+bool ima_check_current_is_parser(void);
+void ima_set_parser(struct task_struct *parser);
+struct task_struct *ima_get_parser(void);
 #else
 static inline struct ima_digest *ima_lookup_digest(u8 *digest,
 						   enum hash_algo algo)
@@ -36,5 +39,16 @@ static inline int ima_parse_compact_list(loff_t size, void *buf)
 {
 	return -ENOTSUPP;
 }
+static inline bool ima_check_current_is_parser(void)
+{
+	return false;
+}
+static inline void ima_set_parser(struct task_struct *parser)
+{
+}
+static inline struct task_struct *ima_get_parser(void)
+{
+	return NULL;
+}
 #endif /*CONFIG_IMA_DIGEST_LIST*/
 #endif /*LINUX_IMA_DIGEST_LIST_H*/
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 06/14] ima: add parser of compact digest list
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

This patch introduces the parser of the compact digest list. The format is
optimized to store a large quantity of data with the same type. It is the
only format supported by the kernel.

Digest lists can be parsed directly by the kernel like the policy or, if
they are in a different format, they can be parsed by a user space parser,
and uploaded to IMA after they have been converted to the compact list
format.

A compact list is a set of consecutive data blocks, each consisting of a
header and a payload. The header indicates the type of data, how many
elements and the length of the payload.

The COMPACT_KEY data type indicates that the payload contains keys for
verifying digest list signatures. COMPACT_PARSER is used for digests of the
parser binary and shared libraries. COMPACT_FILE is used for digests of
regular files.

The header contains also a type modifier to indicate attributes of the
elements included in the payload. The COMPACT_MOD_IMMUTABLE modifier
indicates that a file can be opened only for read, if appraisal is in
enforcing mode.

This patch also introduces ima_lookup_loaded_digest() and
ima_add_digest_data_entry() to search and add digests in the new hash table
(ima_digests_htable).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/Kconfig           |  10 ++
 security/integrity/ima/Makefile          |   1 +
 security/integrity/ima/ima_digest_list.c | 152 +++++++++++++++++++++++
 security/integrity/ima/ima_digest_list.h |  40 ++++++
 security/integrity/integrity.h           |  12 ++
 5 files changed, 215 insertions(+)
 create mode 100644 security/integrity/ima/ima_digest_list.c
 create mode 100644 security/integrity/ima/ima_digest_list.h

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 2ced99dde694..b3a7b46d21cf 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -297,3 +297,13 @@ config IMA_APPRAISE_SIGNED_INIT
 	default n
 	help
 	   This option requires user-space init to be signed.
+
+config IMA_DIGEST_LIST
+	bool "Measure and appraise files with digest lists"
+	depends on IMA
+	default n
+	help
+	   This option allows users to load digest lists. If calculated digests
+	   of accessed files are found in one of those lists, no new entries are
+	   added to the measurement list, and access to the file is granted if
+	   appraisal is in enforcing mode.
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index d921dc4f9eb0..29b49c7aacd0 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -10,4 +10,5 @@ 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_HAVE_IMA_KEXEC) += ima_kexec.o
+ima-$(CONFIG_IMA_DIGEST_LIST) += ima_digest_list.o
 obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c
new file mode 100644
index 000000000000..6c7dd2cfbb68
--- /dev/null
+++ b/security/integrity/ima/ima_digest_list.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017-2019 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: ima_digest_list.c
+ *      Functions to manage digest lists.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/vmalloc.h>
+#include <linux/module.h>
+
+#include "ima.h"
+#include "ima_digest_list.h"
+
+struct ima_h_table ima_digests_htable = {
+	.len = ATOMIC_LONG_INIT(0),
+	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
+};
+
+/*********************
+ * Get/add functions *
+ *********************/
+struct ima_digest *ima_lookup_digest(u8 *digest, enum hash_algo algo)
+{
+	struct ima_digest *d = NULL;
+	int digest_len = hash_digest_size[algo];
+	unsigned int key = ima_hash_key(digest);
+
+	rcu_read_lock();
+	hlist_for_each_entry_rcu(d, &ima_digests_htable.queue[key], hnext)
+		if (d->algo == algo && !memcmp(d->digest, digest, digest_len))
+			break;
+
+	rcu_read_unlock();
+	return d;
+}
+
+static int ima_add_digest_data_entry(u8 *digest, enum hash_algo algo,
+				     enum compact_types type, u16 modifiers)
+{
+	struct ima_digest *d;
+	int digest_len = hash_digest_size[algo];
+	unsigned int key = ima_hash_key(digest);
+
+	d = ima_lookup_digest(digest, algo);
+	if (d) {
+		d->modifiers |= modifiers;
+		return -EEXIST;
+	}
+
+	d = kmalloc(sizeof(*d) + digest_len, GFP_KERNEL);
+	if (d == NULL)
+		return -ENOMEM;
+
+	d->algo = algo;
+	d->type = type;
+	d->modifiers = modifiers;
+
+	memcpy(d->digest, digest, digest_len);
+	hlist_add_head_rcu(&d->hnext, &ima_digests_htable.queue[key]);
+	atomic_long_inc(&ima_digests_htable.len);
+	return 0;
+}
+
+/***********************
+ * Compact list parser *
+ ***********************/
+struct compact_list_hdr {
+	u8 version;
+	u8 _reserved;
+	u16 type;
+	u16 modifiers;
+	u16 algo;
+	u32 count;
+	u32 datalen;
+} __packed;
+
+int ima_parse_compact_list(loff_t size, void *buf)
+{
+	u8 *digest;
+	void *bufp = buf, *bufendp = buf + size;
+	struct compact_list_hdr *hdr;
+	size_t digest_len;
+	int ret, i;
+
+	while (bufp < bufendp) {
+		if (bufp + sizeof(*hdr) > bufendp) {
+			pr_err("compact list, invalid data\n");
+			return -EINVAL;
+		}
+
+		hdr = bufp;
+
+		if (hdr->version != 1) {
+			pr_err("compact list, unsupported version\n");
+			return -EINVAL;
+		}
+
+		if (ima_canonical_fmt) {
+			hdr->type = le16_to_cpu(hdr->type);
+			hdr->modifiers = le16_to_cpu(hdr->modifiers);
+			hdr->algo = le16_to_cpu(hdr->algo);
+			hdr->count = le32_to_cpu(hdr->count);
+			hdr->datalen = le32_to_cpu(hdr->datalen);
+		}
+
+		if (hdr->algo >= HASH_ALGO__LAST)
+			return -EINVAL;
+
+		digest_len = hash_digest_size[hdr->algo];
+
+		if (hdr->type >= COMPACT__LAST) {
+			pr_err("compact list, invalid type %d\n", hdr->type);
+			return -EINVAL;
+		}
+
+		bufp += sizeof(*hdr);
+
+		for (i = 0; i < hdr->count; i++) {
+			if (bufp + digest_len > bufendp) {
+				pr_err("compact list, invalid data\n");
+				return -EINVAL;
+			}
+
+			digest = bufp;
+			bufp += digest_len;
+
+			ret = ima_add_digest_data_entry(digest, hdr->algo,
+							hdr->type,
+							hdr->modifiers);
+			if (ret < 0 && ret != -EEXIST)
+				return ret;
+		}
+
+		if (i != hdr->count ||
+		    bufp != (void *)hdr + sizeof(*hdr) + hdr->datalen) {
+			pr_err("compact list, invalid data\n");
+			return -EINVAL;
+		}
+	}
+
+	return bufp - buf;
+}
diff --git a/security/integrity/ima/ima_digest_list.h b/security/integrity/ima/ima_digest_list.h
new file mode 100644
index 000000000000..13cdc3d954bd
--- /dev/null
+++ b/security/integrity/ima/ima_digest_list.h
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017-2019 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ * File: ima_digest_list.h
+ *      Header of ima_digest_list.c
+ */
+
+#ifndef __LINUX_IMA_DIGEST_LIST_H
+#define __LINUX_IMA_DIGEST_LIST_H
+
+static inline bool ima_digest_is_immutable(struct ima_digest *digest)
+{
+	return (digest->modifiers & (1 << COMPACT_MOD_IMMUTABLE));
+}
+
+#ifdef CONFIG_IMA_DIGEST_LIST
+extern struct ima_h_table ima_digests_htable;
+
+struct ima_digest *ima_lookup_digest(u8 *digest, enum hash_algo algo);
+int ima_parse_compact_list(loff_t size, void *buf);
+#else
+static inline struct ima_digest *ima_lookup_digest(u8 *digest,
+						   enum hash_algo algo)
+{
+	return NULL;
+}
+static inline int ima_parse_compact_list(loff_t size, void *buf)
+{
+	return -ENOTSUPP;
+}
+#endif /*CONFIG_IMA_DIGEST_LIST*/
+#endif /*LINUX_IMA_DIGEST_LIST_H*/
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 54a7ad4f6cc0..1a9bff2e01ec 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -11,6 +11,7 @@
 #include <crypto/sha.h>
 #include <linux/key.h>
 #include <linux/audit.h>
+#include <linux/hash_info.h>
 
 /* iint action cache flags */
 #define IMA_MEASURE		0x00000001
@@ -127,6 +128,17 @@ struct integrity_iint_cache {
 	struct ima_digest_data *ima_hash;
 };
 
+enum compact_types { COMPACT_KEY, COMPACT_PARSER, COMPACT_FILE, COMPACT__LAST };
+enum compact_modifiers { COMPACT_MOD_IMMUTABLE, COMPACT_MOD__LAST };
+
+struct ima_digest {
+	struct hlist_node hnext;
+	enum hash_algo algo;
+	enum compact_types type;
+	u16 modifiers;
+	u8 digest[0];
+};
+
 /* rbtree tree calls to lookup, insert, delete
  * integrity data associated with an inode.
  */
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 05/14] ima: use ima_show_htable_value to show violations and hash table data
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

ima_show_htable_violations() and ima_show_measurements_count() both call
ima_show_htable_value() to copy the value of an atomic_long_t variable to
a buffer.

This patch modifies the definition of ima_show_htable_value(), so that this
function can be used in any file_operations structure. The atomic_long_t
variable used as source is chosen depending on the opened file in the
securityfs filesystem.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_fs.c | 38 +++++++++++----------------------
 1 file changed, 12 insertions(+), 26 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index c8bbc56f735e..0f503b7cd396 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -47,38 +47,24 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
 
 static int valid_policy = 1;
 
-static ssize_t ima_show_htable_value(char __user *buf, size_t count,
-				     loff_t *ppos, atomic_long_t *val)
+static ssize_t ima_show_htable_value(struct file *filp, char __user *buf,
+				     size_t count, loff_t *ppos)
 {
+	atomic_long_t *val = NULL;
 	char tmpbuf[32];	/* greater than largest 'long' string value */
 	ssize_t len;
 
+	if (filp->f_path.dentry == violations)
+		val = &ima_htable.violations;
+	else if (filp->f_path.dentry == runtime_measurements_count)
+		val = &ima_htable.len;
+
 	len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", atomic_long_read(val));
 	return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
 }
 
-static ssize_t ima_show_htable_violations(struct file *filp,
-					  char __user *buf,
-					  size_t count, loff_t *ppos)
-{
-	return ima_show_htable_value(buf, count, ppos, &ima_htable.violations);
-}
-
-static const struct file_operations ima_htable_violations_ops = {
-	.read = ima_show_htable_violations,
-	.llseek = generic_file_llseek,
-};
-
-static ssize_t ima_show_measurements_count(struct file *filp,
-					   char __user *buf,
-					   size_t count, loff_t *ppos)
-{
-	return ima_show_htable_value(buf, count, ppos, &ima_htable.len);
-
-}
-
-static const struct file_operations ima_measurements_count_ops = {
-	.read = ima_show_measurements_count,
+static const struct file_operations ima_htable_value_ops = {
+	.read = ima_show_htable_value,
 	.llseek = generic_file_llseek,
 };
 
@@ -527,13 +513,13 @@ int __init ima_fs_init(void)
 	runtime_measurements_count =
 	    securityfs_create_file("runtime_measurements_count",
 				   S_IRUSR | S_IRGRP, ima_dir, NULL,
-				   &ima_measurements_count_ops);
+				   &ima_htable_value_ops);
 	if (IS_ERR(runtime_measurements_count))
 		goto out;
 
 	violations =
 	    securityfs_create_file("violations", S_IRUSR | S_IRGRP,
-				   ima_dir, NULL, &ima_htable_violations_ops);
+				   ima_dir, NULL, &ima_htable_value_ops);
 	if (IS_ERR(violations))
 		goto out;
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 04/14] ima: generalize policy file operations
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

This patch renames ima_open_policy() and ima_release_policy() respectively
to ima_open_data_upload() and ima_release_data_upload(). They will be used
to implement file operations for interfaces allowing to load data from user
space.

A new flag (IMA_POLICY_BUSY) has been defined to prevent concurrent policy
upload.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_fs.c | 58 ++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 15 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 9a10b62e380f..c8bbc56f735e 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -386,9 +386,20 @@ static ssize_t ima_write_data(struct file *file, const char __user *buf,
 }
 
 enum ima_fs_flags {
+	IMA_POLICY_BUSY,
 	IMA_FS_BUSY,
 };
 
+static enum ima_fs_flags ima_get_dentry_flag(struct dentry *dentry)
+{
+	enum ima_fs_flags flag = IMA_FS_BUSY;
+
+	if (dentry == ima_policy)
+		flag = IMA_POLICY_BUSY;
+
+	return flag;
+}
+
 static unsigned long ima_fs_flags;
 
 #ifdef	CONFIG_IMA_READ_POLICY
@@ -401,22 +412,32 @@ static const struct seq_operations ima_policy_seqops = {
 #endif
 
 /*
- * ima_open_policy: sequentialize access to the policy file
+ * ima_open_data_upload: sequentialize access to the data upload interface
  */
-static int ima_open_policy(struct inode *inode, struct file *filp)
+static int ima_open_data_upload(struct inode *inode, struct file *filp)
 {
+	struct dentry *dentry = file_dentry(filp);
+	const struct seq_operations *seq_ops = NULL;
+	enum ima_fs_flags flag = ima_get_dentry_flag(dentry);
+	bool read_allowed = false;
+
+	if (dentry == ima_policy) {
+#ifdef	CONFIG_IMA_READ_POLICY
+		read_allowed = true;
+		seq_ops = &ima_policy_seqops;
+#endif
+	}
+
 	if (!(filp->f_flags & O_WRONLY)) {
-#ifndef	CONFIG_IMA_READ_POLICY
-		return -EACCES;
-#else
+		if (!read_allowed)
+			return -EACCES;
 		if ((filp->f_flags & O_ACCMODE) != O_RDONLY)
 			return -EACCES;
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
-		return seq_open(filp, &ima_policy_seqops);
-#endif
+		return seq_open(filp, seq_ops);
 	}
-	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
+	if (test_and_set_bit(flag, &ima_fs_flags))
 		return -EBUSY;
 	return 0;
 }
@@ -428,13 +449,20 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
  * point to the new policy rules, and remove the securityfs policy file,
  * assuming a valid policy.
  */
-static int ima_release_policy(struct inode *inode, struct file *file)
+static int ima_release_data_upload(struct inode *inode, struct file *file)
 {
+	struct dentry *dentry = file_dentry(file);
 	const char *cause = valid_policy ? "completed" : "failed";
+	enum ima_fs_flags flag = ima_get_dentry_flag(dentry);
 
 	if ((file->f_flags & O_ACCMODE) == O_RDONLY)
 		return seq_release(inode, file);
 
+	if (dentry != ima_policy) {
+		clear_bit(flag, &ima_fs_flags);
+		return 0;
+	}
+
 	if (valid_policy && ima_check_policy() < 0) {
 		cause = "failed";
 		valid_policy = 0;
@@ -447,7 +475,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 	if (!valid_policy) {
 		ima_delete_rules();
 		valid_policy = 1;
-		clear_bit(IMA_FS_BUSY, &ima_fs_flags);
+		clear_bit(flag, &ima_fs_flags);
 		return 0;
 	}
 
@@ -456,18 +484,18 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 	securityfs_remove(ima_policy);
 	ima_policy = NULL;
 #elif defined(CONFIG_IMA_WRITE_POLICY)
-	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
+	clear_bit(flag, &ima_fs_flags);
 #elif defined(CONFIG_IMA_READ_POLICY)
 	inode->i_mode &= ~S_IWUSR;
 #endif
 	return 0;
 }
 
-static const struct file_operations ima_measure_policy_ops = {
-	.open = ima_open_policy,
+static const struct file_operations ima_data_upload_ops = {
+	.open = ima_open_data_upload,
 	.write = ima_write_data,
 	.read = seq_read,
-	.release = ima_release_policy,
+	.release = ima_release_data_upload,
 	.llseek = generic_file_llseek,
 };
 
@@ -511,7 +539,7 @@ int __init ima_fs_init(void)
 
 	ima_policy = securityfs_create_file("policy", POLICY_FILE_FLAGS,
 					    ima_dir, NULL,
-					    &ima_measure_policy_ops);
+					    &ima_data_upload_ops);
 	if (IS_ERR(ima_policy))
 		goto out;
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 03/14] ima: generalize ima_write_policy() and raise uploaded data size limit
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

ima_write_policy() is being used to load a new policy from user space. This
function can be reused to load different types of data.

This patch renames ima_write_policy() to ima_write_data() and executes the
appropriate actions depending on the opened file in the securityfs
filesystem.

Also, this patch raises the uploaded data size limit to 64M, to accept
files (e.g. digest lists) larger than a policy. The same limit is used
for the SELinux policy.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_fs.c | 68 +++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 28 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 02980b55a3f1..9a10b62e380f 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -27,6 +27,14 @@
 
 static DEFINE_MUTEX(ima_write_mutex);
 
+static struct dentry *ima_dir;
+static struct dentry *ima_symlink;
+static struct dentry *binary_runtime_measurements;
+static struct dentry *ascii_runtime_measurements;
+static struct dentry *runtime_measurements_count;
+static struct dentry *violations;
+static struct dentry *ima_policy;
+
 bool ima_canonical_fmt;
 static int __init default_canonical_fmt_setup(char *str)
 {
@@ -319,25 +327,32 @@ static ssize_t ima_read_file(char *path, enum kernel_read_file_id file_id)
 		return pathlen;
 }
 
-static ssize_t ima_write_policy(struct file *file, const char __user *buf,
-				size_t datalen, loff_t *ppos)
+static ssize_t ima_write_data(struct file *file, const char __user *buf,
+			      size_t datalen, loff_t *ppos)
 {
 	char *data;
 	ssize_t result;
-
-	if (datalen >= PAGE_SIZE)
-		datalen = PAGE_SIZE - 1;
+	struct dentry *dentry = file_dentry(file);
 
 	/* No partial writes. */
 	result = -EINVAL;
 	if (*ppos != 0)
 		goto out;
 
-	data = memdup_user_nul(buf, datalen);
-	if (IS_ERR(data)) {
-		result = PTR_ERR(data);
+	result = -EFBIG;
+	if (datalen + 1 > 64 * 1024 * 1024)
 		goto out;
-	}
+
+	result = -ENOMEM;
+	data = vmalloc(datalen + 1);
+	if (!data)
+		goto out;
+
+	result = -EFAULT;
+	if (copy_from_user(data, buf, datalen) != 0)
+		goto out_free;
+
+	data[datalen] = '\0';
 
 	result = mutex_lock_interruptible(&ima_write_mutex);
 	if (result < 0)
@@ -345,34 +360,31 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 
 	if (data[0] == '/') {
 		result = ima_read_file(data, READING_POLICY);
-	} else if (ima_appraise & IMA_APPRAISE_POLICY) {
-		pr_err("signed policy file (specified as an absolute pathname) required\n");
-		integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
-				    "policy_update", "signed policy required",
-				    1, 0);
-		if (ima_appraise & IMA_APPRAISE_ENFORCE)
-			result = -EACCES;
+	} else if (dentry == ima_policy) {
+		if (ima_appraise & IMA_APPRAISE_POLICY) {
+			pr_err("signed policy file (specified as an absolute pathname) required\n");
+			integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
+					    "policy_update",
+					    "signed policy required", 1, 0);
+			if (ima_appraise & IMA_APPRAISE_ENFORCE)
+				result = -EACCES;
+		} else {
+			result = ima_parse_add_rule(data);
+		}
 	} else {
-		result = ima_parse_add_rule(data);
+		pr_err("Unknown data type\n");
+		result = -EINVAL;
 	}
 	mutex_unlock(&ima_write_mutex);
 out_free:
-	kfree(data);
+	vfree(data);
 out:
-	if (result < 0)
+	if (dentry == ima_policy && result < 0)
 		valid_policy = 0;
 
 	return result;
 }
 
-static struct dentry *ima_dir;
-static struct dentry *ima_symlink;
-static struct dentry *binary_runtime_measurements;
-static struct dentry *ascii_runtime_measurements;
-static struct dentry *runtime_measurements_count;
-static struct dentry *violations;
-static struct dentry *ima_policy;
-
 enum ima_fs_flags {
 	IMA_FS_BUSY,
 };
@@ -453,7 +465,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 
 static const struct file_operations ima_measure_policy_ops = {
 	.open = ima_open_policy,
-	.write = ima_write_policy,
+	.write = ima_write_data,
 	.read = seq_read,
 	.release = ima_release_policy,
 	.llseek = generic_file_llseek,
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 02/14] ima: generalize ima_read_policy()
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

Rename ima_read_policy() to ima_read_file(), and add file_id as a new
parameter. If file_id is equal to READING_POLICY, ima_read_file() behavior
remains unchanged. If file_id will be READING_DIGEST_LIST (not yet
defined), ima_read_file() will read and parse a digest list from a file
whose path is written to securityfs.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_fs.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 2000e8df0301..02980b55a3f1 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -272,7 +272,7 @@ static const struct file_operations ima_ascii_measurements_ops = {
 	.release = seq_release,
 };
 
-static ssize_t ima_read_policy(char *path)
+static ssize_t ima_read_file(char *path, enum kernel_read_file_id file_id)
 {
 	void *data;
 	char *datap;
@@ -285,16 +285,26 @@ static ssize_t ima_read_policy(char *path)
 	datap = path;
 	strsep(&datap, "\n");
 
-	rc = kernel_read_file_from_path(path, &data, &size, 0, READING_POLICY);
+	rc = kernel_read_file_from_path(path, &data, &size, 0, file_id);
 	if (rc < 0) {
 		pr_err("Unable to open file: %s (%d)", path, rc);
 		return rc;
 	}
 
 	datap = data;
-	while (size > 0 && (p = strsep(&datap, "\n"))) {
-		pr_debug("rule: %s\n", p);
-		rc = ima_parse_add_rule(p);
+	while (size > 0) {
+		switch (file_id) {
+		case READING_POLICY:
+			p = strsep(&datap, "\n");
+			if (p == NULL)
+				break;
+
+			pr_debug("rule: %s\n", p);
+			rc = ima_parse_add_rule(p);
+			break;
+		default:
+			break;
+		}
 		if (rc < 0)
 			break;
 		size -= rc;
@@ -334,7 +344,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 		goto out_free;
 
 	if (data[0] == '/') {
-		result = ima_read_policy(data);
+		result = ima_read_file(data, READING_POLICY);
 	} else if (ima_appraise & IMA_APPRAISE_POLICY) {
 		pr_err("signed policy file (specified as an absolute pathname) required\n");
 		integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 01/14] ima: read hash algorithm from security.ima even if appraisal is not enabled
From: Roberto Sassu @ 2019-06-14 17:55 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu
In-Reply-To: <20190614175513.27097-1-roberto.sassu@huawei.com>

IMA reads the hash algorithm from security.ima, if exists, so that a
signature can be verified even if the algorithm used for the signature
differs from IMA algorithm.

This patch moves ima_read_xattr() and ima_get_hash_algo() to ima_main.c, to
retrieve the algorithm even if appraisal is not enabled. Knowing the
algorithm in advance would be useful also for measurement. The new Digest
Lists extension does not add a new entry to the measurement list if the
actual file digest is found in a loaded list. It might be possible that the
algorithm used for the digest lists differs from IMA algorithm.

This patch also changes the requirement that security.ima must contain a
signature, if the type is EVM_IMA_XATTR_DIGSIG. A signature with length
zero is accepted.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h          | 16 --------
 security/integrity/ima/ima_appraise.c | 55 ++-------------------------
 security/integrity/ima/ima_main.c     | 51 +++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 67 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ee509c68fe14..b4a0d2a02ff2 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -248,10 +248,6 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
 void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
 enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
 					   enum ima_hooks func);
-enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
-				 int xattr_len);
-int ima_read_xattr(struct dentry *dentry,
-		   struct evm_ima_xattr_data **xattr_value);
 
 #else
 static inline int ima_appraise_measurement(enum ima_hooks func,
@@ -282,18 +278,6 @@ static inline enum integrity_status ima_get_cache_status(struct integrity_iint_c
 	return INTEGRITY_UNKNOWN;
 }
 
-static inline enum hash_algo
-ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len)
-{
-	return ima_hash_algo;
-}
-
-static inline int ima_read_xattr(struct dentry *dentry,
-				 struct evm_ima_xattr_data **xattr_value)
-{
-	return 0;
-}
-
 #endif /* CONFIG_IMA_APPRAISE */
 
 /* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 9b3b172f9748..2566dbfd2464 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -151,57 +151,6 @@ static void ima_cache_flags(struct integrity_iint_cache *iint,
 	}
 }
 
-enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
-				 int xattr_len)
-{
-	struct signature_v2_hdr *sig;
-	enum hash_algo ret;
-
-	if (!xattr_value || xattr_len < 2)
-		/* return default hash algo */
-		return ima_hash_algo;
-
-	switch (xattr_value->type) {
-	case EVM_IMA_XATTR_DIGSIG:
-		sig = (typeof(sig))xattr_value;
-		if (sig->version != 2 || xattr_len <= sizeof(*sig))
-			return ima_hash_algo;
-		return sig->hash_algo;
-		break;
-	case IMA_XATTR_DIGEST_NG:
-		ret = xattr_value->digest[0];
-		if (ret < HASH_ALGO__LAST)
-			return ret;
-		break;
-	case IMA_XATTR_DIGEST:
-		/* this is for backward compatibility */
-		if (xattr_len == 21) {
-			unsigned int zero = 0;
-			if (!memcmp(&xattr_value->digest[16], &zero, 4))
-				return HASH_ALGO_MD5;
-			else
-				return HASH_ALGO_SHA1;
-		} else if (xattr_len == 17)
-			return HASH_ALGO_MD5;
-		break;
-	}
-
-	/* return default hash algo */
-	return ima_hash_algo;
-}
-
-int ima_read_xattr(struct dentry *dentry,
-		   struct evm_ima_xattr_data **xattr_value)
-{
-	ssize_t ret;
-
-	ret = vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value,
-				 0, GFP_NOFS);
-	if (ret == -EOPNOTSUPP)
-		ret = 0;
-	return ret;
-}
-
 /*
  * ima_appraise_measurement - appraise file measurement
  *
@@ -226,6 +175,10 @@ int ima_appraise_measurement(enum ima_hooks func,
 	if (!(inode->i_opflags & IOP_XATTR))
 		return INTEGRITY_UNKNOWN;
 
+	if (xattr_len == sizeof(struct signature_v2_hdr) &&
+	    xattr_value->type == EVM_IMA_XATTR_DIGSIG)
+		rc = -ENODATA;
+
 	if (rc <= 0) {
 		if (rc && rc != -ENODATA)
 			goto out;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 0c49cf6470a4..dc53ed8b0dd0 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -143,6 +143,57 @@ static void ima_rdwr_violation_check(struct file *file,
 				  "invalid_pcr", "open_writers");
 }
 
+static enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
+					int xattr_len)
+{
+	struct signature_v2_hdr *sig;
+	enum hash_algo ret;
+
+	if (!xattr_value || xattr_len < 2)
+		/* return default hash algo */
+		return ima_hash_algo;
+
+	switch (xattr_value->type) {
+	case EVM_IMA_XATTR_DIGSIG:
+		sig = (typeof(sig))xattr_value;
+		if (sig->version != 2 || xattr_len < sizeof(*sig))
+			return ima_hash_algo;
+		return sig->hash_algo;
+	case IMA_XATTR_DIGEST_NG:
+		ret = xattr_value->digest[0];
+		if (ret < HASH_ALGO__LAST)
+			return ret;
+		break;
+	case IMA_XATTR_DIGEST:
+		/* this is for backward compatibility */
+		if (xattr_len == 21) {
+			unsigned int zero = 0;
+
+			if (!memcmp(&xattr_value->digest[16], &zero, 4))
+				return HASH_ALGO_MD5;
+			else
+				return HASH_ALGO_SHA1;
+		} else if (xattr_len == 17)
+			return HASH_ALGO_MD5;
+		break;
+	}
+
+	/* return default hash algo */
+	return ima_hash_algo;
+}
+
+static int ima_read_xattr(struct dentry *dentry,
+			  struct evm_ima_xattr_data **xattr_value)
+{
+	ssize_t ret;
+
+	ret = vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value,
+				 0, GFP_NOFS);
+	if (ret == -EOPNOTSUPP)
+		ret = 0;
+	return ret;
+}
+
 static void ima_check_last_writer(struct integrity_iint_cache *iint,
 				  struct inode *inode, struct file *file)
 {
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 00/14] ima: introduce IMA Digest Lists extension
From: Roberto Sassu @ 2019-06-14 17:54 UTC (permalink / raw)
  To: zohar, dmitry.kasatkin, mjg59
  Cc: linux-integrity, linux-security-module, linux-fsdevel, linux-doc,
	linux-kernel, silviu.vlasceanu, Roberto Sassu

This patch set introduces a new IMA extension called IMA Digest Lists.

At early boot, the extension preloads in kernel memory reference digest
values, that can be compared with actual file digests when files are
accessed in the system.

The extension will open for new possibilities: PCR with predictable value,
that can be used for sealing policies associated to data or TPM keys;
appraisal based on reference digests already provided by Linux distribution
vendors in the software packages.

The first objective can be achieved because the PCR values does not depend
on which and when files are measured: the extension measures digest lists
sequentially and files whose digest is not in the digest list.

The second objective can be reached because the extension is able to
extract reference measurements from packages (with a user space tool) and
use it as a source for appraisal verification as the reference came from
the security.ima xattr. This approach will also reduce the overhead as only
one signature is verified for many files (as opposed to one signature for
each file with the current implementation).

This version of the patch set provides a clear separation between current
and new functionality. First, the new functionality must be explicitly
enabled from the kernel command line. Second, results of operations
performed by the extension can be distinguished from those obtained from
the existing code: measurement entries created by the extension have a
different PCR; mutable files appraised with the extension have a different
security.ima type.

The review of this patch set should start from patch 11 and 12, which
modify the IMA-Measure and IMA-Appraise submodules to use digest lists.
Patch 1 to 5 are prerequisites. Patch 6 to 10 adds support for digest
lists. Finally, patch 13 introduces two new policies to measure/appraise
rootfs and patch 14 adds the documentation (including a flow chart to
show how IMA has been modified).

The user space tools to configure digest lists are available at:

https://github.com/euleros/digest-list-tools/releases/tag/v0.3

The patch set applies on top of linux-integrity/next-queued-testing
(73589972b987).

It is necessary to apply also:
https://patchwork.kernel.org/cover/10957495/

To use appraisal, it is necessary to use a modified cpio and a modified
dracut:

https://github.com/euleros/cpio/tree/xattr-v1
https://github.com/euleros/dracut/tree/digest-lists

For now, please use it only in a testing environment.


Changelog

v3:
- move ima_lookup_loaded_digest() and ima_add_digest_data_entry() from
  ima_queue.c to ima_digest_list.c
- remove patch that introduces security.ima_algo
- add version number and type modifiers to the compact list header
- remove digest list metadata, all digest lists in the directory are
  accessed
- move loading of signing keys to user space
- add violation for both PCRs if they are selected
- introduce two new appraisal modes

v2:
- add support for multiple hash algorithms
- remove RPM parser from the kernel
- add support for parsing digest lists in user space

v1:
- add support for immutable/mutable files
- add support for appraisal with digest lists


Roberto Sassu (14):
  ima: read hash algorithm from security.ima even if appraisal is not
    enabled
  ima: generalize ima_read_policy()
  ima: generalize ima_write_policy() and raise uploaded data size limit
  ima: generalize policy file operations
  ima: use ima_show_htable_value to show violations and hash table data
  ima: add parser of compact digest list
  ima: restrict upload of converted digest lists
  ima: prevent usage of digest lists that are not measured/appraised
  ima: introduce new securityfs files
  ima: load parser digests and execute the parser at boot time
  ima: add support for measurement with digest lists
  ima: add support for appraisal with digest lists
  ima: introduce new policies initrd and appraise_initrd
  ima: add Documentation/security/IMA-digest-lists.txt

 .../admin-guide/kernel-parameters.txt         |  16 +-
 Documentation/security/IMA-digest-lists.txt   | 226 +++++++++++++
 include/linux/evm.h                           |   6 +
 include/linux/fs.h                            |   1 +
 security/integrity/evm/evm_main.c             |   2 +-
 security/integrity/iint.c                     |   1 +
 security/integrity/ima/Kconfig                |  25 ++
 security/integrity/ima/Makefile               |   1 +
 security/integrity/ima/ima.h                  |  32 +-
 security/integrity/ima/ima_api.c              |  43 ++-
 security/integrity/ima/ima_appraise.c         |  92 +++---
 security/integrity/ima/ima_digest_list.c      | 309 ++++++++++++++++++
 security/integrity/ima/ima_digest_list.h      |  69 ++++
 security/integrity/ima/ima_fs.c               | 224 ++++++++-----
 security/integrity/ima/ima_init.c             |   2 +-
 security/integrity/ima/ima_main.c             |  81 ++++-
 security/integrity/ima/ima_policy.c           |  29 +-
 security/integrity/integrity.h                |  22 ++
 18 files changed, 1018 insertions(+), 163 deletions(-)
 create mode 100644 Documentation/security/IMA-digest-lists.txt
 create mode 100644 security/integrity/ima/ima_digest_list.c
 create mode 100644 security/integrity/ima/ima_digest_list.h

-- 
2.17.1


^ permalink raw reply

* Re: [RFC PATCH v1 2/3] LSM/x86/sgx: Implement SGX specific hooks in SELinux
From: Sean Christopherson @ 2019-06-14 17:53 UTC (permalink / raw)
  To: Xing, Cedric
  Cc: Stephen Smalley, linux-security-module@vger.kernel.org,
	selinux@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-sgx@vger.kernel.org, jarkko.sakkinen@linux.intel.com,
	luto@kernel.org, jmorris@namei.org, serge@hallyn.com,
	paul@paul-moore.com, eparis@parisplace.org, jethro@fortanix.com,
	Hansen, Dave, tglx@linutronix.de, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, nhorman@redhat.com,
	pmccallum@redhat.com, Ayoun, Serge, Katz-zamir, Shay,
	Huang, Haitao, andriy.shevchenko@linux.intel.com, Svahn, Kai,
	bp@alien8.de, josh@joshtriplett.org, Huang, Kai,
	rientjes@google.com, Roberts, William C, Tricca, Philip B
In-Reply-To: <20190614174556.GJ12191@linux.intel.com>

On Fri, Jun 14, 2019 at 10:45:56AM -0700, Sean Christopherson wrote:
> The state tracking of #2/#3 doesn't scare me, it's purely the auditing.
> Holding an audit message for an indeterminate amount of time is a
> nightmare.
> 
> Here's a thought.  What if we simply require FILE__EXECUTE or AA_EXEC_MAP
> to load any enclave page from a file?  Alternatively, we could add an SGX
> specific file policity, e.g. FILE__ENCLAVELOAD and AA_MAY_LOAD_ENCLAVE.
> As in my other email, SELinux's W^X restrictions can be tied to the process,
> i.e. they can be checked at mmap()/mprotect() without throwing a wrench in
> auditing.

We would also need to require VM_MAYEXEC on all enclave pages, or forego
enforcing path_noexec() for enclaves.

^ permalink raw reply

* Re: [PATCH V8 2/3] Define a new ima template field buf
From: prakhar srivastava @ 2019-06-14 17:52 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-integrity, linux-security-module, linux-kernel,
	Roberto Sassu, thiago Jung Bauermann
In-Reply-To: <1560521651.4072.7.camel@linux.ibm.com>

On Fri, Jun 14, 2019 at 7:14 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> > > > diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> > > > index 993d0f1915ff..c8591406c0e2 100644
> > > > --- a/security/integrity/ima/ima_init.c
> > > > +++ b/security/integrity/ima/ima_init.c
> > > > @@ -50,7 +50,7 @@ static int __init ima_add_boot_aggregate(void)
> > > >   struct ima_template_entry *entry;
> > > >   struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
> > > >   struct ima_event_data event_data = {iint, NULL, boot_aggregate_name,
> > > > -                                     NULL, 0, NULL};
> > > > +                                     NULL, 0, NULL, NULL, 0};
> > >
> > > here, don't belong in this patch.  It belongs in "IMA: support for per
> > > policy rule template formats", in case it should ever be backported.
> > >  Please post this as a separate patch, that will be squashed with
> > > "IMA: support for per policy rule template formats".
> >
> > My mistake.  I should have picked up Thaigo's "ima: Use designated
> > initializers for struct ima_event_data".  Please drop these changes
> > instead.
>
> Sorry for the confusion.  I just pushed out Thiago's patch.
>
Just to clarify:
- no split up of patch is needed.
- only formatting needs to cleaned up.

Apologies for the formatting issues, my editor switches back to
tab as 4 chars.

Thanks,
Prakhar Srivastava
> thanks,
>
> Mimi
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox