Big batch of KAUTH_GENERIC_ISSUSER removal

2012-02-17 Thread Elad Efrat
Hi,

The final version of what I would like to check in is available here:

http://www.netbsd.org/~elad/issuser.diff

(it's ~100k so I didn't attach it.)

It contains both earlier diffs, plus some similar, mechanical changes
to file-system code and changes to the kauth(9) man-page.

It will not be committed before 2/22 so take your time reviewing.

Elad


Veriexec lock_state misuse

2012-01-18 Thread Elad Efrat
Hi,

The current code confuses lock_state for something that it isn't. I
discussed this a while ago with hannken@ and blymn@ and came up with
the attached patch. We change the semantics to require locking and add
a KASSERT to make sure they're followed. Missing locking bits are
added in two places to comply.

I'm not seeking feedback on this one as there's more Veriexec locking
work pending. It's posted merely for the chance that someone might
want this in netbsd-6.

Elad


veriexec_lock_state.diff
Description: Binary data


Re: secmodel_register(9) API

2011-12-05 Thread Elad Efrat
I don't understand what the problems are. In any case, this does not
violate anything. The language used to describe the supposed issues
with the new API is ridiculous.

What the new API allows is interaction between secmodels that are
built by people who are not part of NetBSD and don't want their
secmodel to become part of NetBSD but do want to take advantage of
features in secmodels provided by NetBSD.

Personally I don't care if this stays or not. All I can say is that I
have not seen a single argument worthy of consideration against it and
I would strongly recommend to leave it in.

Please don't CC me any further emails about this.

Elad

On Mon, Dec 5, 2011 at 1:33 PM, Jean-Yves Migeon
jeanyves.mig...@free.fr wrote:
 (I am CC'ing Elad, as we both worked on it)

 On Mon, 5 Dec 2011 16:22:33 +0100, Christoph Badura wrote:

 That is by design.  The idea behind splitting the decision process into
 separate secmodels is to decouple the models and the decision making.


 I can't see how -- secmodels are responsible for the decision making, so we
 cannot decouple these easily.

 So, when one wants to create sysctls that can only be changed when
 securelevel is below a certain level, you end up adding more hooks to
 secmodel_securelevel(9)...


 That is intentional.  Every time a new control is added all the secmodels
 need to be examined if they need updating.

 which is problematic, because the sysctl does
 not necessarily belong to this secmodel (consider curtain and suser, as
 an exemple).


 Uh, sysctls do not belong to a secmodel.  Whatever that is supposed to
 mean.


 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.

 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.

 error = secmodel_eval(org.netbsd.secmodel.suser, is-root,
    cred, isroot, NULL);


 This one asks the suser module if the user cred is assumed to be
 root when evaluated by suser module. If the module is present, it
 will respond. If absent, the call will return an error.


 This so-called API is a complete perversion of the kauth system.  It
 pulls the implementation details that the other secmodel is supposed
 to hide and abstract away out into unrelated code again.


 That's weird: what you describe here is the situation before this patch:
 curtain and usermount were planted inside a secmodel they do not belong to
 (secmodel_suser is about making decisions about root's rights, not ordinary
 users).

 This is adding
 back all the interdependencies and knowledge about internals that kauth
 is supposed to abstract and hide.


 That's precisely what we are trying to avoid.

 Knowledge about internals were the de facto standard before: if you wanted
 to have an extension which implements a policy (users can only see state of
 their objects) with an explicit dependency on implementation internals (is
 that user privileged?), you had to put the extension inside the secmodel(9)
 that implements the privileged evaluation.

 The problem with this is that it doesn't scale.  You are just arbitrarily
 moving the code around that needs to be modified when new actions need to
 be authorized.


 No; we are allowing secmodel(9) to expose evaluation logic (a question) to
 other secmodels, so they can query for a result that depends on their
 internals.

 The ultimate decision remains in the hand of the one making the query.
 Asking a question in the form of do you allow that action to happen? is a
 plain miss-use of this API; that should indeed be done by kauth(9).

 It doesn't scale in the case when a new secmodel is introduced that
 needs to have a say on this action.


 The evaluation API is not designed to accept hooks, nor shall it be. It's
 meant for a secmodel(9) to query safely (as is: no symbol/run-time
 dependency on code being loaded) a secmodel for internal state (are you in
 this state?) or internal logic (given these credentials, do you consider
 them privileged or not?).

 And it doesn't scale when bsd44
 or suser is replace with a secmodel that defines privilege other than
 is-root.


 It should not. If there's another secmodel that appears later that redefines
 privilege, that secmodel will have to expose evaluation callback and let
 the curtain extension handle it.

 For the is-root case, the evaluation will return an error if the
 secmodel_suser(9) is not loaded. The curtain extension will then make the
 decision concerning this, and fallback to its default case.

 IAW not even does this not scale from 1 to 2.  It doesn't even scale from
 A to B.

 Args and command are 

Re: kauth and socket calls (esp. bind())

2010-04-08 Thread Elad Efrat

Thor Lancelot Simon wrote:

According to kauth(9):

 Listeners might sleep, so no locks can be held when calling
 an authorization wrapper.

According to uipc_socket.c:sobind():

 solock(so);
 error = (*so-so_proto-pr_usrreq)(so, PRU_BIND, NULL, nam,
  NULL, l);
 sounlock(so);

According to in_pcb.c:in_pcbbind():

 kauth_authorize_network(cred, KAUTH_NETWORK_BIND,
 KAUTH_REQ_NETWORK_BIND_PRIVPORT, so,
 sin, NULL)

Um.  Is it the documentation or the code which should be corrected?


The idea is to encourage developers to structure code so that kauth(9)
calls are made with ideally no locks etc. held, but like the man-page
states, kauth(9) is under development.


I'm not sure I grasp how things like the filesystem or device scopes could
even really work if you can't make kauth calls with locks held.


Which is why kauth(9) isn't yet fully integrated. (See e.g. tmpfs code
as the only file-system using kauth(9)) Perhaps you can step up to the
plate and address those issues.

-e.



Re: lower-case kernel option names

2010-04-08 Thread Elad Efrat

Masao Uebayashi wrote:

On Fri, Apr 9, 2010 at 1:16 AM, Thor Lancelot Simon t...@panix.com wrote:

I just confused myself considerably because this config file fragment
didn't work:

   no options SECMODEL_BSD44
   options SECMODEL_OVERLAY

It turns out these kernel options (in sys/conf/std) have *lowercase* names.

Why?  Shouldn't I change them?


Quick grep shows that only secmodel_* flags are lower-cased.  I think
they should be changed.


Go for it.

-e.



Re: [gsoc] syscall/libc fuzzer proposal

2010-03-20 Thread Elad Efrat
On Sat, Mar 20, 2010 at 3:24 PM, David Holland dholland-t...@netbsd.org wrote:
 On Sat, Mar 20, 2010 at 01:54:49PM -0400, Elad Efrat wrote:
 Thor Lancelot Simon wrote:
 If not, I don't think this adds any benefit to your proposal and is likely
 to simply be a distraction; I'd urge you in that case to drop it.

 Strongly seconded. There are so many great ways to improve NetBSD and
 wasting time and money on fuzzing is about as suboptimal as it gets.

 Um.

 First of all, that's not what Thor said;

Sorry? Are you saying that me agreeing with Thor that unless this
proposal shows some clear advantage over what we already have --
specifically Coverity Scan -- it should probably be dropped is not
what Thor said?

 second of all, you really
 should not be telling potential gsoc students that their project ideas
 are flatly worthless, even if your judgment were correct;

I said exactly what I think and I'll repeat it again: there are many
ways to improve NetBSD. Wasting both time and money, even if it's
someone else's, on things that aren't likely to benefit NetBSD in the
long term is a waste. There's a list of projects NetBSD's interested
in, and when someone proposes a project not on the list it should be
reviewed.

What I said is my opinion. I don't decide which projects are selected,
nor do I participate or plan to participate; it's an honest, objective
opinion.

 and third,
 I'm rather surprised that anyone who claims to work on security would
 call testing and analysis tools worthless.

I don't *claim* anything, David; I *work*, at least as opposed to,
say, assigning bugs to me, claiming for years I'll do something about
them (together with many other grand ideas) and instead fix, I dunno,
whitespace and grammar issues. Take your preaching elsewhere; I
couldn't care less.

As for the issue at hand, well, I suggest you look at what the
proposal is, what we already have for years, and draw your own
conclusions.

-e.