Re: [Linux-ima-user] [PATCH v3] evm: provide portable EVM signature version.

[Mikhail K. <vie...@vi...>]

В Wed, 11 Jan 2017 18:31:02 -0500
Stefan Berger <st...@li...> пишет:

> On 01/11/2017 06:18 PM, Mikhail Kurinnoi wrote:
> > В Wed, 11 Jan 2017 15:15:13 -0500
> > Stefan Berger <st...@li...> пишет:
> >  
> >> On 01/09/2017 08:29 PM, Mikhail Kurinnoi wrote:  
> >>> В Mon, 09 Jan 2017 18:11:01 -0500
> >>> Mimi Zohar <zo...@li...> пишет:
> >>>     
> >>>> On Tue, 2017-01-10 at 00:06 +0300, Mikhail Kurinnoi wrote:  
> >>>>> В Mon, 09 Jan 2017 08:40:45 -0500
> >>>>> Mimi Zohar <zo...@li...> пишет:
> >>>>>         
> >>>>>> On Sat, 2017-01-07 at 01:23 +0300, Mikhail Kurinnoi wrote:  
> >>>>>>> version 3 changes:
> >>>>>>> 1) Prevent ima_fix_xattr() use __vfs_setxattr_noperm() if EVM
> >>>>>>> signature verification failed.  
> >>>>>> This can't be right.  The boot comnand line option
> >>>>>> "ima_appraise=fix" allows IMA to go into a special mode to
> >>>>>> "fix" the xattrs.  
> >>>>>>> Since, __vfs_setxattr_noperm() don't count on
> >>>>>>> evm_inode_setxattr(), during file unpacking from atchive EVM
> >>>>>>> zero-filled sign could be updated by IMA without EVM signature
> >>>>>>> status checking. So, even if you have EVM portable signature
> >>>>>>> signed by another key, IMA will update EVM signature to HMAC
> >>>>>>> any way, no matter what you have in security.evm xattr or you
> >>>>>>> don't have it at all.  
> >>>>>> EVM and IMA are two separate modules, which communicate with
> >>>>>> each other via the evm_verifyxattr() API.  Changes to EVM,
> >>>>>> should not affect IMA. If they do, then something is not
> >>>>>> right.  
> >>>>> Something is wrong, because if we have FS mounted with iversion
> >>>>> flag, we call ima_file_free() -> ima_check_last_writer(), that
> >>>>> don't care about evm_verifyxattr() API at all.  
> >>>>    This is working as designed.  The file integrity verification
> >>>> occurred on file open.  On __fput() the file hash is updated,
> >>>> which causes security.evm to be updated.  
> >>> I see.
> >>>
> >>> Probably, we could add into evm_ima_xattr_type enumeration
> >>> EVM_XATTR_PROTECTED, that we could use as EVM xattr signature type
> >>> if we have EVM portable signature verification fail, that will be
> >>> used only for "security.evm" attr, as additional check with
> >>> evm_protected_xattr() in evm_inode_post_setxattr() and
> >>> evm_inode_post_removexattr(). In this way we have INTEGRITY_FAIL
> >>> in evm_verify_hmac(), since evm_verify_hmac() have no idea about
> >>> EVM_XATTR_PROTECTED signature type, and prevent EVM xattr update
> >>> on __fput() if we use IMA xattr hash but not IMA_DIGSIG.
> >>>
> >>> Is it possible?
> >>>
> >>>     
> >>>>> I have this issue without IMA/EVM fix mode. I mean, I faced with
> >>>>> this issue on IMA/EVM normal mode during archive unpacking with
> >>>>> EVM portable signature signed by wrong cert, that was replaced
> >>>>> on EVM zero-filled signature with evm_reset_status(). And after
> >>>>> that IMA update inode EVM xattr with proper HMAC by
> >>>>> ima_file_free() call.  
> >>>>>>> 2) Added integrity_audit_msg() in order to get some error in
> >>>>>>> audit syslog on EVM portable signature verification fail.
> >>>>>>>
> >>>>>>> version 2 changes:
> >>>>>>> 1) evm_verify_hmac() prevent work with EVM portable signature.
> >>>>>>> If file signed by EVM portable signature, evm_verify_hmac()
> >>>>>>> return INTEGRITY_FAIL;
> >>>>>>> 2) evm_inode_post_setxattr() now count on EVM portable
> >>>>>>> signature.  
> >>>>>> I originally suggested that the portable EVM xattr should be
> >>>>>> converted before it is written (in evm_inode_post_setxattr()),
> >>>>>> but by the time we get to evm_inode_post_setxattr(),  the
> >>>>>> portable EVM xattr has already been written.  Can the
> >>>>>> verification be done earlier in evm_inode_setxattr(), before
> >>>>>> the portable EVM xattr is written?  
> >>>>> We could do everything in evm_inode_setxattr() and return some
> >>>>> special error code into vfs_setxattr() that will:
> >>>>> 1) don't allow call __vfs_setxattr_noperm();
> >>>>> 2) prevent special error code return and return 0 instead in the
> >>>>> same time.  
> >>>> I'm wondering if there will be an issue with the order in which
> >>>> the security xattrs are installed.   For example, what
> >>>> guarantees that security.ima is installed before security.evm?  
> >>> This is archiver's part of work. As I wrote before (and you also
> >>> could see this in my patch for tar), archiver must copy EVM xattr
> >>> the last one. We have nothing to do with this in kernel.  
> >> I didn't follow the entire discussion. But that this xattr needs to
> >> be written last was kind of what I was looking for.
> >>
> >> So now you are writing security.evm with a portable digital
> >> signature. I suppose that this involves signature verification by
> >> the kernel. What happens if the signature cannot be verified, e.g.,
> >> target doesn't have the public key. How does the kernel react?  
> > Kernel will react in the same way as for file signed by standard EVM
> > digsig without public key loaded - verification will be failed.
> > Plus, kernel will install EVM "immutable" signature with invalid
> > payload that will never changed to HMAC.  
> 
> What does that mean now? Can I access that file or do I have to
> remove it ?

You can't access that file, you be able only remove it.


> >
> > If you have target system without public key and you have archive
> > with "portable" "security.evm", you could use "exclude" archiver's
> > flag for "security.evm" xattr during file extracting. In this way,
> > you will have all xattrs installed and HMAC (generated by EVM).  
> 
> So at least the following applications likely need to learn about
> this new feature:
> rsync, libarchive (bsdtar), GNU tar, libattr (copying xattrs when 
> copying files).

Yes, we need application support for sure. But we don't need portable
signature support during file copying.

The main support for EVM portable signature should be realized in
archivers. You don't need EVM portable signature support during file
copying, all you need in this case - don't allow any
"security.evm" be copyed, since on source you will have HMAC or
EVM digsig connected to inode. No reason convert HMAC > portable >
HMAC if we do this at realtime, dest. will have proper HMAC for sure
or you will see an error (if you can't copy some xattr,
mode/gid/uid...). EVM portable digsig aimed to protect archived data
from manipulation.


> >
> > We could have a lot of options here in archiver. We could "exclude"
> > or "demand" "security.evm" xattr depending of what we need for
> > target system.
> >  
> I guess several apps may need to have such a flag then...
> 

Exactly.


-- 
Best regards,
Mikhail Kurinnoi