linux-ima-user

[Linux-ima-user] copying files with security.ima xattr

[Patrick O. <pat...@in...>]

Hello!

I just noticed some unexpected behavior. Kernel is 4.4.11, with IMA
enabled in the build configuration.

After booting, copying files which have a hash in security.ima (like
0x0196d920f04ec85c33438117740d5dcae7bbdbfe3a) with rsync or bsdtar fails
in fsetxattr with permission denied. From bsdtar:

open("build", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE|O_CLOEXEC, 0644) = 3
fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
write(3, "-----------------------\nBuild Co"..., 2022) = 2022
...
fchown32(3, 0, 0)                       = 0
utimensat(3, NULL, [{1467894919, 0}, {1467894885, 0}], 0) = 0
fsetxattr(3, "security.SMACK64", "System::Shared", 14, 0) = 0
fsetxattr(3, "security.ima", "\1\226\331 \360N\310\\3C\201\27t\r]\312\347\273\333\376:", 21, 0) = -1 EPERM (Operation not permitted)
close(3)                                = 0

It does not matter whether an IMA policy has been loaded or not.

fsetxattr() succeeds if the security.ima value is a signed hash.

We ran into this while working on an installer which runs from the
initramfs and copies the content of the rootfs from an USB stick to some
other media, like internal flash.

Ideally, that installer should get started before loading an IMA policy.
That way, writing files will be faster (no hashing). But that doesn't
work because the security.ima of hashed files cannot be set.

It works after loading an IMA policy which hashes the files on the
target partition: rsync or bsdtar try write security.ima, print an
error, continue, and the kernel sets the same hash that the tools
weren't allowed to set earlier.

Besides the slower copy operation, we also get errors that we need to
ignore because (most likely) they are harmless. Not nice, because at
some point there might be genuine errors which also get ignored.

Would it make sense to always allow setting security.ima when the caller
has the right priviliges? If someone sets a broken value, that's their
problem.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [Linux-ima-user] copying files with security.ima xattr

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

On Thu, 2016-07-07 at 15:02 +0200, Patrick Ohly wrote:
> Hello!
> 
> I just noticed some unexpected behavior. Kernel is 4.4.11, with IMA
> enabled in the build configuration.
> 
> After booting, copying files which have a hash in security.ima (like
> 0x0196d920f04ec85c33438117740d5dcae7bbdbfe3a) with rsync or bsdtar fails
> in fsetxattr with permission denied. From bsdtar:
> 
> open("build", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE|O_CLOEXEC, 0644) = 3
> fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
> write(3, "-----------------------\nBuild Co"..., 2022) = 2022
> ...
> fchown32(3, 0, 0)                       = 0
> utimensat(3, NULL, [{1467894919, 0}, {1467894885, 0}], 0) = 0
> fsetxattr(3, "security.SMACK64", "System::Shared", 14, 0) = 0
> fsetxattr(3, "security.ima", "\1\226\331 \360N\310\\3C\201\27t\r]\312\347\273\333\376:", 21, 0) = -1 EPERM (Operation not permitted)
> close(3)                                = 0
> 
> It does not matter whether an IMA policy has been loaded or not.
> 
> fsetxattr() succeeds if the security.ima value is a signed hash.

Right, there's no good reason for explicitly writing the hash.  If the
file is in policy, on __fput() the file will automatically be hashed.
Only writing a file signature as security.ima xattr is permitted.
Refer to commit c68ed80 "ima: limit file hash setting by user to fix and
log modes".

> We ran into this while working on an installer which runs from the
> initramfs and copies the content of the rootfs from an USB stick to some
> other media, like internal flash.
> 
> Ideally, that installer should get started before loading an IMA policy.
> That way, writing files will be faster (no hashing). But that doesn't
> work because the security.ima of hashed files cannot be set.

(I assume you meant signatures can not be written on unhashed files.)   

Writing the file signature on new files, before __fput() is called,
would prevent the file hash from being calculated.  For example, GNU tar
writes the file signature before the file data.   (Refer to commit
05d1a71 "ima: add support for creating files using the mknodat syscall")

> It works after loading an IMA policy which hashes the files on the
> target partition: rsync or bsdtar try write security.ima, print an
> error, continue, and the kernel sets the same hash that the tools
> weren't allowed to set earlier.

Exactly, no need for the userspace application to write the hash.

> Besides the slower copy operation, we also get errors that we need to
> ignore because (most likely) they are harmless. Not nice, because at
> some point there might be genuine errors which also get ignored.

Other than writing file hashes or copying security.evm warnings are
there others?

> Would it make sense to always allow setting security.ima when the caller
> has the right priviliges? If someone sets a broken value, that's their
> problem.

The signature itself is currently not verified, before writing it as an
xattr, but the existing security.evm is checked, before allowing any
protected xattrs to be written.  Otherwise any change would  incorporate
any other intended/unintended changes in the security.evm xattr.

For those files included in the IMA policy, either a file hash or
signature needs to be written before the "new" file status is removed
(eg. __fput).   The only exceptions are for "fix" or "log" modes. 

Mimi

Re: [Linux-ima-user] copying files with security.ima xattr

[Patrick O. <pat...@in...>]

On Tue, 2016-07-12 at 09:00 -0400, Mimi Zohar wrote:
> On Thu, 2016-07-07 at 15:02 +0200, Patrick Ohly wrote:
> > Hello!
> > 
> > I just noticed some unexpected behavior. Kernel is 4.4.11, with IMA
> > enabled in the build configuration.
> > 
> > After booting, copying files which have a hash in security.ima (like
> > 0x0196d920f04ec85c33438117740d5dcae7bbdbfe3a) with rsync or bsdtar fails
> > in fsetxattr with permission denied. From bsdtar:
> > 
> > open("build", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE|O_CLOEXEC, 0644) = 3
> > fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
> > write(3, "-----------------------\nBuild Co"..., 2022) = 2022
> > ...
> > fchown32(3, 0, 0)                       = 0
> > utimensat(3, NULL, [{1467894919, 0}, {1467894885, 0}], 0) = 0
> > fsetxattr(3, "security.SMACK64", "System::Shared", 14, 0) = 0
> > fsetxattr(3, "security.ima", "\1\226\331 \360N\310\\3C\201\27t\r]\312\347\273\333\376:", 21, 0) = -1 EPERM (Operation not permitted)
> > close(3)                                = 0
> > 
> > It does not matter whether an IMA policy has been loaded or not.
> > 
> > fsetxattr() succeeds if the security.ima value is a signed hash.
> 
> Right, there's no good reason for explicitly writing the hash.

Yes, if the file is under the current policy, then it is redundant. But
that implies that the user space tool knows exactly whether that
operation is redundant. That's far from trivial, isn't it?

And that assumes that the tool in question knows about IMA. bsdtar
doesn't support IMA, only xattrs. Therefore it will always write
security.ima, regardless what the value is in the archive that it
extracts and what the current policy on the target filesystem might be.

One could argue that the input archive shouldn't have the security.ima
set when it is not needed, but that (in our case) breaks another use
case, which is a feature of our update tool which compares files on disk
(including their meta data, which includes xattrs) against files stored
on the server. So the server side also needs to have the security.ima.

>   If the
> file is in policy, on __fput() the file will automatically be hashed.
> Only writing a file signature as security.ima xattr is permitted.
> Refer to commit c68ed80 "ima: limit file hash setting by user to fix and
> log modes".

That explains the mechanism, but the commit comes with no further
explanations. Why the "should not be manually set", i.e. what's the
downside of allowing it?

There's also the problem that the patch prevents writing the hash in
situations where there is not active IMA policy. Your argument was that
letting the user write the hash is redundant because the kernel will do
it. But that's not true when the file isn't under the current policy (in
our case, because no policy has been loaded at all), and still the
kernel prevents writing the hash.

> > We ran into this while working on an installer which runs from the
> > initramfs and copies the content of the rootfs from an USB stick to some
> > other media, like internal flash.
> > 
> > Ideally, that installer should get started before loading an IMA policy.
> > That way, writing files will be faster (no hashing). But that doesn't
> > work because the security.ima of hashed files cannot be set.
> 
> (I assume you meant signatures can not be written on unhashed files.)   

No, I meant writing a hash, like 0x0196..e3a in my initial example.

> > It works after loading an IMA policy which hashes the files on the
> > target partition: rsync or bsdtar try write security.ima, print an
> > error, continue, and the kernel sets the same hash that the tools
> > weren't allowed to set earlier.
> 
> Exactly, no need for the userspace application to write the hash.

There is a need: OS gets installed, all hashes are set as required, then
OS boots immediately, without going into "fix" mode. Without writing the
hashes, files would be unusable.

We could boot into fix mode before installing, but that implies booting
with different command line parameters, which isn't possible in our
current setup: we have no boot loader, the firmware directly loads an
EFI blob which contains kernel, command line parameters and initramfs.

Our current image then either installs to internal storage or boots from
USB, depending on some files in the VFAT partition or user interaction.
But that check happens after the kernel started and thus switching modes
between fix and enforcement isn't possible anymore.

> > Besides the slower copy operation, we also get errors that we need to
> > ignore because (most likely) they are harmless. Not nice, because at
> > some point there might be genuine errors which also get ignored.
> 
> Other than writing file hashes or copying security.evm warnings are
> there others?

We are not using security.evm yet (yes, I know, it's a gap that we need
to close). So right now, all errors are about writing security.ima.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [Linux-ima-user] copying files with security.ima xattr

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

On Mo, 2016-07-18 at 09:22 +0200, Patrick Ohly wrote:
> On Tue, 2016-07-12 at 09:00 -0400, Mimi Zohar wrote:
> > On Thu, 2016-07-07 at 15:02 +0200, Patrick Ohly wrote:
> > > Hello!
> > > 
> > > I just noticed some unexpected behavior. Kernel is 4.4.11, with IMA
> > > enabled in the build configuration.
> > > 
> > > After booting, copying files which have a hash in security.ima (like
> > > 0x0196d920f04ec85c33438117740d5dcae7bbdbfe3a) with rsync or bsdtar fails
> > > in fsetxattr with permission denied. From bsdtar:
> > > 
> > > open("build", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE|O_CLOEXEC, 0644) = 3
> > > fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
> > > write(3, "-----------------------\nBuild Co"..., 2022) = 2022
> > > ...
> > > fchown32(3, 0, 0)                       = 0
> > > utimensat(3, NULL, [{1467894919, 0}, {1467894885, 0}], 0) = 0
> > > fsetxattr(3, "security.SMACK64", "System::Shared", 14, 0) = 0
> > > fsetxattr(3, "security.ima", "\1\226\331 \360N\310\\3C\201\27t\r]\312\347\273\333\376:", 21, 0) = -1 EPERM (Operation not permitted)
> > > close(3)                                = 0
> > > 
> > > It does not matter whether an IMA policy has been loaded or not.
> > > 
> > > fsetxattr() succeeds if the security.ima value is a signed hash.
> > 
> > Right, there's no good reason for explicitly writing the hash.
> 
> Yes, if the file is under the current policy, then it is redundant. But
> that implies that the user space tool knows exactly whether that
> operation is redundant. That's far from trivial, isn't it?
> 
> And that assumes that the tool in question knows about IMA. bsdtar
> doesn't support IMA, only xattrs. Therefore it will always write
> security.ima, regardless what the value is in the archive that it
> extracts and what the current policy on the target filesystem might be.
> 
> One could argue that the input archive shouldn't have the security.ima
> set when it is not needed, but that (in our case) breaks another use
> case, which is a feature of our update tool which compares files on disk
> (including their meta data, which includes xattrs) against files stored
> on the server. So the server side also needs to have the security.ima.
> 
> >   If the
> > file is in policy, on __fput() the file will automatically be hashed.
> > Only writing a file signature as security.ima xattr is permitted.
> > Refer to commit c68ed80 "ima: limit file hash setting by user to fix and
> > log modes".
> 
> That explains the mechanism, but the commit comes with no further
> explanations. Why the "should not be manually set", i.e. what's the
> downside of allowing it?
> 
> There's also the problem that the patch prevents writing the hash in
> situations where there is not active IMA policy. Your argument was that
> letting the user write the hash is redundant because the kernel will do
> it. But that's not true when the file isn't under the current policy (in
> our case, because no policy has been loaded at all), and still the
> kernel prevents writing the hash.
> 
> > > We ran into this while working on an installer which runs from the
> > > initramfs and copies the content of the rootfs from an USB stick to some
> > > other media, like internal flash.
> > > 
> > > Ideally, that installer should get started before loading an IMA policy.
> > > That way, writing files will be faster (no hashing). But that doesn't
> > > work because the security.ima of hashed files cannot be set.
> > 
> > (I assume you meant signatures can not be written on unhashed files.)   
> 
> No, I meant writing a hash, like 0x0196..e3a in my initial example.
> 
> > > It works after loading an IMA policy which hashes the files on the
> > > target partition: rsync or bsdtar try write security.ima, print an
> > > error, continue, and the kernel sets the same hash that the tools
> > > weren't allowed to set earlier.
> > 
> > Exactly, no need for the userspace application to write the hash.
> 
> There is a need: OS gets installed, all hashes are set as required, then
> OS boots immediately, without going into "fix" mode. Without writing the
> hashes, files would be unusable.
> 
> We could boot into fix mode before installing, but that implies booting
> with different command line parameters, which isn't possible in our
> current setup: we have no boot loader, the firmware directly loads an
> EFI blob which contains kernel, command line parameters and initramfs.
> 
> Our current image then either installs to internal storage or boots from
> USB, depending on some files in the VFAT partition or user interaction.
> But that check happens after the kernel started and thus switching modes
> between fix and enforcement isn't possible anymore.

> > > Besides the slower copy operation, we also get errors that we need to
> > > ignore because (most likely) they are harmless. Not nice, because at
> > > some point there might be genuine errors which also get ignored.
> > 
> > Other than writing file hashes or copying security.evm warnings are
> > there others?
> 
> We are not using security.evm yet (yes, I know, it's a gap that we need
> to close). So right now, all errors are about writing security.ima.

Commit c68ed80 "ima: limit file hash setting by user to fix and log
modes" is a form of system hardening.  Before reverting it, let's see if
there is another option.

Could we summarize the problem as:
- the kernel prevents writing security.ima hashes.
- the kernel only writes security.ima hashes for files that are in
policy.
- the userspace tool doesn't know what is in/out of policy.
- the userspace tool doesn't differentiate between hashes and
signatures.
- the boot process doesn't permit changing the boot command line options
(eg. fix mode).
- the update tool compares file data and metadata with those on the
server

Currently, you said there is no IMA policy.  Would it make sense to
invert that to all files are in policy?  That way the kernel would write
the file hash as security.ima on all files (that are not signed).

The file metadata would match the update server.  Only the userspace
tool (bsdtar) would need to be modified to selectively write xattrs  -
either no security.ima xattrs or only security.ima file signatures.

Mimi

Re: [Linux-ima-user] copying files with security.ima xattr

[Patrick O. <pat...@in...>]

On Mon, 2016-07-18 at 08:08 -0400, Mimi Zohar wrote:
> Commit c68ed80 "ima: limit file hash setting by user to fix and log
> modes" is a form of system hardening.  Before reverting it, let's see if
> there is another option.
> 
> Could we summarize the problem as:
> - the kernel prevents writing security.ima hashes.
> - the kernel only writes security.ima hashes for files that are in
> policy.
> - the userspace tool doesn't know what is in/out of policy.
> - the userspace tool doesn't differentiate between hashes and
> signatures.
> - the boot process doesn't permit changing the boot command line options
> (eg. fix mode).
> - the update tool compares file data and metadata with those on the
> server

Correct.

> Currently, you said there is no IMA policy.  Would it make sense to
> invert that to all files are in policy?  That way the kernel would write
> the file hash as security.ima on all files (that are not signed).
>
> The file metadata would match the update server.  Only the userspace
> tool (bsdtar) would need to be modified to selectively write xattrs  -
> either no security.ima xattrs or only security.ima file signatures.

This would work, but I don't like the prospect of having to patch bsdtar
like that. Including or excluding xattrs by name is conceptually okay
(GNU tar has that), but filtering by value of an xattr adds
domain-specific knowledge to bsdtar which just doesn't belong there.

Perhaps some custom tool build on top of libarchive would be okay -
basically a custom "bsdtar -xf -".

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [Linux-ima-user] copying files with security.ima xattr

[Patrick O. <pat...@in...>]

On Mon, 2016-07-18 at 14:21 +0200, Patrick Ohly wrote:
> On Mon, 2016-07-18 at 08:08 -0400, Mimi Zohar wrote:
> > Commit c68ed80 "ima: limit file hash setting by user to fix and log
> > modes" is a form of system hardening.  Before reverting it, let's see if
> > there is another option.
> > 
> > Could we summarize the problem as:
> > - the kernel prevents writing security.ima hashes.
> > - the kernel only writes security.ima hashes for files that are in
> > policy.
> > - the userspace tool doesn't know what is in/out of policy.
> > - the userspace tool doesn't differentiate between hashes and
> > signatures.
> > - the boot process doesn't permit changing the boot command line options
> > (eg. fix mode).
> > - the update tool compares file data and metadata with those on the
> > server
> 
> Correct.

I now ran into the very same problem also in another use case, i.e. not
just during system install time.

Ostro OS allows file-based updates on a running system. bsdtar is used
to extract the new revision of each modified or new file in a staging
directory. In our policy, some files are merely hashed and not signed,
because they might be modified on the device.

Extracting such files with bsdtar also fails with:
fsetxattr(4, "security.ima", "\1\217'\taw\210\3\245MrT\351n\336j\316\7z
\212j", 21, 0) = -1 EPERM (Operation not permitted)

Given that this particular kernel check is now causing problems in two
cases, can we revisit the question whether it can be reverted?

There are alternatives to dealing with the problem, but all of them are
considerably more work and more complex.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [Linux-ima-user] copying files with security.ima xattr

[Dmitry K. <dmi...@gm...>]

Hi,


On Tue, Oct 11, 2016 at 3:24 PM, Patrick Ohly <pat...@in...> wrote:
> On Mon, 2016-07-18 at 14:21 +0200, Patrick Ohly wrote:
>> On Mon, 2016-07-18 at 08:08 -0400, Mimi Zohar wrote:
>> > Commit c68ed80 "ima: limit file hash setting by user to fix and log
>> > modes" is a form of system hardening.  Before reverting it, let's see if
>> > there is another option.
>> >
>> > Could we summarize the problem as:
>> > - the kernel prevents writing security.ima hashes.
>> > - the kernel only writes security.ima hashes for files that are in
>> > policy.
>> > - the userspace tool doesn't know what is in/out of policy.
>> > - the userspace tool doesn't differentiate between hashes and
>> > signatures.
>> > - the boot process doesn't permit changing the boot command line options
>> > (eg. fix mode).
>> > - the update tool compares file data and metadata with those on the
>> > server
>>
>> Correct.
>
> I now ran into the very same problem also in another use case, i.e. not
> just during system install time.
>
> Ostro OS allows file-based updates on a running system. bsdtar is used
> to extract the new revision of each modified or new file in a staging
> directory. In our policy, some files are merely hashed and not signed,
> because they might be modified on the device.
>
> Extracting such files with bsdtar also fails with:
> fsetxattr(4, "security.ima", "\1\217'\taw\210\3\245MrT\351n\336j\316\7z
> \212j", 21, 0) = -1 EPERM (Operation not permitted)
>
> Given that this particular kernel check is now causing problems in two
> cases, can we revisit the question whether it can be reverted?
>
> There are alternatives to dealing with the problem, but all of them are
> considerably more work and more complex.
>

Yeah. It was security hardening.
It would be great to have CAP_INTEGRITY_ADMIN or something like that.
But if it seriously harm then just revert it.

Dmitry

> --
> Best Regards, Patrick Ohly
>
> The content of this message is my personal opinion only and although
> I am an employee of Intel, the statements I make here in no way
> represent Intel's position on the issue, nor am I authorized to speak
> on behalf of Intel on this matter.
>
>
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-ima-user mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-ima-user



-- 
Thanks,
Dmitry

Re: [Linux-ima-user] copying files with security.ima xattr

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

Hi Patrick, Dmitry,

Sorry for the long delay in getting back to you ...

On Tue, 2016-10-11 at 16:15 +0300, Dmitry Kasatkin wrote:
> Hi,

> On Tue, Oct 11, 2016 at 3:24 PM, Patrick Ohly <pat...@in...> wrote:
> > On Mon, 2016-07-18 at 14:21 +0200, Patrick Ohly wrote:
> >> On Mon, 2016-07-18 at 08:08 -0400, Mimi Zohar wrote:
> >> > Commit c68ed80 "ima: limit file hash setting by user to fix and log
> >> > modes" is a form of system hardening.  Before reverting it, let's see if
> >> > there is another option.
> >> >
> >> > Could we summarize the problem as:
> >> > - the kernel prevents writing security.ima hashes.
> >> > - the kernel only writes security.ima hashes for files that are in
> >> > policy.
> >> > - the userspace tool doesn't know what is in/out of policy.
> >> > - the userspace tool doesn't differentiate between hashes and
> >> > signatures.
> >> > - the boot process doesn't permit changing the boot command line options
> >> > (eg. fix mode).
> >> > - the update tool compares file data and metadata with those on the
> >> > server
> >>
> >> Correct.
> >
> > I now ran into the very same problem also in another use case, i.e. not
> > just during system install time.
> >
> > Ostro OS allows file-based updates on a running system. bsdtar is used
> > to extract the new revision of each modified or new file in a staging
> > directory. In our policy, some files are merely hashed and not signed,
> > because they might be modified on the device.
> >
> > Extracting such files with bsdtar also fails with:
> > fsetxattr(4, "security.ima", "\1\217'\taw\210\3\245MrT\351n\336j\316\7z
> > \212j", 21, 0) = -1 EPERM (Operation not permitted)
> >
> > Given that this particular kernel check is now causing problems in two
> > cases, can we revisit the question whether it can be reverted?

In order to revert the patch, we need to explain the reason for doing
so.   Could you expand/update the two reasons given below?

- Applications have been modified to write security xattrs, but they are
not necessarily context aware.  In the case of security.ima, the
security xattr can be either a file hash or a file signature.
Permitting writing one, but not the other requires the application to be
context aware.

- Applications write files to a staging area, which might not be in
policy, and then change some file metadata (eg owner) making it in
policy.  As a result, these files are not labeled properly.

Thanks,

Mimi

> > There are alternatives to dealing with the problem, but all of them are
> > considerably more work and more complex.
> >
> 
> Yeah. It was security hardening.
> It would be great to have CAP_INTEGRITY_ADMIN or something like that.
> But if it seriously harm then just revert it.

Re: [Linux-ima-user] copying files with security.ima xattr

[Patrick O. <pat...@in...>]

On Wed, 2016-11-02 at 09:47 -0400, Mimi Zohar wrote:
> In order to revert the patch, we need to explain the reason for doing
> so.   Could you expand/update the two reasons given below?
> 
> - Applications have been modified to write security xattrs, but they are
> not necessarily context aware.  In the case of security.ima, the
> security xattr can be either a file hash or a file signature.
> Permitting writing one, but not the other requires the application to be
> context aware.
> 
> - Applications write files to a staging area, which might not be in
> policy, and then change some file metadata (eg owner) making it in
> policy.  As a result, these files are not labeled properly.

That describes it well.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [Linux-ima-user] copying files with security.ima xattr

[Patrick O. <pat...@in...>]

On Thu, 2016-11-03 at 08:58 +0100, Patrick Ohly wrote:
> On Wed, 2016-11-02 at 09:47 -0400, Mimi Zohar wrote:
> > In order to revert the patch, we need to explain the reason for doing
> > so.   Could you expand/update the two reasons given below?
> > 
> > - Applications have been modified to write security xattrs, but they are
> > not necessarily context aware.  In the case of security.ima, the
> > security xattr can be either a file hash or a file signature.
> > Permitting writing one, but not the other requires the application to be
> > context aware.
> > 
> > - Applications write files to a staging area, which might not be in
> > policy, and then change some file metadata (eg owner) making it in
> > policy.  As a result, these files are not labeled properly.
> 
> That describes it well.

Let's move this forward. Mimi, I'll send a patch to this list to keep
the discussion and the resulting code change in one place. Does that
work for you?

The patch applies cleanly to your "next" branch in
git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

[Linux-ima-user] [PATCH] Revert "ima: limit file hash setting by user to fix and log modes"

[Patrick O. <pat...@in...>]

This reverts commit c68ed80c97d9720f51ef31fe91560fdd1e121533.

The original motivation was security hardening ("File hashes are
automatically set and updated and should not be manually set.")

However, that hardening ignores and breaks some valid use cases:
- File hashes might not be set because the file is currently
  outside of the policy and therefore have to be set by the
  creator. Examples:
  - Booting into an initramfs with an IMA-enabled kernel but
    without setting an IMA policy, then installing
    the OS onto the target partition by unpacking a rootfs archive
    which has the file hashes pre-computed.
  - Unpacking a file into a staging area with meta data (like owner)
    that leaves the file outside of the current policy, then changing
    the meta data such that it becomes part of the current policy.
- "should not be set manually" implies that the creator is aware
  of IMA semantic, the current system's configuration, and then
  skips setting file hashes in security.ima if (and only if) the
  kernel would prevent it. That's not the case for standard, unmodified
  tools. Example: unpacking an archive with security.ima xattrs with
  bsdtar or GNU tar.

Signed-off-by: Patrick Ohly <pat...@in...>
---
 security/integrity/ima/ima_appraise.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 4b9b4a4..b8b2dd9 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -385,14 +385,10 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
 	result = ima_protect_xattr(dentry, xattr_name, xattr_value,
 				   xattr_value_len);
 	if (result == 1) {
-		bool digsig;
-
 		if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
 			return -EINVAL;
-		digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
-		if (!digsig && (ima_appraise & IMA_APPRAISE_ENFORCE))
-			return -EPERM;
-		ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
+		ima_reset_appraise_flags(d_backing_inode(dentry),
+			 (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0);
 		result = 0;
 	}
 	return result;
-- 
2.1.4

Re: [Linux-ima-user] copying files with security.ima xattr

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

On Tue, 2016-11-15 at 15:28 +0100, Patrick Ohly wrote:
> On Thu, 2016-11-03 at 08:58 +0100, Patrick Ohly wrote:
> > On Wed, 2016-11-02 at 09:47 -0400, Mimi Zohar wrote:
> > > In order to revert the patch, we need to explain the reason for doing
> > > so.   Could you expand/update the two reasons given below?
> > > 
> > > - Applications have been modified to write security xattrs, but they are
> > > not necessarily context aware.  In the case of security.ima, the
> > > security xattr can be either a file hash or a file signature.
> > > Permitting writing one, but not the other requires the application to be
> > > context aware.
> > > 
> > > - Applications write files to a staging area, which might not be in
> > > policy, and then change some file metadata (eg owner) making it in
> > > policy.  As a result, these files are not labeled properly.
> > 
> > That describes it well.
> 
> Let's move this forward. Mimi, I'll send a patch to this list to keep
> the discussion and the resulting code change in one place. Does that
> work for you?
> 
> The patch applies cleanly to your "next" branch in
> git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.

I didn't forget.  It's already queued in the #next-fixes branch with a
couple of other fixes.

7bbebfd4f0fc Revert "ima: limit file hash setting by user to fix and log
modes"

Before moving them to the usual #next branch, I need to speak with
Andrew Morton, who is carrying the IMA kexec patches.  Sorry for the
confusion!

Mimi