Subject: Re: CVS commit: [elad-kernelauth] src/sys
To: Elad Efrat <elad@netbsd.org>
From: Jason Thorpe <thorpej@shagadelic.org>
List: source-changes
Date: 03/08/2006 08:42:17
[Moving to tech-kern -- follow-ups there, please]

On Mar 7, 2006, at 9:17 PM, YAMAMOTO Takashi wrote:

>> Module Name:	src
>> Committed By:	elad
>> Date:		Tue Mar  7 23:23:56 UTC 2006
>>
>> Added Files:
>> 	src/sys/kern [elad-kernelauth]: kern_auth.c
>> 	src/sys/sys [elad-kernelauth]: kauth.h
>>
>> Log Message:
>> Add kernel authorization routines.
>
> - IMO, it's better to use "org.netbsd.kauth.generic" as TN says.

I definitely agree with Yamamoto-san here... we should use the  
"reverse DNS name" convention as well (I would like more of our  
subsystems that name things to use this convention, including  
representing dependencies within things like config(8)).

Also, please cite the TN in <sys/kauth.h>, and we should also  
describe which routines are NetBSD extensions (either permanent new  
parts of the KPI that we have created, or transitional things that  
will eventually go away...)

> - how about providing suser() as a wrapper of  
> KAUTH_GENERIC_ISSUSER? (for now?)

I think providing an suser() wrapper would be a fine idea.

> - builtin_process seems like a too generic name to me.
> - will you convert CURTAIN to KAUTH_PROCESS_CANSEE?
> - locking seems broken.  try LOCKDEBUG.
> - consider to follow our style.
> - you can use SIMPLEQ_INSERT_TAIL on an empty queue.

I haven't had a chance to review this version of the code carefully  
yet, but I will be doing so tonight, probably.  So I will probably  
have some additional feedback, as well.

I would like to thank Elad for picking up this ball and rolling with  
it, after I nudged him in this direction.  He has been working on it  
for quite a while.  Kauth will provide the foundation for some major  
improvements in NetBSD, including things like centralized file system  
operation authorization.  Kauth also provides incredible flexibility  
in how security policy is implemented, because it allows modules to  
interpose themselves in the authorization process.

-- thorpej