From: Bryan Steele <brynet@gmail.com>
Subject: Re: GoT Landlock fixes
To: gameoftrees@openbsd.org
Date: Thu, 10 Feb 2022 20:14:18 -0500

On Thu, Feb 10, 2022 at 08:09:05PM -0500, Bryan Steele wrote:
> On Thu, Feb 10, 2022 at 10:52:05PM +0100, Omar Polo wrote:
> > Hello,
> > 
> > Micka??l Sala??n <mic@digikod.net> writes:
> > 
> > > Hi,
> > >
> > > I noticed Omar Polo added support for Landlock to the Linux version of
> > > Game Of Trees [1]. This is great! However, the handled filesystem
> > > access is only LANDLOCK_ACCESS_FS_READ_FILE, and it will still be
> > > allowed to do multiple filesystem-related actions (e.g. write to
> > > files, remove files???). I don't know much about Game Of Trees but,
> > > according to the commit message, I think you would like to revoke any
> > > (currently supported) filesystem access. You should then add the 12
> > > remaining access rights [2]. There is also a typo in the errno check,
> > > it should be EOPNOTSUPP (not ENOTSUP). You'll find a small patch
> > > attached. Let me know if I can help.
> > >
> > > In a nutshell, the ruleset's handled_access_fs is required for
> > > backward and forward compatibility (i.e. the kernel and user space may
> > > not know each other's supported restrictions), hence the need to be
> > > explicit about the denied-by-default access rights.
> > 
> > Yes, the original diff had all the actions, but when I picked that up
> > again before it got committed I got confused and dropped the others (I
> > was sure the checks not listed there were dropped by default.)  I humbly
> > apologies to everyone for the incredibly stupid mistake, I don't have
> > any excuse.
> 
> I wouldn't be so hard on yourself, this is honestly pretty silly. If I
> understand things right, it means that if any addition access rights
> are added later on, that they would be allowed until all software has
> been explictly updated to deny it. That seems like a closed-open design,
> no?

err, fail-open.

> > Regarding the ENOTSUP/EOPNOTSUPP mistake, I thought that on linux the
> > two errno were defined to the same value, but I agree on the change
> > obviously (this point was raised by Brian too recently.)
> > 
> > Thanks a lot for reviewing the diff and fixing my mistake!
> > 
> > > Regards,
> > >  Micka??l
> > >
> > >
> > > [1]
> > > https://git.gameoftrees.org/gitweb/?p=got-portable.git;a=commit;h=97799ccd4b67a81f97039305d4fdd66588da9962
> > > [2] https://docs.kernel.org/userspace-api/landlock.html#filesystem-flags
> > >
> > > [2. text/x-patch; 0001-portable-extend-support-for-Landlock-and-fix-error-h.patch]...
> > 
> >