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

[Torben Hohn]

On Fri, Jun 26, 2020 at 10:10:28AM +0200, Sascha Hauer wrote:
> On Fri, Jun 26, 2020 at 09:27:14AM +0200, Torben Hohn wrote:
> > 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.
> 
> If that's your point then you can add a ubifs_assert(c, c->ro_mount) at
> those places. This has the advantage that it triggers not only in
> authenticated mode, but also in unauthenticated mode. Please add this
> assertion explicitly and not indirectly in ubifs_authenticated_write().
> This function has a strange semantics, the name suggests that it returns
> the status of authenticated write. It's quite unexpected to me that it
> triggers a warning when called with only readonly authentication
> available.

Ok. will do.


> 
> 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