On 28 July 2016 at 06:43, Flavio Leitner <f...@sysclose.org> wrote: > > Adding William since he is the author of commit 484371776e > > On Wed, Jul 27, 2016 at 01:54:45PM -0700, Joe Stringer wrote: >> On 26 July 2016 at 14:10, Flavio Leitner <f...@sysclose.org> wrote: >> > On Tue, Jul 26, 2016 at 12:57:07PM -0700, Joe Stringer wrote: >> >> As far as I can follow, this "domain" type is not just for accessing >> >> OVS directories and files (like openvswitch_t), but ifor a much wider >> >> range of paths: >> >> https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/4/html/SELinux_Guide/rhlcommon-section-0048.html >> >> >> >> "# The domain attribute identifies every type that can be >> >> # assigned to a process. This attribute is used in TE rules >> >> # that should be applied to all domains, e.g. permitting >> >> # init to kill all processes." >> >> >> >> Is my understanding (+documentation) correct here? Is there an similar >> > >> > Your understanding is correct. Turns out that we don't know which >> > process will be the parent, so it could bash unconfined or initrc_t >> > or in any other context (neutron?). >> > >> >> but more restrictive policy that allows ovs-vsctl to access, for >> >> example, /var/run/openvswitch/* (with var_run_openvswitch_t or >> >> similar)? Alternatively is there an example of another daemon that has >> >> a similar policy that set a precedence for writing the policy like >> >> this? >> > >> > I spent few hours on this and I couldn't find a way to restrict it >> > more that I proposed with selinux. Basically the above is an expansion >> > of the interface domain_read_all_domains_state()[1] which other >> > applications are using to read other processes states. However, that >> > seemed relatively new and probably not available on older distros, so >> > I have expanded to the relevant actions removing what we don't need. >> > >> > [1] http://danwalsh.livejournal.com/51435.html >> > >> >> Would you also be able to provide the full ovs-vsctl commandline? It >> >> was a little difficult to understand exactly what was going on during >> >> this event, or try to reproduce. >> > >> > utilities/ovs-vsctl.c:2473 >> > >> > 2472 static char * >> > 2473 vsctl_parent_process_info(void) >> > 2474 { >> > 2475 #ifdef __linux__ >> > 2476 pid_t parent_pid; >> > 2477 char *procfile; >> > 2478 struct ds s; >> > 2479 FILE *f; >> > 2480 >> > 2481 parent_pid = getppid(); >> > 2482 procfile = xasprintf("/proc/%d/cmdline", parent_pid); >> > 2483 >> > 2484 f = fopen(procfile, "r"); >> > >> > That is called from do_vsctl() to find the parent info. If you run as >> > root, then it's unconfined and it works, but it doesn't work during >> > boot time (initrc_t) for instance. >> > >> > To reproduce you just need to configure an OVS interface using ifcfg >> > with ONBOOT=yes and reboot. >> >> Hmm. So all we want to do in this case is to get the parent's name to >> log it. I'm of two minds: > > Yes, funny how that can be a pain :-) > >> 1) Be more restrictive. If this were the only place in OVS that we >> read /proc then maybe it should just print the pid on failure to read >> /proc, then we just accept that SELinux denies this access. It seems >> like it's just a niceity for prettyprinting which shouldn't affect >> actual setup/operation of OVS. While a pid is less than a name, it's >> still just as much information for cases like this where it's being >> invoked from initrc_t. > > It generates AVC messages which triggers notifications, so we actually > need to consider three options: > 1) Allow it (your option 2 below) > 2) Fix the code (proposed in your option 1) > 3) Use dontaudit to deny but not log. > > The problem is that PIDs numbers don't mean anything useful for us when > you are looking at logs from another system. Even if you are on the > system, PIDs are ephemeral and probably gone or worse, recycled.
I was primarily thinking of something like it being called from initrc_t and it printed "pid 1" then that would actually be informative. But I guess this doesn't address your "various scripts/programs are changing configuration" use case. > I think the feature has an interesting value when you consider a OSP > compute node where agents and scripts are running and configuring OVS. > I got reports like "we added an port but there is no traffic", the > next step is to make sense out of ovsdb logs and there you find > an application adding and deleting the same port. Without that, we > have the actions, but we can't tell who did it. How to find that out > then? > > If we can reproduce the issue and start to record pids and names with > another script, then it's fine, but most probably it's one of those > "happened once", "it's in production", "can't enable this or that" > kind of situation. I spoke briefly with Ben about this offline, and the way I understand it is that there were two pieces of debugging information provided around the time this patch was merged: One is to provide the commandline for the current program, and the other is to provide this parent process name. While it would be more informative to have both, the commandline is the more important one. >> 2) Be more lenient. It seems there are a few other /proc reads, >> particularly in netdev-linux.c, which I'm guessing will just need the >> same policy that you're proposing above, so we can just accept this >> policy. If someone's concerned about tightening this policy, they can >> choose not to install this package. > > IIRC, the other reads on /proc are system_u:object_r:proc_net_t:s0 so > it can be more restricted by using that and not 'domain'. This seems more reasonable. Would you follow up on this? >> I suppose that if SELinux isn't granular enough at this point then we >> could go with #2 and tighten the policy later when there are better >> attributes in place. > > As I said, the feature is interesting but only for troubleshooting > most probably, so maybe exposing more everyone because of a possible > use-case might be a bad idea after all. > > Having said that, I think the safest option is to go with your > proposal #1. OK, we can drop this patch. It's probably easier to figure out when to loosen a policy later rather than to tighten it later. > BTW, although I posted the SELinux update along with Bonding patch, > they don't depend on each other besides the context for reproducing > the selinux issue. So, I appreciate any feedback on the bonding > patch as well. Noted ;) _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
