Re: [PATCH v9 00/51] powerpc, mm: Memory Protection Keys

[Ram Pai]

On Tue, Nov 07, 2017 at 02:47:10PM -0800, Dave Hansen wrote:
> On 11/07/2017 02:39 PM, Ram Pai wrote:
> > 
> > As per the current semantics of sys_pkey_free(); the way I understand it,
> > the calling thread is saying disassociate me from this key.
> 
> No.  It is saying: "this *process* no longer has any uses of this key,
> it can be reused".

ok, in light of the corrected semantics, I see no bug in the implimentation.

> On Mon, Nov 06, 2017 at 10:28:41PM +0100, Florian Weimer wrote:
...
> The problem is a pkey_alloc/pthread_create/pkey_free/pkey_alloc
> sequence.  The pthread_create call makes the new thread inherit the
> access rights of the current thread, but then the key is deallocated.
> Reallocation of the same key will have that thread retain its access
> rights, which is IMHO not correct.

Again.. in light of the corrected semantics --
 the child thread or any thread should not free
a key without cleaning up. 
(a) disassociate the key from its address space
(b) reset the permission on the key across all the threads of the
process.

Because any such uncleaned bits can cause unexpected behavior if the 
same key gets reallocated on sys_pkey_alloc().


-- 
Ram Pai

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

Re: [PATCH v9 00/51] powerpc, mm: Memory Protection Keys

[Dave Hansen]

On 11/07/2017 02:39 PM, Ram Pai wrote:
> 
> As per the current semantics of sys_pkey_free(); the way I understand it,
> the calling thread is saying disassociate me from this key.

No.  It is saying: "this *process* no longer has any uses of this key,
it can be reused".
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v9 00/51] powerpc, mm: Memory Protection Keys

[Ram Pai]

On Tue, Nov 07, 2017 at 08:32:16AM +0100, Florian Weimer wrote:
> * Ram Pai:
> 
> > On Mon, Nov 06, 2017 at 10:28:41PM +0100, Florian Weimer wrote:
> >> * Ram Pai:
> >> 
> >> > Testing:
> >> > ---
> >> > This patch series has passed all the protection key
> >> > tests available in the selftest directory.The
> >> > tests are updated to work on both x86 and powerpc.
> >> > The selftests have passed on x86 and powerpc hardware.
> >> 
> >> How do you deal with the key reuse problem?  Is it the same as x86-64,
> >> where it's quite easy to accidentally grant existing threads access to
> >> a just-allocated key, either due to key reuse or a changed init_pkru
> >> parameter?
> >
> > I am not sure how on x86-64, two threads get allocated the same key
> > at the same time? the key allocation is guarded under the mmap_sem
> > semaphore. So there cannot be a race where two threads get allocated
> > the same key.
> 
> The problem is a pkey_alloc/pthread_create/pkey_free/pkey_alloc
> sequence.  The pthread_create call makes the new thread inherit the
> access rights of the current thread, but then the key is deallocated.
> Reallocation of the same key will have that thread retain its access
> rights, which is IMHO not correct.

(Dave Hansen: please correct me if I miss-speak below)

As per the current semantics of sys_pkey_free(); the way I understand it,
the calling thread is saying disassociate me from this key. Other
threads continue to be associated with the key and could continue to
get key-faults, but this calling thread will not get key-faults on that
key any more.

Also the key should not get reallocated till all the threads in the process
have disassocated from the key; by calling sys_pkey_free().

>From that point of view, I think there is a bug in the implementation of
pkey on x86 and now on powerpc aswell.

> 
> > Can you point me to the issue, if it is already discussed somewhere?
> 
> See ‘MPK: pkey_free and key reuse’ on various lists (including
> linux-mm and linux-arch).
> 
> It has a test case attached which demonstrates the behavior.
> 
> > As far as the semantics is concerned, a key allocated in one thread's
> > context has no meaning if used in some other threads context within the
> > same process.  The app should not try to re-use a key allocated in a
> > thread's context in some other threads's context.
> 
> Uh-oh, that's not how this feature works on x86-64 at all.  There, the
> keys are a process-global resource.  Treating them per-thread
> seriously reduces their usefulness.

Sorry. I was not thinking right. Let me restate.

A key is a global resource, but the permissions on a key is
local to a thread. For eg: the same key could disable
access on a page for one thread, while it could disable write
on the same page on another thread.

> 
> >> What about siglongjmp from a signal handler?
> >
> > On powerpc there is some relief.  the permissions on a key can be
> > modified from anywhere, including from the signal handler, and the
> > effect will be immediate.  You dont have to wait till the
> > signal handler returns for the key permissions to be restore.
> 
> My concern is that the signal handler knows nothing about protection
> keys, but the current x86-64 semantics will cause it to clobber the
> access rights of the current thread.
> 
> > also after return from the sigsetjmp();
> > possibly caused by siglongjmp(), the program can restore the permission
> > on any key.
> 
> So that's not really an option.
> 
> > Atleast that is my theory. Can you give me a testcase; if you have one
> > handy.
> 
> The glibc patch I posted under the ‘MPK: pkey_free and key reuse’
> thread covers this, too.

thanks. will try the test case with my kernel patches. But, on
powerpc one can change the permissions on the key in the signal handler
which takes into effect immediately, there should not be a bug
in powerpc.

x86 has this requirement where it has to return from the signal handler
back to the kernel in order to change the permission on a key,
it can cause issues with longjump.

RP

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

RE: [PATCH v2 00/15] ima: digest list feature

[Safford, David (GE Global Research, US)]

> -Original Message-
> From: linux-integrity-ow...@vger.kernel.org [mailto:linux-integrity-
> ow...@vger.kernel.org] On Behalf Of Roberto Sassu
> Sent: Tuesday, November 07, 2017 5:37 AM
> To: linux-integr...@vger.kernel.org
> Cc: linux-security-mod...@vger.kernel.org; linux-fsde...@vger.kernel.org;
> linux-doc@vger.kernel.org; linux-ker...@vger.kernel.org;
> silviu.vlasce...@huawei.com; Roberto Sassu 
> Subject: EXT: [PATCH v2 00/15] ima: digest list feature
> 
> IMA is a security module with the objective of reporting or enforcing the
> integrity of a system, by measuring files accessed with the execve(),
> mmap() and open() system calls. For reporting, it takes advantage of the
> TPM and extends a PCR with the digest of an evaluated event. For enforcing,
> it returns a value which is zero if the operation should be allowed, negative 
> if
> it should be denied.
> 
> Measuring files of an operating system introduces three main issues. First,
> since the overhead introduced by the TPM is noticeable, the performance of
> the system decreases linearly with the number of measurements taken. 
> This can be seen especially at boot time.

If you want the measurement chain of trust, every link must be extended in the 
TPM.
This is inherent in the model. Doing local verification of TCB files is really 
no substitute.
Not to mention that leaving out "known" hashes from attestation eliminates the 
ability to do analytics on the patterns of usage of the good files. Local 
appraisal is a 
good thing, but not a complete substitute for remote attestation.

> Second, managing large measurement
> lists requires computation power and network bandwidth. 

So 200 nodes with 5000 entries, 100bytes per entry average (that's a pretty 
large TCB, but OK):
that's roughly .8 seconds total on a single Gb link.

> Third, it is
> necessary to obtain reference measurements (i.e. digests of software
> known to be good) to evaluate/enforce the integrity of the system. If file
> signatures are used to enforce access, Linux distribution vendors have to
> modify their building systems in order to include signatures in their 
> packages.

Or you can use the initial enrollment to transfer a reference manifest.
Or you can use SWIDS. Or you can sign everything yourself. (That's what we do.)

> Digest lists aim at mitigating these issues. A digest list is a list of 
> digests that
> are taken by IMA as reference measurements and loaded before files are
> accessed. Then, IMA compares calculated digests of accessed files with
> digests from loaded digest lists. If the digest is found, measurement,
> appraisal and audit are not performed.

So who manages the "good" hash lists? They have to go into the initramfs,
and be updated with every package update. And Leaving out attestation of 
good TCB files reduces the potential power of analytics.

> Multiple digest lists can be loaded at the same time, by providing to IMA
> metadata for each list: digest, signature and path. The digest is specified so
> that loaded digest lists can be identified only with the measurement of
> metadata. The signature is used for appraisal. If the verification succeeds,
> IMA loads the digest list even if security.ima is missing.
> 
> Digest lists address the first issue because the TPM is used only if the 
> digest
> of a measured file is unknown. On a minimal system, 10 of 1400
> measurements are unknown because of mutable files (e.g. log files).

At 5ms per extend, you at most save 7 seconds at boot. But the savings are
actually much less, as the extends run simultaneously with most of the
other boot operations. I typically can't tell the difference without a 
stopwatch.

> Digest lists mitigate the second issue because, since digest lists do not
> change, they don't have to be sent at every remote attestation. Sending
> unknown measurements and a reference to digest lists would be sufficient.

The .8 second isn't a problem, and even that can be pretty much eliminated by
sending just the delta measurements.

> Finally, digest lists address also the third issue because Linux distribution
> vendors already provide the digests of files included in each RPM package.
> The digest list is stored in the RPM header, signed by the vendor.

But then tooling is needed to select the desired hashes and put them in
the initramfs for loading.

I guess I don't see the problem, and think the cure introduces issues of its 
own.

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

[PATCH v2] documentation: fb: update list of available compiled-in fonts

[Randy Dunlap]

From: Randy Dunlap 

Update list of available compiled-in fonts in lib/fonts/:
add 6x10 and drop RomanLarge (which was reverted 12 years ago).

Also sort the list alphabetically.

Signed-off-by: Randy Dunlap 
Cc: Geert Uytterhoeven 
Acked-by: Geert Uytterhoeven  # v1
---
 Documentation/fb/fbcon.txt |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- lnx-414-rc8.orig/Documentation/fb/fbcon.txt
+++ lnx-414-rc8/Documentation/fb/fbcon.txt
@@ -77,8 +77,8 @@ C. Boot options
 1. fbcon=font:
 
 Select the initial font to use. The value 'name' can be any of the
-compiled-in fonts: VGA8x16, 7x14, 10x18, VGA8x8, MINI4x6, RomanLarge,
-SUN8x16, SUN12x22, ProFont6x11, Acorn8x8, PEARL8x8.
+compiled-in fonts: 10x18, 6x10, 7x14, Acorn8x8, MINI4x6,
+PEARL8x8, ProFont6x11, SUN12x22, SUN8x16, VGA8x16, VGA8x8.
 
Note, not all drivers can handle font with widths not divisible by 8,
 such as vga16fb.

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

Re: [PATCH v2 00/15] ima: digest list feature

[Matthew Garrett]

On Tue, Nov 7, 2017 at 12:53 PM, Roberto Sassu  wrote:
> On 11/7/2017 3:49 PM, Matthew Garrett wrote:
>> RPM's hardly universal, and distributions are in the process of moving
>> away from using it for distributing non-core applications (Flatpak and
>> Snap are becoming increasingly popular here). I think this needs to be
>> a generic solution rather than having the kernel tied to a specific
>> package format.
>
>
> Support for new digest list formats can be easily added. Digest list
> metadata includes the digest list type, so that the appropriate parser
> is selected.

But we're still left in a state where the kernel has to end up
supporting a number of very niche formats, and userland agility is
tied to the kernel. I think it makes significantly more sense to push
the problem out to userland.

> Digest lists should be parsed directly by the kernel, because processing
> the lists in userspace would increase the chances that a compromised
> tool does not upload to the kernel the expected digests. Also, digest
> lists must be processed before init, otherwise appraisal will deny the
> execution. Lastly, the mechanism of parsing files from the kernel is
> already used to parse the IMA policy.

Isn't failing to upload the expected digest list just a DoS? We
already expect to load keys from initramfs, so it seems fine to parse
stuff there - what's the problem with extracting information from
RPMs, translating them to the generic format and pushing that into the
kernel?
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v16 00/13] support "task_isolation" mode

[Christopher Lameter]

On Tue, 7 Nov 2017, Chris Metcalf wrote:

> > Presumably we have another context there were we may be able to call into
> > the cleanup code with interrupts enabled.
>
> Right now for task isolation we run with interrupts enabled during the
> initial sys_prctl() call, and call quiet_vmstat_sync() there, which currently
> calls refresh_cpu_vm_stats(false).  In fact we could certainly pass "true"
> there instead (and probably should) since we can handle dealing with
> the pagesets at this time.  As we return to userspace we will test that
> nothing surprising happened with vmstat; if so we jam an EAGAIN into
> the syscall result value, but if not, we will be in userspace and won't need
> to touch the vmstat counters until we next go back into the kernel.

If you do it too early and there is another page allocator action then it
may potentially repopulate the caches. Since this is only draining the
caches for remote node allocation it may be rare and not that important.

Re: [PATCH v2 00/15] ima: digest list feature

[Roberto Sassu]

On 11/7/2017 3:49 PM, Matthew Garrett wrote:

On Tue, Nov 7, 2017 at 2:36 AM, Roberto Sassu  wrote:

Finally, digest lists address also the third issue because Linux
distribution vendors already provide the digests of files included in each
RPM package. The digest list is stored in the RPM header, signed by the
vendor.


RPM's hardly universal, and distributions are in the process of moving
away from using it for distributing non-core applications (Flatpak and
Snap are becoming increasingly popular here). I think this needs to be
a generic solution rather than having the kernel tied to a specific
package format.


Support for new digest list formats can be easily added. Digest list
metadata includes the digest list type, so that the appropriate parser
is selected.

I defined a new generic format for digest lists in Patch 7/15. I would
appreciate if we can discuss this format, and if you can give me
suggestions about how to improve it. I think it would not be a problem
to support your use case and associate metadata to each digest.

Digest lists should be parsed directly by the kernel, because processing
the lists in userspace would increase the chances that a compromised
tool does not upload to the kernel the expected digests. Also, digest
lists must be processed before init, otherwise appraisal will deny the
execution. Lastly, the mechanism of parsing files from the kernel is
already used to parse the IMA policy.

Roberto

--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v16 00/13] support "task_isolation" mode

[Chris Metcalf]

On 11/7/2017 12:10 PM, Christopher Lameter wrote:

On Mon, 6 Nov 2017, Chris Metcalf wrote:


On 11/6/2017 10:38 AM, Christopher Lameter wrote:

What about that d*mn 1 Hz clock?

It's still there, so this code still requires some further work before
it can actually get a process into long-term task isolation (without
the obvious one-line kernel hack).  Frederic suggested a while ago
forcing updates on cpustats was required as the last gating factor; do
we think that is still true?  Christoph was working on this at one
point - any progress from your point of view?

Well if you still have the 1 HZ clock then you can simply defer the numa
remote page cleanup of the page allocator to that the time you execute
that tick.

We have to get rid of the 1 Hz tick, so we don't want to tie anything
else to it...

Yes we want to get rid of the 1 HZ tick but the work on that could also
include dealing with the remove page cleanup issue that we have deferred.

Presumably we have another context there were we may be able to call into
the cleanup code with interrupts enabled.


Right now for task isolation we run with interrupts enabled during the
initial sys_prctl() call, and call quiet_vmstat_sync() there, which 
currently

calls refresh_cpu_vm_stats(false).  In fact we could certainly pass "true"
there instead (and probably should) since we can handle dealing with
the pagesets at this time.  As we return to userspace we will test that
nothing surprising happened with vmstat; if so we jam an EAGAIN into
the syscall result value, but if not, we will be in userspace and won't need
to touch the vmstat counters until we next go back into the kernel.

--
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com

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

Re: [PATCH v16 00/13] support "task_isolation" mode

[Christopher Lameter]

On Mon, 6 Nov 2017, Chris Metcalf wrote:

> On 11/6/2017 10:38 AM, Christopher Lameter wrote:
> > > What about that d*mn 1 Hz clock?
> > >
> > > It's still there, so this code still requires some further work before
> > > it can actually get a process into long-term task isolation (without
> > > the obvious one-line kernel hack).  Frederic suggested a while ago
> > > forcing updates on cpustats was required as the last gating factor; do
> > > we think that is still true?  Christoph was working on this at one
> > > point - any progress from your point of view?
> > Well if you still have the 1 HZ clock then you can simply defer the numa
> > remote page cleanup of the page allocator to that the time you execute
> > that tick.
>
> We have to get rid of the 1 Hz tick, so we don't want to tie anything
> else to it...

Yes we want to get rid of the 1 HZ tick but the work on that could also
include dealing with the remove page cleanup issue that we have deferred.

Presumably we have another context there were we may be able to call into
the cleanup code with interrupts enabled.

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

Re: [PATCH v2 00/15] ima: digest list feature

[Roberto Sassu]

On 11/7/2017 2:37 PM, Mimi Zohar wrote:

Hi Roberto,

On Tue, 2017-11-07 at 11:36 +0100, Roberto Sassu wrote:

IMA is a security module with the objective of reporting or enforcing the
integrity of a system, by measuring files accessed with the execve(),
mmap() and open() system calls. For reporting, it takes advantage of the
TPM and extends a PCR with the digest of an evaluated event. For enforcing,
it returns a value which is zero if the operation should be allowed,
negative if it should be denied.

Measuring files of an operating system introduces three main issues. First,
since the overhead introduced by the TPM is noticeable, the performance of
the system decreases linearly with the number of measurements taken. This
can be seen especially at boot time.


I've said this previously.  The solution IS FIRST to improve the
performance of the TPM device driver, before finding solutions around
it.

TPM performance patches:
a233a0289cf9 "tpm: msleep() delays - replace with usleep_range() in i2c nuvoton 
driver"
0afb7118ae02 "tpm: add sleep only for retry in i2c_nuvoton_write_status()"
9f3fc7bcddcb "tpm: replace msleep() with  usleep_range() in TPM 1.2/2.0 generic 
drivers"

Nayna Jain submitted additional performance improvements, that were posted
https://www.spinics.net/lists/linux-integrity/msg00238.html and are
currently being tested.

Even after these TPM performance improvements, there are still more
TPM performance improvements.


Hi Mimi

I applied the patches you mentioned, and indeed the performance
improvement is great, from 1 minute and 30 seconds to 24 seconds
compared to the normal boot time of 8.5 seconds. However, especially for
embedded devices performances requirements might be more strict, and
much more files might be measured. For desktops, also it would be
desirable to have low latency.



Second, managing large measurement
lists requires computation power and network bandwidth.


"Large" for whom?  Large for the attestation server?  Large for the
client?  Smaller devices would have fewer measurements than larger
devices.  We're not discussing gigabytes/terabytes of data here.
  Attestation servers should be able to handle the bandwidth.  If it
becomes a problem, then the attestation server/client communication
could be optimized to send just the recent measurements, not the
entire measurement list.


I'm not considering an individual system, but a datacenter with several
nodes. An attestation server processing for example 200 measurement
lists with 5000 entries must be much more powerful than one that
processes the same number of lists with 10 entries.



Third, it is
necessary to obtain reference measurements (i.e. digests of software known
to be good) to evaluate/enforce the integrity of the system. If file
signatures are used to enforce access, Linux distribution vendors have to
modify their building systems in order to include signatures in their
packages.


Although IMA-appraisal verifies file integrity based on either a file
hash or signature, they are not equivalent.  File signatures provide
file provenance.  Not only does the file hash have to match, but a
certificate used to sign the file data must be loaded onto the IMA
keyring.  File hashes should be limited to mutable files.


Digest lists work the same. If appraisal is in enforcing mode, digest
lists must have a valid digital signature.



Instead of working around the problem of a lack of file signatures in
software packages, help promote including them so that there are
measurement and signature chains of trust anchored in hardware.


One of the key point of digest lists feature is that it reuses
information that is already available, while providing the same security
properties. I find it difficult to promote a solution that would
introduce redundant information and complicate the management of Linux
distributions.



Digest lists aim at mitigating these issues. A digest list is a list of
digests that are taken by IMA as reference measurements and loaded before
files are accessed. Then, IMA compares calculated digests of accessed files
with digests from loaded digest lists. If the digest is found, measurement,
appraisal and audit are not performed.


Although the previous patch set did not break userspace per-se, it
changed the existing meaning of the IMA measurement list.  Without
taking into account my previous comments, this patch set makes similar
changes to IMA-appraisal and IMA-measurement.


For appraisal, I think only the verification method changes: instead of
verifying file signatures individually, IMA verifies one signature per
many file digests.



Instead of including the individual file measurements, the previous
version of this patch set, I assume it hasn't changed, includes a hash
of a file containing a list of all potential file measurements, not
the actual file measurements.


Digest lists do not introduce false positives due to not including a
measurement. Files whose digest is included in a digest 

Re: [PATCH] Documentation/rs485: Fix missing zero init on sample code

[Ricardo Ribalda Delgado]

Hi Jiri

On Tue, Nov 7, 2017 at 2:28 PM, Jiri Slaby  wrote:
> On 11/06/2017, 11:51 AM, Ricardo Ribalda Delgado wrote:
>> The sample code does not initialize to zero a local variable and then it
>> uses the uninitialized code.
>>
>> Fix in case someone copy/paste the sample code.
>>
>> Signed-off-by: Ricardo Ribalda Delgado 
>> ---
>>  Documentation/serial/serial-rs485.txt | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/serial/serial-rs485.txt 
>> b/Documentation/serial/serial-rs485.txt
>> index 389fcd4759e9..2bbae93bf279 100644
>> --- a/Documentation/serial/serial-rs485.txt
>> +++ b/Documentation/serial/serial-rs485.txt
>> @@ -54,7 +54,7 @@
>>   /* Error handling. See errno. */
>>   }
>>
>> - struct serial_rs485 rs485conf;
>> + struct serial_rs485 rs485conf = {};
>
> I think that TIOCGRS485 is missing here instead. Ccing the author of the
> text.
>
That will also do.



> Does it break classifying potatoes :)?

The potato machine has to speak to other industrial machines :).

Hey!: I am running of potatoes!, Need another truck.

>
>>   /* Enable RS485 mode: */
>>   rs485conf.flags |= SER_RS485_ENABLED;
>>
>
> thanks,
> --
> js
> suse labs



-- 
Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/15] ima: digest list feature

[Matthew Garrett]

On Tue, Nov 7, 2017 at 2:36 AM, Roberto Sassu  wrote:
> Finally, digest lists address also the third issue because Linux
> distribution vendors already provide the digests of files included in each
> RPM package. The digest list is stored in the RPM header, signed by the
> vendor.

RPM's hardly universal, and distributions are in the process of moving
away from using it for distributing non-core applications (Flatpak and
Snap are becoming increasingly popular here). I think this needs to be
a generic solution rather than having the kernel tied to a specific
package format.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/15] ima: digest list feature

[Mimi Zohar]

Hi Roberto,

On Tue, 2017-11-07 at 11:36 +0100, Roberto Sassu wrote:
> IMA is a security module with the objective of reporting or enforcing the
> integrity of a system, by measuring files accessed with the execve(),
> mmap() and open() system calls. For reporting, it takes advantage of the
> TPM and extends a PCR with the digest of an evaluated event. For enforcing,
> it returns a value which is zero if the operation should be allowed,
> negative if it should be denied.
> 
> Measuring files of an operating system introduces three main issues. First,
> since the overhead introduced by the TPM is noticeable, the performance of
> the system decreases linearly with the number of measurements taken. This
> can be seen especially at boot time.

I've said this previously.  The solution IS FIRST to improve the
performance of the TPM device driver, before finding solutions around
it.

TPM performance patches:
a233a0289cf9 "tpm: msleep() delays - replace with usleep_range() in i2c nuvoton 
driver"
0afb7118ae02 "tpm: add sleep only for retry in i2c_nuvoton_write_status()"
9f3fc7bcddcb "tpm: replace msleep() with  usleep_range() in TPM 1.2/2.0 generic 
drivers"

Nayna Jain submitted additional performance improvements, that were posted
https://www.spinics.net/lists/linux-integrity/msg00238.html and are
currently being tested.

Even after these TPM performance improvements, there are still more
TPM performance improvements.

> Second, managing large measurement
> lists requires computation power and network bandwidth.

"Large" for whom?  Large for the attestation server?  Large for the
client?  Smaller devices would have fewer measurements than larger
devices.  We're not discussing gigabytes/terabytes of data here.
 Attestation servers should be able to handle the bandwidth.  If it
becomes a problem, then the attestation server/client communication
could be optimized to send just the recent measurements, not the
entire measurement list.

> Third, it is
> necessary to obtain reference measurements (i.e. digests of software known
> to be good) to evaluate/enforce the integrity of the system. If file
> signatures are used to enforce access, Linux distribution vendors have to
> modify their building systems in order to include signatures in their
> packages.

Although IMA-appraisal verifies file integrity based on either a file
hash or signature, they are not equivalent.  File signatures provide
file provenance.  Not only does the file hash have to match, but a
certificate used to sign the file data must be loaded onto the IMA
keyring.  File hashes should be limited to mutable files.

Instead of working around the problem of a lack of file signatures in
software packages, help promote including them so that there are
measurement and signature chains of trust anchored in hardware.

> Digest lists aim at mitigating these issues. A digest list is a list of
> digests that are taken by IMA as reference measurements and loaded before
> files are accessed. Then, IMA compares calculated digests of accessed files
> with digests from loaded digest lists. If the digest is found, measurement,
> appraisal and audit are not performed.

Although the previous patch set did not break userspace per-se, it
changed the existing meaning of the IMA measurement list.  Without
taking into account my previous comments, this patch set makes similar
changes to IMA-appraisal and IMA-measurement.

Instead of including the individual file measurements, the previous
version of this patch set, I assume it hasn't changed, includes a hash
of a file containing a list of all potential file measurements, not
the actual file measurements.

Instead of changing the meaning of the IMA measurement list, I
previously suggested defining a new securityfs file for this purpose.

> Multiple digest lists can be loaded at the same time, by providing to IMA
> metadata for each list: digest, signature and path. The digest is specified
> so that loaded digest lists can be identified only with the measurement of
> metadata. The signature is used for appraisal. If the verification
> succeeds, IMA loads the digest list even if security.ima is missing.

Previously IMA-appraisal verified the file signature of the file
containing the file hashes.  It now sounds like even this guarantee is
gone.

Normally, the protection of kernel memory is out of scope for IMA.
This patch set introduces an in kernel white list, which would be a
prime target for attackers looking for ways of by-passing IMA-
measurement, IMA-appraisal and IMA-audit.  Others might disagree, but
from my perspective, this risk is too high.

Mimi

> Digest lists address the first issue because the TPM is used only if the
> digest of a measured file is unknown. On a minimal system, 10 of 1400
> measurements are unknown because of mutable files (e.g. log files).
> 
> Digest lists mitigate the second issue because, since digest lists do not
> change, they don't have to be sent at every remote attestation. Sending
> 

Re: [PATCHv3 1/1] locking/qspinlock/x86: Avoid test-and-set when PV_DEDICATED is set

[Paolo Bonzini]

On 07/11/2017 13:39, Eduardo Valentin wrote:
>> is this still needed after Waiman's patch to adaptively switch between
>> tas and pvqspinlock?
> Can you please point me to it ? Is it already in tip/master?
> 

No, he just posted it:

https://marc.info/?l=linux-kernel=150972337909996=2

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

Re: [PATCHv3 1/1] locking/qspinlock/x86: Avoid test-and-set when PV_DEDICATED is set

[Eduardo Valentin]

On Tue, Nov 07, 2017 at 01:23:56PM +0100, Paolo Bonzini wrote:
> On 06/11/2017 21:26, Eduardo Valentin wrote:
> > Currently, the existing qspinlock implementation will fallback to
> > test-and-set if the hypervisor has not set the PV_UNHALT flag.
> > 
> > This patch gives the opportunity to guest kernels to select
> > between test-and-set and the regular queueu fair lock implementation
> > based on the PV_DEDICATED KVM feature flag. When the PV_DEDICATED
> > flag is not set, the code will still fall back to test-and-set,
> > but when the PV_DEDICATED flag is set, the code will use
> > the regular queue spinlock implementation.
> > 
> > With this patch, when in autoselect mode, the guest will
> > use the default spinlock implementation based on host feature
> > flags as follows:
> > 
> > PV_DEDICATED = 1, PV_UNHALT = anything: default is qspinlock
> > PV_DEDICATED = 0, PV_UNHALT = 1: default is pvqspinlock
> > PV_DEDICATED = 0, PV_UNHALT = 0: default is tas
> 
> Hi Eduardo,
> 
> besides the suggestion to use a separate word than the one for features,

Ok. I will take a look.

> is this still needed after Waiman's patch to adaptively switch between
> tas and pvqspinlock?

Can you please point me to it ? Is it already in tip/master?

> 
> Paolo
> 
> > Cc: Paolo Bonzini 
> > Cc: "Radim Krčmář" 
> > Cc: Jonathan Corbet 
> > Cc: Thomas Gleixner 
> > Cc: Ingo Molnar 
> > Cc: "H. Peter Anvin" 
> > Cc: x...@kernel.org
> > Cc: Peter Zijlstra 
> > Cc: Waiman Long 
> > Cc: k...@vger.kernel.org
> > Cc: linux-doc@vger.kernel.org
> > Cc: linux-ker...@vger.kernel.org
> > Cc: Jan H. Schoenherr 
> > Cc: Anthony Liguori 
> > Suggested-by: Matt Wilson 
> > Signed-off-by: Eduardo Valentin 
> > ---
> > V3:
> >  - When PV_DEDICATED is set (1), qspinlock is selected,
> >regardless of the value of PV_UNHAULT. Suggested by Paolo Bonzini. 
> >  - Refreshed on top of tip/master.
> > V2:
> >  - rebase on top of tip/master
> > 
> >  Documentation/virtual/kvm/cpuid.txt  | 6 ++
> >  arch/x86/include/asm/qspinlock.h | 4 
> >  arch/x86/include/uapi/asm/kvm_para.h | 1 +
> >  arch/x86/kernel/kvm.c| 2 ++
> >  4 files changed, 13 insertions(+)
> > 
> > diff --git a/Documentation/virtual/kvm/cpuid.txt 
> > b/Documentation/virtual/kvm/cpuid.txt
> > index 3c65feb..117066a 100644
> > --- a/Documentation/virtual/kvm/cpuid.txt
> > +++ b/Documentation/virtual/kvm/cpuid.txt
> > @@ -54,6 +54,12 @@ KVM_FEATURE_PV_UNHALT  || 7 || guest 
> > checks this feature bit
> > ||   || before enabling 
> > paravirtualized
> > ||   || spinlock support.
> >  
> > --
> > +KVM_FEATURE_PV_DEDICATED   || 8 || guest checks this feature 
> > bit
> > +   ||   || to determine if they run on
> > +   ||   || dedicated vCPUs, allowing 
> > opti-
> > +   ||   || mizations such as usage of
> > +   ||   || qspinlocks.
> > +--
> >  KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||24 || host will warn if no 
> > guest-side
> > ||   || per-cpu warps are expected 
> > in
> > ||   || kvmclock.
> > diff --git a/arch/x86/include/asm/qspinlock.h 
> > b/arch/x86/include/asm/qspinlock.h
> > index 5e16b5d..de42694 100644
> > --- a/arch/x86/include/asm/qspinlock.h
> > +++ b/arch/x86/include/asm/qspinlock.h
> > @@ -3,6 +3,8 @@
> >  #define _ASM_X86_QSPINLOCK_H
> >  
> >  #include 
> > +#include 
> > +
> >  #include 
> >  #include 
> >  #include 
> > @@ -58,6 +60,8 @@ static inline bool virt_spin_lock(struct qspinlock *lock)
> > if (!static_branch_likely(_spin_lock_key))
> > return false;
> >  
> > +   if (kvm_para_has_feature(KVM_FEATURE_PV_DEDICATED))
> > +   return false;
> > /*
> >  * On hypervisors without PARAVIRT_SPINLOCKS support we fall
> >  * back to a Test-and-Set spinlock, because fair locks have
> > diff --git a/arch/x86/include/uapi/asm/kvm_para.h 
> > b/arch/x86/include/uapi/asm/kvm_para.h
> > index 554aa8f..85a9875 100644
> > --- a/arch/x86/include/uapi/asm/kvm_para.h
> > +++ b/arch/x86/include/uapi/asm/kvm_para.h
> > @@ -25,6 +25,7 @@
> >  #define KVM_FEATURE_STEAL_TIME 5
> >  #define KVM_FEATURE_PV_EOI 6
> >  #define KVM_FEATURE_PV_UNHALT  7
> > +#define KVM_FEATURE_PV_DEDICATED   8
> >  
> >  /* The last 8 bits are used to indicate how to interpret the flags field
> >   * in pvclock 

Re: [PATCHv3 1/1] locking/qspinlock/x86: Avoid test-and-set when PV_DEDICATED is set

[Paolo Bonzini]

On 06/11/2017 21:26, Eduardo Valentin wrote:
> Currently, the existing qspinlock implementation will fallback to
> test-and-set if the hypervisor has not set the PV_UNHALT flag.
> 
> This patch gives the opportunity to guest kernels to select
> between test-and-set and the regular queueu fair lock implementation
> based on the PV_DEDICATED KVM feature flag. When the PV_DEDICATED
> flag is not set, the code will still fall back to test-and-set,
> but when the PV_DEDICATED flag is set, the code will use
> the regular queue spinlock implementation.
> 
> With this patch, when in autoselect mode, the guest will
> use the default spinlock implementation based on host feature
> flags as follows:
> 
> PV_DEDICATED = 1, PV_UNHALT = anything: default is qspinlock
> PV_DEDICATED = 0, PV_UNHALT = 1: default is pvqspinlock
> PV_DEDICATED = 0, PV_UNHALT = 0: default is tas

Hi Eduardo,

besides the suggestion to use a separate word than the one for features,
is this still needed after Waiman's patch to adaptively switch between
tas and pvqspinlock?

Paolo

> Cc: Paolo Bonzini 
> Cc: "Radim Krčmář" 
> Cc: Jonathan Corbet 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: x...@kernel.org
> Cc: Peter Zijlstra 
> Cc: Waiman Long 
> Cc: k...@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: Jan H. Schoenherr 
> Cc: Anthony Liguori 
> Suggested-by: Matt Wilson 
> Signed-off-by: Eduardo Valentin 
> ---
> V3:
>  - When PV_DEDICATED is set (1), qspinlock is selected,
>regardless of the value of PV_UNHAULT. Suggested by Paolo Bonzini. 
>  - Refreshed on top of tip/master.
> V2:
>  - rebase on top of tip/master
> 
>  Documentation/virtual/kvm/cpuid.txt  | 6 ++
>  arch/x86/include/asm/qspinlock.h | 4 
>  arch/x86/include/uapi/asm/kvm_para.h | 1 +
>  arch/x86/kernel/kvm.c| 2 ++
>  4 files changed, 13 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/cpuid.txt 
> b/Documentation/virtual/kvm/cpuid.txt
> index 3c65feb..117066a 100644
> --- a/Documentation/virtual/kvm/cpuid.txt
> +++ b/Documentation/virtual/kvm/cpuid.txt
> @@ -54,6 +54,12 @@ KVM_FEATURE_PV_UNHALT  || 7 || guest 
> checks this feature bit
> ||   || before enabling 
> paravirtualized
> ||   || spinlock support.
>  
> --
> +KVM_FEATURE_PV_DEDICATED   || 8 || guest checks this feature bit
> +   ||   || to determine if they run on
> +   ||   || dedicated vCPUs, allowing 
> opti-
> +   ||   || mizations such as usage of
> +   ||   || qspinlocks.
> +--
>  KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||24 || host will warn if no 
> guest-side
> ||   || per-cpu warps are expected in
> ||   || kvmclock.
> diff --git a/arch/x86/include/asm/qspinlock.h 
> b/arch/x86/include/asm/qspinlock.h
> index 5e16b5d..de42694 100644
> --- a/arch/x86/include/asm/qspinlock.h
> +++ b/arch/x86/include/asm/qspinlock.h
> @@ -3,6 +3,8 @@
>  #define _ASM_X86_QSPINLOCK_H
>  
>  #include 
> +#include 
> +
>  #include 
>  #include 
>  #include 
> @@ -58,6 +60,8 @@ static inline bool virt_spin_lock(struct qspinlock *lock)
>   if (!static_branch_likely(_spin_lock_key))
>   return false;
>  
> + if (kvm_para_has_feature(KVM_FEATURE_PV_DEDICATED))
> + return false;
>   /*
>* On hypervisors without PARAVIRT_SPINLOCKS support we fall
>* back to a Test-and-Set spinlock, because fair locks have
> diff --git a/arch/x86/include/uapi/asm/kvm_para.h 
> b/arch/x86/include/uapi/asm/kvm_para.h
> index 554aa8f..85a9875 100644
> --- a/arch/x86/include/uapi/asm/kvm_para.h
> +++ b/arch/x86/include/uapi/asm/kvm_para.h
> @@ -25,6 +25,7 @@
>  #define KVM_FEATURE_STEAL_TIME   5
>  #define KVM_FEATURE_PV_EOI   6
>  #define KVM_FEATURE_PV_UNHALT7
> +#define KVM_FEATURE_PV_DEDICATED 8
>  
>  /* The last 8 bits are used to indicate how to interpret the flags field
>   * in pvclock structure. If no bits are set, all flags are ignored.
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 8bb9594..dacd7cf 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -642,6 +642,8 @@ void __init kvm_spinlock_init(void)
>  {
>   if (!kvm_para_available())
>   return;
> + if 

Re: [PATCH 1/1] usb: gadget: add USB Audio Device Class 3.0 gadget support

[kbuild test robot]

Hi Ruslan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on balbi-usb/next]
[also build test ERROR on v4.14-rc8 next-20171107]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Ruslan-Bilovol/usb-gadget-add-USB-Audio-Device-Class-3-0-gadget-support/20171107-175202
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> drivers/usb/gadget/function/f_uac3.c:10:32: fatal error: 
>> linux/usb/audio-v3.h: No such file or directory
#include 
   ^
   compilation terminated.

vim +10 drivers/usb/gadget/function/f_uac3.c

  > 10  #include 
11  #include 
12  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

[PATCH v2 02/15] ima: generalize ima_write_policy()

[Roberto Sassu]

This patch renames ima_write_policy() to ima_write_data(). Also, it
determines the kernel_read_file_id from the dentry associated to the file,
and passes it to ima_read_file().

Signed-off-by: Roberto Sassu 
---
 security/integrity/ima/ima_fs.c | 55 ++---
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 27de4558303e..864d34581081 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -28,6 +28,21 @@
 
 static DEFINE_MUTEX(ima_write_mutex);
 
+static struct dentry *ima_dir;
+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;
+
+static enum kernel_read_file_id ima_get_file_id(struct dentry *dentry)
+{
+   if (dentry == ima_policy)
+   return READING_POLICY;
+
+   return READING_UNKNOWN;
+}
+
 bool ima_canonical_fmt;
 static int __init default_canonical_fmt_setup(char *str)
 {
@@ -315,11 +330,12 @@ 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;
+   enum kernel_read_file_id file_id = ima_get_file_id(file->f_path.dentry);
 
if (datalen >= PAGE_SIZE)
datalen = PAGE_SIZE - 1;
@@ -340,34 +356,33 @@ static ssize_t ima_write_policy(struct file *file, const 
char __user *buf,
goto out_free;
 
if (data[0] == '/') {
-   result = ima_read_file(data, READING_POLICY);
-   } else if (ima_appraise & IMA_APPRAISE_POLICY) {
-   pr_err("IMA: 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;
+   result = ima_read_file(data, file_id);
+   } else if (file_id == READING_POLICY) {
+   if (ima_appraise & IMA_APPRAISE_POLICY) {
+   pr_err("IMA: 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(_write_mutex);
 out_free:
kfree(data);
 out:
-   if (result < 0)
+   if (file_id == READING_POLICY && result < 0)
valid_policy = 0;
 
return result;
 }
 
-static struct dentry *ima_dir;
-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,
 };
@@ -446,7 +461,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.11.0

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

[PATCH v2 07/15] ima: add parser of compact digest list

[Roberto Sassu]

This patch introduces the parser for the compact digest list.
Its format is:

entry_id[2] count[4] data_len[4]
data[data_len]
entry_id[2] count[4] data_len[4]
data[data_len]
...

entry_id, count and data_len are in little endian.

This format is suitable to store a large number of digests, as there is no
metadata provided for each. Digests (which have all the same size) are
concatenated together and placed after the header.

COMPACT_DIGEST (0) and COMPACT_DIGEST_MUTABLE (1) entry IDs are supported.

If the entry ID is COMPACT_DIGEST and appraisal is in enforcing mode, file
updates are denied. If the entry ID is COMPACT_DIGEST_MUTABLE, file updates
are permitted.

Changelog

v1:
- Renamed COMPACT_LIST_ID_DIGEST to COMPACT_DIGEST
- Added support for immutable/mutable files

Signed-off-by: Roberto Sassu 
---
 security/integrity/ima/ima_digest_list.c | 66 
 1 file changed, 66 insertions(+)

diff --git a/security/integrity/ima/ima_digest_list.c 
b/security/integrity/ima/ima_digest_list.c
index 28172424e5a2..6ad00ba32c94 100644
--- a/security/integrity/ima/ima_digest_list.c
+++ b/security/integrity/ima/ima_digest_list.c
@@ -23,6 +23,69 @@ enum digest_metadata_fields {DATA_ALGO, DATA_DIGEST, 
DATA_SIGNATURE,
 DATA_FILE_PATH, DATA_REF_ID, DATA_TYPE,
 DATA__LAST};
 
+enum digest_data_types {DATA_TYPE_COMPACT_LIST};
+
+enum compact_list_entry_ids {COMPACT_DIGEST, COMPACT_DIGEST_MUTABLE};
+
+struct compact_list_hdr {
+   u16 entry_id;
+   u32 count;
+   u32 datalen;
+} __packed;
+
+static int ima_parse_compact_list(loff_t size, void *buf)
+{
+   void *bufp = buf, *bufendp = buf + size;
+   int digest_len = hash_digest_size[ima_hash_algo];
+   struct compact_list_hdr *hdr;
+   u8 is_mutable = 0;
+   int ret, i;
+
+   while (bufp < bufendp) {
+   if (bufp + sizeof(*hdr) > bufendp) {
+   pr_err("compact list, missing header\n");
+   return -EINVAL;
+   }
+
+   hdr = bufp;
+
+   if (ima_canonical_fmt) {
+   hdr->entry_id = le16_to_cpu(hdr->entry_id);
+   hdr->count = le32_to_cpu(hdr->count);
+   hdr->datalen = le32_to_cpu(hdr->datalen);
+   }
+
+   switch (hdr->entry_id) {
+   case COMPACT_DIGEST_MUTABLE:
+   is_mutable = 1;
+   case COMPACT_DIGEST:
+   break;
+   default:
+   pr_err("compact list, invalid data type\n");
+   return -EINVAL;
+   }
+
+   bufp += sizeof(*hdr);
+
+   for (i = 0; i < hdr->count &&
+bufp + digest_len <= bufendp; i++) {
+   ret = ima_add_digest_data_entry(bufp, is_mutable);
+   if (ret < 0 && ret != -EEXIST)
+   return ret;
+
+   bufp += digest_len;
+   }
+
+   if (i != hdr->count ||
+   bufp != (void *)hdr + sizeof(*hdr) + hdr->datalen) {
+   pr_err("compact list, invalid data\n");
+   return -EINVAL;
+   }
+   }
+
+   return 0;
+}
+
 static int ima_parse_digest_list_data(struct ima_field_data *data)
 {
void *digest_list;
@@ -47,6 +110,9 @@ static int ima_parse_digest_list_data(struct ima_field_data 
*data)
}
 
switch (data_type) {
+   case DATA_TYPE_COMPACT_LIST:
+   ret = ima_parse_compact_list(digest_list_size, digest_list);
+   break;
default:
pr_err("Parser for data type %d not implemented\n", data_type);
ret = -EINVAL;
-- 
2.11.0

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

[PATCH v2 06/15] ima: add parser of digest lists metadata

[Roberto Sassu]

Digest lists can be uploaded to IMA by supplying the path of their
metadata.

Digest list metadata are:

- DATA_ALGO: algorithm of the digests to be uploaded
- DATA_DIGEST: digest of the file containing the digest list
- DATA_SIGNATURE: signature of the file containing the digest list
- DATA_FILE_PATH: pathname
- DATA_REF_ID: reference ID of the digest list
- DATA_TYPE: type of digest list

The new function ima_load_digest_list_metadata() loads digest list metadata
from a predefined position (/etc/ima/digest_lists/metadata), when rootfs
becomes available. Digest lists must be loaded before IMA appraisal is in
enforcing mode.

The new function ima_parse_digest_list_metadata() parses the metadata and
loads each digest list individually. Then, it parses the data according to
the data type specified.

To avoid the delay due to extending a PCR for each digest list, digests of
digest lists are added to the hash table. If appraisal is in enforcing
mode, this is done only if the signature verification succeeds. IMA does
not add the digest of an accessed file to the measurement list if the
digest is found in the hash table.

Changelog

v1:
- Verify signature of digest lists if appraisal is enabled
- Load digest lists when rootfs is available
- Ignore digest lists if no policy is loaded

Signed-off-by: Roberto Sassu 
---
 include/linux/fs.h   |   2 +
 security/integrity/iint.c|   1 +
 security/integrity/ima/Kconfig   |  19 
 security/integrity/ima/Makefile  |   1 +
 security/integrity/ima/ima.h |  12 +++
 security/integrity/ima/ima_digest_list.c | 152 +++
 security/integrity/integrity.h   |   8 ++
 7 files changed, 195 insertions(+)
 create mode 100644 security/integrity/ima/ima_digest_list.c

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8d7d2850963c..06737235665b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2793,6 +2793,8 @@ extern int do_pipe_flags(int *, int);
id(KEXEC_INITRAMFS, kexec-initramfs)\
id(POLICY, security-policy) \
id(X509_CERTIFICATE, x509-certificate)  \
+   id(DIGEST_LIST_METADATA, digest-list-metadata)  \
+   id(DIGEST_LIST, digest-list)\
id(MAX_ID, )
 
 #define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index c84e05866052..68c14d1dfc0c 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -209,4 +209,5 @@ void __init integrity_load_keys(void)
 {
ima_load_x509();
evm_load_x509();
+   ima_load_digest_list_metadata();
 }
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 35ef69312811..fa40cee1e1a2 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -227,3 +227,22 @@ config IMA_APPRAISE_SIGNED_INIT
default n
help
   This option requires user-space init to be signed.
+
+config IMA_DIGEST_LIST
+   bool "Measure/appraise/audit files depending on uploaded digest lists"
+   depends on IMA
+   default n
+   help
+  This option allows users to load digest lists. If a measured file
+  has the same digest of one from loaded lists, IMA will not create
+  a new measurement entry or an audit log. They will be created only
+  when digest lists are loaded. If appraisal is enabled, access will
+  be permitted if the digest is in the digest list. File updates
+  will be permitted if, in addition, the digest is marked as mutable.
+
+config IMA_DIGEST_LIST_METADATA_PATH
+   string "IMA digest list metadata path"
+   depends on IMA_DIGEST_LIST
+   default "/etc/ima/digest_lists/metadata"
+   help
+  This option defines IMA digest list metadata path.
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 29f198bde02b..00dbe3a8cb71 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -9,4 +9,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.h b/security/integrity/ima/ima.h
index 1f6591a57fea..1f43284788eb 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -158,6 +158,18 @@ int ima_restore_measurement_entry(struct 
ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
 struct ima_digest *ima_lookup_loaded_digest(u8 *digest);
 int ima_add_digest_data_entry(u8 *digest, u8 is_mutable);
+#ifdef CONFIG_IMA_DIGEST_LIST
+void __init ima_load_digest_list_metadata(void);
+ssize_t 

[PATCH v2 09/15] ima: introduce securityfs interfaces for digest lists

[Roberto Sassu]

This patch introduces the file 'digest_lists' in the securityfs filesystem,
to load digest lists metadata. IMA will parse the metadata and load the
digest lists from the path provided.

It also introduces 'digests_count', to show the number of digests stored in
the ima_digests_htable hash table.

Signed-off-by: Roberto Sassu 

Changelog

v1:
- Deny upload of digest lists if no policy is loaded
---
 security/integrity/ima/ima_fs.c | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 4158ced5d3c9..1ed717d94487 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -34,11 +34,15 @@ static struct dentry *ascii_runtime_measurements;
 static struct dentry *runtime_measurements_count;
 static struct dentry *violations;
 static struct dentry *ima_policy;
+static struct dentry *digest_lists;
+static struct dentry *digests_count;
 
 static enum kernel_read_file_id ima_get_file_id(struct dentry *dentry)
 {
if (dentry == ima_policy)
return READING_POLICY;
+   else if (dentry == digest_lists)
+   return READING_DIGEST_LIST_METADATA;
 
return READING_UNKNOWN;
 }
@@ -66,6 +70,8 @@ static ssize_t ima_show_htable_value(struct file *filp, char 
__user *buf,
val = _htable.violations;
else if (filp->f_path.dentry == runtime_measurements_count)
val = _htable.len;
+   else if (filp->f_path.dentry == digests_count)
+   val = _digests_htable.len;
 
len = scnprintf(tmpbuf, TMPBUFLEN, "%li\n", atomic_long_read(val));
return simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
@@ -301,6 +307,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);
+   } else if (file_id == READING_DIGEST_LIST_METADATA) {
+   rc = ima_parse_digest_list_metadata(size, datap);
+   datap += rc;
}
if (rc < 0)
break;
@@ -401,7 +410,8 @@ static int ima_open_data_upload(struct inode *inode, struct 
file *filp)
read_allowed = true;
seq_ops = _policy_seqops;
 #endif
-   }
+   } else if (file_id == READING_DIGEST_LIST_METADATA && !ima_policy_flag)
+   return -EACCES;
 
if (!(filp->f_flags & O_WRONLY)) {
if (!read_allowed)
@@ -510,8 +520,22 @@ int __init ima_fs_init(void)
if (IS_ERR(ima_policy))
goto out;
 
+#ifdef CONFIG_IMA_DIGEST_LIST
+   digest_lists = securityfs_create_file("digest_lists", S_IWUSR, ima_dir,
+ NULL, _data_upload_ops);
+   if (IS_ERR(digest_lists))
+   goto out;
+
+   digests_count = securityfs_create_file("digests_count",
+  S_IRUSR | S_IRGRP, ima_dir,
+  NULL, _htable_value_ops);
+   if (IS_ERR(digests_count))
+   goto out;
+#endif
return 0;
 out:
+   securityfs_remove(digests_count);
+   securityfs_remove(digest_lists);
securityfs_remove(violations);
securityfs_remove(runtime_measurements_count);
securityfs_remove(ascii_runtime_measurements);
-- 
2.11.0

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

[PATCH v2 10/15] ima: disable digest lookup if digest lists are not checked

[Roberto Sassu]

This patch introduces two new hooks DIGEST_LIST_METADATA_CHECK and
DIGEST_LIST_CHECK, which are called respectively when parsing digest list
metadata and digest lists.

It also checks that rules for these two hooks are always specified in the
current policy. Without them, digest lists could be uploaded to IMA without
adding a new entry to the measurement list, without verifying the
signature, or without auditing the operation. Digest lookup is disabled for
each missing policy action (measure, appraise, audit).

Digest lookup is also disabled if CONFIG_IMA_DIGEST_LIST is not defined.

Changelog

v1:
- clear IMA_MEASURE action flag only if it was in the policy
- check if at least one action is allowed before searching the file digest
- retrieve ima_digest structure
- set IMA action flags in ima_disable_digest_lookup
- added DIGEST_LIST_METADATA_CHECK hook
- update MEASURE/APPRAISE policies

Signed-off-by: Roberto Sassu 
---
 security/integrity/ima/ima.h|  2 ++
 security/integrity/ima/ima_main.c   | 38 +++--
 security/integrity/ima/ima_policy.c | 16 
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 1f43284788eb..4b3b1ca5c09a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -204,6 +204,8 @@ static inline unsigned long ima_hash_key(u8 *digest)
hook(KEXEC_KERNEL_CHECK)\
hook(KEXEC_INITRAMFS_CHECK) \
hook(POLICY_CHECK)  \
+   hook(DIGEST_LIST_METADATA_CHECK)\
+   hook(DIGEST_LIST_CHECK) \
hook(MAX_CHECK)
 #define __ima_hook_enumify(ENUM)   ENUM,
 
diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
index 766fe2e77419..840362734f91 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -29,6 +29,12 @@
 
 int ima_initialized;
 
+#ifdef CONFIG_IMA_DIGEST_LIST
+static int ima_disable_digest_lookup;
+#else
+static int ima_disable_digest_lookup = IMA_DO_MASK & ~IMA_APPRAISE_SUBMASK;
+#endif
+
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise = IMA_APPRAISE_ENFORCE;
 #else
@@ -168,12 +174,20 @@ static int process_measurement(struct file *file, char 
*buf, loff_t size,
char *pathbuf = NULL;
char filename[NAME_MAX];
const char *pathname = NULL;
-   int rc = -ENOMEM, action, must_appraise;
+   int rc = -ENOMEM, action, action_done, must_appraise, digest_lookup;
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;
+   int disable_mask = (func == DIGEST_LIST_CHECK) ?
+  IMA_DO_MASK & ~IMA_APPRAISE_SUBMASK :
+  IMA_DO_MASK & ~(IMA_APPRAISE | IMA_APPRAISE_SUBMASK);
+
+   if ((func == DIGEST_LIST_METADATA_CHECK || func == DIGEST_LIST_CHECK) &&
+   !ima_policy_flag)
+   ima_disable_digest_lookup = disable_mask;
 
if (!ima_policy_flag || !S_ISREG(inode->i_mode))
return 0;
@@ -185,6 +199,9 @@ static int process_measurement(struct file *file, char 
*buf, loff_t size,
action = ima_get_action(inode, mask, func, );
violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
   (ima_policy_flag & IMA_MEASURE));
+   if (func == DIGEST_LIST_METADATA_CHECK || func == DIGEST_LIST_CHECK)
+   ima_disable_digest_lookup |= (~action & disable_mask);
+
if (!action && !violation_check)
return 0;
 
@@ -242,6 +259,21 @@ static int process_measurement(struct file *file, char 
*buf, loff_t size,
if (rc != 0 && rc != -EBADF && rc != -EINVAL)
goto out_digsig;
 
+   digest_lookup = action & ~ima_disable_digest_lookup;
+   if (digest_lookup) {
+   found_digest = ima_lookup_loaded_digest(iint->ima_hash->digest);
+   if (found_digest) {
+   action_done = digest_lookup & (IMA_MEASURE | IMA_AUDIT);
+   action &= ~action_done;
+   iint->flags |= (action_done << 1);
+
+   if (!(digest_lookup & IMA_APPRAISE))
+   found_digest = NULL;
+   if (digest_lookup & IMA_MEASURE)
+   iint->measured_pcrs |= (0x1 << pcr);
+   }
+   }
+
if (!pathbuf)   /* ima_rdwr_violation possibly pre-fetched */
pathname = ima_d_path(>f_path, , filename);
 
@@ -378,7 +410,9 @@ static int read_idmap[READING_MAX_ID] = {
[READING_MODULE] = MODULE_CHECK,
[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
-   [READING_POLICY] = 

[PATCH v2 15/15] ima: add Documentation/security/IMA-digest-lists.txt

[Roberto Sassu]

This patch adds the documentation of digest lists.

Signed-off-by: Roberto Sassu 
---
 Documentation/security/IMA-digest-lists.txt | 161 
 1 file changed, 161 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 ..afa860bbe53e
--- /dev/null
+++ b/Documentation/security/IMA-digest-lists.txt
@@ -0,0 +1,161 @@
+
+Digest Lists
+
+
+
+INTRODUCTION
+
+
+IMA is a security module with the objective of reporting or enforcing the
+integrity of a system, by measuring files accessed with the execve(),
+mmap() and open() system calls. For reporting, it takes advantage of the
+TPM and extends a PCR with the digest of an evaluated event. For enforcing,
+it returns a value which is zero if the operation should be allowed,
+negative if it should be denied.
+
+Measuring files of an operating system introduces three main issues. First,
+since the overhead introduced by the TPM is noticeable, the performance of
+the system decreases linearly with the number of measurements taken. This
+can be seen especially at boot time. Second, managing large measurement
+lists requires computation power and network bandwidth. Third, it is
+necessary to obtain reference measurements (i.e. digests of software known
+to be good) to evaluate/enforce the integrity of the system. If file
+signatures are used to enforce access, Linux distribution vendors have to
+modify their building systems in order to include signatures in their
+packages.
+
+Digest lists aim at mitigating these issues. A digest list is a list of
+digests that are taken by IMA as reference measurements and loaded before
+files are accessed. Then, IMA compares calculated digests of accessed files
+with digests from loaded digest lists. If the digest is found, measurement,
+appraisal and audit are not performed.
+
+Multiple digest lists can be loaded at the same time, by providing to IMA
+metadata for each list: digest, signature and path. The digest is specified
+so that loaded digest lists can be identified only with the measurement of
+metadata. The signature is used for appraisal. If the verification
+succeeds, IMA loads the digest list even if security.ima is missing.
+
+Digest lists address the first issue because the TPM is used only if the
+digest of a measured file is unknown. On a minimal system, 10 of 1400
+measurements are unknown because of mutable files (e.g. log files).
+
+Digest lists mitigate the second issue because, since digest lists do not
+change, they don't have to be sent at every remote attestation. Sending
+unknown measurements and a reference to digest lists would be sufficient.
+
+Finally, digest lists address also the third issue because Linux
+distribution vendors already provide the digests of files included in each
+RPM package. The digest list is stored in the RPM header, signed by the
+vendor.
+
+When using digest lists, a limitation must be considered. Since a
+measurement is not reported if the digest of an accessed file is found in a
+digest list, the measurement list does not show which files have been
+actually accessed, and in which sequence.
+
+A possible solution would be to load a list with digest of files which are
+usually accessed. Also, it is possible to selectively enable digest list
+lookup only for a subset of IMA policy rules. For example, a policy could
+enable digest lookup only for file accesses from the TCB and disable it
+for execve() and mmap() from regular users.
+
+
+
+SETUP
+=
+
+Digest lists should be placed in the /etc/ima/digest_lists directory and
+metadata should be written to /etc/ima/digest_lists/metadata.
+
+If digest lists are included in the initial ram disk, IMA will load them
+early in the boot process. Otherwise, a patched systemd can check if the
+file with digest list metadata exists in the filesystem and, if yes, send
+the path to IMA through the 'digest_lists' securityfs interface. The main
+use case for the patched systemd is to load digest lists of newly installed
+packages, which are not included in the initial ram disk.
+
+
+
+FORMATS
+===
+
+The format of digest list metadata is:
+
+algo[2]
+digest_len[4] digest[digest_len]
+signature_len[4] signature[signature_len]
+path_len[4] path[path_len]
+ref_id_len[4] ref_id[ref_id_len]
+list_type[2]
+
+algo and list_type are in little endian.
+
+algo values are defined in include/uapi/linux/hash_info.h. The algorithms
+in the list metadata must be the same of ima_hash_algo (algorithm used by
+IMA to calculate the file digest).
+
+list type values:
+
+0: compact digest list
+1: RPM package header
+
+
+The format of the compact digest list is:
+
+entry_id[2] count[4] data_len[4]
+data[data_len]
+[...]
+entry_id[2] count[4] data_len[4]
+data[data_len]
+
+entry_id, count and data_len are in little 

[PATCH v2 14/15] ima: add support for appraisal with digest lists

[Roberto Sassu]

Appraisal verification consists on comparing the calculated digest of an
accessed file with the value of the security.ima extended attribute. With
digest lists, appraisal verification succeeds if the calculated digest is
included in a list. Since the digital signature of each digest list is
verified, it is not possible to allow access of unauthorized files.

For mutable files, IMA writes the current digest to security.ima so that
next file accesses are allowed even if the files have been modified. For
immutable files, IMA writes security.ima only if also additional extended
attributes should be protected by EVM. Otherwise, security.ima would be
redundant, as digest lists provide reference values.

When IMA writes security.ima, EVM calculates the HMAC based on the current
value of protected extended attributes. Without file signatures, initial
extended attribute values will not checked until digest lists include them.
When extended attribute values are available, IMA will check them as the
same as the digest, and will not write security.ima for immutable files if
values are provided for all extended attributes protected by EVM.

Signed-off-by: Roberto Sassu 
---
 security/integrity/ima/ima.h  |  6 +++--
 security/integrity/ima/ima_appraise.c | 47 +++
 security/integrity/ima/ima_main.c |  6 +++--
 security/integrity/integrity.h|  2 ++
 4 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ddd0e1e7e99b..5f8e0740a33e 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -261,7 +261,8 @@ 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 opened);
+int xattr_len, int opened,
+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,
@@ -277,7 +278,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 opened)
+  int xattr_len, int opened,
+  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 1b2236e637ff..fd03a0278fba 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -201,17 +201,27 @@ 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 opened)
+int xattr_len, int opened,
+struct ima_digest *found_digest)
 {
static const char op[] = "appraise_data";
char *cause = "unknown";
struct dentry *dentry = file_dentry(file);
struct inode *inode = d_backing_inode(dentry);
enum integrity_status status = INTEGRITY_UNKNOWN;
-   int rc = xattr_len, hash_start = 0;
+   struct evm_ima_xattr_data digest_list_value;
+   char *list_metadata = XATTR_NAME_IMA;
+   int rc = xattr_len, hash_start = 0, cache_flags_disabled = 0;
 
if (!(inode->i_opflags & IOP_XATTR))
-   return INTEGRITY_UNKNOWN;
+   return found_digest ? INTEGRITY_PASS : INTEGRITY_UNKNOWN;
+
+   if (found_digest && (!rc || rc == -ENODATA)) {
+   digest_list_value.type = found_digest->is_mutable ?
+   IMA_DIGEST_LIST_MUTABLE : IMA_DIGEST_LIST_IMMUTABLE;
+   xattr_value = _list_value;
+   rc = sizeof(*xattr_value);
+   }
 
if (rc <= 0) {
if (rc && rc != -ENODATA)
@@ -228,6 +238,9 @@ int ima_appraise_measurement(enum ima_hooks func,
goto out;
}
 
+   if (xattr_value == _list_value)
+   goto no_evm_check;
+
status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
if ((status == INTEGRITY_NOLABEL)
@@ -237,13 +250,17 

[PATCH v2 08/15] ima: add parser of RPM package headers

[Roberto Sassu]

This patch introduces a parser of RPM package headers. It extracts the
digests from the RPMTAG_FILEDIGESTS header section and converts them to
binary data before adding them to the hash table.

The advantage of this data type is that verifiers can determine who
produced that data, as headers are signed by Linux distribution vendors.
RPM header signatures can be provided as digest list metadata.

The parser also checks the RPMTAG_FILEMODES section. If the file is not
executable, the setuid/setgid/sticky bits are not set and has write
permission, the digest is marked as mutable (file updates are permitted if
appraisal is in enforcing mode).

Changelog

v1:
- Moved parser of file digests outside the first loop
- Added support for immutable/mutable files

Signed-off-by: Roberto Sassu 
---
 security/integrity/ima/ima_digest_list.c | 110 ++-
 1 file changed, 109 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_digest_list.c 
b/security/integrity/ima/ima_digest_list.c
index 6ad00ba32c94..664a4994efbb 100644
--- a/security/integrity/ima/ima_digest_list.c
+++ b/security/integrity/ima/ima_digest_list.c
@@ -19,11 +19,14 @@
 #include "ima.h"
 #include "ima_template_lib.h"
 
+#define RPMTAG_FILEDIGESTS 1035
+#define RPMTAG_FILEMODES 1030
+
 enum digest_metadata_fields {DATA_ALGO, DATA_DIGEST, DATA_SIGNATURE,
 DATA_FILE_PATH, DATA_REF_ID, DATA_TYPE,
 DATA__LAST};
 
-enum digest_data_types {DATA_TYPE_COMPACT_LIST};
+enum digest_data_types {DATA_TYPE_COMPACT_LIST, DATA_TYPE_RPM};
 
 enum compact_list_entry_ids {COMPACT_DIGEST, COMPACT_DIGEST_MUTABLE};
 
@@ -33,6 +36,20 @@ struct compact_list_hdr {
u32 datalen;
 } __packed;
 
+struct rpm_hdr {
+   u32 magic;
+   u32 reserved;
+   u32 tags;
+   u32 datasize;
+} __packed;
+
+struct rpm_entryinfo {
+   int32_t tag;
+   u32 type;
+   int32_t offset;
+   u32 count;
+} __packed;
+
 static int ima_parse_compact_list(loff_t size, void *buf)
 {
void *bufp = buf, *bufendp = buf + size;
@@ -86,6 +103,94 @@ static int ima_parse_compact_list(loff_t size, void *buf)
return 0;
 }
 
+static int ima_parse_rpm(loff_t size, void *buf)
+{
+   void *bufp = buf, *bufendp = buf + size;
+   struct rpm_hdr *hdr = bufp;
+   u32 tags = be32_to_cpu(hdr->tags);
+   struct rpm_entryinfo *entry;
+   void *datap = bufp + sizeof(*hdr) + tags * sizeof(struct rpm_entryinfo);
+   void *digests = NULL, *modes = NULL;
+   u32 digests_count, modes_count;
+   int digest_len = hash_digest_size[ima_hash_algo];
+   u8 digest[digest_len];
+   int ret, i;
+
+   const unsigned char rpm_header_magic[8] = {
+   0x8e, 0xad, 0xe8, 0x01, 0x00, 0x00, 0x00, 0x00
+   };
+
+   if (size < sizeof(*hdr)) {
+   pr_err("Missing RPM header\n");
+   return -EINVAL;
+   }
+
+   if (memcmp(bufp, rpm_header_magic, sizeof(rpm_header_magic))) {
+   pr_err("Invalid RPM header\n");
+   return -EINVAL;
+   }
+
+   bufp += sizeof(*hdr);
+
+   for (i = 0; i < tags && (bufp + sizeof(*entry)) <= bufendp;
+i++, bufp += sizeof(*entry)) {
+   entry = bufp;
+
+   if (be32_to_cpu(entry->tag) == RPMTAG_FILEDIGESTS) {
+   digests = datap + be32_to_cpu(entry->offset);
+   digests_count = be32_to_cpu(entry->count);
+   }
+   if (be32_to_cpu(entry->tag) == RPMTAG_FILEMODES) {
+   modes = datap + be32_to_cpu(entry->offset);
+   modes_count = be32_to_cpu(entry->count);
+   }
+   if (digests && modes)
+   break;
+   }
+
+   if (digests == NULL)
+   return 0;
+
+   for (i = 0; i < digests_count && digests < bufendp; i++) {
+   u8 is_mutable = 0;
+   u16 mode;
+
+   if (strlen(digests) == 0) {
+   digests++;
+   continue;
+   }
+
+   if (modes) {
+   if (modes + (i + 1) * sizeof(mode) > bufendp) {
+   pr_err("RPM header read at invalid offset\n");
+   return -EINVAL;
+   }
+
+   mode = be16_to_cpu(*(u16 *)(modes + i * sizeof(mode)));
+   if (!(mode & (S_IXUGO | S_ISUID | S_ISGID | S_ISVTX)) &&
+   (mode & S_IWUGO))
+   is_mutable = 1;
+   }
+
+   if (digests + digest_len * 2 + 1 > bufendp) {
+   pr_err("RPM header read at invalid offset\n");
+   return -EINVAL;
+   }
+
+   ret = hex2bin(digest, digests, digest_len);
+   if (ret < 0)
+   

[PATCH v2 05/15] ima: add functions to manage digest lists

[Roberto Sassu]

This patch first introduces a new structure called ima_digest, which
contains a digest parsed from a digest list. It has been preferred to
ima_queue_entry, as the existing structure includes an additional member
(a list head), which is not necessary for digest lookup. It also introduces
the is_mutable field, which indicates if a file with a given digest can be
updated or not.

Finally, this patch introduces functions to lookup and add a digest to the
new ima_digests_htable hash table.

Changelog

v1:
- added support for immutable/mutable files

Signed-off-by: Roberto Sassu 
---
 security/integrity/ima/ima.h   |  9 
 security/integrity/ima/ima_queue.c | 42 ++
 2 files changed, 51 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487ad259..1f6591a57fea 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -107,6 +107,12 @@ struct ima_queue_entry {
 };
 extern struct list_head ima_measurements;  /* list of all measurements */
 
+struct ima_digest {
+   struct hlist_node hnext;
+   u8 is_mutable;
+   u8 digest[0];
+};
+
 /* Some details preceding the binary serialized measurement list */
 struct ima_kexec_hdr {
u16 version;
@@ -150,6 +156,8 @@ void ima_print_digest(struct seq_file *m, u8 *digest, u32 
size);
 struct ima_template_desc *ima_template_desc_current(void);
 int ima_restore_measurement_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
+struct ima_digest *ima_lookup_loaded_digest(u8 *digest);
+int ima_add_digest_data_entry(u8 *digest, u8 is_mutable);
 int ima_measurements_show(struct seq_file *m, void *v);
 unsigned long ima_get_binary_runtime_size(void);
 int ima_init_template(void);
@@ -166,6 +174,7 @@ struct ima_h_table {
struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
 };
 extern struct ima_h_table ima_htable;
+extern struct ima_h_table ima_digests_htable;
 
 static inline unsigned long ima_hash_key(u8 *digest)
 {
diff --git a/security/integrity/ima/ima_queue.c 
b/security/integrity/ima/ima_queue.c
index a02a86d51102..96c91c413430 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -42,6 +42,11 @@ struct ima_h_table ima_htable = {
.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
 };
 
+struct ima_h_table ima_digests_htable = {
+   .len = ATOMIC_LONG_INIT(0),
+   .queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
+};
+
 /* mutex protects atomicity of extending measurement list
  * and extending the TPM PCR aggregate. Since tpm_extend can take
  * long (and the tpm driver uses a mutex), we can't use the spinlock.
@@ -212,3 +217,40 @@ int ima_restore_measurement_entry(struct 
ima_template_entry *entry)
mutex_unlock(_extend_list_mutex);
return result;
 }
+
+struct ima_digest *ima_lookup_loaded_digest(u8 *digest)
+{
+   struct ima_digest *d = NULL;
+   int digest_len = hash_digest_size[ima_hash_algo];
+   unsigned int key = ima_hash_key(digest);
+
+   rcu_read_lock();
+   hlist_for_each_entry_rcu(d, _digests_htable.queue[key], hnext) {
+   if (memcmp(d->digest, digest, digest_len) == 0)
+   break;
+   }
+   rcu_read_unlock();
+   return d;
+}
+
+int ima_add_digest_data_entry(u8 *digest, u8 is_mutable)
+{
+   struct ima_digest *d = ima_lookup_loaded_digest(digest);
+   int digest_len = hash_digest_size[ima_hash_algo];
+   unsigned int key = ima_hash_key(digest);
+
+   if (d) {
+   d->is_mutable = is_mutable;
+   return -EEXIST;
+   }
+
+   d = kmalloc(sizeof(*d) + digest_len, GFP_KERNEL);
+   if (d == NULL)
+   return -ENOMEM;
+
+   d->is_mutable = is_mutable;
+   memcpy(d->digest, digest, digest_len);
+   hlist_add_head_rcu(>hnext, _digests_htable.queue[key]);
+   atomic_long_inc(_digests_htable.len);
+   return 0;
+}
-- 
2.11.0

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

[PATCH v2 04/15] ima: use ima_show_htable_value to show hash table data

[Roberto Sassu]

This patch removes ima_show_htable_violations() and
ima_show_measurements_count(). ima_show_htable_value(), called by those
functions, determines which hash table data should be copied to the buffer
depending on the dentry of the file passed as argument.

Signed-off-by: Roberto Sassu 
---
 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 a5b82e075ec8..4158ced5d3c9 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -55,38 +55,24 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
 
 static int valid_policy = 1;
 #define TMPBUFLEN 12
-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[TMPBUFLEN];
ssize_t len;
 
+   if (filp->f_path.dentry == violations)
+   val = _htable.violations;
+   else if (filp->f_path.dentry == runtime_measurements_count)
+   val = _htable.len;
+
len = scnprintf(tmpbuf, TMPBUFLEN, "%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, _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, _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,
 };
 
@@ -508,13 +494,13 @@ int __init ima_fs_init(void)
runtime_measurements_count =
securityfs_create_file("runtime_measurements_count",
   S_IRUSR | S_IRGRP, ima_dir, NULL,
-  _measurements_count_ops);
+  _htable_value_ops);
if (IS_ERR(runtime_measurements_count))
goto out;
 
violations =
securityfs_create_file("violations", S_IRUSR | S_IRGRP,
-  ima_dir, NULL, _htable_violations_ops);
+  ima_dir, NULL, _htable_value_ops);
if (IS_ERR(violations))
goto out;
 
-- 
2.11.0

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

[PATCH v2 01/15] ima: generalize ima_read_policy()

[Roberto Sassu]

Rename ima_read_policy() to ima_read_file(), and add file_id as new
parameter. If file_id is equal to READING_POLICY, ima_read_file() behavior
remains unchanged. ima_read_file() will be used to read digest list
metadata.

Signed-off-by: Roberto Sassu 
---
 security/integrity/ima/ima_fs.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index fa540c0469da..27de4558303e 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,22 @@ static ssize_t ima_read_policy(char *path)
datap = path;
strsep(, "\n");
 
-   rc = kernel_read_file_from_path(path, , , 0, READING_POLICY);
+   rc = kernel_read_file_from_path(path, , , 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(, "\n"))) {
-   pr_debug("rule: %s\n", p);
-   rc = ima_parse_add_rule(p);
+   while (size > 0) {
+   if (file_id == READING_POLICY) {
+   p = strsep(, "\n");
+   if (p == NULL)
+   break;
+
+   pr_debug("rule: %s\n", p);
+   rc = ima_parse_add_rule(p);
+   }
if (rc < 0)
break;
size -= rc;
@@ -334,7 +340,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("IMA: signed policy file (specified as an absolute 
pathname) required\n");
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
-- 
2.11.0

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

[PATCH v2 00/15] ima: digest list feature

[Roberto Sassu]

IMA is a security module with the objective of reporting or enforcing the
integrity of a system, by measuring files accessed with the execve(),
mmap() and open() system calls. For reporting, it takes advantage of the
TPM and extends a PCR with the digest of an evaluated event. For enforcing,
it returns a value which is zero if the operation should be allowed,
negative if it should be denied.

Measuring files of an operating system introduces three main issues. First,
since the overhead introduced by the TPM is noticeable, the performance of
the system decreases linearly with the number of measurements taken. This
can be seen especially at boot time. Second, managing large measurement
lists requires computation power and network bandwidth. Third, it is
necessary to obtain reference measurements (i.e. digests of software known
to be good) to evaluate/enforce the integrity of the system. If file
signatures are used to enforce access, Linux distribution vendors have to
modify their building systems in order to include signatures in their
packages.

Digest lists aim at mitigating these issues. A digest list is a list of
digests that are taken by IMA as reference measurements and loaded before
files are accessed. Then, IMA compares calculated digests of accessed files
with digests from loaded digest lists. If the digest is found, measurement,
appraisal and audit are not performed.

Multiple digest lists can be loaded at the same time, by providing to IMA
metadata for each list: digest, signature and path. The digest is specified
so that loaded digest lists can be identified only with the measurement of
metadata. The signature is used for appraisal. If the verification
succeeds, IMA loads the digest list even if security.ima is missing.

Digest lists address the first issue because the TPM is used only if the
digest of a measured file is unknown. On a minimal system, 10 of 1400
measurements are unknown because of mutable files (e.g. log files).

Digest lists mitigate the second issue because, since digest lists do not
change, they don't have to be sent at every remote attestation. Sending
unknown measurements and a reference to digest lists would be sufficient.

Finally, digest lists address also the third issue because Linux
distribution vendors already provide the digests of files included in each
RPM package. The digest list is stored in the RPM header, signed by the
vendor.

When using digest lists, a limitation must be considered. Since a
measurement is not reported if the digest of an accessed file is found in a
digest list, the measurement list does not show which files have been
actually accessed, and in which sequence.

A possible solution would be to load a list with digest of files which are
usually accessed. Also, it is possible to selectively enable digest list
lookup only for a subset of IMA policy rules. For example, a policy could
enable digest lookup only for file accesses from the TCB and disable it
for execve() and mmap() from regular users.

Changelog

v1:
- added new policy option digest_list to selectively enable digest lookup
- added support for appraisal
- added support for immutable/mutable files

Roberto Sassu (15):
  ima: generalize ima_read_policy()
  ima: generalize ima_write_policy()
  ima: generalize policy file operations
  ima: use ima_show_htable_value to show hash table data
  ima: add functions to manage digest lists
  ima: add parser of digest lists metadata
  ima: add parser of compact digest list
  ima: add parser of RPM package headers
  ima: introduce securityfs interfaces for digest lists
  ima: disable digest lookup if digest lists are not checked
  ima: add policy action digest_list
  ima: do not update security.ima if appraisal status is not
INTEGRITY_PASS
  evm: add kernel command line option to select protected xattrs
  ima: add support for appraisal with digest lists
  ima: add Documentation/security/IMA-digest-lists.txt

 Documentation/admin-guide/kernel-parameters.txt |   4 +
 Documentation/security/IMA-digest-lists.txt | 161 
 include/linux/evm.h |   6 +
 include/linux/fs.h  |   2 +
 security/integrity/evm/evm_main.c   |  36 +++
 security/integrity/iint.c   |   1 +
 security/integrity/ima/Kconfig  |  19 ++
 security/integrity/ima/Makefile |   1 +
 security/integrity/ima/ima.h|  33 ++-
 security/integrity/ima/ima_api.c|   7 +-
 security/integrity/ima/ima_appraise.c   |  52 +++-
 security/integrity/ima/ima_digest_list.c| 326 
 security/integrity/ima/ima_fs.c | 181 -
 security/integrity/ima/ima_main.c   |  47 +++-
 security/integrity/ima/ima_policy.c |  33 ++-
 security/integrity/ima/ima_queue.c  |  42 +++
 security/integrity/integrity.h  |  11 +
 17 files changed, 877 

Re: [PATCH] documentation: update list of available compiled-in fonts

[Geert Uytterhoeven]

Hi Randy,

On Tue, Nov 7, 2017 at 5:48 AM, Randy Dunlap  wrote:
> From: Randy Dunlap 
>
> Update list of available compiled-in fonts in lib/fonts/:
> add 6x10 and drop RomanLarge (which was reverted 12 years ago).
>
> Signed-off-by: Randy Dunlap 
> Cc: Geert Uytterhoeven 

Acked-by: Geert Uytterhoeven 

> --- lnx-414-rc8.orig/Documentation/fb/fbcon.txt
> +++ lnx-414-rc8/Documentation/fb/fbcon.txt
> @@ -77,8 +77,8 @@ C. Boot options
>  1. fbcon=font:
>
>  Select the initial font to use. The value 'name' can be any of the
> -compiled-in fonts: VGA8x16, 7x14, 10x18, VGA8x8, MINI4x6, RomanLarge,
> -SUN8x16, SUN12x22, ProFont6x11, Acorn8x8, PEARL8x8.
> +compiled-in fonts: VGA8x16, 7x14, 10x18, VGA8x8, MINI4x6,
> +SUN8x16, SUN12x22, 6x10, ProFont6x11, Acorn8x8, PEARL8x8.

While at it, perhaps you want to sort the list alphabetically?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html