Re: secmodel_register(9) API

[Jean-Yves Migeon <jeanyves.migeon%free.fr@localhost>]

(removing Elad from CC)

On Tue, 6 Dec 2011 01:27:22 +0100, Christoph Badura wrote:
> secmodel_suser doesn't know about securelevel.  secmodel_securelevel
> doesn't know about root.  Complete decoupling between models.

Yep. What about secmodel_extensions?

> > Okay, let's put that differently: enabling/disabling a security
> > feature like curtain has nothing to do with secmodel_suser(9)
> > itself. Curtain is a feature that says: "only owners of an object
> > can query information about it." So securelevel, suser, or bsd44...
> > do not intervene in this process, _except_ when you are root
> > (pragmatism always add exceptions). That's it.
>
> That ("_except_ when you are root") works only by strongly coupling
> curtain with secmodel_suser.  The coupling is actually so strong that 
> you
> cannot bring yourself to consider that asking "is-root" is not the 
> only
> possible approach.
>
> OK.  Assuming that is the only possible way for the moment...
> What you really want to do amounts to calling suser_isroot.  You 
> could just
> as well make that call directly and let the linker resolve the 
> symbol.

And when the suser() module is not loaded, it will get attached 
automagically to kauth and register listeners. So you are overriding the 
current kauth(9) ACLs because you called suser_isroot() from curtain, 
without giving a chance to curtain to handle the module's absence 
gracefully.

> That way you would satisfy the requirement that secmodel_suser is 
> loaded
> in a simple way.

You are asking for trouble by loading secmodel(9) automagically.

> Instead you create a baroque interface that doesn't scale
> and ends up doing the same thing just more complex and costly.

Fine; please point me to the part of the code where you can make calls 
to functions (or fetch a symbol) found in a module(7), while still 
permitting it to fail in a graceful manner. Automagically loading 
security models is bad(tm).

> So far in my books that is more bloat for no discernable gain.
> Maybe there is some additional gain that I have missed in your 
> descriptions.
> If so, please provide concrete examples.

You have two in secmodel_extensions, heh:
http://nxr.netbsd.org/xref/src/sys/secmodel/extensions/secmodel_extensions.c#228
http://nxr.netbsd.org/xref/src/sys/secmodel/extensions/secmodel_extensions.c#390

> I think calling the introduction of a variadic function as interface 
> in
> the kernel certainly warrants describing it as baroque.

Huh? The prototype is by no means variadic:

int secmodel_eval(const char *id, const char *what, void *arg, void 
*ret);

All I see there: 4 arguments.

> > But the "am I root?" evaluation is more an exception than the rule
> > (a weak dependency). Turning curtain into a full fledged suser
> > extension because there is one slight divergence is problematic.
>
> How is it "more an exception than the rule"?  AFAICT if you need to 
> make
> an exception for root on all decisions made by curtain then "am I 
> root"
> needs to be evaluated for every call for a decision by curtain.
>
> I'm not clear what you mean by "there is one slight divergence" and 
> how it
> is problematic.  Can you explain that with a concrete example?

It's a slight divergence because you have to let curtain handle the 
exceptional condition ("caller is privileged and has the right to see 
all objects").

It's problematic because the kauth abstraction makes this leaky. Just 
provide me a patch with a solution that implements the current bsd44 
logic, which means:
- suser
- securelevel
- curtain (at least), which:
  - hides objects from people not owning them except root,
  - can be enabled whenever you want (regardless of securelevel), but 
cannot be disabled when securelevel is above 0.

Of course, try keeping the secmodel(9) separate. The point of 
abstraction is to provide interfaces, not blend everything together.

> It isn't difficult to imagine that curtain and usermount be 
> implemented
> as separate, full-fledged secmodels with no knowledge about the 
> internals
> of other secmodels.  Of course, we would need an API enhancement to 
> ask
> through the kauth system if the credential at hand is considered
> privileged.  Using that API would mean asking all active secmodels 
> whether
> they consider the credential as privileged.

See above.

FWIW, by "planting" I meant "implementing".

> Now that would be an interesting addition.  And it would let you do 
> things
> that weren't possible before.

> > Please stop with that one; me and Elad agreed right from the start
> > (I can send the private mails if you want them) that
> > secmodel_eval(9) _is_ _not_ meant as an alternative for
> > authorization call, and should never be used as such. It allows a
> > secmodel to expose internal code evaluation logic to the outside
> > world at his own discretion.
>
> I have difficulty following you.  What possible weight could you and 
> Elad
> agreeing on something beforehand have on the outcome of a technical
> discussion?  Please explain.

Let's put more pragmatism to this discussion. Propose a correct 
alternative to this, so we can compare both implementations. The patch 
shouldn't be that hard to pull out, now that 
curtain/usermount/user_set_cpu_affinity do not pollute 
suser/securelebel/bsd44 any more.

I prefer reading code with actual solutions than thoughts and words. 
Nothing is perfect right from the beginning, I know that.

You saw our solution; let's see yours.

--
Jean-Yves Migeon
jeanyves.migeon%free.fr@localhost