Peter Dunlap writes: > James Carlson wrote: > > > > 957-966: this doesn't look right to me. I strongly suggest that you > > contact the TX team ([EMAIL PROTECTED] or > > [EMAIL PROTECTED]) to discuss the meaning of this > > flag. Only MAC-aware applications ought to be setting this, and I > > don't see where this code handles mandatory access controls at all. > > > > It's not as bad as it looks -- we are giving up the privilege rather > than acquiring it.
Giving it up for whom? This is blowing away the NET_MAC_AWARE flag on the proc_t associated with the current CRED(). I don't know what thread is running here, and thus I don't know what cred_t you're actually using (and thus what proc_t gets hit). If it's a kernel thread, then you're disabling the mac-aware processing for the kernel itself. If it's a user thread, then you're removing mac-aware processing for some arbitrary user process, and arbitrarily turning off a special privilege that the process once held. I don't see how manipulating the process flags or privileges from within a driver (particularly something as common and invisible to applications as a block storage driver) is a useful thing to do. It might possibly make some sense if you had your own kernel thread or special daemon process and were manipulating the flags on that, though that might beg other questions, but doing it on a thread that belongs to someone else just can't be right. Can it? This almost certainly breaks TX. What originally caused me to write a code review comment here was the phrase "[t]here seem to be some security ramifications." Seems, sir? Nay, it is; I know not "seems." (Which is another way of saying "you need at least a design level comment here; if not something more significant." Thanks to Sowmini for the Hamlet reference reminder. ;-}) > CIFS server actually does the same thing: As I noted in other cases, I don't really care who or what already does things like this. > Which if I recall correctly was causing problems binding to our > service/accept socket here: > > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/inet/tcp/tcp.c#3579 I don't see how that's the case. NET_MAC_AWARE is just the process flag. SO_MAC_EXCEPT is a specific socket flag, and is what TCP checks. > My understanding when I originally came up with this workaround (for the > CIFS server code) was that since we were using a kernel thread we got > the NET_MAC_AWARE privilege whether we wanted it or not. Typically a > socket thread would come from user space and would not have this > problem. The only components using sockfs from the kernel are iSCSI > initiator (which doesn't call soaccept), CIFS server (which uses this > workaround) and now our code. This is the sort of thing that makes me > eagerly anticipate the volo project. > > I will solicit some input from rampart-dev-team. With the explanation > above do you think its busted or is it just fishy? It looks busted to me. -- James Carlson, Solaris Networking <[EMAIL PROTECTED]> Sun Microsystems / 35 Network Drive 71.232W Vox +1 781 442 2084 MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677 _______________________________________________ networking-discuss mailing list [email protected]
