Re: Support for multi-position electro-mechanical keylocks

[Marc Balmer <marc%msys.ch@localhost>]

Am 02.08.2009 um 18:02 schrieb Christoph Badura:

> On Sun, Aug 02, 2009 at 11:54:09AM +0200, Marc Balmer wrote:
> > - Generic support for keylocks in the kernel.  The number of keylock
> > positions and the current keylock position can be read from the  
> > kernel
> > using two functions, userland can access them through the
> > hw.keylock.npos and hw.keylock.pos sysctl variables.
>
> Is that for a single keylock only? Why don't you attach it at
> hw.keylock.0.numpos and  hw.keylock.0.curpos instead and make it
> generic from the start?  I'm also for more descriptive names.
>
> I ask because just this week I've seen boxen with two keylocks at  
> work.

multiple keylocks is the way to go in the future, though a little  
different than what you suggest.  But before working on multiple  
keylock support, I wanted to know if the idea per se is accepted or not.

> > - secmodel_keylock, a kauth(9) security model that authorizes based  
> > on
> > the keylock "closedness".
>
> For me it is more natural to think of the lock as "unlocked" and  
> "locked" (
> or perhaps as "engaged").  To me the code would be clearer if it would
> test for the lock's state to be "locked" or "unlocked".  Whether  
> there is
> a key in the lock is IMHO irrelevant.  Therefore I'd name the variable
> "lockstate" not "kstate" which seems to indicate the state of the  
> key to
> me.

the name is not important.  it is just a variable.

> Could you describe what exactly it authorizes?  I noticed that only a
> fraction of the possible kauth actions are controlled by the secmodel.
> I think we'd need a specification of what it is supposed to control  
> before
> we can talk about the secmodel itself.

It has been modeled after secmodel_securemodel.  Think of the keylock  
as a hardware "securelevel" variable.  This is not very elaborate,  
just a start for an experiment...

> > Wheter the rightmost (default) or leftmost
> > position of the keylock means open can be controlled using the
> > security.models.keylock.order sysctl variable (access to which will  
> > be
> > protected later).
>
> How is that controlled through that sysctl variable?  I is not clear  
> to
> me.

0 means default behaviour, != 0 means reverse the order.

> > The enable this, "options KEYLOCK" and "options secmodel_keylock"  
> > must
> > be set in the kernel configuration; to use the gpiolock(4) driver att
> > a "gpiolock* at gpio?" line.
>
> This seems awkward to me.  So we need the following lines:
> options KEYLOCK
> options secmodel_keylock
> gpiolock* at gpio?
>
> It looks to me like kern_keylock.c isn't optional from either
> secmodel_keylock and gpiolock(4).  Hard to tell, though because there
> is no documentation.

kern_keylock is needed for keylock operation.  secmodel_keylock should  
be optional.  gpiolock(4) is an actual implementation of a keylock  
device.

> I think "options KEYLOCK" is unnecessary.  secmodel_keylock should  
> pull
> that file in automatically.  And so should gpiolock.

That could be fine tuned, indeed.  The idea is that keylock support is  
optional, and once keylock support is in, secmodel_keylock is optional  
as well.

> And we should, of course, get the secmodel_XXX_start() calls out of
> secmodel_bsd44_start().  That is a completely ridiculous place for  
> them
> to be.

That is not ridicoulous, that is where Elad wanted it to have for  
now.  As long as we are talking about experimental code, you should  
not classifiy it as ridicoulous, but rather make constructive  
suggestions, ok?

> I have two more questions: Why do we want that secmodel in tree?   
> And why
> isn't it a kernel module that gets loaded during system bootstrap?

Yes, I want this secmodel in tree.

> --chris