Re: [PATCH 1/1] ubifs: support authentication, for ro mount, when no key is given

[Torben Hohn <torben.hohn@linutronix.de>]

On Fri, Jun 26, 2020 at 06:31:20AM +0200, Sascha Hauer wrote:
> Hi Torben,
> 
> On Thu, Jun 25, 2020 at 05:59:27PM +0200, Torben Hohn wrote:
> > diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> > index 7fc2f3f07c16..ec95f1f50e5e 100644
> > --- a/fs/ubifs/super.c
> > +++ b/fs/ubifs/super.c
> > @@ -1291,6 +1291,17 @@ static int mount_ubifs(struct ubifs_info *c)
> >  			err = -EINVAL;
> >  			goto out_free;
> >  		}
> > +	} else if (c->auth_hash_name) {
> > +		if (IS_ENABLED(CONFIG_UBIFS_FS_AUTHENTICATION)) {
> > +			err = ubifs_init_authentication_read_only(c);
> > +			if (err)
> > +				goto out_free;
> > +		} else {
> > +			ubifs_err(c, "auth_hash_name, but UBIFS is built without"
> > +				  " authentication support");
> > +			err = -EINVAL;
> > +			goto out_free;
> > +		}
> >  	}
> 
> In case we don't have a key available for HMAC and can only verify the
> FS is correctly signed then we have to be sure that we are mounting
> readonly. This means the above needs an additional check for
> c->ro_mount.

Indeed, i had that check in authenticate_sb_node() in an earlier
version, and forgot to add it here.

Will do.

> 
> Once we can be sure that UBIFS is in readonly mode when we can't do HMAC
> then there's no point in adding a ubifs_authenticated_write(), because
> the places where you call it will never be hit in a readonly mounted
> filesystem.

The point is making sure, that it really is never hit in a readonly
filesystem. Now, and in the future. If we miss one point, we might
trigger the hmac code with an empty hmac. Although it might just crash.


Maybe we could generate a random key, to lessen the impact of such
an error. But i doubt that i have enough entropy to make that more than
a fig leaf.


Another topic:

Richard raised the point on irc, that in a dirty filesystem, the journal
would be replayed even in ro mode.

i dont see that happening.

--------------------------------------------------------------------------
static int mount_ubifs(struct ubifs_info *c)
{
	[...]

	if (!c->ro_mount) {

	   [...]

	} else if (c->need_recovery) {
		err = ubifs_recover_size(c, false);
		if (err)
			goto out_orphans;
	} else {
		/*
		 * Even if we mount read-only, we have to set space in GC LEB
		 * to proper value because this affects UBIFS free space
		 * reporting. We do not want to have a situation when
		 * re-mounting from R/O to R/W changes amount of free space.
		 */
		err = take_gc_lnum(c);
		if (err)
			goto out_orphans;
	}

	[...]

}
--------------------------------------------------------------------------

ubifs_recover_size() with in_place = false, does not seem to write.



> 
> Regards,
>  Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

-- 
Torben Hohn
Linutronix GmbH | Bahnhofstrasse 3 | D-88690 Uhldingen-Mühlhofen
Phone: +49 7556 25 999 18; Fax.: +49 7556 25 999 99

Hinweise zum Datenschutz finden Sie hier (Informations on data privacy 
can be found here): https://linutronix.de/kontakt/Datenschutz.php

Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen | 
Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 
806 | Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/