IMA/EVM writing xattrs during remount filesystem

IMA/EVM writing xattrs during remount filesystem

[Sascha Hauer]

Hi All,

When a filesystem is remounted from rw to ro then
sb_prepare_remount_readonly() is called. After this call there shouldn't
be any writers left on the filesystem. However, IMA/EVM is not aware of
this as it never calls mnt_want_write[_file](), but only looks add the
MS_RDONLY superblock flag before writing to its xattrs. This flag is
only changed after sb->s_op->remount_fs() is called. As a consequence
IMA/EVM still updates xattrs while the filesystem is going to readonly
mode.

We observed that on a 4.0 Kernel in conjunction with UBIFS, but the
relevant code in IMA/EVM still looks the same so I assume it's present
in the current kernel aswell.

UBIFS calculates its free space before and after the remount_fs op and
if there's a difference it prints a backtrace (dbg_check_space_info:
free space changed from x to y). We see this backtrace sometimes when
remounting the fs readonly. If I understand the situation correctly this
is not UBIFS's fault, right? Any hint what we can do about it?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: IMA/EVM writing xattrs during remount filesystem

[Mimi Zohar]

Hi Sascha,

On Mon, 2018-02-26 at 15:23 +0100, Sascha Hauer wrote:
> Hi All,
> 
> When a filesystem is remounted from rw to ro then
> sb_prepare_remount_readonly() is called. After this call there shouldn't
> be any writers left on the filesystem. However, IMA/EVM is not aware of
> this as it never calls mnt_want_write[_file](), but only looks add the
> MS_RDONLY superblock flag before writing to its xattrs. This flag is
> only changed after sb->s_op->remount_fs() is called. As a consequence
> IMA/EVM still updates xattrs while the filesystem is going to readonly
> mode.
> 
> We observed that on a 4.0 Kernel in conjunction with UBIFS, but the
> relevant code in IMA/EVM still looks the same so I assume it's present
> in the current kernel aswell.
> 
> UBIFS calculates its free space before and after the remount_fs op and
> if there's a difference it prints a backtrace (dbg_check_space_info:
> free space changed from x to y). We see this backtrace sometimes when
> remounting the fs readonly. If I understand the situation correctly this
> is not UBIFS's fault, right? Any hint what we can do about it?

Not updating the file hashes could result in verification errors.  I
would classify updating the xattrs as working as designed.  Wouldn't
you?

Perhaps the files changing should not be included in the IMA-appraisal 
policy?

Mimi

Re: IMA/EVM writing xattrs during remount filesystem

[Sascha Hauer]

On Mon, Feb 26, 2018 at 10:12:05AM -0500, Mimi Zohar wrote:
> Hi Sascha,
> 
> On Mon, 2018-02-26 at 15:23 +0100, Sascha Hauer wrote:
> > Hi All,
> > 
> > When a filesystem is remounted from rw to ro then
> > sb_prepare_remount_readonly() is called. After this call there shouldn't
> > be any writers left on the filesystem. However, IMA/EVM is not aware of
> > this as it never calls mnt_want_write[_file](), but only looks add the
> > MS_RDONLY superblock flag before writing to its xattrs. This flag is
> > only changed after sb->s_op->remount_fs() is called. As a consequence
> > IMA/EVM still updates xattrs while the filesystem is going to readonly
> > mode.
> > 
> > We observed that on a 4.0 Kernel in conjunction with UBIFS, but the
> > relevant code in IMA/EVM still looks the same so I assume it's present
> > in the current kernel aswell.
> > 
> > UBIFS calculates its free space before and after the remount_fs op and
> > if there's a difference it prints a backtrace (dbg_check_space_info:
> > free space changed from x to y). We see this backtrace sometimes when
> > remounting the fs readonly. If I understand the situation correctly this
> > is not UBIFS's fault, right? Any hint what we can do about it?
> 
> Not updating the file hashes could result in verification errors.  I
> would classify updating the xattrs as working as designed.  Wouldn't
> you?

If IMA updates the hash for a file that actually gets written then the
file is opened for writing. Trying remount the filesystem readonly in
this situation will return -EBUSY. Everything fine here, but look at
evm_verify_hmac(). Here a file is not necessarily opened for writing,
but if the file is signed the code replaces the signature with a HMAC by
calling evm_update_evmxattr(). This function updates the xattr even when
the VFS is in the process of remounting the filesystem readonly.

More specifically look at do_remount_sb():

>	/* If we are remounting RDONLY and current sb is read/write,
>	   make sure there are no rw files opened */
>	if (remount_ro) {
>		if (force) {
>			sb->s_readonly_remount = 1;
>			smp_wmb();
>		} else {
>			retval = sb_prepare_remount_readonly(sb);
>			if (retval)
>				return retval;
>		}
>	}

When sb_prepare_remount_readonly() succeeds there are no writers left.

>
>	if (sb->s_op->remount_fs) {
>		retval = sb->s_op->remount_fs(sb, &sb_flags, data);
>		if (retval) {
>			if (!force)
>				goto cancel_readonly;
>			/* If forced remount, go ahead despite any errors */
>			WARN(1, "forced remount of a %s fs returned %i\n",
>			     sb->s_type->name, retval);
>		}
>	}

remount_fs assumes VFS has stopped writing. At least UBIFS expects this,
maybe it's wrong:

/**
 * ubifs_remount_ro - re-mount in read-only mode.
 * @c: UBIFS file-system description object
 *
 * We assume VFS has stopped writing. Possibly the background thread could be
 * running a commit, however kthread_stop will wait in that case.
 */

>	sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (sb_flags & MS_RMT_MASK);

Here, *after* remount_fs has returned the MS_RDONLY sb flag is set which
EVM tests for before calling evm_update_evmxattr() and the race window
closes.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: IMA/EVM writing xattrs during remount filesystem

[Mimi Zohar]

On Mon, 2018-02-26 at 16:38 +0100, Sascha Hauer wrote:
> On Mon, Feb 26, 2018 at 10:12:05AM -0500, Mimi Zohar wrote:
> > Hi Sascha,
> > 
> > On Mon, 2018-02-26 at 15:23 +0100, Sascha Hauer wrote:
> > > Hi All,
> > > 
> > > When a filesystem is remounted from rw to ro then
> > > sb_prepare_remount_readonly() is called. After this call there shouldn't
> > > be any writers left on the filesystem. However, IMA/EVM is not aware of
> > > this as it never calls mnt_want_write[_file](), but only looks add the
> > > MS_RDONLY superblock flag before writing to its xattrs. This flag is
> > > only changed after sb->s_op->remount_fs() is called. As a consequence
> > > IMA/EVM still updates xattrs while the filesystem is going to readonly
> > > mode.
> > > 
> > > We observed that on a 4.0 Kernel in conjunction with UBIFS, but the
> > > relevant code in IMA/EVM still looks the same so I assume it's present
> > > in the current kernel aswell.
> > > 
> > > UBIFS calculates its free space before and after the remount_fs op and
> > > if there's a difference it prints a backtrace (dbg_check_space_info:
> > > free space changed from x to y). We see this backtrace sometimes when
> > > remounting the fs readonly. If I understand the situation correctly this
> > > is not UBIFS's fault, right? Any hint what we can do about it?
> > 
> > Not updating the file hashes could result in verification errors.  I
> > would classify updating the xattrs as working as designed.  Wouldn't
> > you?
> 
> If IMA updates the hash for a file that actually gets written then the
> file is opened for writing. Trying remount the filesystem readonly in
> this situation will return -EBUSY. Everything fine here, but look at
> evm_verify_hmac(). Here a file is not necessarily opened for writing,
> but if the file is signed the code replaces the signature with a HMAC by
> calling evm_update_evmxattr(). This function updates the xattr even when
> the VFS is in the process of remounting the filesystem readonly.
> 
> More specifically look at do_remount_sb():
> 
> >	/* If we are remounting RDONLY and current sb is read/write,
> >	   make sure there are no rw files opened */
> >	if (remount_ro) {
> >		if (force) {
> >			sb->s_readonly_remount = 1;
> >			smp_wmb();
> >		} else {
> >			retval = sb_prepare_remount_readonly(sb);
> >			if (retval)
> >				return retval;
> >		}
> >	}
> 
> When sb_prepare_remount_readonly() succeeds there are no writers left.
> 
> >
> >	if (sb->s_op->remount_fs) {
> >		retval = sb->s_op->remount_fs(sb, &sb_flags, data);
> >		if (retval) {
> >			if (!force)
> >				goto cancel_readonly;
> >			/* If forced remount, go ahead despite any errors */
> >			WARN(1, "forced remount of a %s fs returned %i\n",
> >			     sb->s_type->name, retval);
> >		}
> >	}
> 
> remount_fs assumes VFS has stopped writing. At least UBIFS expects this,
> maybe it's wrong:
> 
> /**
>  * ubifs_remount_ro - re-mount in read-only mode.
>  * @c: UBIFS file-system description object
>  *
>  * We assume VFS has stopped writing. Possibly the background thread could be
>  * running a commit, however kthread_stop will wait in that case.
>  */
> 
> >	sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (sb_flags & MS_RMT_MASK);
> 
> Here, *after* remount_fs has returned the MS_RDONLY sb flag is set which
> EVM tests for before calling evm_update_evmxattr() and the race window
> closes.

So the cause of the problem is not IMA, per se, but EVM converting the
EVM signature to an HMAC.  There's no harm in not re-writing the xattr
signature as an HMAC.  Feel free to add the additional
"s_readonly_remount" test.

During this open window, we upstreamed support for EVM portable and
immutable file signatures.  Please make sure you base the change on
the linux-integrity #next-integrity branch.

thanks,

Mimi

Re: IMA/EVM writing xattrs during remount filesystem

[Sascha Hauer]

On Mon, Feb 26, 2018 at 11:03:18AM -0500, Mimi Zohar wrote:
> >  * ubifs_remount_ro - re-mount in read-only mode.
> >  * @c: UBIFS file-system description object
> >  *
> >  * We assume VFS has stopped writing. Possibly the background thread could be
> >  * running a commit, however kthread_stop will wait in that case.
> >  */
> > 
> > >	sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (sb_flags & MS_RMT_MASK);
> > 
> > Here, *after* remount_fs has returned the MS_RDONLY sb flag is set which
> > EVM tests for before calling evm_update_evmxattr() and the race window
> > closes.
> 
> So the cause of the problem is not IMA, per se, but EVM converting the
> EVM signature to an HMAC.  There's no harm in not re-writing the xattr
> signature as an HMAC.  Feel free to add the additional
> "s_readonly_remount" test.

Ok, that should work. I'll give it some testing here before I send a
patch.

> 
> During this open window, we upstreamed support for EVM portable and
> immutable file signatures.  Please make sure you base the change on
> the linux-integrity #next-integrity branch.

sure, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |