Thor Lancelot Simon wrote:

> Unfortunately, the only correct thing for the driver to do, given our
> current kauth support for passthru ioctls, is to ask the secmodel
> whether it can do a "passthru" request or not.  If the secmodel is to
> respond in any safe way that is not simply "no" then it (the secmodel!)
> must itself parse the device-specific command structure, which seems
> like a poor idea: we will be unable to do many useful things without
> embedding code that knows about device internals in the default secmodel.

yep. that's certainly an issue, one we've seen before: network
interfaces. for those, what I did to be able to provide a thin layer
of abstraction -- yet allow the secmodel make up basic decisions -- was
pass a request indicating the generic type of the request.

so we introduced, as sub-actions of KAUTH_NETWORK_INTERFACE, requests
to get/set/get-privileged/set-privileged information for the interface.

similarly, we have read/write/read-write requests to indicate specfs
access requested as well.

> One dreadful workaround would be to have the driver lie about what the
> request is, and not ask the passthru kauth question if it believes the
> request is "safe".  I think this is not something we should do.

absolutely not.

> I think the right solution is to add more information to the passthru
> question, so that the listener can distinguish diferent kinds of
> requests without parsing them if it does not know how.  This leaves,
> by default, the code that knows how the device commands work in the
> device driver, which is where it belongs.  I propose that we
> distinguish passthru commands by their worst possible consequences
> (device drivers that cannot parse a given passthru command should
> treat it as the most damaging command possible) into (at least) these
> types: READ, WRITE, READCONF, WRITECONF where the latter two indicate
> commands that can read (or write) device configuration/metadata rather
> than data stored on the device or in host memory.  I am not sure that
> WRITECONF really needs to be separated from WRITE but I am sure that
> READCONF does, in order to accurately describe the MegaRAID command
> set and the security implications of using particular commands.
> I am proposing WRITECONF pretty much for orthagonality.

unfortunately, the current interface, kauth_authorize_device_passthru(),
takes credentials, device identifier, and void * for the command itself.
we should add a 'req' with the indicators you suggested above.

I would suggest considering falling back to the standard authorization
wrapper for this scope -- but I will not object to just adding one more
variable to the existing function.

> These could also be bits in a word, for more flexibility in describing
> commands.  I'm not sure if this is better, but it would make determining
> what the "worst possible" is much easier!
> 
> Opinions?

this is very easy; reach consensus and I'll do it.

-e.

-- 
Elad Efrat